-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow users to create bindings to roles #14547
Allow users to create bindings to roles #14547
Conversation
Flake #14555 |
Nice, this makes sense to me. |
policyBinding, err := m.getPolicyBindingForPolicy(ctx, roleBinding.RoleRef.Namespace, allowEscalation) | ||
// get or auto create policy binding so we can deprecate policy and policy binding objects in 3.6 | ||
// thus normal users can always create a role binding referring to a role | ||
policyBinding, err := m.getPolicyBindingForPolicy(ctx, roleBinding.RoleRef.Namespace, true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would allows someone to create a rolebinding pointing to a role in another namespace. That is an anti-goal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, @deads2k instead of passing true
I could pass roleBinding.RoleRef.Namespace == namespaceFromContext
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so. Add a test to check.
This change makes it so that you no longer need cluster admin privileges to create a role binding that references a role in your namespace. In the past we required a cluster admin to create the policy binding object before a normal user could perform these bindings. This change is required for us to deprecate policy and policy binding objects in 3.6. Signed-off-by: Monis Khan <[email protected]>
4bf4db4
to
491b2f4
Compare
Evaluated for origin test up to 491b2f4 |
@deads2k PTAL. |
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/2151/) (Base Commit: acb8636) |
Flake #14129 |
lgtm [merge] |
Evaluated for origin merge up to 491b2f4 |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/993/) (Base Commit: 653b218) (Image: devenv-rhel7_6358) |
@enj can you sweep docs for references to the need to create this and make updates as needed. also, I think this is the last piece we need to announce deprecation of {cluster,}policy{binding,} in 3.6, right? |
(means updating comments on API types and a release note in openshift/openshift-docs#4021) |
@liggitt I believe so. |
This change makes it so that you no longer need cluster admin privileges to create a role binding that references a role in your namespace. In the past we required a cluster admin to create the policy binding object before a normal user could perform these bindings. This change is required for us to deprecate policy and policy binding objects in 3.6.
Signed-off-by: Monis Khan [email protected]
Fixes #14078
cc @liggitt @deads2k PTAL
cc @benjaminapetersen @bparees @jfchevrette since you have encountered this before.
[test]