fix(chart): grant cert-manager certificates RBAC to the operator ClusterRole#409
Merged
Conversation
…terRole The Helm chart's hand-maintained ClusterRole (charts/kagenti-operator/ templates/rbac/role.yaml) drifted from the kubebuilder-generated config/rbac/role.yaml: it was missing the cert-manager.io/certificates + clusterissuers get/list/watch rules. PR #383 (SharedTrust controller) added a Watches(&cmv1.Certificate{}) informer and the corresponding +kubebuilder:rbac marker, and regenerated config/rbac/role.yaml — but did not update the chart's separate ClusterRole. Since installs render RBAC from the chart, the deployed operator's ServiceAccount cannot list/watch cert-manager Certificates. The Certificate informer never syncs, the manager misses its cache-sync deadline and exits 1 (~2.5 min crashloop), so NO controllers run — including ClientRegistrationReconciler. Agents' per-agent Keycloak client-credentials secrets are therefore never created, and agent pods hang on FailedMount (observed on weather-agent; matches a user report on a fresh main install). Add the cert-manager.io rules to the chart role to match the generated role. Follow-up (not in this PR): the chart ClusterRole is maintained separately from config/rbac/role.yaml with no sync, so any future RBAC marker can silently drift again. Worth generating the chart role from the kubebuilder role or adding a CI drift check. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
mrsabath
approved these changes
Jun 6, 2026
mrsabath
left a comment
mrsabath
left a comment
Contributor
There was a problem hiding this comment.
Verified the fix against the kubebuilder-generated kagenti-operator/config/rbac/role.yaml on main: the added cert-manager.io certificates/clusterissuers get;list;watch block matches the generated role byte-for-byte, and the +kubebuilder:rbac markers in sharedtrust_controller.go (#383) confirm the root cause. The chart ClusterRole on main was indeed missing these rules. Read-only verbs only — no over-grant. All CI green incl. E2E.
Agreed the chart-role-vs-generated-role drift deserves a CI guard as a follow-up. 亡羊补牢 (mend the pen after losing a sheep) — better to close this gap before a third controller gets bitten.
Areas reviewed: Helm/RBAC, commit/PR conventions, security, CI · Commits: 1, signed-off: yes · CI: passing
github-project-automation
Bot
moved this from New /:ToDo
to Done
in Kagenti Issue Prioritization
kevincogan
pushed a commit
to kevincogan/kagenti-operator
that referenced
this pull request
Jun 18, 2026
Restore manual webhook manifest overrides clobbered by `make manifests`, add a deprecated annotation fallback in the mode resolution chain, and clean up minor code-review nits. Per-comment changes: rossoctl#1 (must-fix): Restore namespaceSelector / objectSelector / timeoutSeconds on the inject.kagenti.io webhook in config/webhook/manifests.yaml. PR rossoctl#357 manually re-added these fields after `make manifests` clobbered them; the prior commit on this branch wiped them again. Without them the webhook fires on every pod admission cluster-wide which, combined with the new proxy-sidecar default, is a much wider blast radius than today. rossoctl#3 (suggestion): Re-introduce AnnotationAuthBridgeMode as a deprecated fourth layer of the mode resolution chain (between the namespace ConfigMap and the cluster-wide proxy-sidecar fallback). Workloads that still set kagenti.io/authbridge-mode get the same mode they had before this PR, with a `DEPRECATED:` log line per pod admission to push them toward AgentRuntime.Spec.AuthBridgeMode. rossoctl#4 (suggestion): Add a TODO on InjectionDecision.SpiffeHelper marking it for rename to SpireEnabled in a follow-up PR. rossoctl#5 (suggestion): Replace the `evaluateSidecar("spiffe-helper", true, ...)` magic-true call with a dedicated `evaluateSpiffeHelper(label)` helper, mirroring the previous evaluateClientRegistration pattern. rossoctl#6 (nit): Update the proxy-sidecar test error message that still referenced the old "authbridge-light" image name. Comment rossoctl#2 (spiffe-helper bundling readiness) is a coordination concern between this PR and kagenti-extensions PR rossoctl#409. The combined Dockerfiles on rossoctl#409's branch (76d2cb3) DO bundle spiffe-helper and the entrypoint-proxy.sh / entrypoint-envoy.sh DO conditionally start it on SPIRE_ENABLED — so end-state is correct as long as rossoctl#409 lands first. No code change here; merge ordering is the answer. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
kevincogan
pushed a commit
to kevincogan/kagenti-operator
that referenced
this pull request
Jun 18, 2026
…ad code Three classes of fixes from a cross-PR audit against kagenti-extensions PR rossoctl#409 and the future kagenti chart PR: e2e tests * test/e2e/e2e_suite_test.go: drop authbridge-light + standalone spiffe-helper images from the Kind preload list (PR rossoctl#409 no longer publishes them). * test/e2e/e2e_test.go: replace assertions that expect a separate spiffe-helper container with assertions that envoy-proxy carries SPIRE_ENABLED=true. Spiffe-helper is now bundled inside the combined images and gated by SPIRE_ENABLED, so it does not appear as its own container. Resolution-chain unit-test coverage * pod_mutator_test.go: add five tests covering layers 2-4 of the mode resolution chain that were previously only exercised end-to-end: - namespace authbridge-runtime-config mode field selects mode - CR field beats namespace ConfigMap - deprecated kagenti.io/authbridge-mode annotation falls through - CR field beats annotation - cluster default is proxy-sidecar when nothing is set Dead-code cleanup * Remove PodMutator.EnableClientRegistration field + the enableClientRegistration parameter to NewPodMutator. The field was set in cmd/main.go to a hardcoded false and never read anywhere after the in-pod client-registration path was removed. * Remove SidecarDefaults / SidecarDefault types and the Sidecars.EnvoyProxy.Enabled value. Defined and logged but never read for injection decisions; FeatureGates.EnvoyProxy is the actual knob. Drop the matching defaults.sidecars block from charts/kagenti-operator/values.yaml. * Remove AuthBridgeContainerName ("authbridge"). It was the old combined-mode container name, never used to name an actually injected container after PR B. Drop the matching skip-list entry in authbridge_webhook.isAlreadyInjected and the obsolete "combined authbridge container (idempotency)" test. * Remove SpiffeHelperContainerName / ClientRegistrationContainerName constants and their entries in pod_mutator's port-stealing and HTTP_PROXY-injection skip-lists, plus the matching webhook isAlreadyInjected check. Neither container is ever injected anymore; the filters could never match. Net diff: +197 / -124. Test pass: full unit + integration suite green. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On a fresh chart install, the operator (
kagenti-controller-manager) crashloops and never becomes Ready, which silently breaks agent onboarding: agent pods hang inInitwithRoot cause
The Helm chart's hand-maintained ClusterRole (
charts/kagenti-operator/templates/rbac/role.yaml) drifted from the kubebuilder-generatedconfig/rbac/role.yaml: it is missing thecert-manager.iocertificates/clusterissuersget;list;watchrules.#383 (SharedTrust controller) added
Watches(&cmv1.Certificate{})plus the+kubebuilder:rbac:groups=cert-manager.io,resources=certificatesmarker and regeneratedconfig/rbac/role.yaml— but did not update the chart's separate ClusterRole. Since installs render RBAC from the chart, the deployed ServiceAccountkagenti-system:controller-managercannot list/watch cert-managerCertificates:That informer never syncs → the manager misses its cache-sync deadline and exits 1 (~2.5 min crashloop) → no controllers run, including
ClientRegistrationReconciler. So agents are never registered in Keycloak, their per-agent client-credentials secrets are never created, and the webhook-mounted secret volume never resolves → permanentFailedMount.Reproduced on
weather-agent(team1) on a fresh install ofmain; matches a user report (#383-era). Confirmed the fix on a live cluster: granting these rules + restarting the operator → Ready → reconciler creates the secret → agent reaches2/2 Running.Fix
Add the
cert-manager.iocertificates/clusterissuersget;list;watchrules to the chart ClusterRole to match the generatedconfig/rbac/role.yaml.helm lint+helm templateverified.Follow-up (separate)
The chart ClusterRole is maintained independently of
config/rbac/role.yamlwith no sync step, so any future+kubebuilder:rbacmarker can silently drift again (this is the second controller bitten — see also the MLflow/OTEL RBAC edits in #404/#378 that hand-patch the same file). Worth generating the chart role from the kubebuilder role, or adding a CI check that fails on drift.Assisted-By: Claude (Anthropic AI) noreply@anthropic.com