kubernetes-sigs / kubernetes-sigs/node-readiness-controller
[Helm] Catch RBAC drift between config/rbac and the Helm chart
- Dominant language
- Go
- Stars
- 163
- Forks
- 74
- Avg merge
- 2d 18h
- Merged PRs (30d)
- 9
Description
### What happened?
Nothing verifies that the Helm chart grants the same permissions the controller declares, so RBAC can drift silently between `config/rbac` and `charts/nrr-controller/templates/rbac.yaml`.
This is not hypothetical. #350 is exactly that: the chart's manager ClusterRole was missing the `events.k8s.io` group, so every taint event was refused on a Helm install while the kustomize path worked fine. It sat there unnoticed because nothing compares the two.
`hack/verify-chart-drift.sh` runs in the Helm workflow, but it only diffs the CRD:
```bash
diff -u \
config/crd/bases/readiness.node.x-k8s.io_nodereadinessrules.yaml \
charts/nrr-controller/crds/nodereadinessrules.readiness.node.x-k8s.io.yaml
```
The gap matters more for RBAC than for most resources. `config/rbac/role.yaml` is generated by controller-gen from the kubebuilder markers on the controller, so it updates itself whenever the controller's permissions change. The chart's ClusterRole is hand written, so it only updates when someone remembers. That is a one-way ratchet: every future permission change is another chance for the chart to fall behind, and the failure mode is silent because the controller keeps working apart from whatever the missing grant covered.
Raised by @ajaysundark on #351, who asked whether there is room to expand the drift check to cover resources beyond CRDs, and cc'd @honghainguyen777.
### Steps to Reproduce
1. Add a permission to the controller, for example a new `+kubebuilder:rbac` marker.
2. Run `make manifests`. `config/rbac/role.yaml` picks it up.
3. Leave `charts/nrr-controller/templates/rbac.yaml` untouched.
4. Run `hack/verify-chart-drift.sh` and CI. Both pass.
5. Install with Helm. The controller is denied on the new permission at runtime.
The same sequence with a CRD change is caught immediately, which is the inconsistency.
### Expected Behavior
The drift check should fail when the chart's rendered RBAC does not grant what `config/rbac` grants, in the same way it already fails on CRD drift.
### Controller Version / Image Tag
`main` (commit `af1f57d`)
### Kubernetes Version
Not version specific, this is a repo tooling gap.
### Additional Environment Details
A plain `diff` will not work here. The generated roles carry short names and kustomize labels, while the chart templates the name from the release name and adds Helm labels, so the two files never match textually even when the permissions are identical. It needs to render both sides and compare the rules semantically, and it should normalise ordering, since the order of `apiGroups`, `resources` and `verbs` within a rule means nothing to the API server and should not be reported as drift.
I have this working and will open a PR. Against current `main` it reports:
```
RBAC drift between config/rbac and the Helm chart:
ClusterRole "manager-role" has different rules
only in config: apiGroups=["",events.k8s.io] resources=[events] verbs=[create,patch]
only in chart: apiGroups=[""] resources=[events] verbs=[create,patch]
```
which is #350, found without anyone knowing to look for it. With #351 applied it reports `RBAC in the Helm chart matches config/rbac (7 roles compared)`.
Worth noting the scope this does not cover, in case it is wanted later. It compares Roles and ClusterRoles only, not bindings or subjects, and it does not look at the Deployment, Services or webhook configuration. I checked all of those by hand while reviewing #351 and they currently agree, so RBAC seemed like the right place to start given it is the one that has already bitten us.
Contributor guide
Research direction
Start with hack/verify-chart-drift.sh, config/rbac/role.yaml, and charts/nrr-controller/templates/rbac.yaml; run make manifests and inspect the existing Helm workflow check. Render both RBAC sources, compare Roles and ClusterRoles semantically with normalized rule ordering, and verify the check reports the #350 mismatch while passing when the seven roles match.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, helm, kubernetes, shell
- Domain
- ci-cd, devops, tooling
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100