🤖 fix: make generated RBAC the single source of truth#60
Merged
Conversation
- add MCP kubebuilder RBAC markers and regenerate manager-role permissions - move ServiceAccount/bindings into config/rbac and remove duplicated e2e/deploy RBAC files - update kind/CI/doc flows to apply config/rbac instead of deploy/rbac.yaml --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `openai:gpt-5.3-codex` • Thinking: `xhigh` • Cost: `$0.93`_ <!-- mux-attribution: model=openai:gpt-5.3-codex thinking=xhigh costs=0.93 -->
Member
Author
|
@codex review Please review this RBAC single-source consolidation PR. |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Member
Author
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
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.
Summary
Consolidate RBAC manifests so
config/rbac/is the single source of truth for in-cluster RBAC, remove duplicated RBAC files, and update tooling/docs to apply RBAC fromconfig/rbac/.Background
deploy/rbac.yamlhad drifted from generated RBAC and was missing permissions needed by controllers/MCP flows. This caused real-cluster informer sync failures due to forbiddenlist/watchoperations.Implementation
+kubebuilder:rbacmarkers ininternal/app/mcpapp/server.gofor events, namespaces, pods, pod logs, services, deployments, and aggregated API resources.config/rbac/role.yamlincludes the MCP-required permissions.config/rbac/:serviceaccount.yamlclusterrolebinding.yamlauth-delegator-binding.yamlauthentication-reader-binding.yamldeploy/rbac.yamlconfig/e2e/serviceaccount.yamlconfig/e2e/clusterrole-binding.yamlhack/kind-dev.shnow applies namespace +config/rbac/(no e2e RBAC duplicates)config/e2e/deployment.yamlkubectl apply -f config/rbac/.Validation
make manifestsmake verify-vendormake testmake buildmake lintmake docs-checkRisks
Low-to-moderate. This touches deployment RBAC wiring and removes legacy manifest paths. The mitigations are generated RBAC from source markers, CI/developer flow updates, and full local validation.
📋 Implementation Plan
Plan: Make generated RBAC the single source of truth (remove deploy/rbac.yaml)
Context / Why
Today we have two RBAC definitions:
config/rbac/role.yaml(generated bycontroller-genfrom+kubebuilder:rbacmarkers; ClusterRolemanager-role).deploy/rbac.yaml(hand-curated “one file to apply”; ClusterRolecoder-k8s).These have drifted:
deploy/rbac.yamlis missing permissions that the controllers (notablyCoderProvisioner) require (list/watchon secrets/serviceaccounts/roles/rolebindings). In a real cluster this causes controller-runtime informers to fail to sync and the operator to exit/restart.Goal: Eliminate drift by making the generated RBAC under
config/rbac/the only source of RBAC rules, and update docs/examples to applyconfig/rbac/instead ofdeploy/rbac.yaml.Evidence
hack/update-manifests.sh(controller-gen rbac:roleName=manager-role … output:rbac:artifacts:config=config/rbac).… is forbidden: User "system:serviceaccount:coder-system:coder-k8s" cannot list …for secrets/serviceaccounts/roles/rolebindings.kubectl auth can-i --as=system:serviceaccount:coder-system:coder-k8s list secrets→no.deploy/rbac.yamlcurrently grantssecrets: [get]only, while controllers and MCP tools use list/watch/update APIs.deploy/rbac.yamlin:docs/how-to/deploy-controller.mddocs/how-to/deploy-aggregated-apiserver.mddocs/how-to/mcp-server.mdexamples/cloudnativepg/README.mdImplementation plan
1) Ensure generated ClusterRole contains all required permissions (controller + MCP)
Add missing
+kubebuilder:rbacmarkers for MCP tointernal/app/mcpapp/server.go(orhttp.go) socontroller-genincludes them inconfig/rbac/role.yaml.MCP tools currently call:
clientset.CoreV1().Events(ns).List(...)→ needsevents: list.clientset.CoreV1().Pods(ns).GetLogs(...).Stream(...)→ needspods/log: get.clientset.CoreV1().Namespaces().List(...)→ needsnamespaces: list.pods(list)deployments(get)services(get)coderworkspaces,codertemplates(get/list/update)Suggested marker set (exact verbs can be trimmed later, but start with the union needed for current code paths):
Run
make manifeststo regenerateconfig/rbac/role.yaml(and ensure it now contains the MCP permissions).(Optional but recommended) Add a CI/Makefile guardrail:
make verify-manifeststhat runsbash ./hack/update-manifests.shand fails ifgit diff --exit-codeis non-empty..github/workflows/ci.yamlso generated RBAC/CRDs can’t drift from code.Why RBAC markers in MCP code (not deploy YAML)?
We want the permissions list to be generated from actual code usage. MCP currently needed permissions were only encoded in
deploy/rbac.yaml, which is how we ended up with drift. Adding markers makesconfig/rbac/role.yamlthe authoritative rule set.2) Move “apply-time” RBAC objects into
config/rbac/controller-genonly outputs the ClusterRole rules. We still need a ServiceAccount + bindings for in-cluster runs.Add the following non-generated manifests under
config/rbac/(these are glue; the rules remain generated):serviceaccount.yaml(SAcoder-k8sincoder-system)clusterrolebinding.yamlbinding SA → ClusterRolemanager-roleauth-delegator-binding.yaml(ClusterRoleBinding SA →system:auth-delegator) for aggregated apiserver delegationauthentication-reader-binding.yaml(RoleBinding inkube-systemtoextension-apiserver-authentication-reader)These resources should match what
deploy/rbac.yamlprovided without duplicating the ClusterRole rules.Remove/stop using duplicated RBAC manifests elsewhere:
config/e2e/serviceaccount.yamlconfig/e2e/clusterrole-binding.yaml…and update:
hack/kind-dev.sh upto stop applying those files (sinceconfig/rbac/will now include them).config/e2e/deployment.yaml(or apply the directory after removing the RBAC files).3) Retire
deploy/rbac.yamldeploy/rbac.yaml(or replace it with a clear deprecation stub if removal is too disruptive).4) Update docs + examples to use
config/rbac/Update commands everywhere RBAC is applied:
docs/how-to/deploy-controller.mdkubectl apply -f deploy/rbac.yaml→kubectl apply -f config/rbac/docs/how-to/deploy-aggregated-apiserver.mddocs/how-to/mcp-server.mdexamples/cloudnativepg/README.mdAlso update any narrative text that claims
deploy/rbac.yamlis the unified ServiceAccount RBAC.5) Validation (local)
make manifestsgit diff --exit-code(no uncommitted changes after regen)./hack/kind-dev.sh upkubectl apply -f deploy/deployment.yaml(or the e2e deployment)--app=mcp-httpand confirm tools that touch events/pod logs/namespaces work.kubectl get coderworkspaces.aggregation.coder.com -Asucceeds.Deliverables
deploy/rbac.yamlremoved (or deprecated) and not referenced.config/rbac/is the one-stop directory to apply RBAC for in-cluster deployments.+kubebuilder:rbacmarkers.config/rbac/.Generated with
mux• Model:openai:gpt-5.3-codex• Thinking:xhigh• Cost:$0.93