diff --git a/changelog/unreleased/kong/vault-init-warmup.yml b/changelog/unreleased/kong/vault-init-warmup.yml new file mode 100644 index 000000000000..611277be75b9 --- /dev/null +++ b/changelog/unreleased/kong/vault-init-warmup.yml @@ -0,0 +1,3 @@ +message: Properly warmup Vault caches on init +type: bugfix +scope: Core diff --git a/kong/pdk/vault.lua b/kong/pdk/vault.lua index 32a35e51d82d..7023d55cbc88 100644 --- a/kong/pdk/vault.lua +++ b/kong/pdk/vault.lua @@ -183,7 +183,7 @@ end local function new(self) -- Don't put this onto the top level of the file unless you're prepared for a surprise local Schema = require "kong.db.schema" - + local ROTATION_MUTEX_OPTS = { name = "vault-rotation", exptime = ROTATION_INTERVAL * 1.5, -- just in case the lock is not properly released @@ -682,7 +682,7 @@ local function new(self) return nil, err end - if kong and kong.licensing and kong.licensing:license_type() == "free" and strategy.license_required then + if strategy.license_required and self.licensing and self.licensing:license_type() == "free" then return nil, "vault " .. name .. " requires a license to be used" end @@ -738,6 +738,35 @@ local function new(self) return value, nil, ttl end + --- + -- Function `get_cache_value_and_ttl` returns a value for caching and its ttl + -- + -- @local + -- @function get_from_vault + -- @tparam string value the vault returned value for a reference + -- @tparam table config the configuration settings to be used + -- @tparam[opt] number ttl the possible vault returned ttl + -- @treturn string value to be stored in shared dictionary + -- @treturn number shared dictionary ttl + -- @treturn number lru ttl + -- @usage local value, err = get_from_vault(reference, strategy, config, cache_key, parsed_reference) + 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 + + else + -- 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 + end + --- -- Function `get_from_vault` retrieves a value from the vault using the provided strategy. @@ -759,19 +788,7 @@ local function new(self) -- @usage local value, err = get_from_vault(reference, strategy, config, cache_key, parsed_reference) local function get_from_vault(reference, strategy, config, cache_key, parsed_reference) local value, err, ttl = invoke_strategy(strategy, config, parsed_reference) - local cache_value, shdict_ttl - if value then - -- adjust ttl to the minimum and maximum values configured - ttl = adjust_ttl(ttl, config) - shdict_ttl = max(ttl + (config.resurrect_ttl or DAO_MAX_TTL), SECRETS_CACHE_MIN_TTL) - cache_value = value - - else - -- 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 - + local cache_value, shdict_ttl, lru_ttl = get_cache_value_and_ttl(value, config, ttl) local ok, cache_err = SECRETS_CACHE:safe_set(cache_key, cache_value, shdict_ttl) if not ok then return nil, cache_err @@ -782,7 +799,7 @@ local function new(self) return nil, fmt("could not get value from external vault (%s)", err) end - LRU:set(reference, value, ttl) + LRU:set(reference, value, lru_ttl) return value end @@ -866,26 +883,14 @@ local function new(self) --- - -- Function `update` recursively updates a configuration table. - -- - -- This function recursively in-place updates a configuration table by - -- replacing reference fields with values fetched from a cache. The references - -- are specified in a `$refs` field. - -- - -- If a reference cannot be fetched from the cache, the corresponding field is - -- set to nil and an warning is logged. + -- Recurse over config and calls the callback for each found reference. -- -- @local - -- @function update - -- @tparam table config a table representing the configuration to update (if `config` - -- is not a table, the function immediately returns it without any modifications) - -- @treturn table the config table (with possibly updated values). - -- - -- @usage - -- local config = update(config) - -- OR - -- update(config) - local function update(config) + -- @function recurse_config_refs + -- @tparam table config config table to recurse. + -- @tparam function callback callback to call on each reference. + -- @treturn table config that might have been updated, depending on callback. + local function recurse_config_refs(config, callback) -- silently ignores other than tables if type(config) ~= "table" then return config @@ -893,7 +898,7 @@ local function new(self) for key, value in pairs(config) do if key ~= "$refs" and type(value) == "table" then - update(value) + recurse_config_refs(value, callback) end end @@ -904,11 +909,11 @@ local function new(self) for name, reference in pairs(references) do if type(reference) == "string" then -- a string reference - update_from_cache(reference, config, name) + callback(reference, config, name) elseif type(reference) == "table" then -- array, set or map of references for key, ref in pairs(reference) do - update_from_cache(ref, config[name], key) + callback(ref, config[name], key) end end end @@ -917,6 +922,31 @@ local function new(self) end + --- + -- Function `update` recursively updates a configuration table. + -- + -- This function recursively in-place updates a configuration table by + -- replacing reference fields with values fetched from a cache. The references + -- are specified in a `$refs` field. + -- + -- If a reference cannot be fetched from the cache, the corresponding field is + -- set to nil and an warning is logged. + -- + -- @local + -- @function update + -- @tparam table config a table representing the configuration to update (if `config` + -- is not a table, the function immediately returns it without any modifications) + -- @treturn table the config table (with possibly updated values). + -- + -- @usage + -- local config = update(config) + -- OR + -- update(config) + local function update(config) + return recurse_config_refs(config, update_from_cache) + end + + --- -- Function `get_references` recursively iterates over options and returns -- all the references in an array. The same reference is in array only once. @@ -1105,7 +1135,7 @@ local function new(self) -- We cannot retry, so let's just call the callback and return return callback(options) end - + local name = "vault.try:" .. calculate_hash(concat(references, ".")) local old_updated_at = RETRY_LRU:get(name) or 0 @@ -1296,10 +1326,6 @@ local function new(self) initialized = true - if self.configuration.role == "control_plane" then - return - end - if self.configuration.database ~= "off" then self.worker_events.register(handle_vault_crud_event, "crud", "vaults") end @@ -1311,6 +1337,61 @@ local function new(self) end + --- + -- Called on `init` phase, and stores value in secrets cache. + -- + -- @local + -- @function init_in_cache_from_value + -- @tparam string reference a vault reference. + -- @tparan value string value that is stored in secrets cache. + local function init_in_cache_from_value(reference, value) + local strategy, err, config, cache_key = get_strategy(reference) + if not strategy then + return nil, err + end + + -- doesn't support vault returned ttl, but none of the vaults supports it, + -- and the support for vault returned ttl might be removed later. + local cache_value, shdict_ttl, lru_ttl = get_cache_value_and_ttl(value, config) + + local ok, cache_err = SECRETS_CACHE:safe_set(cache_key, cache_value, shdict_ttl) + if not ok then + return nil, cache_err + end + + if value then + LRU:set(reference, value, lru_ttl) + end + + return true + end + + + --- + -- Called on `init` phase, and used to warmup secrets cache. + -- + -- @local + -- @function init_in_cache + -- @tparam string reference a vault reference. + -- @tparan table record a table that is a container for de-referenced value. + -- @tparam field string field name in a record to which to store the de-referenced value. + local function init_in_cache(reference, record, field) + local value, err = init_in_cache_from_value(reference, record[field]) + if not value then + self.log.warn("error caching secret reference ", reference, ": ", err) + end + end + + + --- + -- Called on `init` phase, and used to warmup secrets cache. + -- @local + -- @function init + local function init() + recurse_config_refs(self.configuration, init_in_cache) + end + + local _VAULT = {} -- the public PDK interfaces @@ -1482,6 +1563,9 @@ local function new(self) init_worker() end + if get_phase() == "init" then + init() + end return _VAULT end diff --git a/spec/02-integration/02-cmd/02-start_stop_spec.lua b/spec/02-integration/02-cmd/02-start_stop_spec.lua index 1bc151b0bb38..01540451b2aa 100644 --- a/spec/02-integration/02-cmd/02-start_stop_spec.lua +++ b/spec/02-integration/02-cmd/02-start_stop_spec.lua @@ -141,6 +141,8 @@ describe("kong start/stop #" .. strategy, function() })) assert.not_matches("failed to dereference {vault://env/pg_password}", stderr, nil, true) + assert.logfile().has.no.line("[warn]", true) + assert.logfile().has.no.line("env/pg_password", true) assert.matches("Kong started", stdout, nil, true) assert(kong_exec("stop", { prefix = PREFIX,