-
Notifications
You must be signed in to change notification settings - Fork 590
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
feat: store only secrets that are possible to be used in object cache of dataplane #3047
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
randmonkey
force-pushed
the
feat/reduce_secret_cache
branch
from
October 13, 2022 05:53
d5f2d54
to
7fe8061
Compare
randmonkey
force-pushed
the
feat/reduce_secret_cache
branch
from
October 13, 2022 06:44
7fe8061
to
6e772d1
Compare
randmonkey
force-pushed
the
feat/reduce_secret_cache
branch
from
October 13, 2022 06:48
6e772d1
to
82e2b79
Compare
randmonkey
force-pushed
the
feat/reduce_secret_cache
branch
from
October 13, 2022 07:07
82e2b79
to
3f202a3
Compare
czeslavo
reviewed
Oct 13, 2022
randmonkey
force-pushed
the
feat/reduce_secret_cache
branch
from
October 14, 2022 06:10
3f202a3
to
05f27ed
Compare
randmonkey
force-pushed
the
feat/reduce_secret_cache
branch
from
October 14, 2022 06:19
05f27ed
to
a6306ce
Compare
czeslavo
reviewed
Oct 24, 2022
pmalek
reviewed
Oct 25, 2022
randmonkey
force-pushed
the
feat/reduce_secret_cache
branch
from
October 25, 2022 18:14
f78e89e
to
4bffd59
Compare
pmalek
approved these changes
Oct 25, 2022
jrsmroz
approved these changes
Oct 25, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
czeslavo
pushed a commit
that referenced
this pull request
Oct 27, 2022
… of dataplane (#3047) * add indexer for secret reference update reference in controllers * delete secrets when they are not referred * refactor: move reference indexder to controller/reference package * add unit tests and address comments * add more unit tests and update integration tests * fix conflicts on generated controllers * fix: does not delete secret if it has ca cert label * rework to use map for lookup referred secrets * refactor listreferredObjects and remove refs for gateways * address comments: update secret controller and fix typos
3 tasks
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Reduce secret cache size by only storing secrets that might be used. The secrets includes:
konghq.com/ca-cert: true
Service
(in annotationkonghq.com/client-cert
),Ingress
,KongPlugin
,KongClusterPlugin
,KongConsumer
,TCPIngress
,Gateway
,knative.Ingress
.Current implementation allows referring to secrets which does not exist yet, so we only store the object holding
GroupVersionKind
and namespace/name in the reference indexer, not the actual secret.The implementation adds a standalone secret controller instead of the generated secret controller, and updated the controllers reconciling the resources referring secrets to store the reference relationship and add referred secrets (which are ) to the cache.
controllers other than
Secret
(Service,Ingress,Gateway,...) will update the reference indexer if we need to use secrets referred when configuring the object, and add secret to cache if the secret is present. The secret controller adds secret to controller when it is created or updated if the secret either:konghq.com/ca-cert: true
There may still something missing, but the PR is ready for a comprehensive review.
Which issue this PR fixes:
expected to fix #2868
Special notes for your reviewer:
Here I added an
cache.Indexer
to store* -> Secret
reference relationships. When a object that is possible to refer to a secret is created or updated, the controller checks up the secret it references and store the reference relationship.Note here we still store the reference relationship when the secret does not exist, so the reconciliation will be done when the secret is actually created and secret controller started reconciliation for the secret.
REVIEW: use a flag/feature gate to enable or disable storing only referenced
Secret
s in object cache?PR Readiness Checklist:
Complete these before marking the PR as
ready to review
:CHANGELOG.md
release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR