From c98e9c47ca004f03b8df5a51bcfc3fab464e67c1 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 30 Apr 2021 09:31:41 +0200 Subject: [PATCH 1/4] Use designated test domains (RFC2606) in tests Some tests were using domain names that were intended to be "fake", but are actually registered domain names (such as mycorp.com). Even though we were not actually making connections to these domains, it's better to use domains that are designated for testing/examples in RFC2606: https://tools.ietf.org/html/rfc2606 Signed-off-by: Sebastiaan van Stijn (cherry picked from commit f3886f354a918d61933d648db10b128a5f6401b1) Signed-off-by: Sebastiaan van Stijn --- cli/command/cli_test.go | 8 +++---- cli/command/container/create_test.go | 2 +- cli/command/context/create_test.go | 6 ++--- cli/command/context/list_test.go | 2 +- cli/command/context/testdata/inspect.golden | 4 ++-- cli/command/context/testdata/list.golden | 10 ++++---- cli/command/context/testdata/test-kubeconfig | 2 +- cli/command/image/trust_test.go | 6 ++--- cli/command/registry_test.go | 4 ++-- cli/command/swarm/ca_test.go | 14 ++++++------ cli/config/configfile/file_test.go | 24 ++++++++++---------- cli/config/credentials/file_store_test.go | 4 ++-- cli/connhelper/connhelper.go | 4 ++-- 13 files changed, 45 insertions(+), 45 deletions(-) diff --git a/cli/command/cli_test.go b/cli/command/cli_test.go index 65cacdc61d27..4099e7e7bb36 100644 --- a/cli/command/cli_test.go +++ b/cli/command/cli_test.go @@ -81,8 +81,8 @@ func TestNewAPIClientFromFlagsWithAPIVersionFromEnv(t *testing.T) { } func TestNewAPIClientFromFlagsWithHttpProxyEnv(t *testing.T) { - defer env.Patch(t, "HTTP_PROXY", "http://proxy.acme.com:1234")() - defer env.Patch(t, "DOCKER_HOST", "tcp://docker.acme.com:2376")() + 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{} @@ -91,11 +91,11 @@ func TestNewAPIClientFromFlagsWithHttpProxyEnv(t *testing.T) { 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.com:2376", 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.com:1234", url.String())) + assert.Check(t, is.Equal("http://proxy.acme.example.com:1234", url.String())) } type fakeClient struct { diff --git a/cli/command/container/create_test.go b/cli/command/container/create_test.go index 15a9c4174915..54a83ce75d44 100644 --- a/cli/command/container/create_test.go +++ b/cli/command/container/create_test.go @@ -133,7 +133,7 @@ func TestCreateContainerImagePullPolicy(t *testing.T) { return ioutil.NopCloser(strings.NewReader("")), nil }, infoFunc: func() (types.Info, error) { - return types.Info{IndexServerAddress: "http://indexserver"}, nil + return types.Info{IndexServerAddress: "https://indexserver.example.com"}, nil }, } cli := test.NewFakeCli(client) diff --git a/cli/command/context/create_test.go b/cli/command/context/create_test.go index 916672daee6a..afdd8bef3f23 100644 --- a/cli/command/context/create_test.go +++ b/cli/command/context/create_test.go @@ -169,7 +169,7 @@ func validateTestKubeEndpoint(t *testing.T, s store.Reader, name string) { kubeMeta := ctxMetadata.Endpoints[kubernetes.KubernetesEndpoint].(kubernetes.EndpointMeta) kubeEP, err := kubeMeta.WithTLSData(s, name) assert.NilError(t, err) - assert.Equal(t, "https://someserver", kubeEP.Host) + assert.Equal(t, "https://someserver.example.com", kubeEP.Host) assert.Equal(t, "the-ca", string(kubeEP.TLSData.CA)) assert.Equal(t, "the-cert", string(kubeEP.TLSData.Cert)) assert.Equal(t, "the-key", string(kubeEP.TLSData.Key)) @@ -287,7 +287,7 @@ func TestCreateFromContext(t *testing.T) { assert.Equal(t, newContextTyped.Description, c.expectedDescription) assert.Equal(t, newContextTyped.StackOrchestrator, c.expectedOrchestrator) assert.Equal(t, dockerEndpoint.Host, "tcp://42.42.42.42:2375") - assert.Equal(t, kubeEndpoint.Host, "https://someserver") + assert.Equal(t, kubeEndpoint.Host, "https://someserver.example.com") }) } } @@ -361,7 +361,7 @@ func TestCreateFromCurrent(t *testing.T) { assert.Equal(t, newContextTyped.Description, c.expectedDescription) assert.Equal(t, newContextTyped.StackOrchestrator, c.expectedOrchestrator) assert.Equal(t, dockerEndpoint.Host, "tcp://42.42.42.42:2375") - assert.Equal(t, kubeEndpoint.Host, "https://someserver") + assert.Equal(t, kubeEndpoint.Host, "https://someserver.example.com") }) } } diff --git a/cli/command/context/list_test.go b/cli/command/context/list_test.go index d628493fc31b..b71a78b5a98c 100644 --- a/cli/command/context/list_test.go +++ b/cli/command/context/list_test.go @@ -18,7 +18,7 @@ func createTestContextWithKubeAndSwarm(t *testing.T, cli command.Cli, name strin DefaultStackOrchestrator: orchestrator, Description: "description of " + name, Kubernetes: map[string]string{keyFrom: "default"}, - Docker: map[string]string{keyHost: "https://someswarmserver"}, + Docker: map[string]string{keyHost: "https://someswarmserver.example.com"}, }) assert.NilError(t, err) } diff --git a/cli/command/context/testdata/inspect.golden b/cli/command/context/testdata/inspect.golden index d520b4f93ca3..5578411950ab 100644 --- a/cli/command/context/testdata/inspect.golden +++ b/cli/command/context/testdata/inspect.golden @@ -7,11 +7,11 @@ }, "Endpoints": { "docker": { - "Host": "https://someswarmserver", + "Host": "https://someswarmserver.example.com", "SkipTLSVerify": false }, "kubernetes": { - "Host": "https://someserver", + "Host": "https://someserver.example.com", "SkipTLSVerify": false, "DefaultNamespace": "default" } diff --git a/cli/command/context/testdata/list.golden b/cli/command/context/testdata/list.golden index 6909a8d7f5bf..b1a2ec7c240d 100644 --- a/cli/command/context/testdata/list.golden +++ b/cli/command/context/testdata/list.golden @@ -1,5 +1,5 @@ -NAME DESCRIPTION DOCKER ENDPOINT KUBERNETES ENDPOINT ORCHESTRATOR -current * description of current https://someswarmserver https://someserver (default) all -default Current DOCKER_HOST based configuration unix:///var/run/docker.sock swarm -other description of other https://someswarmserver https://someserver (default) all -unset description of unset https://someswarmserver https://someserver (default) +NAME DESCRIPTION DOCKER ENDPOINT KUBERNETES ENDPOINT ORCHESTRATOR +current * description of current https://someswarmserver.example.com https://someserver.example.com (default) all +default Current DOCKER_HOST based configuration unix:///var/run/docker.sock swarm +other description of other https://someswarmserver.example.com https://someserver.example.com (default) all +unset description of unset https://someswarmserver.example.com https://someserver.example.com (default) diff --git a/cli/command/context/testdata/test-kubeconfig b/cli/command/context/testdata/test-kubeconfig index f6baf8e843de..5d5c858ed8f2 100644 --- a/cli/command/context/testdata/test-kubeconfig +++ b/cli/command/context/testdata/test-kubeconfig @@ -2,7 +2,7 @@ apiVersion: v1 clusters: - cluster: certificate-authority-data: dGhlLWNh - server: https://someserver + server: https://someserver.example.com name: test-cluster contexts: - context: diff --git a/cli/command/image/trust_test.go b/cli/command/image/trust_test.go index 11b3e1b170fa..b7279dc330b7 100644 --- a/cli/command/image/trust_test.go +++ b/cli/command/image/trust_test.go @@ -15,17 +15,17 @@ import ( ) func TestENVTrustServer(t *testing.T) { - defer env.PatchAll(t, map[string]string{"DOCKER_CONTENT_TRUST_SERVER": "https://notary-test.com:5000"})() + defer env.PatchAll(t, map[string]string{"DOCKER_CONTENT_TRUST_SERVER": "https://notary-test.example.com:5000"})() indexInfo := ®istrytypes.IndexInfo{Name: "testserver"} output, err := trust.Server(indexInfo) - expectedStr := "https://notary-test.com:5000" + expectedStr := "https://notary-test.example.com:5000" if err != nil || output != expectedStr { t.Fatalf("Expected server to be %s, got %s", expectedStr, output) } } func TestHTTPENVTrustServer(t *testing.T) { - defer env.PatchAll(t, map[string]string{"DOCKER_CONTENT_TRUST_SERVER": "http://notary-test.com:5000"})() + defer env.PatchAll(t, map[string]string{"DOCKER_CONTENT_TRUST_SERVER": "http://notary-test.example.com:5000"})() indexInfo := ®istrytypes.IndexInfo{Name: "testserver"} _, err := trust.Server(indexInfo) if err == nil { diff --git a/cli/command/registry_test.go b/cli/command/registry_test.go index ae7c3ac052f0..ba6a17a0550c 100644 --- a/cli/command/registry_test.go +++ b/cli/command/registry_test.go @@ -66,10 +66,10 @@ func TestElectAuthServer(t *testing.T) { }, }, { - expectedAuthServer: "https://foo.bar", + expectedAuthServer: "https://foo.example.com", expectedWarning: "", infoFunc: func() (types.Info, error) { - return types.Info{IndexServerAddress: "https://foo.bar"}, nil + return types.Info{IndexServerAddress: "https://foo.example.com"}, nil }, }, { diff --git a/cli/command/swarm/ca_test.go b/cli/command/swarm/ca_test.go index e9fafcaf71f0..1f65647c5880 100644 --- a/cli/command/swarm/ca_test.go +++ b/cli/command/swarm/ca_test.go @@ -104,20 +104,20 @@ func TestDisplayTrustRootInvalidFlags(t *testing.T) { errorMsg: "flag requires the `--rotate` flag to update the CA", }, { - args: []string{"--external-ca=protocol=cfssl,url=https://some.com/https/url"}, + args: []string{"--external-ca=protocol=cfssl,url=https://some.example.com/https/url"}, errorMsg: "flag requires the `--rotate` flag to update the CA", }, { // to make sure we're not erroring because we didn't provide a CA cert and external CA args: []string{ "--ca-cert=" + tmpfile, - "--external-ca=protocol=cfssl,url=https://some.com/https/url", + "--external-ca=protocol=cfssl,url=https://some.example.com/https/url", }, errorMsg: "flag requires the `--rotate` flag to update the CA", }, { args: []string{ "--rotate", - "--external-ca=protocol=cfssl,url=https://some.com/https/url", + "--external-ca=protocol=cfssl,url=https://some.example.com/https/url", }, errorMsg: "rotating to an external CA requires the `--ca-cert` flag to specify the external CA's cert - " + "to add an external CA with the current root CA certificate, use the `update` command instead", @@ -243,7 +243,7 @@ func TestUpdateSwarmSpecCertAndExternalCA(t *testing.T) { "--rotate", "--detach", "--ca-cert=" + certfile, - "--external-ca=protocol=cfssl,url=https://some.external.ca"}) + "--external-ca=protocol=cfssl,url=https://some.external.ca.example.com"}) cmd.SetOut(cli.OutBuffer()) assert.NilError(t, cmd.Execute()) @@ -253,7 +253,7 @@ func TestUpdateSwarmSpecCertAndExternalCA(t *testing.T) { expected.CAConfig.ExternalCAs = []*swarm.ExternalCA{ { Protocol: swarm.ExternalCAProtocolCFSSL, - URL: "https://some.external.ca", + URL: "https://some.external.ca.example.com", CACert: cert, Options: make(map[string]string), }, @@ -281,7 +281,7 @@ func TestUpdateSwarmSpecCertAndKeyAndExternalCA(t *testing.T) { "--detach", "--ca-cert=" + certfile, "--ca-key=" + keyfile, - "--external-ca=protocol=cfssl,url=https://some.external.ca"}) + "--external-ca=protocol=cfssl,url=https://some.external.ca.example.com"}) cmd.SetOut(cli.OutBuffer()) assert.NilError(t, cmd.Execute()) @@ -291,7 +291,7 @@ func TestUpdateSwarmSpecCertAndKeyAndExternalCA(t *testing.T) { expected.CAConfig.ExternalCAs = []*swarm.ExternalCA{ { Protocol: swarm.ExternalCAProtocolCFSSL, - URL: "https://some.external.ca", + URL: "https://some.external.ca.example.com", CACert: cert, Options: make(map[string]string), }, diff --git a/cli/config/configfile/file_test.go b/cli/config/configfile/file_test.go index 0ed7b73fdd04..f174bf739eb5 100644 --- a/cli/config/configfile/file_test.go +++ b/cli/config/configfile/file_test.go @@ -27,10 +27,10 @@ func TestEncodeAuth(t *testing.T) { } func TestProxyConfig(t *testing.T) { - httpProxy := "http://proxy.mycorp.com:3128" - httpsProxy := "https://user:password@proxy.mycorp.com:3129" - ftpProxy := "http://ftpproxy.mycorp.com:21" - noProxy := "*.intra.mycorp.com" + httpProxy := "http://proxy.mycorp.example.com:3128" + httpsProxy := "https://user:password@proxy.mycorp.example.com:3129" + ftpProxy := "http://ftpproxy.mycorp.example.com:21" + noProxy := "*.intra.mycorp.example.com" defaultProxyConfig := ProxyConfig{ HTTPProxy: httpProxy, HTTPSProxy: httpsProxy, @@ -59,12 +59,12 @@ func TestProxyConfig(t *testing.T) { } func TestProxyConfigOverride(t *testing.T) { - httpProxy := "http://proxy.mycorp.com:3128" + httpProxy := "http://proxy.mycorp.example.com:3128" overrideHTTPProxy := "http://proxy.example.com:3128" overrideNoProxy := "" - httpsProxy := "https://user:password@proxy.mycorp.com:3129" - ftpProxy := "http://ftpproxy.mycorp.com:21" - noProxy := "*.intra.mycorp.com" + httpsProxy := "https://user:password@proxy.mycorp.example.com:3129" + ftpProxy := "http://ftpproxy.mycorp.example.com:21" + noProxy := "*.intra.mycorp.example.com" defaultProxyConfig := ProxyConfig{ HTTPProxy: httpProxy, HTTPSProxy: httpsProxy, @@ -102,10 +102,10 @@ func TestProxyConfigOverride(t *testing.T) { } func TestProxyConfigPerHost(t *testing.T) { - httpProxy := "http://proxy.mycorp.com:3128" - httpsProxy := "https://user:password@proxy.mycorp.com:3129" - ftpProxy := "http://ftpproxy.mycorp.com:21" - noProxy := "*.intra.mycorp.com" + httpProxy := "http://proxy.mycorp.example.com:3128" + httpsProxy := "https://user:password@proxy.mycorp.example.com:3129" + ftpProxy := "http://ftpproxy.mycorp.example.com:21" + noProxy := "*.intra.mycorp.example.com" extHTTPProxy := "http://proxy.example.com:3128" extHTTPSProxy := "https://user:password@proxy.example.com:3129" diff --git a/cli/config/credentials/file_store_test.go b/cli/config/credentials/file_store_test.go index 3a95f2be207b..f04fe22e1ca0 100644 --- a/cli/config/credentials/file_store_test.go +++ b/cli/config/credentials/file_store_test.go @@ -70,7 +70,7 @@ func TestFileStoreGet(t *testing.T) { func TestFileStoreGetAll(t *testing.T) { s1 := "https://example.com" - s2 := "https://example2.com" + s2 := "https://example2.example.com" f := newStore(map[string]types.AuthConfig{ s1: { Auth: "super_secret_token", @@ -80,7 +80,7 @@ func TestFileStoreGetAll(t *testing.T) { s2: { Auth: "super_secret_token2", Email: "foo@example2.com", - ServerAddress: "https://example2.com", + ServerAddress: "https://example2.example.com", }, }) diff --git a/cli/connhelper/connhelper.go b/cli/connhelper/connhelper.go index e349b3eaefc3..9ac9d6744d45 100644 --- a/cli/connhelper/connhelper.go +++ b/cli/connhelper/connhelper.go @@ -49,7 +49,7 @@ func getConnectionHelper(daemonURL string, sshFlags []string) (*ConnectionHelper Dialer: func(ctx context.Context, network, addr string) (net.Conn, error) { return commandconn.New(ctx, "ssh", append(sshFlags, sp.Args("docker", "system", "dial-stdio")...)...) }, - Host: "http://docker", + Host: "http://docker.example.com", }, nil } // Future version may support plugins via ~/.docker/config.json. e.g. "dind" @@ -63,6 +63,6 @@ func GetCommandConnectionHelper(cmd string, flags ...string) (*ConnectionHelper, Dialer: func(ctx context.Context, network, addr string) (net.Conn, error) { return commandconn.New(ctx, cmd, flags...) }, - Host: "http://docker", + Host: "http://docker.example.com", }, nil } From 1e9575e81a41f7662c88cdc75cfeeb9070cb2023 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 30 Apr 2021 09:43:13 +0200 Subject: [PATCH 2/4] cli/config/configfile: various test cleanups - use var/const blocks when declaring a list of variables - use const where possible TestCheckKubernetesConfigurationRaiseAnErrorOnInvalidValue: - use keys when assigning values - make sure test is dereferenced in the loop - use subtests Signed-off-by: Sebastiaan van Stijn (cherry picked from commit be327a4f0fa8aa8a8da8ad47f060a10ed67bc3d7) Signed-off-by: Sebastiaan van Stijn --- cli/config/configfile/file_test.go | 181 ++++++++++++++++------------- 1 file changed, 98 insertions(+), 83 deletions(-) diff --git a/cli/config/configfile/file_test.go b/cli/config/configfile/file_test.go index f174bf739eb5..fbab2930f776 100644 --- a/cli/config/configfile/file_test.go +++ b/cli/config/configfile/file_test.go @@ -27,16 +27,19 @@ func TestEncodeAuth(t *testing.T) { } func TestProxyConfig(t *testing.T) { - httpProxy := "http://proxy.mycorp.example.com:3128" - httpsProxy := "https://user:password@proxy.mycorp.example.com:3129" - ftpProxy := "http://ftpproxy.mycorp.example.com:21" - noProxy := "*.intra.mycorp.example.com" - defaultProxyConfig := ProxyConfig{ - HTTPProxy: httpProxy, - HTTPSProxy: httpsProxy, - FTPProxy: ftpProxy, - NoProxy: noProxy, - } + var ( + httpProxy = "http://proxy.mycorp.example.com:3128" + httpsProxy = "https://user:password@proxy.mycorp.example.com:3129" + ftpProxy = "http://ftpproxy.mycorp.example.com:21" + noProxy = "*.intra.mycorp.example.com" + + defaultProxyConfig = ProxyConfig{ + HTTPProxy: httpProxy, + HTTPSProxy: httpsProxy, + FTPProxy: ftpProxy, + NoProxy: noProxy, + } + ) cfg := ConfigFile{ Proxies: map[string]ProxyConfig{ @@ -59,18 +62,21 @@ func TestProxyConfig(t *testing.T) { } func TestProxyConfigOverride(t *testing.T) { - httpProxy := "http://proxy.mycorp.example.com:3128" - overrideHTTPProxy := "http://proxy.example.com:3128" - overrideNoProxy := "" - httpsProxy := "https://user:password@proxy.mycorp.example.com:3129" - ftpProxy := "http://ftpproxy.mycorp.example.com:21" - noProxy := "*.intra.mycorp.example.com" - defaultProxyConfig := ProxyConfig{ - HTTPProxy: httpProxy, - HTTPSProxy: httpsProxy, - FTPProxy: ftpProxy, - NoProxy: noProxy, - } + var ( + httpProxy = "http://proxy.mycorp.example.com:3128" + httpProxyOverride = "http://proxy.example.com:3128" + httpsProxy = "https://user:password@proxy.mycorp.example.com:3129" + ftpProxy = "http://ftpproxy.mycorp.example.com:21" + noProxy = "*.intra.mycorp.example.com" + noProxyOverride = "" + + defaultProxyConfig = ProxyConfig{ + HTTPProxy: httpProxy, + HTTPSProxy: httpsProxy, + FTPProxy: ftpProxy, + NoProxy: noProxy, + } + ) cfg := ConfigFile{ Proxies: map[string]ProxyConfig{ @@ -84,46 +90,49 @@ func TestProxyConfigOverride(t *testing.T) { } ropts := map[string]*string{ - "HTTP_PROXY": clone(overrideHTTPProxy), - "NO_PROXY": clone(overrideNoProxy), + "HTTP_PROXY": clone(httpProxyOverride), + "NO_PROXY": clone(noProxyOverride), } proxyConfig := cfg.ParseProxyConfig("/var/run/docker.sock", ropts) expected := map[string]*string{ - "HTTP_PROXY": &overrideHTTPProxy, + "HTTP_PROXY": &httpProxyOverride, "http_proxy": &httpProxy, "HTTPS_PROXY": &httpsProxy, "https_proxy": &httpsProxy, "FTP_PROXY": &ftpProxy, "ftp_proxy": &ftpProxy, - "NO_PROXY": &overrideNoProxy, + "NO_PROXY": &noProxyOverride, "no_proxy": &noProxy, } assert.Check(t, is.DeepEqual(expected, proxyConfig)) } func TestProxyConfigPerHost(t *testing.T) { - httpProxy := "http://proxy.mycorp.example.com:3128" - httpsProxy := "https://user:password@proxy.mycorp.example.com:3129" - ftpProxy := "http://ftpproxy.mycorp.example.com:21" - noProxy := "*.intra.mycorp.example.com" - - extHTTPProxy := "http://proxy.example.com:3128" - extHTTPSProxy := "https://user:password@proxy.example.com:3129" - extFTPProxy := "http://ftpproxy.example.com:21" - extNoProxy := "*.intra.example.com" - - defaultProxyConfig := ProxyConfig{ - HTTPProxy: httpProxy, - HTTPSProxy: httpsProxy, - FTPProxy: ftpProxy, - NoProxy: noProxy, - } - externalProxyConfig := ProxyConfig{ - HTTPProxy: extHTTPProxy, - HTTPSProxy: extHTTPSProxy, - FTPProxy: extFTPProxy, - NoProxy: extNoProxy, - } + var ( + httpProxy = "http://proxy.mycorp.example.com:3128" + httpsProxy = "https://user:password@proxy.mycorp.example.com:3129" + ftpProxy = "http://ftpproxy.mycorp.example.com:21" + noProxy = "*.intra.mycorp.example.com" + + extHTTPProxy = "http://proxy.example.com:3128" + extHTTPSProxy = "https://user:password@proxy.example.com:3129" + extFTPProxy = "http://ftpproxy.example.com:21" + extNoProxy = "*.intra.example.com" + + defaultProxyConfig = ProxyConfig{ + HTTPProxy: httpProxy, + HTTPSProxy: httpsProxy, + FTPProxy: ftpProxy, + NoProxy: noProxy, + } + + externalProxyConfig = ProxyConfig{ + HTTPProxy: extHTTPProxy, + HTTPSProxy: extHTTPSProxy, + FTPProxy: extFTPProxy, + NoProxy: extNoProxy, + } + ) cfg := ConfigFile{ Proxies: map[string]ProxyConfig{ @@ -226,9 +235,11 @@ func TestGetAllCredentialsCredsStore(t *testing.T) { } func TestGetAllCredentialsCredHelper(t *testing.T) { - testCredHelperSuffix := "test_cred_helper" - testCredHelperRegistryHostname := "credhelper.com" - testExtraCredHelperRegistryHostname := "somethingweird.com" + const ( + testCredHelperSuffix = "test_cred_helper" + testCredHelperRegistryHostname = "credhelper.com" + testExtraCredHelperRegistryHostname = "somethingweird.com" + ) unexpectedCredHelperAuth := types.AuthConfig{ Username: "file_store_user", @@ -265,9 +276,11 @@ func TestGetAllCredentialsCredHelper(t *testing.T) { } func TestGetAllCredentialsFileStoreAndCredHelper(t *testing.T) { - testFileStoreRegistryHostname := "example.com" - testCredHelperSuffix := "test_cred_helper" - testCredHelperRegistryHostname := "credhelper.com" + const ( + testFileStoreRegistryHostname = "example.com" + testCredHelperSuffix = "test_cred_helper" + testCredHelperRegistryHostname = "credhelper.com" + ) expectedFileStoreAuth := types.AuthConfig{ Username: "file_store_user", @@ -301,10 +314,12 @@ func TestGetAllCredentialsFileStoreAndCredHelper(t *testing.T) { } func TestGetAllCredentialsCredStoreAndCredHelper(t *testing.T) { - testCredStoreSuffix := "test_creds_store" - testCredStoreRegistryHostname := "credstore.com" - testCredHelperSuffix := "test_cred_helper" - testCredHelperRegistryHostname := "credhelper.com" + const ( + testCredStoreSuffix = "test_creds_store" + testCredStoreRegistryHostname = "credstore.com" + testCredHelperSuffix = "test_cred_helper" + testCredHelperRegistryHostname = "credhelper.com" + ) configFile := New("filename") configFile.CredentialsStore = testCredStoreSuffix @@ -343,9 +358,11 @@ func TestGetAllCredentialsCredStoreAndCredHelper(t *testing.T) { } func TestGetAllCredentialsCredHelperOverridesDefaultStore(t *testing.T) { - testCredStoreSuffix := "test_creds_store" - testCredHelperSuffix := "test_cred_helper" - testRegistryHostname := "example.com" + const ( + testCredStoreSuffix = "test_creds_store" + testCredHelperSuffix = "test_cred_helper" + testRegistryHostname = "example.com" + ) configFile := New("filename") configFile.CredentialsStore = testCredStoreSuffix @@ -424,38 +441,36 @@ func TestCheckKubernetesConfigurationRaiseAnErrorOnInvalidValue(t *testing.T) { expectError bool }{ { - "no kubernetes config is valid", - nil, - false, + name: "no kubernetes config is valid", }, { - "enabled is valid", - &KubernetesConfig{AllNamespaces: "enabled"}, - false, + name: "enabled is valid", + config: &KubernetesConfig{AllNamespaces: "enabled"}, }, { - "disabled is valid", - &KubernetesConfig{AllNamespaces: "disabled"}, - false, + name: "disabled is valid", + config: &KubernetesConfig{AllNamespaces: "disabled"}, }, { - "empty string is valid", - &KubernetesConfig{AllNamespaces: ""}, - false, + name: "empty string is valid", + config: &KubernetesConfig{AllNamespaces: ""}, }, { - "other value is invalid", - &KubernetesConfig{AllNamespaces: "unknown"}, - true, + name: "other value is invalid", + config: &KubernetesConfig{AllNamespaces: "unknown"}, + expectError: true, }, } - for _, test := range testCases { - err := checkKubernetesConfiguration(test.config) - if test.expectError { - assert.Assert(t, err != nil, test.name) - } else { - assert.NilError(t, err, test.name) - } + for _, tc := range testCases { + test := tc + t.Run(test.name, func(t *testing.T) { + err := checkKubernetesConfiguration(test.config) + if test.expectError { + assert.Assert(t, err != nil, test.name) + } else { + assert.NilError(t, err, test.name) + } + }) } } From 6288e8b1ac446b90ec661fe6fe9c6967c9156fa4 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 28 Jul 2021 16:26:52 +0200 Subject: [PATCH 3/4] 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 (cherry picked from commit 40c6b117e70b92edd72fac1a554db83aa8102174) 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 4099e7e7bb36..370ecec803b9 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 45bf2cf1f6b9..863079b1c32c 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") +} From 0b924e51fc31fc90a29790f054b1d2c349ffdf8f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 15 Jul 2021 15:15:38 +0200 Subject: [PATCH 4/4] Update to go1.16.6 Keeping the dockerfiles/Dockerfile.cross image at 1.13, as we don't have more current versions of that image. However, I don't think it's still used, so we should remove it. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit a477a727fccdcd392ad495076b99c359af8f8d18) Signed-off-by: Sebastiaan van Stijn --- Dockerfile | 18 +++++++++--------- appveyor.yml | 2 +- dockerfiles/Dockerfile.binary-native | 2 +- dockerfiles/Dockerfile.dev | 9 +++++---- dockerfiles/Dockerfile.e2e | 2 +- dockerfiles/Dockerfile.lint | 3 ++- scripts/build/plugins | 2 +- scripts/test/e2e/run | 1 + 8 files changed, 21 insertions(+), 18 deletions(-) diff --git a/Dockerfile b/Dockerfile index 1027df05c216..5a6752aa14ec 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,17 +1,17 @@ # syntax=docker/dockerfile:1.3 ARG BASE_VARIANT=alpine -ARG GO_VERSION=1.13.15 +ARG GO_VERSION=1.16.6 FROM --platform=$BUILDPLATFORM golang:${GO_VERSION}-${BASE_VARIANT} AS gostable -FROM --platform=$BUILDPLATFORM golang:1.16-${BASE_VARIANT} AS golatest - -FROM gostable AS go-linux -FROM golatest AS go-darwin -FROM golatest AS go-windows-amd64 -FROM golatest AS go-windows-386 -FROM golatest AS go-windows-arm -FROM --platform=$BUILDPLATFORM golang:1.17rc1-${BASE_VARIANT} AS go-windows-arm64 +FROM --platform=$BUILDPLATFORM golang:1.17rc1-${BASE_VARIANT} AS golatest + +FROM gostable AS go-linux +FROM gostable AS go-darwin +FROM gostable AS go-windows-amd64 +FROM gostable AS go-windows-386 +FROM gostable AS go-windows-arm +FROM golatest AS go-windows-arm64 FROM go-windows-${TARGETARCH} AS go-windows FROM --platform=$BUILDPLATFORM tonistiigi/xx@sha256:620d36a9d7f1e3b102a5c7e8eff12081ac363828b3a44390f24fa8da2d49383d AS xx diff --git a/appveyor.yml b/appveyor.yml index f019485c75ef..b05ffc6aebda 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -4,7 +4,7 @@ clone_folder: c:\gopath\src\github.com\docker\cli environment: GOPATH: c:\gopath - GOVERSION: 1.13.15 + GOVERSION: 1.16.6 DEPVERSION: v0.4.1 install: diff --git a/dockerfiles/Dockerfile.binary-native b/dockerfiles/Dockerfile.binary-native index 37c1c64fe8d3..7d7cc4226888 100644 --- a/dockerfiles/Dockerfile.binary-native +++ b/dockerfiles/Dockerfile.binary-native @@ -1,4 +1,4 @@ -ARG GO_VERSION=1.13.15 +ARG GO_VERSION=1.16.6 FROM golang:${GO_VERSION}-alpine diff --git a/dockerfiles/Dockerfile.dev b/dockerfiles/Dockerfile.dev index a82eb728367d..9df860c9fae4 100644 --- a/dockerfiles/Dockerfile.dev +++ b/dockerfiles/Dockerfile.dev @@ -1,6 +1,6 @@ # syntax=docker/dockerfile:1.3 -ARG GO_VERSION=1.13.15 +ARG GO_VERSION=1.16.6 FROM golang:${GO_VERSION}-alpine AS golang ENV CGO_ENABLED=0 @@ -10,21 +10,21 @@ ARG ESC_VERSION=v0.2.0 RUN --mount=type=cache,target=/root/.cache/go-build \ --mount=type=cache,target=/go/pkg/mod \ --mount=type=tmpfs,target=/go/src/ \ - GO111MODULE=on go get github.com/mjibson/esc@${ESC_VERSION} + GO111MODULE=on go install github.com/mjibson/esc@${ESC_VERSION} FROM golang AS gotestsum ARG GOTESTSUM_VERSION=v0.4.0 RUN --mount=type=cache,target=/root/.cache/go-build \ --mount=type=cache,target=/go/pkg/mod \ --mount=type=tmpfs,target=/go/src/ \ - GO111MODULE=on go get gotest.tools/gotestsum@${GOTESTSUM_VERSION} + GO111MODULE=on go install gotest.tools/gotestsum@${GOTESTSUM_VERSION} FROM golang AS vndr ARG VNDR_VERSION=v0.1.2 RUN --mount=type=cache,target=/root/.cache/go-build \ --mount=type=cache,target=/go/pkg/mod \ --mount=type=tmpfs,target=/go/src/ \ - GO111MODULE=on go get github.com/LK4D4/vndr@${VNDR_VERSION} + GO111MODULE=on go install github.com/LK4D4/vndr@${VNDR_VERSION} FROM golang AS dev RUN apk add --no-cache \ @@ -44,4 +44,5 @@ COPY --from=vndr /go/bin/* /go/bin/ COPY --from=gotestsum /go/bin/* /go/bin/ WORKDIR /go/src/github.com/docker/cli +ENV GO111MODULE=auto COPY . . diff --git a/dockerfiles/Dockerfile.e2e b/dockerfiles/Dockerfile.e2e index afd4e47b6012..36ea70b8c432 100644 --- a/dockerfiles/Dockerfile.e2e +++ b/dockerfiles/Dockerfile.e2e @@ -1,4 +1,4 @@ -ARG GO_VERSION=1.13.15 +ARG GO_VERSION=1.16.6 # Use Debian based image as docker-compose requires glibc. FROM golang:${GO_VERSION}-buster diff --git a/dockerfiles/Dockerfile.lint b/dockerfiles/Dockerfile.lint index 8c4923c1a383..6d272db22711 100644 --- a/dockerfiles/Dockerfile.lint +++ b/dockerfiles/Dockerfile.lint @@ -1,6 +1,6 @@ # syntax=docker/dockerfile:1.3 -ARG GO_VERSION=1.13.15 +ARG GO_VERSION=1.16.6 ARG GOLANGCI_LINTER_SHA="v1.21.0" FROM golang:${GO_VERSION}-alpine AS build @@ -13,6 +13,7 @@ RUN --mount=type=cache,target=/root/.cache/go-build \ go get github.com/golangci/golangci-lint/cmd/golangci-lint@${GOLANGCI_LINTER_SHA} FROM golang:${GO_VERSION}-alpine AS lint +ENV GO111MODULE=off ENV CGO_ENABLED=0 ENV DISABLE_WARN_OUTSIDE_CONTAINER=1 COPY --from=build /go/bin/golangci-lint /usr/local/bin diff --git a/scripts/build/plugins b/scripts/build/plugins index fce2689cdc25..f4e9c74600db 100755 --- a/scripts/build/plugins +++ b/scripts/build/plugins @@ -17,5 +17,5 @@ for p in cli-plugins/examples/* "$@" ; do echo "Building statically linked $TARGET" export CGO_ENABLED=0 - go build -o "${TARGET}" --ldflags "${LDFLAGS}" "github.com/docker/cli/${p}" + GO111MODULE=auto go build -o "${TARGET}" --ldflags "${LDFLAGS}" "github.com/docker/cli/${p}" done diff --git a/scripts/test/e2e/run b/scripts/test/e2e/run index a9c5cc2683ab..5befee97aa52 100755 --- a/scripts/test/e2e/run +++ b/scripts/test/e2e/run @@ -71,6 +71,7 @@ runtests() { PATH="$PWD/build/:/usr/bin:/usr/local/bin:/usr/local/go/bin" \ HOME="$HOME" \ DOCKER_CLI_E2E_PLUGINS_EXTRA_DIRS="$PWD/build/plugins-linux-amd64" \ + GO111MODULE=auto \ "$(command -v gotestsum)" -- ${TESTDIRS:-./e2e/...} ${TESTFLAGS-} }