Skip to content

Commit

Permalink
change TestNewAPIClientFromFlagsWithHttpProxyEnv to an e2e test
Browse files Browse the repository at this point in the history
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 <github@gone.nl>
(cherry picked from commit 40c6b11)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
  • Loading branch information
thaJeztah committed Jul 29, 2021
1 parent 1e9575e commit 6288e8b
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 19 deletions.
19 changes: 0 additions & 19 deletions cli/command/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"crypto/x509"
"fmt"
"io/ioutil"
"net/http"
"os"
"runtime"
"testing"
Expand Down Expand Up @@ -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)
Expand Down
43 changes: 43 additions & 0 deletions e2e/global/cli_test.go
Original file line number Diff line number Diff line change
@@ -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"
)
Expand All @@ -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")
}

0 comments on commit 6288e8b

Please sign in to comment.