charts: omit namespace from KueueViz cluster RBAC - #14816
Conversation
✅ Deploy Preview for kubernetes-sigs-kueue ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
|
|
Welcome @lokesh0186! |
|
Hi @lokesh0186. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change removes invalid namespace metadata from KueueViz cluster-scoped RBAC resources and adds Helm tests for cluster-scoped and namespaced resource behavior. ChangesRBAC namespace scope
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to The KueueViz manifests now omit invalid namespace metadata from cluster-scoped RBAC resources while retaining namespace on valid namespaced resources. The localized change is covered by regression tests and validation, so no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
|
/ok-to-test |
This is not right. Even if you specify namespace for cluster roles, kubernetes doesn't reject it. |
|
/remove-kind bug |
|
/release-note-edit |
|
But, we can backport this PR as same as previous Helm chart cleanup PRs regardless of PR kind. |
|
LGTM label has been added. DetailsGit tree hash: 3e5a3ba74c9d6252a6fde75e72fcfe29c65ccc82 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lokesh0186, tenzen-y The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/cherrypick release-0.18 |
|
@tenzen-y: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Thanks for the correction, you're right. I was validating the RBAC object directly and missed the kube-apiserver namespace normalization that occurs before validation. I've corrected the description. Thanks for reviewing and for the backport guidance. |
|
@tenzen-y: new pull request created: #14824 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@tenzen-y: new pull request created: #14825 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
KueueViz currently emits
metadata.namespaceon itsClusterRoleandClusterRoleBindingeven though both are cluster-scoped resources. Kubernetesnormalizes away namespace metadata for cluster-scoped resources before object
validation, so this is a manifest cleanup rather than an API rejection bug.
This change keeps the rendered RBAC manifests aligned with their resource scope
while preserving namespace where it is valid, including the ServiceAccount
subject and namespaced Role/RoleBinding resources.
Regression coverage uses a non-default Helm release namespace to verify that it
does not leak into cluster-scoped RBAC.
I independently reproduced the redundant rendered namespace metadata with
IaC-Guard-V 0.1.0a6; the supporting reproduction is available at:
https://github.com/lokesh0186/iac-guard-v/tree/4a55d51d852e36a5b64a7ee08e16254b7728078b/examples/public-reproductions/kueue-cluster-rbac-namespace
The immutable reproduction preserves the original direct-object-validation
interpretation. The corrected characterization here reflects the complete
kube-apiserver create path, which clears namespace metadata for cluster-scoped
resources before validation.
AI tools were used to assist with investigation and patch preparation. I reviewed
the resulting changes and validated them with the project's native test suite.
Which issue(s) this PR fixes:
NONE
Special notes for your reviewer:
Validation performed:
helm lint charts/kueue: passed.helm-verify: passed.kubectl kustomize config/kueueviz: passed.namespace removals.
Does this PR introduce a user-facing change?
Summary by CodeRabbit
Maintenance
Tests