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

kubectl clusterrole help doesn't document --user flag #1256

Closed
RichardoC opened this issue Jul 27, 2022 · 21 comments
Closed

kubectl clusterrole help doesn't document --user flag #1256

RichardoC opened this issue Jul 27, 2022 · 21 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/cli Categorizes an issue or PR as relevant to SIG CLI.

Comments

@RichardoC
Copy link

What happened?

--user is missing from the list of options

$ kubectl -n security create rolebinding workloads --clusterrole workloads --help
Create a role binding for a particular role or cluster role.

Examples:
  # Create a role binding for user1, user2, and group1 using the admin cluster role
  kubectl create rolebinding admin --clusterrole=admin --user=user1 --user=user2 --group=group1

Options:
      --allow-missing-template-keys=true: If true, ignore any errors in templates when a field or map key is missing in
the template. Only applies to golang and jsonpath output formats.
      --clusterrole='': ClusterRole this RoleBinding should reference
      --dry-run='none': Must be "none", "server", or "client". If client strategy, only print the object that would be
sent, without sending it. If server strategy, submit server-side request without persisting the resource.
      --field-manager='kubectl-create': Name of the manager used to track field ownership.
      --group=[]: Groups to bind to the role
  -o, --output='': Output format. One of:
json|yaml|name|go-template|go-template-file|template|templatefile|jsonpath|jsonpath-as-json|jsonpath-file.
      --role='': Role this RoleBinding should reference
      --save-config=false: If true, the configuration of current object will be saved in its annotation. Otherwise, the
annotation will be unchanged. This flag is useful when you want to perform kubectl apply on this object in the future.
      --serviceaccount=[]: Service accounts to bind to the role, in the format <namespace>:<name>
      --show-managed-fields=false: If true, keep the managedFields when printing objects in JSON or YAML format.
      --template='': Template string or path to template file to use when -o=go-template, -o=go-template-file. The
template format is golang templates [http://golang.org/pkg/text/template/#pkg-overview].
      --validate=true: If true, use a schema to validate the input before sending it

Usage:
  kubectl create rolebinding NAME --clusterrole=NAME|--role=NAME [--user=username] [--group=groupname]
[--serviceaccount=namespace:serviceaccountname] [--dry-run=server|client|none] [options]

Use "kubectl options" for a list of global command-line options (applies to all commands).

What did you expect to happen?

$ kubectl -n security create rolebinding workloads --clusterrole workloads --help
Create a role binding for a particular role or cluster role.

Examples:

Create a role binding for user1, user2, and group1 using the admin cluster role

kubectl create rolebinding admin --clusterrole=admin --user=user1 --user=user2 --group=group1

Options:
--allow-missing-template-keys=true: If true, ignore any errors in templates when a field or map key is missing in
the template. Only applies to golang and jsonpath output formats.
--clusterrole='': ClusterRole this RoleBinding should reference
--dry-run='none': Must be "none", "server", or "client". If client strategy, only print the object that would be
sent, without sending it. If server strategy, submit server-side request without persisting the resource.
--field-manager='kubectl-create': Name of the manager used to track field ownership.
--group=[]: Groups to bind to the role
-o, --output='': Output format. One of:
json|yaml|name|go-template|go-template-file|template|templatefile|jsonpath|jsonpath-as-json|jsonpath-file.
--role='': Role this RoleBinding should reference
--save-config=false: If true, the configuration of current object will be saved in its annotation. Otherwise, the
annotation will be unchanged. This flag is useful when you want to perform kubectl apply on this object in the future.
--serviceaccount=[]: Service accounts to bind to the role, in the format :
--show-managed-fields=false: If true, keep the managedFields when printing objects in JSON or YAML format.
--template='': Template string or path to template file to use when -o=go-template, -o=go-template-file. The
template format is golang templates [http://golang.org/pkg/text/template/#pkg-overview].
--user=[]: Users to bind to the role
--validate=true: If true, use a schema to validate the input before sending it

Usage:
kubectl create rolebinding NAME --clusterrole=NAME|--role=NAME [--user=username] [--group=groupname]
[--serviceaccount=namespace:serviceaccountname] [--dry-run=server|client|none] [options]

Use "kubectl options" for a list of global command-line options (applies to all commands).

How can we reproduce it (as minimally and precisely as possible)?

$ kubectl -n internal create rolebinding workloads --clusterrole workloads ---help | | grep '--user'
Nothing saying what the --user flag does.

Anything else we need to know?

No response

Kubernetes version

$ kubectl  version
Client Version: version.Info{Major:"1", Minor:"23", GitVersion:"v1.23.1", GitCommit:"86ec240af8cbd1b60bcc4c03c20da9b98005b92e", GitTreeState:"clean", BuildDate:"2021-12-16T11:41:01Z", GoVersion:"go1.17.5", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"23", GitVersion:"v1.23.1", GitCommit:"86ec240af8cbd1b60bcc4c03c20da9b98005b92e", GitTreeState:"clean", BuildDate:"2021-12-16T11:34:54Z", GoVersion:"go1.17.5", Compiler:"gc", Platform:"linux/amd64"}

Cloud provider

N/A

OS version

# On Linux:
$ cat /etc/os-release
# paste output here
$ uname -a
# paste output here

# On Windows:
C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture
# paste output here

Install tools

Container runtime (CRI) and version (if applicable)

Related plugins (CNI, CSI, ...) and versions (if applicable)

@RichardoC RichardoC added the kind/bug Categorizes issue or PR as related to a bug. label Jul 27, 2022
@k8s-ci-robot k8s-ci-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Jul 27, 2022
@k8s-ci-robot
Copy link
Contributor

@RichardoC: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jul 27, 2022
@RichardoC
Copy link
Author

/sig cli

@k8s-ci-robot k8s-ci-robot added sig/cli Categorizes an issue or PR as relevant to SIG CLI. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jul 27, 2022
@RichardoC
Copy link
Author

@muyangren2
Copy link
Contributor

/assign

@muyangren2
Copy link
Contributor

Weirdly I can see it in the code, but not when run?

https://github.com/kubernetes/kubectl/blob/kubernetes-1.23.0/pkg/cmd/create/create_clusterrolebinding.go#L103

Yes, it does exist in the code. I looked at the code and didn't find any problems.

@Shubham82
Copy link
Contributor

Hi,
It is because the user flag which is mentioned here is shadowed by a global flag.
See this comment: kubernetes/kubernetes#108654 (comment)

PR opened for it in past but has not resolved it. see this: kubernetes/kubernetes#103809 (review)

@Shubham82
Copy link
Contributor

/cc @eddiezane

@sftim
Copy link
Contributor

sftim commented Jul 28, 2022

This is a kubectl issue

/transfer kubectl

@k8s-ci-robot k8s-ci-robot transferred this issue from kubernetes/kubernetes Jul 28, 2022
@brianpursley
Copy link
Member

@Shubham82 is right, this is because there is already a global user flag:

$ kubectl options | grep -A2 "\--user="
    --user='':
	The name of the kubeconfig user to use

The problem is we can't just rename the flag because of backward compatibility. People running kubectl create rolebinding are expecting to be able to pass --user even though it is not showing in the help.

I wonder if a solution would be to "overload" this flag, essentially binding two different flags to the o.Users field, like this:

	cmd.Flags().StringArrayVar(&o.Users, "user", o.Users, "Usernames to bind to the role. The flag can be repeated to add multiple users.")
	cmd.Flags().StringArrayVar(&o.Users, "role-user", o.Users, "Usernames to bind to the role. The flag can be repeated to add multiple users.")

Then, --role-user would show up in the help:

$ kubectl create rolebinding --help | grep -A2 "user=\[]"
    --role-user=[]:
	Usernames to bind to the role. The flag can be repeated to add multiple users.

And we could deprecate the use of --user in this command, since it conflicts with a global flag.

Maybe even print a warning if people use that flag so that eventually we could remove it.

Thoughts?

@RichardoC
Copy link
Author

Maybe "username" instead because role-user doesn't really fit with --service-account etc?

@Shubham82
Copy link
Contributor

Hi @brianpursley
After adding another flag(role-user) for the user so we use both flags (--user and --role-user) for binding the user to the role until we totally removed --user flag.
Am I understanding right?

@Shubham82
Copy link
Contributor

IMO role-user is sound better. let's wait for other opinions on it.

@brianpursley
Copy link
Member

@Shubham82 that was what I was thinking, then mark --user (the non-global one) as deprecated, and print a warning if people are using that flag, telling them to use --role-user instead. That way we can someday (afters several future releases) remove the --user from this command.

BUT... I think this should be discussed with SIG-CLI though before doing that, perhaps in the next bug scrub meeting, or in the next regular SIG-CLI meeting.

There is probably a bigger overall issue of "how do we handle/fix shadowed flags"

A quick look and I see (at least) these commands shadow --user with their own flag:

  • create clusterrolebinding
  • create rolebinding
  • set subject
  • config set-context

In fact, config set-context shadows --user, --cluster, and --namespace flags. This might be fine though because the global flags would be meaningless for that command.

The real problem is that shadowed flags don't show up in the help output, so maybe another, different solution would be to figure out how to force them to show up in the help output even though they are shadowing a global flag. Is that possible?

@Shubham82
Copy link
Contributor

Thanks, @brianpursley for this detailed information.
I agree with you!

@eddiezane
Copy link
Member

The root of this is in Cobra spf13/cobra#1651

@eddiezane
Copy link
Member

Ref kubernetes/kubernetes#103769

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 15, 2022
@RichardoC
Copy link
Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 16, 2022
@KnVerey
Copy link
Contributor

KnVerey commented Jan 4, 2023

/triage resolved
/close

This is fixed in v1.26.0:

kubectl create rolebinding -h
...
  --user=[]:
	Usernames to bind to the role. The flag can be repeated to add multiple users.

@k8s-ci-robot
Copy link
Contributor

@KnVerey: The label(s) triage/resolved cannot be applied, because the repository doesn't have them.

In response to this:

/triage resolved
/close

This is fixed in v1.26.0:

kubectl create rolebinding -h
...
 --user=[]:
  Usernames to bind to the role. The flag can be repeated to add multiple users.

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.

@k8s-ci-robot
Copy link
Contributor

@KnVerey: Closing this issue.

In response to this:

/triage resolved
/close

This is fixed in v1.26.0:

kubectl create rolebinding -h
...
 --user=[]:
  Usernames to bind to the role. The flag can be repeated to add multiple users.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/cli Categorizes an issue or PR as relevant to SIG CLI.
Projects
None yet
Development

No branches or pull requests

9 participants