Browse code

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>

Sebastiaan van Stijn authored on 2023/09/12 21:08:54
Showing 31 changed files
... ...
@@ -13,7 +13,7 @@ import (
13 13
 
14 14
 // BuildCachePrune requests the daemon to delete unused cache data
15 15
 func (cli *Client) BuildCachePrune(ctx context.Context, opts types.BuildCachePruneOptions) (*types.BuildCachePruneReport, error) {
16
-	if err := cli.NewVersionError("1.31", "build prune"); err != nil {
16
+	if err := cli.NewVersionError(ctx, "1.31", "build prune"); err != nil {
17 17
 		return nil, err
18 18
 	}
19 19
 
... ...
@@ -254,13 +254,21 @@ func (cli *Client) Close() error {
254 254
 	return nil
255 255
 }
256 256
 
257
+// checkVersion manually triggers API version negotiation (if configured).
258
+// This allows for version-dependent code to use the same version as will
259
+// be negotiated when making the actual requests, and for which cases
260
+// we cannot do the negotiation lazily.
261
+func (cli *Client) checkVersion(ctx context.Context) {
262
+	if cli.negotiateVersion && !cli.negotiated {
263
+		cli.NegotiateAPIVersion(ctx)
264
+	}
265
+}
266
+
257 267
 // getAPIPath returns the versioned request path to call the API.
258 268
 // It appends the query parameters to the path if they are not empty.
259 269
 func (cli *Client) getAPIPath(ctx context.Context, p string, query url.Values) string {
260 270
 	var apiPath string
261
-	if cli.negotiateVersion && !cli.negotiated {
262
-		cli.NegotiateAPIVersion(ctx)
263
-	}
271
+	cli.checkVersion(ctx)
264 272
 	if cli.version != "" {
265 273
 		v := strings.TrimPrefix(cli.version, "v")
266 274
 		apiPath = path.Join(cli.basePath, "/v"+v, p)
... ...
@@ -11,7 +11,7 @@ import (
11 11
 // ConfigCreate creates a new config.
12 12
 func (cli *Client) ConfigCreate(ctx context.Context, config swarm.ConfigSpec) (types.ConfigCreateResponse, error) {
13 13
 	var response types.ConfigCreateResponse
14
-	if err := cli.NewVersionError("1.30", "config create"); err != nil {
14
+	if err := cli.NewVersionError(ctx, "1.30", "config create"); err != nil {
15 15
 		return response, err
16 16
 	}
17 17
 	resp, err := cli.post(ctx, "/configs/create", nil, config, nil)
... ...
@@ -14,7 +14,7 @@ func (cli *Client) ConfigInspectWithRaw(ctx context.Context, id string) (swarm.C
14 14
 	if id == "" {
15 15
 		return swarm.Config{}, nil, objectNotFoundError{object: "config", id: id}
16 16
 	}
17
-	if err := cli.NewVersionError("1.30", "config inspect"); err != nil {
17
+	if err := cli.NewVersionError(ctx, "1.30", "config inspect"); err != nil {
18 18
 		return swarm.Config{}, nil, err
19 19
 	}
20 20
 	resp, err := cli.get(ctx, "/configs/"+id, nil, nil)
... ...
@@ -12,7 +12,7 @@ import (
12 12
 
13 13
 // ConfigList returns the list of configs.
14 14
 func (cli *Client) ConfigList(ctx context.Context, options types.ConfigListOptions) ([]swarm.Config, error) {
15
-	if err := cli.NewVersionError("1.30", "config list"); err != nil {
15
+	if err := cli.NewVersionError(ctx, "1.30", "config list"); err != nil {
16 16
 		return nil, err
17 17
 	}
18 18
 	query := url.Values{}
... ...
@@ -4,7 +4,7 @@ import "context"
4 4
 
5 5
 // ConfigRemove removes a config.
6 6
 func (cli *Client) ConfigRemove(ctx context.Context, id string) error {
7
-	if err := cli.NewVersionError("1.30", "config remove"); err != nil {
7
+	if err := cli.NewVersionError(ctx, "1.30", "config remove"); err != nil {
8 8
 		return err
9 9
 	}
10 10
 	resp, err := cli.delete(ctx, "/configs/"+id, nil, nil)
... ...
@@ -9,7 +9,7 @@ import (
9 9
 
10 10
 // ConfigUpdate attempts to update a config
11 11
 func (cli *Client) ConfigUpdate(ctx context.Context, id string, version swarm.Version, config swarm.ConfigSpec) error {
12
-	if err := cli.NewVersionError("1.30", "config update"); err != nil {
12
+	if err := cli.NewVersionError(ctx, "1.30", "config update"); err != nil {
13 13
 		return err
14 14
 	}
15 15
 	query := url.Values{}
... ...
@@ -23,13 +23,20 @@ type configWrapper struct {
23 23
 func (cli *Client) ContainerCreate(ctx context.Context, config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, platform *ocispec.Platform, containerName string) (container.CreateResponse, error) {
24 24
 	var response container.CreateResponse
25 25
 
26
-	if err := cli.NewVersionError("1.25", "stop timeout"); config != nil && config.StopTimeout != nil && err != nil {
26
+	// Make sure we negotiated (if the client is configured to do so),
27
+	// as code below contains API-version specific handling of options.
28
+	//
29
+	// Normally, version-negotiation (if enabled) would not happen until
30
+	// the API request is made.
31
+	cli.checkVersion(ctx)
32
+
33
+	if err := cli.NewVersionError(ctx, "1.25", "stop timeout"); config != nil && config.StopTimeout != nil && err != nil {
27 34
 		return response, err
28 35
 	}
29
-	if err := cli.NewVersionError("1.41", "specify container image platform"); platform != nil && err != nil {
36
+	if err := cli.NewVersionError(ctx, "1.41", "specify container image platform"); platform != nil && err != nil {
30 37
 		return response, err
31 38
 	}
32
-	if err := cli.NewVersionError("1.44", "specify health-check start interval"); config != nil && config.Healthcheck != nil && config.Healthcheck.StartInterval != 0 && err != nil {
39
+	if err := cli.NewVersionError(ctx, "1.44", "specify health-check start interval"); config != nil && config.Healthcheck != nil && config.Healthcheck.StartInterval != 0 && err != nil {
33 40
 		return response, err
34 41
 	}
35 42
 
... ...
@@ -13,7 +13,14 @@ import (
13 13
 func (cli *Client) ContainerExecCreate(ctx context.Context, container string, config types.ExecConfig) (types.IDResponse, error) {
14 14
 	var response types.IDResponse
15 15
 
16
-	if err := cli.NewVersionError("1.25", "env"); len(config.Env) != 0 && err != nil {
16
+	// Make sure we negotiated (if the client is configured to do so),
17
+	// as code below contains API-version specific handling of options.
18
+	//
19
+	// Normally, version-negotiation (if enabled) would not happen until
20
+	// the API request is made.
21
+	cli.checkVersion(ctx)
22
+
23
+	if err := cli.NewVersionError(ctx, "1.25", "env"); len(config.Env) != 0 && err != nil {
17 24
 		return response, err
18 25
 	}
19 26
 	if versions.LessThan(cli.ClientVersion(), "1.42") {
... ...
@@ -13,7 +13,7 @@ import (
13 13
 func (cli *Client) ContainersPrune(ctx context.Context, pruneFilters filters.Args) (types.ContainersPruneReport, error) {
14 14
 	var report types.ContainersPruneReport
15 15
 
16
-	if err := cli.NewVersionError("1.25", "container prune"); err != nil {
16
+	if err := cli.NewVersionError(ctx, "1.25", "container prune"); err != nil {
17 17
 		return report, err
18 18
 	}
19 19
 
... ...
@@ -17,8 +17,16 @@ func (cli *Client) ContainerRestart(ctx context.Context, containerID string, opt
17 17
 	if options.Timeout != nil {
18 18
 		query.Set("t", strconv.Itoa(*options.Timeout))
19 19
 	}
20
-	if options.Signal != "" && versions.GreaterThanOrEqualTo(cli.version, "1.42") {
21
-		query.Set("signal", options.Signal)
20
+	if options.Signal != "" {
21
+		// Make sure we negotiated (if the client is configured to do so),
22
+		// as code below contains API-version specific handling of options.
23
+		//
24
+		// Normally, version-negotiation (if enabled) would not happen until
25
+		// the API request is made.
26
+		cli.checkVersion(ctx)
27
+		if versions.GreaterThanOrEqualTo(cli.version, "1.42") {
28
+			query.Set("signal", options.Signal)
29
+		}
22 30
 	}
23 31
 	resp, err := cli.post(ctx, "/containers/"+containerID+"/restart", query, nil, nil)
24 32
 	ensureReaderClosed(resp)
... ...
@@ -21,8 +21,16 @@ func (cli *Client) ContainerStop(ctx context.Context, containerID string, option
21 21
 	if options.Timeout != nil {
22 22
 		query.Set("t", strconv.Itoa(*options.Timeout))
23 23
 	}
24
-	if options.Signal != "" && versions.GreaterThanOrEqualTo(cli.version, "1.42") {
25
-		query.Set("signal", options.Signal)
24
+	if options.Signal != "" {
25
+		// Make sure we negotiated (if the client is configured to do so),
26
+		// as code below contains API-version specific handling of options.
27
+		//
28
+		// Normally, version-negotiation (if enabled) would not happen until
29
+		// the API request is made.
30
+		cli.checkVersion(ctx)
31
+		if versions.GreaterThanOrEqualTo(cli.version, "1.42") {
32
+			query.Set("signal", options.Signal)
33
+		}
26 34
 	}
27 35
 	resp, err := cli.post(ctx, "/containers/"+containerID+"/stop", query, nil, nil)
28 36
 	ensureReaderClosed(resp)
... ...
@@ -30,6 +30,12 @@ const containerWaitErrorMsgLimit = 2 * 1024 /* Max: 2KiB */
30 30
 // synchronize ContainerWait with other calls, such as specifying a
31 31
 // "next-exit" condition before issuing a ContainerStart request.
32 32
 func (cli *Client) ContainerWait(ctx context.Context, containerID string, condition container.WaitCondition) (<-chan container.WaitResponse, <-chan error) {
33
+	// Make sure we negotiated (if the client is configured to do so),
34
+	// as code below contains API-version specific handling of options.
35
+	//
36
+	// Normally, version-negotiation (if enabled) would not happen until
37
+	// the API request is made.
38
+	cli.checkVersion(ctx)
33 39
 	if versions.LessThan(cli.ClientVersion(), "1.30") {
34 40
 		return cli.legacyContainerWait(ctx, containerID)
35 41
 	}
... ...
@@ -17,7 +17,7 @@ func (cli *Client) DistributionInspect(ctx context.Context, image, encodedRegist
17 17
 		return distributionInspect, objectNotFoundError{object: "distribution", id: image}
18 18
 	}
19 19
 
20
-	if err := cli.NewVersionError("1.30", "distribution inspect"); err != nil {
20
+	if err := cli.NewVersionError(ctx, "1.30", "distribution inspect"); err != nil {
21 21
 		return distributionInspect, err
22 22
 	}
23 23
 
... ...
@@ -1,6 +1,7 @@
1 1
 package client // import "github.com/docker/docker/client"
2 2
 
3 3
 import (
4
+	"context"
4 5
 	"fmt"
5 6
 
6 7
 	"github.com/docker/docker/api/types/versions"
... ...
@@ -48,9 +49,18 @@ func (e objectNotFoundError) Error() string {
48 48
 	return fmt.Sprintf("Error: No such %s: %s", e.object, e.id)
49 49
 }
50 50
 
51
-// NewVersionError returns an error if the APIVersion required
52
-// if less than the current supported version
53
-func (cli *Client) NewVersionError(APIrequired, feature string) error {
51
+// NewVersionError returns an error if the APIVersion required is less than the
52
+// current supported version.
53
+//
54
+// It performs API-version negotiation if the Client is configured with this
55
+// option, otherwise it assumes the latest API version is used.
56
+func (cli *Client) NewVersionError(ctx context.Context, APIrequired, feature string) error {
57
+	// Make sure we negotiated (if the client is configured to do so),
58
+	// as code below contains API-version specific handling of options.
59
+	//
60
+	// Normally, version-negotiation (if enabled) would not happen until
61
+	// the API request is made.
62
+	cli.checkVersion(ctx)
54 63
 	if cli.version != "" && versions.LessThan(cli.version, APIrequired) {
55 64
 		return fmt.Errorf("%q requires API version %s, but the Docker daemon API version is %s", feature, APIrequired, cli.version)
56 65
 	}
... ...
@@ -18,7 +18,7 @@ import (
18 18
 // The Body in the response implements an io.ReadCloser and it's up to the caller to
19 19
 // close it.
20 20
 func (cli *Client) ImageBuild(ctx context.Context, buildContext io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) {
21
-	query, err := cli.imageBuildOptionsToQuery(options)
21
+	query, err := cli.imageBuildOptionsToQuery(ctx, options)
22 22
 	if err != nil {
23 23
 		return types.ImageBuildResponse{}, err
24 24
 	}
... ...
@@ -43,7 +43,7 @@ func (cli *Client) ImageBuild(ctx context.Context, buildContext io.Reader, optio
43 43
 	}, nil
44 44
 }
45 45
 
46
-func (cli *Client) imageBuildOptionsToQuery(options types.ImageBuildOptions) (url.Values, error) {
46
+func (cli *Client) imageBuildOptionsToQuery(ctx context.Context, options types.ImageBuildOptions) (url.Values, error) {
47 47
 	query := url.Values{
48 48
 		"t":           options.Tags,
49 49
 		"securityopt": options.SecurityOpt,
... ...
@@ -73,7 +73,7 @@ func (cli *Client) imageBuildOptionsToQuery(options types.ImageBuildOptions) (ur
73 73
 	}
74 74
 
75 75
 	if options.Squash {
76
-		if err := cli.NewVersionError("1.25", "squash"); err != nil {
76
+		if err := cli.NewVersionError(ctx, "1.25", "squash"); err != nil {
77 77
 			return query, err
78 78
 		}
79 79
 		query.Set("squash", "1")
... ...
@@ -123,7 +123,7 @@ func (cli *Client) imageBuildOptionsToQuery(options types.ImageBuildOptions) (ur
123 123
 		query.Set("session", options.SessionID)
124 124
 	}
125 125
 	if options.Platform != "" {
126
-		if err := cli.NewVersionError("1.32", "platform"); err != nil {
126
+		if err := cli.NewVersionError(ctx, "1.32", "platform"); err != nil {
127 127
 			return query, err
128 128
 		}
129 129
 		query.Set("platform", strings.ToLower(options.Platform))
... ...
@@ -12,6 +12,13 @@ import (
12 12
 
13 13
 // ImageList returns a list of images in the docker host.
14 14
 func (cli *Client) ImageList(ctx context.Context, options types.ImageListOptions) ([]types.ImageSummary, error) {
15
+	// Make sure we negotiated (if the client is configured to do so),
16
+	// as code below contains API-version specific handling of options.
17
+	//
18
+	// Normally, version-negotiation (if enabled) would not happen until
19
+	// the API request is made.
20
+	cli.checkVersion(ctx)
21
+
15 22
 	var images []types.ImageSummary
16 23
 	query := url.Values{}
17 24
 
... ...
@@ -13,7 +13,7 @@ import (
13 13
 func (cli *Client) ImagesPrune(ctx context.Context, pruneFilters filters.Args) (types.ImagesPruneReport, error) {
14 14
 	var report types.ImagesPruneReport
15 15
 
16
-	if err := cli.NewVersionError("1.25", "image prune"); err != nil {
16
+	if err := cli.NewVersionError(ctx, "1.25", "image prune"); err != nil {
17 17
 		return report, err
18 18
 	}
19 19
 
... ...
@@ -10,6 +10,13 @@ import (
10 10
 
11 11
 // NetworkCreate creates a new network in the docker host.
12 12
 func (cli *Client) NetworkCreate(ctx context.Context, name string, options types.NetworkCreate) (types.NetworkCreateResponse, error) {
13
+	// Make sure we negotiated (if the client is configured to do so),
14
+	// as code below contains API-version specific handling of options.
15
+	//
16
+	// Normally, version-negotiation (if enabled) would not happen until
17
+	// the API request is made.
18
+	cli.checkVersion(ctx)
19
+
13 20
 	networkCreateRequest := types.NetworkCreateRequest{
14 21
 		NetworkCreate: options,
15 22
 		Name:          name,
... ...
@@ -13,7 +13,7 @@ import (
13 13
 func (cli *Client) NetworksPrune(ctx context.Context, pruneFilters filters.Args) (types.NetworksPruneReport, error) {
14 14
 	var report types.NetworksPruneReport
15 15
 
16
-	if err := cli.NewVersionError("1.25", "network prune"); err != nil {
16
+	if err := cli.NewVersionError(ctx, "1.25", "network prune"); err != nil {
17 17
 		return report, err
18 18
 	}
19 19
 
... ...
@@ -14,7 +14,7 @@ import (
14 14
 
15 15
 // PluginUpgrade upgrades a plugin
16 16
 func (cli *Client) PluginUpgrade(ctx context.Context, name string, options types.PluginInstallOptions) (rc io.ReadCloser, err error) {
17
-	if err := cli.NewVersionError("1.26", "plugin upgrade"); err != nil {
17
+	if err := cli.NewVersionError(ctx, "1.26", "plugin upgrade"); err != nil {
18 18
 		return nil, err
19 19
 	}
20 20
 	query := url.Values{}
... ...
@@ -11,7 +11,7 @@ import (
11 11
 // SecretCreate creates a new secret.
12 12
 func (cli *Client) SecretCreate(ctx context.Context, secret swarm.SecretSpec) (types.SecretCreateResponse, error) {
13 13
 	var response types.SecretCreateResponse
14
-	if err := cli.NewVersionError("1.25", "secret create"); err != nil {
14
+	if err := cli.NewVersionError(ctx, "1.25", "secret create"); err != nil {
15 15
 		return response, err
16 16
 	}
17 17
 	resp, err := cli.post(ctx, "/secrets/create", nil, secret, nil)
... ...
@@ -11,7 +11,7 @@ import (
11 11
 
12 12
 // SecretInspectWithRaw returns the secret information with raw data
13 13
 func (cli *Client) SecretInspectWithRaw(ctx context.Context, id string) (swarm.Secret, []byte, error) {
14
-	if err := cli.NewVersionError("1.25", "secret inspect"); err != nil {
14
+	if err := cli.NewVersionError(ctx, "1.25", "secret inspect"); err != nil {
15 15
 		return swarm.Secret{}, nil, err
16 16
 	}
17 17
 	if id == "" {
... ...
@@ -12,7 +12,7 @@ import (
12 12
 
13 13
 // SecretList returns the list of secrets.
14 14
 func (cli *Client) SecretList(ctx context.Context, options types.SecretListOptions) ([]swarm.Secret, error) {
15
-	if err := cli.NewVersionError("1.25", "secret list"); err != nil {
15
+	if err := cli.NewVersionError(ctx, "1.25", "secret list"); err != nil {
16 16
 		return nil, err
17 17
 	}
18 18
 	query := url.Values{}
... ...
@@ -4,7 +4,7 @@ import "context"
4 4
 
5 5
 // SecretRemove removes a secret.
6 6
 func (cli *Client) SecretRemove(ctx context.Context, id string) error {
7
-	if err := cli.NewVersionError("1.25", "secret remove"); err != nil {
7
+	if err := cli.NewVersionError(ctx, "1.25", "secret remove"); err != nil {
8 8
 		return err
9 9
 	}
10 10
 	resp, err := cli.delete(ctx, "/secrets/"+id, nil, nil)
... ...
@@ -9,7 +9,7 @@ import (
9 9
 
10 10
 // SecretUpdate attempts to update a secret.
11 11
 func (cli *Client) SecretUpdate(ctx context.Context, id string, version swarm.Version, secret swarm.SecretSpec) error {
12
-	if err := cli.NewVersionError("1.25", "secret update"); err != nil {
12
+	if err := cli.NewVersionError(ctx, "1.25", "secret update"); err != nil {
13 13
 		return err
14 14
 	}
15 15
 	query := url.Values{}
... ...
@@ -20,6 +20,13 @@ import (
20 20
 func (cli *Client) ServiceCreate(ctx context.Context, service swarm.ServiceSpec, options types.ServiceCreateOptions) (types.ServiceCreateResponse, error) {
21 21
 	var response types.ServiceCreateResponse
22 22
 
23
+	// Make sure we negotiated (if the client is configured to do so),
24
+	// as code below contains API-version specific handling of options.
25
+	//
26
+	// Normally, version-negotiation (if enabled) would not happen until
27
+	// the API request is made.
28
+	cli.checkVersion(ctx)
29
+
23 30
 	// Make sure containerSpec is not nil when no runtime is set or the runtime is set to container
24 31
 	if service.TaskTemplate.ContainerSpec == nil && (service.TaskTemplate.Runtime == "" || service.TaskTemplate.Runtime == swarm.RuntimeContainer) {
25 32
 		service.TaskTemplate.ContainerSpec = &swarm.ContainerSpec{}
... ...
@@ -16,6 +16,13 @@ import (
16 16
 // It should be the value as set *before* the update. You can find this value in the Meta field
17 17
 // of swarm.Service, which can be found using ServiceInspectWithRaw.
18 18
 func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version swarm.Version, service swarm.ServiceSpec, options types.ServiceUpdateOptions) (types.ServiceUpdateResponse, error) {
19
+	// Make sure we negotiated (if the client is configured to do so),
20
+	// as code below contains API-version specific handling of options.
21
+	//
22
+	// Normally, version-negotiation (if enabled) would not happen until
23
+	// the API request is made.
24
+	cli.checkVersion(ctx)
25
+
19 26
 	var (
20 27
 		query    = url.Values{}
21 28
 		response = types.ServiceUpdateResponse{}
... ...
@@ -13,7 +13,7 @@ import (
13 13
 func (cli *Client) VolumesPrune(ctx context.Context, pruneFilters filters.Args) (types.VolumesPruneReport, error) {
14 14
 	var report types.VolumesPruneReport
15 15
 
16
-	if err := cli.NewVersionError("1.25", "volume prune"); err != nil {
16
+	if err := cli.NewVersionError(ctx, "1.25", "volume prune"); err != nil {
17 17
 		return report, err
18 18
 	}
19 19
 
... ...
@@ -10,8 +10,14 @@ import (
10 10
 // VolumeRemove removes a volume from the docker host.
11 11
 func (cli *Client) VolumeRemove(ctx context.Context, volumeID string, force bool) error {
12 12
 	query := url.Values{}
13
-	if versions.GreaterThanOrEqualTo(cli.version, "1.25") {
14
-		if force {
13
+	if force {
14
+		// Make sure we negotiated (if the client is configured to do so),
15
+		// as code below contains API-version specific handling of options.
16
+		//
17
+		// Normally, version-negotiation (if enabled) would not happen until
18
+		// the API request is made.
19
+		cli.checkVersion(ctx)
20
+		if versions.GreaterThanOrEqualTo(cli.version, "1.25") {
15 21
 			query.Set("force", "1")
16 22
 		}
17 23
 	}
... ...
@@ -11,7 +11,7 @@ import (
11 11
 // VolumeUpdate updates a volume. This only works for Cluster Volumes, and
12 12
 // only some fields can be updated.
13 13
 func (cli *Client) VolumeUpdate(ctx context.Context, volumeID string, version swarm.Version, options volume.UpdateOptions) error {
14
-	if err := cli.NewVersionError("1.42", "volume update"); err != nil {
14
+	if err := cli.NewVersionError(ctx, "1.42", "volume update"); err != nil {
15 15
 		return err
16 16
 	}
17 17