Merge pull request #39032 from thaJeztah/improve_version_negotiation

Add client.WithAPIVersionNegotiation() option

client/client.go

@@ -81,6 +81,15 @@ type Client struct {
 	customHTTPHeaders map[string]string
 	// manualOverride is set to true when the version was set by users.
 	manualOverride bool
+
+	// negotiateVersion indicates if the client should automatically negotiate
+	// the API version to use when making requests. API version negotiation is
+	// performed on the first request, after which negotiated is set to "true"
+	// so that subsequent requests do not re-negotiate.
+	negotiateVersion bool
+
+	// negotiated indicates that API version negotiation took place
+	negotiated bool
 }
 
 // CheckRedirect specifies the policy for dealing with redirect responses:
@@ -169,8 +178,11 @@ func (cli *Client) Close() error {
 
 // getAPIPath returns the versioned request path to call the api.
 // It appends the query parameters to the path if they are not empty.
-func (cli *Client) getAPIPath(p string, query url.Values) string {
+func (cli *Client) getAPIPath(ctx context.Context, p string, query url.Values) string {
 	var apiPath string
+	if cli.negotiateVersion && !cli.negotiated {
+		cli.NegotiateAPIVersion(ctx)
+	}
 	if cli.version != "" {
 		v := strings.TrimPrefix(cli.version, "v")
 		apiPath = path.Join(cli.basePath, "/v"+v, p)
@@ -186,19 +198,31 @@ func (cli *Client) ClientVersion() string {
 }
 
 // NegotiateAPIVersion queries the API and updates the version to match the
-// API version. Any errors are silently ignored.
+// API version. Any errors are silently ignored. If a manual override is in place,
+// either through the `DOCKER_API_VERSION` environment variable, or if the client
+// was initialized with a fixed version (`opts.WithVersion(xx)`), no negotiation
+// will be performed.
 func (cli *Client) NegotiateAPIVersion(ctx context.Context) {
-	ping, _ := cli.Ping(ctx)
-	cli.NegotiateAPIVersionPing(ping)
+	if !cli.manualOverride {
+		ping, _ := cli.Ping(ctx)
+		cli.negotiateAPIVersionPing(ping)
+	}
 }
 
 // NegotiateAPIVersionPing updates the client version to match the Ping.APIVersion
-// if the ping version is less than the default version.
+// if the ping version is less than the default version.  If a manual override is
+// in place, either through the `DOCKER_API_VERSION` environment variable, or if
+// the client was initialized with a fixed version (`opts.WithVersion(xx)`), no
+// negotiation is performed.
 func (cli *Client) NegotiateAPIVersionPing(p types.Ping) {
-	if cli.manualOverride {
-		return
+	if !cli.manualOverride {
+		cli.negotiateAPIVersionPing(p)
 	}
+}
 
+// negotiateAPIVersionPing queries the API and updates the version to match the
+// API version. Any errors are silently ignored.
+func (cli *Client) negotiateAPIVersionPing(p types.Ping) {
 	// try the latest version before versioning headers existed
 	if p.APIVersion == "" {
 		p.APIVersion = "1.24"
@@ -213,6 +237,12 @@ func (cli *Client) NegotiateAPIVersionPing(p types.Ping) {
 	if versions.LessThan(p.APIVersion, cli.version) {
 		cli.version = p.APIVersion
 	}
+
+	// Store the results, so that automatic API version negotiation (if enabled)
+	// won't be performed on the next request.
+	if cli.negotiateVersion {
+		cli.negotiated = true
+	}
 }
 
 // DaemonHost returns the host address used by the client

client/client_test.go

@@ -2,10 +2,13 @@ package client // import "github.com/docker/docker/client"
 
 import (
 	"bytes"
+	"context"
+	"io/ioutil"
 	"net/http"
 	"net/url"
 	"os"
 	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/docker/docker/api"
@@ -123,9 +126,10 @@ func TestGetAPIPath(t *testing.T) {
 		{"v1.22", "/networks/kiwl$%^", nil, "/v1.22/networks/kiwl$%25%5E"},
 	}
 
+	ctx := context.TODO()
 	for _, testcase := range testcases {
 		c := Client{version: testcase.version, basePath: "/"}
-		actual := c.getAPIPath(testcase.path, testcase.query)
+		actual := c.getAPIPath(ctx, testcase.path, testcase.query)
 		assert.Check(t, is.Equal(actual, testcase.expected))
 	}
 }
@@ -265,6 +269,35 @@ func TestNegotiateAPVersionOverride(t *testing.T) {
 	assert.Check(t, is.Equal(expected, client.version))
 }
 
+func TestNegotiateAPIVersionAutomatic(t *testing.T) {
+	var pingVersion string
+	httpClient := newMockClient(func(req *http.Request) (*http.Response, error) {
+		resp := &http.Response{StatusCode: http.StatusOK, Header: http.Header{}}
+		resp.Header.Set("API-Version", pingVersion)
+		resp.Body = ioutil.NopCloser(strings.NewReader("OK"))
+		return resp, nil
+	})
+
+	client, err := NewClientWithOpts(
+		WithHTTPClient(httpClient),
+		WithAPIVersionNegotiation(),
+	)
+	assert.NilError(t, err)
+
+	ctx := context.Background()
+	assert.Equal(t, client.ClientVersion(), api.DefaultVersion)
+
+	// First request should trigger negotiation
+	pingVersion = "1.35"
+	_, _ = client.Info(ctx)
+	assert.Equal(t, client.ClientVersion(), "1.35")
+
+	// Once successfully negotiated, subsequent requests should not re-negotiate
+	pingVersion = "1.25"
+	_, _ = client.Info(ctx)
+	assert.Equal(t, client.ClientVersion(), "1.35")
+}
+
 // TestNegotiateAPIVersionWithEmptyVersion asserts that initializing a client
 // with an empty version string does still allow API-version negotiation
 func TestNegotiateAPIVersionWithEmptyVersion(t *testing.T) {

client/hijack.go

@@ -23,7 +23,7 @@ func (cli *Client) postHijacked(ctx context.Context, path string, query url.Valu
 		return types.HijackedResponse{}, err
 	}
 
-	apiPath := cli.getAPIPath(path, query)
+	apiPath := cli.getAPIPath(ctx, path, query)
 	req, err := http.NewRequest("POST", apiPath, bodyEncoded)
 	if err != nil {
 		return types.HijackedResponse{}, err

client/options.go

@@ -159,3 +159,14 @@ func WithVersion(version string) Opt {
 		return nil
 	}
 }
+
+// WithAPIVersionNegotiation enables automatic API version negotiation for the client.
+// With this option enabled, the client automatically negotiates the API version
+// to use when making requests. API version negotiation is performed on the first
+// request; subsequent requests will not re-negotiate.
+func WithAPIVersionNegotiation() Opt {
+	return func(c *Client) error {
+		c.negotiateVersion = true
+		return nil
+	}
+}

client/request.go

@@ -115,7 +115,7 @@ func (cli *Client) buildRequest(method, path string, body io.Reader, headers hea
 }
 
 func (cli *Client) sendRequest(ctx context.Context, method, path string, query url.Values, body io.Reader, headers headers) (serverResponse, error) {
-	req, err := cli.buildRequest(method, cli.getAPIPath(path, query), body, headers)
+	req, err := cli.buildRequest(method, cli.getAPIPath(ctx, path, query), body, headers)
 	if err != nil {
 		return serverResponse{}, err
 	}