From a8cb35f01abd573f5677525f33be326a3dcb9152 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 22 Sep 2025 10:52:01 +0200 Subject: [PATCH 1/3] client: TestPingHeadFallback: check method, path, and fix example response Validate that the client is connecting with the expected endpoint path and method(s). Also fix the Api-Version response to align with the actual format returned, which doesn't include a "v" prefix; curl -sI --unix-socket /var/run/docker.sock 'http://localhost/_ping' | grep 'Api-Version' Api-Version: 1.51 Signed-off-by: Sebastiaan van Stijn --- client/ping_test.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/client/ping_test.go b/client/ping_test.go index e04e7367cf..bb72ff6ba2 100644 --- a/client/ping_test.go +++ b/client/ping_test.go @@ -3,8 +3,10 @@ package client import ( "context" "errors" + "fmt" "io" "net/http" + "slices" "strings" "testing" @@ -85,6 +87,9 @@ func TestPingSuccess(t *testing.T) { // TestPingHeadFallback tests that the client falls back to GET if HEAD fails. func TestPingHeadFallback(t *testing.T) { + const expectedPath = "/_ping" + expMethods := []string{http.MethodHead, http.MethodGet} + tests := []struct { status int expected []string @@ -111,17 +116,23 @@ func TestPingHeadFallback(t *testing.T) { t.Run(http.StatusText(tc.status), func(t *testing.T) { var reqs []string client, err := NewClientWithOpts(WithMockClient(func(req *http.Request) (*http.Response, error) { + if !strings.HasPrefix(req.URL.Path, expectedPath) { + return nil, fmt.Errorf("expected URL '%s', got '%s'", expectedPath, req.URL.Path) + } + if !slices.Contains(expMethods, req.Method) { + return nil, fmt.Errorf("expected one of '%v', got '%s'", expMethods, req.Method) + } reqs = append(reqs, req.Method) resp := &http.Response{StatusCode: http.StatusOK, Header: http.Header{}} if req.Method == http.MethodHead { resp.StatusCode = tc.status } - resp.Header.Add("Api-Version", "v1.2.3") + resp.Header.Add("Api-Version", "1.2.3") return resp, nil })) assert.NilError(t, err) ping, _ := client.Ping(context.Background()) - assert.Check(t, is.Equal(ping.APIVersion, "v1.2.3")) + assert.Check(t, is.Equal(ping.APIVersion, "1.2.3")) assert.Check(t, is.DeepEqual(reqs, tc.expected)) }) } From af3f9714317f68ff7e794fd0cfc9c9f5cf1ec1a8 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 22 Sep 2025 11:20:22 +0200 Subject: [PATCH 2/3] client: TestNegotiateAPIVersionAutomatic: gofumpt Signed-off-by: Sebastiaan van Stijn --- client/client_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/client_test.go b/client/client_test.go index 7ebebbcbcd..11cb72bb6e 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -369,7 +369,8 @@ func TestNegotiateAPIVersionAutomatic(t *testing.T) { resp.Header.Set("Api-Version", pingVersion) resp.Body = io.NopCloser(strings.NewReader("OK")) return resp, nil - })}), + }), + }), WithAPIVersionNegotiation(), ) assert.NilError(t, err) From f5847040ccaf8d017954f9a016fad0b1e50a548e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 22 Sep 2025 11:21:28 +0200 Subject: [PATCH 3/3] client: Client.negotiateAPIVersionPing: trim v-prefix before handling Trim any v-prefix passed to this function to make sure we detect empty API versions. In most cases, the ping-response will originate from the API server, but the exported `Client.NegotiateAPIVersionPing` allows a ping-response to be passed manually. While updating, also update the signature to only accept the version, as only the `PingResponse.APIVersion` is used by this function. Signed-off-by: Sebastiaan van Stijn --- client/client.go | 18 ++++++++++-------- vendor/github.com/moby/moby/client/client.go | 18 ++++++++++-------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/client/client.go b/client/client.go index 61b8a24a7f..b5e2e40c4b 100644 --- a/client/client.go +++ b/client/client.go @@ -275,7 +275,7 @@ func (cli *Client) checkVersion(ctx context.Context) error { if err != nil { return err } - cli.negotiateAPIVersionPing(ping) + cli.negotiateAPIVersionPing(ping.APIVersion) } return nil } @@ -324,7 +324,7 @@ func (cli *Client) NegotiateAPIVersion(ctx context.Context) { // FIXME(thaJeztah): Ping returns an error when failing to connect to the API; we should not swallow the error here, and instead returning it. return } - cli.negotiateAPIVersionPing(ping) + cli.negotiateAPIVersionPing(ping.APIVersion) } } @@ -347,16 +347,18 @@ func (cli *Client) NegotiateAPIVersionPing(pingResponse types.Ping) { cli.negotiateLock.Lock() defer cli.negotiateLock.Unlock() - cli.negotiateAPIVersionPing(pingResponse) + cli.negotiateAPIVersionPing(pingResponse.APIVersion) } } // negotiateAPIVersionPing queries the API and updates the version to match the // API version from the ping response. -func (cli *Client) negotiateAPIVersionPing(pingResponse types.Ping) { +func (cli *Client) negotiateAPIVersionPing(pingVersion string) { + pingVersion = strings.TrimPrefix(pingVersion, "v") + // default to the latest version before versioning headers existed - if pingResponse.APIVersion == "" { - pingResponse.APIVersion = fallbackAPIVersion + if pingVersion == "" { + pingVersion = fallbackAPIVersion } // if the client is not initialized with a version, start with the latest supported version @@ -365,8 +367,8 @@ func (cli *Client) negotiateAPIVersionPing(pingResponse types.Ping) { } // if server version is lower than the client version, downgrade - if versions.LessThan(pingResponse.APIVersion, cli.version) { - cli.version = pingResponse.APIVersion + if versions.LessThan(pingVersion, cli.version) { + cli.version = pingVersion } // Store the results, so that automatic API version negotiation (if enabled) diff --git a/vendor/github.com/moby/moby/client/client.go b/vendor/github.com/moby/moby/client/client.go index 61b8a24a7f..b5e2e40c4b 100644 --- a/vendor/github.com/moby/moby/client/client.go +++ b/vendor/github.com/moby/moby/client/client.go @@ -275,7 +275,7 @@ func (cli *Client) checkVersion(ctx context.Context) error { if err != nil { return err } - cli.negotiateAPIVersionPing(ping) + cli.negotiateAPIVersionPing(ping.APIVersion) } return nil } @@ -324,7 +324,7 @@ func (cli *Client) NegotiateAPIVersion(ctx context.Context) { // FIXME(thaJeztah): Ping returns an error when failing to connect to the API; we should not swallow the error here, and instead returning it. return } - cli.negotiateAPIVersionPing(ping) + cli.negotiateAPIVersionPing(ping.APIVersion) } } @@ -347,16 +347,18 @@ func (cli *Client) NegotiateAPIVersionPing(pingResponse types.Ping) { cli.negotiateLock.Lock() defer cli.negotiateLock.Unlock() - cli.negotiateAPIVersionPing(pingResponse) + cli.negotiateAPIVersionPing(pingResponse.APIVersion) } } // negotiateAPIVersionPing queries the API and updates the version to match the // API version from the ping response. -func (cli *Client) negotiateAPIVersionPing(pingResponse types.Ping) { +func (cli *Client) negotiateAPIVersionPing(pingVersion string) { + pingVersion = strings.TrimPrefix(pingVersion, "v") + // default to the latest version before versioning headers existed - if pingResponse.APIVersion == "" { - pingResponse.APIVersion = fallbackAPIVersion + if pingVersion == "" { + pingVersion = fallbackAPIVersion } // if the client is not initialized with a version, start with the latest supported version @@ -365,8 +367,8 @@ func (cli *Client) negotiateAPIVersionPing(pingResponse types.Ping) { } // if server version is lower than the client version, downgrade - if versions.LessThan(pingResponse.APIVersion, cli.version) { - cli.version = pingResponse.APIVersion + if versions.LessThan(pingVersion, cli.version) { + cli.version = pingVersion } // Store the results, so that automatic API version negotiation (if enabled)