Skip to content

Commit

Permalink
fix(*): patch expanduser to be more friendly to OpenResty environment (
Browse files Browse the repository at this point in the history
…#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.
  • Loading branch information
windmgc authored Apr 19, 2024
1 parent 1f3b8bb commit fad696b
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 18 deletions.
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 ~
Expand Down
1 change: 1 addition & 0 deletions spec/01-generic/01-config_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
35 changes: 27 additions & 8 deletions spec/03-credentials/04-RemoteCredentials_spec.lua
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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")

Expand All @@ -55,6 +55,7 @@ describe("RemoteCredentials", function()
end)

after_each(function()
old_pl_utils.readfile = pl_utils_readfile
restore()
end)

Expand Down Expand Up @@ -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

Expand All @@ -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()
Expand All @@ -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

Expand All @@ -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()
Expand Down
8 changes: 6 additions & 2 deletions spec/03-credentials/08-SharedFileCredentials_spec.lua
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
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 = {}

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)
Expand All @@ -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)

Expand Down
93 changes: 85 additions & 8 deletions src/resty/aws/config.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit fad696b

Please sign in to comment.