Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
c38ca4c
opts/throttledevice.go:51:5: SA4003: unsigned values are never < 0 (s…
silvin-lubecki Apr 2, 2019
65808fc
cli/command/trust/inspect_pretty_test.go:399:24: SA4010: this result …
silvin-lubecki Apr 2, 2019
b2158b2
cli/command/container/stats.go:211:21: SA1015: using time.Tick leaks …
silvin-lubecki Apr 2, 2019
fcbd937
SA1019: httputil.ErrPersistEOF is deprecated: No longer used. (stati…
silvin-lubecki Apr 2, 2019
63bf41f
cli/command/trust/key_generate.go:112:9: nilness: impossible conditio…
silvin-lubecki Apr 2, 2019
0d5c8d0
cli/command/stack/kubernetes/list.go:32:47: nilness: tautological con…
silvin-lubecki Apr 2, 2019
27ebbbe
cli/command/container/start.go:157:20: nilness: nil dereference in ty…
silvin-lubecki Apr 2, 2019
68a8b2c
cli/registry/client/fetcher.go:106:9: nilness: impossible condition: …
silvin-lubecki Apr 2, 2019
31e233b
cli/compose/types/types.go:106:2: structtag: struct field tag `yaml:"…
silvin-lubecki Apr 2, 2019
0850214
opts/ulimit_test.go:11:13: composites: `*github.com/docker/cli/vendor…
silvin-lubecki Apr 2, 2019
2fc15a6
cli/command/container/attach.go:141:15: nilness: impossible condition…
silvin-lubecki Apr 2, 2019
d5bbb94
unchecked errors
silvin-lubecki Apr 2, 2019
3c4788a
cli/command/image/build/context_test.go:244:38: `createTestTempDir` -…
silvin-lubecki Apr 2, 2019
321aff9
cli/command/image/build/context_test.go:252:71: `createTestTempFile` …
silvin-lubecki Apr 2, 2019
216bedc
cli/command/image/build_buildkit.go:450:56: parseSSH - result 1 (erro…
silvin-lubecki Apr 2, 2019
5abbbfc
cli/command/image/build_session.go:133:45: getBuildSharedKey - result…
silvin-lubecki Apr 2, 2019
65237e7
cli/command/plugin/list_test.go:61:31: `TestList$1` - `filter` is unu…
silvin-lubecki Apr 2, 2019
8f98106
cli/command/stack/kubernetes/deploy_test.go:65:68: `checkOwnerReferen…
silvin-lubecki Apr 2, 2019
d110697
cli/command/system/info.go:116:68: prettyPrintClientInfo - result 0 (…
silvin-lubecki Apr 2, 2019
c014ec1
cli/compose/convert/service.go:592:76: convertDNSConfig - result 1 (e…
silvin-lubecki Apr 2, 2019
7a47e81
disable unparam linter on these functions, as we need an error in the…
silvin-lubecki Apr 2, 2019
2f09e17
cli/context/kubernetes/endpoint_test.go:122:38: `checkClientConfig` -…
silvin-lubecki Apr 2, 2019
f443ad3
Disable unparam linter: cli/required.go:102:16: `pluralize` - `word` …
silvin-lubecki Apr 2, 2019
02091c1
Disable unparam linter: e2e/image/push_test.go:299:27: `withNotaryPas…
silvin-lubecki Apr 2, 2019
cd030a1
cli/command/trust/sign_test.go:119:70: unnecessary conversion (unconv…
silvin-lubecki Apr 2, 2019
4a7a541
File is not `goimports`-ed (goimports)
silvin-lubecki Apr 2, 2019
54cab47
cli/compose/convert/service_test.go:274:72: unnecessary conversion (u…
silvin-lubecki Apr 2, 2019
06908e0
cli/command/registry/login_test.go:66:25: unnecessary conversion (unc…
silvin-lubecki Apr 2, 2019
e62ed7a
cli/command/image/build/context_test.go:244:38: `createTestTempDir` -…
silvin-lubecki Apr 2, 2019
fbf90f3
Remove now obsolete gometalinter and use golangci-lint instead with a…
silvin-lubecki Apr 2, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
run:
deadline: 3m
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noice !

skip-dirs:
- cli/command/stack/kubernetes/api/openapi
- cli/command/stack/kubernetes/api/client
skip-files:
- cli/compose/schema/bindata.go
- .*generated.*
- parameter .* always receives
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this in the right section?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔

linters:
disable-all: true
enable:
- deadcode
- gocyclo
- gofmt
- goimports
- golint
- gosimple
- ineffassign
- interfacer
- lll
- misspell
- nakedret
- unconvert
- unparam
- unused
- vet
linters-settings:
nakedret:
command: nakedret
pattern: ^(?P<path>.*?\\.go):(?P<line>\\d+)\\s*(?P<message>.*)$
gocyclo:
min-complexity: 16
lll:
line-length: 200
3 changes: 1 addition & 2 deletions cli/command/config/inspect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down
3 changes: 1 addition & 2 deletions cli/command/config/ls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
10 changes: 1 addition & 9 deletions cli/command/container/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"io"
"net/http/httputil"

"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
Expand Down Expand Up @@ -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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand that change 😓

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The httputil.ErrPersistEOF is no longer used, so should no longer be returned by anything.

That said; the cli could connect to an older daemon that still returns this error, so I think we should change these to an //nolint, which is what we did in the engine;

	//nolint:staticcheck // ignore SA1019 for connecting to old (pre go1.8) daemons

moby/moby@fd65fed (updated in moby/moby@5f47cef)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, actually; so that moby change is the exact path we're hitting here in the ContainerAttach, so the client won't return a httputil.ErrPersistEOF error, so we can safely remove that condition here.

// 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()
Expand Down Expand Up @@ -142,10 +138,6 @@ func runAttach(dockerCli command.Cli, opts *attachOptions) error {
return err
}

if errAttach != nil {
return errAttach
}

return getExitStatus(errC, resultC)
}

Expand Down
3 changes: 1 addition & 2 deletions cli/command/container/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down
6 changes: 1 addition & 5 deletions cli/command/container/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"io"
"net/http/httputil"
"os"
"runtime"
"strings"
Expand Down Expand Up @@ -253,10 +252,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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

return nil, errAttach
}

Expand Down
8 changes: 2 additions & 6 deletions cli/command/container/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"io"
"net/http/httputil"
"strings"

"github.com/docker/cli/cli"
Expand Down Expand Up @@ -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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here

return errAttach
}
defer resp.Close()
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 3 additions & 1 deletion cli/command/container/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,9 @@ func runStats(dockerCli command.Cli, opts *statsOptions) error {
}

var err error
for range time.Tick(500 * time.Millisecond) {
ticker := time.NewTicker(500 * time.Millisecond)
defer ticker.Stop()
for range ticker.C {
cleanScreen()
ccstats := []StatsEntry{}
cStats.mu.Lock()
Expand Down
8 changes: 3 additions & 5 deletions cli/command/idresolver/idresolver_test.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
package idresolver

import (
"context"
"testing"

. "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"
"gotest.tools/assert"
is "gotest.tools/assert/cmp"
// Import builders to get the builder function as package function
"context"

. "github.com/docker/cli/internal/test/builders"
"github.com/pkg/errors"
)

func TestResolveError(t *testing.T) {
Expand Down
43 changes: 22 additions & 21 deletions cli/command/image/build/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ var prepareEmpty = func(t *testing.T) (string, func()) {
}

var prepareNoFiles = func(t *testing.T) (string, func()) {
return createTestTempDir(t, "", "builder-context-test")
return createTestTempDir(t, "builder-context-test")
}

var prepareOneFile = func(t *testing.T) (string, func()) {
contextDir, cleanup := createTestTempDir(t, "", "builder-context-test")
createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents, 0777)
contextDir, cleanup := createTestTempDir(t, "builder-context-test")
createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents)
return contextDir, cleanup
}

Expand All @@ -42,15 +42,15 @@ func testValidateContextDirectory(t *testing.T, prepare func(t *testing.T) (stri
}

func TestGetContextFromLocalDirNoDockerfile(t *testing.T) {
contextDir, cleanup := createTestTempDir(t, "", "builder-context-test")
contextDir, cleanup := createTestTempDir(t, "builder-context-test")
defer cleanup()

_, _, err := GetContextFromLocalDir(contextDir, "")
assert.ErrorContains(t, err, "Dockerfile")
}

func TestGetContextFromLocalDirNotExistingDir(t *testing.T) {
contextDir, cleanup := createTestTempDir(t, "", "builder-context-test")
contextDir, cleanup := createTestTempDir(t, "builder-context-test")
defer cleanup()

fakePath := filepath.Join(contextDir, "fake")
Expand All @@ -60,7 +60,7 @@ func TestGetContextFromLocalDirNotExistingDir(t *testing.T) {
}

func TestGetContextFromLocalDirNotExistingDockerfile(t *testing.T) {
contextDir, cleanup := createTestTempDir(t, "", "builder-context-test")
contextDir, cleanup := createTestTempDir(t, "builder-context-test")
defer cleanup()

fakePath := filepath.Join(contextDir, "fake")
Expand All @@ -70,10 +70,10 @@ func TestGetContextFromLocalDirNotExistingDockerfile(t *testing.T) {
}

func TestGetContextFromLocalDirWithNoDirectory(t *testing.T) {
contextDir, dirCleanup := createTestTempDir(t, "", "builder-context-test")
contextDir, dirCleanup := createTestTempDir(t, "builder-context-test")
defer dirCleanup()

createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents, 0777)
createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents)

chdirCleanup := chdir(t, contextDir)
defer chdirCleanup()
Expand All @@ -86,10 +86,10 @@ func TestGetContextFromLocalDirWithNoDirectory(t *testing.T) {
}

func TestGetContextFromLocalDirWithDockerfile(t *testing.T) {
contextDir, cleanup := createTestTempDir(t, "", "builder-context-test")
contextDir, cleanup := createTestTempDir(t, "builder-context-test")
defer cleanup()

createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents, 0777)
createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents)

absContextDir, relDockerfile, err := GetContextFromLocalDir(contextDir, "")
assert.NilError(t, err)
Expand All @@ -99,11 +99,11 @@ func TestGetContextFromLocalDirWithDockerfile(t *testing.T) {
}

func TestGetContextFromLocalDirLocalFile(t *testing.T) {
contextDir, cleanup := createTestTempDir(t, "", "builder-context-test")
contextDir, cleanup := createTestTempDir(t, "builder-context-test")
defer cleanup()

createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents, 0777)
testFilename := createTestTempFile(t, contextDir, "tmpTest", "test", 0777)
createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents)
testFilename := createTestTempFile(t, contextDir, "tmpTest", "test")

absContextDir, relDockerfile, err := GetContextFromLocalDir(testFilename, "")

Expand All @@ -121,13 +121,13 @@ func TestGetContextFromLocalDirLocalFile(t *testing.T) {
}

func TestGetContextFromLocalDirWithCustomDockerfile(t *testing.T) {
contextDir, cleanup := createTestTempDir(t, "", "builder-context-test")
contextDir, cleanup := createTestTempDir(t, "builder-context-test")
defer cleanup()

chdirCleanup := chdir(t, contextDir)
defer chdirCleanup()

createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents, 0777)
createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents)

absContextDir, relDockerfile, err := GetContextFromLocalDir(contextDir, DefaultDockerfileName)
assert.NilError(t, err)
Expand Down Expand Up @@ -173,10 +173,10 @@ func TestGetContextFromReaderString(t *testing.T) {
}

func TestGetContextFromReaderTar(t *testing.T) {
contextDir, cleanup := createTestTempDir(t, "", "builder-context-test")
contextDir, cleanup := createTestTempDir(t, "builder-context-test")
defer cleanup()

createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents, 0777)
createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents)

tarStream, err := archive.Tar(contextDir, archive.Uncompressed)
assert.NilError(t, err)
Expand Down Expand Up @@ -241,17 +241,18 @@ func TestValidateContextDirectoryWithOneFileExcludes(t *testing.T) {
// createTestTempDir creates a temporary directory for testing.
// It returns the created path and a cleanup function which is meant to be used as deferred call.
// When an error occurs, it terminates the test.
func createTestTempDir(t *testing.T, dir, prefix string) (string, func()) {
path, err := ioutil.TempDir(dir, prefix)
//nolint: unparam
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is unparam needed ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createTestTempDir - prefix always receives "builder-context-test"

I guess instead, we could change one test here as well

func createTestTempDir(t *testing.T, prefix string) (string, func()) {
path, err := ioutil.TempDir("", prefix)
assert.NilError(t, err)
return path, func() { assert.NilError(t, os.RemoveAll(path)) }
}

// createTestTempFile creates a temporary file within dir with specific contents and permissions.
// When an error occurs, it terminates the test
func createTestTempFile(t *testing.T, dir, filename, contents string, perm os.FileMode) string {
func createTestTempFile(t *testing.T, dir, filename, contents string) string {
filePath := filepath.Join(dir, filename)
err := ioutil.WriteFile(filePath, []byte(contents), perm)
err := ioutil.WriteFile(filePath, []byte(contents), 0777)
assert.NilError(t, err)
return filePath
}
Expand Down
9 changes: 3 additions & 6 deletions cli/command/image/build_buildkit.go
Original file line number Diff line number Diff line change
Expand Up @@ -438,22 +438,19 @@ func parseSecret(value string) (*secretsprovider.FileSource, error) {
func parseSSHSpecs(sl []string) (session.Attachable, error) {
configs := make([]sshprovider.AgentConfig, 0, len(sl))
for _, v := range sl {
c, err := parseSSH(v)
if err != nil {
return nil, err
}
c := parseSSH(v)
configs = append(configs, *c)
}
return sshprovider.NewSSHAgentProvider(configs)
}

func parseSSH(value string) (*sshprovider.AgentConfig, error) {
func parseSSH(value string) *sshprovider.AgentConfig {
parts := strings.SplitN(value, "=", 2)
cfg := sshprovider.AgentConfig{
ID: parts[0],
}
if len(parts) > 1 {
cfg.Paths = strings.Split(parts[1], ",")
}
return &cfg, nil
return &cfg
}
21 changes: 9 additions & 12 deletions cli/command/image/build_session.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,13 @@ func isSessionSupported(dockerCli command.Cli, forStream bool) bool {
}

func trySession(dockerCli command.Cli, contextDir string, forStream bool) (*session.Session, error) {
var s *session.Session
if isSessionSupported(dockerCli, forStream) {
sharedKey, err := getBuildSharedKey(contextDir)
if err != nil {
return nil, errors.Wrap(err, "failed to get build shared key")
}
s, err = session.NewSession(context.Background(), filepath.Base(contextDir), sharedKey)
if err != nil {
return nil, errors.Wrap(err, "failed to create session")
}
if !isSessionSupported(dockerCli, forStream) {
return nil, nil
}
sharedKey := getBuildSharedKey(contextDir)
s, err := session.NewSession(context.Background(), filepath.Base(contextDir), sharedKey)
if err != nil {
return nil, errors.Wrap(err, "failed to create session")
}
return s, nil
}
Expand Down Expand Up @@ -130,10 +127,10 @@ func (bw *bufferedWriter) String() string {
return fmt.Sprintf("%s", bw.Writer)
}

func getBuildSharedKey(dir string) (string, error) {
func getBuildSharedKey(dir string) string {
// build session is hash of build dir with node based randomness
s := sha256.Sum256([]byte(fmt.Sprintf("%s:%s", tryNodeIdentifier(), dir)))
return hex.EncodeToString(s[:]), nil
return hex.EncodeToString(s[:])
}

func tryNodeIdentifier() string {
Expand Down
3 changes: 1 addition & 2 deletions cli/command/node/demote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@ import (
"testing"

"github.com/docker/cli/internal/test"
. "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package functions
"github.com/docker/docker/api/types/swarm"
"github.com/pkg/errors"
"gotest.tools/assert"
// Import builders to get the builder function as package function
. "github.com/docker/cli/internal/test/builders"
)

func TestNodeDemoteErrors(t *testing.T) {
Expand Down
Loading