[Backport release/3.5.x] fix(vault): resurrect positive results in lru cache for ttl + resurrect ttl

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

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

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.

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

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 },
         },
       },
     },