Merge pull request #1898 from tiborvass/plugin_experimental

cli-plugins: add concept of experimental plugin, only enabled in experimental mode

cli-plugins/examples/helloworld/main.go

@@ -101,5 +101,6 @@ func main() {
 			SchemaVersion: "0.1.0",
 			Vendor:        "Docker Inc.",
 			Version:       "testing",
+			Experimental:  os.Getenv("HELLO_EXPERIMENTAL") != "",
 		})
 }

cli-plugins/manager/candidate_test.go

@@ -12,9 +12,10 @@ import (
 )
 
 type fakeCandidate struct {
-	path string
-	exec bool
-	meta string
+	path              string
+	exec              bool
+	meta              string
+	allowExperimental bool
 }
 
 func (c *fakeCandidate) Path() string {
@@ -35,9 +36,10 @@ func TestValidateCandidate(t *testing.T) {
 		builtinName  = NamePrefix + "builtin"
 		builtinAlias = NamePrefix + "alias"
 
-		badPrefixPath  = "/usr/local/libexec/cli-plugins/wobble"
-		badNamePath    = "/usr/local/libexec/cli-plugins/docker-123456"
-		goodPluginPath = "/usr/local/libexec/cli-plugins/" + goodPluginName
+		badPrefixPath    = "/usr/local/libexec/cli-plugins/wobble"
+		badNamePath      = "/usr/local/libexec/cli-plugins/docker-123456"
+		goodPluginPath   = "/usr/local/libexec/cli-plugins/" + goodPluginName
+		metaExperimental = `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing", "Experimental": true}`
 	)
 
 	fakeroot := &cobra.Command{Use: "docker"}
@@ -49,40 +51,46 @@ func TestValidateCandidate(t *testing.T) {
 	})
 
 	for _, tc := range []struct {
-		c *fakeCandidate
+		name string
+		c    *fakeCandidate
 
 		// Either err or invalid may be non-empty, but not both (both can be empty for a good plugin).
 		err     string
 		invalid string
 	}{
 		/* Each failing one of the tests */
-		{c: &fakeCandidate{path: ""}, err: "plugin candidate path cannot be empty"},
-		{c: &fakeCandidate{path: badPrefixPath}, err: fmt.Sprintf("does not have %q prefix", NamePrefix)},
-		{c: &fakeCandidate{path: badNamePath}, invalid: "did not match"},
-		{c: &fakeCandidate{path: builtinName}, invalid: `plugin "builtin" duplicates builtin command`},
-		{c: &fakeCandidate{path: builtinAlias}, invalid: `plugin "alias" duplicates an alias of builtin command "builtin"`},
-		{c: &fakeCandidate{path: goodPluginPath, exec: false}, invalid: fmt.Sprintf("failed to fetch metadata: faked a failure to exec %q", goodPluginPath)},
-		{c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `xyzzy`}, invalid: "invalid character"},
-		{c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{}`}, invalid: `plugin SchemaVersion "" is not valid`},
-		{c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "xyzzy"}`}, invalid: `plugin SchemaVersion "xyzzy" is not valid`},
-		{c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0"}`}, invalid: "plugin metadata does not define a vendor"},
-		{c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": ""}`}, invalid: "plugin metadata does not define a vendor"},
+		{name: "empty path", c: &fakeCandidate{path: ""}, err: "plugin candidate path cannot be empty"},
+		{name: "bad prefix", c: &fakeCandidate{path: badPrefixPath}, err: fmt.Sprintf("does not have %q prefix", NamePrefix)},
+		{name: "bad path", c: &fakeCandidate{path: badNamePath}, invalid: "did not match"},
+		{name: "builtin command", c: &fakeCandidate{path: builtinName}, invalid: `plugin "builtin" duplicates builtin command`},
+		{name: "builtin alias", c: &fakeCandidate{path: builtinAlias}, invalid: `plugin "alias" duplicates an alias of builtin command "builtin"`},
+		{name: "fetch failure", c: &fakeCandidate{path: goodPluginPath, exec: false}, invalid: fmt.Sprintf("failed to fetch metadata: faked a failure to exec %q", goodPluginPath)},
+		{name: "metadata not json", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `xyzzy`}, invalid: "invalid character"},
+		{name: "empty schemaversion", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{}`}, invalid: `plugin SchemaVersion "" is not valid`},
+		{name: "invalid schemaversion", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "xyzzy"}`}, invalid: `plugin SchemaVersion "xyzzy" is not valid`},
+		{name: "no vendor", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0"}`}, invalid: "plugin metadata does not define a vendor"},
+		{name: "empty vendor", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": ""}`}, invalid: "plugin metadata does not define a vendor"},
+		{name: "experimental required", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: metaExperimental}, invalid: "requires experimental CLI"},
 		// This one should work
-		{c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing"}`}},
+		{name: "valid", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing"}`}},
+		{name: "valid + allowing experimental", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing"}`, allowExperimental: true}},
+		{name: "experimental + allowing experimental", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: metaExperimental, allowExperimental: true}},
 	} {
-		p, err := newPlugin(tc.c, fakeroot)
-		if tc.err != "" {
-			assert.ErrorContains(t, err, tc.err)
-		} else if tc.invalid != "" {
-			assert.NilError(t, err)
-			assert.Assert(t, cmp.ErrorType(p.Err, reflect.TypeOf(&pluginError{})))
-			assert.ErrorContains(t, p.Err, tc.invalid)
-		} else {
-			assert.NilError(t, err)
-			assert.Equal(t, NamePrefix+p.Name, goodPluginName)
-			assert.Equal(t, p.SchemaVersion, "0.1.0")
-			assert.Equal(t, p.Vendor, "e2e-testing")
-		}
+		t.Run(tc.name, func(t *testing.T) {
+			p, err := newPlugin(tc.c, fakeroot, tc.c.allowExperimental)
+			if tc.err != "" {
+				assert.ErrorContains(t, err, tc.err)
+			} else if tc.invalid != "" {
+				assert.NilError(t, err)
+				assert.Assert(t, cmp.ErrorType(p.Err, reflect.TypeOf(&pluginError{})))
+				assert.ErrorContains(t, p.Err, tc.invalid)
+			} else {
+				assert.NilError(t, err)
+				assert.Equal(t, NamePrefix+p.Name, goodPluginName)
+				assert.Equal(t, p.SchemaVersion, "0.1.0")
+				assert.Equal(t, p.Vendor, "e2e-testing")
+			}
+		})
 	}
 }
 

cli-plugins/manager/manager.go

@@ -1,6 +1,7 @@
 package manager
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -27,10 +28,23 @@ func (e errPluginNotFound) Error() string {
 	return "Error: No such CLI plugin: " + string(e)
 }
 
+type errPluginRequireExperimental string
+
+// Note: errPluginRequireExperimental implements notFound so that the plugin
+// is skipped when listing the plugins.
+func (e errPluginRequireExperimental) NotFound() {}
+
+func (e errPluginRequireExperimental) Error() string {
+	return fmt.Sprintf("plugin candidate %q: requires experimental CLI", string(e))
+}
+
 type notFound interface{ NotFound() }
 
 // IsNotFound is true if the given error is due to a plugin not being found.
 func IsNotFound(err error) bool {
+	if e, ok := err.(*pluginError); ok {
+		err = e.Cause()
+	}
 	_, ok := err.(notFound)
 	return ok
 }
@@ -117,12 +131,14 @@ func ListPlugins(dockerCli command.Cli, rootcmd *cobra.Command) ([]Plugin, error
 			continue
 		}
 		c := &candidate{paths[0]}
-		p, err := newPlugin(c, rootcmd)
+		p, err := newPlugin(c, rootcmd, dockerCli.ClientInfo().HasExperimental)
 		if err != nil {
 			return nil, err
 		}
-		p.ShadowedPaths = paths[1:]
-		plugins = append(plugins, p)
+		if !IsNotFound(p.Err) {
+			p.ShadowedPaths = paths[1:]
+			plugins = append(plugins, p)
+		}
 	}
 
 	return plugins, nil
@@ -159,12 +175,19 @@ func PluginRunCommand(dockerCli command.Cli, name string, rootcmd *cobra.Command
 		}
 
 		c := &candidate{path: path}
-		plugin, err := newPlugin(c, rootcmd)
+		plugin, err := newPlugin(c, rootcmd, dockerCli.ClientInfo().HasExperimental)
 		if err != nil {
 			return nil, err
 		}
 		if plugin.Err != nil {
 			// TODO: why are we not returning plugin.Err?
+
+			err := plugin.Err.(*pluginError).Cause()
+			// if an experimental plugin was invoked directly while experimental mode is off
+			// provide a more useful error message than "not found".
+			if err, ok := err.(errPluginRequireExperimental); ok {
+				return nil, err
+			}
 			return nil, errPluginNotFound(name)
 		}
 		cmd := exec.Command(plugin.Path, args...)

cli-plugins/manager/metadata.go

@@ -22,4 +22,7 @@ type Metadata struct {
 	ShortDescription string `json:",omitempty"`
 	// URL is a pointer to the plugin's homepage.
 	URL string `json:",omitempty"`
+	// Experimental specifies whether the plugin is experimental.
+	// Experimental plugins are not displayed on non-experimental CLIs.
+	Experimental bool `json:",omitempty"`
 }

cli-plugins/manager/plugin.go

@@ -33,7 +33,9 @@ type Plugin struct {
 // is set, and is always a `pluginError`, but the `Plugin` is still
 // returned with no error. An error is only returned due to a
 // non-recoverable error.
-func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
+//
+// nolint: gocyclo
+func newPlugin(c Candidate, rootcmd *cobra.Command, allowExperimental bool) (Plugin, error) {
 	path := c.Path()
 	if path == "" {
 		return Plugin{}, errors.New("plugin candidate path cannot be empty")
@@ -94,7 +96,10 @@ func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
 		p.Err = wrapAsPluginError(err, "invalid metadata")
 		return p, nil
 	}
-
+	if p.Experimental && !allowExperimental {
+		p.Err = &pluginError{errPluginRequireExperimental(p.Name)}
+		return p, nil
+	}
 	if p.Metadata.SchemaVersion != "0.1.0" {
 		p.Err = NewPluginError("plugin SchemaVersion %q is not valid, must be 0.1.0", p.Metadata.SchemaVersion)
 		return p, nil