Merge component 'cli' from git@github.com:docker/cli 19.03

Author: GordonTheTurtle

components/cli/.golangci.yml

@@ -0,0 +1,83 @@
+linters:
+  enable:
+    - bodyclose
+    - deadcode
+    - dogsled
+    - gocyclo
+    - goimports
+    - golint
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - interfacer
+    - lll
+    - megacheck
+    - misspell
+    - nakedret
+    - staticcheck
+    - structcheck
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - varcheck
+
+  disable:
+    - errcheck
+
+run:
+  timeout: 5m
+  skip-dirs:
+    - cli/command/stack/kubernetes/api/openapi
+    - cli/command/stack/kubernetes/api/client
+  skip-files:
+    - cli/compose/schema/bindata.go
+    - .*generated.*
+
+linters-settings:
+  gocyclo:
+    min-complexity: 16
+  govet:
+    check-shadowing: false
+  lll:
+    line-length: 200
+  nakedret:
+    command: nakedret
+    pattern: ^(?P<path>.*?\\.go):(?P<line>\\d+)\\s*(?P<message>.*)$
+
+issues:
+  # The default exclusion rules are a bit too permissive, so copying the relevant ones below
+  exclude-use-default: false
+
+  exclude:
+    - parameter .* always receives
+
+  exclude-rules:
+    # These are copied from the default exclude rules, except for "ineffective break statement"
+    # and GoDoc checks.
+    # https://github.com/golangci/golangci-lint/blob/0cc87df732aaf1d5ad9ce9ca538d38d916918b36/pkg/config/config.go#L36
+    - text: "Error return value of .((os\\.)?std(out|err)\\..*|.*Close|.*Flush|os\\.Remove(All)?|.*printf?|os\\.(Un)?Setenv). is not checked"
+      linters:
+        - errcheck
+    - text: "func name will be used as test\\.Test.* by other packages, and that stutters; consider calling this"
+      linters:
+        - golint
+    - text: "G103: Use of unsafe calls should be audited"
+      linters:
+        - gosec
+    - text: "G104: Errors unhandled"
+      linters:
+        - gosec
+    - text: "G204: Subprocess launch(ed with (variable|function call)|ing should be audited)"
+      linters:
+        - gosec
+    - text: "(G301|G302): (Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)"
+      linters:
+        - gosec
+    - text: "G304: Potential file inclusion via variable"
+      linters:
+        - gosec
+    - text: "(G201|G202): SQL string (formatting|concatenation)"
+      linters:
+        - gosec

components/cli/cli/command/cli_test.go

@@ -152,6 +152,7 @@ func TestInitializeFromClient(t *testing.T) {
 	}
 
 	for _, testcase := range testcases {
+		testcase := testcase
 		t.Run(testcase.doc, func(t *testing.T) {
 			apiclient := &fakeClient{
 				pingFunc: testcase.pingFunc,
@@ -189,6 +190,7 @@ func TestExperimentalCLI(t *testing.T) {
 	}
 
 	for _, testcase := range testcases {
+		testcase := testcase
 		t.Run(testcase.doc, func(t *testing.T) {
 			dir := fs.NewDir(t, testcase.doc, fs.WithFile("config.json", testcase.configfile))
 			defer dir.Remove()
@@ -242,6 +244,7 @@ func TestGetClientWithPassword(t *testing.T) {
 	}
 
 	for _, testcase := range testcases {
+		testcase := testcase
 		t.Run(testcase.doc, func(t *testing.T) {
 			passRetriever := func(_, _ string, _ bool, attempts int) (passphrase string, giveup bool, err error) {
 				// Always return an invalid pass first to test iteration
@@ -294,11 +297,12 @@ func TestNewDockerCliAndOperators(t *testing.T) {
 	inbuf := bytes.NewBuffer([]byte("input"))
 	outbuf := bytes.NewBuffer(nil)
 	errbuf := bytes.NewBuffer(nil)
-	cli.Apply(
+	err = cli.Apply(
 		WithInputStream(ioutil.NopCloser(inbuf)),
 		WithOutputStream(outbuf),
 		WithErrorStream(errbuf),
 	)
+	assert.NilError(t, err)
 	// Check input stream
 	inputStream, err := ioutil.ReadAll(cli.In())
 	assert.NilError(t, err)

components/cli/cli/command/config/inspect_test.go

@@ -7,10 +7,9 @@ import (
 	"time"
 
 	"github.com/docker/cli/internal/test"
+	. "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/pkg/errors"
-	// Import builders to get the builder function as package function
-	. "github.com/docker/cli/internal/test/builders"
 	"gotest.tools/assert"
 	"gotest.tools/golden"
 )

components/cli/cli/command/config/ls_test.go

@@ -7,11 +7,10 @@ import (
 
 	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/cli/internal/test"
+	. "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/pkg/errors"
-	// Import builders to get the builder function as package function
-	. "github.com/docker/cli/internal/test/builders"
 	"gotest.tools/assert"
 	is "gotest.tools/assert/cmp"
 	"gotest.tools/golden"

components/cli/cli/command/container/attach.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"net/http/httputil"
 
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
@@ -103,10 +102,7 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error {
 	}
 
 	resp, errAttach := client.ContainerAttach(ctx, opts.container, options)
-	if errAttach != nil && errAttach != httputil.ErrPersistEOF {
-		// ContainerAttach returns an ErrPersistEOF (connection closed)
-		// means server met an error and put it in Hijacked connection
-		// keep the error and read detailed error message from hijacked connection later
+	if errAttach != nil {
 		return errAttach
 	}
 	defer resp.Close()
@@ -142,10 +138,6 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error {
 		return err
 	}
 
-	if errAttach != nil {
-		return errAttach
-	}
-
 	return getExitStatus(errC, resultC)
 }
 

components/cli/cli/command/container/create_test.go

@@ -151,6 +151,7 @@ func TestNewCreateCommandWithContentTrustErrors(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
+		tc := tc
 		cli := test.NewFakeCli(&fakeClient{
 			createContainerFunc: func(config *container.Config,
 				hostConfig *container.HostConfig,
@@ -209,6 +210,7 @@ func TestNewCreateCommandWithWarnings(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
+		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			cli := test.NewFakeCli(&fakeClient{
 				createContainerFunc: func(config *container.Config,

components/cli/cli/command/container/list_test.go

@@ -7,9 +7,8 @@ import (
 
 	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/cli/internal/test"
+	. "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
 	"github.com/docker/docker/api/types"
-	// Import builders to get the builder function as package function
-	. "github.com/docker/cli/internal/test/builders"
 	"gotest.tools/assert"
 	"gotest.tools/golden"
 )

components/cli/cli/command/container/opts_test.go

@@ -65,7 +65,7 @@ func setupRunFlags() (*pflag.FlagSet, *containerOptions) {
 }
 
 func parseMustError(t *testing.T, args string) {
-	_, _, _, err := parseRun(strings.Split(args+" ubuntu bash", " "))
+	_, _, _, err := parseRun(strings.Split(args+" ubuntu bash", " ")) //nolint:dogsled
 	assert.ErrorContains(t, err, "", args)
 }
 
@@ -539,7 +539,7 @@ func TestParseModes(t *testing.T) {
 	}
 
 	// uts ko
-	_, _, _, err = parseRun([]string{"--uts=container:", "img", "cmd"})
+	_, _, _, err = parseRun([]string{"--uts=container:", "img", "cmd"}) //nolint:dogsled
 	assert.ErrorContains(t, err, "--uts: invalid UTS mode")
 
 	// uts ok
@@ -600,7 +600,7 @@ func TestParseRestartPolicy(t *testing.T) {
 
 func TestParseRestartPolicyAutoRemove(t *testing.T) {
 	expected := "Conflicting options: --restart and --rm"
-	_, _, _, err := parseRun([]string{"--rm", "--restart=always", "img", "cmd"})
+	_, _, _, err := parseRun([]string{"--rm", "--restart=always", "img", "cmd"}) //nolint:dogsled
 	if err == nil || err.Error() != expected {
 		t.Fatalf("Expected error %v, but got none", expected)
 	}

components/cli/cli/command/container/run.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"net/http/httputil"
 	"os"
 	"runtime"
 	"strings"
@@ -248,10 +247,7 @@ func attachContainer(
 	}
 
 	resp, errAttach := dockerCli.Client().ContainerAttach(ctx, containerID, options)
-	if errAttach != nil && errAttach != httputil.ErrPersistEOF {
-		// ContainerAttach returns an ErrPersistEOF (connection closed)
-		// means server met an error and put it in Hijacked connection
-		// keep the error and read detailed error message from hijacked connection later
+	if errAttach != nil {
 		return nil, errAttach
 	}
 

components/cli/cli/command/container/start.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"net/http/httputil"
 	"strings"
 
 	"github.com/docker/cli/cli"
@@ -98,10 +97,7 @@ func runStart(dockerCli command.Cli, opts *startOptions) error {
 		}
 
 		resp, errAttach := dockerCli.Client().ContainerAttach(ctx, c.ID, options)
-		if errAttach != nil && errAttach != httputil.ErrPersistEOF {
-			// ContainerAttach return an ErrPersistEOF (connection closed)
-			// means server met an error and already put it in Hijacked connection,
-			// we would keep the error and read the detailed error message from hijacked connection
+		if errAttach != nil {
 			return errAttach
 		}
 		defer resp.Close()
@@ -154,7 +150,7 @@ func runStart(dockerCli command.Cli, opts *startOptions) error {
 			}
 		}
 		if attachErr := <-cErr; attachErr != nil {
-			if _, ok := err.(term.EscapeError); ok {
+			if _, ok := attachErr.(term.EscapeError); ok {
 				// The user entered the detach escape sequence.
 				return nil
 			}