Remove use of policy API from CLI #15196
Conversation
|
@deads2k @liggitt @simo5 @abstractj PTAL (best viewed while ignoring whitespace). I did not see any other direct uses of policy when interacting with roles or bindings, but I could have missed something. @openshift/security [test] |
|
[severity:blocker] |
|
How comprehensive are the tests for this? |
pkg/cmd/admin/policy/policy.go
Outdated
| t := currBinding | ||
| for _, currBinding := range existingBindings.Items { | ||
| if currBinding.RoleRef.Name == role && currBinding.RoleRef.Namespace == "" { | ||
| t := authorizationapi.ToRoleBinding(&currBinding) |
There was a problem hiding this comment.
Is this a deep copy?
There was a problem hiding this comment.
No but this client side so it should not matter (the previous code was just a shallow copy also).
There was a problem hiding this comment.
previous one didn't take the address of the iterator, this one does, which is a bug
@smarterclayton All invocations of |
| if err != nil { | ||
| return err | ||
| } | ||
| sort.Sort(authorizationapi.PolicyBindingSorter(bindingList.Items)) | ||
| sort.Sort(sort.Reverse(authorizationapi.RoleBindingSorter(roleBindings.Items))) |
|
one iterator address bug, one question on reverse, LGTM otherwise |
This change removes the use of the policy API in CLI commands that interact with roles and bindings. The policy API is deprecated and will be removed in the 3.7 release. Thus this is required to make sure that a 3.6 oc binary continues to work with a 3.7 master. Signed-off-by: Monis Khan <[email protected]>
enj
force-pushed
the
enj/i/remove_policy_cli
branch
from
2a42af8 to
775b51d
Compare
|
re[test] GCE flake: |
|
Evaluated for origin test up to 775b51d |
|
@enj issue for that flake? |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/3172/) (Base Commit: bca9c79) (PR Branch Commit: 775b51d) |
|
[merge] comments addressed and change is fully backwards compatible. @stevekuznetsov it did not make sense to me to tag an unrelated test failure based on a random infrastructure hiccup. |
|
continuous-integration/openshift-jenkins/merge Waiting: You are in the build queue at position: 1 |
|
Evaluated for origin merge up to 775b51d |
How is this an infrastructure flake? We routinely send bug reports to the Docker daemon engineers, if that is where the bug is. Or that could be a |
This change removes the use of the policy API in CLI commands that interact with roles and bindings. The policy API is deprecated and will be removed in the 3.7 release. Thus this is required to make sure that a 3.6
ocbinary continues to work with a 3.7 master.Signed-off-by: Monis Khan [email protected]