From 6347d30b6f2c803504eddf4720c2593578bc30cd Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 25 Jan 2018 17:41:45 -0800 Subject: [PATCH] Fix issue with plugin scanner going to deep The plugin spec says that plugins can live in one of: - /var/run/docker/plugins/.sock - /var/run/docker/plugins//.sock - /etc/docker/plugins/.[json,spec] - /etc/docker/plugins//. - /usr/lib/docker/plugins/. - /usr/lib/docker/plugins//. However, the plugin scanner which is used by the volume list API was doing `filepath.Walk`, which will walk the entire tree for each of the supported paths. This means that even v2 plugins in `/var/run/docker/plugins//.sock` were being detected as a v1 plugin. When the v1 plugin loader tried to load such a plugin it would log an error that it couldn't find it because it doesn't match one of the supported patterns... e.g. when in a subdir, the subdir name must match the plugin name for the socket. There is no behavior change as the error is only on the `Scan()` call, which is passing names to the plugin registry when someone calls the volume list API. Signed-off-by: Brian Goff Upstream-commit: b27f70d45a0fbb744c17dda02f597ffa6a47d4d9 Component: engine --- components/engine/pkg/plugins/discovery.go | 61 +++++++++++++------ .../engine/pkg/plugins/discovery_unix_test.go | 56 +++++++++++++++++ components/engine/pkg/plugins/plugin_test.go | 6 +- 3 files changed, 101 insertions(+), 22 deletions(-) diff --git a/components/engine/pkg/plugins/discovery.go b/components/engine/pkg/plugins/discovery.go index e99581c573..b126876390 100644 --- a/components/engine/pkg/plugins/discovery.go +++ b/components/engine/pkg/plugins/discovery.go @@ -2,7 +2,6 @@ package plugins import ( "encoding/json" - "errors" "fmt" "io/ioutil" "net/url" @@ -10,6 +9,8 @@ import ( "path/filepath" "strings" "sync" + + "github.com/pkg/errors" ) var ( @@ -28,30 +29,52 @@ func newLocalRegistry() localRegistry { // Scan scans all the plugin paths and returns all the names it found func Scan() ([]string, error) { var names []string - if err := filepath.Walk(socketsPath, func(path string, fi os.FileInfo, err error) error { - if err != nil { - return nil + dirEntries, err := ioutil.ReadDir(socketsPath) + if err != nil && !os.IsNotExist(err) { + return nil, errors.Wrap(err, "error reading dir entries") + } + + for _, fi := range dirEntries { + if fi.IsDir() { + fi, err = os.Stat(filepath.Join(socketsPath, fi.Name(), fi.Name()+".sock")) + if err != nil { + continue + } } if fi.Mode()&os.ModeSocket != 0 { - name := strings.TrimSuffix(fi.Name(), filepath.Ext(fi.Name())) - names = append(names, name) + names = append(names, strings.TrimSuffix(filepath.Base(fi.Name()), filepath.Ext(fi.Name()))) } - return nil - }); err != nil { - return nil, err } - for _, path := range specsPaths { - if err := filepath.Walk(path, func(p string, fi os.FileInfo, err error) error { - if err != nil || fi.IsDir() { - return nil + for _, p := range specsPaths { + dirEntries, err := ioutil.ReadDir(p) + if err != nil && !os.IsNotExist(err) { + return nil, errors.Wrap(err, "error reading dir entries") + } + + for _, fi := range dirEntries { + if fi.IsDir() { + infos, err := ioutil.ReadDir(filepath.Join(p, fi.Name())) + if err != nil { + continue + } + + for _, info := range infos { + if strings.TrimSuffix(info.Name(), filepath.Ext(info.Name())) == fi.Name() { + fi = info + break + } + } + } + + ext := filepath.Ext(fi.Name()) + switch ext { + case ".spec", ".json": + plugin := strings.TrimSuffix(fi.Name(), ext) + names = append(names, plugin) + default: } - name := strings.TrimSuffix(fi.Name(), filepath.Ext(fi.Name())) - names = append(names, name) - return nil - }); err != nil { - return nil, err } } return names, nil @@ -81,7 +104,7 @@ func (l *localRegistry) Plugin(name string) (*Plugin, error) { return readPluginInfo(name, p) } } - return nil, ErrNotFound + return nil, errors.Wrapf(ErrNotFound, "could not find plugin %s in v1 plugin registry", name) } func readPluginInfo(name, path string) (*Plugin, error) { diff --git a/components/engine/pkg/plugins/discovery_unix_test.go b/components/engine/pkg/plugins/discovery_unix_test.go index e4d156dbdc..a78f0298d4 100644 --- a/components/engine/pkg/plugins/discovery_unix_test.go +++ b/components/engine/pkg/plugins/discovery_unix_test.go @@ -97,7 +97,63 @@ func TestScan(t *testing.T) { if err != nil { t.Fatal(err) } + if len(pluginNamesNotEmpty) != 1 { + t.Fatalf("expected 1 plugin entry: %v", pluginNamesNotEmpty) + } if p.Name() != pluginNamesNotEmpty[0] { t.Fatalf("Unable to scan plugin with name %s", p.name) } } + +func TestScanNotPlugins(t *testing.T) { + tmpdir, unregister := Setup(t) + defer unregister() + + // not that `Setup()` above sets the sockets path and spec path dirs, which + // `Scan()` uses to find plugins to the returned `tmpdir` + + notPlugin := filepath.Join(tmpdir, "not-a-plugin") + if err := os.MkdirAll(notPlugin, 0700); err != nil { + t.Fatal(err) + } + + // this is named differently than the dir it's in, so the scanner should ignore it + l, err := net.Listen("unix", filepath.Join(notPlugin, "foo.sock")) + if err != nil { + t.Fatal(err) + } + defer l.Close() + + // same let's test a spec path + f, err := os.Create(filepath.Join(notPlugin, "foo.spec")) + if err != nil { + t.Fatal(err) + } + defer f.Close() + + names, err := Scan() + if err != nil { + t.Fatal(err) + } + if len(names) != 0 { + t.Fatalf("expected no plugins, got %v", names) + } + + // Just as a sanity check, let's make an entry that the scanner should read + f, err = os.Create(filepath.Join(notPlugin, "not-a-plugin.spec")) + if err != nil { + t.Fatal(err) + } + defer f.Close() + + names, err = Scan() + if err != nil { + t.Fatal(err) + } + if len(names) != 1 { + t.Fatalf("expected 1 entry in result: %v", names) + } + if names[0] != "not-a-plugin" { + t.Fatalf("expected plugin named `not-a-plugin`, got: %s", names[0]) + } +} diff --git a/components/engine/pkg/plugins/plugin_test.go b/components/engine/pkg/plugins/plugin_test.go index 00fcb85f58..8341b09daf 100644 --- a/components/engine/pkg/plugins/plugin_test.go +++ b/components/engine/pkg/plugins/plugin_test.go @@ -3,7 +3,6 @@ package plugins import ( "bytes" "encoding/json" - "errors" "io" "io/ioutil" "net/http" @@ -15,6 +14,7 @@ import ( "github.com/docker/docker/pkg/plugins/transport" "github.com/docker/go-connections/tlsconfig" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) @@ -78,11 +78,11 @@ func TestGet(t *testing.T) { // check negative case where plugin fruit doesn't implement banana _, err = Get("fruit", "banana") - assert.Equal(t, err, ErrNotImplements) + assert.Equal(t, errors.Cause(err), ErrNotImplements) // check negative case where plugin vegetable doesn't exist _, err = Get("vegetable", "potato") - assert.Equal(t, err, ErrNotFound) + assert.Equal(t, errors.Cause(err), ErrNotFound) }