fix false positive kubectl plugin unit tests
- test.args should be passed instead of the os.Args of the test framework to prevent simple invocation of kubectl without args that could manifest in false positive test runs - plugin execution should have a different test path - tests should invoke functioning kubectl commands instead of the mock ones to ensure the correct subcommand is executed without a failure Kubernetes-commit: 8b9cbe62025da49a31518870f2aea0ce9797d3ce
This commit is contained in:
Filip Křepinský
Kubernetes Publisher
parent
4bd3f1b03d
commit
3ebdc043ce
|
|
@ -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"
|
||||
)
|
||||
|
|
@ -79,7 +80,7 @@ func TestKubectlSubcommandShadowPlugin(t *testing.T) {
|
|||
},
|
||||
{
|
||||
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 {
|
||||
if (pluginsHandler.err != nil && pluginsHandler.err.Error() != test.expectError) ||
|
||||
(pluginsHandler.err == nil && len(test.expectError) > 0) {
|
||||
t.Fatalf("unexpected error: expected %q to occur, but got %q", test.expectError, pluginsHandler.err)
|
||||
}
|
||||
|
||||
if pluginsHandler.lookedup && !pluginsHandler.executed && len(test.expectError) == 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)
|
||||
}
|
||||
|
|
@ -166,13 +184,13 @@ func TestKubectlCommandHandlesPlugins(t *testing.T) {
|
|||
}{
|
||||
{
|
||||
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 {
|
||||
if (pluginsHandler.err != nil && pluginsHandler.err.Error() != test.expectError) ||
|
||||
(pluginsHandler.err == nil && len(test.expectError) > 0) {
|
||||
t.Fatalf("unexpected error: expected %q to occur, but got %q", test.expectError, pluginsHandler.err)
|
||||
}
|
||||
|
||||
if pluginsHandler.lookedup && !pluginsHandler.executed && len(test.expectError) == 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,18 +296,21 @@ func TestKubectlCommandHandlesPlugins(t *testing.T) {
|
|||
|
||||
type testPluginHandler struct {
|
||||
pluginsDirectory string
|
||||
validPrefixes []string
|
||||
|
||||
// lookup results
|
||||
lookedup bool
|
||||
err 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 {
|
||||
|
|
@ -291,17 +329,22 @@ func (h *testPluginHandler) Lookup(filename string) (string, bool) {
|
|||
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.err = 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
|
||||
|
|
|
|||
Loading…
Reference in New Issue