Closed mac-chaffee closed 1 year ago
This issue looks relevant: https://github.com/kubernetes-sigs/controller-runtime/issues/550#issuecomment-518818318
@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.
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: https://github.com/fluxcd/helm-controller/blob/a2147639031eb1362360b52c6f596c066270ecc4/main.go#L113
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: https://github.com/kubernetes-sigs/controller-runtime/pull/1249
@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?
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.
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.
I've noticed that when you specify
valuesFrom
in a HelmRelease, the helm-controller won't just fetch that one secret; it willlist
all secrets in the whole cluster instead: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: https://github.com/fluxcd/helm-controller/blob/5a7d0f8535406d155ed7d0634b27574538febed0/controllers/helmrelease_controller.go#L556 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