From 522f554d5f639515dcf176e61ab24305925e9421 Mon Sep 17 00:00:00 2001 From: Zhefeng C <38037704+catbro666@users.noreply.github.com> Date: Wed, 26 Jul 2023 14:46:17 +0800 Subject: [PATCH] fix(cmd): remove the non-referencable configuration field restriction (#11291) This commit allows some configuration fields to be referenced by using vaults. The limitation is introduced by #11127, and this commit removes the limitation to keep the behaviour to be the same as before [FTI-4937](https://konghq.atlassian.net/browse/FTI-4937) --- CHANGELOG.md | 5 +- kong/conf_loader/init.lua | 10 +--- spec/01-unit/03-conf_loader_spec.lua | 84 ++++++++++++++++++++++++---- 3 files changed, 79 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0e7720f76e1..df532a0c7031 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,9 +101,8 @@ Thanks [@backjo](https://github.com/backjo) for contributing this change. [#10723](https://github.com/Kong/kong/pull/10729) - Make `kong vault get` CLI command work in dbless mode by injecting the necessary directives into the kong cli nginx.conf. -Meanwhile, the following Kong configurations cannot reference vaults as they are required for vaults initializing: -`prefix`, `vaults`, `database`, `lmdb_environment_path`, `lmdb_map_size`, `lua_ssl_trusted_certificate`, `lua_ssl_protocols`, `nginx_http_lua_ssl_protocols`, `nginx_stream_lua_ssl_protocols`, `vault_*`. - [#10675](https://github.com/Kong/kong/pull/10675) + [#11127](https://github.com/Kong/kong/pull/11127) + [#11291](https://github.com/Kong/kong/pull/11291) #### Admin API diff --git a/kong/conf_loader/init.lua b/kong/conf_loader/init.lua index 5ae54624bca5..7b3b919246f4 100644 --- a/kong/conf_loader/init.lua +++ b/kong/conf_loader/init.lua @@ -618,9 +618,8 @@ local CONF_SENSITIVE = { } --- List of setttings whose values can't reference vaults --- because they'll be used before the vaults even ready -local CONF_NO_VAULT = { +-- List of confs necessary for compiling injected nginx conf +local CONF_BASIC = { prefix = true, vaults = true, database = true, @@ -1743,7 +1742,7 @@ local function load(path, custom_conf, opts) -- before executing the main `resty` cmd, i.e. still in `bin/kong` if opts.pre_cmd then for k, v in pairs(conf) do - if not CONF_NO_VAULT[k] then + if not CONF_BASIC[k] then conf[k] = nil end end @@ -1825,9 +1824,6 @@ local function load(path, custom_conf, opts) for k, v in pairs(conf) do v = parse_value(v, "string") if vault.is_reference(v) then - if CONF_NO_VAULT[k] then - return nil, fmt("the value of %s can't reference vault", k) - end if refs then refs[k] = v else diff --git a/spec/01-unit/03-conf_loader_spec.lua b/spec/01-unit/03-conf_loader_spec.lua index 578e39af5288..b90a7e6b57d8 100644 --- a/spec/01-unit/03-conf_loader_spec.lua +++ b/spec/01-unit/03-conf_loader_spec.lua @@ -2043,8 +2043,62 @@ describe("Configuration loader", function() assert.equal(5000, conf.pg_port) assert.equal("{vault://env/pg-port#0}", conf["$refs"].pg_port) end) - it("fields that can't reference vault", function() - local CONF_NO_VAULTS = { + it("fields in CONF_BASIC can reference env non-entity vault", function() + helpers.setenv("VAULT_TEST", "testvalue") + helpers.setenv("VAULT_PG", "postgres") + helpers.setenv("VAULT_CERT", "/tmp") + helpers.setenv("VAULT_DEPTH", "3") + finally(function() + helpers.unsetenv("VAULT_TEST") + helpers.unsetenv("VAULT_PG") + helpers.unsetenv("VAULT_CERT") + helpers.unsetenv("VAULT_DEPTH") + end) + local CONF_BASIC = { + prefix = true, + -- vaults = true, -- except this one + database = true, + lmdb_environment_path = true, + lmdb_map_size = true, + lua_ssl_trusted_certificate = true, + lua_ssl_verify_depth = true, + lua_ssl_protocols = true, + nginx_http_lua_ssl_protocols = true, + nginx_stream_lua_ssl_protocols = true, + -- vault_env_prefix = true, -- except this one + } + for k, _ in pairs(CONF_BASIC) do + if k == "database" then + local conf, err = conf_loader(nil, { + [k] = "{vault://env/vault_pg}", + }) + assert.is_nil(err) + assert.equal("postgres", conf.database) + elseif k == "lua_ssl_trusted_certificate" then + local conf, err = conf_loader(nil, { + [k] = "{vault://env/vault_cert}", + }) + assert.is_nil(err) + assert.equal("table", type(conf.lua_ssl_trusted_certificate)) + assert.equal("/tmp", conf.lua_ssl_trusted_certificate[1]) + elseif k == "lua_ssl_verify_depth" then + local conf, err = conf_loader(nil, { + [k] = "{vault://env/vault_depth}", + }) + assert.is_nil(err) + assert.equal(3, conf.lua_ssl_verify_depth) + else + local conf, err = conf_loader(nil, { + [k] = "{vault://env/vault_test}", + }) + assert.is_nil(err) + -- may be converted into an absolute path + assert.matches(".*testvalue", conf[k]) + end + end + end) + it("fields in CONF_BASIC will fail to reference vault if vault has other dependency", function() + local CONF_BASIC = { prefix = true, vaults = true, database = true, @@ -2057,22 +2111,32 @@ describe("Configuration loader", function() nginx_stream_lua_ssl_protocols = true, vault_env_prefix = true, } - for k, _ in pairs(CONF_NO_VAULTS) do + for k, _ in pairs(CONF_BASIC) do local conf, err = conf_loader(nil, { - [k] = "{vault://env/test}", + [k] = "{vault://test-env/test}", }) - - assert.equal(nil, conf) - if k == "lua_ssl_protocols" then - assert.matches("the value of .*lua_ssl_protocols can't reference vault", err) + -- fail to reference + if k == "lua_ssl_trusted_certificate" or k == "database" then + assert.is_not_nil(err) + elseif k == "lua_ssl_verify_depth" then + assert.is_nil(conf[k]) + elseif k == "vaults" then + assert.is_nil(err) + assert.equal("table", type(conf.vaults)) + assert.matches("{vault://test%-env/test}", conf.vaults[1]) + elseif k == "prefix" then + assert.is_nil(err) + assert.matches(".*{vault:/test%-env/test}", conf[k]) else - assert.equal("the value of " .. k .. " can't reference vault", err) + assert.is_nil(err) + -- path may have a prefix added + assert.matches(".*{vault://test%-env/test}", conf[k]) end end end) it("only load a subset of fields when opts.pre_cmd=true", function() local FIELDS = { - -- CONF_NO_VAULTS + -- CONF_BASIC prefix = true, vaults = true, database = true,