Restrict consumer permissions - #1474
Merged
devdattakulkarni merged 3 commits intoJul 15, 2026
Merged
Conversation
anniegracehu
approved these changes
Jul 14, 2026
| # consumer binding | ||
| run_command("kubectl delete rolebinding " + sa + " -n " + namespace + " --ignore-not-found" + kubeconfigString) | ||
| # legacy consumer binding | ||
| run_command("kubectl delete clusterrolebinding " + sa + " --ignore-not-found" + kubeconfigString) # backwards-compatible |
| run_command("kubectl delete clusterrolebinding " + sa + kubeconfigString) | ||
| run_command("kubectl delete clusterrole " + sa + "-update" + kubeconfigString) | ||
| run_command("kubectl delete clusterrolebinding " + sa + "-update" + kubeconfigString) | ||
| if entity == "consumer": |
Contributor
There was a problem hiding this comment.
Is this duplicate of code from 1062?
Contributor
Author
There was a problem hiding this comment.
I originally kept the sequences of commands in order, but it created that similar-looking conditional, only different by a "-update" in the name. I just added a commit to make it a bit cleaner with only one conditional block
| cmd = f"kubectl auth can-i create deployments.apps -n {ns1} --kubeconfig={consumer2_kubeconfig}" | ||
| out, err = TestKubePlus.run_command(cmd) | ||
| skip_on_connection_error(err) | ||
| self.assertEqual(out.strip(), "no") |
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.
Addresses #1472
Hello! This is my first contribution to the project - I’m happy to make adjustments based on review and feedback.
Problem
Consumer permissions were granted through a
ClusterRoleBinding, which made otherwise namespaced permissions available across the cluster.For example, a consumer associated with
namespace1could create a Deployment innamespace2.Changes
This PR changes consumer RBAC bindings from
ClusterRoleBindingto namespacedRoleBindingobjects. TheRoleBindingcontinues to reference the existingClusterRole, but limits applicable permissions to the consumer’s namespace.Providers continue to use
ClusterRoleBindingobjects because they require cluster-scoped access.The update, revoke, and delete paths previously shared binding-management logic between providers and consumers. I added entity-specific handling so that:
RoleBinding.ClusterRoleBinding.ClusterRoleBindingobjects.I also added a regression test that reproduces the original issue by creating two namespaced consumers and verifying that one consumer cannot create a Deployment in the other consumer’s namespace. The test fails with the previous implementation and passes with this change.
Migration considerations
This PR does not automatically migrate existing consumer
ClusterRoleBindingobjects.The change applies to newly created consumer bindings and permissions added through the update command. Legacy consumer bindings are removed by the delete command, but existing bindings are not migrated during update or revoke operations.
I would appreciate guidance on whether this PR should also include a migration path for existing consumers, or whether migration should be handled separately.
Thanks!