diff --git a/changelog/unreleased/kong/vault-resurrect.yml b/changelog/unreleased/kong/vault-resurrect.yml new file mode 100644 index 000000000000..7dc1b5d9ee1e --- /dev/null +++ b/changelog/unreleased/kong/vault-resurrect.yml @@ -0,0 +1,3 @@ +message: Vault resurrect time is respected in case a vault secret is deleted from a vault +type: bugfix +scope: Core diff --git a/kong/pdk/vault.lua b/kong/pdk/vault.lua index 00fa633168fd..9473ae9a2e09 100644 --- a/kong/pdk/vault.lua +++ b/kong/pdk/vault.lua @@ -27,6 +27,7 @@ local get_updated_now_ms = utils.get_updated_now_ms local ngx = ngx local get_phase = ngx.get_phase local max = math.max +local min = math.min local fmt = string.format local sub = string.sub local byte = string.byte @@ -753,15 +754,25 @@ local function new(self) local function get_cache_value_and_ttl(value, config, ttl) local cache_value, shdict_ttl, lru_ttl if value then - -- adjust ttl to the minimum and maximum values configured - lru_ttl = adjust_ttl(ttl, config) - shdict_ttl = max(lru_ttl + (config.resurrect_ttl or DAO_MAX_TTL), SECRETS_CACHE_MIN_TTL) cache_value = value + -- adjust ttl to the minimum and maximum values configured + ttl = adjust_ttl(ttl, config) + + if config.resurrect_ttl then + lru_ttl = min(ttl + config.resurrect_ttl, DAO_MAX_TTL) + shdict_ttl = max(lru_ttl, SECRETS_CACHE_MIN_TTL) + + else + lru_ttl = ttl + shdict_ttl = DAO_MAX_TTL + end + else + cache_value = NEGATIVELY_CACHED_VALUE + -- negatively cached values will be rotated on each rotation interval shdict_ttl = max(config.neg_ttl or 0, SECRETS_CACHE_MIN_TTL) - cache_value = NEGATIVELY_CACHED_VALUE end return cache_value, shdict_ttl, lru_ttl @@ -794,14 +805,13 @@ local function new(self) return nil, cache_err end - if not value then - LRU:delete(reference) + if cache_value == NEGATIVELY_CACHED_VALUE then return nil, fmt("could not get value from external vault (%s)", err) end - LRU:set(reference, value, lru_ttl) + LRU:set(reference, cache_value, lru_ttl) - return value + return cache_value end @@ -823,8 +833,7 @@ local function new(self) -- @usage -- local value, err = get(reference, cache_only) local function get(reference, cache_only) - -- the LRU stale value is ignored as the resurrection logic - -- is deferred to the shared dictionary + -- the LRU stale value is ignored local value = LRU:get(reference) if value then return value @@ -1359,8 +1368,8 @@ local function new(self) return nil, cache_err end - if value then - LRU:set(reference, value, lru_ttl) + if cache_value ~= NEGATIVELY_CACHED_VALUE then + LRU:set(reference, cache_value, lru_ttl) end return true diff --git a/spec/02-integration/13-vaults/05-ttl_spec.lua b/spec/02-integration/13-vaults/05-ttl_spec.lua index 21736bb94b18..e6f65fd56465 100644 --- a/spec/02-integration/13-vaults/05-ttl_spec.lua +++ b/spec/02-integration/13-vaults/05-ttl_spec.lua @@ -64,7 +64,7 @@ local VAULTS = { }, create_secret = function(self, _, value) - -- Currently, crate_secret is called _before_ starting Kong. + -- Currently, create_secret is called _before_ starting Kong. -- -- This means our backend won't be available yet because it is -- piggy-backing on Kong as an HTTP mock fixture. diff --git a/spec/02-integration/13-vaults/07-resurrect_spec.lua b/spec/02-integration/13-vaults/07-resurrect_spec.lua new file mode 100644 index 000000000000..d91bbcabd86b --- /dev/null +++ b/spec/02-integration/13-vaults/07-resurrect_spec.lua @@ -0,0 +1,240 @@ +local helpers = require "spec.helpers" + +-- using the full path so that we don't have to modify package.path in +-- this context +local test_vault = require "spec.fixtures.custom_vaults.kong.vaults.test" + +local CUSTOM_VAULTS = "./spec/fixtures/custom_vaults" +local CUSTOM_PLUGINS = "./spec/fixtures/custom_plugins" + +local LUA_PATH = CUSTOM_VAULTS .. "/?.lua;" .. + CUSTOM_VAULTS .. "/?/init.lua;" .. + CUSTOM_PLUGINS .. "/?.lua;" .. + CUSTOM_PLUGINS .. "/?/init.lua;;" + +local DUMMY_HEADER = "Dummy-Plugin" +local fmt = string.format + + + +--- A vault test harness is a driver for vault backends, which implements +--- all the necessary glue for initializing a vault backend and performing +--- secret read/write operations. +--- +--- All functions defined here are called as "methods" (e.g. harness:fn()), so +--- it is permitted to keep state on the harness object (self). +--- +---@class vault_test_harness +--- +---@field name string +--- +--- this table is passed directly to kong.db.vaults:insert() +---@field config table +--- +--- create_secret() is called once per test run for a given secret +---@field create_secret fun(self: vault_test_harness, secret: string, value: string, opts?: table) +--- +--- update_secret() may be called more than once per test run for a given secret +---@field update_secret fun(self: vault_test_harness, secret: string, value: string, opts?: table) +--- +--- setup() is called before kong is started and before any DB entities +--- have been created and is best used for things like validating backend +--- credentials and establishing a connection to a backend +---@field setup fun(self: vault_test_harness) +--- +--- teardown() is exactly what you'd expect +---@field teardown fun(self: vault_test_harness) +--- +--- fixtures() output is passed directly to `helpers.start_kong()` +---@field fixtures fun(self: vault_test_harness):table|nil +--- +--- +---@field prefix string # generated by the test suite +---@field host string # generated by the test suite + + +---@type vault_test_harness[] +local VAULTS = { + { + name = "test", + + config = { + default_value = "DEFAULT", + default_value_ttl = 1, + }, + + create_secret = function(self, _, value) + -- Currently, create_secret is called _before_ starting Kong. + -- + -- This means our backend won't be available yet because it is + -- piggy-backing on Kong as an HTTP mock fixture. + -- + -- We can, however, inject a default value into our configuration. + self.config.default_value = value + end, + + update_secret = function(_, secret, value, opts) + return test_vault.client.put(secret, value, opts) + end, + + delete_secret = function(_, secret) + return test_vault.client.delete(secret) + end, + + fixtures = function() + return { + http_mock = { + test_vault = test_vault.http_mock, + } + } + end, + }, +} + + +local noop = function(...) end + +for _, vault in ipairs(VAULTS) do + -- fill out some values that we'll use in route/service/plugin config + vault.prefix = vault.name .. "-ttl-test" + vault.host = vault.name .. ".vault-ttl.test" + + -- ...and fill out non-required methods + vault.setup = vault.setup or noop + vault.teardown = vault.teardown or noop + vault.fixtures = vault.fixtures or noop +end + + +for _, strategy in helpers.each_strategy() do +for _, vault in ipairs(VAULTS) do + + +describe("vault resurrect_ttl and rotation (#" .. strategy .. ") #" .. vault.name, function() + local client + local secret = "my-secret" + + + local function http_get(path) + path = path or "/" + + local res = client:get(path, { + headers = { + host = assert(vault.host), + }, + }) + + assert.response(res).has.status(200) + + return res + end + + + lazy_setup(function() + helpers.setenv("KONG_LUA_PATH_OVERRIDE", LUA_PATH) + helpers.setenv("KONG_VAULT_ROTATION_INTERVAL", "1") + + helpers.test_conf.loaded_plugins = { + dummy = true, + } + + vault:setup() + vault:create_secret(secret, "init") + + local bp = helpers.get_db_utils(strategy, + { "vaults", "routes", "services", "plugins" }, + { "dummy" }, + { vault.name }) + + + assert(bp.vaults:insert({ + name = vault.name, + prefix = vault.prefix, + config = vault.config, + })) + + local route = assert(bp.routes:insert({ + name = vault.host, + hosts = { vault.host }, + paths = { "/" }, + service = assert(bp.services:insert()), + })) + + + -- used by the plugin config test case + assert(bp.plugins:insert({ + name = "dummy", + config = { + resp_header_value = fmt("{vault://%s/%s?ttl=%d&resurrect_ttl=%d}", + vault.prefix, secret, 2, 2), + }, + route = { id = route.id }, + })) + + assert(helpers.start_kong({ + database = strategy, + nginx_conf = "spec/fixtures/custom_nginx.template", + vaults = vault.name, + plugins = "dummy", + log_level = "info", + }, nil, nil, vault:fixtures() )) + + client = helpers.proxy_client() + end) + + + lazy_teardown(function() + if client then + client:close() + end + + helpers.stop_kong(nil, true) + vault:teardown() + + helpers.unsetenv("KONG_LUA_PATH_OVERRIDE") + end) + + + it("resurrects plugin config references when secret is deleted (backend: #" .. vault.name .. ")", function() + local function check_plugin_secret(expect, ttl, leeway) + leeway = leeway or 0.25 -- 25% + + local timeout = ttl + (ttl * leeway) + + assert + .with_timeout(timeout) + .with_step(0.5) + .eventually(function() + local res = http_get("/") + local value + if expect == "" then + value = res.headers[DUMMY_HEADER] or "" + if value == "" then + return true + end + + else + value = assert.response(res).has.header(DUMMY_HEADER) + if value == expect then + return true + end + end + + return nil, { expected = expect, got = value } + end) + .is_truthy("expected plugin secret to be updated to '" .. tostring(expect) .. "' " + .. "within " .. tostring(timeout) .. " seconds") + end + + vault:update_secret(secret, "old", { ttl = 2, resurrect_ttl = 2 }) + check_plugin_secret("old", 5) + vault:delete_secret(secret) + ngx.sleep(2.5) + check_plugin_secret("old", 5) + check_plugin_secret("", 5) + end) +end) + + +end -- each vault backend +end -- each strategy diff --git a/spec/fixtures/custom_vaults/kong/vaults/test/schema.lua b/spec/fixtures/custom_vaults/kong/vaults/test/schema.lua index 4b4a335e9cb8..019179b2a5ab 100644 --- a/spec/fixtures/custom_vaults/kong/vaults/test/schema.lua +++ b/spec/fixtures/custom_vaults/kong/vaults/test/schema.lua @@ -1,3 +1,6 @@ +local typedefs = require "kong.db.schema.typedefs" + + return { name = "test", fields = { @@ -7,6 +10,9 @@ return { fields = { { default_value = { type = "string", required = false } }, { default_value_ttl = { type = "number", required = false } }, + { ttl = typedefs.ttl }, + { neg_ttl = typedefs.ttl }, + { resurrect_ttl = typedefs.ttl }, }, }, },