From e703b3d25377e763b117805b3d88fe7236f3ff76 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 --- staging/src/k8s.io/kubectl/pkg/cmd/cmd.go | 14 +++++++++++--- staging/src/k8s.io/kubectl/pkg/cmd/cmd_test.go | 13 +++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go b/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go index fcdc8574bc3..78d3a9c04e0 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go @@ -102,9 +102,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/staging/src/k8s.io/kubectl/pkg/cmd/cmd_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/cmd_test.go index d01cf04fc8d..1e24ca71742 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/cmd_test.go +++ b/staging/src/k8s.io/kubectl/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 d41c2685b67317fea1d1a3843d8f5870bc724b3b 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 --- staging/src/k8s.io/kubectl/pkg/cmd/cmd.go | 10 +++++- .../src/k8s.io/kubectl/pkg/cmd/cmd_test.go | 32 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go b/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go index 78d3a9c04e0..7155effdbd2 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go @@ -105,7 +105,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/staging/src/k8s.io/kubectl/pkg/cmd/cmd_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/cmd_test.go index 1e24ca71742..f64d84365b1 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/cmd_test.go +++ b/staging/src/k8s.io/kubectl/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) }