From 6b0ecacd921cefbed1d79466b88080620b17e5ed Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 16 Apr 2021 16:03:57 +0200 Subject: [PATCH 01/19] gosec: G404: Use of weak random number generator These should be ok to ignore for the purpose they're used pkg/namesgenerator/names-generator.go:843:36: G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec) name := fmt.Sprintf("%s_%s", left[rand.Intn(len(left))], right[rand.Intn(len(right))]) ^ pkg/namesgenerator/names-generator.go:849:36: G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec) name = fmt.Sprintf("%s%d", name, rand.Intn(10)) ^ testutil/stringutils.go:11:18: G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec) b[i] = letters[rand.Intn(len(letters))] ^ pkg/namesgenerator/names-generator.go:849:36: G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec) name = fmt.Sprintf("%s%d", name, rand.Intn(10)) ^ Signed-off-by: Sebastiaan van Stijn --- pkg/namesgenerator/names-generator.go | 4 ++-- testutil/stringutils.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/namesgenerator/names-generator.go b/pkg/namesgenerator/names-generator.go index 4b50a0a128..d0d8c652d8 100644 --- a/pkg/namesgenerator/names-generator.go +++ b/pkg/namesgenerator/names-generator.go @@ -840,13 +840,13 @@ var ( // integer between 0 and 10 will be added to the end of the name, e.g `focused_turing3` func GetRandomName(retry int) string { begin: - name := fmt.Sprintf("%s_%s", left[rand.Intn(len(left))], right[rand.Intn(len(right))]) + name := fmt.Sprintf("%s_%s", left[rand.Intn(len(left))], right[rand.Intn(len(right))]) //nolint:gosec // G404: Use of weak random number generator (math/rand instead of crypto/rand) if name == "boring_wozniak" /* Steve Wozniak is not boring */ { goto begin } if retry > 0 { - name = fmt.Sprintf("%s%d", name, rand.Intn(10)) + name = fmt.Sprintf("%s%d", name, rand.Intn(10)) //nolint:gosec // G404: Use of weak random number generator (math/rand instead of crypto/rand) } return name } diff --git a/testutil/stringutils.go b/testutil/stringutils.go index 62092faa36..885ebe560b 100644 --- a/testutil/stringutils.go +++ b/testutil/stringutils.go @@ -8,7 +8,7 @@ func GenerateRandomAlphaOnlyString(n int) string { letters := []byte("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") b := make([]byte, n) for i := range b { - b[i] = letters[rand.Intn(len(letters))] + b[i] = letters[rand.Intn(len(letters))] //nolint: gosec // G404: Use of weak random number generator (math/rand instead of crypto/rand) } return string(b) } From d43bcc8974122faed16ac34cfe2b5da400948d3e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 16 Apr 2021 16:07:25 +0200 Subject: [PATCH 02/19] daemon/logger/journald: fix linting errors daemon/logger/journald/read.go:128:3 comment on exported function `CErr` should be of the form `CErr ...` daemon/logger/journald/read.go:131:36: unnecessary conversion (unconvert) return C.GoString(C.strerror(C.int(-ret))) ^ daemon/logger/journald/read.go:380:2: S1023: redundant `return` statement (gosimple) return ^ Signed-off-by: Sebastiaan van Stijn --- daemon/logger/journald/read.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/daemon/logger/journald/read.go b/daemon/logger/journald/read.go index d2bd112d27..76557815ab 100644 --- a/daemon/logger/journald/read.go +++ b/daemon/logger/journald/read.go @@ -125,10 +125,10 @@ func (s *journald) Close() error { return nil } -// convert error code returned from a sd_journal_* function +// CErr converts error code returned from a sd_journal_* function // (which returns -errno) to a string func CErr(ret C.int) string { - return C.GoString(C.strerror(C.int(-ret))) + return C.GoString(C.strerror(-ret)) } func (s *journald) drainJournal(logWatcher *logger.LogWatcher, j *C.sd_journal, oldCursor *C.char, untilUnixMicro uint64) (*C.char, bool, int) { @@ -377,7 +377,6 @@ func (s *journald) readLogs(logWatcher *logger.LogWatcher, config logger.ReadCon } C.free(unsafe.Pointer(cursor)) - return } func (s *journald) ReadLogs(config logger.ReadConfig) *logger.LogWatcher { From b92be7e297179ad0792643ce8c36579b5c48e109 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 16 Apr 2021 16:15:10 +0200 Subject: [PATCH 03/19] client: S1031: unnecessary nil check around range (gosimple) client/request.go:245:2: S1031: unnecessary nil check around range (gosimple) if headers != nil { ^ Signed-off-by: Sebastiaan van Stijn --- client/request.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/client/request.go b/client/request.go index 813eac2c9e..f7aebdb2d2 100644 --- a/client/request.go +++ b/client/request.go @@ -242,10 +242,8 @@ func (cli *Client) addHeaders(req *http.Request, headers headers) *http.Request req.Header.Set(k, v) } - if headers != nil { - for k, v := range headers { - req.Header[k] = v - } + for k, v := range headers { + req.Header[k] = v } return req } From f7433d6190cb716b6d748b4e87c607dffb7161fb Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 16 Apr 2021 16:28:10 +0200 Subject: [PATCH 04/19] staticcheck: SA4001: &*x will be simplified to x. It will not copy x daemon/volumes_unix_test.go:228:13: SA4001: &*x will be simplified to x. It will not copy x. (staticcheck) mp: &(*c.MountPoints["/jambolan"]), // copy the mountpoint, expect no changes ^ daemon/logger/local/local_test.go:214:22: SA4001: &*x will be simplified to x. It will not copy x. (staticcheck) dst.PLogMetaData = &(*src.PLogMetaData) ^ Signed-off-by: Sebastiaan van Stijn --- daemon/logger/local/local_test.go | 3 ++- daemon/volumes_unix_test.go | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/daemon/logger/local/local_test.go b/daemon/logger/local/local_test.go index 21ea8e6a82..d59a17df4f 100644 --- a/daemon/logger/local/local_test.go +++ b/daemon/logger/local/local_test.go @@ -211,7 +211,8 @@ func copyLogMessage(src *logger.Message) *logger.Message { dst.Err = src.Err dst.Line = append(dst.Line, src.Line...) if src.PLogMetaData != nil { - dst.PLogMetaData = &(*src.PLogMetaData) + lmd := *src.PLogMetaData + dst.PLogMetaData = &lmd } return dst } diff --git a/daemon/volumes_unix_test.go b/daemon/volumes_unix_test.go index 36e19110d1..8b7db877b6 100644 --- a/daemon/volumes_unix_test.go +++ b/daemon/volumes_unix_test.go @@ -96,6 +96,8 @@ func TestBackportMountSpec(t *testing.T) { return string(b) } + mpc := *c.MountPoints["/jambolan"] + for _, x := range []expected{ { mp: &volumemounts.MountPoint{ @@ -225,7 +227,7 @@ func TestBackportMountSpec(t *testing.T) { comment: "partially configured named volume caused by #32613", }, { - mp: &(*c.MountPoints["/jambolan"]), // copy the mountpoint, expect no changes + mp: &mpc, comment: "volume defined in mounts API", }, { From f77213efc2a0a5f27f9bda4d9c72dc21275a20d2 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 16 Apr 2021 16:30:04 +0200 Subject: [PATCH 05/19] gosimple: S1039: unnecessary use of fmt.Sprintf pkg/devicemapper/devmapper.go:383:28: S1039: unnecessary use of fmt.Sprintf (gosimple) if err := task.setMessage(fmt.Sprintf("@cancel_deferred_remove")); err != nil { ^ integration/plugin/graphdriver/external_test.go:321:18: S1039: unnecessary use of fmt.Sprintf (gosimple) http.Error(w, fmt.Sprintf("missing id"), 409) ^ integration-cli/docker_api_stats_test.go:70:31: S1039: unnecessary use of fmt.Sprintf (gosimple) _, body, err := request.Get(fmt.Sprintf("/info")) ^ integration-cli/docker_cli_build_test.go:4547:19: S1039: unnecessary use of fmt.Sprintf (gosimple) "--build-arg", fmt.Sprintf("FOO1=fromcmd"), ^ integration-cli/docker_cli_build_test.go:4548:19: S1039: unnecessary use of fmt.Sprintf (gosimple) "--build-arg", fmt.Sprintf("FOO2="), ^ integration-cli/docker_cli_build_test.go:4549:19: S1039: unnecessary use of fmt.Sprintf (gosimple) "--build-arg", fmt.Sprintf("FOO3"), // set in env ^ integration-cli/docker_cli_build_test.go:4668:32: S1039: unnecessary use of fmt.Sprintf (gosimple) cli.WithFlags("--build-arg", fmt.Sprintf("tag=latest"))) ^ integration-cli/docker_cli_build_test.go:4690:32: S1039: unnecessary use of fmt.Sprintf (gosimple) cli.WithFlags("--build-arg", fmt.Sprintf("baz=abc"))) ^ pkg/jsonmessage/jsonmessage_test.go:255:4: S1039: unnecessary use of fmt.Sprintf (gosimple) fmt.Sprintf("ID: status\n"), ^ Signed-off-by: Sebastiaan van Stijn --- integration-cli/docker_api_stats_test.go | 10 ++++----- integration-cli/docker_cli_build_test.go | 22 +++++++++---------- .../plugin/graphdriver/external_test.go | 4 ++-- pkg/devicemapper/devmapper.go | 2 +- pkg/jsonmessage/jsonmessage_test.go | 2 +- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/integration-cli/docker_api_stats_test.go b/integration-cli/docker_api_stats_test.go index 6802cfbe2b..cd614a5e20 100644 --- a/integration-cli/docker_api_stats_test.go +++ b/integration-cli/docker_api_stats_test.go @@ -67,7 +67,7 @@ func (s *DockerSuite) TestAPIStatsStoppedContainerInGoroutines(c *testing.T) { id := strings.TrimSpace(out) getGoRoutines := func() int { - _, body, err := request.Get(fmt.Sprintf("/info")) + _, body, err := request.Get("/info") assert.NilError(c, err) info := types.Info{} err = json.NewDecoder(body).Decode(&info) @@ -78,7 +78,7 @@ func (s *DockerSuite) TestAPIStatsStoppedContainerInGoroutines(c *testing.T) { // When the HTTP connection is closed, the number of goroutines should not increase. routines := getGoRoutines() - _, body, err := request.Get(fmt.Sprintf("/containers/%s/stats", id)) + _, body, err := request.Get("/containers/" + id + "/stats") assert.NilError(c, err) body.Close() @@ -190,7 +190,7 @@ func (s *DockerSuite) TestAPIStatsNetworkStatsVersioning(c *testing.T) { func getNetworkStats(c *testing.T, id string) map[string]types.NetworkStats { var st *types.StatsJSON - _, body, err := request.Get(fmt.Sprintf("/containers/%s/stats?stream=false", id)) + _, body, err := request.Get("/containers/" + id + "/stats?stream=false") assert.NilError(c, err) err = json.NewDecoder(body).Decode(&st) @@ -207,7 +207,7 @@ func getNetworkStats(c *testing.T, id string) map[string]types.NetworkStats { func getVersionedStats(c *testing.T, id string, apiVersion string) map[string]interface{} { stats := make(map[string]interface{}) - _, body, err := request.Get(fmt.Sprintf("/%s/containers/%s/stats?stream=false", apiVersion, id)) + _, body, err := request.Get("/" + apiVersion + "/containers/" + id + "/stats?stream=false") assert.NilError(c, err) defer body.Close() @@ -284,7 +284,7 @@ func (s *DockerSuite) TestAPIStatsNoStreamConnectedContainers(c *testing.T) { ch := make(chan error, 1) go func() { - resp, body, err := request.Get(fmt.Sprintf("/containers/%s/stats?stream=false", id2)) + resp, body, err := request.Get("/containers/" + id2 + "/stats?stream=false") defer body.Close() if err != nil { ch <- err diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index c7dc934d62..968e1d67a0 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -4544,16 +4544,16 @@ func (s *DockerSuite) TestBuildBuildTimeArgEnv(c *testing.T) { ` result := buildImage("testbuildtimeargenv", cli.WithFlags( - "--build-arg", fmt.Sprintf("FOO1=fromcmd"), - "--build-arg", fmt.Sprintf("FOO2="), - "--build-arg", fmt.Sprintf("FOO3"), // set in env - "--build-arg", fmt.Sprintf("FOO4"), // not set in env - "--build-arg", fmt.Sprintf("FOO5=fromcmd"), + "--build-arg", "FOO1=fromcmd", + "--build-arg", "FOO2=", + "--build-arg", "FOO3", // set in env + "--build-arg", "FOO4", // not set in env + "--build-arg", "FOO5=fromcmd", // FOO6 is not set at all - "--build-arg", fmt.Sprintf("FOO7=fromcmd"), // should produce a warning - "--build-arg", fmt.Sprintf("FOO8="), // should produce a warning - "--build-arg", fmt.Sprintf("FOO9"), // should produce a warning - "--build-arg", fmt.Sprintf("FO10"), // not set in env, empty value + "--build-arg", "FOO7=fromcmd", // should produce a warning + "--build-arg", "FOO8=", // should produce a warning + "--build-arg", "FOO9", // should produce a warning + "--build-arg", "FO10", // not set in env, empty value ), cli.WithEnvironmentVariables(append(os.Environ(), "FOO1=fromenv", @@ -4665,7 +4665,7 @@ func (s *DockerSuite) TestBuildMultiStageGlobalArg(c *testing.T) { result := cli.BuildCmd(c, imgName, build.WithDockerfile(dockerfile), - cli.WithFlags("--build-arg", fmt.Sprintf("tag=latest"))) + cli.WithFlags("--build-arg", "tag=latest")) result.Assert(c, icmd.Success) result = cli.DockerCmd(c, "images", "-q", "-f", "label=multifromtest=1") @@ -4687,7 +4687,7 @@ func (s *DockerSuite) TestBuildMultiStageUnusedArg(c *testing.T) { result := cli.BuildCmd(c, imgName, build.WithDockerfile(dockerfile), - cli.WithFlags("--build-arg", fmt.Sprintf("baz=abc"))) + cli.WithFlags("--build-arg", "baz=abc")) result.Assert(c, icmd.Success) assert.Assert(c, strings.Contains(result.Combined(), "[Warning]")) assert.Assert(c, strings.Contains(result.Combined(), "[baz] were not consumed")) diff --git a/integration/plugin/graphdriver/external_test.go b/integration/plugin/graphdriver/external_test.go index eadcdc8e66..87ce1b82d7 100644 --- a/integration/plugin/graphdriver/external_test.go +++ b/integration/plugin/graphdriver/external_test.go @@ -129,7 +129,7 @@ func setupPlugin(t *testing.T, ec map[string]*graphEventsCounter, ext string, mu w.Header().Set("Content-Type", "application/vnd.docker.plugins.v1+json") switch t := data.(type) { case error: - fmt.Fprintln(w, fmt.Sprintf(`{"Err": %q}`, t.Error())) + fmt.Fprintf(w, "{\"Err\": %q}\n", t.Error()) case string: fmt.Fprintln(w, t) default: @@ -318,7 +318,7 @@ func setupPlugin(t *testing.T, ec map[string]*graphEventsCounter, ext string, mu parent := r.URL.Query().Get("parent") if id == "" { - http.Error(w, fmt.Sprintf("missing id"), 409) + http.Error(w, "missing id", 409) } size, err := driver.ApplyDiff(id, parent, diff) diff --git a/pkg/devicemapper/devmapper.go b/pkg/devicemapper/devmapper.go index f9356f5ba6..edc6009ff3 100644 --- a/pkg/devicemapper/devmapper.go +++ b/pkg/devicemapper/devmapper.go @@ -380,7 +380,7 @@ func CancelDeferredRemove(deviceName string) error { return fmt.Errorf("devicemapper: Can't set sector %s", err) } - if err := task.setMessage(fmt.Sprintf("@cancel_deferred_remove")); err != nil { + if err := task.setMessage("@cancel_deferred_remove"); err != nil { return fmt.Errorf("devicemapper: Can't set message %s", err) } diff --git a/pkg/jsonmessage/jsonmessage_test.go b/pkg/jsonmessage/jsonmessage_test.go index 2ce422687f..f1a7be2a7b 100644 --- a/pkg/jsonmessage/jsonmessage_test.go +++ b/pkg/jsonmessage/jsonmessage_test.go @@ -252,7 +252,7 @@ func TestDisplayJSONMessagesStream(t *testing.T) { // Without progress, with ID "{ \"id\": \"ID\",\"status\": \"status\" }": { "ID: status\n", - fmt.Sprintf("ID: status\n"), + "ID: status\n", }, // With progress "{ \"id\": \"ID\", \"status\": \"status\", \"progress\": \"ProgressMessage\" }": { From d13997b4badce89927bdab071c23e45684027f87 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 16 Apr 2021 17:21:26 +0200 Subject: [PATCH 06/19] gosec: G601: Implicit memory aliasing in for loop plugin/v2/plugin.go:141:50: G601: Implicit memory aliasing in for loop. (gosec) updateSettingsEnv(&p.PluginObj.Settings.Env, &s) ^ libcontainerd/remote/client.go:572:13: G601: Implicit memory aliasing in for loop. (gosec) cpDesc = &m ^ distribution/push_v2.go:400:34: G601: Implicit memory aliasing in for loop. (gosec) (metadata.CheckV2MetadataHMAC(&mountCandidate, pd.hmacKey) || ^ builder/dockerfile/builder.go:261:84: G601: Implicit memory aliasing in for loop. (gosec) currentCommandIndex = printCommand(b.Stdout, currentCommandIndex, totalCommands, &meta) ^ builder/dockerfile/builder.go:278:46: G601: Implicit memory aliasing in for loop. (gosec) if err := initializeStage(dispatchRequest, &stage); err != nil { ^ daemon/container.go:283:40: G601: Implicit memory aliasing in for loop. (gosec) if err := parser.ValidateMountConfig(&cfg); err != nil { ^ Signed-off-by: Sebastiaan van Stijn --- builder/dockerfile/builder.go | 9 +++++---- daemon/container.go | 3 ++- distribution/push_v2.go | 3 ++- libcontainerd/remote/client.go | 1 + plugin/v2/plugin.go | 4 +++- 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index 0369a1d82d..52e2c7d50f 100644 --- a/builder/dockerfile/builder.go +++ b/builder/dockerfile/builder.go @@ -254,10 +254,10 @@ func (b *Builder) dispatchDockerfileWithCancellation(parseResult []instructions. totalCommands += len(stage.Commands) } shlex := shell.NewLex(escapeToken) - for _, meta := range metaArgs { - currentCommandIndex = printCommand(b.Stdout, currentCommandIndex, totalCommands, &meta) + for i := range metaArgs { + currentCommandIndex = printCommand(b.Stdout, currentCommandIndex, totalCommands, &metaArgs[i]) - err := processMetaArg(meta, shlex, buildArgs) + err := processMetaArg(metaArgs[i], shlex, buildArgs) if err != nil { return nil, err } @@ -265,7 +265,8 @@ func (b *Builder) dispatchDockerfileWithCancellation(parseResult []instructions. stagesResults := newStagesBuildResults() - for _, stage := range parseResult { + for _, s := range parseResult { + stage := s if err := stagesResults.checkStageNameAvailable(stage.Name); err != nil { return nil, err } diff --git a/daemon/container.go b/daemon/container.go index 70cf612a09..5912f26b62 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -279,7 +279,8 @@ func validateHostConfig(hostConfig *containertypes.HostConfig, platform string) } // Validate mounts; check if host directories still exist parser := volumemounts.NewParser(platform) - for _, cfg := range hostConfig.Mounts { + for _, c := range hostConfig.Mounts { + cfg := c if err := parser.ValidateMountConfig(&cfg); err != nil { return err } diff --git a/distribution/push_v2.go b/distribution/push_v2.go index 8934ac71a2..3d2c13bf25 100644 --- a/distribution/push_v2.go +++ b/distribution/push_v2.go @@ -326,7 +326,8 @@ func (pd *v2PushDescriptor) Upload(ctx context.Context, progressOutput progress. // Attempt to find another repository in the same registry to mount the layer from to avoid an unnecessary upload candidates := getRepositoryMountCandidates(pd.repoInfo, pd.hmacKey, maxMountAttempts, v2Metadata) isUnauthorizedError := false - for _, mountCandidate := range candidates { + for _, mc := range candidates { + mountCandidate := mc logrus.Debugf("attempting to mount layer %s (%s) from %s", diffID, mountCandidate.Digest, mountCandidate.SourceRepository) createOpts := []distribution.BlobCreateOption{} diff --git a/libcontainerd/remote/client.go b/libcontainerd/remote/client.go index 7561706241..f82911884e 100644 --- a/libcontainerd/remote/client.go +++ b/libcontainerd/remote/client.go @@ -568,6 +568,7 @@ func (c *client) CreateCheckpoint(ctx context.Context, containerID, checkpointDi var cpDesc *v1.Descriptor for _, m := range index.Manifests { + m := m if m.MediaType == images.MediaTypeContainerd1Checkpoint { cpDesc = &m // nolint:gosec break diff --git a/plugin/v2/plugin.go b/plugin/v2/plugin.go index 3e6e063f4a..d42e1bd0b9 100644 --- a/plugin/v2/plugin.go +++ b/plugin/v2/plugin.go @@ -126,7 +126,9 @@ func (p *Plugin) Set(args []string) error { // TODO(vieux): lots of code duplication here, needs to be refactored. next: - for _, s := range sets { + for _, set := range sets { + s := set + // range over all the envs in the config for _, env := range p.PluginObj.Config.Env { // found the env in the config From b4c0c7c076bb825c656c49ce73dfe94d070df0c9 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 16 Apr 2021 18:06:43 +0200 Subject: [PATCH 07/19] G601: Implicit memory aliasing in for loop daemon/cluster/executor/container/adapter.go:446:42: G601: Implicit memory aliasing in for loop. (gosec) req := c.container.volumeCreateRequest(&mount) ^ daemon/network.go:577:10: G601: Implicit memory aliasing in for loop. (gosec) np := &n ^ Signed-off-by: Sebastiaan van Stijn --- daemon/cluster/executor/container/adapter.go | 1 + daemon/network.go | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/daemon/cluster/executor/container/adapter.go b/daemon/cluster/executor/container/adapter.go index 57a4a23db3..00b47e77ce 100644 --- a/daemon/cluster/executor/container/adapter.go +++ b/daemon/cluster/executor/container/adapter.go @@ -431,6 +431,7 @@ func (c *containerAdapter) remove(ctx context.Context) error { func (c *containerAdapter) createVolumes(ctx context.Context) error { // Create plugin volumes that are embedded inside a Mount for _, mount := range c.container.task.Spec.GetContainer().Mounts { + mount := mount if mount.Type != api.MountTypeVolume { continue } diff --git a/daemon/network.go b/daemon/network.go index 71bd92f1e6..46c286025f 100644 --- a/daemon/network.go +++ b/daemon/network.go @@ -573,9 +573,9 @@ func (daemon *Daemon) GetNetworks(filter filters.Args, config types.NetworkListC } if config.Detailed { - for i, n := range list { - np := &n - buildDetailedNetworkResources(np, idx[n.ID], config.Verbose) + for i := range list { + np := &list[i] + buildDetailedNetworkResources(np, idx[np.ID], config.Verbose) list[i] = *np } } From 7c91fd4240ed7024cd1f23ac0a27a5338671c251 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 16 Apr 2021 21:39:29 +0200 Subject: [PATCH 08/19] unused: ignore false positives integration/build/build_session_test.go:92:6: func `testBuildWithSession` is unused (unused) func testBuildWithSession(t *testing.T, client dclient.APIClient, daemonHost string, dir, dockerfile string) (outStr string) { ^ integration/container/checkpoint_test.go:23:6: func `containerExec` is unused (unused) func containerExec(t *testing.T, client client.APIClient, cID string, cmd []string) { ^ integration/network/service_test.go:295:6: func `swarmIngressReady` is unused (unused) func swarmIngressReady(client client.NetworkAPIClient) func(log poll.LogT) poll.Result { ^ Signed-off-by: Sebastiaan van Stijn --- integration/build/build_session_test.go | 1 + integration/container/checkpoint_test.go | 1 + integration/network/service_test.go | 1 + 3 files changed, 3 insertions(+) diff --git a/integration/build/build_session_test.go b/integration/build/build_session_test.go index d1219c79f5..7c626416a1 100644 --- a/integration/build/build_session_test.go +++ b/integration/build/build_session_test.go @@ -89,6 +89,7 @@ func TestBuildWithSession(t *testing.T) { assert.Check(t, is.Equal(du.BuilderSize, int64(0))) } +//nolint:unused // false positive: linter detects this as "unused" func testBuildWithSession(t *testing.T, client dclient.APIClient, daemonHost string, dir, dockerfile string) (outStr string) { ctx := context.Background() sess, err := session.NewSession(ctx, "foo1", "foo") diff --git a/integration/container/checkpoint_test.go b/integration/container/checkpoint_test.go index 88ecce18b6..fb37fcea60 100644 --- a/integration/container/checkpoint_test.go +++ b/integration/container/checkpoint_test.go @@ -20,6 +20,7 @@ import ( "gotest.tools/v3/skip" ) +//nolint:unused // false positive: linter detects this as "unused" func containerExec(t *testing.T, client client.APIClient, cID string, cmd []string) { t.Logf("Exec: %s", cmd) ctx := context.Background() diff --git a/integration/network/service_test.go b/integration/network/service_test.go index 6fc71fab7b..bff45c0909 100644 --- a/integration/network/service_test.go +++ b/integration/network/service_test.go @@ -292,6 +292,7 @@ func TestServiceRemoveKeepsIngressNetwork(t *testing.T) { assert.Assert(t, ok, "ingress-sbox not present in ingress network") } +//nolint:unused // for some reason, the "unused" linter marks this function as "unused" func swarmIngressReady(client client.NetworkAPIClient) func(log poll.LogT) poll.Result { return func(log poll.LogT) poll.Result { netInfo, err := client.NetworkInspect(context.Background(), ingressNet, types.NetworkInspectOptions{ From 09191c093645f308e5fbce4275b5cdfefb048a67 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 16 Apr 2021 22:04:29 +0200 Subject: [PATCH 09/19] daemon/stats: fix notRunningErr / notFoundErr detected as unused (false positive) Also looks like a false positive, but given that these were basically testing for the `errdefs.Conflict` and `errdefs.NotFound` interfaces, I replaced these with those; daemon/stats/collector.go:154:6: type `notRunningErr` is unused (unused) type notRunningErr interface { ^ daemon/stats/collector.go:159:6: type `notFoundErr` is unused (unused) type notFoundErr interface { ^ Signed-off-by: Sebastiaan van Stijn --- daemon/stats/collector.go | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/daemon/stats/collector.go b/daemon/stats/collector.go index 5f1d84fe4f..4356661c40 100644 --- a/daemon/stats/collector.go +++ b/daemon/stats/collector.go @@ -7,6 +7,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/container" + "github.com/docker/docker/errdefs" "github.com/docker/docker/pkg/pubsub" "github.com/sirupsen/logrus" ) @@ -131,7 +132,7 @@ func (s *Collector) Run() { pair.publisher.Publish(*stats) - case notRunningErr, notFoundErr: + case errdefs.ErrConflict, errdefs.ErrNotFound: // publish empty stats containing only name and ID if not running or not found pair.publisher.Publish(types.StatsJSON{ Name: pair.container.Name, @@ -150,13 +151,3 @@ func (s *Collector) Run() { time.Sleep(s.interval) } } - -type notRunningErr interface { - error - Conflict() -} - -type notFoundErr interface { - error - NotFound() -} From 7b071e055716028dc8f45842105cd6ce0ff7db1e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 16 Apr 2021 22:29:06 +0200 Subject: [PATCH 10/19] pkg/archive: RebaseArchiveEntries(): ignore G110 pkg/archive/copy.go:357:16: G110: Potential DoS vulnerability via decompression bomb (gosec) if _, err = io.Copy(rebasedTar, srcTar); err != nil { ^ Ignoring GoSec G110. See https://github.com/securego/gosec/pull/433 and https://cure53.de/pentest-report_opa.pdf, which recommends to replace io.Copy with io.CopyN7. The latter allows to specify the maximum number of bytes that should be read. By properly defining the limit, it can be assured that a GZip compression bomb cannot easily cause a Denial-of-Service. After reviewing, this should not affect us, because here we do not read into memory. Signed-off-by: Sebastiaan van Stijn --- pkg/archive/copy.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/archive/copy.go b/pkg/archive/copy.go index 57fddac078..4b9f504d7d 100644 --- a/pkg/archive/copy.go +++ b/pkg/archive/copy.go @@ -354,6 +354,16 @@ func RebaseArchiveEntries(srcContent io.Reader, oldBase, newBase string) io.Read return } + // Ignoring GoSec G110. See https://github.com/securego/gosec/pull/433 + // and https://cure53.de/pentest-report_opa.pdf, which recommends to + // replace io.Copy with io.CopyN7. The latter allows to specify the + // maximum number of bytes that should be read. By properly defining + // the limit, it can be assured that a GZip compression bomb cannot + // easily cause a Denial-of-Service. + // After reviewing with @tonistiigi and @cpuguy83, this should not + // affect us, because here we do not read into memory, hence should + // not be vulnerable to this code consuming memory. + //nolint:gosec // G110: Potential DoS vulnerability via decompression bomb (gosec) if _, err = io.Copy(rebasedTar, srcTar); err != nil { w.CloseWithError(err) return From 4004a39d53d1922a764eddcf2326c660aec308c3 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 31 May 2021 11:26:10 +0200 Subject: [PATCH 11/19] daemon/splunk: ignore G402: TLS MinVersion too low for now daemon/logger/splunk/splunk.go:173:16: G402: TLS MinVersion too low. (gosec) tlsConfig := &tls.Config{} ^ Signed-off-by: Sebastiaan van Stijn --- daemon/logger/splunk/splunk.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/daemon/logger/splunk/splunk.go b/daemon/logger/splunk/splunk.go index 05a4c8c93a..760a255003 100644 --- a/daemon/logger/splunk/splunk.go +++ b/daemon/logger/splunk/splunk.go @@ -170,7 +170,8 @@ func New(info logger.Info) (logger.Logger, error) { return nil, fmt.Errorf("%s: %s is expected", driverName, splunkTokenKey) } - tlsConfig := &tls.Config{} + // FIXME set minimum TLS version for splunk (see https://github.com/moby/moby/issues/42443) + tlsConfig := &tls.Config{} //nolint: gosec // G402: TLS MinVersion too low. // Splunk is using autogenerated certificates by default, // allow users to trust them with skipping verification From dd1374f7b279f265093878de1b28bf1d2eb4a4fc Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 31 May 2021 11:32:33 +0200 Subject: [PATCH 12/19] if-return: redundant if ...; err != nil check (revive) builder/builder-next/adapters/snapshot/snapshot.go:386:3: if-return: redundant if ...; err != nil check, just return error instead. (revive) if err := b.Put(keyIsCommitted, []byte{}); err != nil { return err } plugin/fetch_linux.go:112:2: if-return: redundant if ...; err != nil check, just return error instead. (revive) if err := images.Dispatch(ctx, images.Handlers(handlers...), nil, desc); err != nil { return err } Signed-off-by: Sebastiaan van Stijn --- builder/builder-next/adapters/snapshot/snapshot.go | 5 +---- plugin/fetch_linux.go | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/builder/builder-next/adapters/snapshot/snapshot.go b/builder/builder-next/adapters/snapshot/snapshot.go index 14ff5fd289..19ee488b8e 100644 --- a/builder/builder-next/adapters/snapshot/snapshot.go +++ b/builder/builder-next/adapters/snapshot/snapshot.go @@ -383,10 +383,7 @@ func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snap if err != nil { return err } - if err := b.Put(keyIsCommitted, []byte{}); err != nil { - return err - } - return nil + return b.Put(keyIsCommitted, []byte{}) }) } diff --git a/plugin/fetch_linux.go b/plugin/fetch_linux.go index 19b9bbf18e..8ec85f9f36 100644 --- a/plugin/fetch_linux.go +++ b/plugin/fetch_linux.go @@ -109,10 +109,7 @@ func (pm *Manager) fetch(ctx context.Context, ref reference.Named, auth *types.A fp := withFetchProgress(pm.blobStore, out, ref) handlers = append([]images.Handler{fp, remotes.FetchHandler(pm.blobStore, fetcher)}, handlers...) - if err := images.Dispatch(ctx, images.Handlers(handlers...), nil, desc); err != nil { - return err - } - return nil + return images.Dispatch(ctx, images.Handlers(handlers...), nil, desc) } // applyLayer makes an images.HandlerFunc which applies a fetched image rootfs layer to a directory. From bb17074119520860bb09d145eb9e2c86bfd21330 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 31 May 2021 11:39:04 +0200 Subject: [PATCH 13/19] reformat "nolint" comments Unlike regular comments, nolint comments should not have a leading space. Signed-off-by: Sebastiaan van Stijn --- builder/builder-next/worker/gc_unix.go | 2 +- daemon/config/config_unix.go | 2 +- daemon/daemon_unix.go | 8 ++++---- daemon/graphdriver/copy/copy.go | 4 ++-- daemon/graphdriver/devmapper/deviceset.go | 4 ++-- daemon/graphdriver/devmapper/devmapper_test.go | 2 +- daemon/network.go | 4 ++-- daemon/top_unix.go | 2 +- integration/internal/network/network.go | 2 +- oci/oci.go | 2 +- pkg/archive/archive_unix.go | 4 ++-- pkg/archive/archive_unix_test.go | 2 +- pkg/devicemapper/devmapper.go | 2 +- pkg/loopback/loopback.go | 2 +- pkg/system/chtimes_linux_test.go | 10 +++++----- pkg/system/stat_linux.go | 2 +- volume/drivers/extpoint.go | 2 +- 17 files changed, 28 insertions(+), 28 deletions(-) diff --git a/builder/builder-next/worker/gc_unix.go b/builder/builder-next/worker/gc_unix.go index 7457a06654..ffa63c5aad 100644 --- a/builder/builder-next/worker/gc_unix.go +++ b/builder/builder-next/worker/gc_unix.go @@ -11,7 +11,7 @@ func detectDefaultGCCap(root string) int64 { if err := syscall.Statfs(root, &st); err != nil { return defaultCap } - diskSize := int64(st.Bsize) * int64(st.Blocks) // nolint unconvert + diskSize := int64(st.Bsize) * int64(st.Blocks) //nolint unconvert avail := diskSize / 10 return (avail/(1<<30) + 1) * 1e9 // round up } diff --git a/daemon/config/config_unix.go b/daemon/config/config_unix.go index 4a32f55cbf..2ca5309418 100644 --- a/daemon/config/config_unix.go +++ b/daemon/config/config_unix.go @@ -89,7 +89,7 @@ func verifyDefaultIpcMode(mode string) error { func verifyDefaultCgroupNsMode(mode string) error { cm := containertypes.CgroupnsMode(mode) if !cm.Valid() { - return fmt.Errorf("Default cgroup namespace mode (%v) is invalid. Use \"host\" or \"private\".", cm) // nolint: golint + return fmt.Errorf("Default cgroup namespace mode (%v) is invalid. Use \"host\" or \"private\".", cm) //nolint: golint } return nil diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index 34c2db8ee3..6313a39658 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -186,8 +186,8 @@ func getBlkioWeightDevices(config containertypes.Resources) ([]specs.LinuxWeight weight := weightDevice.Weight d := specs.LinuxWeightDevice{Weight: &weight} // The type is 32bit on mips. - d.Major = int64(unix.Major(uint64(stat.Rdev))) // nolint: unconvert - d.Minor = int64(unix.Minor(uint64(stat.Rdev))) // nolint: unconvert + d.Major = int64(unix.Major(uint64(stat.Rdev))) //nolint: unconvert + d.Minor = int64(unix.Minor(uint64(stat.Rdev))) //nolint: unconvert blkioWeightDevices = append(blkioWeightDevices, d) } @@ -258,8 +258,8 @@ func getBlkioThrottleDevices(devs []*blkiodev.ThrottleDevice) ([]specs.LinuxThro } d := specs.LinuxThrottleDevice{Rate: d.Rate} // the type is 32bit on mips - d.Major = int64(unix.Major(uint64(stat.Rdev))) // nolint: unconvert - d.Minor = int64(unix.Minor(uint64(stat.Rdev))) // nolint: unconvert + d.Major = int64(unix.Major(uint64(stat.Rdev))) //nolint: unconvert + d.Minor = int64(unix.Minor(uint64(stat.Rdev))) //nolint: unconvert throttleDevices = append(throttleDevices, d) } diff --git a/daemon/graphdriver/copy/copy.go b/daemon/graphdriver/copy/copy.go index 962eddfd7b..98188070c4 100644 --- a/daemon/graphdriver/copy/copy.go +++ b/daemon/graphdriver/copy/copy.go @@ -144,7 +144,7 @@ func DirCopy(srcDir, dstDir string, copyMode Mode, copyXattrs bool) error { switch mode := f.Mode(); { case mode.IsRegular(): // the type is 32bit on mips - id := fileID{dev: uint64(stat.Dev), ino: stat.Ino} // nolint: unconvert + id := fileID{dev: uint64(stat.Dev), ino: stat.Ino} //nolint: unconvert if copyMode == Hardlink { isHardlink = true if err2 := os.Link(srcPath, dstPath); err2 != nil { @@ -223,7 +223,7 @@ func DirCopy(srcDir, dstDir string, copyMode Mode, copyXattrs bool) error { } // system.Chtimes doesn't support a NOFOLLOW flag atm - // nolint: unconvert + //nolint: unconvert if f.IsDir() { dirsToSetMtimes.PushFront(&dirMtimeInfo{dstPath: &dstPath, stat: stat}) } else if !isSymlink { diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go index 590e062123..c1b617f4dc 100644 --- a/daemon/graphdriver/devmapper/deviceset.go +++ b/daemon/graphdriver/devmapper/deviceset.go @@ -1534,7 +1534,7 @@ func getDeviceMajorMinor(file *os.File) (uint64, uint64, error) { } // the type is 32bit on mips - dev := uint64(stat.Rdev) // nolint: unconvert + dev := uint64(stat.Rdev) //nolint: unconvert majorNum := major(dev) minorNum := minor(dev) @@ -1746,7 +1746,7 @@ func (devices *DeviceSet) initDevmapper(doInit bool) (retErr error) { // - The target of this device is at major and minor // - If is defined, use that file inside the device as a loopback image. Otherwise use the device itself. // The type Dev in Stat_t is 32bit on mips. - devices.devicePrefix = fmt.Sprintf("docker-%d:%d-%d", major(uint64(st.Dev)), minor(uint64(st.Dev)), st.Ino) // nolint: unconvert + devices.devicePrefix = fmt.Sprintf("docker-%d:%d-%d", major(uint64(st.Dev)), minor(uint64(st.Dev)), st.Ino) //nolint: unconvert logger.Debugf("Generated prefix: %s", devices.devicePrefix) // Check for the existence of the thin-pool device diff --git a/daemon/graphdriver/devmapper/devmapper_test.go b/daemon/graphdriver/devmapper/devmapper_test.go index afd6c5b4d9..c530d74d9c 100644 --- a/daemon/graphdriver/devmapper/devmapper_test.go +++ b/daemon/graphdriver/devmapper/devmapper_test.go @@ -41,7 +41,7 @@ func initLoopbacks() error { // only create new loopback files if they don't exist if _, err := os.Stat(loopPath); err != nil { if mkerr := syscall.Mknod(loopPath, - uint32(statT.Mode|syscall.S_IFBLK), int((7<<8)|(i&0xff)|((i&0xfff00)<<12))); mkerr != nil { // nolint: unconvert + uint32(statT.Mode|syscall.S_IFBLK), int((7<<8)|(i&0xff)|((i&0xfff00)<<12))); mkerr != nil { //nolint: unconvert return mkerr } os.Chown(loopPath, int(statT.Uid), int(statT.Gid)) diff --git a/daemon/network.go b/daemon/network.go index 46c286025f..fc84eaa164 100644 --- a/daemon/network.go +++ b/daemon/network.go @@ -155,7 +155,7 @@ var ( func (daemon *Daemon) startIngressWorker() { ingressJobsChannel = make(chan *ingressJob, 100) go func() { - // nolint: gosimple + //nolint: gosimple for { select { case r := <-ingressJobsChannel: @@ -365,7 +365,7 @@ func (daemon *Daemon) createNetwork(create types.NetworkCreateRequest, id string n, err := c.NewNetwork(driver, create.Name, id, nwOptions...) if err != nil { if _, ok := err.(libnetwork.ErrDataStoreNotInitialized); ok { - // nolint: golint + //nolint: golint return nil, errors.New("This node is not a swarm manager. Use \"docker swarm init\" or \"docker swarm join\" to connect this node to swarm and try again.") } return nil, err diff --git a/daemon/top_unix.go b/daemon/top_unix.go index 570cdce2eb..cedda94bcb 100644 --- a/daemon/top_unix.go +++ b/daemon/top_unix.go @@ -20,7 +20,7 @@ func validatePSArgs(psArgs string) error { // NOTE: \\s does not detect unicode whitespaces. // So we use fieldsASCII instead of strings.Fields in parsePSOutput. // See https://github.com/docker/docker/pull/24358 - // nolint: gosimple + //nolint: gosimple re := regexp.MustCompile("\\s+([^\\s]*)=\\s*(PID[^\\s]*)") for _, group := range re.FindAllStringSubmatch(psArgs, -1) { if len(group) >= 3 { diff --git a/integration/internal/network/network.go b/integration/internal/network/network.go index 4f1f176993..04f29c7bb2 100644 --- a/integration/internal/network/network.go +++ b/integration/internal/network/network.go @@ -26,7 +26,7 @@ func Create(ctx context.Context, client client.APIClient, name string, ops ...fu } // CreateNoError creates a network with the specified options and verifies there were no errors -func CreateNoError(ctx context.Context, t *testing.T, client client.APIClient, name string, ops ...func(*types.NetworkCreate)) string { // nolint: golint +func CreateNoError(ctx context.Context, t *testing.T, client client.APIClient, name string, ops ...func(*types.NetworkCreate)) string { //nolint: golint t.Helper() name, err := createNetwork(ctx, client, name, ops...) diff --git a/oci/oci.go b/oci/oci.go index fdc1e06de2..2a4c0d79aa 100644 --- a/oci/oci.go +++ b/oci/oci.go @@ -13,7 +13,7 @@ import ( // that *only* passes `a` as value: `echo a > /sys/fs/cgroup/1/devices.allow, which would be // the "implicit" equivalent of "a *:* rwm". Source-code also looks to confirm this, and returns // early for "a" (all); https://github.com/torvalds/linux/blob/v5.10/security/device_cgroup.c#L614-L642 -// nolint: gosimple +//nolint: gosimple var deviceCgroupRuleRegex = regexp.MustCompile("^([acb]) ([0-9]+|\\*):([0-9]+|\\*) ([rwm]{1,3})$") // SetCapabilities sets the provided capabilities on the spec diff --git a/pkg/archive/archive_unix.go b/pkg/archive/archive_unix.go index 0b92bb0f4a..ef774fe6b7 100644 --- a/pkg/archive/archive_unix.go +++ b/pkg/archive/archive_unix.go @@ -51,8 +51,8 @@ func setHeaderForSpecialDevice(hdr *tar.Header, name string, stat interface{}) ( // Currently go does not fill in the major/minors if s.Mode&unix.S_IFBLK != 0 || s.Mode&unix.S_IFCHR != 0 { - hdr.Devmajor = int64(unix.Major(uint64(s.Rdev))) // nolint: unconvert - hdr.Devminor = int64(unix.Minor(uint64(s.Rdev))) // nolint: unconvert + hdr.Devmajor = int64(unix.Major(uint64(s.Rdev))) //nolint: unconvert + hdr.Devminor = int64(unix.Minor(uint64(s.Rdev))) //nolint: unconvert } } diff --git a/pkg/archive/archive_unix_test.go b/pkg/archive/archive_unix_test.go index 6274744b61..c5767785c6 100644 --- a/pkg/archive/archive_unix_test.go +++ b/pkg/archive/archive_unix_test.go @@ -186,7 +186,7 @@ func getNlink(path string) (uint64, error) { return 0, fmt.Errorf("expected type *syscall.Stat_t, got %t", stat.Sys()) } // We need this conversion on ARM64 - // nolint: unconvert + //nolint: unconvert return uint64(statT.Nlink), nil } diff --git a/pkg/devicemapper/devmapper.go b/pkg/devicemapper/devmapper.go index edc6009ff3..926ae8a7b7 100644 --- a/pkg/devicemapper/devmapper.go +++ b/pkg/devicemapper/devmapper.go @@ -14,7 +14,7 @@ import ( ) // Same as DM_DEVICE_* enum values from libdevmapper.h -// nolint: deadcode,unused,varcheck +//nolint: deadcode,unused,varcheck const ( deviceCreate TaskType = iota deviceReload diff --git a/pkg/loopback/loopback.go b/pkg/loopback/loopback.go index 40fde96621..5e0def6c56 100644 --- a/pkg/loopback/loopback.go +++ b/pkg/loopback/loopback.go @@ -38,7 +38,7 @@ func FindLoopDeviceFor(file *os.File) *os.File { } targetInode := stat.Ino // the type is 32bit on mips - targetDevice := uint64(stat.Dev) // nolint: unconvert + targetDevice := uint64(stat.Dev) //nolint: unconvert for i := 0; true; i++ { path := fmt.Sprintf("/dev/loop%d", i) diff --git a/pkg/system/chtimes_linux_test.go b/pkg/system/chtimes_linux_test.go index 273fc0a893..075f573059 100644 --- a/pkg/system/chtimes_linux_test.go +++ b/pkg/system/chtimes_linux_test.go @@ -26,7 +26,7 @@ func TestChtimesLinux(t *testing.T) { } stat := f.Sys().(*syscall.Stat_t) - aTime := time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) // nolint: unconvert + aTime := time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) //nolint: unconvert if aTime != unixEpochTime { t.Fatalf("Expected: %s, got: %s", unixEpochTime, aTime) } @@ -40,7 +40,7 @@ func TestChtimesLinux(t *testing.T) { } stat = f.Sys().(*syscall.Stat_t) - aTime = time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) // nolint: unconvert + aTime = time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) //nolint: unconvert if aTime != unixEpochTime { t.Fatalf("Expected: %s, got: %s", unixEpochTime, aTime) } @@ -54,7 +54,7 @@ func TestChtimesLinux(t *testing.T) { } stat = f.Sys().(*syscall.Stat_t) - aTime = time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) // nolint: unconvert + aTime = time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) //nolint: unconvert if aTime != unixEpochTime { t.Fatalf("Expected: %s, got: %s", unixEpochTime, aTime) } @@ -68,7 +68,7 @@ func TestChtimesLinux(t *testing.T) { } stat = f.Sys().(*syscall.Stat_t) - aTime = time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) // nolint: unconvert + aTime = time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) //nolint: unconvert if aTime != afterUnixEpochTime { t.Fatalf("Expected: %s, got: %s", afterUnixEpochTime, aTime) } @@ -82,7 +82,7 @@ func TestChtimesLinux(t *testing.T) { } stat = f.Sys().(*syscall.Stat_t) - aTime = time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) // nolint: unconvert + aTime = time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) //nolint: unconvert if aTime.Truncate(time.Second) != unixMaxTime.Truncate(time.Second) { t.Fatalf("Expected: %s, got: %s", unixMaxTime.Truncate(time.Second), aTime.Truncate(time.Second)) } diff --git a/pkg/system/stat_linux.go b/pkg/system/stat_linux.go index 17d5d131a3..3ac02393f0 100644 --- a/pkg/system/stat_linux.go +++ b/pkg/system/stat_linux.go @@ -9,7 +9,7 @@ func fromStatT(s *syscall.Stat_t) (*StatT, error) { uid: s.Uid, gid: s.Gid, // the type is 32bit on mips - rdev: uint64(s.Rdev), // nolint: unconvert + rdev: uint64(s.Rdev), //nolint: unconvert mtim: s.Mtim}, nil } diff --git a/volume/drivers/extpoint.go b/volume/drivers/extpoint.go index dbe0086ab6..3878736cbb 100644 --- a/volume/drivers/extpoint.go +++ b/volume/drivers/extpoint.go @@ -21,7 +21,7 @@ const extName = "VolumeDriver" // volumeDriver defines the available functions that volume plugins must implement. // This interface is only defined to generate the proxy objects. // It's not intended to be public or reused. -// nolint: deadcode +//nolint: deadcode type volumeDriver interface { // Create a volume with the given name Create(name string, opts map[string]string) (err error) From 16ced7622b29df51a5bb77a17332d825a414ccd0 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 31 May 2021 11:48:30 +0200 Subject: [PATCH 14/19] daemon/config: error strings should not be capitalized daemon/config/config_unix.go:92:21: error-strings: error strings should not be capitalized or end with punctuation or a newline (revive) return fmt.Errorf("Default cgroup namespace mode (%v) is invalid. Use \"host\" or \"private\".", cm) // nolint: golint ^ Signed-off-by: Sebastiaan van Stijn --- daemon/config/config_unix.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/daemon/config/config_unix.go b/daemon/config/config_unix.go index 2ca5309418..3aef861475 100644 --- a/daemon/config/config_unix.go +++ b/daemon/config/config_unix.go @@ -74,14 +74,14 @@ func (conf *Config) IsSwarmCompatible() error { } func verifyDefaultIpcMode(mode string) error { - const hint = "Use \"shareable\" or \"private\"." + const hint = `use "shareable" or "private"` dm := containertypes.IpcMode(mode) if !dm.Valid() { - return fmt.Errorf("Default IPC mode setting (%v) is invalid. "+hint, dm) + return fmt.Errorf("default IPC mode setting (%v) is invalid; "+hint, dm) } if dm != "" && !dm.IsPrivate() && !dm.IsShareable() { - return fmt.Errorf("IPC mode \"%v\" is not supported as default value. "+hint, dm) + return fmt.Errorf(`IPC mode "%v" is not supported as default value; `+hint, dm) } return nil } @@ -89,7 +89,7 @@ func verifyDefaultIpcMode(mode string) error { func verifyDefaultCgroupNsMode(mode string) error { cm := containertypes.CgroupnsMode(mode) if !cm.Valid() { - return fmt.Errorf("Default cgroup namespace mode (%v) is invalid. Use \"host\" or \"private\".", cm) //nolint: golint + return fmt.Errorf(`default cgroup namespace mode (%v) is invalid; use "host" or "private"`, cm) } return nil From d61b7c1211c32c033c1a42cf5a6f9314b4e1ffc6 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 31 May 2021 11:54:56 +0200 Subject: [PATCH 15/19] daemon: var-declaration: should omit type bool (revive) daemon/list.go:556:18: var-declaration: should omit type bool from declaration of var shouldSkip; it will be inferred from the right-hand side (revive) shouldSkip bool = true ^ Signed-off-by: Sebastiaan van Stijn --- daemon/list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemon/list.go b/daemon/list.go index fde1342e69..e3c4153582 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -553,7 +553,7 @@ func includeContainerInList(container *container.Snapshot, ctx *listContext) ite if len(ctx.expose) > 0 || len(ctx.publish) > 0 { var ( - shouldSkip bool = true + shouldSkip = true publishedPort nat.Port exposedPort nat.Port ) From e6dabfa97721805762c6402289169edfea4617b7 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 31 May 2021 13:08:46 +0200 Subject: [PATCH 16/19] graphdriver: temporarily ignore unsafeptr: possible misuse of reflect.SliceHeader Probably needs a similar change as c208f03fbddb4355729c3225bb2550c4d54a2c5e, but this code makes my head spin, so for now suppressing, and created a tracking issue: daemon/graphdriver/graphtest/graphtest_unix.go:305:12: unsafeptr: possible misuse of reflect.SliceHeader (govet) header := *(*reflect.SliceHeader)(unsafe.Pointer(&buf)) ^ daemon/graphdriver/graphtest/graphtest_unix.go:308:36: unsafeptr: possible misuse of reflect.SliceHeader (govet) data := *(*[]byte)(unsafe.Pointer(&header)) ^ Signed-off-by: Sebastiaan van Stijn --- daemon/graphdriver/graphtest/graphtest_unix.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/daemon/graphdriver/graphtest/graphtest_unix.go b/daemon/graphdriver/graphtest/graphtest_unix.go index 4073e0b7a0..f0c0e78f81 100644 --- a/daemon/graphdriver/graphtest/graphtest_unix.go +++ b/daemon/graphdriver/graphtest/graphtest_unix.go @@ -302,10 +302,10 @@ func writeRandomFile(path string, size uint64) error { } // Cast to []byte - header := *(*reflect.SliceHeader)(unsafe.Pointer(&buf)) + header := *(*reflect.SliceHeader)(unsafe.Pointer(&buf)) //nolint:govet // FIXME: unsafeptr: possible misuse of reflect.SliceHeader (govet) see https://github.com/moby/moby/issues/42444 header.Len *= 8 header.Cap *= 8 - data := *(*[]byte)(unsafe.Pointer(&header)) + data := *(*[]byte)(unsafe.Pointer(&header)) //nolint:govet // FIXME: unsafeptr: possible misuse of reflect.SliceHeader (govet) see https://github.com/moby/moby/issues/42444 return ioutil.WriteFile(path, data, 0700) } From ea74765a5853ec3d026fc6bc8598d665d457b340 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 16 Apr 2021 16:50:31 +0200 Subject: [PATCH 17/19] golangci.yml: update regex for ignoring SA1019 The message changed from "is deprecated" to "has been deprecated": client/hijack.go:85:16: SA1019: httputil.NewClientConn has been deprecated since Go 1.0: Use the Client or Transport in package net/http instead. (staticcheck) clientconn := httputil.NewClientConn(conn, nil) ^ integration/plugin/authz/authz_plugin_test.go:180:7: SA1019: httputil.NewClientConn has been deprecated since Go 1.0: Use the Client or Transport in package net/http instead. (staticcheck) c := httputil.NewClientConn(conn, nil) ^ integration/plugin/authz/authz_plugin_test.go:479:12: SA1019: httputil.NewClientConn has been deprecated since Go 1.0: Use the Client or Transport in package net/http instead. (staticcheck) client := httputil.NewClientConn(conn, nil) ^ Signed-off-by: Sebastiaan van Stijn --- hack/validate/golangci-lint.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hack/validate/golangci-lint.yml b/hack/validate/golangci-lint.yml index c4494b9d18..8af0a140cf 100644 --- a/hack/validate/golangci-lint.yml +++ b/hack/validate/golangci-lint.yml @@ -86,11 +86,11 @@ issues: linters: - staticcheck # FIXME temporarily suppress these. See #39926 - - text: "SA1019: httputil.NewClientConn is deprecated" + - text: "SA1019: httputil.NewClientConn" linters: - staticcheck # FIXME temporarily suppress these (related to the ones above) - - text: "SA1019: httputil.ErrPersistEOF is deprecated" + - text: "SA1019: httputil.ErrPersistEOF" linters: - staticcheck # This code is doing some fun stuff with reflect and it trips up the linter. From 22ce0f8faa3d5809603aef1a9f91070968bbbb42 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 17 Apr 2021 00:13:23 +0200 Subject: [PATCH 18/19] golangci.yml: skip some tests Signed-off-by: Sebastiaan van Stijn --- hack/validate/golangci-lint.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/hack/validate/golangci-lint.yml b/hack/validate/golangci-lint.yml index 8af0a140cf..7f1f9c4396 100644 --- a/hack/validate/golangci-lint.yml +++ b/hack/validate/golangci-lint.yml @@ -81,6 +81,10 @@ issues: - text: "(G201|G202): SQL string (formatting|concatenation)" linters: - gosec + # FIXME: evaluate these and fix where needed: G307: Deferring unsafe method "*os.File" on type "Close" (gosec) + - text: "G307: Deferring unsafe method" + linters: + - gosec # FIXME temporarily suppress these. See #39924 - text: "SA1019: .*\\.Xattrs is deprecated: Use PAXRecords instead" linters: @@ -93,6 +97,11 @@ issues: - text: "SA1019: httputil.ErrPersistEOF" linters: - staticcheck + # FIXME temporarily suppress these for false positives in tests (see https://github.com/dominikh/go-tools/issues/1022) + - text: "SA5011" + path: _test\.go + linters: + - staticcheck # This code is doing some fun stuff with reflect and it trips up the linter. - text: "field `foo` is unused" path: "libnetwork/options/options_test.go" @@ -105,4 +114,4 @@ issues: path: libnetwork/network.go linters: - structcheck - - unused \ No newline at end of file + - unused From 594c972fc541a6b179f4f47707bf70a7e420de7e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 31 May 2021 13:45:56 +0200 Subject: [PATCH 19/19] golangci.yml: do not limit max reported issues Signed-off-by: Sebastiaan van Stijn --- hack/validate/golangci-lint.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hack/validate/golangci-lint.yml b/hack/validate/golangci-lint.yml index 7f1f9c4396..c2081ed83f 100644 --- a/hack/validate/golangci-lint.yml +++ b/hack/validate/golangci-lint.yml @@ -115,3 +115,9 @@ issues: linters: - structcheck - unused + + # Maximum issues count per one linter. Set to 0 to disable. Default is 50. + max-issues-per-linter: 0 + + # Maximum count of issues with the same text. Set to 0 to disable. Default is 3. + max-same-issues: 0