From 97b50bca2ce1ecd7f64180f55b59ff56db135bd9 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 28 Jan 2025 21:32:23 +0100 Subject: [PATCH] client: WithVersion: strip v-prefix when setting API version When overriding the API version through DOCKER_API_VERSION, no validation happens on the given version. However, some code-paths in the client do some minor normalizing, and strip the "v" prefix (if present) as part of [`Client.getAPIPath()`][1]. This resulted in some inconsistent handling of the version that's set. For example, [`Client.checkResponseErr()`][2] decides whether or not the API response is expected to support errors in JSON format (`types.ErrorResponse`), which would fail because `versions.GreaterThan()` does not strip the prefix, therefore making the first element "zero" (ranking lower than any valid version). Net result was "mixed" because of this; for example in the following, half the output is handled correctly ("downgraded from 1.47"), but the response is handled as < 1.23 (so printed as-is); DOCKER_API_VERSION=v1.23 docker version Client: Docker Engine - Community Version: 27.5.1 API version: v1.23 (downgraded from 1.47) Go version: go1.22.11 Git commit: 9f9e405 Built: Wed Jan 22 13:41:13 2025 OS/Arch: linux/amd64 Context: default Error response from daemon: {"message":"client version 1.23 is too old. Minimum supported API version is 1.24, please upgrade your client to a newer version"} Passing the version without v-prefix corrects this problem; DOCKER_API_VERSION=1.23 docker version Client: Docker Engine - Community Version: 27.5.1 API version: 1.99 (downgraded from 1.47) Go version: go1.22.11 Git commit: 9f9e405 Built: Wed Jan 22 13:41:13 2025 OS/Arch: linux/amd64 Context: default Error response from daemon: client version 1.99 is too new. Maximum supported API version is 1.47 DOCKER_API_VERSION=v1.99 docker version Client: Docker Engine - Community Version: 27.5.1 API version: v1.99 (downgraded from 1.47) Go version: go1.22.11 Git commit: 9f9e405 Built: Wed Jan 22 13:41:13 2025 OS/Arch: linux/amd64 Context: default Error response from daemon: {"message":"client version 1.99 is too new. Maximum supported API version is 1.47"} This patch strips the prefix when setting a custom version, so that normalization happens consistently. The existing code to strip the prefix in [`Client.getAPIPath()`][1] is kept for now, in case values are set through other ways. [1]: https://github.com/moby/moby/blob/47dc8d5dd8c9226ad53e22cdad2011a7ab48278a/client/client.go#L303-L309 [2]: https://github.com/moby/moby/blob/47dc8d5dd8c9226ad53e22cdad2011a7ab48278a/client/request.go#L231-L241 Signed-off-by: Sebastiaan van Stijn --- client/client.go | 3 +-- client/client_test.go | 52 +++++++++++++++++++++++++++++++++++++++++++ client/options.go | 5 +++-- 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/client/client.go b/client/client.go index 46832d8a44..97dbbf70b9 100644 --- a/client/client.go +++ b/client/client.go @@ -304,8 +304,7 @@ func (cli *Client) getAPIPath(ctx context.Context, p string, query url.Values) s var apiPath string _ = cli.checkVersion(ctx) if cli.version != "" { - v := strings.TrimPrefix(cli.version, "v") - apiPath = path.Join(cli.basePath, "/v"+v, p) + apiPath = path.Join(cli.basePath, "/v"+strings.TrimPrefix(cli.version, "v"), p) } else { apiPath = path.Join(cli.basePath, p) } diff --git a/client/client_test.go b/client/client_test.go index 4322ff0c84..e1712010b0 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -420,6 +420,58 @@ func TestNegotiateAPIVersionWithFixedVersion(t *testing.T) { assert.Equal(t, client.ClientVersion(), customVersion) } +// TestCustomAPIVersion tests initializing the client with a custom +// version. +func TestCustomAPIVersion(t *testing.T) { + tests := []struct { + version string + expected string + }{ + { + version: "", + expected: api.DefaultVersion, + }, + { + version: "1.0", + expected: "1.0", + }, + { + version: "9.99", + expected: "9.99", + }, + { + version: "v", + expected: api.DefaultVersion, + }, + { + version: "v1.0", + expected: "1.0", + }, + { + version: "v9.99", + expected: "9.99", + }, + { + // When manually setting a version, no validation happens. + // so anything is accepted. + version: "something-weird", + expected: "something-weird", + }, + } + for _, tc := range tests { + t.Run(tc.version, func(t *testing.T) { + client, err := NewClientWithOpts(WithVersion(tc.version)) + assert.NilError(t, err) + assert.Equal(t, client.ClientVersion(), tc.expected) + + t.Setenv(EnvOverrideAPIVersion, tc.expected) + client, err = NewClientWithOpts(WithVersionFromEnv()) + assert.NilError(t, err) + assert.Equal(t, client.ClientVersion(), tc.expected) + }) + } +} + type roundTripFunc func(*http.Request) (*http.Response, error) func (rtf roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { diff --git a/client/options.go b/client/options.go index ddb0ca3991..85b12447d5 100644 --- a/client/options.go +++ b/client/options.go @@ -6,6 +6,7 @@ import ( "net/http" "os" "path/filepath" + "strings" "time" "github.com/docker/go-connections/sockets" @@ -194,8 +195,8 @@ func WithTLSClientConfigFromEnv() Opt { // (see [WithAPIVersionNegotiation]). func WithVersion(version string) Opt { return func(c *Client) error { - if version != "" { - c.version = version + if v := strings.TrimPrefix(version, "v"); v != "" { + c.version = v c.manualOverride = true } return nil