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

feat: store only secrets that are possible to be used in object cache of dataplane #3047

Merged
merged 12 commits into from
Oct 26, 2022

Conversation

randmonkey
Copy link
Contributor

@randmonkey randmonkey commented Oct 13, 2022

What this PR does / why we need it:

Reduce secret cache size by only storing secrets that might be used. The secrets includes:

  • Secrets with label konghq.com/ca-cert: true
  • Secrets referred by: Service (in annotation konghq.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:

  • has label konghq.com/ca-cert: true
  • reference record could be found in the indexer.

There may still something missing, but the PR is ready for a comprehensive review.

  • Tests specially for the indexer and secret cache

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 Secrets in object cache?

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@randmonkey randmonkey requested a review from a team as a code owner October 13, 2022 04:07
@randmonkey randmonkey temporarily deployed to Configure ci October 13, 2022 04:07 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 13, 2022 04:29 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 13, 2022 04:29 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 13, 2022 05:53 Inactive
@randmonkey randmonkey marked this pull request as draft October 13, 2022 06:05
@randmonkey randmonkey temporarily deployed to Configure ci October 13, 2022 06:18 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 13, 2022 06:18 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 13, 2022 06:48 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 13, 2022 07:07 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 13, 2022 07:30 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 13, 2022 07:30 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 13, 2022 07:35 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 13, 2022 07:35 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 13, 2022 07:42 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 13, 2022 07:52 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 13, 2022 07:52 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 13, 2022 07:58 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 14, 2022 06:11 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 14, 2022 06:19 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 14, 2022 06:51 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 14, 2022 06:51 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 24, 2022 02:50 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 24, 2022 02:52 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 24, 2022 03:40 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 24, 2022 04:01 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 24, 2022 04:20 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 24, 2022 04:20 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 25, 2022 03:40 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 25, 2022 04:02 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 25, 2022 04:02 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 25, 2022 17:57 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 25, 2022 18:39 Inactive
Copy link
Contributor

@jrsmroz jrsmroz left a comment

Choose a reason for hiding this comment

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

lgtm

@randmonkey randmonkey temporarily deployed to Configure ci October 26, 2022 02:25 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 26, 2022 02:49 Inactive
@randmonkey randmonkey temporarily deployed to Configure ci October 26, 2022 02:49 Inactive
@randmonkey randmonkey merged commit ec0f270 into main Oct 26, 2022
@randmonkey randmonkey deleted the feat/reduce_secret_cache branch October 26, 2022 02:51
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
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.

Limit ingested Secrets to those actually used
4 participants