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 3a2bdeac8ee..c3f4a6d8be3 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/cmd_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/cmd_test.go @@ -28,6 +28,7 @@ import ( "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/cli-runtime/pkg/genericiooptions" + "k8s.io/kubectl/pkg/cmd/plugin" cmdtesting "k8s.io/kubectl/pkg/cmd/testing" cmdutil "k8s.io/kubectl/pkg/cmd/util" ) @@ -60,11 +61,11 @@ func TestNormalizationFuncGlobalExistence(t *testing.T) { func TestKubectlSubcommandShadowPlugin(t *testing.T) { tests := []struct { - name string - args []string - expectPlugin string - expectPluginArgs []string - expectError string + name string + args []string + expectPlugin string + expectPluginArgs []string + expectLookupError string }{ { name: "test that a plugin executable is found based on command args when builtin subcommand does not exist", @@ -73,13 +74,13 @@ func TestKubectlSubcommandShadowPlugin(t *testing.T) { expectPluginArgs: []string{"--bar", "--bar2", "--namespace", "test-namespace"}, }, { - name: "test that a plugin executable is not found based on command args when also builtin subcommand does not exist", - args: []string{"kubectl", "create", "foo2", "--bar", "--bar2", "--namespace", "test-namespace"}, - expectError: "unable to find a plugin executable \"kubectl-create-foo2\"", + name: "test that a plugin executable is not found based on command args when also builtin subcommand does not exist", + args: []string{"kubectl", "create", "foo2", "--bar", "--bar2", "--namespace", "test-namespace"}, + expectLookupError: "unable to find a plugin executable \"kubectl-create-foo2\"", }, { name: "test that normal commands are able to be executed, when builtin subcommand exists", - args: []string{"kubectl", "create", "role", "--bar", "--bar2", "--namespace", "test-namespace"}, + args: []string{"kubectl", "create", "job", "foo", "--image=busybox", "--dry-run=client", "--namespace", "test-namespace"}, expectPlugin: "", expectPluginArgs: []string{}, }, @@ -87,7 +88,7 @@ func TestKubectlSubcommandShadowPlugin(t *testing.T) { // just to retest them also when feature is enabled. { name: "test that normal commands are able to be executed, when no plugin overshadows them", - args: []string{"kubectl", "get", "foo"}, + args: []string{"kubectl", "config", "get-clusters"}, expectPlugin: "", expectPluginArgs: []string{}, }, @@ -99,7 +100,7 @@ func TestKubectlSubcommandShadowPlugin(t *testing.T) { }, { name: "test that a plugin does not execute over an existing command by the same name", - args: []string{"kubectl", "version"}, + args: []string{"kubectl", "version", "--client=true"}, }, { name: "test that a plugin does not execute over Cobra's help command", @@ -107,11 +108,11 @@ func TestKubectlSubcommandShadowPlugin(t *testing.T) { }, { name: "test that a plugin does not execute over Cobra's __complete command", - args: []string{"kubectl", cobra.ShellCompRequestCmd}, + args: []string{"kubectl", cobra.ShellCompRequestCmd, "de"}, }, { name: "test that a plugin does not execute over Cobra's __completeNoDesc command", - args: []string{"kubectl", cobra.ShellCompNoDescRequestCmd}, + args: []string{"kubectl", cobra.ShellCompNoDescRequestCmd, "de"}, }, { name: "test that a flag does not break Cobra's help command", @@ -132,22 +133,39 @@ func TestKubectlSubcommandShadowPlugin(t *testing.T) { t.Run(test.name, func(t *testing.T) { pluginsHandler := &testPluginHandler{ pluginsDirectory: "plugin/testdata", + validPrefixes: plugin.ValidPluginFilenamePrefixes, } ioStreams, _, _, _ := genericiooptions.NewTestIOStreams() - root := NewDefaultKubectlCommandWithArgs(KubectlOptions{PluginHandler: pluginsHandler, Arguments: test.args, IOStreams: ioStreams}) - if err := root.Execute(); err != nil { - t.Fatalf("unexpected error: %v", err) + // original plugin handler (DefaultPluginHandler) is implemented by exec call so no additional actions are expected on the cobra command if we activate the plugin flow + if !pluginsHandler.lookedup && !pluginsHandler.executed { + // args must be set, otherwise Execute will use os.Args (args used for starting the test) and test.args would not be passed + // to the command which might invoke only "kubectl" without any additional args and give false positives + root.SetArgs(test.args[1:]) + // Important note! Incorrect command or command failing validation might just call os.Exit(1) which would interrupt execution of the test + if err := root.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } } - if pluginsHandler.err != nil && pluginsHandler.err.Error() != test.expectError { - t.Fatalf("unexpected error: expected %q to occur, but got %q", test.expectError, pluginsHandler.err) + if (pluginsHandler.lookupErr != nil && pluginsHandler.lookupErr.Error() != test.expectLookupError) || + (pluginsHandler.lookupErr == nil && len(test.expectLookupError) > 0) { + t.Fatalf("unexpected error: expected %q to occur, but got %q", test.expectLookupError, pluginsHandler.lookupErr) + } + + if pluginsHandler.lookedup && !pluginsHandler.executed && len(test.expectLookupError) == 0 { + // we have to fail here, because we have found the plugin, but not executed the plugin, nor the command (this would normally result in an error: unknown command) + t.Fatalf("expected plugin execution, but did not occur") } if pluginsHandler.executedPlugin != test.expectPlugin { t.Fatalf("unexpected plugin execution: expected %q, got %q", test.expectPlugin, pluginsHandler.executedPlugin) } + if pluginsHandler.executed && len(test.expectPlugin) == 0 { + t.Fatalf("unexpected plugin execution: expected no plugin, got %q", pluginsHandler.executedPlugin) + } + if !cmp.Equal(pluginsHandler.withArgs, test.expectPluginArgs, cmpopts.EquateEmpty()) { t.Fatalf("unexpected plugin execution args: expected %q, got %q", test.expectPluginArgs, pluginsHandler.withArgs) } @@ -158,21 +176,21 @@ func TestKubectlSubcommandShadowPlugin(t *testing.T) { func TestKubectlCommandHandlesPlugins(t *testing.T) { tests := []struct { - name string - args []string - expectPlugin string - expectPluginArgs []string - expectError string + name string + args []string + expectPlugin string + expectPluginArgs []string + expectLookupError string }{ { name: "test that normal commands are able to be executed, when no plugin overshadows them", - args: []string{"kubectl", "get", "foo"}, + args: []string{"kubectl", "config", "get-clusters"}, expectPlugin: "", expectPluginArgs: []string{}, }, { name: "test that normal commands are able to be executed, when no plugin overshadows them and shadowing feature is not enabled", - args: []string{"kubectl", "create", "foo"}, + args: []string{"kubectl", "create", "job", "foo", "--image=busybox", "--dry-run=client"}, expectPlugin: "", expectPluginArgs: []string{}, }, @@ -184,7 +202,7 @@ 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"}, + args: []string{"kubectl", "version", "--client=true"}, }, // The following tests make sure that commands added by Cobra cannot be shadowed by a plugin // See https://github.com/kubernetes/kubectl/issues/1116 @@ -194,11 +212,11 @@ func TestKubectlCommandHandlesPlugins(t *testing.T) { }, { name: "test that a plugin does not execute over Cobra's __complete command", - args: []string{"kubectl", cobra.ShellCompRequestCmd}, + args: []string{"kubectl", cobra.ShellCompRequestCmd, "de"}, }, { name: "test that a plugin does not execute over Cobra's __completeNoDesc command", - args: []string{"kubectl", cobra.ShellCompNoDescRequestCmd}, + args: []string{"kubectl", cobra.ShellCompNoDescRequestCmd, "de"}, }, // 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. @@ -236,22 +254,39 @@ func TestKubectlCommandHandlesPlugins(t *testing.T) { t.Run(test.name, func(t *testing.T) { pluginsHandler := &testPluginHandler{ pluginsDirectory: "plugin/testdata", + validPrefixes: plugin.ValidPluginFilenamePrefixes, } ioStreams, _, _, _ := genericiooptions.NewTestIOStreams() - root := NewDefaultKubectlCommandWithArgs(KubectlOptions{PluginHandler: pluginsHandler, Arguments: test.args, IOStreams: ioStreams}) - if err := root.Execute(); err != nil { - t.Fatalf("unexpected error: %v", err) + // original plugin handler (DefaultPluginHandler) is implemented by exec call so no additional actions are expected on the cobra command if we activate the plugin flow + if !pluginsHandler.lookedup && !pluginsHandler.executed { + // args must be set, otherwise Execute will use os.Args (args used for starting the test) and test.args would not be passed + // to the command which might invoke only "kubectl" without any additional args and give false positives + root.SetArgs(test.args[1:]) + // Important note! Incorrect command or command failing validation might just call os.Exit(1) which would interrupt execution of the test + if err := root.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } } - if pluginsHandler.err != nil && pluginsHandler.err.Error() != test.expectError { - t.Fatalf("unexpected error: expected %q to occur, but got %q", test.expectError, pluginsHandler.err) + if (pluginsHandler.lookupErr != nil && pluginsHandler.lookupErr.Error() != test.expectLookupError) || + (pluginsHandler.lookupErr == nil && len(test.expectLookupError) > 0) { + t.Fatalf("unexpected error: expected %q to occur, but got %q", test.expectLookupError, pluginsHandler.lookupErr) + } + + if pluginsHandler.lookedup && !pluginsHandler.executed && len(test.expectLookupError) == 0 { + // we have to fail here, because we have found the plugin, but not executed the plugin, nor the command (this would normally result in an error: unknown command) + t.Fatalf("expected plugin execution, but did not occur") } if pluginsHandler.executedPlugin != test.expectPlugin { t.Fatalf("unexpected plugin execution: expected %q, got %q", test.expectPlugin, pluginsHandler.executedPlugin) } + if pluginsHandler.executed && len(test.expectPlugin) == 0 { + t.Fatalf("unexpected plugin execution: expected no plugin, got %q", pluginsHandler.executedPlugin) + } + if !cmp.Equal(pluginsHandler.withArgs, test.expectPluginArgs, cmpopts.EquateEmpty()) { t.Fatalf("unexpected plugin execution args: expected %q, got %q", test.expectPluginArgs, pluginsHandler.withArgs) } @@ -261,47 +296,55 @@ func TestKubectlCommandHandlesPlugins(t *testing.T) { type testPluginHandler struct { pluginsDirectory string + validPrefixes []string + + // lookup results + lookedup bool + lookupErr error // execution results + executed bool executedPlugin string withArgs []string withEnv []string - - err error } func (h *testPluginHandler) Lookup(filename string) (string, bool) { - // append supported plugin prefix to the filename - filename = fmt.Sprintf("%s-%s", "kubectl", filename) + h.lookedup = true dir, err := os.Stat(h.pluginsDirectory) if err != nil { - h.err = err + h.lookupErr = err return "", false } if !dir.IsDir() { - h.err = fmt.Errorf("expected %q to be a directory", h.pluginsDirectory) + h.lookupErr = fmt.Errorf("expected %q to be a directory", h.pluginsDirectory) return "", false } plugins, err := os.ReadDir(h.pluginsDirectory) if err != nil { - h.err = err + h.lookupErr = err return "", false } - for _, p := range plugins { - if p.Name() == filename { - return fmt.Sprintf("%s/%s", h.pluginsDirectory, p.Name()), true + filenameWithSuportedPrefix := "" + for _, prefix := range h.validPrefixes { + for _, p := range plugins { + filenameWithSuportedPrefix = fmt.Sprintf("%s-%s", prefix, filename) + if p.Name() == filenameWithSuportedPrefix { + return fmt.Sprintf("%s/%s", h.pluginsDirectory, p.Name()), true + } } } - h.err = fmt.Errorf("unable to find a plugin executable %q", filename) + h.lookupErr = fmt.Errorf("unable to find a plugin executable %q", filenameWithSuportedPrefix) return "", false } func (h *testPluginHandler) Execute(executablePath string, cmdArgs, env []string) error { + h.executed = true h.executedPlugin = executablePath h.withArgs = cmdArgs h.withEnv = env