Skip to content

Commit

Permalink
Merge pull request #46134 from thaJeztah/24.0_backport_fix_daemon_int…
Browse files Browse the repository at this point in the history 
…egration_test

[24.0 backport] Improve test daemon logging, and fix TestDaemonProxy integration tests
  • Loading branch information
@thaJeztah
thaJeztah committed Aug 4, 2023
2 parents 00e46f8 + 4cd50eb commit 8895c32
Show file tree
Hide file tree
Showing 2 changed files with 204 additions and 91 deletions.
195 changes: 104 additions & 91 deletions integration/daemon/daemon_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"os/exec"
"path/filepath"
"runtime"
"strings"
"syscall"
"testing"

Expand All @@ -22,6 +21,7 @@ import (
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
"gotest.tools/v3/icmd"
"gotest.tools/v3/poll"
"gotest.tools/v3/skip"
)

Expand Down Expand Up @@ -169,27 +169,40 @@ 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)
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:5000/some/image:latest", types.ImagePullOptions{})
assert.ErrorContains(t, err, "", "pulling should have failed")
Expand All @@ -199,102 +212,111 @@ 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:5000", "should not have used proxy")

info := d.Info(t)
assert.Equal(t, info.HTTPProxy, proxyServer.URL)
assert.Equal(t, info.HTTPSProxy, proxyServer.URL)
assert.Equal(t, info.NoProxy, "example.com")
d.Stop(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")

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))
}
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)

c := d.NewClientT(t)
defer func() { _ = c.Close() }()
ctx := context.Background()

_, err = c.ImagePull(ctx, "example.org:5001/some/image:latest", types.ImagePullOptions{})
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"))

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")

// Test NoProxy: example.com should not hit the proxy, and "received" variable should not be changed.
_, 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:5001", "should not have used proxy")

info := d.Info(t)
assert.Equal(t, info.HTTPProxy, proxyServer.URL)
assert.Equal(t, info.HTTPSProxy, proxyServer.URL)
assert.Equal(t, info.NoProxy, "example.com")

d.Stop(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)

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))
}
d.Start(t, "--iptables=false", "--config-file", configFile)
defer d.Stop(t)

_, err = c.ImagePull(ctx, "example.org:5002/some/image:latest", types.ImagePullOptions{})
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"))

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")

// Test NoProxy: example.com should not hit the proxy, and "received" variable should not be changed.
_, 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")

info := d.Info(t)
assert.Equal(t, info.HTTPProxy, proxyServer.URL)
assert.Equal(t, info.HTTPSProxy, proxyServer.URL)
assert.Equal(t, info.NoProxy, "example.com")

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"
Expand All @@ -308,43 +330,34 @@ 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
t.Run("reload sanitized", func(t *testing.T) {
t.Parallel()

ctx := context.Background()
const (
proxyRawURL = "https://" + userPass + "example.org"
proxyURL = "https://xxxxx:xxxxx@example.org"
)

d := daemon.New(t)
d.Start(t, "--http-proxy", proxyRawURL, "--https-proxy", proxyRawURL, "--no-proxy", "example.com")
d.Start(t, "--iptables=false", "--http-proxy", proxyRawURL, "--https-proxy", proxyRawURL, "--no-proxy", "example.com")
defer d.Stop(t)
err := d.Signal(syscall.SIGHUP)
assert.NilError(t, err)

logs, err := d.ReadLogFile()
assert.NilError(t, err)

// FIXME: there appears to ba a race condition, which causes ReadLogFile
// to not contain the full logs after signaling the daemon to reload,
// causing the test to fail here. As a workaround, check if we
// received the "reloaded" message after signaling, and only then
// check that it's sanitized properly. For more details on this
// issue, see https://github.com/moby/moby/pull/42835/files#r713120315
if !strings.Contains(string(logs), "Reloaded configuration:") {
t.Skip("Skipping test, because we did not find 'Reloaded configuration' in the logs")
}
poll.WaitOn(t, d.PollCheckLogs(ctx, daemon.ScanLogsMatchAll("Reloaded configuration:", proxyURL)))

assert.Assert(t, is.Contains(string(logs), proxyURL))
assert.Assert(t, !strings.Contains(string(logs), userPass), "logs should not contain the non-sanitized proxy URL: %s", string(logs))
ok, logs := d.ScanLogsT(ctx, t, daemon.ScanLogsMatchString(userPass))
assert.Assert(t, !ok, "logs should not contain the non-sanitized proxy URL: %s", logs)
})
}

Expand Down
Loading

0 comments on commit 8895c32

Please sign in to comment.