From e0d79dff72a1e8508af87f7aedbe31f72a326a7d Mon Sep 17 00:00:00 2001 From: Phil Estes Date: Fri, 19 Feb 2016 10:12:39 -0800 Subject: [PATCH 1/2] Clean up authz integration-cli test - Order the flow of the handlers more cleanly--read req, do actions, write response. - Add "always allowed" endpoints to handle `/_ping` and `/info` usage from the test framework/daemon start/restart management Docker-DCO-1.1-Signed-off-by: Phil Estes (github: estesp) Upstream-commit: 074561b0ecc1e1b2e476c5aa06a8e6ea858239c1 Component: engine --- .../docker_cli_authz_unix_test.go | 48 ++++++++++++++----- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/components/engine/integration-cli/docker_cli_authz_unix_test.go b/components/engine/integration-cli/docker_cli_authz_unix_test.go index 9e0de88fad..e5858e90e9 100644 --- a/components/engine/integration-cli/docker_cli_authz_unix_test.go +++ b/components/engine/integration-cli/docker_cli_authz_unix_test.go @@ -30,6 +30,10 @@ const ( containerListAPI = "/containers/json" ) +var ( + alwaysAllowed = []string{"/_ping", "/info"} +) + func init() { check.Suite(&DockerAuthzSuite{ ds: &DockerSuite{}, @@ -74,12 +78,6 @@ func (s *DockerAuthzSuite) SetUpSuite(c *check.C) { }) mux.HandleFunc("/AuthZPlugin.AuthZReq", func(w http.ResponseWriter, r *http.Request) { - if s.ctrl.reqRes.Err != "" { - w.WriteHeader(http.StatusInternalServerError) - } - b, err := json.Marshal(s.ctrl.reqRes) - c.Assert(err, check.IsNil) - w.Write(b) defer r.Body.Close() body, err := ioutil.ReadAll(r.Body) c.Assert(err, check.IsNil) @@ -96,16 +94,20 @@ func (s *DockerAuthzSuite) SetUpSuite(c *check.C) { } s.ctrl.requestsURIs = append(s.ctrl.requestsURIs, authReq.RequestURI) + + reqRes := s.ctrl.reqRes + if isAllowed(authReq.RequestURI) { + reqRes = authorization.Response{Allow: true} + } + if reqRes.Err != "" { + w.WriteHeader(http.StatusInternalServerError) + } + b, err := json.Marshal(reqRes) + c.Assert(err, check.IsNil) + w.Write(b) }) mux.HandleFunc("/AuthZPlugin.AuthZRes", func(w http.ResponseWriter, r *http.Request) { - if s.ctrl.resRes.Err != "" { - w.WriteHeader(http.StatusInternalServerError) - } - b, err := json.Marshal(s.ctrl.resRes) - c.Assert(err, check.IsNil) - w.Write(b) - defer r.Body.Close() body, err := ioutil.ReadAll(r.Body) c.Assert(err, check.IsNil) @@ -120,6 +122,16 @@ func (s *DockerAuthzSuite) SetUpSuite(c *check.C) { if strings.HasSuffix(authReq.RequestURI, containerListAPI) { s.ctrl.psResponseCnt++ } + resRes := s.ctrl.resRes + if isAllowed(authReq.RequestURI) { + resRes = authorization.Response{Allow: true} + } + if resRes.Err != "" { + w.WriteHeader(http.StatusInternalServerError) + } + b, err := json.Marshal(resRes) + c.Assert(err, check.IsNil) + w.Write(b) }) err := os.MkdirAll("/etc/docker/plugins", 0755) @@ -130,6 +142,16 @@ func (s *DockerAuthzSuite) SetUpSuite(c *check.C) { c.Assert(err, checker.IsNil) } +// check for always allowed endpoints to not inhibit test framework functions +func isAllowed(reqURI string) bool { + for _, endpoint := range alwaysAllowed { + if strings.HasSuffix(reqURI, endpoint) { + return true + } + } + return false +} + // assertAuthHeaders validates authentication headers are removed func assertAuthHeaders(c *check.C, headers map[string]string) error { for k := range headers { From f30528fead2640866fd8b18c7b3c82d0f8846401 Mon Sep 17 00:00:00 2001 From: Phil Estes Date: Thu, 18 Feb 2016 22:00:45 -0800 Subject: [PATCH 2/2] Allow post-start load of busybox to remove restarts The restarts in the authz plugin test suite seems to be causing flakiness in CI, and can be avoided by separating the daemon start and busybox image load. Docker-DCO-1.1-Signed-off-by: Phil Estes (github: estesp) Upstream-commit: fe015c5ce0a260a8b5bc346a297507ac91f0ccb4 Component: engine --- .../docker_cli_authz_unix_test.go | 13 ++---- .../engine/integration-cli/docker_utils.go | 41 +++++++++++-------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/components/engine/integration-cli/docker_cli_authz_unix_test.go b/components/engine/integration-cli/docker_cli_authz_unix_test.go index e5858e90e9..b12201fb0a 100644 --- a/components/engine/integration-cli/docker_cli_authz_unix_test.go +++ b/components/engine/integration-cli/docker_cli_authz_unix_test.go @@ -193,13 +193,10 @@ func (s *DockerAuthzSuite) TearDownSuite(c *check.C) { func (s *DockerAuthzSuite) TestAuthZPluginAllowRequest(c *check.C) { // start the daemon and load busybox, --net=none build fails otherwise // cause it needs to pull busybox - c.Assert(s.d.StartWithBusybox(), check.IsNil) - // restart the daemon and enable the plugin, otherwise busybox loading - // is blocked by the plugin itself - c.Assert(s.d.Restart("--authorization-plugin="+testAuthZPlugin), check.IsNil) - + c.Assert(s.d.Start("--authorization-plugin="+testAuthZPlugin), check.IsNil) s.ctrl.reqRes.Allow = true s.ctrl.resRes.Allow = true + c.Assert(s.d.LoadBusybox(), check.IsNil) // Ensure command successful out, err := s.d.Cmd("run", "-d", "busybox", "top") @@ -254,12 +251,10 @@ func (s *DockerAuthzSuite) TestAuthZPluginAllowEventStream(c *check.C) { testRequires(c, DaemonIsLinux) // start the daemon and load busybox to avoid pulling busybox from Docker Hub - c.Assert(s.d.StartWithBusybox(), check.IsNil) - // restart the daemon and enable the authorization plugin, otherwise busybox loading - // is blocked by the plugin itself - c.Assert(s.d.Restart("--authorization-plugin="+testAuthZPlugin), check.IsNil) + c.Assert(s.d.Start("--authorization-plugin="+testAuthZPlugin), check.IsNil) s.ctrl.reqRes.Allow = true s.ctrl.resRes.Allow = true + c.Assert(s.d.LoadBusybox(), check.IsNil) startTime := strconv.FormatInt(daemonTime(c).Unix(), 10) // Add another command to to enable event pipelining diff --git a/components/engine/integration-cli/docker_utils.go b/components/engine/integration-cli/docker_utils.go index e4f05c0b54..454372cc80 100644 --- a/components/engine/integration-cli/docker_utils.go +++ b/components/engine/integration-cli/docker_utils.go @@ -321,24 +321,7 @@ func (d *Daemon) StartWithBusybox(arg ...string) error { if err := d.Start(arg...); err != nil { return err } - bb := filepath.Join(d.folder, "busybox.tar") - if _, err := os.Stat(bb); err != nil { - if !os.IsNotExist(err) { - return fmt.Errorf("unexpected error on busybox.tar stat: %v", err) - } - // saving busybox image from main daemon - if err := exec.Command(dockerBinary, "save", "--output", bb, "busybox:latest").Run(); err != nil { - return fmt.Errorf("could not save busybox image: %v", err) - } - } - // loading busybox image to this daemon - if out, err := d.Cmd("load", "--input", bb); err != nil { - return fmt.Errorf("could not load busybox image: %s", out) - } - if err := os.Remove(bb); err != nil { - d.c.Logf("could not remove %s: %v", bb, err) - } - return nil + return d.LoadBusybox() } // Stop will send a SIGINT every second and wait for the daemon to stop. @@ -413,6 +396,28 @@ func (d *Daemon) Restart(arg ...string) error { return d.Start(arg...) } +// LoadBusybox will load the stored busybox into a newly started daemon +func (d *Daemon) LoadBusybox() error { + bb := filepath.Join(d.folder, "busybox.tar") + if _, err := os.Stat(bb); err != nil { + if !os.IsNotExist(err) { + return fmt.Errorf("unexpected error on busybox.tar stat: %v", err) + } + // saving busybox image from main daemon + if err := exec.Command(dockerBinary, "save", "--output", bb, "busybox:latest").Run(); err != nil { + return fmt.Errorf("could not save busybox image: %v", err) + } + } + // loading busybox image to this daemon + if out, err := d.Cmd("load", "--input", bb); err != nil { + return fmt.Errorf("could not load busybox image: %s", out) + } + if err := os.Remove(bb); err != nil { + d.c.Logf("could not remove %s: %v", bb, err) + } + return nil +} + func (d *Daemon) queryRootDir() (string, error) { // update daemon root by asking /info endpoint (to support user // namespaced daemon with root remapped uid.gid directory)