-
Notifications
You must be signed in to change notification settings - Fork 38.8k
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
Plumb subresource through subjectaccessreview #40935
Conversation
f7378b0
to
e94137f
Compare
e94137f
to
a455dcc
Compare
/lgtm |
"testing" | ||
|
||
"k8s.io/apiserver/pkg/authorization/authorizer" | ||
"k8s.io/client-go/pkg/util/sets" |
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 your goimports clipped this but it should be k8s.io/kuberentes/pkg/util/sets until we've moved that package into staging/
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 your goimports clipped this but it should be k8s.io/kuberentes/pkg/util/sets until we've moved that package into staging/
Already moved to apimachinery.
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.
Ok then we should change this to k8s.io/apimachinery/pkg/util/sets
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.
Ok then we should change this to k8s.io/apimachinery/pkg/util/sets
I suspect this code doesn't build in master but does build for 1.5
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.
Well we can always just use k8s.io/kuberentes/pkg/util/sets
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.
fixed, will tweak in the cherry-pick
a455dcc
to
5f4f624
Compare
added a rest test as well |
5f4f624
to
3a89d33
Compare
fixed a govet issue, retagging |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED The following people have approved this PR: liggitt, smarterclayton Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
weird github merge failure |
@k8s-bot cross build test this |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
@liggitt: The following test(s) failed:
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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/test-infra repository. I understand the commands that are listed here. |
Automatic merge from submit-queue |
Commit found in the "release-1.5" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked. |
plumb all fields for subjectaccessreview into the resulting
authorizer.AttributesRecord