Merge pull request #51763 from thaJeztah/client_linting

client: fix linting issues

.golangci.yml

@@ -343,6 +343,11 @@ linters:
         linters:
           - forbidigo
 
+      # These interfaces in the client module are identical by design to allow future expansion.
+      - text: "^identical: interface '(ContainerExportResult|ContainerLogsResult|ImagePullResponse|ImagePushResponse|ImageImportResult|ImageLoadResult|ImageSaveResult|ServiceLogsResult|TaskLogsResult)'"
+        linters:
+          - iface
+
     # Log a warning if an exclusion rule is unused.
     # Default: false
     warn-unused: true

client/client_options.go

@@ -133,8 +133,6 @@ func (tf testRoundTripper) RoundTrip(req *http.Request) (*http.Response, error)
 	return tf(req)
 }
 
-func (testRoundTripper) skipConfigureTransport() bool { return true }
-
 // WithHostFromEnv overrides the client host with the host specified in the
 // DOCKER_HOST ([EnvOverrideHost]) environment variable. If DOCKER_HOST is not set,
 // or set to an empty value, the host is not modified.

client/client_test.go

@@ -367,6 +367,7 @@ func TestNegotiateAPIVersionOverride(t *testing.T) {
 	_, err = client.Ping(t.Context(), PingOptions{
 		NegotiateAPIVersion: true,
 	})
+	assert.NilError(t, err)
 	assert.Check(t, is.Equal(client.ClientVersion(), expected))
 }
 
@@ -381,6 +382,8 @@ func TestNegotiateAPIVersionConnectionFailure(t *testing.T) {
 	_, err = client.Ping(t.Context(), PingOptions{
 		NegotiateAPIVersion: true,
 	})
+	assert.Check(t, is.ErrorType(err, IsErrConnectionFailed))
+	assert.Check(t, is.ErrorContains(err, `failed to connect to the docker API at tcp://no-such-host.invalid`))
 	assert.Check(t, is.Equal(client.ClientVersion(), expected))
 }
 
@@ -425,6 +428,7 @@ func TestNegotiateAPIVersionWithEmptyVersion(t *testing.T) {
 	_, err = client.Ping(t.Context(), PingOptions{
 		NegotiateAPIVersion: true,
 	})
+	assert.NilError(t, err)
 	assert.Check(t, is.Equal(client.ClientVersion(), expected))
 }
 

client/container_exec.go

@@ -5,7 +5,7 @@ import (
 	"encoding/json"
 	"net/http"
 
-	"github.com/containerd/errdefs"
+	cerrdefs "github.com/containerd/errdefs"
 	"github.com/moby/moby/api/types/container"
 )
 
@@ -152,7 +152,7 @@ func (cli *Client) ExecAttach(ctx context.Context, execID string, options ExecAt
 func getConsoleSize(hasTTY bool, consoleSize ConsoleSize) (*[2]uint, error) {
 	if consoleSize.Height != 0 || consoleSize.Width != 0 {
 		if !hasTTY {
-			return nil, errdefs.ErrInvalidArgument.WithMessage("console size is only supported when TTY is enabled")
+			return nil, cerrdefs.ErrInvalidArgument.WithMessage("console size is only supported when TTY is enabled")
 		}
 		return &[2]uint{consoleSize.Height, consoleSize.Width}, nil
 	}

client/container_exec_test.go

@@ -2,11 +2,11 @@ package client
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"testing"
 
-	"github.com/containerd/errdefs"
 	cerrdefs "github.com/containerd/errdefs"
 	"github.com/moby/moby/api/types/container"
 	"gotest.tools/v3/assert"
@@ -150,7 +150,7 @@ func TestExecStartConsoleSize(t *testing.T) {
 			client, err := New(
 				WithMockClient(func(req *http.Request) (*http.Response, error) {
 					if tc.expErr != "" {
-						return nil, fmt.Errorf("should not have made API request")
+						return nil, errors.New("should not have made API request")
 					}
 					if err := json.NewDecoder(req.Body).Decode(&actualReq); err != nil {
 						return nil, err
@@ -163,7 +163,7 @@ func TestExecStartConsoleSize(t *testing.T) {
 
 			_, err = client.ExecStart(t.Context(), "exec_id", tc.options)
 			if tc.expErr != "" {
-				assert.Check(t, is.ErrorType(err, errdefs.IsInvalidArgument))
+				assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument))
 				assert.Check(t, is.ErrorContains(err, tc.expErr))
 				assert.Check(t, is.DeepEqual(actualReq, tc.expReq))
 			} else {

client/container_rename.go

@@ -5,7 +5,7 @@ import (
 	"net/url"
 	"strings"
 
-	"github.com/containerd/errdefs"
+	cerrdefs "github.com/containerd/errdefs"
 )
 
 // ContainerRenameOptions represents the options for renaming a container.
@@ -28,7 +28,7 @@ func (cli *Client) ContainerRename(ctx context.Context, containerID string, opti
 	if options.NewName == "" || strings.TrimPrefix(options.NewName, "/") == "" {
 		// daemons before v29.0 did not handle the canonical name ("/") well
 		// let's be nice and validate it here before sending
-		return ContainerRenameResult{}, errdefs.ErrInvalidArgument.WithMessage("new name cannot be blank")
+		return ContainerRenameResult{}, cerrdefs.ErrInvalidArgument.WithMessage("new name cannot be blank")
 	}
 
 	query := url.Values{}

client/pkg/security/security_opts_test.go

@@ -5,7 +5,7 @@ import (
 	"testing"
 
 	"gotest.tools/v3/assert"
-	"gotest.tools/v3/assert/cmp"
+	is "gotest.tools/v3/assert/cmp"
 )
 
 func TestDecode(t *testing.T) {
@@ -161,7 +161,7 @@ func TestDecode(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			got := DecodeOptions(tc.opts)
-			assert.Check(t, cmp.DeepEqual(got, tc.want))
+			assert.Check(t, is.DeepEqual(got, tc.want))
 		})
 	}
 }

client/system_disk_usage.go

@@ -276,6 +276,8 @@ func containerDiskUsageFromLegacyAPI(du *legacyDiskUsage) ContainersDiskUsage {
 		case container.StateRunning, container.StatePaused, container.StateRestarting:
 			cdu.ActiveCount++
 			used += c.SizeRw
+		case container.StateCreated, container.StateRemoving, container.StateExited, container.StateDead:
+			// not active
 		}
 	}
 

client/volume_prune.go

@@ -6,7 +6,7 @@ import (
 	"fmt"
 	"net/url"
 
-	"github.com/containerd/errdefs"
+	cerrdefs "github.com/containerd/errdefs"
 	"github.com/moby/moby/api/types/volume"
 )
 
@@ -29,7 +29,7 @@ type VolumePruneResult struct {
 func (cli *Client) VolumePrune(ctx context.Context, options VolumePruneOptions) (VolumePruneResult, error) {
 	if options.All {
 		if _, ok := options.Filters["all"]; ok {
-			return VolumePruneResult{}, errdefs.ErrInvalidArgument.WithMessage(`conflicting options: cannot specify both "all" and "all" filter`)
+			return VolumePruneResult{}, cerrdefs.ErrInvalidArgument.WithMessage(`conflicting options: cannot specify both "all" and "all" filter`)
 		}
 		if options.Filters == nil {
 			options.Filters = Filters{}

client/volume_prune_test.go

@@ -5,7 +5,7 @@ import (
 	"net/http"
 	"testing"
 
-	"github.com/containerd/errdefs"
+	cerrdefs "github.com/containerd/errdefs"
 	"github.com/moby/moby/api/types/volume"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
@@ -81,7 +81,7 @@ func TestVolumePrune(t *testing.T) {
 
 			_, err = client.VolumePrune(t.Context(), tc.opts)
 			if tc.expectedError != "" {
-				assert.Check(t, is.ErrorType(err, errdefs.IsInvalidArgument))
+				assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument))
 				assert.Check(t, is.Error(err, tc.expectedError))
 			} else {
 				assert.NilError(t, err)

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

@@ -133,8 +133,6 @@ func (tf testRoundTripper) RoundTrip(req *http.Request) (*http.Response, error)
 	return tf(req)
 }
 
-func (testRoundTripper) skipConfigureTransport() bool { return true }
-
 // WithHostFromEnv overrides the client host with the host specified in the
 // DOCKER_HOST ([EnvOverrideHost]) environment variable. If DOCKER_HOST is not set,
 // or set to an empty value, the host is not modified.

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

@@ -5,7 +5,7 @@ import (
 	"encoding/json"
 	"net/http"
 
-	"github.com/containerd/errdefs"
+	cerrdefs "github.com/containerd/errdefs"
 	"github.com/moby/moby/api/types/container"
 )
 
@@ -152,7 +152,7 @@ func (cli *Client) ExecAttach(ctx context.Context, execID string, options ExecAt
 func getConsoleSize(hasTTY bool, consoleSize ConsoleSize) (*[2]uint, error) {
 	if consoleSize.Height != 0 || consoleSize.Width != 0 {
 		if !hasTTY {
-			return nil, errdefs.ErrInvalidArgument.WithMessage("console size is only supported when TTY is enabled")
+			return nil, cerrdefs.ErrInvalidArgument.WithMessage("console size is only supported when TTY is enabled")
 		}
 		return &[2]uint{consoleSize.Height, consoleSize.Width}, nil
 	}

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

@@ -5,7 +5,7 @@ import (
 	"net/url"
 	"strings"
 
-	"github.com/containerd/errdefs"
+	cerrdefs "github.com/containerd/errdefs"
 )
 
 // ContainerRenameOptions represents the options for renaming a container.
@@ -28,7 +28,7 @@ func (cli *Client) ContainerRename(ctx context.Context, containerID string, opti
 	if options.NewName == "" || strings.TrimPrefix(options.NewName, "/") == "" {
 		// daemons before v29.0 did not handle the canonical name ("/") well
 		// let's be nice and validate it here before sending
-		return ContainerRenameResult{}, errdefs.ErrInvalidArgument.WithMessage("new name cannot be blank")
+		return ContainerRenameResult{}, cerrdefs.ErrInvalidArgument.WithMessage("new name cannot be blank")
 	}
 
 	query := url.Values{}

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

@@ -276,6 +276,8 @@ func containerDiskUsageFromLegacyAPI(du *legacyDiskUsage) ContainersDiskUsage {
 		case container.StateRunning, container.StatePaused, container.StateRestarting:
 			cdu.ActiveCount++
 			used += c.SizeRw
+		case container.StateCreated, container.StateRemoving, container.StateExited, container.StateDead:
+			// not active
 		}
 	}
 

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

@@ -6,7 +6,7 @@ import (
 	"fmt"
 	"net/url"
 
-	"github.com/containerd/errdefs"
+	cerrdefs "github.com/containerd/errdefs"
 	"github.com/moby/moby/api/types/volume"
 )
 
@@ -29,7 +29,7 @@ type VolumePruneResult struct {
 func (cli *Client) VolumePrune(ctx context.Context, options VolumePruneOptions) (VolumePruneResult, error) {
 	if options.All {
 		if _, ok := options.Filters["all"]; ok {
-			return VolumePruneResult{}, errdefs.ErrInvalidArgument.WithMessage(`conflicting options: cannot specify both "all" and "all" filter`)
+			return VolumePruneResult{}, cerrdefs.ErrInvalidArgument.WithMessage(`conflicting options: cannot specify both "all" and "all" filter`)
 		}
 		if options.Filters == nil {
 			options.Filters = Filters{}