From 1a51898d2ea45a8f6e0d349878ea410782a24208 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Fri, 28 Jul 2023 22:15:10 +0000 Subject: [PATCH] TestDaemonProxy: use new scanners to check logs Also fixes up some cleanup issues. Signed-off-by: Brian Goff Signed-off-by: Sebastiaan van Stijn --- integration/daemon/daemon_test.go | 142 +++++++++++++++++------------- testutil/daemon/daemon.go | 46 ++++++++-- 2 files changed, 117 insertions(+), 71 deletions(-) diff --git a/integration/daemon/daemon_test.go b/integration/daemon/daemon_test.go index 78d13d1256c23..f4565c9616766 100644 --- a/integration/daemon/daemon_test.go +++ b/integration/daemon/daemon_test.go @@ -9,7 +9,6 @@ import ( "os/exec" "path/filepath" "runtime" - "strings" "syscall" "testing" @@ -170,27 +169,34 @@ func TestDaemonProxy(t *testing.T) { skip.If(t, runtime.GOOS == "windows", "cannot start multiple daemons on windows") skip.If(t, os.Getenv("DOCKER_ROOTLESS") != "", "cannot connect to localhost proxy in rootless environment") - var received string - proxyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - received = r.Host - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte("OK")) - })) - defer proxyServer.Close() + newProxy := func(rcvd *string, t *testing.T) *httptest.Server { + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + *rcvd = r.Host + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte("OK")) + })) + t.Cleanup(s.Close) + return s + } const userPass = "myuser:mypassword@" // Configure proxy through env-vars t.Run("environment variables", func(t *testing.T) { - t.Setenv("HTTP_PROXY", proxyServer.URL) - t.Setenv("HTTPS_PROXY", proxyServer.URL) - t.Setenv("NO_PROXY", "example.com") + t.Parallel() - d := daemon.New(t) - c := d.NewClientT(t) - defer func() { _ = c.Close() }() ctx := context.Background() - d.Start(t) + var received string + proxyServer := newProxy(&received, t) + + d := daemon.New(t, daemon.WithEnvVars( + "HTTP_PROXY="+proxyServer.URL, + "HTTPS_PROXY="+proxyServer.URL, + "NO_PROXY=example.com", + )) + c := d.NewClientT(t) + + d.Start(t, "--iptables=false") defer d.Stop(t) info := d.Info(t) @@ -210,35 +216,45 @@ func TestDaemonProxy(t *testing.T) { // Configure proxy through command-line flags t.Run("command-line options", func(t *testing.T) { - t.Setenv("HTTP_PROXY", "http://"+userPass+"from-env-http.invalid") - t.Setenv("http_proxy", "http://"+userPass+"from-env-http.invalid") - t.Setenv("HTTPS_PROXY", "https://"+userPass+"myuser:mypassword@from-env-https.invalid") - t.Setenv("https_proxy", "https://"+userPass+"myuser:mypassword@from-env-https.invalid") - t.Setenv("NO_PROXY", "ignore.invalid") - t.Setenv("no_proxy", "ignore.invalid") + t.Parallel() - d := daemon.New(t) - d.Start(t, "--http-proxy", proxyServer.URL, "--https-proxy", proxyServer.URL, "--no-proxy", "example.com") + ctx := context.Background() + var received string + proxyServer := newProxy(&received, t) + + d := daemon.New(t, daemon.WithEnvVars( + "HTTP_PROXY="+"http://"+userPass+"from-env-http.invalid", + "http_proxy="+"http://"+userPass+"from-env-http.invalid", + "HTTPS_PROXY="+"https://"+userPass+"myuser:mypassword@from-env-https-invalid", + "https_proxy="+"https://"+userPass+"myuser:mypassword@from-env-https-invalid", + "NO_PROXY=ignore.invalid", + "no_proxy=ignore.invalid", + )) + d.Start(t, "--iptables=false", "--http-proxy", proxyServer.URL, "--https-proxy", proxyServer.URL, "--no-proxy", "example.com") defer d.Stop(t) - logs, err := d.ReadLogFile() - assert.NilError(t, err) - assert.Assert(t, is.Contains(string(logs), "overriding existing proxy variable with value from configuration")) - for _, v := range []string{"http_proxy", "HTTP_PROXY", "https_proxy", "HTTPS_PROXY", "no_proxy", "NO_PROXY"} { - assert.Assert(t, is.Contains(string(logs), "name="+v)) - assert.Assert(t, !strings.Contains(string(logs), userPass), "logs should not contain the non-sanitized proxy URL: %s", string(logs)) - } - c := d.NewClientT(t) - defer func() { _ = c.Close() }() - ctx := context.Background() info := d.Info(t) assert.Check(t, is.Equal(info.HTTPProxy, proxyServer.URL)) assert.Check(t, is.Equal(info.HTTPSProxy, proxyServer.URL)) assert.Check(t, is.Equal(info.NoProxy, "example.com")) - _, err = c.ImagePull(ctx, "example.org:5001/some/image:latest", types.ImagePullOptions{}) + ok, _ := d.ScanLogsT(ctx, t, daemon.ScanLogsMatchAll( + "overriding existing proxy variable with value from configuration", + "http_proxy", + "HTTP_PROXY", + "https_proxy", + "HTTPS_PROXY", + "no_proxy", + "NO_PROXY", + )) + assert.Assert(t, ok) + + ok, logs := d.ScanLogsT(ctx, t, daemon.ScanLogsMatchString(userPass)) + assert.Assert(t, !ok, "logs should not contain the non-sanitized proxy URL: %s", logs) + + _, err := c.ImagePull(ctx, "example.org:5001/some/image:latest", types.ImagePullOptions{}) assert.ErrorContains(t, err, "", "pulling should have failed") assert.Equal(t, received, "example.org:5001") @@ -250,23 +266,27 @@ func TestDaemonProxy(t *testing.T) { // Configure proxy through configuration file t.Run("configuration file", func(t *testing.T) { - t.Setenv("HTTP_PROXY", "http://"+userPass+"from-env-http.invalid") - t.Setenv("http_proxy", "http://"+userPass+"from-env-http.invalid") - t.Setenv("HTTPS_PROXY", "https://"+userPass+"myuser:mypassword@from-env-https.invalid") - t.Setenv("https_proxy", "https://"+userPass+"myuser:mypassword@from-env-https.invalid") - t.Setenv("NO_PROXY", "ignore.invalid") - t.Setenv("no_proxy", "ignore.invalid") + t.Parallel() + ctx := context.Background() - d := daemon.New(t) + var received string + proxyServer := newProxy(&received, t) + + d := daemon.New(t, daemon.WithEnvVars( + "HTTP_PROXY="+"http://"+userPass+"from-env-http.invalid", + "http_proxy="+"http://"+userPass+"from-env-http.invalid", + "HTTPS_PROXY="+"https://"+userPass+"myuser:mypassword@from-env-https-invalid", + "https_proxy="+"https://"+userPass+"myuser:mypassword@from-env-https-invalid", + "NO_PROXY=ignore.invalid", + "no_proxy=ignore.invalid", + )) c := d.NewClientT(t) - defer func() { _ = c.Close() }() - ctx := context.Background() configFile := filepath.Join(d.RootDir(), "daemon.json") configJSON := fmt.Sprintf(`{"proxies":{"http-proxy":%[1]q, "https-proxy": %[1]q, "no-proxy": "example.com"}}`, proxyServer.URL) assert.NilError(t, os.WriteFile(configFile, []byte(configJSON), 0644)) - d.Start(t, "--config-file", configFile) + d.Start(t, "--iptables=false", "--config-file", configFile) defer d.Stop(t) info := d.Info(t) @@ -274,15 +294,17 @@ func TestDaemonProxy(t *testing.T) { assert.Check(t, is.Equal(info.HTTPSProxy, proxyServer.URL)) assert.Check(t, is.Equal(info.NoProxy, "example.com")) - logs, err := d.ReadLogFile() - assert.NilError(t, err) - assert.Assert(t, is.Contains(string(logs), "overriding existing proxy variable with value from configuration")) - for _, v := range []string{"http_proxy", "HTTP_PROXY", "https_proxy", "HTTPS_PROXY", "no_proxy", "NO_PROXY"} { - assert.Assert(t, is.Contains(string(logs), "name="+v)) - assert.Assert(t, !strings.Contains(string(logs), userPass), "logs should not contain the non-sanitized proxy URL: %s", string(logs)) - } - - _, err = c.ImagePull(ctx, "example.org:5002/some/image:latest", types.ImagePullOptions{}) + d.ScanLogsT(ctx, t, daemon.ScanLogsMatchAll( + "overriding existing proxy variable with value from configuration", + "http_proxy", + "HTTP_PROXY", + "https_proxy", + "HTTPS_PROXY", + "no_proxy", + "NO_PROXY", + )) + + _, err := c.ImagePull(ctx, "example.org:5002/some/image:latest", types.ImagePullOptions{}) assert.ErrorContains(t, err, "", "pulling should have failed") assert.Equal(t, received, "example.org:5002") @@ -290,12 +312,11 @@ func TestDaemonProxy(t *testing.T) { _, err = c.ImagePull(ctx, "example.com/some/image:latest", types.ImagePullOptions{}) assert.ErrorContains(t, err, "", "pulling should have failed") assert.Equal(t, received, "example.org:5002", "should not have used proxy") - - d.Stop(t) }) // Conflicting options (passed both through command-line options and config file) t.Run("conflicting options", func(t *testing.T) { + ctx := context.Background() const ( proxyRawURL = "https://" + userPass + "example.org" proxyURL = "https://xxxxx:xxxxx@example.org" @@ -309,13 +330,12 @@ func TestDaemonProxy(t *testing.T) { err := d.StartWithError("--http-proxy", proxyRawURL, "--https-proxy", proxyRawURL, "--no-proxy", "example.com", "--config-file", configFile, "--validate") assert.ErrorContains(t, err, "daemon exited during startup") - logs, err := d.ReadLogFile() - assert.NilError(t, err) + expected := fmt.Sprintf( `the following directives are specified both as a flag and in the configuration file: http-proxy: (from flag: %[1]s, from file: %[1]s), https-proxy: (from flag: %[1]s, from file: %[1]s), no-proxy: (from flag: example.com, from file: example.com)`, proxyURL, ) - assert.Assert(t, is.Contains(string(logs), expected)) + poll.WaitOn(t, d.PollCheckLogs(ctx, daemon.ScanLogsMatchString(expected))) }) // Make sure values are sanitized when reloading the daemon-config @@ -334,11 +354,9 @@ func TestDaemonProxy(t *testing.T) { err := d.Signal(syscall.SIGHUP) assert.NilError(t, err) - poll.WaitOn(t, d.PollCheckLogs(ctx, "Reloaded configuration:")) - poll.WaitOn(t, d.PollCheckLogs(ctx, proxyURL)) + poll.WaitOn(t, d.PollCheckLogs(ctx, daemon.ScanLogsMatchAll("Reloaded configuration:", proxyURL))) - ok, logs, err := d.ScanLogs(ctx, userPass) - assert.NilError(t, err) + ok, logs := d.ScanLogsT(ctx, t, daemon.ScanLogsMatchString(userPass)) assert.Assert(t, !ok, "logs should not contain the non-sanitized proxy URL: %s", logs) }) } diff --git a/testutil/daemon/daemon.go b/testutil/daemon/daemon.go index 2cd7a28b4d134..fc489f5786964 100644 --- a/testutil/daemon/daemon.go +++ b/testutil/daemon/daemon.go @@ -277,6 +277,7 @@ func (d *Daemon) NewClientT(t testing.TB, extraOpts ...client.Opt) *client.Clien c, err := d.NewClient(extraOpts...) assert.NilError(t, err, "[%s] could not create daemon client", d.id) + t.Cleanup(func() { c.Close() }) return c } @@ -313,23 +314,51 @@ func (d *Daemon) TailLogsT(t LogT, n int) { } } -// PollCheckLogs is a poll.Check that checks the daemon logs for the passed in string (`contains`). -func (d *Daemon) PollCheckLogs(ctx context.Context, contains string) poll.Check { +// PollCheckLogs is a poll.Check that checks the daemon logs using the passed in match function. +func (d *Daemon) PollCheckLogs(ctx context.Context, match func(s string) bool) poll.Check { return func(t poll.LogT) poll.Result { - ok, _, err := d.ScanLogs(ctx, contains) + ok, _, err := d.ScanLogs(ctx, match) if err != nil { return poll.Error(err) } if !ok { - return poll.Continue("waiting for %q in daemon logs", contains) + return poll.Continue("waiting for daemon logs match") } return poll.Success() } } -// ScanLogs scans the daemon logs for the passed in string (`contains`). -// If the context is canceled, the function returns false but does not error out the test. -func (d *Daemon) ScanLogs(ctx context.Context, contains string) (bool, string, error) { +// ScanLogsMatchString returns a function that can be used to scan the daemon logs for the passed in string (`contains`). +func ScanLogsMatchString(contains string) func(string) bool { + return func(line string) bool { + return strings.Contains(line, contains) + } +} + +// ScanLogsMatchAll returns a function that can be used to scan the daemon logs until *all* the passed in strings are matched +func ScanLogsMatchAll(contains ...string) func(string) bool { + matched := make(map[string]bool) + return func(line string) bool { + for _, c := range contains { + if strings.Contains(line, c) { + matched[c] = true + } + } + return len(matched) == len(contains) + } +} + +// ScanLogsT uses `ScanLogs` to match the daemon logs using the passed in match function. +// If there is an error or the match fails, the test will fail. +func (d *Daemon) ScanLogsT(ctx context.Context, t testing.TB, match func(s string) bool) (bool, string) { + t.Helper() + ok, line, err := d.ScanLogs(ctx, match) + assert.NilError(t, err) + return ok, line +} + +// ScanLogs scans the daemon logs and passes each line to the match function. +func (d *Daemon) ScanLogs(ctx context.Context, match func(s string) bool) (bool, string, error) { stat, err := d.logFile.Stat() if err != nil { return false, "", err @@ -338,7 +367,7 @@ func (d *Daemon) ScanLogs(ctx context.Context, contains string) (bool, string, e scanner := bufio.NewScanner(rdr) for scanner.Scan() { - if strings.Contains(scanner.Text(), contains) { + if match(scanner.Text()) { return true, scanner.Text(), nil } select { @@ -364,7 +393,6 @@ func (d *Daemon) TailLogs(n int) ([][]byte, error) { } return lines, nil - } // Start starts the daemon and return once it is ready to receive requests.