From d3f76f431e3cf4a0e7d2a5d62ffa7d674ede8beb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Thu, 9 Mar 2023 16:16:01 +0300 Subject: [PATCH] Enable plugin resolution as subcommand for selected builtin commands (#116293) * Enable plugin resolution as subcommand for selected builtin commands This PR adds external plugin resolution as subcommand for selected builtin commands if subcommand does not exist as builtin. In it's alpha stage, this will only be enabled for create command and this feature is hidden behind `KUBECTL_ENABLE_CMD_SHADOW` environment variable. * Rename parameter to exactMatch to better reflect Kubernetes-commit: a901bb630b5a353898c1b35df582a7faeef160a0 --- pkg/cmd/cmd.go | 44 ++++++++- pkg/cmd/cmd_test.go | 107 +++++++++++++++++++++ pkg/cmd/plugin/plugin_test.go | 1 + pkg/cmd/plugin/testdata/kubectl-create-foo | 3 + pkg/cmd/util/helpers.go | 5 +- 5 files changed, 155 insertions(+), 5 deletions(-) create mode 100644 pkg/cmd/plugin/testdata/kubectl-create-foo diff --git a/pkg/cmd/cmd.go b/pkg/cmd/cmd.go index 8313179b0..85b2d7842 100644 --- a/pkg/cmd/cmd.go +++ b/pkg/cmd/cmd.go @@ -83,6 +83,10 @@ import ( const kubectlCmdHeaders = "KUBECTL_COMMAND_HEADERS" +var ( + allowedCmdsSubcommandPlugin = map[string]struct{}{"create": {}} +) + type KubectlOptions struct { PluginHandler PluginHandler Arguments []string @@ -116,7 +120,7 @@ func NewDefaultKubectlCommandWithArgs(o KubectlOptions) *cobra.Command { // only look for suitable extension executables if // the specified command does not already exist - if _, _, err := cmd.Find(cmdPathPieces); err != nil { + if foundCmd, foundArgs, err := cmd.Find(cmdPathPieces); err != nil { // 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. @@ -132,11 +136,39 @@ func NewDefaultKubectlCommandWithArgs(o KubectlOptions) *cobra.Command { case "help", cobra.ShellCompRequestCmd, cobra.ShellCompNoDescRequestCmd: // Don't search for a plugin default: - if err := HandlePluginCommand(o.PluginHandler, cmdPathPieces); err != nil { + if err := HandlePluginCommand(o.PluginHandler, cmdPathPieces, false); err != nil { fmt.Fprintf(o.IOStreams.ErrOut, "Error: %v\n", err) os.Exit(1) } } + } else if err == nil { + if cmdutil.CmdPluginAsSubcommand.IsEnabled() { + // Command exists(e.g. kubectl create), but it is not certain that + // subcommand also exists (e.g. kubectl create networkpolicy) + if _, ok := allowedCmdsSubcommandPlugin[foundCmd.Name()]; ok { + var subcommand string + for _, arg := range foundArgs { // first "non-flag" argument as subcommand + if !strings.HasPrefix(arg, "-") { + subcommand = arg + break + } + } + builtinSubcmdExist := false + for _, subcmd := range foundCmd.Commands() { + if subcmd.Name() == subcommand { + builtinSubcmdExist = true + break + } + } + + if !builtinSubcmdExist { + if err := HandlePluginCommand(o.PluginHandler, cmdPathPieces, true); err != nil { + fmt.Fprintf(o.IOStreams.ErrOut, "Error: %v\n", err) + os.Exit(1) + } + } + } + } } } @@ -224,7 +256,7 @@ func (h *DefaultPluginHandler) Execute(executablePath string, cmdArgs, environme // HandlePluginCommand receives a pluginHandler and command-line arguments and attempts to find // a plugin executable on the PATH that satisfies the given arguments. -func HandlePluginCommand(pluginHandler PluginHandler, cmdArgs []string) error { +func HandlePluginCommand(pluginHandler PluginHandler, cmdArgs []string, exactMatch bool) error { var remainingArgs []string // all "non-flag" arguments for _, arg := range cmdArgs { if strings.HasPrefix(arg, "-") { @@ -244,6 +276,12 @@ func HandlePluginCommand(pluginHandler PluginHandler, cmdArgs []string) error { for len(remainingArgs) > 0 { path, found := pluginHandler.Lookup(strings.Join(remainingArgs, "-")) if !found { + if exactMatch { + // if exactMatch is true, we shouldn't continue searching with shorter names. + // this is especially for not searching kubectl-create plugin + // when kubectl-create-foo plugin is not found. + break + } remainingArgs = remainingArgs[:len(remainingArgs)-1] continue } diff --git a/pkg/cmd/cmd_test.go b/pkg/cmd/cmd_test.go index a8bc0c842..15a7708c7 100644 --- a/pkg/cmd/cmd_test.go +++ b/pkg/cmd/cmd_test.go @@ -25,7 +25,10 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/spf13/cobra" + "k8s.io/cli-runtime/pkg/genericclioptions" + cmdtesting "k8s.io/kubectl/pkg/cmd/testing" + cmdutil "k8s.io/kubectl/pkg/cmd/util" ) func TestNormalizationFuncGlobalExistence(t *testing.T) { @@ -54,6 +57,104 @@ func TestNormalizationFuncGlobalExistence(t *testing.T) { } } +func TestKubectlSubcommandShadowPlugin(t *testing.T) { + tests := []struct { + name string + args []string + expectPlugin string + expectPluginArgs []string + expectError string + }{ + { + name: "test that a plugin executable is found based on command args when builtin subcommand does not exist", + args: []string{"kubectl", "create", "foo", "--bar", "--bar2", "--namespace", "test-namespace"}, + expectPlugin: "plugin/testdata/kubectl-create-foo", + 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 normal commands are able to be executed, when builtin subcommand exists", + args: []string{"kubectl", "create", "role", "--bar", "--bar2", "--namespace", "test-namespace"}, + expectPlugin: "", + expectPluginArgs: []string{}, + }, + // rest of the tests are copied from TestKubectlCommandHandlesPlugins function, + // 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"}, + expectPlugin: "", + expectPluginArgs: []string{}, + }, + { + name: "test that a plugin executable is found based on command args", + args: []string{"kubectl", "foo", "--bar"}, + expectPlugin: "plugin/testdata/kubectl-foo", + expectPluginArgs: []string{"--bar"}, + }, + { + name: "test that a plugin does not execute over an existing command by the same name", + args: []string{"kubectl", "version"}, + }, + { + 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}, + }, + { + 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}, + }, + } + + for _, test := range tests { + cmdtesting.WithAlphaEnvs([]cmdutil.FeatureGate{cmdutil.CmdPluginAsSubcommand}, t, func(t *testing.T) { + t.Run(test.name, func(t *testing.T) { + pluginsHandler := &testPluginHandler{ + pluginsDirectory: "plugin/testdata", + } + ioStreams, _, _, _ := genericclioptions.NewTestIOStreams() + + root := NewDefaultKubectlCommandWithArgs(KubectlOptions{PluginHandler: pluginsHandler, Arguments: test.args, IOStreams: ioStreams}) + 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.executedPlugin != test.expectPlugin { + t.Fatalf("unexpected plugin execution: expected %q, got %q", test.expectPlugin, 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) + } + }) + }) + } +} + func TestKubectlCommandHandlesPlugins(t *testing.T) { tests := []struct { name string @@ -68,6 +169,12 @@ func TestKubectlCommandHandlesPlugins(t *testing.T) { 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"}, + expectPlugin: "", + expectPluginArgs: []string{}, + }, { name: "test that a plugin executable is found based on command args", args: []string{"kubectl", "foo", "--bar"}, diff --git a/pkg/cmd/plugin/plugin_test.go b/pkg/cmd/plugin/plugin_test.go index 817d24fdd..f7f598eee 100644 --- a/pkg/cmd/plugin/plugin_test.go +++ b/pkg/cmd/plugin/plugin_test.go @@ -182,6 +182,7 @@ func TestPluginPathsAreValid(t *testing.T) { func TestListPlugins(t *testing.T) { pluginPath, _ := filepath.Abs("./testdata") expectPlugins := []string{ + filepath.Join(pluginPath, "kubectl-create-foo"), filepath.Join(pluginPath, "kubectl-foo"), filepath.Join(pluginPath, "kubectl-version"), } diff --git a/pkg/cmd/plugin/testdata/kubectl-create-foo b/pkg/cmd/plugin/testdata/kubectl-create-foo new file mode 100644 index 000000000..a9a7b5e1e --- /dev/null +++ b/pkg/cmd/plugin/testdata/kubectl-create-foo @@ -0,0 +1,3 @@ +#!/bin/bash + +echo "I am plugin create foo" \ No newline at end of file diff --git a/pkg/cmd/util/helpers.go b/pkg/cmd/util/helpers.go index 9df43137b..9083be79e 100644 --- a/pkg/cmd/util/helpers.go +++ b/pkg/cmd/util/helpers.go @@ -425,8 +425,9 @@ func GetPodRunningTimeoutFlag(cmd *cobra.Command) (time.Duration, error) { type FeatureGate string const ( - ApplySet FeatureGate = "KUBECTL_APPLYSET" - ExplainOpenapiV3 FeatureGate = "KUBECTL_EXPLAIN_OPENAPIV3" + ApplySet FeatureGate = "KUBECTL_APPLYSET" + ExplainOpenapiV3 FeatureGate = "KUBECTL_EXPLAIN_OPENAPIV3" + CmdPluginAsSubcommand FeatureGate = "KUBECTL_ENABLE_CMD_SHADOW" ) func (f FeatureGate) IsEnabled() bool {