fix(vault): let shdict secret vault cache presist enough time during resurrect_ttl #13471

windmgc · 2024-08-08T09:24:41Z

Summary

This PR fixes an issue that rotate_secret may flush a secret value with NEGATIVE_CACHED_VALUE when vault backend is down and a secret value stored in the shared dict has passed its ttl and hasn't finished consuming its resurrect_ttl.

TLDR; this issue happens easily when a reference is being used via the vault PDK function in custom codes(serverless functions, custom plugins, etc.), and some of the worker processes may not be triggered via the service/routes that use these custom codes, and these worker processes do not hold a valid LRU cache for the secret value

The issue was first reported in FTI-6137.

Checklist

The Pull Request has tests
A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

FTI-6137

…resurrect_ttl

kong/pdk/vault.lua

tzssangglass · 2024-08-16T06:50:53Z

LGTM

changelog/unreleased/kong/fix-vault-resurrect-ttl-multi-worker.yml

spec/02-integration/13-vaults/07-resurrect_spec.lua

catbro666 · 2024-08-19T06:44:34Z

kong/pdk/vault.lua

      if ttl > resurrect_ttl then
        return true
+
+      elseif ttl > SECRETS_CACHE_MIN_TTL then


Not sure I'm understanding this correctly, but why should ttl > SECRETS_CACHE_MIN_TTL here?

Right now the vault timer is refreshing all the secrets in shared dict with a period that equals to ROTATION_INTERVAL, and SECRETS_CACHE_MIN_TTL is 2 * ROTATION_INTERVAL. This ensures the execution of the secret refreshing will happen at least once within the last remaining time of SECRETS_CACHE_MIN_TTL, so that it can be refreshed either with a null placholder(NEGATIVELY_CACHED_VALUE) or a new updated value.

kong/pdk/vault.lua

changelog/unreleased/kong/fix-vault-resurrect-ttl-multi-worker.yml

bungle · 2024-08-21T12:17:52Z

I will take a look on it.

Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>

kong/pdk/vault.lua

windmgc · 2024-08-21T14:23:44Z

@bungle I applied the reviews above, do you want to take another look?

It seems something is wrong with Github Action's worker, all tests are pending, we may need to wait until all tests passed

catbro666 · 2024-08-23T03:11:50Z

kong/pdk/vault.lua

+    if resurrect and value == nil then
+      local resurrected_value = SECRETS_CACHE:get(cache_key)
+      if resurrected_value then
+        return resurrected_value


What do you think about adding a log here?

…resurrect_ttl (#13471) This PR fixes an issue that rotate_secret may flush a secret value with NEGATIVE_CACHED_VALUE when vault backend is down and a secret value stored in the shared dict has passed its ttl and hasn't finished consuming its resurrect_ttl. TLDR; this issue happens easily when a reference is being used via the vault PDK function in custom codes(serverless functions, custom plugins, etc.), and some of the worker processes may not be triggered via the service/routes that use these custom codes, and these worker processes do not hold a valid LRU cache for the secret value The issue was first reported in FTI-6137. --------- Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com> Co-authored-by: Aapo Talvensaari <aapo.talvensaari@gmail.com> (cherry picked from commit 9269195)

team-gateway-bot · 2024-08-23T03:13:22Z

Successfully created backport PR for release/3.8.x:

[backport -> release/3.8.x] fix(vault): let shdict secret vault cache presist enough time during resurrect_ttl #13561

team-gateway-bot · 2024-08-23T03:13:22Z

Successfully created cherry-pick PR for master:

https://github.com/kong/kong-ee/pull/10066

…resurrect_ttl (#13471) This PR fixes an issue that rotate_secret may flush a secret value with NEGATIVE_CACHED_VALUE when vault backend is down and a secret value stored in the shared dict has passed its ttl and hasn't finished consuming its resurrect_ttl. TLDR; this issue happens easily when a reference is being used via the vault PDK function in custom codes(serverless functions, custom plugins, etc.), and some of the worker processes may not be triggered via the service/routes that use these custom codes, and these worker processes do not hold a valid LRU cache for the secret value The issue was first reported in FTI-6137. --------- Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com> Co-authored-by: Aapo Talvensaari <aapo.talvensaari@gmail.com> (cherry picked from commit 9269195)

team-gateway-bot · 2024-09-14T03:24:33Z

Cherry-pick failed for master, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git remote add upstream https://github.com/kong/kong-ee
git fetch upstream master
git worktree add -d .worktree/cherry-pick-13471-to-master-to-upstream upstream/master
cd .worktree/cherry-pick-13471-to-master-to-upstream
git checkout -b cherry-pick-13471-to-master-to-upstream
ancref=$(git merge-base 9bc3deb30041dbc45b1fe44fe8069f21ab69caaa 30215e6ff8c7711162107c53a7252f9d78691ca6)
git cherry-pick -x $ancref..30215e6ff8c7711162107c53a7252f9d78691ca6

…resurrect_ttl (#13471) This PR fixes an issue that rotate_secret may flush a secret value with NEGATIVE_CACHED_VALUE when vault backend is down and a secret value stored in the shared dict has passed its ttl and hasn't finished consuming its resurrect_ttl. TLDR; this issue happens easily when a reference is being used via the vault PDK function in custom codes(serverless functions, custom plugins, etc.), and some of the worker processes may not be triggered via the service/routes that use these custom codes, and these worker processes do not hold a valid LRU cache for the secret value The issue was first reported in FTI-6137. --------- Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com> Co-authored-by: Aapo Talvensaari <aapo.talvensaari@gmail.com> (cherry picked from commit 9269195)

team-gateway-bot · 2024-09-14T03:24:38Z

Successfully created backport PR for release/3.7.x:

[backport -> release/3.7.x] fix(vault): let shdict secret vault cache presist enough time during resurrect_ttl #13672

…resurrect_ttl (#13471) This PR fixes an issue that rotate_secret may flush a secret value with NEGATIVE_CACHED_VALUE when vault backend is down and a secret value stored in the shared dict has passed its ttl and hasn't finished consuming its resurrect_ttl. TLDR; this issue happens easily when a reference is being used via the vault PDK function in custom codes(serverless functions, custom plugins, etc.), and some of the worker processes may not be triggered via the service/routes that use these custom codes, and these worker processes do not hold a valid LRU cache for the secret value The issue was first reported in FTI-6137. --------- Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com> Co-authored-by: Aapo Talvensaari <aapo.talvensaari@gmail.com> (cherry picked from commit 9269195)

team-gateway-bot · 2024-09-14T03:24:43Z

Successfully created backport PR for release/3.8.x:

[backport -> release/3.8.x] fix(vault): let shdict secret vault cache presist enough time during resurrect_ttl #13673

…resurrect_ttl (#13471) This PR fixes an issue that rotate_secret may flush a secret value with NEGATIVE_CACHED_VALUE when vault backend is down and a secret value stored in the shared dict has passed its ttl and hasn't finished consuming its resurrect_ttl. TLDR; this issue happens easily when a reference is being used via the vault PDK function in custom codes(serverless functions, custom plugins, etc.), and some of the worker processes may not be triggered via the service/routes that use these custom codes, and these worker processes do not hold a valid LRU cache for the secret value The issue was first reported in FTI-6137. --------- Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com> Co-authored-by: Aapo Talvensaari <aapo.talvensaari@gmail.com> (cherry picked from commit 9269195)

pull-request-size bot added the size/S label Aug 8, 2024

github-actions bot added core/pdk cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels Aug 8, 2024

github-actions bot assigned windmgc Aug 8, 2024

windmgc added 2 commits August 14, 2024 16:14

fix(vault): let shdict secret vault cache presist enough time during …

047eb3d

…resurrect_ttl

tests(vault): add a multi-worker resurrect ttl test

61a3e20

windmgc force-pushed the vault-shdict-resurrect-ttl branch from 74ff0b0 to 61a3e20 Compare August 14, 2024 08:14

pull-request-size bot added size/L and removed size/S labels Aug 14, 2024

docs(changelog): add changelog

0a55391

windmgc marked this pull request as ready for review August 14, 2024 08:29

tzssangglass reviewed Aug 16, 2024

View reviewed changes

kong/pdk/vault.lua Outdated Show resolved Hide resolved

tzssangglass approved these changes Aug 16, 2024

View reviewed changes

outsinre approved these changes Aug 16, 2024

View reviewed changes

changelog/unreleased/kong/fix-vault-resurrect-ttl-multi-worker.yml Outdated Show resolved Hide resolved

spec/02-integration/13-vaults/07-resurrect_spec.lua Show resolved Hide resolved

docs(changelog): refine word

ee364f4

catbro666 reviewed Aug 19, 2024

View reviewed changes

AndyZhang0707 added this to the 3.8.0 milestone Aug 19, 2024

catbro666 reviewed Aug 19, 2024

View reviewed changes

kong/pdk/vault.lua Show resolved Hide resolved

catbro666 reviewed Aug 19, 2024

View reviewed changes

kong/pdk/vault.lua Outdated Show resolved Hide resolved

AndyZhang0707 added the backport release/3.8.x label Aug 20, 2024

fix(*): apply suggestions from code review

b955823

windmgc requested a review from catbro666 August 21, 2024 06:15

catbro666 reviewed Aug 21, 2024

View reviewed changes

changelog/unreleased/kong/fix-vault-resurrect-ttl-multi-worker.yml Outdated Show resolved Hide resolved

catbro666 approved these changes Aug 21, 2024

View reviewed changes

simpler implementation

1c59849

Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>

bungle approved these changes Aug 21, 2024

View reviewed changes

bungle reviewed Aug 21, 2024

View reviewed changes

kong/pdk/vault.lua Outdated Show resolved Hide resolved

windmgc commented Aug 21, 2024

View reviewed changes

kong/pdk/vault.lua Outdated Show resolved Hide resolved

windmgc added 2 commits August 21, 2024 22:08

fix(*): apply suggestions

4ebed9c

fix(*): add else block for early exit

30215e6

catbro666 reviewed Aug 23, 2024

View reviewed changes

windmgc merged commit 9269195 into master Aug 23, 2024
26 checks passed

windmgc deleted the vault-shdict-resurrect-ttl branch August 23, 2024 03:13

team-gateway-bot mentioned this pull request Aug 23, 2024

[backport -> release/3.8.x] fix(vault): let shdict secret vault cache presist enough time during resurrect_ttl #13561

Merged

3 tasks

windmgc added the backport release/3.7.x label Sep 14, 2024

github-actions bot added the incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention label Sep 14, 2024

team-gateway-bot mentioned this pull request Sep 14, 2024

[backport -> release/3.7.x] fix(vault): let shdict secret vault cache presist enough time during resurrect_ttl #13672

Merged

3 tasks

team-gateway-bot mentioned this pull request Sep 14, 2024

[backport -> release/3.8.x] fix(vault): let shdict secret vault cache presist enough time during resurrect_ttl #13673

Closed

3 tasks

kikito removed the incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention label Sep 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(vault): let shdict secret vault cache presist enough time during resurrect_ttl #13471

fix(vault): let shdict secret vault cache presist enough time during resurrect_ttl #13471

windmgc commented Aug 8, 2024 •

edited

Loading

tzssangglass commented Aug 16, 2024

catbro666 Aug 19, 2024

windmgc Aug 19, 2024

bungle commented Aug 21, 2024

windmgc commented Aug 21, 2024 •

edited

Loading

catbro666 Aug 23, 2024

team-gateway-bot commented Aug 23, 2024

team-gateway-bot commented Aug 23, 2024

team-gateway-bot commented Sep 14, 2024

team-gateway-bot commented Sep 14, 2024

team-gateway-bot commented Sep 14, 2024

fix(vault): let shdict secret vault cache presist enough time during resurrect_ttl #13471

fix(vault): let shdict secret vault cache presist enough time during resurrect_ttl #13471

Conversation

windmgc commented Aug 8, 2024 • edited Loading

Summary

Checklist

Issue reference

tzssangglass commented Aug 16, 2024

catbro666 Aug 19, 2024

Choose a reason for hiding this comment

windmgc Aug 19, 2024

Choose a reason for hiding this comment

bungle commented Aug 21, 2024

windmgc commented Aug 21, 2024 • edited Loading

catbro666 Aug 23, 2024

Choose a reason for hiding this comment

team-gateway-bot commented Aug 23, 2024

team-gateway-bot commented Aug 23, 2024

team-gateway-bot commented Sep 14, 2024

team-gateway-bot commented Sep 14, 2024

team-gateway-bot commented Sep 14, 2024

windmgc commented Aug 8, 2024 •

edited

Loading

windmgc commented Aug 21, 2024 •

edited

Loading