From fad696b92654eb683ab832c720114c0341f87b7a Mon Sep 17 00:00:00 2001 From: Keery Nie Date: Fri, 19 Apr 2024 13:23:43 +0800 Subject: [PATCH] fix(*): patch expanduser to be more friendly to OpenResty environment (#111) pl.path.expanduser may return with nil, err when received a path argument starting with ~ but $HOME(or other home path related) environment variable is not present. This PR adds proper error handling code when calling expanduser. --- README.md | 6 ++ spec/01-generic/01-config_spec.lua | 1 + .../04-RemoteCredentials_spec.lua | 35 +++++-- .../08-SharedFileCredentials_spec.lua | 8 +- src/resty/aws/config.lua | 93 +++++++++++++++++-- 5 files changed, 125 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 5834092..e6694df 100644 --- a/README.md +++ b/README.md @@ -176,6 +176,12 @@ Release process: 1. upload using: `VERSION=x.y.z APIKEY=abc... make upload` 1. test installing the rock from LuaRocks + +### Unreleased + +- fix: patch expanduser function to be more friendly to OpenResty environment + [111](https://github.com/Kong/lua-resty-aws/pull/111) + ### 1.4.0 (20-Mar-2024) - fix: aws configuration cannot be loaded due to pl.path cannot resolve the path started with ~ diff --git a/spec/01-generic/01-config_spec.lua b/spec/01-generic/01-config_spec.lua index 13b39f2..cd36c6c 100644 --- a/spec/01-generic/01-config_spec.lua +++ b/spec/01-generic/01-config_spec.lua @@ -19,6 +19,7 @@ describe("config loader", function() local config before_each(function() restore() + restore.setenv("HOME") pl_utils.writefile(config_filename, config_info) end) diff --git a/spec/03-credentials/04-RemoteCredentials_spec.lua b/spec/03-credentials/04-RemoteCredentials_spec.lua index 1d9dfc5..c77d604 100644 --- a/spec/03-credentials/04-RemoteCredentials_spec.lua +++ b/spec/03-credentials/04-RemoteCredentials_spec.lua @@ -1,6 +1,7 @@ local json = require("cjson.safe").new() local restore = require "spec.helpers" +local old_pl_utils = require("pl.utils") -- Mock for HTTP client local response = {} -- override in tests @@ -33,18 +34,17 @@ local http = { end, } -local pl_utils = { - readfile = function() - return "testtokenabc123" - end -} - describe("RemoteCredentials", function() local RemoteCredentials + local pl_utils_readfile = old_pl_utils.readfile before_each(function() + pl_utils_readfile = old_pl_utils.readfile + old_pl_utils.readfile = function() + return "testtokenabc123" + end restore() restore.setenv("AWS_CONTAINER_CREDENTIALS_FULL_URI", "https://localhost/test/path") @@ -55,6 +55,7 @@ describe("RemoteCredentials", function() end) after_each(function() + old_pl_utils.readfile = pl_utils_readfile restore() end) @@ -96,6 +97,16 @@ describe("RemoteCredentials with customized full URI", function () end) describe("RemoteCredentials with full URI and token file", function () + local pl_utils_readfile + before_each(function() + pl_utils_readfile = old_pl_utils.readfile + old_pl_utils.readfile = function() + return "testtokenabc123" + end + end) + after_each(function() + old_pl_utils.readfile = pl_utils_readfile + end) it("fetches credentials", function () local RemoteCredentials @@ -105,7 +116,6 @@ describe("RemoteCredentials with full URI and token file", function () local _ = require("resty.aws.config").global -- load config before mocking http client package.loaded["resty.luasocket.http"] = http - package.loaded["pl.utils"] = pl_utils RemoteCredentials = require "resty.aws.credentials.RemoteCredentials" finally(function() @@ -125,6 +135,16 @@ describe("RemoteCredentials with full URI and token file", function () end) describe("RemoteCredentials with full URI and token and token file, file takes higher precedence", function () + local pl_utils_readfile + before_each(function() + pl_utils_readfile = old_pl_utils.readfile + old_pl_utils.readfile = function() + return "testtokenabc123" + end + end) + after_each(function() + old_pl_utils.readfile = pl_utils_readfile + end) it("fetches credentials", function () local RemoteCredentials @@ -135,7 +155,6 @@ describe("RemoteCredentials with full URI and token and token file, file takes h local _ = require("resty.aws.config").global -- load config before mocking http client package.loaded["resty.luasocket.http"] = http - package.loaded["pl.utils"] = pl_utils RemoteCredentials = require "resty.aws.credentials.RemoteCredentials" finally(function() diff --git a/spec/03-credentials/08-SharedFileCredentials_spec.lua b/spec/03-credentials/08-SharedFileCredentials_spec.lua index 0b70b78..e37c7f9 100644 --- a/spec/03-credentials/08-SharedFileCredentials_spec.lua +++ b/spec/03-credentials/08-SharedFileCredentials_spec.lua @@ -1,6 +1,7 @@ local pl_path = require "pl.path" local pl_config = require "pl.config" local tbl_clear = require "table.clear" +local restore = require "spec.helpers" local hooked_file = {} @@ -8,11 +9,11 @@ local origin_read = pl_config.read local origin_isfile = pl_path.isfile pl_config.read = function(name, ...) - return hooked_file[pl_path.expanduser(name)] or origin_read(name, ...) + return hooked_file[name] or origin_read(name, ...) end pl_path.isfile = function(name) - return hooked_file[pl_path.expanduser(name)] and true or origin_isfile(name) + return hooked_file[name] and true or origin_isfile(name) end local function hook_config_file(name, content) @@ -24,12 +25,15 @@ describe("SharedFileCredentials_spec", function() local SharedFileCredentials_spec before_each(function() + -- make ci happy + restore.setenv("HOME", "/home/ci-test") local _ = require("resty.aws.config").global -- load config before anything else SharedFileCredentials_spec = require "resty.aws.credentials.SharedFileCredentials" end) after_each(function() + restore() tbl_clear(hooked_file) end) diff --git a/src/resty/aws/config.lua b/src/resty/aws/config.lua index d2942d5..f56a42d 100644 --- a/src/resty/aws/config.lua +++ b/src/resty/aws/config.lua @@ -63,7 +63,12 @@ local pl_path = require "pl.path" local pl_config = require "pl.config" +local pl_utils = require 'pl.utils' +local sub = string.sub +local os_getenv = os.getenv +local assert_string = pl_utils.assert_string +local is_windows = pl_utils.is_windows -- Convention: variable values are stored in the config table by the name of @@ -154,6 +159,14 @@ local env_vars = { HTTP_PROXY = { name = "http_proxy", default = nil }, HTTPS_PROXY = { name = "https_proxy", default = nil }, NO_PROXY = { name = "no_proxy", default = nil }, + + -- Environment variables for expanding user home path + -- Nix specific + HOME = { name = "HOME", default = nil }, + -- Windows specific + USERPROFILE = { name = "USERPROFILE", default = nil }, + HOMEPATH = { name = "HOMEPATH", default = nil }, + HOMEDRIVE = { name = "HOMEDRIVE", default = nil }, } -- populate the env vars with their values, or defaults @@ -173,18 +186,72 @@ local config = { env_vars = env_vars } + +--- Returns the environment variable value or the cached +--- environment variable value in the `env_vars` table. +-- @string var_name The environment variable name +-- @treturn[1] string The environment variable value or the cached value in `env_vars` table +-- @treturn[2] nil If the environment variable is not set and the cached value is not available +local function getenv(var_name) + return os_getenv(var_name) or env_vars[var_name].value +end + + +--- Replace a starting '~' with the user's home directory. +--- This is a patched version of the original `pl.path.expanduser` function. +--- In lua-resty-aws the environment variables must be fetched in `init_phase` +--- So we need to cache those home path related values and fall back to them +--- if expanduser function failed to fetch the environment variables. +-- @string P A file path +-- @treturn[1] string The file path with the `~` prefix substituted, or the input path if it had no prefix. +-- @treturn[2] nil +-- @treturn[2] string Error message if the environment variables were unavailable. +local function expanduser(P) + assert_string(1,P) + if P:sub(1,1) ~= '~' then + return P + end + + local home = getenv('HOME') + if (not home) and (not is_windows) then + -- no more options to try on Nix + return nil, "failed to expand '~' (HOME not set)" + end + + if (not home) then + -- try alternatives on Windows + home = getenv('USERPROFILE') + if not home then + local hd = getenv('HOMEDRIVE') + local hp = getenv('HOMEPATH') + if not (hd and hp) then + return nil, "failed to expand '~' (HOME, USERPROFILE, and HOMEDRIVE and/or HOMEPATH not set)" + end + home = hd..hp + end + end + + return home..sub(P,2) +end + do -- load a config file. If section given returns section only, otherwise full file. -- returns an empty table if the section does not exist local function load_file(filename, section) assert(type(filename) == "string", "expected filename to be a string") - if not pl_path.isfile(pl_path.expanduser(filename)) then + + local expanded_filename, err = expanduser(filename) + if not expanded_filename then + return nil, "failed expanding path '"..filename.."': "..tostring(err) + end + + if not pl_path.isfile(expanded_filename) then return nil, "not a file: '"..filename.."'" end - local contents, err = pl_config.read(filename, { variabilize = false }) + local contents, err = pl_config.read(expanded_filename, { variabilize = false }) if not contents then - return nil, "failed reading file '"..filename.."': "..tostring(err) + return nil, "failed reading file '"..filename.."'(expanded: '"..expanded_filename.."'): "..tostring(err) end if not section then @@ -193,11 +260,11 @@ do assert(type(section) == "string", "expected section to be a string or falsy") if not contents[section] then - ngx.log(ngx.DEBUG, "section '",section,"' does not exist in file '",filename,"'") + ngx.log(ngx.DEBUG, "section '",section,"' does not exist in file '",filename,"'(expanded: '"..expanded_filename.."')") return {} end - ngx.log(ngx.DEBUG, "loaded section '",section,"' from file '",filename,"'") + ngx.log(ngx.DEBUG, "loaded section '",section,"' from file '",filename,"'(expanded: '"..expanded_filename.."')") return contents[section] end @@ -237,7 +304,9 @@ end -- table if the config file does not exist. -- @return options table as gotten from the configuration file, or nil+err. function config.load_config() - if not pl_path.isfile(pl_path.expanduser(env_vars.AWS_CONFIG_FILE.value)) then + local expanded_path, err = expanduser(env_vars.AWS_CONFIG_FILE.value) + if not (expanded_path and pl_path.isfile(expanded_path)) then + ngx.log(ngx.DEBUG, "failed to expand config file path or file does not exist: ", err) -- file doesn't exist return {} end @@ -252,11 +321,15 @@ end -- @return credentials table as gotten from the credentials file, or a table -- with the key, id, and token from the configuration file, table can be empty. function config.load_credentials() - if pl_path.isfile(pl_path.expanduser(env_vars.AWS_SHARED_CREDENTIALS_FILE.value)) then + local expanded_path, err = expanduser(env_vars.AWS_SHARED_CREDENTIALS_FILE.value) + if expanded_path and pl_path.isfile(expanded_path) then local creds = config.load_credentials_file(env_vars.AWS_SHARED_CREDENTIALS_FILE.value, env_vars.AWS_PROFILE.value) if creds then -- ignore error, already logged return creds end + + else + ngx.log(ngx.DEBUG, "failed to expand credential file path or file does not exist: ", err) end -- fall back to config file @@ -288,7 +361,8 @@ end function config.get_config() local cfg = config.load_config() or {} -- ignore error, already logged - if pl_path.isfile(pl_path.expanduser(env_vars.AWS_SHARED_CREDENTIALS_FILE.value)) then + local expanded_path, err = expanduser(env_vars.AWS_SHARED_CREDENTIALS_FILE.value) + if expanded_path and pl_path.isfile(expanded_path) then -- there is a creds file, so override creds with creds file local creds = config.load_credentials_file( env_vars.AWS_SHARED_CREDENTIALS_FILE.value, env_vars.AWS_PROFILE.value) -- ignore error, already logged @@ -297,6 +371,9 @@ function config.get_config() cfg.aws_secret_access_key = creds.aws_secret_access_key cfg.aws_session_token = creds.aws_session_token end + + else + ngx.log(ngx.DEBUG, "failed to expand credential file path or file does not exist: ", err) end -- add environment variables