Skip to content

refactor(helm): remove rbac.clusterScoped, derive RBAC scope from rbac.namespaces#1728

Merged
EItanya merged 4 commits intokagent-dev:mainfrom
supreme-gg-gg:fix/rbac-ns-chart
Apr 22, 2026
Merged

refactor(helm): remove rbac.clusterScoped, derive RBAC scope from rbac.namespaces#1728
EItanya merged 4 commits intokagent-dev:mainfrom
supreme-gg-gg:fix/rbac-ns-chart

Conversation

@supreme-gg-gg
Copy link
Copy Markdown
Contributor

Collapses the redundant rbac.clusterScoped boolean into rbac.namespaces alone. Empty list creates ClusterRole/ClusterRoleBinding (default, unchanged), non-empty list creates namespaced Role/RoleBinding per listed namespace plus auto-derived WATCH_NAMESPACES. Adds helm fail guards so setting the removed field or omitting the install namespace from rbac.namespaces errors loudly instead of silently changing scope. This avoids confusion when the user sees clusterScoped: true but namespaces: [a, b, c]

Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>
Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>
Copilot AI review requested due to automatic review settings April 22, 2026 15:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the Helm chart’s RBAC configuration to derive RBAC scope solely from rbac.namespaces, removing the redundant rbac.clusterScoped flag and adding explicit guardrails to prevent ambiguous or silently-changing configurations.

Changes:

  • Remove rbac.clusterScoped and make RBAC scope depend only on whether rbac.namespaces is empty vs non-empty.
  • Add Helm template validation to fail rendering if rbac.clusterScoped is set or if rbac.namespaces omits the install namespace.
  • Update Helm unit tests and CI workflow to match the new RBAC configuration semantics, including deriving WATCH_NAMESPACES from rbac.namespaces.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
helm/kagent/values.yaml Updates user-facing RBAC values documentation to rely on rbac.namespaces only.
helm/kagent/tests/rbac_test.yaml Updates/extends RBAC rendering tests and adds failure-guard tests.
helm/kagent/tests/controller-deployment_test.yaml Adds tests ensuring WATCH_NAMESPACES derives from rbac.namespaces unless explicitly overridden.
helm/kagent/templates/rbac/writer-rolebinding.yaml Switches RBAC rendering logic to key off rbac.namespaces and adds validation include.
helm/kagent/templates/rbac/writer-role.yaml Same RBAC scope refactor + validation include for writer Role/ClusterRole.
helm/kagent/templates/rbac/getter-rolebinding.yaml Same RBAC scope refactor + validation include for getter RoleBinding/ClusterRoleBinding.
helm/kagent/templates/rbac/getter-role.yaml Same RBAC scope refactor + validation include for getter Role/ClusterRole.
helm/kagent/templates/_helpers.tpl Updates watch namespace derivation precedence and adds kagent.rbac.validate guards.
.github/workflows/ci.yaml Removes usage of the deleted rbac.clusterScoped flag in e2e workflow overrides.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread helm/kagent/templates/rbac/getter-role.yaml Outdated
Comment thread helm/kagent/templates/rbac/getter-rolebinding.yaml Outdated
Comment thread helm/kagent/templates/rbac/writer-role.yaml Outdated
Comment thread helm/kagent/templates/rbac/writer-rolebinding.yaml Outdated
supreme-gg-gg and others added 2 commits April 22, 2026 12:26
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Jet Chiang <jetjiang.ez@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Jet Chiang <jetjiang.ez@gmail.com>
@EItanya EItanya merged commit 0ad6b4a into kagent-dev:main Apr 22, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants