Browse code

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 <github@gone.nl>

Sebastiaan van Stijn authored on 2025/01/29 05:32:23
Showing 3 changed files
... ...
@@ -304,8 +304,7 @@ func (cli *Client) getAPIPath(ctx context.Context, p string, query url.Values) s
304 304
 	var apiPath string
305 305
 	_ = cli.checkVersion(ctx)
306 306
 	if cli.version != "" {
307
-		v := strings.TrimPrefix(cli.version, "v")
308
-		apiPath = path.Join(cli.basePath, "/v"+v, p)
307
+		apiPath = path.Join(cli.basePath, "/v"+strings.TrimPrefix(cli.version, "v"), p)
309 308
 	} else {
310 309
 		apiPath = path.Join(cli.basePath, p)
311 310
 	}
... ...
@@ -420,6 +420,58 @@ func TestNegotiateAPIVersionWithFixedVersion(t *testing.T) {
420 420
 	assert.Equal(t, client.ClientVersion(), customVersion)
421 421
 }
422 422
 
423
+// TestCustomAPIVersion tests initializing the client with a custom
424
+// version.
425
+func TestCustomAPIVersion(t *testing.T) {
426
+	tests := []struct {
427
+		version  string
428
+		expected string
429
+	}{
430
+		{
431
+			version:  "",
432
+			expected: api.DefaultVersion,
433
+		},
434
+		{
435
+			version:  "1.0",
436
+			expected: "1.0",
437
+		},
438
+		{
439
+			version:  "9.99",
440
+			expected: "9.99",
441
+		},
442
+		{
443
+			version:  "v",
444
+			expected: api.DefaultVersion,
445
+		},
446
+		{
447
+			version:  "v1.0",
448
+			expected: "1.0",
449
+		},
450
+		{
451
+			version:  "v9.99",
452
+			expected: "9.99",
453
+		},
454
+		{
455
+			// When manually setting a version, no validation happens.
456
+			// so anything is accepted.
457
+			version:  "something-weird",
458
+			expected: "something-weird",
459
+		},
460
+	}
461
+	for _, tc := range tests {
462
+		t.Run(tc.version, func(t *testing.T) {
463
+			client, err := NewClientWithOpts(WithVersion(tc.version))
464
+			assert.NilError(t, err)
465
+			assert.Equal(t, client.ClientVersion(), tc.expected)
466
+
467
+			t.Setenv(EnvOverrideAPIVersion, tc.expected)
468
+			client, err = NewClientWithOpts(WithVersionFromEnv())
469
+			assert.NilError(t, err)
470
+			assert.Equal(t, client.ClientVersion(), tc.expected)
471
+		})
472
+	}
473
+}
474
+
423 475
 type roundTripFunc func(*http.Request) (*http.Response, error)
424 476
 
425 477
 func (rtf roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) {
... ...
@@ -6,6 +6,7 @@ import (
6 6
 	"net/http"
7 7
 	"os"
8 8
 	"path/filepath"
9
+	"strings"
9 10
 	"time"
10 11
 
11 12
 	"github.com/docker/go-connections/sockets"
... ...
@@ -194,8 +195,8 @@ func WithTLSClientConfigFromEnv() Opt {
194 194
 // (see [WithAPIVersionNegotiation]).
195 195
 func WithVersion(version string) Opt {
196 196
 	return func(c *Client) error {
197
-		if version != "" {
198
-			c.version = version
197
+		if v := strings.TrimPrefix(version, "v"); v != "" {
198
+			c.version = v
199 199
 			c.manualOverride = true
200 200
 		}
201 201
 		return nil