From 40c6b117e70b92edd72fac1a554db83aa8102174 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 28 Jul 2021 16:26:52 +0200 Subject: [PATCH] change TestNewAPIClientFromFlagsWithHttpProxyEnv to an e2e test Golang uses a `sync.Once` when determining the proxy to use. This means that it's not possible to test the proxy configuration in unit tests, because the proxy configuration will be "fixated" the first time Golang detects the proxy configuration. This patch changes TestNewAPIClientFromFlagsWithHttpProxyEnv to an e2e test so that we can verify the CLI picks up the proxy configuration. Signed-off-by: Sebastiaan van Stijn --- cli/command/cli_test.go | 19 ------------------ e2e/global/cli_test.go | 43 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/cli/command/cli_test.go b/cli/command/cli_test.go index 4099e7e7bb..370ecec803 100644 --- a/cli/command/cli_test.go +++ b/cli/command/cli_test.go @@ -6,7 +6,6 @@ import ( "crypto/x509" "fmt" "io/ioutil" - "net/http" "os" "runtime" "testing" @@ -80,24 +79,6 @@ func TestNewAPIClientFromFlagsWithAPIVersionFromEnv(t *testing.T) { assert.Check(t, is.Equal(customVersion, apiclient.ClientVersion())) } -func TestNewAPIClientFromFlagsWithHttpProxyEnv(t *testing.T) { - defer env.Patch(t, "HTTP_PROXY", "http://proxy.acme.example.com:1234")() - defer env.Patch(t, "DOCKER_HOST", "tcp://docker.acme.example.com:2376")() - - opts := &flags.CommonOptions{} - configFile := &configfile.ConfigFile{} - apiclient, err := NewAPIClientFromFlags(opts, configFile) - assert.NilError(t, err) - transport, ok := apiclient.HTTPClient().Transport.(*http.Transport) - assert.Assert(t, ok) - assert.Assert(t, transport.Proxy != nil) - request, err := http.NewRequest(http.MethodGet, "tcp://docker.acme.example.com:2376", nil) - assert.NilError(t, err) - url, err := transport.Proxy(request) - assert.NilError(t, err) - assert.Check(t, is.Equal("http://proxy.acme.example.com:1234", url.String())) -} - type fakeClient struct { client.Client pingFunc func() (types.Ping, error) diff --git a/e2e/global/cli_test.go b/e2e/global/cli_test.go index 45bf2cf1f6..863079b1c3 100644 --- a/e2e/global/cli_test.go +++ b/e2e/global/cli_test.go @@ -1,9 +1,13 @@ package global import ( + "net/http" + "net/http/httptest" + "strings" "testing" "github.com/docker/cli/internal/test/environment" + "gotest.tools/v3/assert" "gotest.tools/v3/icmd" "gotest.tools/v3/skip" ) @@ -22,3 +26,42 @@ func TestTLSVerify(t *testing.T) { result = icmd.RunCmd(icmd.Command("docker", "--tlsverify=true", "ps")) result.Assert(t, icmd.Expected{ExitCode: 1, Err: "ca.pem"}) } + +// TestTCPSchemeUsesHTTPProxyEnv verifies that the cli uses HTTP_PROXY if +// DOCKER_HOST is set to use the 'tcp://' scheme. +// +// Prior to go1.16, https:// schemes would use HTTPS_PROXY, and any other +// scheme would use HTTP_PROXY. However, golang/net@7b1cca2 (per a request in +// golang/go#40909) changed this behavior to only use HTTP_PROXY for http:// +// schemes, no longer using a proxy for any other scheme. +// +// Docker uses the tcp:// scheme as a default for API connections, to indicate +// that the API is not "purely" HTTP. Various parts in the code also *require* +// this scheme to be used. While we could change the default and allow http(s) +// schemes to be used, doing so will take time, taking into account that there +// are many installs in existence that have tcp:// configured as DOCKER_HOST. +// +// Note that due to Golang's use of sync.Once for proxy-detection, this test +// cannot be done as a unit-test, hence it being an e2e test. +func TestTCPSchemeUsesHTTPProxyEnv(t *testing.T) { + const responseJSON = `{"Version": "99.99.9", "ApiVersion": "1.41", "MinAPIVersion": "1.12"}` + 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(responseJSON)) + })) + defer proxyServer.Close() + + // Configure the CLI to use our proxyServer. DOCKER_HOST can point to any + // address (as it won't be connected to), but must use tcp:// for this test, + // to verify it's using HTTP_PROXY. + result := icmd.RunCmd( + icmd.Command("docker", "version", "--format", "{{ .Server.Version }}"), + icmd.WithEnv("HTTP_PROXY="+proxyServer.URL, "DOCKER_HOST=tcp://docker.acme.example.com:2376"), + ) + // Verify the command ran successfully, and that it connected to the proxyServer + result.Assert(t, icmd.Success) + assert.Equal(t, strings.TrimSpace(result.Stdout()), "99.99.9") + assert.Equal(t, received, "docker.acme.example.com:2376") +}