Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(certificate): unable to refresh certificate entity with vault reference when initial with an invalid string #14049

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ms2008
Copy link
Contributor

@ms2008 ms2008 commented Dec 24, 2024

Manual backport https://github.com/Kong/kong-ee/pull/10984 to CE

Original description

Summary

When set the cert to an invalid string in the vault, and then correct the string, kong will not start to use the correct certificate even though kong vault command shows the correct certiticate being returned.

This is because the initial invalid string has already been cached in the L2 cache by mlcache, and it will not be updated via the L3 cache callback afterward.

When mlcache fails during the execution of the l1_serializer, it caches the result retrieved from the L3 cache callback the first time in the L2 cache permanently. This means that subsequent calls to cache.get() will never fetch data from the L3 cache but will directly retrieve it from the L2 cache.

To avoid this situation, when the l1_serializer fails, we no longer allow mlcache to store the data retrieved from L3 into L2.


I believe this is more of a defect in mlcache. When mlcache fails during the execution of the l1_serializer, it caches the result retrieved from the L3 cache callback the first time in the L2 cache permanently. This means that subsequent calls to cache.get() will never fetch data from the L3 cache but will directly retrieve it from the L2 cache.

From the perspective of mlcache, this doesn't seem to be an issue. When the data is updated, the invalidation should be explicitly triggered by the "data owner" themselves. This design allows mlcache to maximize its cache hit rate. However, in the case of Kong, this doesn't seem practical, as some data is stored in external systems, which cannot conveniently invalidate mlcache's cache, resulting in our inability to retrieve the most up-to-date data in a timely manner.

Checklist

  • The Pull Request has tests
  • A changelog file has been added to CHANGELOG/unreleased/kong or adding skip-changelog label on PR if unnecessary. README.md
  • The Pull Request has backports to all the versions it needs to cover
  • There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

Fix FTI-6392

…erence when initial with an invalid string (#10984)

When set the cert to an invalid string in the vault, and then correct the
string, kong will not start to use the correct certificate even though `kong
vault` command shows the correct certiticate being returned.

This is because the initial invalid string has already been cached in the L2
cache by mlcache, and it will not be updated via the L3 cache callback
afterward.

When mlcache fails during the execution of the `l1_serializer`, it caches the
result retrieved from the L3 cache callback the first time in the L2 cache
permanently. This means that subsequent calls to `cache.get()` will never fetch
data from the L3 cache but will directly retrieve it from the L2 cache.

To avoid this situation, when the `l1_serializer` fails, we no longer allow
mlcache to store the data retrieved from L3 into L2.
@github-actions github-actions bot added the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Dec 24, 2024
@ms2008 ms2008 removed the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Dec 24, 2024
@ms2008 ms2008 marked this pull request as ready for review December 24, 2024 10:51
@ms2008 ms2008 requested a review from fffonion December 24, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant