Skip to content
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

Merged
merged 1 commit into from Feb 13, 2019

Conversation

@WanLinghao
Copy link
Contributor

WanLinghao commented Jun 6, 2018

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:

Resources                                       Non-Resource URLs        Resource Names   Verbs
---------                                       -----------------        --------------   -----
*.*                                             []                       []               [*]
                                                [*]                      []               [*]
selfsubjectaccessreviews.authorization.k8s.io   []                       []               [create]
selfsubjectrulesreviews.authorization.k8s.io    []                       []               [create]
                                                [/api/*]                 []               [get]
                                                [/api]                   []               [get]
                                                [/apis/*]                []               [get]
                                                [/apis]                  []               [get]
                                                [/healthz]               []               [get]
                                                [/openapi/*]             []               [get]
                                                [/openapi]               []               [get]
                                                [/swagger-2.0.0.pb-v1]   []               [get]
                                                [/swagger.json]          []               [get]
                                                [/swaggerapi/*]          []               [get]
                                                [/swaggerapi]            []               [get]
                                                [/version/]              []               [get]
                                                [/version]               []               [get]

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:

Add `kubectl auth can-i --list` option which could help users know what actions they can do in specific namespace.
@liggitt

This comment has been minimized.

@CaoShuFeng

This comment has been minimized.

Copy link
Member

CaoShuFeng commented Jun 7, 2018

/assign
/cc

@php-coder

This comment has been minimized.

Copy link
Contributor

php-coder commented Jun 7, 2018

I'd mention it in the release notes.

@WanLinghao WanLinghao force-pushed the WanLinghao:ctl_selfsubjectrulesreview_support branch from b2f904b to fe25b71 Jun 7, 2018

@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Jun 7, 2018

@php-coder Done

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 {

This comment has been minimized.

@CaoShuFeng

CaoShuFeng Jun 12, 2018

Member

kubectl api-resources doesn't have such line.

This comment has been minimized.

@WanLinghao

WanLinghao Jun 12, 2018

Author Contributor

@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]
return err
}

err = o.printStatus(response.Status)

This comment has been minimized.

@zjj2wry

zjj2wry Jun 12, 2018

Member

return o.printStatus(response.Status)

defer w.Flush()

if o.NonResourceOnly {
if err := printNonResourceURLs(w, compactRules, o.NoHeader); err != nil {

This comment has been minimized.

@zjj2wry

zjj2wry Jun 12, 2018

Member

ditto

return nil
}
if o.ResourceOnly {
if err := printResources(w, compactRules, o.NoHeader); err != nil {

This comment has been minimized.

@zjj2wry

zjj2wry Jun 12, 2018

Member

ditto

}
return nil
}
if err := printAll(w, compactRules, o.NoHeader); err != nil {

This comment has been minimized.

@zjj2wry

zjj2wry Jun 12, 2018

Member

ditto

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"

This comment has been minimized.

@zjj2wry

zjj2wry Jun 12, 2018

Member

May need to use v1clientset

return ret
}

func convertNonResourceToPolicyRule(status authorizationapi.SubjectRulesReviewStatus) []rbacv1.PolicyRule {

This comment has been minimized.

@zjj2wry

zjj2wry Jun 12, 2018

Member

can use v1 api directly? so you can need not add version conversion func

@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Jun 12, 2018

@zjj2wry comments addressed, PTAL

@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Jun 12, 2018

/retest

@zjj2wry
Copy link
Member

zjj2wry left a comment

lgtm

}

cmd := &cobra.Command{
Use: "i-can [--only-resources] [--only-non-resource-urls] [--quiet]",

This comment has been minimized.

@CaoShuFeng

CaoShuFeng Jun 14, 2018

Member

nit --no-headers


func (o *ICanOptions) printStatus(status authorizationapi.SubjectRulesReviewStatus) error {
if status.Incomplete && !o.Quiet {
fmt.Fprintln(o.Out, "warning: the list may be incomplete")

This comment has been minimized.

@CaoShuFeng

CaoShuFeng Jun 14, 2018

Member

o.ErrOut

fmt.Fprintln(o.Out, "warning: the list may be incomplete")
}
if len(status.EvaluationError) != 0 && !o.Quiet {
fmt.Fprintln(o.Out, status.EvaluationError)

This comment has been minimized.

@CaoShuFeng
@CaoShuFeng

This comment has been minimized.

Copy link
Member

CaoShuFeng commented Jun 14, 2018

test ok in my environment.

@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Jun 15, 2018

@CaoShuFeng comments addressed

@WanLinghao WanLinghao force-pushed the WanLinghao:ctl_selfsubjectrulesreview_support branch from f390e26 to fe6c3a4 Nov 19, 2018

@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Nov 19, 2018

@soltysh Now this patch would not import any unexpected packages any more. PTAL

@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Nov 19, 2018

/retest

@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Nov 20, 2018

/test pull-kubernetes-integration

@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Nov 21, 2018

@soltysh friendly ping

@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Dec 3, 2018

@soltysh PTAL thanks

@WanLinghao WanLinghao force-pushed the WanLinghao:ctl_selfsubjectrulesreview_support branch 2 times, most recently from 77e07ad to 48d0d54 Dec 18, 2018

@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Dec 18, 2018

@soltysh Rebase done, PTAL

@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Dec 18, 2018

/test pull-kubernetes-e2e-kops-aws

@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Dec 27, 2018

@soltysh friendly ping

@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Jan 3, 2019

@soltysh
Copy link
Contributor

soltysh left a comment

Final nits, when you're done please ping me on slack so that I can approve that right away.

@@ -145,6 +158,7 @@ func (o *CanIOptions) Complete(f cmdutil.Factory, args []string) error {
return err
}
o.SelfSARClient = client.AuthorizationV1()
o.SelfSRRClient = client.AuthorizationV1()

This comment has been minimized.

@soltysh

soltysh Jan 18, 2019

Contributor

You don't need twice the same client.

@@ -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")

This comment has been minimized.

@soltysh

soltysh Jan 18, 2019

Contributor

cannot be set...

@@ -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 {

This comment has been minimized.

@soltysh

soltysh Jan 18, 2019

Contributor

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.

@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Jan 19, 2019

@soltysh Thanks for your review, all comments addressed.

Add `kubectl auth can-i --list` option which could help users know wh…
…at actions they can do in specific namespace.

@WanLinghao WanLinghao force-pushed the WanLinghao:ctl_selfsubjectrulesreview_support branch from 4f22ab2 to d4f5228 Jan 19, 2019

@@ -167,9 +197,28 @@ func (o *CanIOptions) Validate() error {
return fmt.Errorf("NonResourceURL and ResourceName can not specified together")
}
}

if o.NoHeaders {

This comment has been minimized.

@soltysh

soltysh Jan 24, 2019

Contributor

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?

This comment has been minimized.

@WanLinghao

WanLinghao Jan 25, 2019

Author Contributor

@soltysh I checked List flag in Line 185

This comment has been minimized.

@soltysh

soltysh Feb 12, 2019

Contributor

Right, sorry I missed the return nil after the internal condition.

@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Feb 12, 2019

@soltysh PTAL

@soltysh
Copy link
Contributor

soltysh left a comment

/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 {

This comment has been minimized.

@soltysh

soltysh Feb 12, 2019

Contributor

Right, sorry I missed the return nil after the internal condition.

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Feb 12, 2019

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fejta-bot

This comment has been minimized.

Copy link

fejta-bot commented Feb 12, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Feb 13, 2019

/test pull-kubernetes-e2e-gce

1 similar comment
@WanLinghao

This comment has been minimized.

Copy link
Contributor Author

WanLinghao commented Feb 13, 2019

/test pull-kubernetes-e2e-gce

@k8s-ci-robot k8s-ci-robot merged commit a92729a into kubernetes:master Feb 13, 2019

19 checks passed

cla/linuxfoundation WanLinghao authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-kops-aws Context retired without replacement.
pull-kubernetes-e2e-kubeadm-gce Skipped
pull-kubernetes-godeps Skipped
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.