[WIP] cli-plugins: look for plugins in experimental folder when experimental is enabled

cli-plugins/manager/candidate.go

@@ -8,16 +8,22 @@ import (
 type Candidate interface {
 	Path() string
 	Metadata() ([]byte, error)
+	Experimental() bool
 }
 
 type candidate struct {
-	path string
+	path         string
+	experimental bool
 }
 
 func (c *candidate) Path() string {
 	return c.path
 }
 
+func (c *candidate) Experimental() bool {
+	return c.experimental
+}
+
 func (c *candidate) Metadata() ([]byte, error) {
 	return exec.Command(c.path, MetadataSubcommandName).Output()
 }

cli-plugins/manager/candidate_test.go

@@ -12,9 +12,14 @@ import (
 )
 
 type fakeCandidate struct {
-	path string
-	exec bool
-	meta string
+	path              string
+	exec              bool
+	meta              string
+	allowExperimental bool
+}
+
+func (c *fakeCandidate) Experimental() bool {
+	return strings.Contains(c.path, "-experimental/")
 }
 
 func (c *fakeCandidate) Path() string {
@@ -35,9 +40,12 @@ 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
+		goodMeta = `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing"}`
+
+		badPrefixPath          = "/usr/local/libexec/cli-plugins/wobble"
+		badNamePath            = "/usr/local/libexec/cli-plugins/docker-123456"
+		goodPluginPath         = "/usr/local/libexec/cli-plugins/" + goodPluginName
+		experimentalPluginPath = "/usr/local/libexec/cli-plugins-experimental/" + goodPluginName
 	)
 
 	fakeroot := &cobra.Command{Use: "docker"}
@@ -49,40 +57,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: experimentalPluginPath, exec: true, meta: goodMeta}, 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: goodMeta}},
+		{name: "valid on experimental CLI", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: goodMeta, allowExperimental: true}},
+		{name: "experimental on experimental CLI", c: &fakeCandidate{path: experimentalPluginPath, exec: true, meta: goodMeta, 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
 }
@@ -48,10 +62,18 @@ func getPluginDirs(dockerCli command.Cli) ([]string, error) {
 
 	pluginDirs = append(pluginDirs, pluginDir)
 	pluginDirs = append(pluginDirs, defaultSystemPluginDirs...)
+	if dockerCli.ClientInfo().HasExperimental {
+		dirs := make([]string, 0, len(pluginDirs)*2)
+		for _, dir := range pluginDirs {
+			dirs = append(dirs, dir, dir+"-experimental")
+		}
+		pluginDirs = dirs
+	}
 	return pluginDirs, nil
 }
 
-func addPluginCandidatesFromDir(res map[string][]string, d string) error {
+func addPluginCandidatesFromDir(res map[string][]candidate, d string) error {
+	isExperimental := strings.HasSuffix(d, "-experimental")
 	dentries, err := ioutil.ReadDir(d)
 	if err != nil {
 		return err
@@ -73,14 +95,14 @@ func addPluginCandidatesFromDir(res map[string][]string, d string) error {
 		if name, err = trimExeSuffix(name); err != nil {
 			continue
 		}
-		res[name] = append(res[name], filepath.Join(d, dentry.Name()))
+		res[name] = append(res[name], candidate{path: filepath.Join(d, dentry.Name()), experimental: isExperimental})
 	}
 	return nil
 }
 
 // listPluginCandidates returns a map from plugin name to the list of (unvalidated) Candidates. The list is in descending order of priority.
-func listPluginCandidates(dirs []string) (map[string][]string, error) {
-	result := make(map[string][]string)
+func listPluginCandidates(dirs []string) (map[string][]candidate, error) {
+	result := make(map[string][]candidate)
 	for _, d := range dirs {
 		// Silently ignore any directories which we cannot
 		// Stat (e.g. due to permissions or anything else) or
@@ -106,23 +128,27 @@ func ListPlugins(dockerCli command.Cli, rootcmd *cobra.Command) ([]Plugin, error
 		return nil, err
 	}
 
-	candidates, err := listPluginCandidates(pluginDirs)
+	candidateMap, err := listPluginCandidates(pluginDirs)
 	if err != nil {
 		return nil, err
 	}
 
 	var plugins []Plugin
-	for _, paths := range candidates {
-		if len(paths) == 0 {
+	for _, candidates := range candidateMap {
+		if len(candidates) == 0 {
 			continue
 		}
-		c := &candidate{paths[0]}
-		p, err := newPlugin(c, rootcmd)
+		c := &candidates[0]
+		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) {
+			for _, c := range candidates[1:] {
+				p.ShadowedPaths = append(p.ShadowedPaths, c.path)
+			}
+			plugins = append(plugins, p)
+		}
 	}
 
 	return plugins, nil
@@ -159,12 +185,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/manager_test.go

@@ -1,6 +1,7 @@
 package manager
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
@@ -52,33 +53,40 @@ func TestListPluginCandidates(t *testing.T) {
 
 	candidates, err := listPluginCandidates(dirs)
 	assert.NilError(t, err)
-	exp := map[string][]string{
+	exp := map[string][]candidate{
 		"plugin1": {
-			dir.Join("plugins1", "docker-plugin1"),
-			dir.Join("plugins2", "docker-plugin1"),
-			dir.Join("plugins3", "docker-plugin1"),
+			{path: dir.Join("plugins1", "docker-plugin1")},
+			{path: dir.Join("plugins2", "docker-plugin1")},
+			{path: dir.Join("plugins3", "docker-plugin1")},
 		},
 		"symlinked1": {
-			dir.Join("plugins1", "docker-symlinked1"),
+			{path: dir.Join("plugins1", "docker-symlinked1")},
 		},
 		"symlinked2": {
-			dir.Join("plugins1", "docker-symlinked2"),
+			{path: dir.Join("plugins1", "docker-symlinked2")},
 		},
 		"hardlink1": {
-			dir.Join("plugins2", "docker-hardlink1"),
+			{path: dir.Join("plugins2", "docker-hardlink1")},
 		},
 		"hardlink2": {
-			dir.Join("plugins2", "docker-hardlink2"),
+			{path: dir.Join("plugins2", "docker-hardlink2")},
 		},
 		"brokensymlink": {
-			dir.Join("plugins3", "docker-brokensymlink"),
+			{path: dir.Join("plugins3", "docker-brokensymlink")},
 		},
 		"symlinked": {
-			dir.Join("plugins3", "docker-symlinked"),
+			{path: dir.Join("plugins3", "docker-symlinked")},
 		},
 	}
 
-	assert.DeepEqual(t, candidates, exp)
+	for name, candidateList := range candidates {
+		for i, c := range candidateList {
+			t.Run(fmt.Sprintf("%s-%d", name, i), func(t *testing.T) {
+				assert.Equal(t, c.path, exp[name][i].path)
+				assert.Equal(t, c.experimental, exp[name][i].experimental)
+			})
+		}
+	}
 }
 
 func TestErrPluginNotFound(t *testing.T) {

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")
@@ -58,6 +60,11 @@ func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
 		Path: path,
 	}
 
+	if c.Experimental() && !allowExperimental {
+		p.Err = &pluginError{errPluginRequireExperimental(p.Name)}
+		return p, nil
+	}
+
 	// Now apply the candidate tests, so these update p.Err.
 	if !pluginNameRe.MatchString(p.Name) {
 		p.Err = NewPluginError("plugin candidate %q did not match %q", p.Name, pluginNameRe.String())
@@ -94,7 +101,6 @@ func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
 		p.Err = wrapAsPluginError(err, "invalid metadata")
 		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