kubectl plugins do not work when options placed before plugin name

[skaven81]

Edit:
The root cause of this issue has nothing to do with strace; it is because calling a kubectl plugin with options before the plugin name causes kubectl to not know how to find the plugin.

# works
kubectl my-plugin -n some-namespace

# fails, because kubectl stops looking for a plugin name after it sees the -n
kubectl -n some-namespace my-plugin

What happened:

kubectl plugin list shows that I have some plugins installed:

$ kubectl plugin list
The following compatible plugins are available:

/usr/bin/kubectl-pkrizak
/usr/local/bin/kubectl-who_can

But calling them does not work:

$ kubectl pkrizak
Error: unknown command "pkrizak" for "kubectl"
Run 'kubectl --help' for usage.
$ kubectl who-can
Error: unknown command "who-can" for "kubectl"
Run 'kubectl --help' for usage.

However, calling them under strace works somehow:

$ strace -f -o /tmp/trace kubectl pkrizak
hello there
$ strace -f -o /tmp/trace kubectl who-can
Error: you must specify two or three arguments: verb, resource, and optional resourceName

What you expected to happen: Plugins listed in kubectl plugin list should be called without having to wrap the command with strace.

How to reproduce it (as minimally and precisely as possible):

1. Install a kubectl plugin somewhere in $PATH
2. Verify that kubectl plugin list shows the plugin
3. Run kubectl <plugin> and it returns an error
4. Run strace <args> kubectl <plugin> and it works

Anything else we need to know?:

Environment:

• Kubernetes client and server versions (use kubectl version):

Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.3", GitCommit:"2e7996e3e2712684bc73f0dec0200d64eec7fe40", GitTreeState:"clean", BuildDate:"2020-05-20T12:52:00Z", GoVersion:"go1.13.9", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"15", GitVersion:"v1.15.12", GitCommit:"e2a822d9f3c2fdb5c9bfbe64313cf9f657f0a725", GitTreeState:"clean", BuildDate:"2020-05-06T05:09:48Z", GoVersion:"go1.12.17", Compiler:"gc", Platform:"linux/amd64"}

• Cloud provider or hardware configuration: n/a
• OS (e.g: cat /etc/os-release):

NAME="Ubuntu"
VERSION="16.04.6 LTS (Xenial Xerus)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 16.04.6 LTS"
VERSION_ID="16.04"
HOME_URL="http://www.ubuntu.com/"
SUPPORT_URL="http://help.ubuntu.com/"
BUG_REPORT_URL="http://bugs.launchpad.net/ubuntu/"
VERSION_CODENAME=xenial
UBUNTU_CODENAME=xenial

[brianpursley]

What is the output of this?

ls -l /usr/bin/kubectl-pkrizak

[skaven81]

Typical permissions:

$ ll /usr/local/bin/kubectl-who_can
-r-xr-xr-x 1 root root 25M Jun  9 15:16 /usr/local/bin/kubectl-who_can
$ ll /usr/bin/kubectl-pkrizak 
-rwxr-xr-x 1 root root 31 Jun 16 16:31 /usr/bin/kubectl-pkrizak

[brianpursley]

Hmm. If you do which kubectl-pkrizak does it return the path to kubectl-pkrizak?

[skaven81]

Yes it does. And it's worth noting again that it works properly and runs the plugin if I run kubectl under strace. So there's something weird going on...maybe something to do with threading inside kubectl? Just a guess.

[brianpursley]

I was able to reproduce your problem when I made an alias for kubectl, like this:

alias kubectl="kubectl -v9"

By any chance, have you defined an alias for kubectl?
What is the output of alias | grep kubectl?

[skaven81]

Indeed!

$ alias kubectl
alias kubectl='kubectl -n ${K8S_NAMESPACE:-default}

Removing the alias makes it work. Is this a bug in kubectl, or an odd side-effect of how aliases work?

[brianpursley]

Not sure if there is some way to fix or prevent this, but here is what I think is happening:

Kubectl expects the plugin name needs to be the first thing after kubectl, but when you have an alias, it expands before the kubectl code runs.

So when you have an alias of this:

alias kubectl="kubectl -n test"

And then try to use the foo plugin like this:

kubectl foo

the code actually sees this (I think, haven't confirmed):

kubectl -n test foo

It seems like it should be made to work somehow though, because if I do kubectl get pods with the alias (or kubectl -n test get pods without the alias) that works fine. It only seems to affect plugins.

I would also like to point out this is a problem with aliases in general, not just if you define a kubectl alias. So this would also not be able to call the foo plugin:

alias k="kubectl -n test"
k foo

[skaven81]

Agreed, this is what I'm seeing as well. The alias itself isn't the issue -- it occurs any time an argument appears before the plugin name.

# No alias set
[pkrizak@balthasar master]$ alias kubectl
bash: alias: kubectl: not found

# plugin works without any arguments
[pkrizak@balthasar master]$ kubectl who-can 
Error: you must specify two or three arguments: verb, resource, and optional resourceName

# adding an argument before plugin fails
[pkrizak@balthasar master]$ kubectl -n foo who-can
Error: unknown command "who-can" for "kubectl"
Run 'kubectl --help' for usage.

# arguments after plugin works
[pkrizak@balthasar master]$ kubectl who-can -n foo
Error: you must specify two or three arguments: verb, resource, and optional resourceName

[knight42]

this problem is a bit interesting and I think I have found out the reason.

tl;dr
kubectl expects plugin to be invoked in the format kubectl <plugin name> <flags>. If user places flags before the plugin name, kubectl would consider the plugin name as its subcommand.

for detailed reason, we can take a look at these lines:
https://github.com/kubernetes/kubernetes/blob/dcb587e09f43e4e98123da94e0614bc1672e074a/pkg/kubectl/cmd/cmd.go#L395-L426

let's say we run kubectl -n foo who-can, then the value of cmdArgs is ["-n", "foo", "who-can"], and the remainingArgs would be empty since -n starts with -, so kubectl cannot find the binary and simply considers who-can as its subcommand.

[skaven81]

Agreed -- it looks like that code block needs to be updated so that it enumerates through the whole list of cmdArgs and skip over anything that is an option, and just grab the first argument. If the first argument is a plugin, then call the plugin with the options that came before it appended, followed by the remainingArgs

[brianpursley]

@skaven81 and @knight42 I think your ideas are on the right track, but it gets a little tricky to figure out what to do when you consider this:

--namespace=test
vs
--namespace test

First one is simple to parse because it has no space, but the second one is harder because you need to know something about the context of the flag to know whether test belongs to the flag or is a separate command arg. Is that arg a two-part arg or one-part? Does that arg have a default value if the second part is omitted? I used namespace here, but it could be any flag in theory.

Take a look at Cobra's Traverse method that can separate out the command and args:
https://github.com/spf13/cobra/blob/master/command.go#L671-L710

Maybe that can somehow be used, but I think it only works if the command you are looking for is a subcommand, and plugins are not. But maybe a temporary dummy subcommand could be created for the plugin so that this method could be used to parse out the args without having to re-invent that logic.

Just a thought.

Also check out stripFlags from the same file:
https://github.com/spf13/cobra/blob/master/command.go#L550-L586

[knight42]

I am not sure whether it is a good idea to call the plugin with options before it, since users may assume plugin accepts the global flags as kubectl subcommand, but that is not always true.

[knight42]

also I think we'd better change the title of this issue to something like kubectl plugins doesn't work with flags before its name

[skaven81]

I edited the title and description to make it more obvious to newcomers that this has nothing to do with strace.

[knight42]

IMHO I think a better choice might be warn users about the options before the plugin name and call the plugin with remaining args after the plugin name.

[brianpursley]

@knight42 That sounds like a good approach to me.

I think there is still the situation of how do you identify what is supposed to be the plugin name vs some flag value.

For example, if you have this alias:

alias k="kubectl --namespace test"

If you try to run the foo plugin like this:

k foo

I think the code would consider the args to be test foo and it won't be able to find the foo plugin. It would look for the following plugins and not find them:

• kubectl-test-foo
• kubectl-test

BUT...

I think as long as the warning that you proposed is there, you could just accept that having flags without = before the plugin name is going to break it entirely. The warning should provide enough information to the user for them to figure it out.

[knight42]

/assign

[knight42]

@brianpursley @skaven81 I have filed a PR kubernetes/kubernetes#92343.

EDIT:
based on the the discussion in kubernetes/kubernetes#92343, I decided to pass all the flags to plugin regardless of their position.

I have changed my mind a little bit, I decided to simply print an error message and exit rather than calling the plugin with remaining args since I think that would lead to unexpected behavior of plugin to users.

[seans3]

/area kubectl

[seans3]

/sig cli

[seans3]

/priority P2

[liggitt]

this was discussed in kubernetes/kubernetes#77666 and during the initial plugin impl review at juanvallejo/kubernetes#9 (comment) and juanvallejo/kubernetes#9 (comment)

I don't think we have sufficient information to know how to relocate flags/args that appear ahead of plugin names

[misterikkit]

For me, the main use case is to specify --kubeconfig=some-other-kubeconfig because most kubectl plugins will talk to your cluster. I find it much easier to manage separate kubeconfig files versus many contexts in one file, and I have some aliases like ka='kubectl --kubeconfig="$ADMIN_KUBECONFIG"'

[ahmetb]

Since I've approved most of the 100 plugins in Krew, I can say that majority of the plugins out there don't even support -n/--namespace –let alone --kubeconfig. :)

As long as users can still tell what's the error, it's fine.

[misterikkit]

@ahmetb Thanks for that data point. It seems fair that we shouldn't worry about supporting this use case. I do agree that the error output could be improved.

It seems like plugins tend to rely on the KUBECONFIG env, so for anyone else reading this issue, I changed my alias from

alias ka='kubectl --kubeconfig="$ADMIN_KUBECONFIG"'

to

alias ka='KUBECONFIG="$ADMIN_KUBECONFIG" kubectl'

And now I can run commands like ka krew

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[AntonOfTheWoods]

Since I've approved most of the 100 plugins in Krew, I can say that majority of the plugins out there don't even support -n/--namespace –let alone --kubeconfig. :)

As long as users can still tell what's the error, it's fine.

@ahmetb this may just be me but this definitely violates the "principle of least surprise" for me. I have long found it quite frustrating (and a little un-unixy) that kubectl often can't interpret --kubeconfig when it appears as the first parameter, which seems to me to be the most unixy default behaviour. That said, life is like a box of chocolates... But would it not be better to put this information clearly in the docs to manage user expecations? The [intro docs] (https://kubernetes.io/docs/reference/kubectl/overview/) state:

The kubectl command line tool lets you control Kubernetes clusters. For configuration, kubectl looks for a file named config in the $HOME/.kube directory. You can specify other kubeconfig files by setting the KUBECONFIG environment variable or by setting the --kubeconfig flag.

There is no mention that users will likely encounter issues if they use one of these in particular. I know krew probably isn't "official" but it seems to me to detract from the overall user experience. I don't see any mention of this in the plugin docs either. Basically my default assumption would be that kubectl takes control of a first pass of any parameters and gets parameters that are responsible for basic stuff like config and, in this case, overrides the default config options before passing on to the plugins. Obvously us users can get used to anything but it seems this way creates some unneeded friction...

[sastorsl]

Without going into the way kubectl employs plugins.

When considering aliases you should look into functions instead, which can handle parameters.

# Create an alias
$ alias kd="kubectl -n devops"

# Getting an error
$ kd view-secret deployer-token-8d28z
Error: flags cannot be placed before plugin name: -n

# Removing the alias and adding a function instead
$ unalias kd
$ function kd(){ kubectl "$@" -n devops; }

# Using the function
$ kd view-secret deployer-token-8d28z
Multiple sub keys found. Specify another argument, one of:
-> ca.crt
-> namespace
-> service-ca.crt
-> token