Fix events RBAC and automate Helm ClusterRole generation from markers (#292)

## Problem

### Two related issues:

1. Events permission used the wrong API group.
The +kubebuilder:rbac marker for events specified groups=events.k8s.io
(the newer structured events API), but controller-runtime records
v1.Event objects against the core "" API group. This caused the
controller to log Server rejected event (will not retry!) errors when
deployed in a different namespace from the TemporalWorkerDeployment CRs
it manages — common cluster-wide deployment pattern.

2. The Helm ClusterRole was hand-maintained and had drifted from the Go
markers.
The +kubebuilder:rbac markers in worker_controller.go were incomplete:
workerresourcetemplates, workerresourcetemplates/status, and
subjectaccessreviews were all present in the Helm chart but had no
corresponding markers. This made it easy for RBAC rules to fall out of
sync — which is exactly how the events bug happened in the first place.

## Changes

Fix the events API group (worker_controller.go, config/rbac/role.yaml):
correct groups=events.k8s.io → groups=core in the marker; the generated
manifest now uses apiGroups: [""].

Add missing markers (worker_controller.go): add markers for
workerresourcetemplates (get/list/watch/patch/update),
workerresourcetemplates/status (get/patch/update), and
authorization.k8s.io/subjectaccessreviews (create) to match what was
already deployed by the Helm chart.

Automate Helm ClusterRole sync (hack/sync-rbac-rules.py, Makefile,
helm/.../rbac.yaml): make manifests now runs a script that reads the
controller-gen-generated config/rbac/role.yaml and replaces the #
GENERATED RULES BEGIN / # GENERATED RULES END section in the Helm
template. The Helm-templated dynamic rules (the allowedResources range)
are left untouched. Going forward, adding a +kubebuilder:rbac marker and
running make manifests is all that's needed to update the deployed
ClusterRole.

Closes #277

Author: carlydf

.github/workflows/helm-validate.yml

@@ -74,13 +74,27 @@ jobs:
       - name: Template CRDs chart
         run: helm template test-release helm/temporal-worker-controller-crds
 
+  helm-check-rbac:
+    name: Check Helm RBAC wasn't manually updated
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-go@v5
+        with:
+          go-version-file: go.mod
+
+      - name: Regenerate and diff
+        run: make manifests && git diff --exit-code helm/temporal-worker-controller/templates/rbac.yaml
+
   helm-validate-succeed:
     name: All Helm Validations Succeed
     needs:
       - helm-lint
       - helm-template
       - helm-lint-crds
       - helm-template-crds
+      - helm-check-rbac
     runs-on: ubuntu-latest
     if: always()
     env:

Makefile

@@ -153,6 +153,7 @@ help: ## Display this help.
 manifests: controller-gen ## Generate ClusterRole and CustomResourceDefinition objects.
 	GOWORK=off GO111MODULE=on $(CONTROLLER_GEN) rbac:roleName=manager-role crd:allowDangerousTypes=true,maxDescLen=0,generateEmbeddedObjectMeta=true paths=./api/... paths=./internal/... paths=./cmd/... \
     output:crd:artifacts:config=helm/temporal-worker-controller-crds/templates
+	python3 hack/sync-rbac-rules.py
 
 .PHONY: generate
 generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.

hack/sync-rbac-rules.py

@@ -0,0 +1,65 @@
+#!/usr/bin/env python3
+"""Sync rules from config/rbac/role.yaml into the Helm ClusterRole template.
+
+The Helm template must contain:
+  # GENERATED RULES BEGIN
+  ...
+  # GENERATED RULES END
+markers inside its ClusterRole rules section.  Everything between the markers
+is replaced with the rules from config/rbac/role.yaml (indented by two spaces).
+"""
+import re
+import sys
+
+ROLE_YAML = "config/rbac/role.yaml"
+HELM_RBAC = "helm/temporal-worker-controller/templates/rbac.yaml"
+BEGIN_MARKER = "  # GENERATED RULES BEGIN"
+END_MARKER = "  # GENERATED RULES END"
+
+
+def extract_rules_text(path):
+    with open(path) as f:
+        content = f.read()
+    idx = content.find("\nrules:\n")
+    if idx == -1:
+        print(f"ERROR: 'rules:' not found in {path}", file=sys.stderr)
+        sys.exit(1)
+    rules_body = content[idx + len("\nrules:\n"):]
+    # Indent lines relative to the `rules:` key in the Helm template.
+    # controller-gen emits two indentation levels:
+    #   col 0: outer list items  (e.g. "- apiGroups:")   → add 2 spaces
+    #   col 2: inner list values (e.g. "  - events")     → add 4 spaces
+    #   col 2: mapping keys      (e.g. "  resources:")   → add 2 spaces
+    # The extra indent on inner list values matches the style used by the
+    # hand-authored rules in the Helm template.
+    lines = rules_body.splitlines(keepends=True)
+    result = []
+    for line in lines:
+        if not line.strip():
+            result.append(line)
+        elif line.startswith("  - "):
+            result.append("    " + line)  # inner list value: 2 global + 2 extra
+        else:
+            result.append("  " + line)    # outer list item or mapping key: 2 global
+    return "".join(result)
+
+
+def update_helm(path, rules_text):
+    with open(path) as f:
+        content = f.read()
+    pattern = re.compile(
+        r"(" + re.escape(BEGIN_MARKER) + r"[^\n]*\n)(.*?)(" + re.escape(END_MARKER) + r")",
+        re.DOTALL,
+    )
+    if not pattern.search(content):
+        print(f"ERROR: markers not found in {path}", file=sys.stderr)
+        sys.exit(1)
+    updated = pattern.sub(r"\g<1>" + rules_text + r"\g<3>", content)
+    with open(path, "w") as f:
+        f.write(updated)
+    print(f"Synced RBAC rules from {ROLE_YAML} → {HELM_RBAC}")
+
+
+if __name__ == "__main__":
+    rules = extract_rules_text(ROLE_YAML)
+    update_helm(HELM_RBAC, rules)

helm/temporal-worker-controller/templates/rbac.yaml

@@ -63,6 +63,14 @@ kind: ClusterRole
 metadata:
   name: {{ .Release.Name }}-{{ .Release.Namespace }}-manager-role
 rules:
+  # GENERATED RULES BEGIN - do not edit; run 'make manifests' to update
+  - apiGroups:
+      - ""
+    resources:
+      - events
+    verbs:
+      - create
+      - patch
   - apiGroups:
       - ""
     resources:
@@ -89,6 +97,12 @@ rules:
       - deployments/scale
     verbs:
       - update
+  - apiGroups:
+      - authorization.k8s.io
+    resources:
+      - subjectaccessreviews
+    verbs:
+      - create
   - apiGroups:
       - temporal.io
     resources:
@@ -119,6 +133,7 @@ rules:
       - temporal.io
     resources:
       - temporalworkerdeployments/status
+      - workerresourcetemplates/status
     verbs:
       - get
       - patch
@@ -130,30 +145,10 @@ rules:
     verbs:
       - get
       - list
-      - watch
       - patch
       - update
-  - apiGroups:
-      - temporal.io
-    resources:
-      - workerresourcetemplates/status
-    verbs:
-      - get
-      - patch
-      - update
-  - apiGroups:
-      - ""
-    resources:
-      - events
-    verbs:
-      - create
-      - patch
-  - apiGroups:
-      - authorization.k8s.io
-    resources:
-      - subjectaccessreviews
-    verbs:
-      - create
+      - watch
+  # GENERATED RULES END
   # Rules for managing resources attached via WorkerResourceTemplate.
   # Driven entirely by workerResourceTemplate.allowedResources in values.yaml.
   {{- range .Values.workerResourceTemplate.allowedResources }}

internal/controller/worker_controller.go

@@ -95,14 +95,17 @@ type TemporalWorkerDeploymentReconciler struct {
 	MaxDeploymentVersionsIneligibleForDeletion int32
 }
 
-//+kubebuilder:rbac:groups=temporal.io,resources=temporalworkerdeployments,verbs=get;list;watch;create;update;patch;delete
-//+kubebuilder:rbac:groups=temporal.io,resources=temporalworkerdeployments/status,verbs=get;update;patch
-//+kubebuilder:rbac:groups=temporal.io,resources=temporalworkerdeployments/finalizers,verbs=update
-//+kubebuilder:rbac:groups=temporal.io,resources=temporalconnections,verbs=get;list;watch
-//+kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch
-//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete
-//+kubebuilder:rbac:groups=apps,resources=deployments/scale,verbs=update
-// +kubebuilder:rbac:groups=events.k8s.io,resources=events,verbs=create;patch
+// +kubebuilder:rbac:groups=temporal.io,resources=temporalworkerdeployments,verbs=get;list;watch;create;update;patch;delete
+// +kubebuilder:rbac:groups=temporal.io,resources=temporalworkerdeployments/status,verbs=get;update;patch
+// +kubebuilder:rbac:groups=temporal.io,resources=temporalworkerdeployments/finalizers,verbs=update
+// +kubebuilder:rbac:groups=temporal.io,resources=temporalconnections,verbs=get;list;watch
+// +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch
+// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete
+// +kubebuilder:rbac:groups=apps,resources=deployments/scale,verbs=update
+// +kubebuilder:rbac:groups=core,resources=events,verbs=create;patch
+// +kubebuilder:rbac:groups=temporal.io,resources=workerresourcetemplates,verbs=get;list;watch;patch;update
+// +kubebuilder:rbac:groups=temporal.io,resources=workerresourcetemplates/status,verbs=get;patch;update
+// +kubebuilder:rbac:groups=authorization.k8s.io,resources=subjectaccessreviews,verbs=create
 
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.