fix(vault): allow arrays in conf loader to be referenced

### Summary Some properties, like `KONG_SSL_CERT` and `KONG_SSL_CERT_KEY` are arrays and users can specify many. Vaults didn't work in this scenario: For example below didn't work before: ``` CERT_1=$(<cert1.crt) \ KEY_1=$(<key1.key) \ CERT_2=$(<cert2.crt) \ KEY_2=$(<key2.key) \ KONG_SSL_CERT_KEY="{vault://env/key-1},{vault://env/key-2}" \ KONG_SSL_CERT="{vault://env/cert-1},{vault://env/cert-2}" \ kong prepare --vv ``` There were also erroneous warning in logs like because of bad array handling: ``` [warn] 680#0: [kong] vault.lua:1475 error caching secret reference {vault://env/cert-1}: bad value type ``` This fixes those. Signed-off-by: Aapo Talvensaari <[email protected]>
Kong · Jul 5, 2024 · ae30a71 · ae30a71
1 parent 8f9b82d
commit ae30a71
Show file tree

Hide file tree

Showing 9 changed files with 144 additions and 67 deletions.
diff --git a/kong/cmd/stop.lua b/kong/cmd/stop.lua
@@ -22,7 +22,7 @@ local function execute(args, opts)
   -- load <PREFIX>/kong.conf containing running node's config
   local conf = assert(conf_loader(default_conf.kong_env, {
     prefix = args.prefix
-  }))
+  }, { stopping = true }))
   assert(nginx_signals.stop(conf))
 
   if opts.quiet then

diff --git a/kong/cmd/utils/inject_confs.lua b/kong/cmd/utils/inject_confs.lua
@@ -20,7 +20,9 @@ local function load_conf(args)
 
   if pl_path.exists(conf.kong_env) then
     -- load <PREFIX>/kong.conf containing running node's config
-    conf = assert(conf_loader(conf.kong_env))
+    conf = assert(conf_loader(conf.kong_env, {
+      prefix = conf.prefix
+    }))
   end
 
   -- make sure necessary files like `.ca_combined` exist

diff --git a/kong/cmd/utils/prefix_handler.lua b/kong/cmd/utils/prefix_handler.lua
@@ -28,6 +28,7 @@ local nginx_signals = require "kong.cmd.utils.nginx_signals"
 
 local strip = require("kong.tools.string").strip
 local split = require("kong.tools.string").split
+local shallow_copy = require("kong.tools.table").shallow_copy
 
 
 local getmetatable = getmetatable
@@ -822,21 +823,32 @@ local function prepare_prefix(kong_config, nginx_custom_template_path, skip_writ
     "",
   }
 
-  local refs = kong_config["$refs"]
-  local has_refs = refs and type(refs) == "table"
-
-  local secrets
-  if write_process_secrets and has_refs then
-    secrets = process_secrets.extract(kong_config)
-  end
-
   local function quote_hash(s)
     return s:gsub("#", "\\#")
   end
 
+  local refs = kong_config["$refs"]
+  local has_refs = refs and type(refs) == "table"
+  local secrets = process_secrets.extract(kong_config)
+
   for k, v in pairs(kong_config) do
+    -- do not output secrets in .kong_env
     if has_refs and refs[k] then
-      v = refs[k]
+      local ref = refs[k]
+      if type(ref) == "table" then
+        if type(v) ~= "table" then
+          v = { v }
+        else
+          v = shallow_copy(v)
+        end
+
+        for i, r in pairs(ref) do
+          v[i] = r
+        end
+
+      elseif ref then
+        v = ref
+      end
     end
 
     if type(v) == "table" then
@@ -862,7 +874,7 @@ local function prepare_prefix(kong_config, nginx_custom_template_path, skip_writ
     prepare_prefixed_interface_dir("/usr/local/kong", "gui", kong_config)
   end
 
-  if secrets then
+  if secrets and write_process_secrets then
     secrets, err = process_secrets.serialize(secrets, kong_config.kong_env)
     if not secrets then
       return nil, err

diff --git a/kong/cmd/utils/process_secrets.lua b/kong/cmd/utils/process_secrets.lua
@@ -11,6 +11,7 @@ local fmt = string.format
 local sub = string.sub
 local type = type
 local pairs = pairs
+local shallow_copy = require("kong.tools.table").shallow_copy
 
 
 local CIPHER_ALG = "aes-256-gcm"
@@ -64,7 +65,7 @@ local function extract(conf)
 
   local secrets = {}
   for k in pairs(refs) do
-    secrets[k] = conf[k]
+    secrets[k] = shallow_copy(conf[k])
   end
 
   return secrets

diff --git a/kong/conf_loader/constants.lua b/kong/conf_loader/constants.lua
@@ -574,6 +574,7 @@ local CONF_SENSITIVE = {
   admin_gui_ssl_cert_key = true,
   status_ssl_cert_key = true,
   debug_ssl_cert_key = true,
+  ["$refs"] = true, -- for internal use only, no need to log
 }
 
 

diff --git a/kong/conf_loader/init.lua b/kong/conf_loader/init.lua
@@ -313,7 +313,7 @@ local function load(path, custom_conf, opts)
         return nil, err
       end
 
-      for k, v in pairs(env_vars) do
+      for k in pairs(env_vars) do
         local kong_var = match(lower(k), "^kong_(.+)")
         if kong_var then
           -- the value will be read in `overrides()`
@@ -360,7 +360,7 @@ local function load(path, custom_conf, opts)
   -- remove the unnecessary fields if we are still at the very early stage
   -- before executing the main `resty` cmd, i.e. still in `bin/kong`
   if opts.pre_cmd then
-    for k, v in pairs(conf) do
+    for k in pairs(conf) do
       if not conf_constants.CONF_BASIC[k] then
         conf[k] = nil
       end
@@ -396,65 +396,122 @@ local function load(path, custom_conf, opts)
 
     loaded_vaults = setmetatable(vaults, conf_constants._NOP_TOSTRING_MT)
 
-    if get_phase() == "init" then
-      local secrets = getenv("KONG_PROCESS_SECRETS")
-      if secrets then
-        unsetenv("KONG_PROCESS_SECRETS")
-
-      else
-        local path = pl_path.join(abspath(ngx.config.prefix()), unpack(conf_constants.PREFIX_PATHS.kong_process_secrets))
-        if exists(path) then
-          secrets, err = pl_file.read(path, true)
-          pl_file.delete(path)
-          if not secrets then
-            return nil, fmt("failed to read process secrets file: %s", err)
+    -- collect references
+    local is_reference = require "kong.pdk.vault".is_reference
+    for k, v in pairs(conf) do
+      local typ = (conf_constants.CONF_PARSERS[k] or {}).typ
+      v = parse_value(v, typ == "array" and "array" or "string")
+      if typ == "array" then
+        local found
+
+        for i, r in ipairs(v) do
+          if is_reference(r) then
+            found = true
+            if not refs then
+              refs = setmetatable({}, conf_constants._NOP_TOSTRING_MT)
+            end
+            if not refs[k] then
+              refs[k] = {}
+            end
+            refs[k][i] = r
           end
         end
+
+        if found then
+          conf[k] = v
+        end
+
+      elseif is_reference(v) then
+        if not refs then
+          refs = setmetatable({}, conf_constants._NOP_TOSTRING_MT)
+        end
+        refs[k] = v
       end
+    end
 
-      if secrets then
-        secrets, err = process_secrets.deserialize(secrets, path)
+    local prefix = abspath(conf.prefix or ngx.config.prefix())
+    local secret_env
+    local secret_file
+    local secrets = getenv("KONG_PROCESS_SECRETS")
+    if secrets then
+      secret_env = "KONG_PROCESS_SECRETS"
+
+    else
+      local secrets_path = pl_path.join(prefix, unpack(conf_constants.PREFIX_PATHS.kong_process_secrets))
+      if exists(secrets_path) then
+        secrets, err = pl_file.read(secrets_path, true)
         if not secrets then
-          return nil, err
+          pl_file.delete(secrets_path)
+          return nil, fmt("failed to read process secrets file: %s", err)
         end
+        secret_file = secrets_path
+      end
+    end
 
-        for k, deref in pairs(secrets) do
-          local v = parse_value(conf[k], "string")
-          if refs then
-            refs[k] = v
-          else
-            refs = setmetatable({ [k] = v }, conf_constants._NOP_TOSTRING_MT)
-          end
+    if secrets then
+      local env_path = pl_path.join(prefix, unpack(conf_constants.PREFIX_PATHS.kong_env))
+      secrets, err = process_secrets.deserialize(secrets, env_path)
+      if not secrets then
+        return nil, err
+      end
+
+      -- TODO: remember!
+      for k, deref in pairs(secrets) do
+        conf[k] = deref
+      end
+    end
 
-          conf[k] = deref
+    if get_phase() == "init" then
+      if secrets then
+        if secret_env then
+          C.unsetenv(secret_env)
+        end
+        if secret_file then
+          pl_file.delete(secret_file)
         end
       end
 
-    else
+    elseif refs then
       local vault_conf = { loaded_vaults = loaded_vaults }
       for k, v in pairs(conf) do
         if sub(k, 1, 6) == "vault_" then
           vault_conf[k] = parse_value(v, "string")
         end
       end
-
       local vault = require("kong.pdk.vault").new({ configuration = vault_conf })
-
-      for k, v in pairs(conf) do
-        v = parse_value(v, "string")
-        if vault.is_reference(v) then
-          if refs then
-            refs[k] = v
-          else
-            refs = setmetatable({ [k] = v }, conf_constants._NOP_TOSTRING_MT)
+      for k, v in pairs(refs) do
+        if type(v) == "table" then
+          for i, r in pairs(v) do
+            local deref, deref_err = vault.get(r)
+            if deref == nil or deref_err then
+              if opts.starting then
+                return nil, fmt("failed to dereference '%s': %s for config option '%s[%d]'", r, deref_err, k, i)
+              end
+
+              -- not that fatal if resolving fails during stopping (e.g. missing environment variables)
+              if opts.stopping then
+                conf[k] = ""
+                break
+              end
+
+            else
+              conf[k][i] = deref
+            end
           end
 
+        else
           local deref, deref_err = vault.get(v)
           if deref == nil or deref_err then
             if opts.starting then
               return nil, fmt("failed to dereference '%s': %s for config option '%s'", v, deref_err, k)
             end
 
+            -- not that fatal if resolving fails during stopping (e.g. missing environment variables)
+            if opts.stopping then
+              conf[k] = ""
+              break
+            end
+
           else
             conf[k] = deref
           end
@@ -465,7 +522,6 @@ local function load(path, custom_conf, opts)
 
   -- validation
   local ok, err, errors = check_and_parse(conf, opts)
-
   if not opts.starting then
     log.enable()
   end
@@ -706,12 +762,14 @@ local function load(path, custom_conf, opts)
     local conf_arr = {}
 
     for k, v in pairs(conf) do
-      local to_print = v
-      if conf_constants.CONF_SENSITIVE[k] then
-        to_print = conf_constants.CONF_SENSITIVE_PLACEHOLDER
-      end
+      if k ~= "$refs" then
+        local to_print = v
+        if conf_constants.CONF_SENSITIVE[k] then
+          to_print = conf_constants.CONF_SENSITIVE_PLACEHOLDER
+        end
 
-      conf_arr[#conf_arr+1] = k .. " = " .. pl_pretty.write(to_print, "")
+        conf_arr[#conf_arr+1] = k .. " = " .. pl_pretty.write(to_print, "")
+      end
     end
 
     sort(conf_arr)

diff --git a/kong/conf_loader/parse.lua b/kong/conf_loader/parse.lua
@@ -220,22 +220,24 @@ local function check_and_parse(conf, opts)
   local errors = {}
 
   for k, value in pairs(conf) do
-    local v_schema = conf_constants.CONF_PARSERS[k] or {}
+    if k ~= "$refs" then
+      local v_schema = conf_constants.CONF_PARSERS[k] or {}
 
-    value = parse_value(value, v_schema.typ)
+      value = parse_value(value, v_schema.typ)
 
-    local typ = v_schema.typ or "string"
-    if value and not conf_constants.TYP_CHECKS[typ](value) then
-      errors[#errors + 1] = fmt("%s is not a %s: '%s'", k, typ,
-                                tostring(value))
+      local typ = v_schema.typ or "string"
+      if value and not conf_constants.TYP_CHECKS[typ](value) then
+        errors[#errors + 1] = fmt("%s is not a %s: '%s'", k, typ,
+                                  tostring(value))
 
-    elseif v_schema.enum and not tablex.find(v_schema.enum, value) then
-      errors[#errors + 1] = fmt("%s has an invalid value: '%s' (%s)", k,
-                                tostring(value), concat(v_schema.enum, ", "))
+      elseif v_schema.enum and not tablex.find(v_schema.enum, value) then
+        errors[#errors + 1] = fmt("%s has an invalid value: '%s' (%s)", k,
+                                  tostring(value), concat(v_schema.enum, ", "))
 
-    end
+      end
 
-    conf[k] = value
+      conf[k] = value
+    end
   end
 
   ---------------------

diff --git a/kong/pdk/vault.lua b/kong/pdk/vault.lua
@@ -1673,6 +1673,7 @@ local function new(self)
     init_worker()
   end
 
+
   ---
   -- Warmups vault caches from config.
   --

diff --git a/spec/02-integration/13-vaults/03-mock_spec.lua b/spec/02-integration/13-vaults/03-mock_spec.lua
@@ -110,7 +110,7 @@ for _, strategy in helpers.each_strategy() do
         local body = assert.res_status(200, res)
         local json = cjson.decode(body)
         assert.equal(meta._VERSION, json.version)
-        assert.equal("{vault://mock/admin-listen}", json.configuration.admin_listen)
+        assert.same({ "{vault://mock/admin-listen}" }, json.configuration.admin_listen)
         assert.falsy(exists(join(helpers.test_conf.prefix, ".kong_process_secrets")))
       end)
     end)
@@ -141,7 +141,7 @@ for _, strategy in helpers.each_strategy() do
         local body = assert.res_status(200, res)
         local json = cjson.decode(body)
         assert.equal(meta._VERSION, json.version)
-        assert.equal("{vault://mock/listen?prefix=admin_}", json.configuration.admin_listen)
+        assert.same({ "{vault://mock/listen?prefix=admin_}" }, json.configuration.admin_listen)
       end)
     end)
   end)