Skip to content

fix: invalidate secret LRU cache on /secrets changes#13668

Merged
shreemaan-abhishek merged 2 commits into
apache:masterfrom
shreemaan-abhishek:fix/secret-cache-stale-on-delete
Jul 10, 2026
Merged

fix: invalidate secret LRU cache on /secrets changes#13668
shreemaan-abhishek merged 2 commits into
apache:masterfrom
shreemaan-abhishek:fix/secret-cache-stale-on-delete

Conversation

@shreemaan-abhishek

Copy link
Copy Markdown
Contributor

Description

After a secret object is deleted via the Admin API, $secret:// references keep resolving to the previously cached value indefinitely. Updates to a secret are only picked up after the cache TTL window.

Root cause: apisix/secret.lua resolved secret URIs through the secret LRU cache with a constant version string:

return secrets_cache(uri, "", fetch_by_uri, uri)

core.lrucache uses the version argument to detect staleness (an entry is re-resolved only when obj.ver ~= version). Every other cached resource passes its real conf_version; the secret cache passed "", so no /secrets change ever invalidated an entry. With refresh_stale = true an expired entry keeps being served while a background timer retries, so once the secret is deleted the refresh fails forever and the stale value is served forever.

Fix: Pass the real secrets conf_version (already exposed via _M.secrets()). A change to /secrets bumps the version, so the cache re-resolves on the next access, matching how route/service caches behave.

Notes:

  • /secrets has one shared conf_version, so any secret change invalidates all cached secret values (consistent with route/service cache behavior).
  • $env:// shares this cache; bumping on /secrets changes also drops env entries, which is harmless since env values are static.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change (no user-facing config/behavior change; restores expected behavior)
  • I have verified that this change is backward compatible

Test

t/secret/secret_lru.t adds a case that resolves a secret through the cache, deletes it, and asserts the reference stops resolving (falls back to the literal string) instead of serving the stale value. Verified the test fails before the fix and passes after.

The secret resolver passed a constant "" version to the secret LRU
cache, so core.lrucache never detected staleness: a deleted secret kept
resolving to its cached value indefinitely, and updates were only picked
up after the TTL window. With refresh_stale = true an expired entry is
served while a background refresh retries, so once a secret is deleted
the refresh fails forever and the stale value is served forever.

Pass the real secrets conf_version (already exposed via _M.secrets) so a
change to /secrets re-resolves the entry on the next access, matching how
route/service caches behave. $env:// shares this cache; env values are
static, so dropping them on a /secrets bump is harmless.

Signed-off-by: Abhishek Choudhary <shreemaan.abhishek@gmail.com>
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Jul 7, 2026
TEST 3 polls for the /secrets version bump after the delete, which can
take a couple of seconds on a loaded runner. Give the request a 20s
ceiling so a slow poll fails on the assertion, not a client timeout.

Signed-off-by: Abhishek Choudhary <shreemaan.abhishek@gmail.com>
Comment thread apisix/secret.lua

@membphis membphis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shreemaan-abhishek shreemaan-abhishek merged commit 2f0898c into apache:master Jul 10, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants