-
Notifications
You must be signed in to change notification settings - Fork 41k
Add kubectl auth can-i --list
option which could help users know what actions they can do in specific namespace
#64820
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 kubectl auth can-i --list
option which could help users know what actions they can do in specific namespace
#64820
Conversation
@kubernetes/sig-cli-pr-reviews @kubernetes/sig-auth-pr-reviews |
/assign |
I'd mention it in the release notes. |
b2f904b
to
fe25b71
Compare
@php-coder Done |
pkg/kubectl/cmd/auth/ican.go
Outdated
if _, err := fmt.Fprintf(out, "Resources\tNon-Resource URLs\tResource Names\tVerbs\n"); err != nil { | ||
return err | ||
} | ||
if _, err := fmt.Fprintf(out, "---------\t-----------------\t--------------\t-----\n"); err != nil { |
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.
kubectl api-resources
doesn't have such line.
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.
@CaoShuFeng Since the output here is similar with RBAC clusterrole/role, maybe we can just keep same with them.
And the output of kubectl describe role xxxx
is like:
Resources Non-Resource URLs Resource Names Verbs
--------- ----------------- -------------- -----
serviceaccounts [] [] [create delete deletecollection get list patch update watch impersonate]
configmaps [] [] [create delete deletecollection get list patch update watch]
endpoints [] [] [create delete deletecollection get list patch update watch]
persistentvolumeclaims [] [] [create delete deletecollection get list patch update watch]
pods/attach [] [] [create delete deletecollection get list patch update watch]
pods/exec [] [] [create delete deletecollection get list patch update watch]
pods/portforward [] [] [create delete deletecollection get list patch update watch]
pods/proxy [] [] [create delete deletecollection get list patch update watch]
pods [] [] [create delete deletecollection get list patch update watch]
replicationcontrollers/scale [] [] [create delete deletecollection get list patch update watch]
pkg/kubectl/cmd/auth/ican.go
Outdated
return err | ||
} | ||
|
||
err = o.printStatus(response.Status) |
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.
return o.printStatus(response.Status)
pkg/kubectl/cmd/auth/ican.go
Outdated
defer w.Flush() | ||
|
||
if o.NonResourceOnly { | ||
if err := printNonResourceURLs(w, compactRules, o.NoHeader); err != nil { |
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.
ditto
pkg/kubectl/cmd/auth/ican.go
Outdated
return nil | ||
} | ||
if o.ResourceOnly { | ||
if err := printResources(w, compactRules, o.NoHeader); err != nil { |
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.
ditto
pkg/kubectl/cmd/auth/ican.go
Outdated
} | ||
return nil | ||
} | ||
if err := printAll(w, compactRules, o.NoHeader); err != nil { |
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.
ditto
pkg/kubectl/cmd/auth/ican.go
Outdated
rbacv1 "k8s.io/api/rbac/v1" | ||
authorizationapi "k8s.io/kubernetes/pkg/apis/authorization" | ||
rbacv1helpers "k8s.io/kubernetes/pkg/apis/rbac/v1" | ||
internalauthorizationclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/authorization/internalversion" |
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.
May need to use v1clientset
pkg/kubectl/cmd/auth/ican.go
Outdated
return ret | ||
} | ||
|
||
func convertNonResourceToPolicyRule(status authorizationapi.SubjectRulesReviewStatus) []rbacv1.PolicyRule { |
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.
can use v1 api directly? so you can need not add version conversion func
@zjj2wry comments addressed, PTAL |
/retest |
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.
lgtm
pkg/kubectl/cmd/auth/ican.go
Outdated
} | ||
|
||
cmd := &cobra.Command{ | ||
Use: "i-can [--only-resources] [--only-non-resource-urls] [--quiet]", |
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.
nit --no-headers
pkg/kubectl/cmd/auth/ican.go
Outdated
|
||
func (o *ICanOptions) printStatus(status authorizationapi.SubjectRulesReviewStatus) error { | ||
if status.Incomplete && !o.Quiet { | ||
fmt.Fprintln(o.Out, "warning: the list may be incomplete") |
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.
o.ErrOut
pkg/kubectl/cmd/auth/ican.go
Outdated
fmt.Fprintln(o.Out, "warning: the list may be incomplete") | ||
} | ||
if len(status.EvaluationError) != 0 && !o.Quiet { | ||
fmt.Fprintln(o.Out, status.EvaluationError) |
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.
ditto
test ok in my environment. |
@CaoShuFeng comments addressed |
fd8f8d7
to
f390e26
Compare
f390e26
to
fe6c3a4
Compare
@soltysh Now this patch would not import any unexpected packages any more. PTAL |
/retest |
/test pull-kubernetes-integration |
@soltysh friendly ping |
@soltysh PTAL thanks |
77e07ad
to
48d0d54
Compare
@soltysh Rebase done, PTAL |
/test pull-kubernetes-e2e-kops-aws |
@soltysh friendly ping |
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.
Final nits, when you're done please ping me on slack so that I can approve that right away.
pkg/kubectl/cmd/auth/cani.go
Outdated
@@ -145,6 +158,7 @@ func (o *CanIOptions) Complete(f cmdutil.Factory, args []string) error { | |||
return err | |||
} | |||
o.SelfSARClient = client.AuthorizationV1() | |||
o.SelfSRRClient = client.AuthorizationV1() |
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.
You don't need twice the same client.
pkg/kubectl/cmd/auth/cani.go
Outdated
@@ -167,11 +188,19 @@ func (o *CanIOptions) Validate() error { | |||
return fmt.Errorf("NonResourceURL and ResourceName can not specified together") | |||
} | |||
} | |||
|
|||
if o.NoHeaders { | |||
return fmt.Errorf("--no-headers can not set without --list specified") |
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.
cannot be set...
pkg/kubectl/cmd/auth/cani.go
Outdated
@@ -214,9 +242,12 @@ func (o *CanIOptions) RunAccessCheck() (bool, error) { | |||
fmt.Fprintf(o.Out, " - %v", response.Status.EvaluationError) | |||
} | |||
fmt.Fprintln(o.Out) | |||
if o.Quiet { |
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.
Wait, this is changing previous behavior. We used to os.Exit(1)
always when !allowed
now you're doing that only when --quiet
was specified.
@soltysh Thanks for your review, all comments addressed. |
…at actions they can do in specific namespace.
4f22ab2
to
d4f5228
Compare
@@ -167,9 +197,28 @@ func (o *CanIOptions) Validate() error { | |||
return fmt.Errorf("NonResourceURL and ResourceName can not specified together") | |||
} | |||
} | |||
|
|||
if o.NoHeaders { |
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 can't be set w/o list flag, yet you're not checking its value so how do you know it wasn't set?
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.
@soltysh I checked List flag in Line 185
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.
Right, sorry I missed the return nil
after the internal condition.
@soltysh PTAL |
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.
/lgtm
/approve
/hold cancel
@@ -167,9 +197,28 @@ func (o *CanIOptions) Validate() error { | |||
return fmt.Errorf("NonResourceURL and ResourceName can not specified together") | |||
} | |||
} | |||
|
|||
if o.NoHeaders { |
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.
Right, sorry I missed the return nil
after the internal condition.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: soltysh, WanLinghao The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest Review the full test history for this PR. Silence the bot with an |
/test pull-kubernetes-e2e-gce |
1 similar comment
/test pull-kubernetes-e2e-gce |
What this PR does / why we need it:
Since API SelfSubjectRulesReview has implemented, I think we need ctl support to make it
easier for users to know what actions they can do in specific namespace.
kubectl auth can-i --list --namespace=test-namespace
The output looks like:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #64821
Special notes for your reviewer:
Release note: