Begin prototyping plugin invocation #9

juanvallejo · 2018-05-29T18:15:30Z

Opening here first to generate some discussion about implementation details.

With the current design, a user is able to place an executable file anywhere on their PATH.
A search is performed based on the command line arguments given by the user.
For a path kubectl plugin foo, for example, we:

search to see if the subcommand "plugin" exists.

if "plugin" exists, we lookup its annotations to see if it is marked as "can-be-extended"
if "plugin" does not exist, kubectl cannot be "extended", therefore, we treat that as a "command-not-found" error always.

search to see if subcommand "foo" exists.

if "foo" exists, we lookup its annotations to see if it is marked as "can-be-extended"
if "foo" does not exist, we check to see if its parent was marked as "can-be-extended"
- if foo's parent was marked as "can-be-extended", we attempt to look for the executable kubectl-plugin-foo [3] (kubectl-plugin-foo.exe on windows) [2] in the user's PATH.
  - if the executable (kubectl-plugin-foo) is not found in the user's PATH, we exit with error [1].
- if foo's parent was not marked as "can-be-extended", we exit with error [1].

error: unknown command "foo"
See 'kubectl plugin -h' for help and examples.

Note that the binary's name is not bound to kubectl-plugin-..., if we were writing a plugin for "create", for example, we would look for a binary kubectl-create-foo
Also note that the current implementation looks for the longest possible binary in the user's path that it can find. For example, if a user specifies kubectl plugin foo bar baz, and the user's PATH contains two binaries (kubectl-plugin-foo, and kubectl-plugin-foo-bar-baz), then kubectl-plugin-foo-bar-baz would always be executed.

cc @deads2k @soltysh @mfojtik

juanvallejo · 2018-05-29T18:31:48Z

Example:

$ cat ./plugins/kubectl-plugin-foo
#!/bin/bash

echo "Hello world"

$ kubectl plugin foo
Hello world

$ kubectl plugin nonexistent
error: unknown command "nonexistent"
See 'kubectl plugin -h' for help and examples.

juanvallejo · 2018-05-29T19:42:09Z

TODO:

Handle passing current working directory to plugin binary
- ~~we could pass this information via an environment variable to the plugin binary~~
- This information is preserved via the execve call
Handle environment and how it is passed to plugin binary (document)
- execve syscall receives an argument containing environment variables to make available to the subprocess
Handle commands with dashes in their name
- if a plugin binary is extending a command whose name contains dashes (e.g. kubectl api-resources foo), those must be represented as underscores in the binary name: kubectl-api_resources-foo
Describe where we load plugins from (user's PATH)
- plugin binaries are looked up on the user's PATH.
- a plugin binary must start with "kubectl-" in its filename
- a plugin binary must include the full "command path" that it is extending, with each argument separated by dashes (kubectl-plugin-foo-bar)
- if there are two "competing" binaries in the user's path, (e.g. kubectl-plugin-foo, and kubectl-plugin-foo-bar), then the binary with the "longest matching" name will be executed. See [1].
commands opt-in to be extensible
- commands do this by setting a cobra command annotation in their constructor
Describe how args are passed to a plugin binary
- all args are passed as-is to the plugin binary via an execve syscall
Handle verification of commands that opt-in to be extensible (extensions-verifier that allows us to approve new commands that opt-in to be extended)
Figure out how to best handle sub-command aliases when mapping command args to a plugin binary's name (e.g. kubectl cp foo -> plugins/kubectl-copy-foo)

$ ls plugins
kubectl-plugin-foo  kubectl-plugin-foo-bar

$ kubectl plugin foo bar baz
# executing binary `kubectl-plugin-foo-bar`

$ kubectl plugin foo baz buz
# executing binary `kubectl-plugin-foo`

juanvallejo · 2018-05-29T21:43:29Z

cc @liggitt

liggitt · 2018-05-30T05:34:01Z

pkg/kubectl/cmd/cmd.go

+
+	platformSuffix := ""
+	if runtime.GOOS == "windows" {
+		platformSuffix = ".exe"


LookPath on windows appears to append this on its own - https://golang.org/src/os/exec/lp_windows.go#L59

liggitt · 2018-05-30T05:40:32Z

pkg/kubectl/cmd/cmd.go

+	}
+
+	// invoke cmd binary relaying the current environment and args given
+	if err := syscall.Exec(foundBinaryPath, os.Args[1:], os.Environ()); err != nil {


does this preserve the current working dir of the kubectl invocation?

should we pass os.Args[len(remainingArgs):], so that kubectl plugin foo bar baz passes baz to kubectl-plugin-foo-bar?

should we pass cmdArgs[...] rather than os.Args[...]?

does this preserve the current working dir of the kubectl invocation?

At least on a local env, I saw this to be true - os.Getwd() in kubectl returned the same value as pwd in the plugin executable when run via the execve call.

should we pass os.Args[len(remainingArgs):], so that kubectl plugin foo bar baz passes baz to kubectl-plugin-foo-bar?

I was not sure about this since there was some discussion about passing args "as-is", but I'm not against this.

liggitt · 2018-05-30T05:46:04Z

pkg/kubectl/cmd/cmd.go

+func handleEndpointExtensions(cmdArgs []string) {
+	remainingArgs := cmdArgs
+	for idx := range remainingArgs {
+		remainingArgs[idx] = strings.Replace(remainingArgs[idx], "-", "_", -1)


should we stop as soon as we encounter an option (an arg starting with -)?

append the normalized form to a new slice, overwriting here modifies cmdArgs and cmdPathPieces and os.Args

should we stop as soon as we encounter an option (an arg starting with -)?

Would this be an invalid operation? Could we instead "move" those from remainingArgs to a separate slice, then append them to the list of args that we ultimately pass to the subprocess? I would be in support of allowing flags to be passed as arguments to plugins

append the normalized form to a new slice, overwriting here modifies cmdArgs and cmdPathPieces and os.Args

ack

Could we instead "move" those from remainingArgs to a separate slice, then append them to the list of args that we ultimately pass to the subprocess?

we don't have enough info to move flags here. you would also have to move accompanying value args (e.g. move --foo bar as a unit, without knowing whether --foo is an arg that expects a value).

I would be in support of allowing flags to be passed as arguments to plugins

absolutely, I'm saying we should not include them in the list of args considered when searching for a named plugin.

If I call kubectl plugins foo bar --baz qux, we should start our search with kubectl-plugins-foo-bar. If we find kubectl-plugins-foo, I'd expect to pass the args ["bar","--baz","qux"] to it

Note that this implies all flags come after the command name, which is sometimes-required-and-sometimes-not for native kubectl commands, depending on whether they are global flags like --namespace or command-specific flags like --dry-run

@liggitt

If I call kubectl plugins foo bar --baz qux, we should start our search with kubectl-plugins-foo-bar. If we find kubectl-plugins-foo, I'd expect to pass the args ["bar","--baz","qux"] to it

This was my original approach when searching for a plugin binary, however @deads2k pointed out that if a user has two binaries kubectl-plugins-foo and kubectl-plugins-foo-bar in his PATH, it is more reasonable for our use-case that we match kubectl-plugins-foo-bar over the other one, hence why I switched to reverse matching the binary name.

Also, thinking about flag handling a bit more, another scenario to consider would be when a user specifies a flag [and a potential value for it] in the middle of a command invocation: kubectl plugin foo --flagname maybeflagvalue bar baz (binary to match here could be kubectl-plugin-foo-bar)

This was my original approach when searching for a plugin binary, however @deads2k pointed out that if a user has two binaries kubectl-plugins-foo and kubectl-plugins-foo-bar in his PATH, it is more reasonable for our use-case that we match kubectl-plugins-foo-bar over the other one, hence why I switched to reverse matching the binary name.

I agree with the reverse search. In my example, the reverse search would consist of:

kubectl-plugins-foo-bar

kubectl-plugins-foo

It would not include kubectl-plugins-foo-bar-__bar-qux

Also, thinking about flag handling a bit more, another scenario to consider would be when a user specifies a flag [and a potential value for it] in the middle of a command invocation: kubectl plugin foo --flagname maybeflagvalue bar baz (binary to match here could be kubectl-plugin-foo-bar)

We don't have sufficient information to support that case. That's what I was trying to describe in this comment:

we don't have enough info to move flags here. you would also have to move accompanying value args (e.g. move --foo bar as a unit, without knowing whether --foo is an arg that expects a value).

We don't have sufficient information to support that case. That's what I was trying to describe in this comment:

Makes sense, will update reverse search to take "flag-value" pairs into account. Thanks

liggitt · 2018-05-30T05:47:03Z

Handle passing current working directory to plugin binary

is that not inherited by an exec call?

mfojtik · 2018-05-30T09:19:14Z

@liggitt it is, we use same way to execute in oc adm for example...

juanvallejo · 2018-05-31T15:27:46Z

Example with --flags

$ ls plugins
kubectl-plugin-foo  kubectl-plugin-foo-bar

$ kubectl plugin foo bar --flag buz baz
exec plugin "foo bar"
with options: baz --flag buz

$ kubectl plugin foo bar --flag=buz baz
exec plugin "foo bar"
with options: baz --flag=buz

$ touch plugins/kubectl-plugin-foo-bar-baz
# populate new plugin file...

$ ls plugins
kubectl-plugin-foo  kubectl-plugin-foo-bar  kubectl-plugin-foo-bar-baz

$ kubectl plugin foo bar --flag buz baz
exec plugin "foo bar baz"
with options: --flag buz

liggitt · 2018-05-31T15:34:20Z

don't reorder args at all... you don't know if the arg following a flag is associated with the flag or not

$ kubectl plugin foo bar --flag buz baz
exec plugin "foo bar"
with options: --flag buz baz

$ kubectl plugin foo bar --flag=buz baz
exec plugin "foo bar"
with options: --flag=buz baz

notably, kubectl plugin foo bar --flag buz baz would not be able to locate kubectl-plugin-foo-bar-baz:

$ kubectl plugin foo bar --flag buz baz
exec plugin "foo bar"
with options: --flag buz baz

juanvallejo · 2018-05-31T18:07:38Z

@liggitt

don't reorder args at all... you don't know if the arg following a flag is associated with the flag or not

Done in latest commit:

$ kubectl plugin foo bar --flag buz baz
exec plugin "foo bar"
with options: --flag buz baz

$ kubectl plugin foo bar baz --flag buz
exec plugin "foo bar"
with options: baz --flag buz

$ kubectl plugin foo --flag buz bar baz
exec plugin "foo bar"
with options: --flag buz baz

liggitt · 2018-05-31T18:23:33Z

close, but the last one didn't look right:

$ kubectl plugin foo --flag buz bar baz
exec plugin "foo"
with options: --flag buz bar baz

as soon as you hit a flag arg, stop searching for plugins

juanvallejo · 2018-05-31T19:01:37Z

@liggitt

as soon as you hit a flag arg, stop searching for plugins

ack, updated last commit

liggitt · 2018-06-01T03:08:12Z

pkg/kubectl/cmd/cmd.go

+		// if dealing with a binary extension.
+		currentCmd := cmd
+		canBeExtended := false
+		for _, target := range cmdPathPieces {


can this be simplified to:

args := os.Args[1:] leafCmd, _, _ := cmd.Find(args) if leafCmd != nil && leafCmd.Annotations[EndpointCanBeExtendedAnnotation] == "true" { handleEndpointExtensions(args) }

soltysh

lgtm

juanvallejo · 2018-08-01T20:12:14Z

This has been opened against upstream.

juanvallejo force-pushed the jvallejo/prototype-plugins branch 2 times, most recently from 0ce1e8d to 68259c2 Compare May 29, 2018 18:18

juanvallejo force-pushed the jvallejo/prototype-plugins branch from 68259c2 to 261838b Compare May 29, 2018 18:45

juanvallejo force-pushed the jvallejo/prototype-plugins branch 6 times, most recently from a6baaa6 to 95199b2 Compare May 29, 2018 21:20

liggitt reviewed May 30, 2018

View reviewed changes

juanvallejo force-pushed the jvallejo/prototype-plugins branch 6 times, most recently from e8b71b0 to 91f8077 Compare May 30, 2018 16:10

juanvallejo force-pushed the jvallejo/prototype-plugins branch from 105e962 to fe78d6d Compare May 31, 2018 19:01

liggitt reviewed Jun 1, 2018

View reviewed changes

juanvallejo force-pushed the jvallejo/prototype-plugins branch from fe78d6d to 1ef6617 Compare June 1, 2018 14:16

juanvallejo mentioned this pull request Jul 24, 2018

add draft juanvallejo/community#1

Closed

soltysh approved these changes Jul 25, 2018

View reviewed changes

juanvallejo force-pushed the jvallejo/prototype-plugins branch from 601080a to b4cc10d Compare July 25, 2018 17:54

juanvallejo force-pushed the jvallejo/prototype-plugins branch from b4cc10d to d5c2c4d Compare August 1, 2018 20:03

add updated plugin mechanism

da2908e

juanvallejo force-pushed the jvallejo/prototype-plugins branch from d5c2c4d to da2908e Compare August 1, 2018 20:07

juanvallejo closed this Aug 1, 2018

liggitt mentioned this pull request May 11, 2019

Kubectl plugins: support global options kubernetes/kubernetes#77666

Closed

liggitt mentioned this pull request Jul 8, 2020

kubectl plugins do not work when options placed before plugin name kubernetes/kubectl#884

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Begin prototyping plugin invocation #9

Begin prototyping plugin invocation #9

juanvallejo commented May 29, 2018 •

edited

Loading

juanvallejo commented May 29, 2018

juanvallejo commented May 29, 2018 •

edited

Loading

juanvallejo commented May 29, 2018

liggitt May 30, 2018 •

edited

Loading

liggitt May 30, 2018

juanvallejo May 30, 2018

liggitt May 30, 2018

juanvallejo May 30, 2018

juanvallejo May 30, 2018

liggitt May 30, 2018

liggitt May 30, 2018 •

edited

Loading

juanvallejo May 31, 2018

liggitt May 31, 2018

liggitt May 31, 2018

juanvallejo May 31, 2018

liggitt commented May 30, 2018

mfojtik commented May 30, 2018

juanvallejo commented May 31, 2018 •

edited

Loading

liggitt commented May 31, 2018 •

edited

Loading

juanvallejo commented May 31, 2018 •

edited

Loading

liggitt commented May 31, 2018 •

edited

Loading

juanvallejo commented May 31, 2018 •

edited

Loading

liggitt Jun 1, 2018 •

edited

Loading

soltysh left a comment

juanvallejo commented Aug 1, 2018

Begin prototyping plugin invocation #9

Begin prototyping plugin invocation #9

Conversation

juanvallejo commented May 29, 2018 • edited Loading

juanvallejo commented May 29, 2018

juanvallejo commented May 29, 2018 • edited Loading

juanvallejo commented May 29, 2018

liggitt May 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt May 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt commented May 30, 2018

mfojtik commented May 30, 2018

juanvallejo commented May 31, 2018 • edited Loading

liggitt commented May 31, 2018 • edited Loading

juanvallejo commented May 31, 2018 • edited Loading

liggitt commented May 31, 2018 • edited Loading

juanvallejo commented May 31, 2018 • edited Loading

liggitt Jun 1, 2018 • edited Loading

Choose a reason for hiding this comment

soltysh left a comment

Choose a reason for hiding this comment

juanvallejo commented Aug 1, 2018

juanvallejo commented May 29, 2018 •

edited

Loading

juanvallejo commented May 29, 2018 •

edited

Loading

liggitt May 30, 2018 •

edited

Loading

liggitt May 30, 2018 •

edited

Loading

juanvallejo commented May 31, 2018 •

edited

Loading

liggitt commented May 31, 2018 •

edited

Loading

juanvallejo commented May 31, 2018 •

edited

Loading

liggitt commented May 31, 2018 •

edited

Loading

juanvallejo commented May 31, 2018 •

edited

Loading

liggitt Jun 1, 2018 •

edited

Loading