feat: verify chart RBAC does not drift from config/rbac

[tejassinghbhati]

Description

Follows up on @ajaysundark's question in #351 about whether hack/verify-chart-drift.sh could cover more than CRDs. This extends it to RBAC.

The script previously diffed only the bundled CRD, so nothing caught the chart granting different permissions than the controller declares. #350 was exactly that, and it went unnoticed precisely because nothing compares the two.

RBAC is the case most likely to drift. config/rbac/role.yaml is generated by controller-gen from the kubebuilder markers, so it updates itself whenever the controller's permissions change. The chart's ClusterRole is hand written, so it only updates when someone remembers. Every future permission change is another chance for the chart to fall behind, and the failure is silent apart from whatever the missing grant covered.

A plain diff does not work here. The generated roles carry short names and kustomize labels, the chart templates the name from the release name and adds Helm labels, so the files never match textually even when the permissions are identical. The check therefore renders the chart, matches each Role and ClusterRole by name ignoring the release prefix, and compares only the rules.

Rules are normalised before comparison, sorting the entries within each rule and then the rules themselves, because the order of apiGroups, resources and verbs means nothing to the API server and should not be reported as drift. Without that the check would be noisy enough that people would learn to ignore it.

Output points at the offending permission rather than dumping both role definitions:

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]

The chart grants different permissions than the controller declares.
Update charts/nrr-controller/templates/rbac.yaml to match config/rbac.

A role the chart renders that has no counterpart in config is reported as a note rather than a failure, since the chart may legitimately ship something kustomize does not.

Merge order. This depends on #351. Run against main as it stands today the check fails, because it correctly finds the missing events.k8s.io grant that #351 fixes. That failure is the proof it works, but it does mean this should land after #351, or CI on main will flag a bug that already has a fix in flight.

Scope. Roles and ClusterRoles only, not bindings, subjects, the Deployment, Services or webhook configuration. I checked all of those by hand while reviewing #351 and they currently agree, so RBAC seemed the right place to start given it is the one that has already bitten us. Easy to extend later if that is wanted.

Related Issue

Fixes #371

Type of Change

/kind feature

Testing

Verified both directions against a real drift rather than a synthetic one.

Against main, where the chart is still missing the events grant, the check fails with the output shown above and exits 1. With #351's template change applied, it passes:

RBAC in the Helm chart matches config/rbac (7 roles compared)

All seven roles are compared: manager-role, leader-election-role, metrics-auth-role, metrics-reader, and the nodereadinessrule admin, editor and viewer roles.

golangci-lint v2.12.1 reports 0 issues across the repo including the new tool, gofmt is clean, and the boilerplate header matches hack/boilerplate/boilerplate.go.txt exactly. bash -n passes on the modified script.

One note on local verification: I could not run the script end to end on Windows, because make manifests fails there with no Go files in E:\... from controller-gen's path handling. That reproduces identically on a clean checkout of main, so it is pre-existing and unrelated to this change. I ran the RBAC section verbatim instead, and CI runs on ubuntu-latest where the CRD step works.

The Helm workflow already sets up Helm, Go and Python before invoking this script, so no workflow change is needed. If helm is missing locally the script now fails with a message pointing at make ensure-helm-install rather than skipping silently.

Checklist

• make test passes
• make lint passes

Does this PR introduce a user-facing change?

NONE

[netlify]

✅ Deploy Preview for node-readiness-controller canceled.

Name	Link
🔨 Latest commit	018ca4d
🔍 Latest deploy log	https://app.netlify.com/projects/node-readiness-controller/deploys/6a7632e5a1f0e900089ce2c2

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tejassinghbhati
Once this PR has been reviewed and has the lgtm label, please assign haircommander for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[kubernetes-prow]

Hi @tejassinghbhati. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.