Skip to content

Fix unnecessary list+watch RBAC requirement on serviceaccounts - #1587

Open
tsaarni wants to merge 1 commit into
fluxcd:mainfrom
Nordix:disable-sa-cache
Open

Fix unnecessary list+watch RBAC requirement on serviceaccounts#1587
tsaarni wants to merge 1 commit into
fluxcd:mainfrom
Nordix:disable-sa-cache

Conversation

@tsaarni

@tsaarni tsaarni commented Sep 10, 2026

Copy link
Copy Markdown

This PR adds corev1.ServiceAccount to the client's cache exclusion list so the existence check in reconcileDelete() bypasses the cache and reads directly from the API server. This avoids creating a list+watch informer that requires list and watch RBAC on ServiceAccounts.

This same approach was used in #513 for Secrets and ConfigMaps. It is behind --feature-gate but I suggest always excluding ServiceAccount from the cache since it is only read once per HelmRelease delete. Watching all ServiceAccounts in the cluster for occasional lookups has no benefit and likely adds unnecessary overhead, and maybe it was accidental in the first place.

There is no automated test for this change. Testing the cache bypass behavior requires a cluster with restricted RBAC, which is bit complicated to set up. I will include manual test on Kind cluster in a comment.

Fixes #1586

Signed-off-by: Tero Saarni <tero.saarni@est.tech>
@tsaarni

tsaarni commented Sep 10, 2026

Copy link
Copy Markdown
Author

Here is manual test procedure for this PR

kind create cluster --name flux-repro

make docker-build IMG=fluxcd/helm-controller:without-sa-list-watch
kind load docker-image fluxcd/helm-controller:without-sa-list-watch --name flux-repro

kubectl apply -f https://github.com/fluxcd/flux2/releases/download/v2.9.5/install.yaml

# Run helm-controller with RBAC that lacks list and watch on serviceaccounts
kubectl apply -f - <<'EOF'
apiVersion: v1
kind: ServiceAccount
metadata:
  name: restricted
  namespace: flux-system
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: restricted
rules:
  - apiGroups: [helm.toolkit.fluxcd.io]
    resources: ['*']
    verbs: ['*']
  - apiGroups: [source.toolkit.fluxcd.io]
    resources: ['*']
    verbs: ['*']
  - apiGroups: ['']
    resources: [secrets, configmaps]
    verbs: [get, list, watch]
  - apiGroups: ['']
    resources: [events]
    verbs: [create, patch]
  - apiGroups: [coordination.k8s.io]
    resources: [leases]
    verbs: ['*']
  - apiGroups: ['']
    resources: [serviceaccounts]
    verbs: [get, impersonate]   # list and watch removed
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: restricted
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: restricted
subjects:
  - kind: ServiceAccount
    name: restricted
    namespace: flux-system
EOF
kubectl -n flux-system set serviceaccount deployment/helm-controller restricted
kubectl -n flux-system set image deployment/helm-controller manager=fluxcd/helm-controller:without-sa-list-watch
kubectl -n flux-system delete pod -l app=helm-controller --force


# ServiceAccount to impersonate, plus a HelmRelease that uses it
kubectl apply -f - <<'EOF'
apiVersion: v1
kind: ServiceAccount
metadata:
  name: deployer
  namespace: default
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: deployer
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: cluster-admin
subjects:
  - kind: ServiceAccount
    name: deployer
    namespace: default
---
apiVersion: source.toolkit.fluxcd.io/v1
kind: HelmRepository
metadata:
  name: podinfo
spec:
  url: https://stefanprodan.github.io/podinfo
---
apiVersion: helm.toolkit.fluxcd.io/v2
kind: HelmRelease
metadata:
  name: podinfo
spec:
  serviceAccountName: deployer
  interval: 5m
  chart:
    spec:
      chart: podinfo
      sourceRef:
        kind: HelmRepository
        name: podinfo
EOF

kubectl wait helmrelease/podinfo --for=condition=ready

# Delete does not hang anymore
kubectl delete helmrelease podinfo

# No errors logged anymore
kubectl -n flux-system logs deployment/helm-controller --since=30s | grep ServiceAccount

# Delete is successful
kubectl get deployment podinfo || echo "Deployment podinfo not found"

# Cleanup
kind delete cluster --name flux-repro

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.

HelmRelease delete unnecessarily requires list+watch RBAC on serviceaccounts

1 participant