diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index af95da5975..10e8ae6867 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -46,6 +46,10 @@ | Add `-a` flag as an alias for `--annotation` | https://github.com/knative/client/pull/782[#782] +| 🎁 +| Allow plugins to extends all command groups +| https://github.com/knative/client/pull/834[#834] + |=== ## v0.13.2 (2020-04-15) diff --git a/pkg/kn/commands/plugin/plugin.go b/pkg/kn/commands/plugin/plugin.go index 58143957cf..acdbe41871 100644 --- a/pkg/kn/commands/plugin/plugin.go +++ b/pkg/kn/commands/plugin/plugin.go @@ -53,16 +53,13 @@ func BindPluginsFlagToViper(cmd *cobra.Command) { viper.SetDefault("lookup-plugins", false) } -// AllowedExtensibleCommandGroups the list of command groups that can be -// extended with plugins, e.g., a plugin named `kn-source-kafka` for Kafka -// event sources is allowed. This is defined as a fixed [...]string since -// cannot defined Golang []string constants -var AllowedExtensibleCommandGroups = [...]string{"source"} +// CoreCommandNames names of all core `kn` commands +var CoreCommandNames = []string{} // InAllowedExtensibleCommandGroups checks that the name is in the list of allowed // extensible command groups func InAllowedExtensibleCommandGroups(name string) bool { - for _, groupName := range AllowedExtensibleCommandGroups { + for _, groupName := range CoreCommandNames { if name == groupName { return true } diff --git a/pkg/kn/commands/plugin/plugin_test.go b/pkg/kn/commands/plugin/plugin_test.go index ac604b3918..513f0c4c1b 100644 --- a/pkg/kn/commands/plugin/plugin_test.go +++ b/pkg/kn/commands/plugin/plugin_test.go @@ -74,11 +74,15 @@ func TestNewPluginCommand(t *testing.T) { } func TestInAllowedExtensibleCommandGroups(t *testing.T) { - isExtensibleCommand := InAllowedExtensibleCommandGroups("fake") - assert.Assert(t, isExtensibleCommand == false) + t.Run("returns true for any string in CoreCommandNames", func(t *testing.T) { + assert.Assert(t, len(CoreCommandNames) > 0) + for _, cmdName := range CoreCommandNames { + assert.Assert(t, InAllowedExtensibleCommandGroups(cmdName) == true) + } + }) - for _, name := range AllowedExtensibleCommandGroups { - isExtensibleCommand = InAllowedExtensibleCommandGroups(name) - assert.Assert(t, isExtensibleCommand == true) - } + t.Run("returns false for random string", func(t *testing.T) { + assert.Assert(t, len(CoreCommandNames) > 0) + assert.Assert(t, InAllowedExtensibleCommandGroups("fake-cmd") == false) + }) } diff --git a/pkg/kn/core/root.go b/pkg/kn/core/root.go index 1693fcef03..eac5461d86 100644 --- a/pkg/kn/core/root.go +++ b/pkg/kn/core/root.go @@ -78,18 +78,36 @@ func NewDefaultKnCommandWithArgs(rootCmd *cobra.Command, cmdPathPieces := args[1:] cmdPathPieces = removeKnPluginFlags(cmdPathPieces) // Plugin does not need these flags + // Return fast if -h or --help is in path pieces + if helpOptionsPresent(cmdPathPieces) { + return rootCmd + } + // only look for suitable extension executables if // the specified command does not already exist foundCmd, innerArgs, err := rootCmd.Find(cmdPathPieces) - if err != nil || plugin.InAllowedExtensibleCommandGroups(foundCmd.Name()) { + if err != nil { err := plugin.HandlePluginCommand(pluginHandler, cmdPathPieces) if err != nil { fmt.Fprintf(rootCmd.OutOrStderr(), "Error: unknown command '%s' \nRun 'kn --help' for usage.\n", args[1]) os.Exit(1) } - } else if foundCmd.HasSubCommands() { - if _, _, err := rootCmd.Find(innerArgs); err != nil { - fmt.Fprintf(rootCmd.OutOrStderr(), showSubcommands(foundCmd, cmdPathPieces, innerArgs[0])) + } + + // look for case of plugins with command that shadows existing command + if foundCmd.HasSubCommands() && len(innerArgs) > 0 { + cmdName := innerArgs[0] + for _, subcommand := range foundCmd.Commands() { + if subcommand.Name() == cmdName { + fmt.Fprintf(rootCmd.OutOrStderr(), fmt.Sprintf("Error: subcommand '%s' for '%s' already exists.\nRun 'kn --help' for usage.\n", cmdName, foundCmd.Name())) + os.Exit(1) + } + } + + // try to handle a plugin for a command extending a core comand group + err = plugin.HandlePluginCommand(pluginHandler, cmdPathPieces) + if err != nil { + fmt.Fprintf(rootCmd.OutOrStderr(), "Error: unknown command '%s' \nRun 'kn --help' for usage.\n", args[1]) os.Exit(1) } } @@ -175,6 +193,11 @@ func NewKnCommand(params ...commands.KnParams) *cobra.Command { // For glog parse error. flag.CommandLine.Parse([]string{}) + // Set all current core commands to plugin.CoreCommandNames + for _, cmd := range rootCmd.Commands() { + plugin.CoreCommandNames = append(plugin.CoreCommandNames, cmd.Name()) + } + return rootCmd } @@ -340,7 +363,9 @@ func removeKnPluginFlags(args []string) []string { if arg == "--plugins-dir" || strings.HasPrefix(arg, "--plugins-dir=") || arg == "--lookup-plugins" || - strings.HasPrefix(arg, "--lookup-plugins=") { + strings.HasPrefix(arg, "--lookup-plugins=") || + // remove -test.* args which are added when running go test + strings.HasPrefix(arg, "-test.") { continue } else { remainingArgs = append(remainingArgs, arg) @@ -373,3 +398,12 @@ func showSubcommands(cmd *cobra.Command, args []string, innerArg string) string } return fmt.Sprintf("Error: unknown subcommand '%s' for '%s'. Available subcommands: %s\nRun 'kn --help' for usage.\n", innerArg, getCommands(args, innerArg), strings.Join(strs, ", ")) } + +func helpOptionsPresent(args []string) bool { + for _, arg := range args { + if arg == "-h" || arg == "--help" { + return true + } + } + return false +} diff --git a/pkg/kn/core/root_test.go b/pkg/kn/core/root_test.go index 48c3ead501..2fefc394fe 100644 --- a/pkg/kn/core/root_test.go +++ b/pkg/kn/core/root_test.go @@ -76,11 +76,12 @@ func TestNewDefaultKnCommandWithArgs(t *testing.T) { err error ) + pluginName = "fake-plugin-name" + beforeEach := func(t *testing.T) { tmpPathDir, err = ioutil.TempDir("", "plugin_list") assert.Assert(t, err == nil) - pluginName = "fake-plugin-name" pluginPath = plugin.CreateTestPluginInPath(t, "kn-"+pluginName, plugin.KnTestPluginScript, plugin.FileModeExecutable, tmpPathDir) } @@ -89,12 +90,56 @@ func TestNewDefaultKnCommandWithArgs(t *testing.T) { assert.Assert(t, err == nil) } - beforeEach(t) - args = []string{pluginPath, pluginName} - setup(t) - defer afterEach(t) + t.Run("when -h or --help option is present for plugin, return valid root command", func(t *testing.T) { + helpOptions := []string{"-h", "--help"} + for _, helpOption := range helpOptions { + beforeEach(t) + args = []string{pluginPath, pluginName, helpOption} + setup(t) + defer afterEach(t) + + checkRootCmd(t, rootCmd) + } + }) + + t.Run("when --help option is present for normal command, return valid root command", func(t *testing.T) { + helpOptions := []string{"-h", "--help"} + for _, helpOption := range helpOptions { + beforeEach(t) + args = []string{"service", helpOption} + setup(t) + defer afterEach(t) + + checkRootCmd(t, rootCmd) + } + }) t.Run("tries to handle args[1:] as plugin and return valid root command", func(t *testing.T) { + beforeEach(t) + args = []string{pluginPath, pluginName} + setup(t) + defer afterEach(t) + + checkRootCmd(t, rootCmd) + }) + + t.Run("when plugin extends an existing command group it return valid root command", func(t *testing.T) { + pluginName = "service-fakecmd" + beforeEach(t) + args = []string{pluginPath, pluginName} + setup(t) + defer afterEach(t) + + checkRootCmd(t, rootCmd) + }) + + t.Run("when plugin extends and shadows an existing command group it fails", func(t *testing.T) { + pluginName = "service-create" + beforeEach(t) + args = []string{pluginPath, pluginName, "test"} + setup(t) + defer afterEach(t) + checkRootCmd(t, rootCmd) }) }) diff --git a/test/e2e/plugins_test.go b/test/e2e/plugins_test.go index d893a3e04c..ddead3150f 100644 --- a/test/e2e/plugins_test.go +++ b/test/e2e/plugins_test.go @@ -84,12 +84,6 @@ func (pc *pluginTestConfig) teardown() { os.RemoveAll(pc.knConfigDir) } -func createPluginFile(fileName, fileContent, filePath string, fileMode os.FileMode) (string, error) { - file := filepath.Join(filePath, fileName) - err := ioutil.WriteFile(file, []byte(fileContent), fileMode) - return file, err -} - func TestPluginWithoutLookup(t *testing.T) { t.Parallel() @@ -112,8 +106,6 @@ func TestPluginWithoutLookup(t *testing.T) { t.Log("does not list any other plugin in $PATH") listPlugin(r, knFlags, []string{pc.knPluginPath}, []string{pc.knPluginPath2}) - - t.Log("with --lookup-plugins is true") } func TestPluginWithLookup(t *testing.T) { @@ -167,6 +159,14 @@ func TestExecutePluginInPath(t *testing.T) { runPlugin(r, knFlags, "hello2e2e", []string{}, []string{"Hello Knative, I'm a Kn plugin"}) } +// Private + +func createPluginFile(fileName, fileContent, filePath string, fileMode os.FileMode) (string, error) { + file := filepath.Join(filePath, fileName) + err := ioutil.WriteFile(file, []byte(fileContent), fileMode) + return file, err +} + func setupPluginTestConfigWithNewPath(t *testing.T) (pluginTestConfig, string) { pc := pluginTestConfig{} assert.NilError(t, pc.setup()) @@ -180,8 +180,6 @@ func tearDownWithPath(pc pluginTestConfig, oldPath string) { pc.teardown() } -// Private - func listPlugin(r *test.KnRunResultCollector, knFlags []string, expectedPlugins []string, unexpectedPlugins []string) { knArgs := append(knFlags, "plugin", "list")