[20.10 backport] Prepare for Go 1.19 upgrade

.golangci.yml

@@ -1,7 +1,6 @@
 linters:
   enable:
     - bodyclose
-    - deadcode
     - dogsled
     - gocyclo
     - goimports
@@ -13,14 +12,12 @@ linters:
     - megacheck
     - misspell
     - nakedret
+    - revive
     - staticcheck
-    - structcheck
     - typecheck
     - unconvert
     - unparam
     - unused
-    - revive
-    - varcheck
 
   disable:
     - errcheck
@@ -93,6 +90,12 @@ issues:
       linters:
         - gosec
 
+    # G113 Potential uncontrolled memory consumption in Rat.SetString (CVE-2022-23772)
+    # only affects gp < 1.16.14. and go < 1.17.7
+    - text: "(G113)"
+      linters:
+        - gosec
+
     # Looks like the match in "EXC0007" above doesn't catch this one
     # TODO: consider upstreaming this to golangci-lint's default exclusion rules
     - text: "G204: Subprocess launched with a potential tainted input or cmd arguments"
@@ -104,12 +107,23 @@ issues:
       linters:
         - gosec
 
+    # TODO: make sure all packages have a description. Currently, there's 67 packages without.
+    - text: "package-comments: should have a package comment"
+      linters:
+        - revive
+
     # Exclude some linters from running on tests files.
     - path: _test\.go
       linters:
         - errcheck
         - gosec
 
+    # Fixing these lints would change user-facing output, which would be
+    # undesirable to introduce in a patch release.
+    - text: "ST1005: error strings should not be capitalized"
+      linters:
+        - stylecheck
+
   # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
   max-issues-per-linter: 0
 

cli-plugins/manager/manager.go

@@ -1,7 +1,6 @@
 package manager
 
 import (
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"sort"
@@ -57,12 +56,12 @@ func getPluginDirs(dockerCli command.Cli) ([]string, error) {
 }
 
 func addPluginCandidatesFromDir(res map[string][]string, d string) error {
-	dentries, err := ioutil.ReadDir(d)
+	dentries, err := os.ReadDir(d)
 	if err != nil {
 		return err
 	}
 	for _, dentry := range dentries {
-		switch dentry.Mode() & os.ModeType {
+		switch dentry.Type() & os.ModeType {
 		case 0, os.ModeSymlink:
 			// Regular file or symlink, keep going
 		default:

cli-plugins/manager/plugin.go

@@ -33,8 +33,6 @@ type Plugin struct {
 // is set, and is always a `pluginError`, but the `Plugin` is still
 // returned with no error. An error is only returned due to a
 // non-recoverable error.
-//
-// nolint: gocyclo
 func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
 	path := c.Path()
 	if path == "" {

cli/command/checkpoint/create_test.go

@@ -1,7 +1,7 @@
 package checkpoint
 
 import (
-	"io/ioutil"
+	"io"
 	"strings"
 	"testing"
 
@@ -41,7 +41,7 @@ func TestCheckpointCreateErrors(t *testing.T) {
 		})
 		cmd := newCreateCommand(cli)
 		cmd.SetArgs(tc.args)
-		cmd.SetOut(ioutil.Discard)
+		cmd.SetOut(io.Discard)
 		assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
 	}
 }

cli/command/checkpoint/list_test.go

@@ -1,7 +1,7 @@
 package checkpoint
 
 import (
-	"io/ioutil"
+	"io"
 	"testing"
 
 	"github.com/docker/cli/internal/test"
@@ -41,7 +41,7 @@ func TestCheckpointListErrors(t *testing.T) {
 		})
 		cmd := newListCommand(cli)
 		cmd.SetArgs(tc.args)
-		cmd.SetOut(ioutil.Discard)
+		cmd.SetOut(io.Discard)
 		assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
 	}
 }

cli/command/checkpoint/remove_test.go

@@ -1,7 +1,7 @@
 package checkpoint
 
 import (
-	"io/ioutil"
+	"io"
 	"testing"
 
 	"github.com/docker/cli/internal/test"
@@ -40,7 +40,7 @@ func TestCheckpointRemoveErrors(t *testing.T) {
 		})
 		cmd := newRemoveCommand(cli)
 		cmd.SetArgs(tc.args)
-		cmd.SetOut(ioutil.Discard)
+		cmd.SetOut(io.Discard)
 		assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
 	}
 }

cli/command/cli.go

@@ -3,7 +3,6 @@ package command
 import (
 	"context"
 	"io"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -279,7 +278,7 @@ func NewAPIClientFromFlags(opts *cliflags.CommonOptions, configFile *configfile.
 	store := &ContextStoreWithDefault{
 		Store: store.New(cliconfig.ContextStoreDir(), storeConfig),
 		Resolver: func() (*DefaultContext, error) {
-			return ResolveDefaultContext(opts, configFile, storeConfig, ioutil.Discard)
+			return ResolveDefaultContext(opts, configFile, storeConfig, io.Discard)
 		},
 	}
 	contextName, err := resolveContextName(opts, configFile, store)

cli/command/cli_options_test.go

@@ -15,23 +15,13 @@ func contentTrustEnabled(t *testing.T) bool {
 
 // NB: Do not t.Parallel() this test -- it messes with the process environment.
 func TestWithContentTrustFromEnv(t *testing.T) {
-	envvar := "DOCKER_CONTENT_TRUST"
-	if orig, ok := os.LookupEnv(envvar); ok {
-		defer func() {
-			os.Setenv(envvar, orig)
-		}()
-	} else {
-		defer func() {
-			os.Unsetenv(envvar)
-		}()
-	}
-
-	os.Setenv(envvar, "true")
-	assert.Assert(t, contentTrustEnabled(t))
-	os.Setenv(envvar, "false")
-	assert.Assert(t, !contentTrustEnabled(t))
-	os.Setenv(envvar, "invalid")
-	assert.Assert(t, contentTrustEnabled(t))
+	const envvar = "DOCKER_CONTENT_TRUST"
+	t.Setenv(envvar, "true")
+	assert.Check(t, contentTrustEnabled(t))
+	t.Setenv(envvar, "false")
+	assert.Check(t, !contentTrustEnabled(t))
+	t.Setenv(envvar, "invalid")
+	assert.Check(t, contentTrustEnabled(t))
 	os.Unsetenv(envvar)
-	assert.Assert(t, !contentTrustEnabled(t))
+	assert.Check(t, !contentTrustEnabled(t))
 }

cli/command/cli_test.go

@@ -5,7 +5,7 @@ import (
 	"context"
 	"crypto/x509"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"os"
 	"runtime"
 	"testing"
@@ -275,23 +275,23 @@ func TestNewDockerCliAndOperators(t *testing.T) {
 	outbuf := bytes.NewBuffer(nil)
 	errbuf := bytes.NewBuffer(nil)
 	err = cli.Apply(
-		WithInputStream(ioutil.NopCloser(inbuf)),
+		WithInputStream(io.NopCloser(inbuf)),
 		WithOutputStream(outbuf),
 		WithErrorStream(errbuf),
 	)
 	assert.NilError(t, err)
 	// Check input stream
-	inputStream, err := ioutil.ReadAll(cli.In())
+	inputStream, err := io.ReadAll(cli.In())
 	assert.NilError(t, err)
 	assert.Equal(t, string(inputStream), "input")
 	// Check output stream
 	fmt.Fprintf(cli.Out(), "output")
-	outputStream, err := ioutil.ReadAll(outbuf)
+	outputStream, err := io.ReadAll(outbuf)
 	assert.NilError(t, err)
 	assert.Equal(t, string(outputStream), "output")
 	// Check error stream
 	fmt.Fprintf(cli.Err(), "error")
-	errStream, err := ioutil.ReadAll(errbuf)
+	errStream, err := io.ReadAll(errbuf)
 	assert.NilError(t, err)
 	assert.Equal(t, string(errStream), "error")
 }

cli/command/config/create.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"io/ioutil"
 
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
@@ -61,7 +60,7 @@ func RunConfigCreate(dockerCli command.Cli, options CreateOptions) error {
 		defer file.Close()
 	}
 
-	configData, err := ioutil.ReadAll(in)
+	configData, err := io.ReadAll(in)
 	if err != nil {
 		return errors.Errorf("Error reading content from %q: %v", options.File, err)
 	}

cli/command/config/create_test.go

@@ -1,7 +1,8 @@
 package config
 
 import (
-	"io/ioutil"
+	"io"
+	"os"
 	"path/filepath"
 	"reflect"
 	"strings"
@@ -46,7 +47,7 @@ func TestConfigCreateErrors(t *testing.T) {
 			}),
 		)
 		cmd.SetArgs(tc.args)
-		cmd.SetOut(ioutil.Discard)
+		cmd.SetOut(io.Discard)
 		assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
 	}
 }
@@ -82,7 +83,7 @@ func TestConfigCreateWithLabels(t *testing.T) {
 	}
 	name := "foo"
 
-	data, err := ioutil.ReadFile(filepath.Join("testdata", configDataFile))
+	data, err := os.ReadFile(filepath.Join("testdata", configDataFile))
 	assert.NilError(t, err)
 
 	expected := swarm.ConfigSpec{

cli/command/config/inspect_test.go

@@ -2,7 +2,7 @@ package config
 
 import (
 	"fmt"
-	"io/ioutil"
+	"io"
 	"testing"
 	"time"
 
@@ -59,7 +59,7 @@ func TestConfigInspectErrors(t *testing.T) {
 		for key, value := range tc.flags {
 			cmd.Flags().Set(key, value)
 		}
-		cmd.SetOut(ioutil.Discard)
+		cmd.SetOut(io.Discard)
 		assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
 	}
 }

cli/command/config/ls_test.go

@@ -1,7 +1,7 @@
 package config
 
 import (
-	"io/ioutil"
+	"io"
 	"testing"
 	"time"
 
@@ -40,7 +40,7 @@ func TestConfigListErrors(t *testing.T) {
 			}),
 		)
 		cmd.SetArgs(tc.args)
-		cmd.SetOut(ioutil.Discard)
+		cmd.SetOut(io.Discard)
 		assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
 	}
 }

cli/command/config/remove_test.go

@@ -1,7 +1,7 @@
 package config
 
 import (
-	"io/ioutil"
+	"io"
 	"strings"
 	"testing"
 
@@ -36,7 +36,7 @@ func TestConfigRemoveErrors(t *testing.T) {
 			}),
 		)
 		cmd.SetArgs(tc.args)
-		cmd.SetOut(ioutil.Discard)
+		cmd.SetOut(io.Discard)
 		assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
 	}
 }
@@ -73,7 +73,7 @@ func TestConfigRemoveContinueAfterError(t *testing.T) {
 
 	cmd := newConfigRemoveCommand(cli)
 	cmd.SetArgs(names)
-	cmd.SetOut(ioutil.Discard)
+	cmd.SetOut(io.Discard)
 	assert.Error(t, cmd.Execute(), "error removing config: foo")
 	assert.Check(t, is.DeepEqual(names, removedConfigs))
 }

cli/command/container/attach_test.go

@@ -2,7 +2,7 @@ package container
 
 import (
 	"fmt"
-	"io/ioutil"
+	"io"
 	"testing"
 
 	"github.com/docker/cli/cli"
@@ -71,7 +71,7 @@ func TestNewAttachCommandErrors(t *testing.T) {
 	}
 	for _, tc := range testCases {
 		cmd := NewAttachCommand(test.NewFakeCli(&fakeClient{inspectFunc: tc.containerInspectFunc}))
-		cmd.SetOut(ioutil.Discard)
+		cmd.SetOut(io.Discard)
 		cmd.SetArgs(tc.args)
 		assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
 	}

cli/command/container/cp.go

@@ -285,10 +285,12 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo
 // in a valid LOCALPATH, like `file:name.txt`. We can resolve this ambiguity by
 // requiring a LOCALPATH with a `:` to be made explicit with a relative or
 // absolute path:
-// 	`/path/to/file:name.txt` or `./file:name.txt`
+//
+//	`/path/to/file:name.txt` or `./file:name.txt`
 //
 // This is apparently how `scp` handles this as well:
-// 	http://www.cyberciti.biz/faq/rsync-scp-file-name-with-colon-punctuation-in-it/
+//
+//	http://www.cyberciti.biz/faq/rsync-scp-file-name-with-colon-punctuation-in-it/
 //
 // We can't simply check for a filepath separator because container names may
 // have a separator, e.g., "host0/cname1" if container is in a Docker cluster,

cli/command/container/cp_test.go

@@ -2,7 +2,6 @@ package container
 
 import (
 	"io"
-	"io/ioutil"
 	"os"
 	"runtime"
 	"strings"
@@ -54,7 +53,7 @@ func TestRunCopyFromContainerToStdout(t *testing.T) {
 	fakeClient := &fakeClient{
 		containerCopyFromFunc: func(container, srcPath string) (io.ReadCloser, types.ContainerPathStat, error) {
 			assert.Check(t, is.Equal("container", container))
-			return ioutil.NopCloser(strings.NewReader(tarContent)), types.ContainerPathStat{}, nil
+			return io.NopCloser(strings.NewReader(tarContent)), types.ContainerPathStat{}, nil
 		},
 	}
 	options := copyOptions{source: "container:/path", destination: "-"}
@@ -84,7 +83,7 @@ func TestRunCopyFromContainerToFilesystem(t *testing.T) {
 	assert.Check(t, is.Equal("", cli.OutBuffer().String()))
 	assert.Check(t, is.Equal("", cli.ErrBuffer().String()))
 
-	content, err := ioutil.ReadFile(destDir.Join("file1"))
+	content, err := os.ReadFile(destDir.Join("file1"))
 	assert.NilError(t, err)
 	assert.Check(t, is.Equal("content\n", string(content)))
 }