Repository: knative/client
Pull request #991 (2020-08-25 13:40:12)

Plugin inlining (backport)
This is a backport of #902 

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/client/pull/991#" title="Author self-approved">rhuss</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fclient).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative/client/blob/release-0.16/OWNERS)~~ [rhuss]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/client/pull/991#pullrequestreview-474533909" title="Approved">navidshaikh</a>*, *<a href="https://github.com/knative/client/pull/991#" title="Author self-approved">rhuss</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fclient).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative/client/blob/release-0.16/OWNERS)~~ [navidshaikh,rhuss]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/client/pull/991#pullrequestreview-474568800" title="LGTM">navidshaikh</a>*, *<a href="https://github.com/knative/client/pull/991#" title="Author self-approved">rhuss</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fclient).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative/client/blob/release-0.16/OWNERS)~~ [navidshaikh,rhuss]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
index d2886ec4..f7db3a09 100644
--- a/CHANGELOG.adoc
+++ b/CHANGELOG.adoc
@@ -12,7 +12,7 @@
 | https://github.com/knative/client/pull/[#]
 ////
 
-## v0.16.1 (2020-08-18)
+## v0.16.1 (2020-08-25)
 
 [cols="1,10,3", options="header", width="100%"]
 |===
@@ -41,6 +41,10 @@
 | 🐛
 | fix(tekton e2e): Refer tasks from new tekton catalog task structure
 | https://github.com/knative/client/pull/966[#966]
+
+| 🎁
+| Add support for internal plugins
+| https://github.com/knative/client/pull/902[#902]
 |===
 
 ## v0.16.0 (2020-07-14)
diff --git a/docs/plugins/README.md b/docs/plugins/README.md
index 4f0d1ab4..ccdcfd0d 100644
--- a/docs/plugins/README.md
+++ b/docs/plugins/README.md
@@ -21,3 +21,39 @@ Please refer to the documentation and examples for more information on how to
 write your own plugins.
 
 - [kn plugin](../cmd/kn_plugin.md) - Plugin command group
+
+
+## Plugin Inlining
+
+It is possible to inline plugins that are written in golang.
+The following steps are required:
+
+* In your plugin project, create a implementation of the `Plugin` interface and add it to the global `plugin.InternalPlugins` slice in your `init()` method, like in this example:
+
+```go
+package plugin
+
+import (
+	"knative.dev/client/pkg/kn/plugin"
+)
+
+func init() {
+	plugin.InternalPlugins = append(plugin.InternalPlugins, &myPlugin{})
+}
+```
+
+* In your fork of the kn client, add a file `plugin_register.go` to the root package directory which imports your plugin's implementation package:
+
+```go
+package root
+
+import (
+        _ "github.com/rhuss/myplugin/plugin"
+)
+
+// RegisterInlinePlugins is an empty function which however forces the
+// compiler to run all init() methods of the registered imports
+func RegisterInlinePlugins() {}
+```
+
+* Update you `go.mod` file with the new dependency and build your custom distribution of `kn`
diff --git a/go.sum b/go.sum
index 9738f1e3..78cbab8f 100644
--- a/go.sum
+++ b/go.sum
@@ -264,6 +264,7 @@ github.com/evanphx/json-patch v4.5.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLi
 github.com/exponent-io/jsonpath v0.0.0-20151013193312-d6023ce2651d/go.mod h1:ZZMPRZwes7CROmyNKgQzC3XPs6L/G2EJLHddWejkmf4=
 github.com/fatih/camelcase v1.0.0/go.mod h1:yN2Sb0lFhZJUdVvtELVWefmrXpuZESvPmqwoZc+/fpc=
 github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
+github.com/fatih/color v1.9.0 h1:8xPHl4/q1VyqGIPif1F+1V3Y3lSmrq01EabUW3CoW5s=
 github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU=
 github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568/go.mod h1:xEzjJPgXI435gkrCt3MPfRiAkVrwSbHsst4LCFVfpJc=
 github.com/fortytw2/leaktest v1.2.0/go.mod h1:jDsjWgpAGjm2CA7WthBh/CdZYEPF31XHquHwclZch5g=
@@ -567,10 +568,12 @@ github.com/marstr/guid v1.1.0/go.mod h1:74gB1z2wpxxInTG6yaqA7KrtM0NZ+RbrcqDvYHef
 github.com/mattbaird/jsonpatch v0.0.0-20171005235357-81af80346b1a/go.mod h1:M1qoD/MqPgTZIk0EWKB38wE28ACRfVcn+cU08jyArI0=
 github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
 github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=
+github.com/mattn/go-colorable v0.1.4 h1:snbPLB8fVfU9iwbbo30TPtbLRzwWu6aJS6Xh4eaaviA=
 github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=
 github.com/mattn/go-ieproxy v0.0.0-20190610004146-91bb50d98149/go.mod h1:31jz6HNzdxOmlERGGEc4v/dMssOfmp2p5bT/okiKFFc=
 github.com/mattn/go-isatty v0.0.4/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4=
 github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s=
+github.com/mattn/go-isatty v0.0.11 h1:FxPOTFNqGkuDUGi3H/qkUbQO4ZiBa2brKq5r0l8TGeM=
 github.com/mattn/go-isatty v0.0.11/go.mod h1:PhnuNfih5lzO57/f3n+odYbM4JtupLOxQOAqxQCu2WE=
 github.com/mattn/go-runewidth v0.0.2/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU=
 github.com/mattn/go-runewidth v0.0.8/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI=
diff --git a/pkg/kn/config/config.go b/pkg/kn/config/config.go
index 48088e25..22e1fe39 100644
--- a/pkg/kn/config/config.go
+++ b/pkg/kn/config/config.go
@@ -129,7 +129,7 @@ func BootstrapConfig() error {
 	viper.SetConfigFile(GlobalConfig.ConfigFile())
 	viper.AutomaticEnv() // read in environment variables that match
 
-	// Defaults are taken from the parsed flags, which in turn have bootstrapDefaults
+	// Defaults are taken from the parsed flags, which in turn have bootstrap defaults
 	// TODO: Re-enable when legacy handling for plugin config has been removed
 	// For now default handling is happening directly in the getter of GlobalConfig
 	// viper.SetDefault(keyPluginsDirectory, bootstrapDefaults.pluginsDir)
diff --git a/pkg/kn/plugin/manager.go b/pkg/kn/plugin/manager.go
index a136c33f..a4f09cf4 100644
--- a/pkg/kn/plugin/manager.go
+++ b/pkg/kn/plugin/manager.go
@@ -30,6 +30,9 @@ import (
 	"github.com/spf13/cobra"
 )
 
+// Allow plugins to register to this slice for inlining
+var InternalPlugins PluginList
+
 // Interface describing a plugin
 type Plugin interface {
 	// Get the name of the plugin (the file name without extensions)
@@ -99,13 +102,19 @@ func (manager *Manager) FindPlugin(parts []string) (Plugin, error) {
 		return nil, nil
 	}
 
+	// Try to find internal plugin fist
+	plugin := lookupInternalPlugin(parts)
+	if plugin != nil {
+		return plugin, nil
+	}
+
 	// Try to find plugin in pluginsDir
 	pluginDir, err := homedir.Expand(manager.pluginsDir)
 	if err != nil {
 		return nil, err
 	}
 
-	return findMostSpecificPlugin(pluginDir, parts, manager.lookupInPath)
+	return findMostSpecificPluginInPath(pluginDir, parts, manager.lookupInPath)
 }
 
 // ListPlugins lists all plugins that can be found in the plugin directory or in the path (if configured)
@@ -116,7 +125,9 @@ func (manager *Manager) ListPlugins() (PluginList, error) {
 // ListPluginsForCommandGroup lists all plugins that can be found in the plugin directory or in the path (if configured),
 // and which fits to a command group
 func (manager *Manager) ListPluginsForCommandGroup(commandGroupParts []string) (PluginList, error) {
-	var plugins []Plugin
+
+	// Initialize with list of internal plugins
+	var plugins = append([]Plugin{}, filterPluginsByCommandGroup(InternalPlugins, commandGroupParts)...)
 
 	dirs, err := manager.pluginLookupDirectories()
 	if err != nil {
@@ -125,6 +136,9 @@ func (manager *Manager) ListPluginsForCommandGroup(commandGroupParts []string) (
 
 	// Examine all files in possible plugin directories
 	hasSeen := make(map[string]bool)
+	for _, pl := range plugins {
+		hasSeen[pl.Name()] = true
+	}
 	for _, dir := range dirs {
 		files, err := ioutil.ReadDir(dir)
 
@@ -144,12 +158,12 @@ func (manager *Manager) ListPluginsForCommandGroup(commandGroupParts []string) (
 			}
 
 			// Check if plugin matches a command group
-			if !isPartOfCommandGroup(commandGroupParts, f.Name()) {
+			if !isPluginFileNamePartOfCommandGroup(commandGroupParts, f.Name()) {
 				continue
 			}
 
 			// Ignore all plugins that are shadowed
-			if _, ok := hasSeen[name]; !ok {
+			if seen, ok := hasSeen[name]; !ok || !seen {
 				plugins = append(plugins, &plugin{
 					path:         filepath.Join(dir, f.Name()),
 					name:         stripWindowsExecExtensions(f.Name()),
@@ -165,18 +179,34 @@ func (manager *Manager) ListPluginsForCommandGroup(commandGroupParts []string) (
 	return plugins, nil
 }
 
-func isPartOfCommandGroup(commandGroupParts []string, name string) bool {
+func filterPluginsByCommandGroup(plugins PluginList, commandGroupParts []string) PluginList {
+	ret := PluginList{}
+	for _, pl := range plugins {
+		if isPartOfCommandGroup(commandGroupParts, pl.CommandParts()) {
+			ret = append(ret, pl)
+		}
+	}
+	return ret
+}
+
+func isPartOfCommandGroup(commandGroupParts []string, commandParts []string) bool {
+	if len(commandParts) != len(commandGroupParts)+1 {
+		return false
+	}
+	for i := range commandGroupParts {
+		if commandParts[i] != commandGroupParts[i] {
+			return false
+		}
+	}
+	return true
+}
+
+func isPluginFileNamePartOfCommandGroup(commandGroupParts []string, pluginFileName string) bool {
 	if commandGroupParts == nil {
 		return true
 	}
 
-	commandParts := extractPluginCommandFromFileName(name)
-
-	// commandParts must be one more element then the parts of the command group
-	// it belongs to. E.g. for the command "service", "log" (2 elements) the containing
-	// group only has one element ("service"). This condition is here for
-	// shortcut and ensure that we don't run in an out-of-bound array error
-	// in the loop below.
+	commandParts := extractPluginCommandFromFileName(pluginFileName)
 	if len(commandParts) != len(commandGroupParts)+1 {
 		return false
 	}
@@ -343,7 +373,7 @@ func stripWindowsExecExtensions(name string) string {
 // Return the path and the parts building the most specific plugin in the given directory
 // If lookupInPath is true, then also the OS PATH is checked.
 // An error returned if any IO operation fails
-func findMostSpecificPlugin(dir string, parts []string, lookupInPath bool) (Plugin, error) {
+func findMostSpecificPluginInPath(dir string, parts []string, lookupInPath bool) (Plugin, error) {
 	for i := len(parts); i > 0; i-- {
 
 		// Construct plugin name to lookup
@@ -429,3 +459,31 @@ func findInDirOrPath(name string, dir string, lookupInPath bool) (string, error)
 	// Not found
 	return "", nil
 }
+
+// lookupInternalPlugin looks up internally registered plugins. Return nil if none is found.
+// Start with longest argument path first to find the most specific match
+func lookupInternalPlugin(parts []string) Plugin {
+	for i := len(parts); i > 0; i-- {
+		checkParts := parts[0:i]
+		for _, plugin := range InternalPlugins {
+			if equalsSlice(plugin.CommandParts(), checkParts) {
+				return plugin
+			}
+		}
+	}
+	return nil
+}
+
+// equalsSlice return true if two string slices contain the same elements
+func equalsSlice(a, b []string) bool {
+	if len(a) != len(b) || len(a) == 0 {
+		return false
+	}
+
+	for i := range a {
+		if a[i] != b[i] {
+			return false
+		}
+	}
+	return true
+}


Test patch:
diff --git a/lib/test/broker.go b/lib/test/broker.go
index 8fdd5bd4..3784f176 100644
--- a/lib/test/broker.go
+++ b/lib/test/broker.go
@@ -24,7 +24,7 @@ import (
 
 // LabelNamespaceForDefaultBroker adds label 'knative-eventing-injection=enabled' to the configured namespace
 func LabelNamespaceForDefaultBroker(r *KnRunResultCollector) error {
-	cmd := []string{"label", "namespace", r.KnTest().Kn().Namespace(), v1beta1.DeprecatedInjectionAnnotation + "=enabled"}
+	cmd := []string{"label", "namespace", r.KnTest().Kn().Namespace(), v1beta1.InjectionAnnotation + "=enabled"}
 	_, err := Kubectl{}.Run(cmd...)
 
 	if err != nil {
@@ -43,7 +43,7 @@ func LabelNamespaceForDefaultBroker(r *KnRunResultCollector) error {
 
 // UnlabelNamespaceForDefaultBroker removes label 'knative-eventing-injection=enabled' from the configured namespace
 func UnlabelNamespaceForDefaultBroker(r *KnRunResultCollector) {
-	cmd := []string{"label", "namespace", r.KnTest().Kn().Namespace(), v1beta1.DeprecatedInjectionAnnotation + "-"}
+	cmd := []string{"label", "namespace", r.KnTest().Kn().Namespace(), v1beta1.InjectionAnnotation + "-"}
 	_, err := Kubectl{}.Run(cmd...)
 	if err != nil {
 		r.T().Fatalf("error executing '%s': %s", strings.Join(cmd, " "), err.Error())
diff --git a/pkg/kn/plugin/manager_test.go b/pkg/kn/plugin/manager_test.go
index 66c319b2..72e8c43e 100644
--- a/pkg/kn/plugin/manager_test.go
+++ b/pkg/kn/plugin/manager_test.go
@@ -21,6 +21,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/spf13/cobra"
@@ -39,6 +40,18 @@ type testContext struct {
 	pluginManager *Manager
 }
 
+type testPlugin struct {
+	parts []string
+}
+
+func (t testPlugin) Name() string                 { return "kn-" + strings.Join(t.parts, "-") }
+func (t testPlugin) Execute(args []string) error  { return nil }
+func (t testPlugin) Description() (string, error) { return "desc: " + t.Name(), nil }
+func (t testPlugin) CommandParts() []string       { return t.parts }
+func (t testPlugin) Path() string                 { return "" }
+
+var _ Plugin = testPlugin{}
+
 func TestEmptyFind(t *testing.T) {
 	ctx := setup(t)
 	defer cleanup(t, ctx)
@@ -65,7 +78,7 @@ func TestLookupInPluginsDir(t *testing.T) {
 	assert.Equal(t, out, "OK \n")
 }
 
-func TestLookupWithNotFoundResult(t *testing.T) {
+func TestFindWithNotFoundResult(t *testing.T) {
 	ctx := setup(t)
 	defer cleanup(t, ctx)
 
@@ -74,7 +87,7 @@ func TestLookupWithNotFoundResult(t *testing.T) {
 	assert.NilError(t, err, "no error expected")
 }
 
-func TestPluginInPath(t *testing.T) {
+func TestFindPluginInPath(t *testing.T) {
 	ctx := setup(t)
 	defer cleanup(t, ctx)
 
@@ -90,6 +103,9 @@ func TestPluginInPath(t *testing.T) {
 	plugin, err := ctx.pluginManager.FindPlugin(pluginCommands)
 	assert.NilError(t, err)
 	assert.Assert(t, plugin != nil)
+	desc, err := plugin.Description()
+	assert.NilError(t, err)
+	assert.Assert(t, desc != "")
 	assert.Equal(t, plugin.Path(), filepath.Join(tmpPathDir, "kn-path-test"))
 	assert.DeepEqual(t, plugin.CommandParts(), pluginCommands)
 
@@ -100,6 +116,31 @@ func TestPluginInPath(t *testing.T) {
 	assert.Assert(t, plugin == nil)
 }
 
+func TestFindPluginInternally(t *testing.T) {
+	ctx := setup(t)
+	defer cleanup(t, ctx)
+
+	// Initialize registered plugins
+	defer (prepareInternalPlugins(
+		testPlugin{[]string{"a", "b"}},
+		testPlugin{[]string{"a"}}))()
+
+	data := []struct {
+		parts []string
+		name  string
+	}{
+		{[]string{"a", "b"}, "kn-a-b"},
+		{[]string{"a"}, "kn-a"},
+		{[]string{"a", "c"}, "kn-a"},
+	}
+	for _, d := range data {
+		plugin, err := ctx.pluginManager.FindPlugin(d.parts)
+		assert.NilError(t, err)
+		assert.Assert(t, plugin != nil)
+		assert.Equal(t, plugin.Name(), d.name)
+	}
+}
+
 func TestPluginExecute(t *testing.T) {
 	ctx := setup(t)
 	defer cleanup(t, ctx)
@@ -112,20 +153,80 @@ func TestPluginExecute(t *testing.T) {
 	assert.Equal(t, out, "OK arg1 arg2\n")
 }
 
+func TestPluginMixed(t *testing.T) {
+	ctx := setup(t)
+	defer cleanup(t, ctx)
+
+	createTestPlugin(t, "kn-external", ctx)
+	createTestPlugin(t, "kn-shadow", ctx)
+
+	// Initialize registered plugins
+	defer (prepareInternalPlugins(
+		testPlugin{[]string{"internal"}},
+		testPlugin{[]string{"shadow"}},
+	))()
+
+	data := []struct {
+		path       []string
+		name       string
+		isInternal bool
+	}{
+		{[]string{"external"}, "kn-external", false},
+		{[]string{"internal"}, "kn-internal", true},
+		{[]string{"shadow"}, "kn-shadow", true},
+	}
+	for _, d := range data {
+		plugin, err := ctx.pluginManager.FindPlugin(d.path)
+		assert.NilError(t, err)
+		assert.Assert(t, plugin != nil)
+		assert.Equal(t, plugin.Name(), d.name)
+		_, ok := plugin.(testPlugin)
+		assert.Equal(t, d.isInternal, ok)
+	}
+}
+
+func prepareInternalPlugins(plugins ...Plugin) func() {
+	oldPlugins := InternalPlugins
+	InternalPlugins = plugins
+	return func() {
+		InternalPlugins = oldPlugins
+	}
+}
+
 func TestPluginListForCommandGroup(t *testing.T) {
 	ctx := setup(t)
 	defer cleanup(t, ctx)
-	createTestPlugin(t, "kn-service-log_2", ctx)
+	createTestPlugin(t, "kn-service-external", ctx)
+	createTestPlugin(t, "kn-foo-bar", ctx)
+	createTestPlugin(t, "kn-service-shadow", ctx)
+
+	// Internal plugin should be filtered out if not belong to the service group
+	defer (prepareInternalPlugins(
+		testPlugin{[]string{"service", "internal"}},
+		testPlugin{[]string{"service", "shadow"}},
+		testPlugin{[]string{"bla", "blub"}},
+		testPlugin{[]string{"bla", "blub", "longer"}}))()
 
 	pluginList, err := ctx.pluginManager.ListPluginsForCommandGroup([]string{"service"})
 	assert.NilError(t, err)
-	assert.Assert(t, pluginList.Len() == 1)
-	assert.Equal(t, pluginList[0].Name(), "kn-service-log_2")
+	assert.Assert(t, pluginList.Len() == 3)
+	assert.Assert(t, containsPluginWithName(pluginList, "kn-service-internal"))
+	assert.Assert(t, containsPluginWithName(pluginList, "kn-service-external"))
+	assert.Assert(t, containsPluginWithName(pluginList, "kn-service-shadow"))
 	pluginList, err = ctx.pluginManager.ListPluginsForCommandGroup([]string{})
 	assert.NilError(t, err)
 	assert.Assert(t, pluginList.Len() == 0)
 }
 
+func containsPluginWithName(plugins PluginList, name string) bool {
+	for _, pl := range plugins {
+		if pl.Name() == name {
+			return true
+		}
+	}
+	return false
+}
+
 func TestPluginHelpMessage(t *testing.T) {
 	ctx := setup(t)
 	defer cleanup(t, ctx)