Skip to content

Commit 200fada

Browse files
author
Tibor Vass
committed
cli-plugins: look for plugins in experimental folder when experimental is enabled
This patch expands the cli plugin search paths by adding `-experimental` to each, only if the CLI has experimental mode enabled. To test that experimental plugins do not work by default: ``` $ ln -s plugins-linux-amd64 build/plugins-linux-amd64-experimental $ vim ~/.config/docker.json # add $(pwd)/build/plugins-linux-amd64 to "cliPluginsExtraDirs" $ make plugins $ make binary $ docker helloworld ``` To show it enabled: ``` DOCKER_CLI_EXPERIMENTAL=enabled docker helloworld ``` Signed-off-by: Tibor Vass <tibor@docker.com>
1 parent 57aa773 commit 200fada

File tree

5 files changed

+95
-33
lines changed

5 files changed

+95
-33
lines changed

cli-plugins/manager/candidate.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,22 @@ import (
88
type Candidate interface {
99
Path() string
1010
Metadata() ([]byte, error)
11+
Experimental() bool
1112
}
1213

1314
type candidate struct {
14-
path string
15+
path string
16+
experimental bool
1517
}
1618

1719
func (c *candidate) Path() string {
1820
return c.path
1921
}
2022

23+
func (c *candidate) Experimental() bool {
24+
return c.experimental
25+
}
26+
2127
func (c *candidate) Metadata() ([]byte, error) {
2228
return exec.Command(c.path, MetadataSubcommandName).Output()
2329
}

cli-plugins/manager/candidate_test.go

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,14 @@ import (
1212
)
1313

1414
type fakeCandidate struct {
15-
path string
16-
exec bool
17-
meta string
15+
path string
16+
exec bool
17+
meta string
18+
allowExperimental bool
19+
}
20+
21+
func (c *fakeCandidate) Experimental() bool {
22+
return c.allowExperimental
1823
}
1924

2025
func (c *fakeCandidate) Path() string {
@@ -35,9 +40,10 @@ func TestValidateCandidate(t *testing.T) {
3540
builtinName = NamePrefix + "builtin"
3641
builtinAlias = NamePrefix + "alias"
3742

38-
badPrefixPath = "/usr/local/libexec/cli-plugins/wobble"
39-
badNamePath = "/usr/local/libexec/cli-plugins/docker-123456"
40-
goodPluginPath = "/usr/local/libexec/cli-plugins/" + goodPluginName
43+
badPrefixPath = "/usr/local/libexec/cli-plugins/wobble"
44+
badNamePath = "/usr/local/libexec/cli-plugins/docker-123456"
45+
goodPluginPath = "/usr/local/libexec/cli-plugins/" + goodPluginName
46+
experimentalPluginPath = "/usr/local/libexec/cli-plugins-experimental/" + goodPluginName
4147
)
4248

4349
fakeroot := &cobra.Command{Use: "docker"}
@@ -67,10 +73,13 @@ func TestValidateCandidate(t *testing.T) {
6773
{c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "xyzzy"}`}, invalid: `plugin SchemaVersion "xyzzy" is not valid`},
6874
{c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0"}`}, invalid: "plugin metadata does not define a vendor"},
6975
{c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": ""}`}, invalid: "plugin metadata does not define a vendor"},
76+
{c: &fakeCandidate{path: experimentalPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing"}`}, invalid: "requires experimental CLI"},
7077
// This one should work
7178
{c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing"}`}},
79+
{c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing"}`, allowExperimental: true}},
80+
{c: &fakeCandidate{path: experimentalPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing"}`, allowExperimental: true}},
7281
} {
73-
p, err := newPlugin(tc.c, fakeroot)
82+
p, err := newPlugin(tc.c, fakeroot, tc.c.allowExperimental)
7483
if tc.err != "" {
7584
assert.ErrorContains(t, err, tc.err)
7685
} else if tc.invalid != "" {

cli-plugins/manager/manager.go

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package manager
22

33
import (
4+
"fmt"
45
"io/ioutil"
56
"os"
67
"os/exec"
@@ -27,10 +28,23 @@ func (e errPluginNotFound) Error() string {
2728
return "Error: No such CLI plugin: " + string(e)
2829
}
2930

31+
type errPluginRequireExperimental string
32+
33+
// Note: errPluginRequireExperimental implements notFound so that the plugin
34+
// is skipped when listing the plugins.
35+
func (e errPluginRequireExperimental) NotFound() {}
36+
37+
func (e errPluginRequireExperimental) Error() string {
38+
return fmt.Sprintf("plugin candidate %q: requires experimental CLI", string(e))
39+
}
40+
3041
type notFound interface{ NotFound() }
3142

3243
// IsNotFound is true if the given error is due to a plugin not being found.
3344
func IsNotFound(err error) bool {
45+
if e, ok := err.(*pluginError); ok {
46+
err = e.Cause()
47+
}
3448
_, ok := err.(notFound)
3549
return ok
3650
}
@@ -48,10 +62,18 @@ func getPluginDirs(dockerCli command.Cli) ([]string, error) {
4862

4963
pluginDirs = append(pluginDirs, pluginDir)
5064
pluginDirs = append(pluginDirs, defaultSystemPluginDirs...)
65+
if dockerCli.ClientInfo().HasExperimental {
66+
dirs := make([]string, 0, len(pluginDirs)*2)
67+
for _, dir := range pluginDirs {
68+
dirs = append(dirs, dir, dir+"-experimental")
69+
}
70+
pluginDirs = dirs
71+
}
5172
return pluginDirs, nil
5273
}
5374

54-
func addPluginCandidatesFromDir(res map[string][]string, d string) error {
75+
func addPluginCandidatesFromDir(res map[string][]candidate, d string) error {
76+
isExperimental := strings.HasSuffix(d, "-experimental")
5577
dentries, err := ioutil.ReadDir(d)
5678
if err != nil {
5779
return err
@@ -73,14 +95,14 @@ func addPluginCandidatesFromDir(res map[string][]string, d string) error {
7395
if name, err = trimExeSuffix(name); err != nil {
7496
continue
7597
}
76-
res[name] = append(res[name], filepath.Join(d, dentry.Name()))
98+
res[name] = append(res[name], candidate{path: filepath.Join(d, dentry.Name()), experimental: isExperimental})
7799
}
78100
return nil
79101
}
80102

81103
// listPluginCandidates returns a map from plugin name to the list of (unvalidated) Candidates. The list is in descending order of priority.
82-
func listPluginCandidates(dirs []string) (map[string][]string, error) {
83-
result := make(map[string][]string)
104+
func listPluginCandidates(dirs []string) (map[string][]candidate, error) {
105+
result := make(map[string][]candidate)
84106
for _, d := range dirs {
85107
// Silently ignore any directories which we cannot
86108
// Stat (e.g. due to permissions or anything else) or
@@ -106,23 +128,27 @@ func ListPlugins(dockerCli command.Cli, rootcmd *cobra.Command) ([]Plugin, error
106128
return nil, err
107129
}
108130

109-
candidates, err := listPluginCandidates(pluginDirs)
131+
candidateMap, err := listPluginCandidates(pluginDirs)
110132
if err != nil {
111133
return nil, err
112134
}
113135

114136
var plugins []Plugin
115-
for _, paths := range candidates {
116-
if len(paths) == 0 {
137+
for _, candidates := range candidateMap {
138+
if len(candidates) == 0 {
117139
continue
118140
}
119-
c := &candidate{paths[0]}
120-
p, err := newPlugin(c, rootcmd)
141+
c := &candidates[0]
142+
p, err := newPlugin(c, rootcmd, dockerCli.ClientInfo().HasExperimental)
121143
if err != nil {
122144
return nil, err
123145
}
124-
p.ShadowedPaths = paths[1:]
125-
plugins = append(plugins, p)
146+
if !IsNotFound(p.Err) {
147+
for _, c := range candidates[1:] {
148+
p.ShadowedPaths = append(p.ShadowedPaths, c.path)
149+
}
150+
plugins = append(plugins, p)
151+
}
126152
}
127153

128154
return plugins, nil
@@ -159,12 +185,19 @@ func PluginRunCommand(dockerCli command.Cli, name string, rootcmd *cobra.Command
159185
}
160186

161187
c := &candidate{path: path}
162-
plugin, err := newPlugin(c, rootcmd)
188+
plugin, err := newPlugin(c, rootcmd, dockerCli.ClientInfo().HasExperimental)
163189
if err != nil {
164190
return nil, err
165191
}
166192
if plugin.Err != nil {
167193
// TODO: why are we not returning plugin.Err?
194+
195+
err := plugin.Err.(*pluginError).Cause()
196+
// if an experimental plugin was invoked directly while experimental mode is off
197+
// provide a more useful error message than "not found".
198+
if err, ok := err.(errPluginRequireExperimental); ok {
199+
return nil, err
200+
}
168201
return nil, errPluginNotFound(name)
169202
}
170203
cmd := exec.Command(plugin.Path, args...)

cli-plugins/manager/manager_test.go

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package manager
22

33
import (
4+
"fmt"
45
"strings"
56
"testing"
67

@@ -52,33 +53,40 @@ func TestListPluginCandidates(t *testing.T) {
5253

5354
candidates, err := listPluginCandidates(dirs)
5455
assert.NilError(t, err)
55-
exp := map[string][]string{
56+
exp := map[string][]candidate{
5657
"plugin1": {
57-
dir.Join("plugins1", "docker-plugin1"),
58-
dir.Join("plugins2", "docker-plugin1"),
59-
dir.Join("plugins3", "docker-plugin1"),
58+
{path: dir.Join("plugins1", "docker-plugin1")},
59+
{path: dir.Join("plugins2", "docker-plugin1")},
60+
{path: dir.Join("plugins3", "docker-plugin1")},
6061
},
6162
"symlinked1": {
62-
dir.Join("plugins1", "docker-symlinked1"),
63+
{path: dir.Join("plugins1", "docker-symlinked1")},
6364
},
6465
"symlinked2": {
65-
dir.Join("plugins1", "docker-symlinked2"),
66+
{path: dir.Join("plugins1", "docker-symlinked2")},
6667
},
6768
"hardlink1": {
68-
dir.Join("plugins2", "docker-hardlink1"),
69+
{path: dir.Join("plugins2", "docker-hardlink1")},
6970
},
7071
"hardlink2": {
71-
dir.Join("plugins2", "docker-hardlink2"),
72+
{path: dir.Join("plugins2", "docker-hardlink2")},
7273
},
7374
"brokensymlink": {
74-
dir.Join("plugins3", "docker-brokensymlink"),
75+
{path: dir.Join("plugins3", "docker-brokensymlink")},
7576
},
7677
"symlinked": {
77-
dir.Join("plugins3", "docker-symlinked"),
78+
{path: dir.Join("plugins3", "docker-symlinked")},
7879
},
7980
}
8081

81-
assert.DeepEqual(t, candidates, exp)
82+
for name, candidateList := range candidates {
83+
for i, c := range candidateList {
84+
t.Run(fmt.Sprintf("%s-%d", name, i), func(t *testing.T) {
85+
assert.Equal(t, c.path, exp[name][i].path)
86+
assert.Equal(t, c.experimental, exp[name][i].experimental)
87+
})
88+
}
89+
}
8290
}
8391

8492
func TestErrPluginNotFound(t *testing.T) {

cli-plugins/manager/plugin.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ type Plugin struct {
3333
// is set, and is always a `pluginError`, but the `Plugin` is still
3434
// returned with no error. An error is only returned due to a
3535
// non-recoverable error.
36-
func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
36+
//
37+
// nolint: gocyclo
38+
func newPlugin(c Candidate, rootcmd *cobra.Command, allowExperimental bool) (Plugin, error) {
3739
path := c.Path()
3840
if path == "" {
3941
return Plugin{}, errors.New("plugin candidate path cannot be empty")
@@ -58,6 +60,11 @@ func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
5860
Path: path,
5961
}
6062

63+
if c.Experimental() && !allowExperimental {
64+
p.Err = &pluginError{errPluginRequireExperimental(p.Name)}
65+
return p, nil
66+
}
67+
6168
// Now apply the candidate tests, so these update p.Err.
6269
if !pluginNameRe.MatchString(p.Name) {
6370
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) {
94101
p.Err = wrapAsPluginError(err, "invalid metadata")
95102
return p, nil
96103
}
97-
98104
if p.Metadata.SchemaVersion != "0.1.0" {
99105
p.Err = NewPluginError("plugin SchemaVersion %q is not valid, must be 0.1.0", p.Metadata.SchemaVersion)
100106
return p, nil

0 commit comments

Comments
 (0)