* allow plugins to extends all comamnd groups, e.g., kn-service-blah …

…is OK

* prevent plugins that shadow commands of exsiting groups, e.g., kn-service-create is NOT OK
* list plugins that extend existing groups
* warn plugins that shadow command of existing groups
* add UTs for above

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"}
+// 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
 		}

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)
+	})
 }

pkg/kn/core/root.go

@@ -80,16 +80,30 @@ 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)
 		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 +189,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 +359,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)

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")