KEP-3638: Promote plugin resolution to beta (#120663)

* Promote plugin resolution to beta

* Not use plugin for kubectl create -f command execution

`kubectl create -f` is legitimate command execution and we shouldn't
search plugins if user invokes this.

* Add integration test for plugin resolution for create command

* Reintroduce feature flag to ability to disable it explicitly

staging/src/k8s.io/kubectl/pkg/cmd/cmd.go

@@ -142,17 +142,12 @@ func NewDefaultKubectlCommandWithArgs(o KubectlOptions) *cobra.Command {
 				}
 			}
 		} else if err == nil {
-			if cmdutil.CmdPluginAsSubcommand.IsEnabled() {
+			if !cmdutil.CmdPluginAsSubcommand.IsDisabled() {
 				// Command exists(e.g. kubectl create), but it is not certain that
 				// subcommand also exists (e.g. kubectl create networkpolicy)
-				if IsSubcommandPluginAllowed(foundCmd.Name()) {
+				// we also have to eliminate kubectl create -f
-					var subcommand string
+				if IsSubcommandPluginAllowed(foundCmd.Name()) && len(foundArgs) >= 1 && !strings.HasPrefix(foundArgs[0], "-") {
-					for _, arg := range foundArgs { // first "non-flag" argument as subcommand
+					subcommand := foundArgs[0]
-						if !strings.HasPrefix(arg, "-") {
-							subcommand = arg
-							break
-						}
-					}
 					builtinSubcmdExist := false
 					for _, subcmd := range foundCmd.Commands() {
 						if subcmd.Name() == subcommand {

staging/src/k8s.io/kubectl/pkg/cmd/cmd_test.go

@@ -29,8 +29,6 @@ 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"
 )
 
 func TestNormalizationFuncGlobalExistence(t *testing.T) {
@@ -129,47 +127,45 @@ func TestKubectlSubcommandShadowPlugin(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		cmdtesting.WithAlphaEnvs([]cmdutil.FeatureGate{cmdutil.CmdPluginAsSubcommand}, t, func(t *testing.T) {
+		t.Run(test.name, func(t *testing.T) {
-			t.Run(test.name, func(t *testing.T) {
+			pluginsHandler := &testPluginHandler{
-				pluginsHandler := &testPluginHandler{
+				pluginsDirectory: "plugin/testdata",
-					pluginsDirectory: "plugin/testdata",
+				validPrefixes:    plugin.ValidPluginFilenamePrefixes,
-					validPrefixes:    plugin.ValidPluginFilenamePrefixes,
+			}
-				}
+			ioStreams, _, _, _ := genericiooptions.NewTestIOStreams()
-				ioStreams, _, _, _ := genericiooptions.NewTestIOStreams()
+			root := NewDefaultKubectlCommandWithArgs(KubectlOptions{PluginHandler: pluginsHandler, Arguments: test.args, IOStreams: ioStreams})
-				root := NewDefaultKubectlCommandWithArgs(KubectlOptions{PluginHandler: pluginsHandler, Arguments: test.args, IOStreams: ioStreams})
+			// 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
-				// 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 {
-				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
-					// 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
-					// to the command which might invoke only "kubectl" without any additional args and give false positives
+				root.SetArgs(test.args[1:])
-					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
-					// 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 {
-					if err := root.Execute(); err != nil {
+					t.Fatalf("unexpected error: %v", err)
-						t.Fatalf("unexpected error: %v", err)
-					}
 				}
+			}
 
-				if (pluginsHandler.lookupErr != nil && pluginsHandler.lookupErr.Error() != test.expectLookupError) ||
+			if (pluginsHandler.lookupErr != nil && pluginsHandler.lookupErr.Error() != test.expectLookupError) ||
-					(pluginsHandler.lookupErr == nil && len(test.expectLookupError) > 0) {
+				(pluginsHandler.lookupErr == nil && len(test.expectLookupError) > 0) {
-					t.Fatalf("unexpected error: expected %q to occur, but got %q", test.expectLookupError, pluginsHandler.lookupErr)
+				t.Fatalf("unexpected error: expected %q to occur, but got %q", test.expectLookupError, pluginsHandler.lookupErr)
-				}
+			}
 
-				if pluginsHandler.lookedup && !pluginsHandler.executed && len(test.expectLookupError) == 0 {
+			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)
+				// 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")
+				t.Fatalf("expected plugin execution, but did not occur")
-				}
+			}
 
-				if pluginsHandler.executedPlugin != test.expectPlugin {
+			if pluginsHandler.executedPlugin != test.expectPlugin {
-					t.Fatalf("unexpected plugin execution: expected %q, got %q", test.expectPlugin, pluginsHandler.executedPlugin)
+				t.Fatalf("unexpected plugin execution: expected %q, got %q", test.expectPlugin, pluginsHandler.executedPlugin)
-				}
+			}
 
-				if pluginsHandler.executed && len(test.expectPlugin) == 0 {
+			if pluginsHandler.executed && len(test.expectPlugin) == 0 {
-					t.Fatalf("unexpected plugin execution: expected no plugin, got %q", pluginsHandler.executedPlugin)
+				t.Fatalf("unexpected plugin execution: expected no plugin, got %q", pluginsHandler.executedPlugin)
-				}
+			}
 
-				if !cmp.Equal(pluginsHandler.withArgs, test.expectPluginArgs, cmpopts.EquateEmpty()) {
+			if !cmp.Equal(pluginsHandler.withArgs, test.expectPluginArgs, cmpopts.EquateEmpty()) {
-					t.Fatalf("unexpected plugin execution args: expected %q, got %q", test.expectPluginArgs, pluginsHandler.withArgs)
+				t.Fatalf("unexpected plugin execution args: expected %q, got %q", test.expectPluginArgs, pluginsHandler.withArgs)
-				}
+			}
-			})
 		})
 	}
 }

staging/src/k8s.io/kubectl/pkg/cmd/util/helpers.go

@@ -429,10 +429,20 @@ const (
 	CmdPluginAsSubcommand FeatureGate = "KUBECTL_ENABLE_CMD_SHADOW"
 )
 
+// IsEnabled returns true iff environment variable is set to true.
+// All other cases, it returns false.
 func (f FeatureGate) IsEnabled() bool {
 	return os.Getenv(string(f)) == "true"
 }
 
+// IsDisabled returns true iff environment variable is set to false.
+// All other cases, it returns true.
+// This function is used for the cases where feature is enabled by default,
+// but it may be needed to provide a way to ability to disable this feature.
+func (f FeatureGate) IsDisabled() bool {
+	return os.Getenv(string(f)) == "false"
+}
+
 func AddValidateFlags(cmd *cobra.Command) {
 	cmd.Flags().String(
 		"validate",

test/cmd/plugins.sh

@@ -57,6 +57,14 @@ run_plugins_tests() {
   kube::test::if_has_string "${output_message}" 'Client Version'
   kube::test::if_has_not_string "${output_message}" 'overshadows an existing plugin'
 
+  # attempt to run a plugin as a subcommand of kubectl create in the user's PATH
+  output_message=$(PATH=${TEMP_PATH}:"test/fixtures/pkg/kubectl/plugins/create" kubectl create foo)
+  kube::test::if_has_string "${output_message}" 'plugin foo as a subcommand of kubectl create command'
+
+  # ensure that a kubectl create cronjob builtin command supersedes a plugin that overshadows it
+  output_message=$(PATH=${TEMP_PATH}:"test/fixtures/pkg/kubectl/plugins/create" kubectl create cronjob --help)
+  kube::test::if_has_not_string "${output_message}" 'plugin cronjob as a subcommand of kubectl create command'
+
   rm -fr "${TEMP_PATH}"
 
   set +o nounset

test/fixtures/pkg/kubectl/plugins/create/kubectl-create-cronjob

@@ -0,0 +1,3 @@
+#!/bin/bash
+
+echo "I am plugin cronjob as a subcommand of kubectl create command"

test/fixtures/pkg/kubectl/plugins/create/kubectl-create-foo

@@ -0,0 +1,3 @@
+#!/bin/bash
+
+echo "I am plugin foo as a subcommand of kubectl create command"