Skip to content

[BUG] Helm chart RBAC is missing the events.k8s.io group, so taint events are never recorded #350

Description

@tejassinghbhati

What happened?

The Helm chart's manager ClusterRole does not grant permission on the events.k8s.io API group, so the controller cannot create the Node events it emits for taint operations. Every TaintAdded, TaintRemoved and TaintAdopted event is rejected by the API server when the controller is installed with Helm.

The kubebuilder markers on the controller declare both API groups (nodereadinessrule_controller.go#L97-L98):

// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch
// +kubebuilder:rbac:groups=events.k8s.io,resources=events,verbs=create;patch

and the generated config/rbac/role.yaml reflects that:

- apiGroups:
  - ""
  - events.k8s.io
  resources:
  - events
  verbs:
  - create
  - patch

The chart's ClusterRole is maintained by hand and only carries the core group. Rendering it shows the gap:

$ helm template nrr-controller charts/nrr-controller --show-only templates/rbac.yaml
...
  name: nrr-controller-manager-role
rules:
  - apiGroups: [""]
    resources: ["events"]
    verbs: ["create", "patch"]
  - apiGroups: [""]
    resources: ["nodes"]
    verbs: ["get", "list", "patch", "update", "watch"]
...

The core group entry is not sufficient, because the controller writes through the newer events API rather than the legacy one. RuleReadinessController.EventRecorder is populated from mgr.GetEventRecorder(...), whose type is k8s.io/client-go/tools/events.EventRecorder. In controller-runtime v0.24.1 that recorder is backed by a broadcaster built in pkg/manager/manager.go:

evtCl, err := eventsv1client.NewForConfigAndClient(config, httpClient)
...
return record.NewBroadcaster(), events.NewBroadcaster(&events.EventSinkImpl{Interface: evtCl}), true

eventsv1client is k8s.io/client-go/kubernetes/typed/events/v1, so the writes land on events.k8s.io/v1 and are authorised against the events.k8s.io group. With the chart's role the API server returns a forbidden error instead, and the events never appear.

This only affects the Helm installation path. Installing with kustomize applies the generated config/rbac/role.yaml, which has the correct permissions, so the behaviour differs between the two documented install methods.

Worth noting that node events for taint operations were added deliberately in #134 and PR #158 and are described in the docs as the way to observe what the controller did to a node. On a Helm install that observability is silently missing, and the only hint is repeated forbidden errors in the controller log.

I think the underlying reason this slipped through is that nothing verifies the chart RBAC against the generated role. hack/verify-chart-drift.sh runs in the Helm workflow but only diffs the CRD:

diff -u \
  config/crd/bases/readiness.node.x-k8s.io_nodereadinessrules.yaml \
  charts/nrr-controller/crds/nodereadinessrules.readiness.node.x-k8s.io.yaml

so any divergence in the ClusterRole goes unnoticed. Comparing the two by hand, the events rule is currently the only difference; nodes, nodes/status and the three nodereadinessrules rules all match.

Steps to Reproduce

  1. Install the controller with Helm, with rbac.create left at its default of true.
  2. Create a rule whose condition a node does not satisfy, so the controller applies a taint.
  3. Confirm the taint was applied: kubectl get node <node> -o jsonpath='{.spec.taints}'.
  4. Look for the corresponding event: kubectl get events --field-selector involvedObject.name=<node>. No TaintAdded event is recorded.
  5. The controller log shows the write being refused, along the lines of events.events.k8s.io is forbidden: User "system:serviceaccount:<ns>:nrr-controller" cannot create resource "events" in API group "events.k8s.io".

You can see the missing grant without a cluster:

$ helm template nrr-controller charts/nrr-controller --show-only templates/rbac.yaml | grep -A2 'resources: \["events"\]'
  - apiGroups: [""]
    resources: ["events"]
    verbs: ["create", "patch"]

compared with config/rbac/role.yaml, which lists "" and events.k8s.io together.

Expected Behavior

The chart's manager ClusterRole should grant the same permissions the controller actually needs, matching the generated config/rbac/role.yaml, so taint events are recorded on a Helm install exactly as they are on a kustomize install.

Controller Version / Image Tag

main (commit 021cd1d), chart nrr-controller-0.1.0, appVersion v0.4.1

Kubernetes Version

Not version specific. The chart was rendered with Helm v3.15.1, the version pinned in the Helm workflow.

Additional Environment Details

I searched the existing issues and PRs first and did not find this reported. Of the open PRs that touch the chart, #340 changes templates/deployment.yaml for KubeLinter and #315 changes the bundled CRD, so neither overlaps with the ClusterRole.

Happy to send a PR adding the missing group, along with a chart unit test so it cannot regress. Extending the drift check to cover RBAC as well as CRDs seems worthwhile too, but the generated role and the templated one differ in name and labels so a plain diff will not work, and that feels like a separate piece of work rather than something to fold into the fix.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions