Flux unnecessarily fetching all secrets in the cluster

[mac-chaffee]

I've noticed that when you specify valuesFrom in a HelmRelease, the helm-controller won't just fetch that one secret; it will list all secrets in the whole cluster instead:

W0802 16:18:16.709401       7 reflector.go:324] pkg/mod/k8s.io/client-go@v0.24.1/tools/cache/reflector.go:167: 
  failed to list *v1.Secret: secrets is forbidden: User "system:serviceaccount:flux-system:helm-controller" 
  cannot list resource "secrets" in API group "" at the cluster scope

I understand Flux is meant to be installed cluster-wide on the assumption that you can use Flux to install stuff in any namespace, but I don't have any intention of doing that in e.g. "kube-system", so I've deployed flux with custom permissions that only allow it to read secrets in specific namespaces. So that list operation fails for me.

I believe the list operation is triggered because of the client-go caching mechanism. The helm-controller's code doesn't perform a list:

helm-controller/controllers/helmrelease_controller.go

Line 556 in 5a7d0f8

if err := r.Get(ctx, namespacedName, resource); err != nil {

But maybe the cache config of the controller-runtime client may have the answer? https://github.com/kubernetes-sigs/controller-runtime/blob/f0351217e9e026533aece74d054b23cae5441659/pkg/client/client.go#L79

[mac-chaffee]

This issue looks relevant: kubernetes-sigs/controller-runtime#550 (comment)

[pjbgf]

@mac-chaffee On a quick search around the code (including the extract you shared), we tend to do a Get with a namespaced name which should restrict the requirements at a namespace level. The audit logs may provide further insights as to how this can be overcome - it would be great if you could share the logs here.

A good way to identify RBAC permissions that are missing is to export the audit logs and run it through audit2rbac.

[mac-chaffee]

The caching happens at the cluster scope regardless of whether the Get is namespace-scoped because the cache config is not namespace-scoped.

When creating the Manager here:

helm-controller/main.go

Line 113 in a214763

mgr, err := ctrl.NewManager(restConfig, ctrl.Options{

no namespace is specified (EDIT: unless you restrict Flux to a single namespace rather than a few specific namespaces), which means all secrets in the cluster are fetched and cached: https://github.com/kubernetes-sigs/controller-runtime/blob/1e4d87c9f9e15e4a58bb81909dd787f30ede7693/pkg/manager/manager.go#L188-L194

More info here: kubernetes-sigs/controller-runtime#1249

[pjbgf]

@mac-chaffee I tested this and could only reproduce this behaviour when the controller was started with --watch-all-namespaces (the default setting):

{"serviceaccount":"system:serviceaccount:flux-system:helm-controller","verb":"list","resourceType":"secrets","resourceNs":"flux-system","decision":"allow"}
{"serviceaccount":"system:serviceaccount:flux-system:helm-controller","verb":"list","resourceType":"secrets","resourceNs":null,"decision":"allow"}
{"serviceaccount":"system:serviceaccount:flux-system:helm-controller","verb":"update","resourceType":"secrets","resourceNs":"flux-system","decision":"allow"}
{"serviceaccount":"system:serviceaccount:flux-system:helm-controller","verb":"watch","resourceType":"secrets","resourceNs":null,"decision":"allow"}

Note that the second and fourth operations do not have a namespace associated with it. This is the expected behaviour as in this mode Flux is operating at cluster level. However, by starting the controller with --watch-all-namespaces=false, all the kube apiserver requests for the Secret kind are namespace bound:

{"serviceaccount":"system:serviceaccount:flux-system:helm-controller","verb":"create","resourceType":"secrets","resourceNs":"flux-system","decision":"allow"}
{"serviceaccount":"system:serviceaccount:flux-system:helm-controller","verb":"list","resourceType":"secrets","resourceNs":"flux-system","decision":"allow"}
{"serviceaccount":"system:serviceaccount:flux-system:helm-controller","verb":"update","resourceType":"secrets","resourceNs":"flux-system","decision":"allow"}
{"serviceaccount":"system:serviceaccount:flux-system:helm-controller","verb":"watch","resourceType":"secrets","resourceNs":"flux-system","decision":"allow"}

Can you confirm what's the value of your --watch-all-namespaces flag please?

[mac-chaffee]

I have --watch-all-namespaces=true because Flux's utility is limited if you can only deploy to in one namespace. Someone who only wants to use Flux in a "dev" and a "prod" namespace has to choose between granting Flux access to every secret in their whole cluster, or not using Flux at all. Like a flashlight app refusing to work unless you grant it access to your contacts :)

Ideally, there would be three deployment options:

• --watch-all-namespaces=true: Truly cluster-wide installation where the user does intend to use Flux in almost every single namespace
• --watch-all-namespaces=false: Single-namespace Flux, maybe useful for non-cluster-admins who can only access one namespace anyway
• --watch-all-namespaces=false, --but-not-just-one-namespace: Disables cluster-wide caching (ref), which allows Flux to operate in multiple specific namespaces while following the principle of least privilege.

I've been testing out that third deployment model and it appears to work with the exception of the helm-controller fetching secrets.

[pjbgf]

As of now, we may not be able to completely remove the list verb from the controller's service account - for the reasons you mentioned above. However, by using impersonation and keeping --watch-all-namespaces=true you may get your tenant's service accounts least-privileged and restricted to their own namespaces.

For that use the .spec.ServiceAccount field of HelmRelease so the controller will impersonate the given service account during that object's reconciliation.

The controller service account would still do a list operation across the cluster though.

{"serviceaccount":"system:serviceaccount:flux-system:helm-controller","verb":"list","resourceType":"secrets","resourceNs":null,"decision":"allow"}

The secrets needed for that specific HelmRelease would be namespace-bound, using the specific service account impersonated via the controller's service-account.

{"serviceaccount":"system:serviceaccount:flux-system:tenant","verb":"create","resourceType":"secrets","resourceNs":"tenant-ns","decision":"allow"}
{"serviceaccount":"system:serviceaccount:flux-system:tenant","verb":"list","resourceType":"secrets","resourceNs":"tenant-ns","decision":"allow"}
{"serviceaccount":"system:serviceaccount:flux-system:tenant","verb":"update","resourceType":"secrets","resourceNs":"tenant-ns","decision":"allow"}

You may also be interested in enforcing control plane level boundaries via admission controllers, as we show in our end-to-end example for multi-tenancy: https://github.com/fluxcd/flux2-multi-tenancy.