Browse code

pkg: authorization: do not register the same plugin

This patches avoids registering (and calling) the same plugin more than
once. Using an helper map which indexes by name guarantees this and keeps
the order.
The behavior of overriding the same name in a flag is consistent with,
for instance, the `docker run -v /test -v /test` flag which register
the volume just once.
Adds integration tests.

Without this patch:
```
Dec 20 19:34:52 localhost.localdomain docker[9988]:
time="2015-12-20T19:34:52.080901676+01:00" level=debug msg="Calling
GET
/v1.22/info"
Dec 20 19:34:52 localhost.localdomain docker[9988]:
time="2015-12-20T19:34:52.081213202+01:00" level=debug msg="AuthZ
request using plugin docker-novolume-plugin"
Dec 20 19:34:52 localhost.localdomain docker[9988]:
time="2015-12-20T19:34:52.081268132+01:00" level=debug
msg="docker-novolume-plugin implements: authz"
Dec 20 19:34:52 localhost.localdomain docker[9988]:
time="2015-12-20T19:34:52.081699788+01:00" level=debug msg="AuthZ
request using plugin docker-novolume-plugin"
Dec 20 19:34:52 localhost.localdomain docker[9988]:
time="2015-12-20T19:34:52.081762507+01:00" level=debug
msg="docker-novolume-plugin implements: authz"
Dec 20 19:34:52 localhost.localdomain docker[9988]:
time="2015-12-20T19:34:52.082092480+01:00" level=debug msg="GET
/v1.22/info"
Dec 20 19:34:52 localhost.localdomain docker[9988]:
time="2015-12-20T19:34:52.628691038+01:00" level=debug msg="AuthZ
response using plugin docker-novolume-plugin"
Dec 20 19:34:52 localhost.localdomain docker[9988]:
time="2015-12-20T19:34:52.629880930+01:00" level=debug msg="AuthZ
response using plugin docker-novolume-plugin"
```

With this patch:
```
Dec 20 19:37:32 localhost.localdomain docker[16620]:
time="2015-12-20T19:37:32.376523958+01:00" level=debug msg="Calling
GET
/v1.22/info"
Dec 20 19:37:32 localhost.localdomain docker[16620]:
time="2015-12-20T19:37:32.376715483+01:00" level=debug msg="AuthZ
request using plugin docker-novolume-plugin"
Dec 20 19:37:32 localhost.localdomain docker[16620]:
time="2015-12-20T19:37:32.376771230+01:00" level=debug
msg="docker-novolume-plugin implements: authz"
Dec 20 19:37:32 localhost.localdomain docker[16620]:
time="2015-12-20T19:37:32.377698897+01:00" level=debug msg="GET
/v1.22/info"
Dec 20 19:37:32 localhost.localdomain docker[16620]:
time="2015-12-20T19:37:32.951016441+01:00" level=debug msg="AuthZ
response using plugin docker-novolume-plugin"
```

Also removes a somehow duplicate debug statement (leaving only the
second one as it's a loop of plugin's manifest):
```
Dec 20 19:52:30 localhost.localdomain docker[25767]:
time="2015-12-20T19:52:30.544090518+01:00" level=debug
msg="docker-novolume-plugin's manifest: &{[authz]}"
Dec 20 19:52:30 localhost.localdomain docker[25767]:
time="2015-12-20T19:52:30.544170677+01:00" level=debug
msg="docker-novolume-plugin implements: authz"
```

Signed-off-by: Antonio Murdaca <runcom@redhat.com>

Antonio Murdaca authored on 2015/12/21 03:44:01
Showing 4 changed files
... ...
@@ -244,13 +244,27 @@ func (s *DockerAuthzSuite) TestAuthZPluginErrorRequest(c *check.C) {
244 244
 	c.Assert(res, check.Equals, fmt.Sprintf("Error response from daemon: plugin %s failed with error: %s: %s\n", testAuthZPlugin, authorization.AuthZApiRequest, errorMessage))
245 245
 }
246 246
 
247
+func (s *DockerAuthzSuite) TestAuthZPluginEnsureNoDuplicatePluginRegistration(c *check.C) {
248
+	c.Assert(s.d.Start("--authz-plugin="+testAuthZPlugin, "--authz-plugin="+testAuthZPlugin), check.IsNil)
249
+
250
+	s.ctrl.reqRes.Allow = true
251
+	s.ctrl.resRes.Allow = true
252
+
253
+	out, err := s.d.Cmd("ps")
254
+	c.Assert(err, check.IsNil, check.Commentf(out))
255
+
256
+	// assert plugin is only called once..
257
+	c.Assert(s.ctrl.psRequestCnt, check.Equals, 1)
258
+	c.Assert(s.ctrl.psResponseCnt, check.Equals, 1)
259
+}
260
+
247 261
 // assertURIRecorded verifies that the given URI was sent and recorded in the authz plugin
248 262
 func assertURIRecorded(c *check.C, uris []string, uri string) {
249
-
250
-	found := false
263
+	var found bool
251 264
 	for _, u := range uris {
252 265
 		if strings.Contains(u, uri) {
253 266
 			found = true
267
+			break
254 268
 		}
255 269
 	}
256 270
 	if !found {
... ...
@@ -17,9 +17,14 @@ type Plugin interface {
17 17
 
18 18
 // NewPlugins constructs and initialize the authorization plugins based on plugin names
19 19
 func NewPlugins(names []string) []Plugin {
20
-	plugins := make([]Plugin, len(names))
21
-	for i, name := range names {
22
-		plugins[i] = newAuthorizationPlugin(name)
20
+	plugins := []Plugin{}
21
+	pluginsMap := make(map[string]struct{})
22
+	for _, name := range names {
23
+		if _, ok := pluginsMap[name]; ok {
24
+			continue
25
+		}
26
+		pluginsMap[name] = struct{}{}
27
+		plugins = append(plugins, newAuthorizationPlugin(name))
23 28
 	}
24 29
 	return plugins
25 30
 }
... ...
@@ -13,7 +13,7 @@ import (
13 13
 
14 14
 var (
15 15
 	// ErrNotFound plugin not found
16
-	ErrNotFound = errors.New("Plugin not found")
16
+	ErrNotFound = errors.New("plugin not found")
17 17
 	socketsPath = "/run/docker/plugins"
18 18
 	specsPaths  = []string{"/etc/docker/plugins", "/usr/lib/docker/plugins"}
19 19
 )
... ...
@@ -96,7 +96,6 @@ func (p *Plugin) activateWithLock() error {
96 96
 		return err
97 97
 	}
98 98
 
99
-	logrus.Debugf("%s's manifest: %v", p.Name, m)
100 99
 	p.Manifest = m
101 100
 
102 101
 	for _, iface := range m.Implements {