From 3ebeb6d79bb60607e285933fea043c8f10c54e34 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 8 Oct 2022 13:25:33 +0200 Subject: [PATCH 1/6] linting: address gosec G112/G114 GOGC=75 golangci-lint run services/server/server.go:320:27: G114: Use of net/http serve function that has no support for setting timeouts (gosec) return trapClosedConnErr(http.Serve(l, m)) ^ services/server/server.go:340:27: G114: Use of net/http serve function that has no support for setting timeouts (gosec) return trapClosedConnErr(http.Serve(l, m)) ^ cmd/containerd-stress/main.go:238:13: G114: Use of net/http serve function that has no support for setting timeouts (gosec) if err := http.ListenAndServe(c.Metrics, metrics.Handler()); err != nil { ^ Signed-off-by: Sebastiaan van Stijn --- cmd/containerd-stress/main.go | 7 ++++++- services/server/server.go | 12 ++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/cmd/containerd-stress/main.go b/cmd/containerd-stress/main.go index da0cd0200..044563463 100644 --- a/cmd/containerd-stress/main.go +++ b/cmd/containerd-stress/main.go @@ -235,7 +235,12 @@ func (c config) newClient() (*containerd.Client, error) { func serve(c config) error { go func() { - if err := http.ListenAndServe(c.Metrics, metrics.Handler()); err != nil { + srv := &http.Server{ + Addr: c.Metrics, + Handler: metrics.Handler(), + ReadHeaderTimeout: 5 * time.Minute, // "G112: Potential Slowloris Attack (gosec)"; not a real concern for our use, so setting a long timeout. + } + if err := srv.ListenAndServe(); err != nil { logrus.WithError(err).Error("listen and serve") } }() diff --git a/services/server/server.go b/services/server/server.go index d607e39d4..74c36df0a 100644 --- a/services/server/server.go +++ b/services/server/server.go @@ -317,7 +317,11 @@ func (s *Server) ServeTTRPC(l net.Listener) error { func (s *Server) ServeMetrics(l net.Listener) error { m := http.NewServeMux() m.Handle("/v1/metrics", metrics.Handler()) - return trapClosedConnErr(http.Serve(l, m)) + srv := &http.Server{ + Handler: m, + ReadHeaderTimeout: 5 * time.Minute, // "G112: Potential Slowloris Attack (gosec)"; not a real concern for our use, so setting a long timeout. + } + return trapClosedConnErr(srv.Serve(l)) } // ServeTCP allows services to serve over tcp @@ -337,7 +341,11 @@ func (s *Server) ServeDebug(l net.Listener) error { m.Handle("/debug/pprof/profile", http.HandlerFunc(pprof.Profile)) m.Handle("/debug/pprof/symbol", http.HandlerFunc(pprof.Symbol)) m.Handle("/debug/pprof/trace", http.HandlerFunc(pprof.Trace)) - return trapClosedConnErr(http.Serve(l, m)) + srv := &http.Server{ + Handler: m, + ReadHeaderTimeout: 5 * time.Minute, // "G112: Potential Slowloris Attack (gosec)"; not a real concern for our use, so setting a long timeout. + } + return trapClosedConnErr(srv.Serve(l)) } // Stop the containerd server canceling any open connections From 0eaace3066fe848a337df1e20c50e93974ac56fd Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 31 Aug 2022 09:18:36 +0200 Subject: [PATCH 2/6] golangci-lint: sort linters in config file THis makes it easier to find which linters are enabled when going through the list. Signed-off-by: Sebastiaan van Stijn --- .golangci.yml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index aad6acff2..748863672 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,19 +1,19 @@ linters: enable: - - structcheck - - varcheck - - staticcheck - - unconvert + - exportloopref # Checks for pointers to enclosing loop variables - gofmt - goimports - - revive - - ineffassign - - vet - - unused - - misspell - gosec - - exportloopref # Checks for pointers to enclosing loop variables + - ineffassign + - misspell + - revive + - staticcheck + - structcheck - tenv # Detects using os.Setenv instead of t.Setenv since Go 1.17 + - unconvert + - unused + - varcheck + - vet disable: - errcheck From d215725136312ec652dd7ac6b0b2868d70887f0b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 8 Oct 2022 15:53:22 +0200 Subject: [PATCH 3/6] pkg/cri/(server|sbserver): criService.getTLSConfig() add TODO to verify nolint This `//nolint` was added in https://github.com/containerd/cri/commit/f5c7ac92724405806eb4e330ecab8f4350601089 to suppress warnings about the `NameToCertificate` function being deprecated: // Deprecated: NameToCertificate only allows associating a single certificate // with a given name. Leave that field nil to let the library select the first // compatible chain from Certificates. Looking at that, it was deprecated in Go 1.14 through https://github.com/golang/go/commit/eb93c684d40de4924fc0664d7d9e98a84d5a100b (https://go-review.googlesource.com/c/go/+/205059), which describes: crypto/tls: select only compatible chains from Certificates Now that we have a full implementation of the logic to check certificate compatibility, we can let applications just list multiple chains in Certificates (for example, an RSA and an ECDSA one) and choose the most appropriate automatically. NameToCertificate only maps each name to one chain, so simply deprecate it, and while at it simplify its implementation by not stripping trailing dots from the SNI (which is specified not to have any, see RFC 6066, Section 3) and by not supporting multi-level wildcards, which are not a thing in the WebPKI (and in crypto/x509). We should at least have a comment describing why we are ignoring this, but preferably review whether we should still use it. Signed-off-by: Sebastiaan van Stijn --- pkg/cri/sbserver/image_pull.go | 2 +- pkg/cri/server/image_pull.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cri/sbserver/image_pull.go b/pkg/cri/sbserver/image_pull.go index 148ba3487..b3579b421 100644 --- a/pkg/cri/sbserver/image_pull.go +++ b/pkg/cri/sbserver/image_pull.go @@ -318,7 +318,7 @@ func (c *criService) getTLSConfig(registryTLSConfig criconfig.TLSConfig) (*tls.C if len(cert.Certificate) != 0 { tlsConfig.Certificates = []tls.Certificate{cert} } - tlsConfig.BuildNameToCertificate() // nolint:staticcheck + tlsConfig.BuildNameToCertificate() //nolint:staticcheck // TODO(thaJeztah): verify if we should ignore the deprecation; see https://github.com/containerd/containerd/pull/7349/files#r990644833 } if registryTLSConfig.CAFile != "" { diff --git a/pkg/cri/server/image_pull.go b/pkg/cri/server/image_pull.go index e6af29e8a..fcda1ef33 100644 --- a/pkg/cri/server/image_pull.go +++ b/pkg/cri/server/image_pull.go @@ -318,7 +318,7 @@ func (c *criService) getTLSConfig(registryTLSConfig criconfig.TLSConfig) (*tls.C if len(cert.Certificate) != 0 { tlsConfig.Certificates = []tls.Certificate{cert} } - tlsConfig.BuildNameToCertificate() // nolint:staticcheck + tlsConfig.BuildNameToCertificate() //nolint:staticcheck // TODO(thaJeztah): verify if we should ignore the deprecation; see https://github.com/containerd/containerd/pull/7349/files#r990644833 } if registryTLSConfig.CAFile != "" { From 29c7fc9520c56cae56334dc55fc61b64691e77f4 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 30 Aug 2022 23:05:52 +0200 Subject: [PATCH 4/6] clean-up "nolint" comments, remove unused ones - fix "nolint" comments to be in the correct format (`//nolint:[,` no leading space, required colon (`:`) and linters. - remove "nolint" comments for errcheck, which is disabled in our config. - remove "nolint" comments that were no longer needed (nolintlint). - where known, add a comment describing why a "nolint" was applied. Signed-off-by: Sebastiaan van Stijn --- archive/tar_unix.go | 3 +-- contrib/fuzz/content_fuzzer.go | 2 +- integration/client/restart_monitor_test.go | 3 +-- oci/spec_opts.go | 7 +++---- oci/spec_opts_linux_test.go | 6 +++--- oci/spec_opts_nonlinux.go | 12 ++++++++---- oci/utils_unix.go | 4 ++-- pkg/cri/opts/container.go | 2 +- pkg/cri/sbserver/service.go | 2 +- pkg/cri/server/service.go | 2 +- pkg/cri/store/container/container.go | 2 +- pkg/cri/store/image/image.go | 2 +- pkg/cri/store/sandbox/sandbox.go | 2 +- pkg/netns/netns_linux.go | 6 +++--- pkg/progress/escape.go | 2 +- pkg/runtimeoptions/v1/doc.go | 2 +- 16 files changed, 30 insertions(+), 29 deletions(-) diff --git a/archive/tar_unix.go b/archive/tar_unix.go index ed2b5e696..ac80df349 100644 --- a/archive/tar_unix.go +++ b/archive/tar_unix.go @@ -58,8 +58,7 @@ func setHeaderForSpecialDevice(hdr *tar.Header, name string, fi os.FileInfo) err return errors.New("unsupported stat type") } - // Rdev is int32 on darwin/bsd, int64 on linux/solaris - rdev := uint64(s.Rdev) // nolint: unconvert + rdev := uint64(s.Rdev) //nolint:unconvert // rdev is int32 on darwin/bsd, int64 on linux/solaris // Currently go does not fill in the major/minors if s.Mode&syscall.S_IFBLK != 0 || diff --git a/contrib/fuzz/content_fuzzer.go b/contrib/fuzz/content_fuzzer.go index c0b67db57..da25650fd 100644 --- a/contrib/fuzz/content_fuzzer.go +++ b/contrib/fuzz/content_fuzzer.go @@ -14,7 +14,7 @@ limitations under the License. */ -// nolint: golint +//nolint:golint package fuzz import ( diff --git a/integration/client/restart_monitor_test.go b/integration/client/restart_monitor_test.go index f80930719..0681f6fc8 100644 --- a/integration/client/restart_monitor_test.go +++ b/integration/client/restart_monitor_test.go @@ -40,8 +40,7 @@ import ( exec "golang.org/x/sys/execabs" ) -// the following nolint is for shutting up gometalinter on non-linux. -// nolint: unused +//nolint:unused // Ignore on non-Linux func newDaemonWithConfig(t *testing.T, configTOML string) (*Client, *daemon, func()) { if testing.Short() { t.Skip() diff --git a/oci/spec_opts.go b/oci/spec_opts.go index 8adb59180..45c4fc180 100644 --- a/oci/spec_opts.go +++ b/oci/spec_opts.go @@ -76,7 +76,6 @@ func setLinux(s *Spec) { } } -// nolint func setResources(s *Spec) { if s.Linux != nil { if s.Linux.Resources == nil { @@ -85,7 +84,7 @@ func setResources(s *Spec) { } } -// nolint +//nolint:unused // not used on all platforms func setResourcesWindows(s *Spec) { if s.Windows != nil { if s.Windows.Resources == nil { @@ -94,7 +93,7 @@ func setResourcesWindows(s *Spec) { } } -// nolint +//nolint:unused // not used on all platforms func setCPU(s *Spec) { setResources(s) if s.Linux != nil { @@ -104,7 +103,7 @@ func setCPU(s *Spec) { } } -// nolint +//nolint:deadcode,unused // not used on all platforms func setCPUWindows(s *Spec) { setResourcesWindows(s) if s.Windows != nil { diff --git a/oci/spec_opts_linux_test.go b/oci/spec_opts_linux_test.go index d41e0efca..2f48d52f2 100644 --- a/oci/spec_opts_linux_test.go +++ b/oci/spec_opts_linux_test.go @@ -31,7 +31,7 @@ import ( "golang.org/x/sys/unix" ) -// nolint:gosec +//nolint:gosec func TestWithUserID(t *testing.T) { t.Parallel() @@ -86,7 +86,7 @@ guest:x:405:100:guest:/dev/null:/sbin/nologin } } -// nolint:gosec +//nolint:gosec func TestWithUsername(t *testing.T) { t.Parallel() @@ -148,7 +148,7 @@ guest:x:405:100:guest:/dev/null:/sbin/nologin } -// nolint:gosec +//nolint:gosec func TestWithAdditionalGIDs(t *testing.T) { t.Parallel() expectedPasswd := `root:x:0:0:root:/root:/bin/ash diff --git a/oci/spec_opts_nonlinux.go b/oci/spec_opts_nonlinux.go index 2f3ce80a4..e376a86b3 100644 --- a/oci/spec_opts_nonlinux.go +++ b/oci/spec_opts_nonlinux.go @@ -28,19 +28,22 @@ import ( // WithAllCurrentCapabilities propagates the effective capabilities of the caller process to the container process. // The capability set may differ from WithAllKnownCapabilities when running in a container. -// nolint: deadcode, unused +// +//nolint:unused var WithAllCurrentCapabilities = func(ctx context.Context, client Client, c *containers.Container, s *Spec) error { return WithCapabilities(nil)(ctx, client, c, s) } // WithAllKnownCapabilities sets all the known linux capabilities for the container process -// nolint: deadcode, unused +// +//nolint:unused var WithAllKnownCapabilities = func(ctx context.Context, client Client, c *containers.Container, s *Spec) error { return WithCapabilities(nil)(ctx, client, c, s) } // WithBlockIO sets the container's blkio parameters -// nolint: deadcode, unused +// +//nolint:unused func WithBlockIO(blockio interface{}) SpecOpts { return func(ctx context.Context, _ Client, c *containers.Container, s *Spec) error { return errors.New("blkio not supported") @@ -48,7 +51,8 @@ func WithBlockIO(blockio interface{}) SpecOpts { } // WithCPUShares sets the container's cpu shares -// nolint: deadcode, unused +// +//nolint:unused func WithCPUShares(shares uint64) SpecOpts { return func(ctx context.Context, _ Client, c *containers.Container, s *Spec) error { return nil diff --git a/oci/utils_unix.go b/oci/utils_unix.go index db75b0bad..921e882b2 100644 --- a/oci/utils_unix.go +++ b/oci/utils_unix.go @@ -127,7 +127,7 @@ func getDevices(path, containerPath string) ([]specs.LinuxDevice, error) { // TODO consider adding these consts to the OCI runtime-spec. const ( - wildcardDevice = "a" //nolint // currently unused, but should be included when upstreaming to OCI runtime-spec. + wildcardDevice = "a" //nolint:deadcode,unused,varcheck // currently unused, but should be included when upstreaming to OCI runtime-spec. blockDevice = "b" charDevice = "c" // or "u" fifoDevice = "p" @@ -148,7 +148,7 @@ func DeviceFromPath(path string) (*specs.LinuxDevice, error) { } var ( - devNumber = uint64(stat.Rdev) //nolint: unconvert // the type is 32bit on mips. + devNumber = uint64(stat.Rdev) //nolint:unconvert // the type is 32bit on mips. major = unix.Major(devNumber) minor = unix.Minor(devNumber) ) diff --git a/pkg/cri/opts/container.go b/pkg/cri/opts/container.go index 85fd2fdfe..5ea1b8739 100644 --- a/pkg/cri/opts/container.go +++ b/pkg/cri/opts/container.go @@ -83,7 +83,7 @@ func WithVolumes(volumeMounts map[string]string) containerd.NewContainerOpts { // if it fails but not RM snapshot data. // refer to https://github.com/containerd/containerd/pull/1868 // https://github.com/containerd/containerd/pull/1785 - defer os.Remove(root) // nolint: errcheck + defer os.Remove(root) unmounter := func(mountPath string) { if uerr := mount.Unmount(mountPath, 0); uerr != nil { diff --git a/pkg/cri/sbserver/service.go b/pkg/cri/sbserver/service.go index 02198f586..1d5b5d279 100644 --- a/pkg/cri/sbserver/service.go +++ b/pkg/cri/sbserver/service.go @@ -117,7 +117,7 @@ type criService struct { baseOCISpecs map[string]*oci.Spec // allCaps is the list of the capabilities. // When nil, parsed from CapEff of /proc/self/status. - allCaps []string // nolint + allCaps []string //nolint:unused // Ignore on non-Linux // unpackDuplicationSuppressor is used to make sure that there is only // one in-flight fetch request or unpack handler for a given descriptor's // or chain ID. diff --git a/pkg/cri/server/service.go b/pkg/cri/server/service.go index 2e2cca42b..0e411fa44 100644 --- a/pkg/cri/server/service.go +++ b/pkg/cri/server/service.go @@ -113,7 +113,7 @@ type criService struct { baseOCISpecs map[string]*oci.Spec // allCaps is the list of the capabilities. // When nil, parsed from CapEff of /proc/self/status. - allCaps []string // nolint + allCaps []string //nolint:unused // Ignore on non-Linux // unpackDuplicationSuppressor is used to make sure that there is only // one in-flight fetch request or unpack handler for a given descriptor's // or chain ID. diff --git a/pkg/cri/store/container/container.go b/pkg/cri/store/container/container.go index a5e35396b..524b58291 100644 --- a/pkg/cri/store/container/container.go +++ b/pkg/cri/store/container/container.go @@ -208,6 +208,6 @@ func (s *Store) Delete(id string) { c.IO.Close() } s.labels.Release(c.ProcessLabel) - s.idIndex.Delete(id) // nolint: errcheck + s.idIndex.Delete(id) delete(s.containers, id) } diff --git a/pkg/cri/store/image/image.go b/pkg/cri/store/image/image.go index 592384f35..d5f3d12b5 100644 --- a/pkg/cri/store/image/image.go +++ b/pkg/cri/store/image/image.go @@ -246,6 +246,6 @@ func (s *store) delete(id, ref string) { return } // Remove the image if it is not referenced any more. - s.digestSet.Remove(digest) // nolint: errcheck + s.digestSet.Remove(digest) delete(s.images, digest.String()) } diff --git a/pkg/cri/store/sandbox/sandbox.go b/pkg/cri/store/sandbox/sandbox.go index 1cf208e4c..35e4603a2 100644 --- a/pkg/cri/store/sandbox/sandbox.go +++ b/pkg/cri/store/sandbox/sandbox.go @@ -160,6 +160,6 @@ func (s *Store) Delete(id string) { return } s.labels.Release(s.sandboxes[id].ProcessLabel) - s.idIndex.Delete(id) // nolint: errcheck + s.idIndex.Delete(id) delete(s.sandboxes, id) } diff --git a/pkg/netns/netns_linux.go b/pkg/netns/netns_linux.go index 09b5c9791..03f68a568 100644 --- a/pkg/netns/netns_linux.go +++ b/pkg/netns/netns_linux.go @@ -77,7 +77,7 @@ func newNS(baseDir string) (nsPath string, err error) { defer func() { // Ensure the mount point is cleaned up on errors if err != nil { - os.RemoveAll(nsPath) // nolint: errcheck + os.RemoveAll(nsPath) } }() @@ -107,7 +107,7 @@ func newNS(baseDir string) (nsPath string, err error) { } // Put this thread back to the orig ns, since it might get reused (pre go1.10) - defer origNS.Set() // nolint: errcheck + defer origNS.Set() // bind mount the netns from the current thread (from /proc) onto the // mount point. This causes the namespace to persist, even when there @@ -214,6 +214,6 @@ func (n *NetNS) Do(f func(cnins.NetNS) error) error { if err != nil { return fmt.Errorf("get netns fd: %w", err) } - defer ns.Close() // nolint: errcheck + defer ns.Close() return ns.Do(f) } diff --git a/pkg/progress/escape.go b/pkg/progress/escape.go index d9ce5b088..e648fdda3 100644 --- a/pkg/progress/escape.go +++ b/pkg/progress/escape.go @@ -19,6 +19,6 @@ package progress const ( escape = "\x1b" reset = escape + "[0m" - red = escape + "[31m" // nolint: deadcode, varcheck, unused + red = escape + "[31m" //nolint:deadcode,unused,varcheck green = escape + "[32m" ) diff --git a/pkg/runtimeoptions/v1/doc.go b/pkg/runtimeoptions/v1/doc.go index 62525652f..9617e7404 100644 --- a/pkg/runtimeoptions/v1/doc.go +++ b/pkg/runtimeoptions/v1/doc.go @@ -14,4 +14,4 @@ limitations under the License. */ -package runtimeoptions_v1 //nolint +package runtimeoptions_v1 //nolint:revive // Ignore var-naming: don't use an underscore in package name (revive) From f9c80be1bbc02f0f2b725494ef2cda5abb54ed4c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 31 Aug 2022 09:43:17 +0200 Subject: [PATCH 5/6] remove unneeded nolint-comments (nolintlint), disable deprecated linters Remove nolint-comments that weren't hit by linters, and remove the "structcheck" and "varcheck" linters, as they have been deprecated: WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. WARN [runner] The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. WARN [linters context] structcheck is disabled because of generics. You can track the evolution of the generics support by following the https://github.com/golangci/golangci-lint/issues/2649. Signed-off-by: Sebastiaan van Stijn --- .golangci.yml | 3 +-- archive/tar_unix.go | 2 +- integration/main_test.go | 16 ++++++++-------- oci/spec_opts.go | 6 +++--- oci/spec_opts_nonlinux.go | 8 -------- oci/utils_unix.go | 4 ++-- pkg/cri/sbserver/container_stats.go | 2 +- pkg/cri/sbserver/container_stats_list.go | 2 +- pkg/cri/sbserver/sandbox_stats.go | 4 ++-- pkg/cri/sbserver/sandbox_stats_list.go | 4 ++-- pkg/cri/sbserver/service.go | 2 +- pkg/cri/server/container_stats.go | 2 +- pkg/cri/server/container_stats_list.go | 2 +- pkg/cri/server/sandbox_stats.go | 4 ++-- pkg/cri/server/sandbox_stats_list.go | 4 ++-- pkg/cri/server/service.go | 2 +- pkg/cri/store/container/metadata.go | 3 +-- pkg/cri/store/container/status.go | 3 +-- pkg/cri/store/sandbox/metadata.go | 3 +-- pkg/progress/escape.go | 2 +- 20 files changed, 33 insertions(+), 45 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 748863672..08699988a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -6,13 +6,12 @@ linters: - gosec - ineffassign - misspell + - nolintlint - revive - staticcheck - - structcheck - tenv # Detects using os.Setenv instead of t.Setenv since Go 1.17 - unconvert - unused - - varcheck - vet disable: - errcheck diff --git a/archive/tar_unix.go b/archive/tar_unix.go index ac80df349..7029e56e6 100644 --- a/archive/tar_unix.go +++ b/archive/tar_unix.go @@ -58,7 +58,7 @@ func setHeaderForSpecialDevice(hdr *tar.Header, name string, fi os.FileInfo) err return errors.New("unsupported stat type") } - rdev := uint64(s.Rdev) //nolint:unconvert // rdev is int32 on darwin/bsd, int64 on linux/solaris + rdev := uint64(s.Rdev) //nolint:nolintlint,unconvert // rdev is int32 on darwin/bsd, int64 on linux/solaris // Currently go does not fill in the major/minors if s.Mode&syscall.S_IFBLK != 0 || diff --git a/integration/main_test.go b/integration/main_test.go index 1bb097c0e..dc07d6697 100644 --- a/integration/main_test.go +++ b/integration/main_test.go @@ -210,7 +210,7 @@ func PodSandboxConfigWithCleanup(t *testing.T, name, ns string, opts ...PodSandb } // Set Windows HostProcess on the pod. -func WithWindowsHostProcessPod(p *runtime.PodSandboxConfig) { //nolint:unused +func WithWindowsHostProcessPod(p *runtime.PodSandboxConfig) { if p.Windows == nil { p.Windows = &runtime.WindowsPodSandboxConfig{} } @@ -237,7 +237,7 @@ func WithTestAnnotations() ContainerOpts { } // Add container resource limits. -func WithResources(r *runtime.LinuxContainerResources) ContainerOpts { //nolint:unused +func WithResources(r *runtime.LinuxContainerResources) ContainerOpts { return func(c *runtime.ContainerConfig) { if c.Linux == nil { c.Linux = &runtime.LinuxContainerConfig{} @@ -247,7 +247,7 @@ func WithResources(r *runtime.LinuxContainerResources) ContainerOpts { //nolint: } // Adds Windows container resource limits. -func WithWindowsResources(r *runtime.WindowsContainerResources) ContainerOpts { //nolint:unused +func WithWindowsResources(r *runtime.WindowsContainerResources) ContainerOpts { return func(c *runtime.ContainerConfig) { if c.Windows == nil { c.Windows = &runtime.WindowsContainerConfig{} @@ -265,7 +265,7 @@ func WithVolumeMount(hostPath, containerPath string) ContainerOpts { } } -func WithWindowsUsername(username string) ContainerOpts { //nolint:unused +func WithWindowsUsername(username string) ContainerOpts { return func(c *runtime.ContainerConfig) { if c.Windows == nil { c.Windows = &runtime.WindowsContainerConfig{} @@ -277,7 +277,7 @@ func WithWindowsUsername(username string) ContainerOpts { //nolint:unused } } -func WithWindowsHostProcessContainer() ContainerOpts { //nolint:unused +func WithWindowsHostProcessContainer() ContainerOpts { return func(c *runtime.ContainerConfig) { if c.Windows == nil { c.Windows = &runtime.WindowsContainerConfig{} @@ -322,7 +322,7 @@ func WithLogPath(path string) ContainerOpts { } // WithSupplementalGroups adds supplemental groups. -func WithSupplementalGroups(gids []int64) ContainerOpts { //nolint:unused +func WithSupplementalGroups(gids []int64) ContainerOpts { return func(c *runtime.ContainerConfig) { if c.Linux == nil { c.Linux = &runtime.LinuxContainerConfig{} @@ -335,7 +335,7 @@ func WithSupplementalGroups(gids []int64) ContainerOpts { //nolint:unused } // WithDevice adds a device mount. -func WithDevice(containerPath, hostPath, permissions string) ContainerOpts { //nolint:unused +func WithDevice(containerPath, hostPath, permissions string) ContainerOpts { return func(c *runtime.ContainerConfig) { c.Devices = append(c.Devices, &runtime.Device{ ContainerPath: containerPath, HostPath: hostPath, Permissions: permissions, @@ -558,7 +558,7 @@ func CRIConfig() (*criconfig.Config, error) { } // SandboxInfo gets sandbox info. -func SandboxInfo(id string) (*runtime.PodSandboxStatus, *server.SandboxInfo, error) { //nolint:unused +func SandboxInfo(id string) (*runtime.PodSandboxStatus, *server.SandboxInfo, error) { client, err := RawRuntimeClient() if err != nil { return nil, nil, fmt.Errorf("failed to get raw runtime client: %w", err) diff --git a/oci/spec_opts.go b/oci/spec_opts.go index 45c4fc180..f5e69860f 100644 --- a/oci/spec_opts.go +++ b/oci/spec_opts.go @@ -84,7 +84,7 @@ func setResources(s *Spec) { } } -//nolint:unused // not used on all platforms +//nolint:nolintlint,unused // not used on all platforms func setResourcesWindows(s *Spec) { if s.Windows != nil { if s.Windows.Resources == nil { @@ -93,7 +93,7 @@ func setResourcesWindows(s *Spec) { } } -//nolint:unused // not used on all platforms +//nolint:nolintlint,unused // not used on all platforms func setCPU(s *Spec) { setResources(s) if s.Linux != nil { @@ -103,7 +103,7 @@ func setCPU(s *Spec) { } } -//nolint:deadcode,unused // not used on all platforms +//nolint:deadcode,nolintlint,unused // not used on all platforms func setCPUWindows(s *Spec) { setResourcesWindows(s) if s.Windows != nil { diff --git a/oci/spec_opts_nonlinux.go b/oci/spec_opts_nonlinux.go index e376a86b3..30aedd6cd 100644 --- a/oci/spec_opts_nonlinux.go +++ b/oci/spec_opts_nonlinux.go @@ -28,22 +28,16 @@ import ( // WithAllCurrentCapabilities propagates the effective capabilities of the caller process to the container process. // The capability set may differ from WithAllKnownCapabilities when running in a container. -// -//nolint:unused var WithAllCurrentCapabilities = func(ctx context.Context, client Client, c *containers.Container, s *Spec) error { return WithCapabilities(nil)(ctx, client, c, s) } // WithAllKnownCapabilities sets all the known linux capabilities for the container process -// -//nolint:unused var WithAllKnownCapabilities = func(ctx context.Context, client Client, c *containers.Container, s *Spec) error { return WithCapabilities(nil)(ctx, client, c, s) } // WithBlockIO sets the container's blkio parameters -// -//nolint:unused func WithBlockIO(blockio interface{}) SpecOpts { return func(ctx context.Context, _ Client, c *containers.Container, s *Spec) error { return errors.New("blkio not supported") @@ -51,8 +45,6 @@ func WithBlockIO(blockio interface{}) SpecOpts { } // WithCPUShares sets the container's cpu shares -// -//nolint:unused func WithCPUShares(shares uint64) SpecOpts { return func(ctx context.Context, _ Client, c *containers.Container, s *Spec) error { return nil diff --git a/oci/utils_unix.go b/oci/utils_unix.go index 921e882b2..798680ab9 100644 --- a/oci/utils_unix.go +++ b/oci/utils_unix.go @@ -127,7 +127,7 @@ func getDevices(path, containerPath string) ([]specs.LinuxDevice, error) { // TODO consider adding these consts to the OCI runtime-spec. const ( - wildcardDevice = "a" //nolint:deadcode,unused,varcheck // currently unused, but should be included when upstreaming to OCI runtime-spec. + wildcardDevice = "a" //nolint:deadcode,nolintlint,unused,varcheck // currently unused, but should be included when upstreaming to OCI runtime-spec. blockDevice = "b" charDevice = "c" // or "u" fifoDevice = "p" @@ -148,7 +148,7 @@ func DeviceFromPath(path string) (*specs.LinuxDevice, error) { } var ( - devNumber = uint64(stat.Rdev) //nolint:unconvert // the type is 32bit on mips. + devNumber = uint64(stat.Rdev) //nolint:nolintlint,unconvert // the type is 32bit on mips. major = unix.Major(devNumber) minor = unix.Minor(devNumber) ) diff --git a/pkg/cri/sbserver/container_stats.go b/pkg/cri/sbserver/container_stats.go index a41e4da4f..cf870c77c 100644 --- a/pkg/cri/sbserver/container_stats.go +++ b/pkg/cri/sbserver/container_stats.go @@ -41,7 +41,7 @@ func (c *criService) ContainerStats(ctx context.Context, in *runtime.ContainerSt } cs, err := c.containerMetrics(cntr.Metadata, resp.Metrics[0]) - if err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return nil (stats unimplemented) + if err != nil { return nil, fmt.Errorf("failed to decode container metrics: %w", err) } return &runtime.ContainerStatsResponse{Stats: cs}, nil diff --git a/pkg/cri/sbserver/container_stats_list.go b/pkg/cri/sbserver/container_stats_list.go index aae5f849a..ec482340f 100644 --- a/pkg/cri/sbserver/container_stats_list.go +++ b/pkg/cri/sbserver/container_stats_list.go @@ -58,7 +58,7 @@ func (c *criService) toCRIContainerStats( containerStats := new(runtime.ListContainerStatsResponse) for _, cntr := range containers { cs, err := c.containerMetrics(cntr.Metadata, statsMap[cntr.ID]) - if err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return nil (metrics unimplemented) + if err != nil { return nil, fmt.Errorf("failed to decode container metrics for %q: %w", cntr.ID, err) } containerStats.Stats = append(containerStats.Stats, cs) diff --git a/pkg/cri/sbserver/sandbox_stats.go b/pkg/cri/sbserver/sandbox_stats.go index 3e805e09f..bf59846d1 100644 --- a/pkg/cri/sbserver/sandbox_stats.go +++ b/pkg/cri/sbserver/sandbox_stats.go @@ -34,12 +34,12 @@ func (c *criService) PodSandboxStats( } metrics, err := metricsForSandbox(sandbox) - if err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return nil (unimplemented metrics) + if err != nil { return nil, fmt.Errorf("failed getting metrics for sandbox %s: %w", r.GetPodSandboxId(), err) } podSandboxStats, err := c.podSandboxStats(ctx, sandbox, metrics) - if err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return nil (unimplemented metrics) + if err != nil { return nil, fmt.Errorf("failed to decode pod sandbox metrics %s: %w", r.GetPodSandboxId(), err) } diff --git a/pkg/cri/sbserver/sandbox_stats_list.go b/pkg/cri/sbserver/sandbox_stats_list.go index 69d4336a3..3cff21b65 100644 --- a/pkg/cri/sbserver/sandbox_stats_list.go +++ b/pkg/cri/sbserver/sandbox_stats_list.go @@ -34,12 +34,12 @@ func (c *criService) ListPodSandboxStats( podSandboxStats := new(runtime.ListPodSandboxStatsResponse) for _, sandbox := range sandboxes { metrics, err := metricsForSandbox(sandbox) - if err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return nil (unimplemented metrics) + if err != nil { return nil, fmt.Errorf("failed to obtain metrics for sandbox %q: %w", sandbox.ID, err) } sandboxStats, err := c.podSandboxStats(ctx, sandbox, metrics) - if err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return nil (unimplemented metrics) + if err != nil { return nil, fmt.Errorf("failed to decode sandbox container metrics for sandbox %q: %w", sandbox.ID, err) } podSandboxStats.Stats = append(podSandboxStats.Stats, sandboxStats) diff --git a/pkg/cri/sbserver/service.go b/pkg/cri/sbserver/service.go index 1d5b5d279..9479d4e9f 100644 --- a/pkg/cri/sbserver/service.go +++ b/pkg/cri/sbserver/service.go @@ -117,7 +117,7 @@ type criService struct { baseOCISpecs map[string]*oci.Spec // allCaps is the list of the capabilities. // When nil, parsed from CapEff of /proc/self/status. - allCaps []string //nolint:unused // Ignore on non-Linux + allCaps []string //nolint:nolintlint,unused // Ignore on non-Linux // unpackDuplicationSuppressor is used to make sure that there is only // one in-flight fetch request or unpack handler for a given descriptor's // or chain ID. diff --git a/pkg/cri/server/container_stats.go b/pkg/cri/server/container_stats.go index db62c342b..0ca66eef3 100644 --- a/pkg/cri/server/container_stats.go +++ b/pkg/cri/server/container_stats.go @@ -41,7 +41,7 @@ func (c *criService) ContainerStats(ctx context.Context, in *runtime.ContainerSt } cs, err := c.containerMetrics(cntr.Metadata, resp.Metrics[0]) - if err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return nil (stats unimplemented) + if err != nil { return nil, fmt.Errorf("failed to decode container metrics: %w", err) } return &runtime.ContainerStatsResponse{Stats: cs}, nil diff --git a/pkg/cri/server/container_stats_list.go b/pkg/cri/server/container_stats_list.go index 9459b64b1..cb61eecb3 100644 --- a/pkg/cri/server/container_stats_list.go +++ b/pkg/cri/server/container_stats_list.go @@ -61,7 +61,7 @@ func (c *criService) toCRIContainerStats( containerStats := new(runtime.ListContainerStatsResponse) for _, cntr := range containers { cs, err := c.containerMetrics(cntr.Metadata, statsMap[cntr.ID]) - if err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return nil (metrics unimplemented) + if err != nil { return nil, fmt.Errorf("failed to decode container metrics for %q: %w", cntr.ID, err) } diff --git a/pkg/cri/server/sandbox_stats.go b/pkg/cri/server/sandbox_stats.go index b74dd0ff5..feb19120c 100644 --- a/pkg/cri/server/sandbox_stats.go +++ b/pkg/cri/server/sandbox_stats.go @@ -34,12 +34,12 @@ func (c *criService) PodSandboxStats( } metrics, err := metricsForSandbox(sandbox) - if err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return nil (unimplemented metrics) + if err != nil { return nil, fmt.Errorf("failed getting metrics for sandbox %s: %w", r.GetPodSandboxId(), err) } podSandboxStats, err := c.podSandboxStats(ctx, sandbox, metrics) - if err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return nil (unimplemented metrics) + if err != nil { return nil, fmt.Errorf("failed to decode pod sandbox metrics %s: %w", r.GetPodSandboxId(), err) } diff --git a/pkg/cri/server/sandbox_stats_list.go b/pkg/cri/server/sandbox_stats_list.go index 1cd4f2db8..c989bbfa9 100644 --- a/pkg/cri/server/sandbox_stats_list.go +++ b/pkg/cri/server/sandbox_stats_list.go @@ -34,12 +34,12 @@ func (c *criService) ListPodSandboxStats( podSandboxStats := new(runtime.ListPodSandboxStatsResponse) for _, sandbox := range sandboxes { metrics, err := metricsForSandbox(sandbox) - if err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return nil (unimplemented metrics) + if err != nil { return nil, fmt.Errorf("failed to obtain metrics for sandbox %q: %w", sandbox.ID, err) } sandboxStats, err := c.podSandboxStats(ctx, sandbox, metrics) - if err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return nil (unimplemented metrics) + if err != nil { return nil, fmt.Errorf("failed to decode sandbox container metrics for sandbox %q: %w", sandbox.ID, err) } podSandboxStats.Stats = append(podSandboxStats.Stats, sandboxStats) diff --git a/pkg/cri/server/service.go b/pkg/cri/server/service.go index 0e411fa44..04ec294d3 100644 --- a/pkg/cri/server/service.go +++ b/pkg/cri/server/service.go @@ -113,7 +113,7 @@ type criService struct { baseOCISpecs map[string]*oci.Spec // allCaps is the list of the capabilities. // When nil, parsed from CapEff of /proc/self/status. - allCaps []string //nolint:unused // Ignore on non-Linux + allCaps []string //nolint:nolintlint,unused // Ignore on non-Linux // unpackDuplicationSuppressor is used to make sure that there is only // one in-flight fetch request or unpack handler for a given descriptor's // or chain ID. diff --git a/pkg/cri/store/container/metadata.go b/pkg/cri/store/container/metadata.go index f21914a20..698e495ae 100644 --- a/pkg/cri/store/container/metadata.go +++ b/pkg/cri/store/container/metadata.go @@ -28,10 +28,9 @@ import ( // 2) Metadata is checkpointed as containerd container label. // metadataVersion is current version of container metadata. -const metadataVersion = "v1" // nolint +const metadataVersion = "v1" // versionedMetadata is the internal versioned container metadata. -// nolint type versionedMetadata struct { // Version indicates the version of the versioned container metadata. Version string diff --git a/pkg/cri/store/container/status.go b/pkg/cri/store/container/status.go index 1cf9a204e..b50dc5c54 100644 --- a/pkg/cri/store/container/status.go +++ b/pkg/cri/store/container/status.go @@ -61,10 +61,9 @@ import ( // DELETED // statusVersion is current version of container status. -const statusVersion = "v1" // nolint +const statusVersion = "v1" // versionedStatus is the internal used versioned container status. -// nolint type versionedStatus struct { // Version indicates the version of the versioned container status. Version string diff --git a/pkg/cri/store/sandbox/metadata.go b/pkg/cri/store/sandbox/metadata.go index 80e39c68c..20fe2f1d1 100644 --- a/pkg/cri/store/sandbox/metadata.go +++ b/pkg/cri/store/sandbox/metadata.go @@ -29,10 +29,9 @@ import ( // 2) Metadata is checkpointed as containerd container label. // metadataVersion is current version of sandbox metadata. -const metadataVersion = "v1" // nolint +const metadataVersion = "v1" // versionedMetadata is the internal versioned sandbox metadata. -// nolint type versionedMetadata struct { // Version indicates the version of the versioned sandbox metadata. Version string diff --git a/pkg/progress/escape.go b/pkg/progress/escape.go index e648fdda3..6455c305e 100644 --- a/pkg/progress/escape.go +++ b/pkg/progress/escape.go @@ -19,6 +19,6 @@ package progress const ( escape = "\x1b" reset = escape + "[0m" - red = escape + "[31m" //nolint:deadcode,unused,varcheck + red = escape + "[31m" //nolint:deadcode,nolintlint,unused,varcheck green = escape + "[32m" ) From 8b5df7d347b689a093ecc4bbb05a75d3c091fbeb Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 31 Aug 2022 09:44:08 +0200 Subject: [PATCH 6/6] update golangci-lint to v1.49.0 Also remove "nolint" comments for deadcode, which is deprecated, and removed from the defaults. Signed-off-by: Sebastiaan van Stijn --- .github/workflows/ci.yml | 2 +- oci/spec_opts.go | 2 +- oci/utils_unix.go | 2 +- pkg/progress/escape.go | 2 +- script/setup/install-dev-tools | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d1fc583cb..a40497118 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,7 +35,7 @@ jobs: - uses: actions/checkout@v2 - uses: golangci/golangci-lint-action@v3 with: - version: v1.48.0 + version: v1.49.0 skip-cache: true args: --timeout=8m diff --git a/oci/spec_opts.go b/oci/spec_opts.go index f5e69860f..e13b9a498 100644 --- a/oci/spec_opts.go +++ b/oci/spec_opts.go @@ -103,7 +103,7 @@ func setCPU(s *Spec) { } } -//nolint:deadcode,nolintlint,unused // not used on all platforms +//nolint:nolintlint,unused // not used on all platforms func setCPUWindows(s *Spec) { setResourcesWindows(s) if s.Windows != nil { diff --git a/oci/utils_unix.go b/oci/utils_unix.go index 798680ab9..306f09814 100644 --- a/oci/utils_unix.go +++ b/oci/utils_unix.go @@ -127,7 +127,7 @@ func getDevices(path, containerPath string) ([]specs.LinuxDevice, error) { // TODO consider adding these consts to the OCI runtime-spec. const ( - wildcardDevice = "a" //nolint:deadcode,nolintlint,unused,varcheck // currently unused, but should be included when upstreaming to OCI runtime-spec. + wildcardDevice = "a" //nolint:nolintlint,unused,varcheck // currently unused, but should be included when upstreaming to OCI runtime-spec. blockDevice = "b" charDevice = "c" // or "u" fifoDevice = "p" diff --git a/pkg/progress/escape.go b/pkg/progress/escape.go index 6455c305e..394686f46 100644 --- a/pkg/progress/escape.go +++ b/pkg/progress/escape.go @@ -19,6 +19,6 @@ package progress const ( escape = "\x1b" reset = escape + "[0m" - red = escape + "[31m" //nolint:deadcode,nolintlint,unused,varcheck + red = escape + "[31m" //nolint:nolintlint,unused,varcheck green = escape + "[32m" ) diff --git a/script/setup/install-dev-tools b/script/setup/install-dev-tools index 078974571..c1afbe662 100755 --- a/script/setup/install-dev-tools +++ b/script/setup/install-dev-tools @@ -24,7 +24,7 @@ set -eu -o pipefail go install github.com/containerd/protobuild@v0.2.0 go install github.com/containerd/protobuild/cmd/go-fix-acronym@v0.2.0 go install github.com/cpuguy83/go-md2man/v2@v2.0.1 -go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.48.0 +go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.49.0 go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.28 go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.2 go install github.com/containerd/ttrpc/cmd/protoc-gen-go-ttrpc@944ef4a40df3446714a823207972b7d9858ffac5