Browse code

Fix issue with plugin scanner going to deep

The plugin spec says that plugins can live in one of:

- /var/run/docker/plugins/<name>.sock
- /var/run/docker/plugins/<name>/<name>.sock
- /etc/docker/plugins/<name>.[json,spec]
- /etc/docker/plugins/<name>/<name>.<json,spec>
- /usr/lib/docker/plugins/<name>.<json,spec>
- /usr/lib/docker/plugins/<name>/<name>.<json,spec>

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/<id>/<name>.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 <cpuguy83@gmail.com>

Brian Goff authored on 2018/01/26 10:41:45
Showing 3 changed files
... ...
@@ -2,7 +2,6 @@ package plugins
2 2
 
3 3
 import (
4 4
 	"encoding/json"
5
-	"errors"
6 5
 	"fmt"
7 6
 	"io/ioutil"
8 7
 	"net/url"
... ...
@@ -10,6 +9,8 @@ import (
10 10
 	"path/filepath"
11 11
 	"strings"
12 12
 	"sync"
13
+
14
+	"github.com/pkg/errors"
13 15
 )
14 16
 
15 17
 var (
... ...
@@ -28,30 +29,52 @@ func newLocalRegistry() localRegistry {
28 28
 // Scan scans all the plugin paths and returns all the names it found
29 29
 func Scan() ([]string, error) {
30 30
 	var names []string
31
-	if err := filepath.Walk(socketsPath, func(path string, fi os.FileInfo, err error) error {
32
-		if err != nil {
33
-			return nil
31
+	dirEntries, err := ioutil.ReadDir(socketsPath)
32
+	if err != nil && !os.IsNotExist(err) {
33
+		return nil, errors.Wrap(err, "error reading dir entries")
34
+	}
35
+
36
+	for _, fi := range dirEntries {
37
+		if fi.IsDir() {
38
+			fi, err = os.Stat(filepath.Join(socketsPath, fi.Name(), fi.Name()+".sock"))
39
+			if err != nil {
40
+				continue
41
+			}
34 42
 		}
35 43
 
36 44
 		if fi.Mode()&os.ModeSocket != 0 {
37
-			name := strings.TrimSuffix(fi.Name(), filepath.Ext(fi.Name()))
38
-			names = append(names, name)
45
+			names = append(names, strings.TrimSuffix(filepath.Base(fi.Name()), filepath.Ext(fi.Name())))
39 46
 		}
40
-		return nil
41
-	}); err != nil {
42
-		return nil, err
43 47
 	}
44 48
 
45
-	for _, path := range specsPaths {
46
-		if err := filepath.Walk(path, func(p string, fi os.FileInfo, err error) error {
47
-			if err != nil || fi.IsDir() {
48
-				return nil
49
+	for _, p := range specsPaths {
50
+		dirEntries, err := ioutil.ReadDir(p)
51
+		if err != nil && !os.IsNotExist(err) {
52
+			return nil, errors.Wrap(err, "error reading dir entries")
53
+		}
54
+
55
+		for _, fi := range dirEntries {
56
+			if fi.IsDir() {
57
+				infos, err := ioutil.ReadDir(filepath.Join(p, fi.Name()))
58
+				if err != nil {
59
+					continue
60
+				}
61
+
62
+				for _, info := range infos {
63
+					if strings.TrimSuffix(info.Name(), filepath.Ext(info.Name())) == fi.Name() {
64
+						fi = info
65
+						break
66
+					}
67
+				}
68
+			}
69
+
70
+			ext := filepath.Ext(fi.Name())
71
+			switch ext {
72
+			case ".spec", ".json":
73
+				plugin := strings.TrimSuffix(fi.Name(), ext)
74
+				names = append(names, plugin)
75
+			default:
49 76
 			}
50
-			name := strings.TrimSuffix(fi.Name(), filepath.Ext(fi.Name()))
51
-			names = append(names, name)
52
-			return nil
53
-		}); err != nil {
54
-			return nil, err
55 77
 		}
56 78
 	}
57 79
 	return names, nil
... ...
@@ -81,7 +104,7 @@ func (l *localRegistry) Plugin(name string) (*Plugin, error) {
81 81
 			return readPluginInfo(name, p)
82 82
 		}
83 83
 	}
84
-	return nil, ErrNotFound
84
+	return nil, errors.Wrapf(ErrNotFound, "could not find plugin %s in v1 plugin registry", name)
85 85
 }
86 86
 
87 87
 func readPluginInfo(name, path string) (*Plugin, error) {
... ...
@@ -97,7 +97,63 @@ func TestScan(t *testing.T) {
97 97
 	if err != nil {
98 98
 		t.Fatal(err)
99 99
 	}
100
+	if len(pluginNamesNotEmpty) != 1 {
101
+		t.Fatalf("expected 1 plugin entry: %v", pluginNamesNotEmpty)
102
+	}
100 103
 	if p.Name() != pluginNamesNotEmpty[0] {
101 104
 		t.Fatalf("Unable to scan plugin with name %s", p.name)
102 105
 	}
103 106
 }
107
+
108
+func TestScanNotPlugins(t *testing.T) {
109
+	tmpdir, unregister := Setup(t)
110
+	defer unregister()
111
+
112
+	// not that `Setup()` above sets the sockets path and spec path dirs, which
113
+	// `Scan()` uses to find plugins to the returned `tmpdir`
114
+
115
+	notPlugin := filepath.Join(tmpdir, "not-a-plugin")
116
+	if err := os.MkdirAll(notPlugin, 0700); err != nil {
117
+		t.Fatal(err)
118
+	}
119
+
120
+	// this is named differently than the dir it's in, so the scanner should ignore it
121
+	l, err := net.Listen("unix", filepath.Join(notPlugin, "foo.sock"))
122
+	if err != nil {
123
+		t.Fatal(err)
124
+	}
125
+	defer l.Close()
126
+
127
+	// same let's test a spec path
128
+	f, err := os.Create(filepath.Join(notPlugin, "foo.spec"))
129
+	if err != nil {
130
+		t.Fatal(err)
131
+	}
132
+	defer f.Close()
133
+
134
+	names, err := Scan()
135
+	if err != nil {
136
+		t.Fatal(err)
137
+	}
138
+	if len(names) != 0 {
139
+		t.Fatalf("expected no plugins, got %v", names)
140
+	}
141
+
142
+	// Just as a sanity check, let's make an entry that the scanner should read
143
+	f, err = os.Create(filepath.Join(notPlugin, "not-a-plugin.spec"))
144
+	if err != nil {
145
+		t.Fatal(err)
146
+	}
147
+	defer f.Close()
148
+
149
+	names, err = Scan()
150
+	if err != nil {
151
+		t.Fatal(err)
152
+	}
153
+	if len(names) != 1 {
154
+		t.Fatalf("expected 1 entry in result: %v", names)
155
+	}
156
+	if names[0] != "not-a-plugin" {
157
+		t.Fatalf("expected plugin named `not-a-plugin`, got: %s", names[0])
158
+	}
159
+}
... ...
@@ -3,7 +3,6 @@ package plugins
3 3
 import (
4 4
 	"bytes"
5 5
 	"encoding/json"
6
-	"errors"
7 6
 	"io"
8 7
 	"io/ioutil"
9 8
 	"net/http"
... ...
@@ -15,6 +14,7 @@ import (
15 15
 
16 16
 	"github.com/docker/docker/pkg/plugins/transport"
17 17
 	"github.com/docker/go-connections/tlsconfig"
18
+	"github.com/pkg/errors"
18 19
 	"github.com/stretchr/testify/assert"
19 20
 )
20 21
 
... ...
@@ -78,11 +78,11 @@ func TestGet(t *testing.T) {
78 78
 
79 79
 	// check negative case where plugin fruit doesn't implement banana
80 80
 	_, err = Get("fruit", "banana")
81
-	assert.Equal(t, err, ErrNotImplements)
81
+	assert.Equal(t, errors.Cause(err), ErrNotImplements)
82 82
 
83 83
 	// check negative case where plugin vegetable doesn't exist
84 84
 	_, err = Get("vegetable", "potato")
85
-	assert.Equal(t, err, ErrNotFound)
85
+	assert.Equal(t, errors.Cause(err), ErrNotFound)
86 86
 
87 87
 }
88 88