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.

Author: windmgc

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 ~

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)
 

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()

spec/03-credentials/08-SharedFileCredentials_spec.lua

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

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