From 7b488dfea2bd52ee9fb5f8498b456d2548449a48 Mon Sep 17 00:00:00 2001 From: "dr.max" Date: Tue, 21 Apr 2020 12:01:38 -0700 Subject: [PATCH 1/3] fixes(#814) allow plugins to extend the 'source' command group * introduce a list of command group that can be extended (currently only source) * check if plugin main group is in that list and execute, otherwise fail * add e2e test for both plugin that is allowed and not allowed to extend existing group --- CHANGELOG.adoc | 4 ++++ pkg/kn/commands/plugin/verifier_test.go | 14 ++++++++++++++ pkg/kn/core/root.go | 17 ++++++++++++++++- 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 626667ab8f..c372da1b93 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -26,6 +26,10 @@ | Check DeleteTimestamp before updating resource | https://github.com/knative/client/pull/805[#805] +| 🎁 +| Allow plugins to extend fixed list of command groups, currently `source` +| https://github.com/knative/client/pull/818[#818] + |=== ## v0.13.2 (2020-04-15) diff --git a/pkg/kn/commands/plugin/verifier_test.go b/pkg/kn/commands/plugin/verifier_test.go index 1af5488e65..4a77c47115 100644 --- a/pkg/kn/commands/plugin/verifier_test.go +++ b/pkg/kn/commands/plugin/verifier_test.go @@ -143,6 +143,20 @@ func TestPluginVerifier(t *testing.T) { assert.Assert(t, util.ContainsAll(eaw.errors[0], "overwrite", "kn-plugin")) }) }) + + t.Run("when kn plugin in path overwrites 'source' command (which is allowed)", func(t *testing.T) { + setup(t) + defer cleanup(t) + var overwritingPluginPath = CreateTestPlugin(t, "kn-source-test", KnTestPluginScript, FileModeExecutable) + defer DeleteTestPlugin(t, overwritingPluginPath) + + t.Run("runs successfully", func(t *testing.T) { + eaw := errorsAndWarnings{} + eaw = verifier.verify(eaw, overwritingPluginPath) + assert.Assert(t, len(eaw.errors) == 0) + assert.Assert(t, len(eaw.warnings) == 0) + }) + }) }) } diff --git a/pkg/kn/core/root.go b/pkg/kn/core/root.go index b6877a8fbf..1e63a9e590 100644 --- a/pkg/kn/core/root.go +++ b/pkg/kn/core/root.go @@ -43,6 +43,12 @@ import ( "knative.dev/client/pkg/kn/flags" ) +// 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"} + // NewDefaultKnCommand creates the default `kn` command with a default plugin handler func NewDefaultKnCommand() *cobra.Command { rootCmd := NewKnCommand() @@ -81,7 +87,7 @@ func NewDefaultKnCommandWithArgs(rootCmd *cobra.Command, // only look for suitable extension executables if // the specified command does not already exist foundCmd, innerArgs, err := rootCmd.Find(cmdPathPieces) - if err != nil { + if err != nil || inAllowedExtensibleCommandGroups(foundCmd.Name()) { err := plugin.HandlePluginCommand(pluginHandler, cmdPathPieces) if err != nil { fmt.Fprintf(rootCmd.OutOrStderr(), "Error: unknown command '%s' \nRun 'kn --help' for usage.\n", args[1]) @@ -373,3 +379,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 inAllowedExtensibleCommandGroups(name string) bool { + for _, groupName := range AllowedExtensibleCommandGroups { + if name == groupName { + return true + } + } + return false +} From 6ed686f349acd033134a4dc8acb3fa35efb0c536 Mon Sep 17 00:00:00 2001 From: "dr.max" Date: Tue, 28 Apr 2020 15:56:32 -0700 Subject: [PATCH 2/3] * move IsAllowedExtensibleCommandGroup to plugin/verifier.go * added tests for the public function * change verifier to use IsAllowedExtensibleCommandGroup to allow plugins for extensible command groups --- pkg/kn/commands/plugin/list_test.go | 20 ++++++++++++++++++-- pkg/kn/commands/plugin/verifier.go | 21 ++++++++++++++++++++- pkg/kn/commands/plugin/verifier_test.go | 11 ++++++++++- pkg/kn/core/root.go | 17 +---------------- 4 files changed, 49 insertions(+), 20 deletions(-) diff --git a/pkg/kn/commands/plugin/list_test.go b/pkg/kn/commands/plugin/list_test.go index e33d8d0211..230cd0a159 100644 --- a/pkg/kn/commands/plugin/list_test.go +++ b/pkg/kn/commands/plugin/list_test.go @@ -71,7 +71,6 @@ func (ctx *testContext) createTestPluginWithPath(pluginName string, fileMode os. } func TestPluginList(t *testing.T) { - setup := func(t *testing.T) *testContext { knParams := &commands.KnParams{} pluginCmd := NewPluginCommand(knParams) @@ -159,7 +158,6 @@ func TestPluginList(t *testing.T) { }) t.Run("with plugins with same name", func(t *testing.T) { - t.Run("warns user about second (in $PATH) plugin shadowing first", func(t *testing.T) { ctx := setup(t) defer ctx.cleanup() @@ -201,6 +199,24 @@ func TestPluginList(t *testing.T) { assert.ErrorContains(t, err, "overwrite", "built-in") assert.Assert(t, util.ContainsAll(ctx.output(), "ERROR", "overwrite", "built-in")) }) + + t.Run("allows plugin under the `source` command group", func(t *testing.T) { + ctx := setup(t) + defer ctx.cleanup() + + sourceCmd := &cobra.Command{ + Use: "source", + } + ctx.rootCmd.AddCommand(sourceCmd) + defer ctx.rootCmd.RemoveCommand(sourceCmd) + + err := ctx.createTestPlugin("kn-source-fake", FileModeExecutable, true) + assert.NilError(t, err) + + err = ctx.execute("plugin", "list", "--lookup-plugins=true") + assert.NilError(t, err) + }) + }) }) }) diff --git a/pkg/kn/commands/plugin/verifier.go b/pkg/kn/commands/plugin/verifier.go index 5d40b662c2..349a0ac5eb 100644 --- a/pkg/kn/commands/plugin/verifier.go +++ b/pkg/kn/commands/plugin/verifier.go @@ -33,6 +33,23 @@ import ( "github.com/spf13/cobra" ) +// 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"} + +// InAllowedExtensibleCommandGroups checks that the name is in the list of allowed +// extensible command groups +func InAllowedExtensibleCommandGroups(name string) bool { + for _, groupName := range AllowedExtensibleCommandGroups { + if name == groupName { + return true + } + } + return false +} + // pluginVerifier verifies that existing kn commands are not overridden type pluginVerifier struct { root *cobra.Command @@ -110,7 +127,9 @@ func (v *pluginVerifier) addErrorIfOverwritingExistingCommand(eaw errorsAndWarni for _, c := range [][]string{cmds, convertUnderscoresToDashes(cmds)} { cmd, _, err := v.root.Find(c) if err == nil { - overwrittenCommands[cmd.CommandPath()] = true + if !InAllowedExtensibleCommandGroups(cmd.Name()) { + overwrittenCommands[cmd.CommandPath()] = true + } } } for command := range overwrittenCommands { diff --git a/pkg/kn/commands/plugin/verifier_test.go b/pkg/kn/commands/plugin/verifier_test.go index 4a77c47115..0bf1875f6e 100644 --- a/pkg/kn/commands/plugin/verifier_test.go +++ b/pkg/kn/commands/plugin/verifier_test.go @@ -32,8 +32,17 @@ import ( "gotest.tools/assert" ) -func TestPluginVerifier(t *testing.T) { +func TestInAllowedExtensibleCommandGroups(t *testing.T) { + isExtensibleCommand := InAllowedExtensibleCommandGroups("fake") + assert.Assert(t, isExtensibleCommand == false) + for _, name := range AllowedExtensibleCommandGroups { + isExtensibleCommand = InAllowedExtensibleCommandGroups(name) + assert.Assert(t, isExtensibleCommand == true) + } +} + +func TestPluginVerifier(t *testing.T) { var ( pluginPath string rootCmd *cobra.Command diff --git a/pkg/kn/core/root.go b/pkg/kn/core/root.go index 1e63a9e590..1693fcef03 100644 --- a/pkg/kn/core/root.go +++ b/pkg/kn/core/root.go @@ -43,12 +43,6 @@ import ( "knative.dev/client/pkg/kn/flags" ) -// 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"} - // NewDefaultKnCommand creates the default `kn` command with a default plugin handler func NewDefaultKnCommand() *cobra.Command { rootCmd := NewKnCommand() @@ -87,7 +81,7 @@ func NewDefaultKnCommandWithArgs(rootCmd *cobra.Command, // only look for suitable extension executables if // the specified command does not already exist foundCmd, innerArgs, err := rootCmd.Find(cmdPathPieces) - if err != nil || inAllowedExtensibleCommandGroups(foundCmd.Name()) { + if err != nil || plugin.InAllowedExtensibleCommandGroups(foundCmd.Name()) { err := plugin.HandlePluginCommand(pluginHandler, cmdPathPieces) if err != nil { fmt.Fprintf(rootCmd.OutOrStderr(), "Error: unknown command '%s' \nRun 'kn --help' for usage.\n", args[1]) @@ -379,12 +373,3 @@ 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 inAllowedExtensibleCommandGroups(name string) bool { - for _, groupName := range AllowedExtensibleCommandGroups { - if name == groupName { - return true - } - } - return false -} From 3b0fe249a8e782b6141dc8e5c0dea9286fee03d0 Mon Sep 17 00:00:00 2001 From: "dr.max" Date: Wed, 29 Apr 2020 10:05:01 -0700 Subject: [PATCH 3/3] moved `InAllowedExtensibleCommandGroups` to plugin.go so that it is usable in windows builds --- pkg/kn/commands/plugin/plugin.go | 17 +++++++++++++++++ pkg/kn/commands/plugin/plugin_test.go | 10 ++++++++++ pkg/kn/commands/plugin/verifier.go | 17 ----------------- pkg/kn/commands/plugin/verifier_test.go | 10 ---------- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/pkg/kn/commands/plugin/plugin.go b/pkg/kn/commands/plugin/plugin.go index 211b3d78f6..58143957cf 100644 --- a/pkg/kn/commands/plugin/plugin.go +++ b/pkg/kn/commands/plugin/plugin.go @@ -52,3 +52,20 @@ func BindPluginsFlagToViper(cmd *cobra.Command) { viper.SetDefault("plugins-dir", commands.Cfg.DefaultPluginDir) 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"} + +// InAllowedExtensibleCommandGroups checks that the name is in the list of allowed +// extensible command groups +func InAllowedExtensibleCommandGroups(name string) bool { + for _, groupName := range AllowedExtensibleCommandGroups { + if name == groupName { + return true + } + } + return false +} diff --git a/pkg/kn/commands/plugin/plugin_test.go b/pkg/kn/commands/plugin/plugin_test.go index fb34cfb59b..ac604b3918 100644 --- a/pkg/kn/commands/plugin/plugin_test.go +++ b/pkg/kn/commands/plugin/plugin_test.go @@ -72,3 +72,13 @@ func TestNewPluginCommand(t *testing.T) { assert.Assert(t, pluginCmd.Args == nil) }) } + +func TestInAllowedExtensibleCommandGroups(t *testing.T) { + isExtensibleCommand := InAllowedExtensibleCommandGroups("fake") + assert.Assert(t, isExtensibleCommand == false) + + for _, name := range AllowedExtensibleCommandGroups { + isExtensibleCommand = InAllowedExtensibleCommandGroups(name) + assert.Assert(t, isExtensibleCommand == true) + } +} diff --git a/pkg/kn/commands/plugin/verifier.go b/pkg/kn/commands/plugin/verifier.go index 349a0ac5eb..8ed479f997 100644 --- a/pkg/kn/commands/plugin/verifier.go +++ b/pkg/kn/commands/plugin/verifier.go @@ -33,23 +33,6 @@ import ( "github.com/spf13/cobra" ) -// 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"} - -// InAllowedExtensibleCommandGroups checks that the name is in the list of allowed -// extensible command groups -func InAllowedExtensibleCommandGroups(name string) bool { - for _, groupName := range AllowedExtensibleCommandGroups { - if name == groupName { - return true - } - } - return false -} - // pluginVerifier verifies that existing kn commands are not overridden type pluginVerifier struct { root *cobra.Command diff --git a/pkg/kn/commands/plugin/verifier_test.go b/pkg/kn/commands/plugin/verifier_test.go index 0bf1875f6e..d401169c9d 100644 --- a/pkg/kn/commands/plugin/verifier_test.go +++ b/pkg/kn/commands/plugin/verifier_test.go @@ -32,16 +32,6 @@ import ( "gotest.tools/assert" ) -func TestInAllowedExtensibleCommandGroups(t *testing.T) { - isExtensibleCommand := InAllowedExtensibleCommandGroups("fake") - assert.Assert(t, isExtensibleCommand == false) - - for _, name := range AllowedExtensibleCommandGroups { - isExtensibleCommand = InAllowedExtensibleCommandGroups(name) - assert.Assert(t, isExtensibleCommand == true) - } -} - func TestPluginVerifier(t *testing.T) { var ( pluginPath string