From c28fc06e002e06deed3437da76bc213b7bd752ba Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Sun, 20 Dec 2015 19:44:01 +0100 Subject: [PATCH] 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 --- integration-cli/docker_cli_authz_unix_test.go | 18 ++++++++++++++++-- pkg/authorization/plugin.go | 11 ++++++++--- pkg/plugins/discovery.go | 2 +- pkg/plugins/plugins.go | 1 - 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/integration-cli/docker_cli_authz_unix_test.go b/integration-cli/docker_cli_authz_unix_test.go index f98788ddbb..6fab8b02c8 100644 --- a/integration-cli/docker_cli_authz_unix_test.go +++ b/integration-cli/docker_cli_authz_unix_test.go @@ -244,13 +244,27 @@ func (s *DockerAuthzSuite) TestAuthZPluginErrorRequest(c *check.C) { c.Assert(res, check.Equals, fmt.Sprintf("Error response from daemon: plugin %s failed with error: %s: %s\n", testAuthZPlugin, authorization.AuthZApiRequest, errorMessage)) } +func (s *DockerAuthzSuite) TestAuthZPluginEnsureNoDuplicatePluginRegistration(c *check.C) { + c.Assert(s.d.Start("--authz-plugin="+testAuthZPlugin, "--authz-plugin="+testAuthZPlugin), check.IsNil) + + s.ctrl.reqRes.Allow = true + s.ctrl.resRes.Allow = true + + out, err := s.d.Cmd("ps") + c.Assert(err, check.IsNil, check.Commentf(out)) + + // assert plugin is only called once.. + c.Assert(s.ctrl.psRequestCnt, check.Equals, 1) + c.Assert(s.ctrl.psResponseCnt, check.Equals, 1) +} + // assertURIRecorded verifies that the given URI was sent and recorded in the authz plugin func assertURIRecorded(c *check.C, uris []string, uri string) { - - found := false + var found bool for _, u := range uris { if strings.Contains(u, uri) { found = true + break } } if !found { diff --git a/pkg/authorization/plugin.go b/pkg/authorization/plugin.go index ded43a99c5..1b65ac0a5c 100644 --- a/pkg/authorization/plugin.go +++ b/pkg/authorization/plugin.go @@ -17,9 +17,14 @@ type Plugin interface { // NewPlugins constructs and initialize the authorization plugins based on plugin names func NewPlugins(names []string) []Plugin { - plugins := make([]Plugin, len(names)) - for i, name := range names { - plugins[i] = newAuthorizationPlugin(name) + plugins := []Plugin{} + pluginsMap := make(map[string]struct{}) + for _, name := range names { + if _, ok := pluginsMap[name]; ok { + continue + } + pluginsMap[name] = struct{}{} + plugins = append(plugins, newAuthorizationPlugin(name)) } return plugins } diff --git a/pkg/plugins/discovery.go b/pkg/plugins/discovery.go index d0ee27485f..5ea8db5c0a 100644 --- a/pkg/plugins/discovery.go +++ b/pkg/plugins/discovery.go @@ -13,7 +13,7 @@ import ( var ( // ErrNotFound plugin not found - ErrNotFound = errors.New("Plugin not found") + ErrNotFound = errors.New("plugin not found") socketsPath = "/run/docker/plugins" specsPaths = []string{"/etc/docker/plugins", "/usr/lib/docker/plugins"} ) diff --git a/pkg/plugins/plugins.go b/pkg/plugins/plugins.go index f9e22d651f..5ad4d892f9 100644 --- a/pkg/plugins/plugins.go +++ b/pkg/plugins/plugins.go @@ -96,7 +96,6 @@ func (p *Plugin) activateWithLock() error { return err } - logrus.Debugf("%s's manifest: %v", p.Name, m) p.Manifest = m for _, iface := range m.Implements {