Merge pull request #51020 from thaJeztah/clean_ping_version

client: Client.negotiateAPIVersionPing: trim v-prefix before handling

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)

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)

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))
 		})
 	}

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)