client: negotiate api version before handling version-specific code

We try to perform API-version negotiation as lazy as possible (and only execute
when we are about to make an API request). However, some code requires API-version
dependent handling (to set options, or remove options based on the version of the
API we're using).

Currently this code depended on the caller code to perform API negotiation (or
to configure the API version) first, which may not happen, and because of that
we may be missing options (or set options that are not supported on older API
versions).

This patch:

- splits the code that triggered API-version negotiation to a separate
  Client.checkVersion() function.
- updates NewVersionError to accept a context
- updates NewVersionError to perform API-version negotiation (if enabled)
- updates various Client functions to manually trigger API-version negotiation

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

client/build_prune.go

@@ -13,7 +13,7 @@ import (
 
 // BuildCachePrune requests the daemon to delete unused cache data
 func (cli *Client) BuildCachePrune(ctx context.Context, opts types.BuildCachePruneOptions) (*types.BuildCachePruneReport, error) {
-	if err := cli.NewVersionError("1.31", "build prune"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.31", "build prune"); err != nil {
 		return nil, err
 	}
 

client/client.go

@@ -254,13 +254,21 @@ func (cli *Client) Close() error {
 	return nil
 }
 
+// checkVersion manually triggers API version negotiation (if configured).
+// This allows for version-dependent code to use the same version as will
+// be negotiated when making the actual requests, and for which cases
+// we cannot do the negotiation lazily.
+func (cli *Client) checkVersion(ctx context.Context) {
+	if cli.negotiateVersion && !cli.negotiated {
+		cli.NegotiateAPIVersion(ctx)
+	}
+}
+
 // 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(ctx context.Context, p string, query url.Values) string {
 	var apiPath string
-	if cli.negotiateVersion && !cli.negotiated {
-		cli.NegotiateAPIVersion(ctx)
-	}
+	cli.checkVersion(ctx)
 	if cli.version != "" {
 		v := strings.TrimPrefix(cli.version, "v")
 		apiPath = path.Join(cli.basePath, "/v"+v, p)

client/config_create.go

@@ -11,7 +11,7 @@ import (
 // ConfigCreate creates a new config.
 func (cli *Client) ConfigCreate(ctx context.Context, config swarm.ConfigSpec) (types.ConfigCreateResponse, error) {
 	var response types.ConfigCreateResponse
-	if err := cli.NewVersionError("1.30", "config create"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.30", "config create"); err != nil {
 		return response, err
 	}
 	resp, err := cli.post(ctx, "/configs/create", nil, config, nil)

client/config_inspect.go

@@ -14,7 +14,7 @@ func (cli *Client) ConfigInspectWithRaw(ctx context.Context, id string) (swarm.C
 	if id == "" {
 		return swarm.Config{}, nil, objectNotFoundError{object: "config", id: id}
 	}
-	if err := cli.NewVersionError("1.30", "config inspect"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.30", "config inspect"); err != nil {
 		return swarm.Config{}, nil, err
 	}
 	resp, err := cli.get(ctx, "/configs/"+id, nil, nil)

client/config_list.go

@@ -12,7 +12,7 @@ import (
 
 // ConfigList returns the list of configs.
 func (cli *Client) ConfigList(ctx context.Context, options types.ConfigListOptions) ([]swarm.Config, error) {
-	if err := cli.NewVersionError("1.30", "config list"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.30", "config list"); err != nil {
 		return nil, err
 	}
 	query := url.Values{}

client/config_remove.go

@@ -4,7 +4,7 @@ import "context"
 
 // ConfigRemove removes a config.
 func (cli *Client) ConfigRemove(ctx context.Context, id string) error {
-	if err := cli.NewVersionError("1.30", "config remove"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.30", "config remove"); err != nil {
 		return err
 	}
 	resp, err := cli.delete(ctx, "/configs/"+id, nil, nil)

client/config_update.go

@@ -9,7 +9,7 @@ import (
 
 // ConfigUpdate attempts to update a config
 func (cli *Client) ConfigUpdate(ctx context.Context, id string, version swarm.Version, config swarm.ConfigSpec) error {
-	if err := cli.NewVersionError("1.30", "config update"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.30", "config update"); err != nil {
 		return err
 	}
 	query := url.Values{}

client/container_create.go

@@ -23,13 +23,20 @@ type configWrapper struct {
 func (cli *Client) ContainerCreate(ctx context.Context, config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, platform *ocispec.Platform, containerName string) (container.CreateResponse, error) {
 	var response container.CreateResponse
 
-	if err := cli.NewVersionError("1.25", "stop timeout"); config != nil && config.StopTimeout != nil && err != nil {
+	// Make sure we negotiated (if the client is configured to do so),
+	// as code below contains API-version specific handling of options.
+	//
+	// Normally, version-negotiation (if enabled) would not happen until
+	// the API request is made.
+	cli.checkVersion(ctx)
+
+	if err := cli.NewVersionError(ctx, "1.25", "stop timeout"); config != nil && config.StopTimeout != nil && err != nil {
 		return response, err
 	}
-	if err := cli.NewVersionError("1.41", "specify container image platform"); platform != nil && err != nil {
+	if err := cli.NewVersionError(ctx, "1.41", "specify container image platform"); platform != nil && err != nil {
 		return response, err
 	}
-	if err := cli.NewVersionError("1.44", "specify health-check start interval"); config != nil && config.Healthcheck != nil && config.Healthcheck.StartInterval != 0 && err != nil {
+	if err := cli.NewVersionError(ctx, "1.44", "specify health-check start interval"); config != nil && config.Healthcheck != nil && config.Healthcheck.StartInterval != 0 && err != nil {
 		return response, err
 	}
 

client/container_exec.go

@@ -13,7 +13,14 @@ import (
 func (cli *Client) ContainerExecCreate(ctx context.Context, container string, config types.ExecConfig) (types.IDResponse, error) {
 	var response types.IDResponse
 
-	if err := cli.NewVersionError("1.25", "env"); len(config.Env) != 0 && err != nil {
+	// Make sure we negotiated (if the client is configured to do so),
+	// as code below contains API-version specific handling of options.
+	//
+	// Normally, version-negotiation (if enabled) would not happen until
+	// the API request is made.
+	cli.checkVersion(ctx)
+
+	if err := cli.NewVersionError(ctx, "1.25", "env"); len(config.Env) != 0 && err != nil {
 		return response, err
 	}
 	if versions.LessThan(cli.ClientVersion(), "1.42") {

client/container_prune.go

@@ -13,7 +13,7 @@ import (
 func (cli *Client) ContainersPrune(ctx context.Context, pruneFilters filters.Args) (types.ContainersPruneReport, error) {
 	var report types.ContainersPruneReport
 
-	if err := cli.NewVersionError("1.25", "container prune"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.25", "container prune"); err != nil {
 		return report, err
 	}
 

client/container_restart.go

@@ -17,8 +17,16 @@ func (cli *Client) ContainerRestart(ctx context.Context, containerID string, opt
 	if options.Timeout != nil {
 		query.Set("t", strconv.Itoa(*options.Timeout))
 	}
-	if options.Signal != "" && versions.GreaterThanOrEqualTo(cli.version, "1.42") {
-		query.Set("signal", options.Signal)
+	if options.Signal != "" {
+		// Make sure we negotiated (if the client is configured to do so),
+		// as code below contains API-version specific handling of options.
+		//
+		// Normally, version-negotiation (if enabled) would not happen until
+		// the API request is made.
+		cli.checkVersion(ctx)
+		if versions.GreaterThanOrEqualTo(cli.version, "1.42") {
+			query.Set("signal", options.Signal)
+		}
 	}
 	resp, err := cli.post(ctx, "/containers/"+containerID+"/restart", query, nil, nil)
 	ensureReaderClosed(resp)

client/container_stop.go

@@ -21,8 +21,16 @@ func (cli *Client) ContainerStop(ctx context.Context, containerID string, option
 	if options.Timeout != nil {
 		query.Set("t", strconv.Itoa(*options.Timeout))
 	}
-	if options.Signal != "" && versions.GreaterThanOrEqualTo(cli.version, "1.42") {
-		query.Set("signal", options.Signal)
+	if options.Signal != "" {
+		// Make sure we negotiated (if the client is configured to do so),
+		// as code below contains API-version specific handling of options.
+		//
+		// Normally, version-negotiation (if enabled) would not happen until
+		// the API request is made.
+		cli.checkVersion(ctx)
+		if versions.GreaterThanOrEqualTo(cli.version, "1.42") {
+			query.Set("signal", options.Signal)
+		}
 	}
 	resp, err := cli.post(ctx, "/containers/"+containerID+"/stop", query, nil, nil)
 	ensureReaderClosed(resp)

client/container_wait.go

@@ -30,6 +30,12 @@ const containerWaitErrorMsgLimit = 2 * 1024 /* Max: 2KiB */
 // synchronize ContainerWait with other calls, such as specifying a
 // "next-exit" condition before issuing a ContainerStart request.
 func (cli *Client) ContainerWait(ctx context.Context, containerID string, condition container.WaitCondition) (<-chan container.WaitResponse, <-chan error) {
+	// Make sure we negotiated (if the client is configured to do so),
+	// as code below contains API-version specific handling of options.
+	//
+	// Normally, version-negotiation (if enabled) would not happen until
+	// the API request is made.
+	cli.checkVersion(ctx)
 	if versions.LessThan(cli.ClientVersion(), "1.30") {
 		return cli.legacyContainerWait(ctx, containerID)
 	}

client/distribution_inspect.go

@@ -17,7 +17,7 @@ func (cli *Client) DistributionInspect(ctx context.Context, image, encodedRegist
 		return distributionInspect, objectNotFoundError{object: "distribution", id: image}
 	}
 
-	if err := cli.NewVersionError("1.30", "distribution inspect"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.30", "distribution inspect"); err != nil {
 		return distributionInspect, err
 	}
 

client/errors.go

@@ -1,6 +1,7 @@
 package client // import "github.com/docker/docker/client"
 
 import (
+	"context"
 	"fmt"
 
 	"github.com/docker/docker/api/types/versions"
@@ -48,9 +49,18 @@ func (e objectNotFoundError) Error() string {
 	return fmt.Sprintf("Error: No such %s: %s", e.object, e.id)
 }
 
-// NewVersionError returns an error if the APIVersion required
-// if less than the current supported version
-func (cli *Client) NewVersionError(APIrequired, feature string) error {
+// NewVersionError returns an error if the APIVersion required is less than the
+// current supported version.
+//
+// It performs API-version negotiation if the Client is configured with this
+// option, otherwise it assumes the latest API version is used.
+func (cli *Client) NewVersionError(ctx context.Context, APIrequired, feature string) error {
+	// Make sure we negotiated (if the client is configured to do so),
+	// as code below contains API-version specific handling of options.
+	//
+	// Normally, version-negotiation (if enabled) would not happen until
+	// the API request is made.
+	cli.checkVersion(ctx)
 	if cli.version != "" && versions.LessThan(cli.version, APIrequired) {
 		return fmt.Errorf("%q requires API version %s, but the Docker daemon API version is %s", feature, APIrequired, cli.version)
 	}

client/image_build.go

@@ -18,7 +18,7 @@ import (
 // The Body in the response implements an io.ReadCloser and it's up to the caller to
 // close it.
 func (cli *Client) ImageBuild(ctx context.Context, buildContext io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) {
-	query, err := cli.imageBuildOptionsToQuery(options)
+	query, err := cli.imageBuildOptionsToQuery(ctx, options)
 	if err != nil {
 		return types.ImageBuildResponse{}, err
 	}
@@ -43,7 +43,7 @@ func (cli *Client) ImageBuild(ctx context.Context, buildContext io.Reader, optio
 	}, nil
 }
 
-func (cli *Client) imageBuildOptionsToQuery(options types.ImageBuildOptions) (url.Values, error) {
+func (cli *Client) imageBuildOptionsToQuery(ctx context.Context, options types.ImageBuildOptions) (url.Values, error) {
 	query := url.Values{
 		"t":           options.Tags,
 		"securityopt": options.SecurityOpt,
@@ -73,7 +73,7 @@ func (cli *Client) imageBuildOptionsToQuery(options types.ImageBuildOptions) (ur
 	}
 
 	if options.Squash {
-		if err := cli.NewVersionError("1.25", "squash"); err != nil {
+		if err := cli.NewVersionError(ctx, "1.25", "squash"); err != nil {
 			return query, err
 		}
 		query.Set("squash", "1")
@@ -123,7 +123,7 @@ func (cli *Client) imageBuildOptionsToQuery(options types.ImageBuildOptions) (ur
 		query.Set("session", options.SessionID)
 	}
 	if options.Platform != "" {
-		if err := cli.NewVersionError("1.32", "platform"); err != nil {
+		if err := cli.NewVersionError(ctx, "1.32", "platform"); err != nil {
 			return query, err
 		}
 		query.Set("platform", strings.ToLower(options.Platform))

client/image_list.go

@@ -12,6 +12,13 @@ import (
 
 // ImageList returns a list of images in the docker host.
 func (cli *Client) ImageList(ctx context.Context, options types.ImageListOptions) ([]types.ImageSummary, error) {
+	// Make sure we negotiated (if the client is configured to do so),
+	// as code below contains API-version specific handling of options.
+	//
+	// Normally, version-negotiation (if enabled) would not happen until
+	// the API request is made.
+	cli.checkVersion(ctx)
+
 	var images []types.ImageSummary
 	query := url.Values{}
 

client/image_prune.go

@@ -13,7 +13,7 @@ import (
 func (cli *Client) ImagesPrune(ctx context.Context, pruneFilters filters.Args) (types.ImagesPruneReport, error) {
 	var report types.ImagesPruneReport
 
-	if err := cli.NewVersionError("1.25", "image prune"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.25", "image prune"); err != nil {
 		return report, err
 	}
 

client/network_create.go

@@ -10,6 +10,13 @@ import (
 
 // NetworkCreate creates a new network in the docker host.
 func (cli *Client) NetworkCreate(ctx context.Context, name string, options types.NetworkCreate) (types.NetworkCreateResponse, error) {
+	// Make sure we negotiated (if the client is configured to do so),
+	// as code below contains API-version specific handling of options.
+	//
+	// Normally, version-negotiation (if enabled) would not happen until
+	// the API request is made.
+	cli.checkVersion(ctx)
+
 	networkCreateRequest := types.NetworkCreateRequest{
 		NetworkCreate: options,
 		Name:          name,

client/network_prune.go

@@ -13,7 +13,7 @@ import (
 func (cli *Client) NetworksPrune(ctx context.Context, pruneFilters filters.Args) (types.NetworksPruneReport, error) {
 	var report types.NetworksPruneReport
 
-	if err := cli.NewVersionError("1.25", "network prune"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.25", "network prune"); err != nil {
 		return report, err
 	}
 

client/plugin_upgrade.go

@@ -14,7 +14,7 @@ import (
 
 // PluginUpgrade upgrades a plugin
 func (cli *Client) PluginUpgrade(ctx context.Context, name string, options types.PluginInstallOptions) (rc io.ReadCloser, err error) {
-	if err := cli.NewVersionError("1.26", "plugin upgrade"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.26", "plugin upgrade"); err != nil {
 		return nil, err
 	}
 	query := url.Values{}

client/secret_create.go

@@ -11,7 +11,7 @@ import (
 // SecretCreate creates a new secret.
 func (cli *Client) SecretCreate(ctx context.Context, secret swarm.SecretSpec) (types.SecretCreateResponse, error) {
 	var response types.SecretCreateResponse
-	if err := cli.NewVersionError("1.25", "secret create"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.25", "secret create"); err != nil {
 		return response, err
 	}
 	resp, err := cli.post(ctx, "/secrets/create", nil, secret, nil)

client/secret_inspect.go

@@ -11,7 +11,7 @@ import (
 
 // SecretInspectWithRaw returns the secret information with raw data
 func (cli *Client) SecretInspectWithRaw(ctx context.Context, id string) (swarm.Secret, []byte, error) {
-	if err := cli.NewVersionError("1.25", "secret inspect"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.25", "secret inspect"); err != nil {
 		return swarm.Secret{}, nil, err
 	}
 	if id == "" {

client/secret_list.go

@@ -12,7 +12,7 @@ import (
 
 // SecretList returns the list of secrets.
 func (cli *Client) SecretList(ctx context.Context, options types.SecretListOptions) ([]swarm.Secret, error) {
-	if err := cli.NewVersionError("1.25", "secret list"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.25", "secret list"); err != nil {
 		return nil, err
 	}
 	query := url.Values{}

client/secret_remove.go

@@ -4,7 +4,7 @@ import "context"
 
 // SecretRemove removes a secret.
 func (cli *Client) SecretRemove(ctx context.Context, id string) error {
-	if err := cli.NewVersionError("1.25", "secret remove"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.25", "secret remove"); err != nil {
 		return err
 	}
 	resp, err := cli.delete(ctx, "/secrets/"+id, nil, nil)

client/secret_update.go

@@ -9,7 +9,7 @@ import (
 
 // SecretUpdate attempts to update a secret.
 func (cli *Client) SecretUpdate(ctx context.Context, id string, version swarm.Version, secret swarm.SecretSpec) error {
-	if err := cli.NewVersionError("1.25", "secret update"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.25", "secret update"); err != nil {
 		return err
 	}
 	query := url.Values{}

client/service_create.go

@@ -20,6 +20,13 @@ import (
 func (cli *Client) ServiceCreate(ctx context.Context, service swarm.ServiceSpec, options types.ServiceCreateOptions) (types.ServiceCreateResponse, error) {
 	var response types.ServiceCreateResponse
 
+	// Make sure we negotiated (if the client is configured to do so),
+	// as code below contains API-version specific handling of options.
+	//
+	// Normally, version-negotiation (if enabled) would not happen until
+	// the API request is made.
+	cli.checkVersion(ctx)
+
 	// Make sure containerSpec is not nil when no runtime is set or the runtime is set to container
 	if service.TaskTemplate.ContainerSpec == nil && (service.TaskTemplate.Runtime == "" || service.TaskTemplate.Runtime == swarm.RuntimeContainer) {
 		service.TaskTemplate.ContainerSpec = &swarm.ContainerSpec{}

client/service_update.go

@@ -16,6 +16,13 @@ import (
 // It should be the value as set *before* the update. You can find this value in the Meta field
 // of swarm.Service, which can be found using ServiceInspectWithRaw.
 func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version swarm.Version, service swarm.ServiceSpec, options types.ServiceUpdateOptions) (types.ServiceUpdateResponse, error) {
+	// Make sure we negotiated (if the client is configured to do so),
+	// as code below contains API-version specific handling of options.
+	//
+	// Normally, version-negotiation (if enabled) would not happen until
+	// the API request is made.
+	cli.checkVersion(ctx)
+
 	var (
 		query    = url.Values{}
 		response = types.ServiceUpdateResponse{}

client/volume_prune.go

@@ -13,7 +13,7 @@ import (
 func (cli *Client) VolumesPrune(ctx context.Context, pruneFilters filters.Args) (types.VolumesPruneReport, error) {
 	var report types.VolumesPruneReport
 
-	if err := cli.NewVersionError("1.25", "volume prune"); err != nil {
+	if err := cli.NewVersionError(ctx, "1.25", "volume prune"); err != nil {
 		return report, err
 	}