From 10504c18ade2c0328478f94647a81550bbf06e6d Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Sun, 19 Sep 2021 14:34:10 -0400 Subject: [PATCH 1/2] Do not try to load plugins for cobra commands Cobra adds the commands 'help', '__complete' and '__completeNoDesc' inside rootCmd.Execute(), however, when kubectl decides if it should lookup plugins, rootCmd.Execute() had not been called yet. Therefore, the call to cmd.Find(cmdPathPieces) done by kubectl does not find the commands added by Cobra. To fix this we must check for them explicitly. Signed-off-by: Marc Khouzam Kubernetes-commit: e703b3d25377e763b117805b3d88fe7236f3ff76 --- pkg/cmd/cmd.go | 14 +++++++++++--- pkg/cmd/cmd_test.go | 13 +++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/cmd.go b/pkg/cmd/cmd.go index c01899c48..a89aef7d7 100644 --- a/pkg/cmd/cmd.go +++ b/pkg/cmd/cmd.go @@ -101,9 +101,17 @@ func NewDefaultKubectlCommandWithArgs(pluginHandler PluginHandler, args []string // only look for suitable extension executables if // the specified command does not already exist if _, _, err := cmd.Find(cmdPathPieces); err != nil { - if err := HandlePluginCommand(pluginHandler, cmdPathPieces); err != nil { - fmt.Fprintf(errout, "Error: %v\n", err) - os.Exit(1) + // Also check the commands that will be added by Cobra. + // These commands are only added once rootCmd.Execute() is called, so we + // need to check them explicitly here. + switch cmdPathPieces[0] { + case "help", cobra.ShellCompRequestCmd, cobra.ShellCompNoDescRequestCmd: + // Don't search for a plugin + default: + if err := HandlePluginCommand(pluginHandler, cmdPathPieces); err != nil { + fmt.Fprintf(errout, "Error: %v\n", err) + os.Exit(1) + } } } } diff --git a/pkg/cmd/cmd_test.go b/pkg/cmd/cmd_test.go index d01cf04fc..1e24ca717 100644 --- a/pkg/cmd/cmd_test.go +++ b/pkg/cmd/cmd_test.go @@ -78,6 +78,19 @@ func TestKubectlCommandHandlesPlugins(t *testing.T) { name: "test that a plugin does not execute over an existing command by the same name", args: []string{"kubectl", "version"}, }, + // The following tests make sure that commands added by Cobra cannot be shadowed by a plugin + { + name: "test that a plugin does not execute over Cobra's help command", + args: []string{"kubectl", "help"}, + }, + { + name: "test that a plugin does not execute over Cobra's __complete command", + args: []string{"kubectl", cobra.ShellCompRequestCmd}, + }, + { + name: "test that a plugin does not execute over Cobra's __completeNoDesc command", + args: []string{"kubectl", cobra.ShellCompNoDescRequestCmd}, + }, } for _, test := range tests { From e6118325f3c69bd0752c3562397df60e5b0ddb78 Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Mon, 20 Sep 2021 18:20:49 -0400 Subject: [PATCH 2/2] Ignore flags that could precede the Cobra command See https://github.com/kubernetes/kubectl/issues/1119 Signed-off-by: Marc Khouzam Kubernetes-commit: d41c2685b67317fea1d1a3843d8f5870bc724b3b --- pkg/cmd/cmd.go | 10 +++++++++- pkg/cmd/cmd_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/cmd.go b/pkg/cmd/cmd.go index a89aef7d7..5c13f51fb 100644 --- a/pkg/cmd/cmd.go +++ b/pkg/cmd/cmd.go @@ -104,7 +104,15 @@ func NewDefaultKubectlCommandWithArgs(pluginHandler PluginHandler, args []string // Also check the commands that will be added by Cobra. // These commands are only added once rootCmd.Execute() is called, so we // need to check them explicitly here. - switch cmdPathPieces[0] { + var cmdName string // first "non-flag" arguments + for _, arg := range cmdPathPieces { + if !strings.HasPrefix(arg, "-") { + cmdName = arg + break + } + } + + switch cmdName { case "help", cobra.ShellCompRequestCmd, cobra.ShellCompNoDescRequestCmd: // Don't search for a plugin default: diff --git a/pkg/cmd/cmd_test.go b/pkg/cmd/cmd_test.go index 1e24ca717..f64d84365 100644 --- a/pkg/cmd/cmd_test.go +++ b/pkg/cmd/cmd_test.go @@ -79,6 +79,7 @@ func TestKubectlCommandHandlesPlugins(t *testing.T) { args: []string{"kubectl", "version"}, }, // The following tests make sure that commands added by Cobra cannot be shadowed by a plugin + // See https://github.com/kubernetes/kubectl/issues/1116 { name: "test that a plugin does not execute over Cobra's help command", args: []string{"kubectl", "help"}, @@ -91,6 +92,36 @@ func TestKubectlCommandHandlesPlugins(t *testing.T) { name: "test that a plugin does not execute over Cobra's __completeNoDesc command", args: []string{"kubectl", cobra.ShellCompNoDescRequestCmd}, }, + // The following tests make sure that commands added by Cobra cannot be shadowed by a plugin + // even when a flag is specified first. This can happen when using aliases. + // See https://github.com/kubernetes/kubectl/issues/1119 + { + name: "test that a flag does not break Cobra's help command", + args: []string{"kubectl", "--kubeconfig=/path/to/kubeconfig", "help"}, + }, + { + name: "test that a flag does not break Cobra's __complete command", + args: []string{"kubectl", "--kubeconfig=/path/to/kubeconfig", cobra.ShellCompRequestCmd}, + }, + { + name: "test that a flag does not break Cobra's __completeNoDesc command", + args: []string{"kubectl", "--kubeconfig=/path/to/kubeconfig", cobra.ShellCompNoDescRequestCmd}, + }, + // As for the previous tests, an alias could add a flag without using the = form. + // We don't support this case as parsing the flags becomes quite complicated (flags + // that take a value, versus flags that don't) + // { + // name: "test that a flag with a space does not break Cobra's help command", + // args: []string{"kubectl", "--kubeconfig", "/path/to/kubeconfig", "help"}, + // }, + // { + // name: "test that a flag with a space does not break Cobra's __complete command", + // args: []string{"kubectl", "--kubeconfig", "/path/to/kubeconfig", cobra.ShellCompRequestCmd}, + // }, + // { + // name: "test that a flag with a space does not break Cobra's __completeNoDesc command", + // args: []string{"kubectl", "--kubeconfig", "/path/to/kubeconfig", cobra.ShellCompNoDescRequestCmd}, + // }, } for _, test := range tests { @@ -105,6 +136,7 @@ func TestKubectlCommandHandlesPlugins(t *testing.T) { }) root := NewDefaultKubectlCommandWithArgs(pluginsHandler, test.args, in, out, errOut) + root.SetOut(out) if err := root.Execute(); err != nil { t.Fatalf("unexpected error: %v", err) }