✨ feat: Add optional roleName parameter to RBAC marker#1334
Conversation
Allows users to specify custom role names per marker to avoid name conflicts when using namespace-scoped RBAC across multiple namespaces. Key features: - Optional roleName parameter in RBAC marker - Non-breaking: defaults to --roleName flag if not specified - Useful for avoiding kustomize namespace transformation conflicts - Multiple markers with same roleName in same namespace are merged Example usage: // +kubebuilder:rbac:groups=apps,namespace=infrastructure,roleName=infra-manager,resources=deployments,verbs=get;list // +kubebuilder:rbac:groups="",namespace=users,roleName=user-secrets,resources=secrets,verbs=get This addresses the issue raised in kubernetes-sigs/kubebuilder#5148 where namespace-scoped Roles with the same name in different namespaces cause kustomize namespace transformation ID conflict errors. Unlike PR kubernetes-sigs#1329 (automatic namespace suffix), this approach: - Is opt-in (non-breaking for existing users) - Gives users explicit control over Role names - Works for any use case, not just kustomize - Avoids breaking changes and migration issues
|
Welcome @AlirezaPourchali! |
|
Hi @AlirezaPourchali. 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 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. |
|
/kind feature |
|
/area rbac |
|
@AlirezaPourchali: The label(s) 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. |
|
@JoelSpeed @sbueringer WDYT? |
|
General idea makes sense to me. I think roleName is fine. Just name doesn't seem clear enough to me. Let's see what @JoelSpeed thinks. With this it's clear that this rule belongs to the infra-deployment-manager role: With this it's not really clear what the name is referring to (this marker is for a rule, but a rule itself does not have a name) I realize that namespace is also referring to the Role namespace, but not sure if that's good and we should continue that way /ok-to-test |
|
Re: CI failure (https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_controller-tools/1334/pull-controller-tools-test-main/2021884835185823744) The failure is orthogonal to this PR, looks like our tests are directly downloading envtest/kubebuilder binaries from the old location that is not available anymore. We'll need a PR to migrate this to download from the new location (controller-tools release attachments). I would probably continue to just use curl and not introduce setup-envtest. As part of that we'll also have to download newer binaries (looks like the job is using Kubernetes v1.11, which is not available on the new location) |
|
I agree, roleName works well here, apart from needing to fix CI separately, I think this LGTM |
|
Thx again! /lgtm (We can run retest once CI is fixed on main) |
|
LGTM label has been added. DetailsGit tree hash: 0f32985c89c31a8a5c2cadf923a7e8cfe3c1f51d |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AlirezaPourchali, sbueringer 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 |
|
/retest |
|
Thank you a lot @sbueringer, @JoelSpeed and @AlirezaPourchali 🙏 |
Problem
When using namespace-scoped RBAC markers with different namespaces, controller-gen generates multiple Roles with identical names in different namespaces:
While this is valid Kubernetes, it causes kustomize to fail when applying a global namespace transformation:
Error: namespace transformation produces ID conflict: [{"kind":"Role","metadata":{"name":"manager-role","namespace":"myproject-system"}...} {"kind":"Role","metadata":{"name":"manager-role","namespace":"myproject-system"}...}]Both Roles end up in the same namespace with identical names → conflict!
Reported in: kubernetes-sigs/kubebuilder#5148
Follow up from: #1329
Solution
This PR adds an optional
roleNameparameter to the RBAC marker, allowing users to specify custom role names per marker:Generated output:
Now kustomize's namespace transformation succeeds:
✅ No conflicts!
Key Features
1. Non-breaking (100% backward compatible)
If
roleNameis not specified, behavior is identical to current implementation:2. Opt-in when needed
Only specify
roleNamewhen you need unique names:3. Marker merging still works
Multiple markers with the same
roleNamein the same namespace are merged (as expected):This PR addresses all the concerns while still solving the original problem.
Testing
Added comprehensive tests in
pkg/rbac/testdata/:All existing tests pass unchanged, confirming backward compatibility.
Implementation Details
Rulestruct to include optionalRoleName stringfieldGenerateRoles()to group by(namespace, roleName)instead of justnamespaceMigration Path for Affected Users
Users currently hitting the kustomize conflict can add
roleNameto their markers:Before (conflicts):
After (no conflicts):
Run
make manifests, commit the updated RBAC YAML, and deploy. No other changes needed.