diff --git a/client/client_test.go b/client/client_test.go index 14739f2ca9..dfe32dd2f4 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -87,22 +87,21 @@ func TestNewClientWithOpsFromEnv(t *testing.T) { } defer env.PatchAll(t, nil)() - for _, c := range testcases { - env.PatchAll(t, c.envs) - apiclient, err := NewClientWithOpts(FromEnv) - if c.expectedError != "" { - assert.Check(t, is.Error(err, c.expectedError), c.doc) + for _, tc := range testcases { + env.PatchAll(t, tc.envs) + client, err := NewClientWithOpts(FromEnv) + if tc.expectedError != "" { + assert.Check(t, is.Error(err, tc.expectedError), tc.doc) } else { - assert.Check(t, err, c.doc) - version := apiclient.ClientVersion() - assert.Check(t, is.Equal(c.expectedVersion, version), c.doc) + assert.Check(t, err, tc.doc) + assert.Check(t, is.Equal(client.ClientVersion(), tc.expectedVersion), tc.doc) } - if c.envs["DOCKER_TLS_VERIFY"] != "" { + if tc.envs["DOCKER_TLS_VERIFY"] != "" { // pedantic checking that this is handled correctly - tr := apiclient.client.Transport.(*http.Transport) - assert.Assert(t, tr.TLSClientConfig != nil, c.doc) - assert.Check(t, is.Equal(tr.TLSClientConfig.InsecureSkipVerify, false), c.doc) + tr := client.client.Transport.(*http.Transport) + assert.Assert(t, tr.TLSClientConfig != nil, tc.doc) + assert.Check(t, is.Equal(tr.TLSClientConfig.InsecureSkipVerify, false), tc.doc) } } } @@ -114,9 +113,9 @@ func TestGetAPIPath(t *testing.T) { query url.Values expected string }{ - {"", "/containers/json", nil, "/containers/json"}, - {"", "/containers/json", url.Values{}, "/containers/json"}, - {"", "/containers/json", url.Values{"s": []string{"c"}}, "/containers/json?s=c"}, + {"", "/containers/json", nil, "/v" + api.DefaultVersion + "/containers/json"}, + {"", "/containers/json", url.Values{}, "/v" + api.DefaultVersion + "/containers/json"}, + {"", "/containers/json", url.Values{"s": []string{"c"}}, "/v" + api.DefaultVersion + "/containers/json?s=c"}, {"1.22", "/containers/json", nil, "/v1.22/containers/json"}, {"1.22", "/containers/json", url.Values{}, "/v1.22/containers/json"}, {"1.22", "/containers/json", url.Values{"s": []string{"c"}}, "/v1.22/containers/json?s=c"}, @@ -127,10 +126,14 @@ func TestGetAPIPath(t *testing.T) { } ctx := context.TODO() - for _, testcase := range testcases { - c := Client{version: testcase.version, basePath: "/"} - actual := c.getAPIPath(ctx, testcase.path, testcase.query) - assert.Check(t, is.Equal(actual, testcase.expected)) + for _, tc := range testcases { + client, err := NewClientWithOpts( + WithVersion(tc.version), + WithHost("tcp://localhost:2375"), + ) + assert.NilError(t, err) + actual := client.getAPIPath(ctx, tc.path, tc.query) + assert.Check(t, is.Equal(actual, tc.expected)) } } @@ -167,7 +170,7 @@ func TestParseHostURL(t *testing.T) { if testcase.expectedErr != "" { assert.Check(t, is.ErrorContains(err, testcase.expectedErr)) } - assert.Check(t, is.DeepEqual(testcase.expected, actual)) + assert.Check(t, is.DeepEqual(actual, testcase.expected)) } } @@ -183,90 +186,118 @@ func TestNewClientWithOpsFromEnvSetsDefaultVersion(t *testing.T) { if err != nil { t.Fatal(err) } - assert.Check(t, is.Equal(client.version, api.DefaultVersion)) + assert.Check(t, is.Equal(client.ClientVersion(), api.DefaultVersion)) - expected := "1.22" - os.Setenv("DOCKER_API_VERSION", expected) + const expected = "1.22" + _ = os.Setenv("DOCKER_API_VERSION", expected) client, err = NewClientWithOpts(FromEnv) if err != nil { t.Fatal(err) } - assert.Check(t, is.Equal(expected, client.version)) + assert.Check(t, is.Equal(client.ClientVersion(), expected)) } -// TestNegotiateAPIVersionEmpty asserts that client.Client can -// negotiate a compatible APIVersion when omitted +// TestNegotiateAPIVersionEmpty asserts that client.Client version negotiation +// downgrades to the correct API version if the API's ping response does not +// return an API version. func TestNegotiateAPIVersionEmpty(t *testing.T) { defer env.PatchAll(t, map[string]string{"DOCKER_API_VERSION": ""})() client, err := NewClientWithOpts(FromEnv) assert.NilError(t, err) - ping := types.Ping{ - APIVersion: "", - OSType: "linux", - Experimental: false, - } - // set our version to something new client.version = "1.25" // if no version from server, expect the earliest // version before APIVersion was implemented - expected := "1.24" + const expected = "1.24" // test downgrade - client.NegotiateAPIVersionPing(ping) - assert.Check(t, is.Equal(expected, client.version)) + client.NegotiateAPIVersionPing(types.Ping{}) + assert.Equal(t, client.ClientVersion(), expected) } // TestNegotiateAPIVersion asserts that client.Client can // negotiate a compatible APIVersion with the server func TestNegotiateAPIVersion(t *testing.T) { - client, err := NewClientWithOpts(FromEnv) - assert.NilError(t, err) - - expected := "1.21" - ping := types.Ping{ - APIVersion: expected, - OSType: "linux", - Experimental: false, + tests := []struct { + doc string + clientVersion string + pingVersion string + expectedVersion string + }{ + { + // client should downgrade to the version reported by the daemon. + doc: "downgrade from default", + pingVersion: "1.21", + expectedVersion: "1.21", + }, + { + // client should not downgrade to the version reported by the + // daemon if a custom version was set. + doc: "no downgrade from custom version", + clientVersion: "1.25", + pingVersion: "1.21", + expectedVersion: "1.25", + }, + { + // client should downgrade to the last version before version + // negotiation was added (1.24) if the daemon does not report + // a version. + doc: "downgrade legacy", + pingVersion: "", + expectedVersion: "1.24", + }, + { + // client should downgrade to the version reported by the daemon. + // version negotiation was added in API 1.25, so this is theoretical, + // but it should negotiate to versions before that if the daemon + // gives that as a response. + doc: "downgrade old", + pingVersion: "1.19", + expectedVersion: "1.19", + }, + { + // client should not upgrade to a newer version if a version was set, + // even if both the daemon and the client support it. + doc: "no upgrade", + clientVersion: "1.20", + pingVersion: "1.21", + expectedVersion: "1.20", + }, } - // set our version to something new - client.version = "1.22" - - // test downgrade - client.NegotiateAPIVersionPing(ping) - assert.Check(t, is.Equal(expected, client.version)) - - // set the client version to something older, and verify that we keep the - // original setting. - expected = "1.20" - client.version = expected - client.NegotiateAPIVersionPing(ping) - assert.Check(t, is.Equal(expected, client.version)) - + for _, tc := range tests { + tc := tc + t.Run(tc.doc, func(t *testing.T) { + opts := make([]Opt, 0) + if tc.clientVersion != "" { + // Note that this check is redundant, as WithVersion() considers + // an empty version equivalent to "not setting a version", but + // doing this just to be explicit we are using the default. + opts = append(opts, WithVersion(tc.clientVersion)) + } + client, err := NewClientWithOpts(opts...) + assert.NilError(t, err) + client.NegotiateAPIVersionPing(types.Ping{APIVersion: tc.pingVersion}) + assert.Equal(t, tc.expectedVersion, client.ClientVersion()) + }) + } } -// TestNegotiateAPIVersionOverride asserts that we honor -// the environment variable DOCKER_API_VERSION when negotiating versions +// TestNegotiateAPIVersionOverride asserts that we honor the DOCKER_API_VERSION +// environment variable when negotiating versions. func TestNegotiateAPVersionOverride(t *testing.T) { - expected := "9.99" + const expected = "9.99" defer env.PatchAll(t, map[string]string{"DOCKER_API_VERSION": expected})() client, err := NewClientWithOpts(FromEnv) assert.NilError(t, err) - ping := types.Ping{ - APIVersion: "1.24", - OSType: "linux", - Experimental: false, - } - // test that we honored the env var - client.NegotiateAPIVersionPing(ping) - assert.Check(t, is.Equal(expected, client.version)) + client.NegotiateAPIVersionPing(types.Ping{APIVersion: "1.24"}) + assert.Equal(t, client.ClientVersion(), expected) } func TestNegotiateAPIVersionAutomatic(t *testing.T) { @@ -278,24 +309,28 @@ func TestNegotiateAPIVersionAutomatic(t *testing.T) { return resp, nil }) + ctx := context.Background() client, err := NewClientWithOpts( WithHTTPClient(httpClient), WithAPIVersionNegotiation(), ) assert.NilError(t, err) - ctx := context.Background() - assert.Equal(t, client.ClientVersion(), api.DefaultVersion) + // Client defaults to use api.DefaultVersion before version-negotiation. + expected := api.DefaultVersion + assert.Equal(t, client.ClientVersion(), expected) // First request should trigger negotiation pingVersion = "1.35" + expected = "1.35" _, _ = client.Info(ctx) - assert.Equal(t, client.ClientVersion(), "1.35") + assert.Equal(t, client.ClientVersion(), expected) // Once successfully negotiated, subsequent requests should not re-negotiate pingVersion = "1.25" + expected = "1.35" _, _ = client.Info(ctx) - assert.Equal(t, client.ClientVersion(), "1.35") + assert.Equal(t, client.ClientVersion(), expected) } // TestNegotiateAPIVersionWithEmptyVersion asserts that initializing a client @@ -304,18 +339,20 @@ func TestNegotiateAPIVersionWithEmptyVersion(t *testing.T) { client, err := NewClientWithOpts(WithVersion("")) assert.NilError(t, err) - client.NegotiateAPIVersionPing(types.Ping{APIVersion: "1.35"}) - assert.Equal(t, client.version, "1.35") + const expected = "1.35" + client.NegotiateAPIVersionPing(types.Ping{APIVersion: expected}) + assert.Equal(t, client.ClientVersion(), expected) } // TestNegotiateAPIVersionWithFixedVersion asserts that initializing a client -// with an fixed version disables API-version negotiation +// with a fixed version disables API-version negotiation func TestNegotiateAPIVersionWithFixedVersion(t *testing.T) { - client, err := NewClientWithOpts(WithVersion("1.35")) + const customVersion = "1.35" + client, err := NewClientWithOpts(WithVersion(customVersion)) assert.NilError(t, err) client.NegotiateAPIVersionPing(types.Ping{APIVersion: "1.31"}) - assert.Equal(t, client.version, "1.35") + assert.Equal(t, client.ClientVersion(), customVersion) } type roundTripFunc func(*http.Request) (*http.Response, error) @@ -359,16 +396,19 @@ func TestClientRedirect(t *testing.T) { } for _, tc := range cases { - req, err := http.NewRequest(tc.httpMethod, "/redirectme", nil) - assert.Check(t, err) - resp, err := client.Do(req) - assert.Check(t, is.Equal(tc.statusCode, resp.StatusCode)) - if tc.expectedErr == nil { - assert.Check(t, is.Nil(err)) - } else { - urlError, ok := err.(*url.Error) - assert.Assert(t, ok, "%T is not *url.Error", err) - assert.Check(t, is.Equal(*tc.expectedErr, *urlError)) - } + tc := tc + t.Run(tc.httpMethod, func(t *testing.T) { + req, err := http.NewRequest(tc.httpMethod, "/redirectme", nil) + assert.Check(t, err) + resp, err := client.Do(req) + assert.Check(t, is.Equal(resp.StatusCode, tc.statusCode)) + if tc.expectedErr == nil { + assert.NilError(t, err) + } else { + urlError, ok := err.(*url.Error) + assert.Assert(t, ok, "%T is not *url.Error", err) + assert.Check(t, is.Equal(*urlError, *tc.expectedErr)) + } + }) } }