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

Suppress caching of Secrets and ConfigMaps #66

Merged
merged 2 commits into from
Apr 25, 2023

Conversation

squaremo
Copy link
Contributor

@squaremo squaremo commented Apr 24, 2023

Fixes #23.

As explained in
kubernetes-sigs/controller-runtime#1249, the controller-runtime client will tend to use LIST and WATCH to cache resources requested by the controller. This has two downsides:

  • the mechanism may require broad RBAC access (Secret and ConfigMap in every namespace) even if the actual secrets and configmaps referenced are few, or restricted to few namespaces;
  • the cache uses a lot of memory that it doesn't really need to

This change fixes those problems, with the trade-off that all Secret and ConfigMap requests use a round-trip to the Kubernetes API server.

This follows
fluxcd/source-controller#989. There, a feature flag can be used to restore caching for Secrets and ConfigMaps; I have not included the feature flag, since this controller is much younger and doesn't have an established behaviour.

@bigkevmcd
Copy link
Collaborator

Do we still need to watch secrets?

//+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch

@squaremo
Copy link
Contributor Author

Do we still need to watch secrets?

Good spot 🕵️ Probably not, but let me verify that.

squaremo and others added 2 commits April 25, 2023 13:19
As explained in
kubernetes-sigs/controller-runtime#1249, the
controller-runtime client will tend to use LIST and WATCH to cache
resources requested by the controller. This has two downsides:

 - the mechanism may require broad RBAC access (Secret and ConfigMap
   in every namespace) even if the actual secrets and configmaps
   referenced are few, or restricted to few namespaces;
 - the cache uses a lot of memory that it doesn't really need to

This change fixes those problems, with the trade-off that all Secret
and ConfigMap requests use a round-trip to the Kubernetes API server.

This follows
fluxcd/source-controller#989. There, a feature
flag can be used to restore caching for Secrets and ConfigMaps; I have
not included the feature flag, since this controller is much younger
and doesn't have an established behaviour.

Signed-off-by: Michael Bridgen <[email protected]>
One consequence of disabling caching for Secret resources, as in the
previous commit, is that `watch` permission for those is no longer
required.

Signed-off-by: Michael Bridgen <[email protected]>
@squaremo squaremo force-pushed the no-cache-secrets-configmaps branch from 0989d3e to e2ff348 Compare April 25, 2023 12:24
@squaremo
Copy link
Contributor Author

Do we still need to watch secrets?

Good spot detective Probably not, but let me verify that.

The controller doesn't watch them (in SetupWithManager), and the controller-runtime doesn't need watch if it's not implicitly caching the resources -> logically, watch is not needed. I've removed it from the kubebuilder annotations.

@bigkevmcd bigkevmcd merged commit a96fa7f into weaveworks:main Apr 25, 2023
@bigkevmcd bigkevmcd deleted the no-cache-secrets-configmaps branch April 25, 2023 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't cache Secrets in the client cache
2 participants