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
Add auth API to get self subject attributes #111333
Add auth API to get self subject attributes #111333
Conversation
Hi @nabokihms. Thanks for your PR. I'm waiting for a kubernetes 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. 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. |
/sig auth |
This PR may require API review. If so, when the changes are ready, complete the pre-review checklist and request an API review. Status of requested reviews is tracked in the API Review project. |
@@ -220,6 +220,7 @@ func ClusterRoles() []rbacv1.ClusterRole { | |||
Rules: []rbacv1.PolicyRule{ | |||
// TODO add future selfsubjectrulesreview, project request APIs, project listing APIs | |||
rbacv1helpers.NewRule("create").Groups(authorizationGroup).Resources("selfsubjectaccessreviews", "selfsubjectrulesreviews").RuleOrDie(), | |||
rbacv1helpers.NewRule("create").Groups(authenticationGroup).Resources("selfsubjectattributesreviews").RuleOrDie(), |
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 means that distros that wish to restrict access to this API, must disable API when it goes GA. I think this is reasonable, but wanted to call it out explicitly for other reviewers.
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 have not reviewed this PR yet but the feature gate comment is wrong now.
/lgtm cancel |
9720893
to
a11c98a
Compare
master is open for 1.26, once this gets rebased it looks ready for merge |
a11c98a
to
27a33fa
Compare
Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>
27a33fa
to
00dfba4
Compare
/retest |
confirmed test still present in https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/111333/pull-kubernetes-e2e-gce-alpha-features/1570080230440177664/ /lgtm |
“Docs” should be a link to the documentation for the feature, not to the PR that first added it. @nabokihms would you be willing to revise? |
@sftim yes, I will, but I think we should wait for the doc to be merged first. |
sar := &authenticationv1alpha1.SelfSubjectReview{} | ||
response, err := o.authClient.SelfSubjectReviews().Create(context.TODO(), sar, metav1.CreateOptions{}) | ||
if err != nil { | ||
if errors.IsNotFound(err) { |
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.
if the feature is not enabled on the server, the permission to run this will not be included in the basic-user role, and users are likely to get a forbidden error... it might be worth detecting forbidden errors and saying something like "The selfsubjectreviews API is not enabled in the cluster or you do not have permission to call it..." to clarify the possible reasons
Signed-off-by: m.nabokikh maksim.nabokikh@flant.com
What type of PR is this?
/kind feature
/kind api-change
What this PR does / why we need it:
As KEP states:
Which issue(s) this PR fixes:
This PR is a part of kubernetes/enhancements#3325
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: