Merge pull request #51005 from thaJeztah/cleanup_version_gates

client: fix some version-related handling

client/image_list.go

@@ -30,16 +30,8 @@ func (cli *Client) ImageList(ctx context.Context, options ImageListOptions) ([]i
 
 	query := url.Values{}
 
-	optionFilters := options.Filters
-	referenceFilters := optionFilters.Get("reference")
-	if versions.LessThan(cli.version, "1.25") && len(referenceFilters) > 0 {
-		query.Set("filter", referenceFilters[0])
-		for _, filterValue := range referenceFilters {
-			optionFilters.Del("reference", filterValue)
-		}
-	}
-	if optionFilters.Len() > 0 {
-		filterJSON, err := filters.ToJSON(optionFilters)
+	if options.Filters.Len() > 0 {
+		filterJSON, err := filters.ToJSON(options.Filters)
 		if err != nil {
 			return images, err
 		}

client/image_list_test.go

@@ -114,45 +114,6 @@ func TestImageList(t *testing.T) {
 	}
 }
 
-func TestImageListApiBefore125(t *testing.T) {
-	expectedFilter := "image:tag"
-	client, err := NewClientWithOpts(WithMockClient(func(req *http.Request) (*http.Response, error) {
-		query := req.URL.Query()
-		actualFilter := query.Get("filter")
-		if actualFilter != expectedFilter {
-			return nil, fmt.Errorf("filter not set in URL query properly. Expected '%s', got %s", expectedFilter, actualFilter)
-		}
-		actualFilters := query.Get("filters")
-		if actualFilters != "" {
-			return nil, fmt.Errorf("filters should have not been present, were with value: %s", actualFilters)
-		}
-		content, err := json.Marshal([]image.Summary{
-			{
-				ID: "image_id2",
-			},
-			{
-				ID: "image_id2",
-			},
-		})
-		if err != nil {
-			return nil, err
-		}
-		return &http.Response{
-			StatusCode: http.StatusOK,
-			Body:       io.NopCloser(bytes.NewReader(content)),
-		}, nil
-	}), WithVersion("1.24"))
-	assert.NilError(t, err)
-
-	options := ImageListOptions{
-		Filters: filters.NewArgs(filters.Arg("reference", "image:tag")),
-	}
-
-	images, err := client.ImageList(context.Background(), options)
-	assert.NilError(t, err)
-	assert.Check(t, is.Len(images, 2))
-}
-
 // Checks if shared-size query parameter is set/not being set correctly
 // for /images/json.
 func TestImageListWithSharedSize(t *testing.T) {

client/service_create.go

@@ -33,14 +33,9 @@ func (cli *Client) ServiceCreate(ctx context.Context, service swarm.ServiceSpec,
 		service.TaskTemplate.ContainerSpec = &swarm.ContainerSpec{}
 	}
 
-	if err := validateServiceSpec(service); err != nil {
+	if err := validateServiceSpec(service, cli.version); err != nil {
 		return response, err
 	}
-	if versions.LessThan(cli.version, "1.30") {
-		if err := validateServiceSpecForAPIVersion(service, cli.version); err != nil {
-			return response, err
-		}
-	}
 
 	// ensure that the image is tagged
 	var resolveWarning string
@@ -62,12 +57,6 @@ func (cli *Client) ServiceCreate(ctx context.Context, service swarm.ServiceSpec,
 	}
 
 	headers := http.Header{}
-	if versions.LessThan(cli.version, "1.30") {
-		// the custom "version" header was used by engine API before 20.10
-		// (API 1.30) to switch between client- and server-side lookup of
-		// image digests.
-		headers["version"] = []string{cli.version}
-	}
 	if options.EncodedRegistryAuth != "" {
 		headers[registry.AuthHeader] = []string{options.EncodedRegistryAuth}
 	}
@@ -183,7 +172,7 @@ func digestWarning(image string) string {
 	return fmt.Sprintf("image %s could not be accessed on a registry to record\nits digest. Each node will access %s independently,\npossibly leading to different nodes running different\nversions of the image.\n", image, image)
 }
 
-func validateServiceSpec(s swarm.ServiceSpec) error {
+func validateServiceSpec(s swarm.ServiceSpec, apiVersion string) error {
 	if s.TaskTemplate.ContainerSpec != nil && s.TaskTemplate.PluginSpec != nil {
 		return errors.New("must not specify both a container spec and a plugin spec in the task template")
 	}
@@ -193,18 +182,16 @@ func validateServiceSpec(s swarm.ServiceSpec) error {
 	if s.TaskTemplate.ContainerSpec != nil && (s.TaskTemplate.Runtime != "" && s.TaskTemplate.Runtime != swarm.RuntimeContainer) {
 		return errors.New("mismatched runtime with container spec")
 	}
-	return nil
-}
-
-func validateServiceSpecForAPIVersion(c swarm.ServiceSpec, apiVersion string) error {
-	for _, m := range c.TaskTemplate.ContainerSpec.Mounts {
-		if m.BindOptions != nil {
-			if m.BindOptions.NonRecursive && versions.LessThan(apiVersion, "1.40") {
-				return errors.New("bind-recursive=disabled requires API v1.40 or later")
-			}
-			// ReadOnlyNonRecursive can be safely ignored when API < 1.44
-			if m.BindOptions.ReadOnlyForceRecursive && versions.LessThan(apiVersion, "1.44") {
-				return errors.New("bind-recursive=readonly requires API v1.44 or later")
+	if s.TaskTemplate.ContainerSpec != nil && apiVersion != "" && versions.LessThan(apiVersion, "1.44") {
+		for _, m := range s.TaskTemplate.ContainerSpec.Mounts {
+			if m.BindOptions != nil {
+				if m.BindOptions.NonRecursive && versions.LessThan(apiVersion, "1.40") {
+					return errors.New("bind-recursive=disabled requires API v1.40 or later")
+				}
+				// ReadOnlyNonRecursive can be safely ignored when API < 1.44
+				if m.BindOptions.ReadOnlyForceRecursive && versions.LessThan(apiVersion, "1.44") {
+					return errors.New("bind-recursive=readonly requires API v1.44 or later")
+				}
 			}
 		}
 	}

client/service_update.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/moby/moby/api/types/registry"
 	"github.com/moby/moby/api/types/swarm"
-	"github.com/moby/moby/api/types/versions"
 )
 
 // ServiceUpdate updates a Service. The version number is required to avoid
@@ -29,6 +28,9 @@ func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version
 	if err := cli.checkVersion(ctx); err != nil {
 		return swarm.ServiceUpdateResponse{}, err
 	}
+	if err := validateServiceSpec(service, cli.version); err != nil {
+		return swarm.ServiceUpdateResponse{}, err
+	}
 
 	query := url.Values{}
 	if options.RegistryAuthFrom != "" {
@@ -41,10 +43,6 @@ func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version
 
 	query.Set("version", version.String())
 
-	if err := validateServiceSpec(service); err != nil {
-		return swarm.ServiceUpdateResponse{}, err
-	}
-
 	// ensure that the image is tagged
 	var resolveWarning string
 	switch {
@@ -65,12 +63,6 @@ func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version
 	}
 
 	headers := http.Header{}
-	if versions.LessThan(cli.version, "1.30") {
-		// the custom "version" header was used by engine API before 20.10
-		// (API 1.30) to switch between client- and server-side lookup of
-		// image digests.
-		headers["version"] = []string{cli.version}
-	}
 	if options.EncodedRegistryAuth != "" {
 		headers.Set(registry.AuthHeader, options.EncodedRegistryAuth)
 	}

client/volume_remove.go

@@ -3,8 +3,6 @@ package client
 import (
 	"context"
 	"net/url"
-
-	"github.com/moby/moby/api/types/versions"
 )
 
 // VolumeRemove removes a volume from the docker host.
@@ -16,17 +14,7 @@ func (cli *Client) VolumeRemove(ctx context.Context, volumeID string, force bool
 
 	query := url.Values{}
 	if force {
-		// 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.
-		if err := cli.checkVersion(ctx); err != nil {
-			return err
-		}
-		if versions.GreaterThanOrEqualTo(cli.version, "1.25") {
-			query.Set("force", "1")
-		}
+		query.Set("force", "1")
 	}
 	resp, err := cli.delete(ctx, "/volumes/"+volumeID, query, nil)
 	defer ensureReaderClosed(resp)

client/volume_remove_test.go

@@ -3,6 +3,7 @@ package client
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"io"
 	"net/http"
 	"testing"
@@ -47,6 +48,10 @@ func TestVolumeRemove(t *testing.T) {
 		if err := assertRequest(req, http.MethodDelete, expectedURL); err != nil {
 			return nil, err
 		}
+		if v := req.URL.Query().Get("force"); v != "1" {
+			return nil, fmt.Errorf("expected force=1, got %s", v)
+		}
+
 		return &http.Response{
 			StatusCode: http.StatusOK,
 			Body:       io.NopCloser(bytes.NewReader([]byte("body"))),
@@ -54,6 +59,6 @@ func TestVolumeRemove(t *testing.T) {
 	}))
 	assert.NilError(t, err)
 
-	err = client.VolumeRemove(context.Background(), "volume_id", false)
+	err = client.VolumeRemove(context.Background(), "volume_id", true)
 	assert.NilError(t, err)
 }

daemon/server/router/image/image_routes.go

@@ -427,10 +427,12 @@ func (ir *imageRouter) getImagesJSON(ctx context.Context, w http.ResponseWriter,
 	}
 
 	version := httputils.VersionFromContext(ctx)
-	if versions.LessThan(version, "1.41") {
+
+	// clients may be actively removing the new filter on API 1.24
+	// and under: https://github.com/moby/moby/blob/v28.4.0/client/image_list.go#L34-L40
+	if versions.LessThan(version, "1.25") && !imageFilters.Contains("reference") {
 		// NOTE: filter is a shell glob string applied to repository names.
-		filterParam := r.Form.Get("filter")
-		if filterParam != "" {
+		if filterParam := r.Form.Get("filter"); filterParam != "" {
 			imageFilters.Add("reference", filterParam)
 		}
 	}

vendor/github.com/moby/moby/client/image_list.go

@@ -30,16 +30,8 @@ func (cli *Client) ImageList(ctx context.Context, options ImageListOptions) ([]i
 
 	query := url.Values{}
 
-	optionFilters := options.Filters
-	referenceFilters := optionFilters.Get("reference")
-	if versions.LessThan(cli.version, "1.25") && len(referenceFilters) > 0 {
-		query.Set("filter", referenceFilters[0])
-		for _, filterValue := range referenceFilters {
-			optionFilters.Del("reference", filterValue)
-		}
-	}
-	if optionFilters.Len() > 0 {
-		filterJSON, err := filters.ToJSON(optionFilters)
+	if options.Filters.Len() > 0 {
+		filterJSON, err := filters.ToJSON(options.Filters)
 		if err != nil {
 			return images, err
 		}

vendor/github.com/moby/moby/client/service_create.go

@@ -33,14 +33,9 @@ func (cli *Client) ServiceCreate(ctx context.Context, service swarm.ServiceSpec,
 		service.TaskTemplate.ContainerSpec = &swarm.ContainerSpec{}
 	}
 
-	if err := validateServiceSpec(service); err != nil {
+	if err := validateServiceSpec(service, cli.version); err != nil {
 		return response, err
 	}
-	if versions.LessThan(cli.version, "1.30") {
-		if err := validateServiceSpecForAPIVersion(service, cli.version); err != nil {
-			return response, err
-		}
-	}
 
 	// ensure that the image is tagged
 	var resolveWarning string
@@ -62,12 +57,6 @@ func (cli *Client) ServiceCreate(ctx context.Context, service swarm.ServiceSpec,
 	}
 
 	headers := http.Header{}
-	if versions.LessThan(cli.version, "1.30") {
-		// the custom "version" header was used by engine API before 20.10
-		// (API 1.30) to switch between client- and server-side lookup of
-		// image digests.
-		headers["version"] = []string{cli.version}
-	}
 	if options.EncodedRegistryAuth != "" {
 		headers[registry.AuthHeader] = []string{options.EncodedRegistryAuth}
 	}
@@ -183,7 +172,7 @@ func digestWarning(image string) string {
 	return fmt.Sprintf("image %s could not be accessed on a registry to record\nits digest. Each node will access %s independently,\npossibly leading to different nodes running different\nversions of the image.\n", image, image)
 }
 
-func validateServiceSpec(s swarm.ServiceSpec) error {
+func validateServiceSpec(s swarm.ServiceSpec, apiVersion string) error {
 	if s.TaskTemplate.ContainerSpec != nil && s.TaskTemplate.PluginSpec != nil {
 		return errors.New("must not specify both a container spec and a plugin spec in the task template")
 	}
@@ -193,18 +182,16 @@ func validateServiceSpec(s swarm.ServiceSpec) error {
 	if s.TaskTemplate.ContainerSpec != nil && (s.TaskTemplate.Runtime != "" && s.TaskTemplate.Runtime != swarm.RuntimeContainer) {
 		return errors.New("mismatched runtime with container spec")
 	}
-	return nil
-}
-
-func validateServiceSpecForAPIVersion(c swarm.ServiceSpec, apiVersion string) error {
-	for _, m := range c.TaskTemplate.ContainerSpec.Mounts {
-		if m.BindOptions != nil {
-			if m.BindOptions.NonRecursive && versions.LessThan(apiVersion, "1.40") {
-				return errors.New("bind-recursive=disabled requires API v1.40 or later")
-			}
-			// ReadOnlyNonRecursive can be safely ignored when API < 1.44
-			if m.BindOptions.ReadOnlyForceRecursive && versions.LessThan(apiVersion, "1.44") {
-				return errors.New("bind-recursive=readonly requires API v1.44 or later")
+	if s.TaskTemplate.ContainerSpec != nil && apiVersion != "" && versions.LessThan(apiVersion, "1.44") {
+		for _, m := range s.TaskTemplate.ContainerSpec.Mounts {
+			if m.BindOptions != nil {
+				if m.BindOptions.NonRecursive && versions.LessThan(apiVersion, "1.40") {
+					return errors.New("bind-recursive=disabled requires API v1.40 or later")
+				}
+				// ReadOnlyNonRecursive can be safely ignored when API < 1.44
+				if m.BindOptions.ReadOnlyForceRecursive && versions.LessThan(apiVersion, "1.44") {
+					return errors.New("bind-recursive=readonly requires API v1.44 or later")
+				}
 			}
 		}
 	}

vendor/github.com/moby/moby/client/service_update.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/moby/moby/api/types/registry"
 	"github.com/moby/moby/api/types/swarm"
-	"github.com/moby/moby/api/types/versions"
 )
 
 // ServiceUpdate updates a Service. The version number is required to avoid
@@ -29,6 +28,9 @@ func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version
 	if err := cli.checkVersion(ctx); err != nil {
 		return swarm.ServiceUpdateResponse{}, err
 	}
+	if err := validateServiceSpec(service, cli.version); err != nil {
+		return swarm.ServiceUpdateResponse{}, err
+	}
 
 	query := url.Values{}
 	if options.RegistryAuthFrom != "" {
@@ -41,10 +43,6 @@ func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version
 
 	query.Set("version", version.String())
 
-	if err := validateServiceSpec(service); err != nil {
-		return swarm.ServiceUpdateResponse{}, err
-	}
-
 	// ensure that the image is tagged
 	var resolveWarning string
 	switch {
@@ -65,12 +63,6 @@ func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version
 	}
 
 	headers := http.Header{}
-	if versions.LessThan(cli.version, "1.30") {
-		// the custom "version" header was used by engine API before 20.10
-		// (API 1.30) to switch between client- and server-side lookup of
-		// image digests.
-		headers["version"] = []string{cli.version}
-	}
 	if options.EncodedRegistryAuth != "" {
 		headers.Set(registry.AuthHeader, options.EncodedRegistryAuth)
 	}

vendor/github.com/moby/moby/client/volume_remove.go

@@ -3,8 +3,6 @@ package client
 import (
 	"context"
 	"net/url"
-
-	"github.com/moby/moby/api/types/versions"
 )
 
 // VolumeRemove removes a volume from the docker host.
@@ -16,17 +14,7 @@ func (cli *Client) VolumeRemove(ctx context.Context, volumeID string, force bool
 
 	query := url.Values{}
 	if force {
-		// 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.
-		if err := cli.checkVersion(ctx); err != nil {
-			return err
-		}
-		if versions.GreaterThanOrEqualTo(cli.version, "1.25") {
-			query.Set("force", "1")
-		}
+		query.Set("force", "1")
 	}
 	resp, err := cli.delete(ctx, "/volumes/"+volumeID, query, nil)
 	defer ensureReaderClosed(resp)