Switch to golangci-lint v2

.github/workflows/ci.yml

@@ -34,9 +34,9 @@ jobs:
           cache: false
 
       - name: Run golangci-lint
-        uses: golangci/golangci-lint-action@v6
+        uses: golangci/golangci-lint-action@v8
         with:
-          version: v1.64
+          version: v2.1
           args: >-
             --verbose
             --max-issues-per-linter=0

.golangci.yml

@@ -1,173 +1,165 @@
+version: "2"
+
 run:
-  timeout: 8m
-  tests: true
   build-tags:
     - admin
     - functional
     - integration
+  tests: true
 
 linters:
   enable:
-    # defaults:
-    # - errcheck
-    # - gosimple
-    # - govet
-    # - ineffassign
-    # - staticcheck
-    # - typecheck
-    # - unused
-
     - errorlint # error wrapping (eg, not using `errors.Is`, using `%s` instead of `%w` in `fmt.Errorf`)
-    - gofmt # whether code was gofmt-ed
-    - govet # enabled by default, but just to be sure
     - nolintlint # ill-formed or insufficient nolint directives
-    - stylecheck # golint replacement
     - thelper #  test helpers without t.Helper()
 
-linters-settings:
-  govet:
-    enable-all: true
-    disable:
-      # struct order is often for Win32 compat
-      # also, ignore pointer bytes/GC issues for now until performance becomes an issue
-      - fieldalignment
-
-  stylecheck:
-    # https://staticcheck.io/docs/checks
-    checks: ["all"]
-
-issues:
-  exclude-dirs:
-    # paths are relative to module root
-    - cri-containerd/test-images
-  exclude-rules:
-    # err is very often shadowed in nested scopes
-    - linters:
-        - govet
-      text: '^shadow: declaration of "err" shadows declaration'
-
-    # path is relative to module root, which is ./test/
-    - path: cri-containerd
-      linters:
-        - stylecheck
-      text: "^ST1003: should not use underscores in package names$"
-      source: "^package cri_containerd$"
-
-    # don't bother with propper error wrapping in test code
-    - path: cri-containerd
-      linters:
-        - errorlint
-      text: "non-wrapping format verb for fmt.Errorf"
-
-    # This repo has a LOT of generated schema files, operating system bindings, and other
-    # things that ST1003 from stylecheck won't like (screaming case Windows api constants for example).
-    # There's also some structs that we *could* change the initialisms to be Go friendly
-    # (Id -> ID) but they're exported and it would be a breaking change.
-    # This makes it so that most new code, code that isn't supposed to be a pretty faithful
-    # mapping to an OS call/constants, or non-generated code still checks if we're following idioms,
-    # while ignoring the things that are just noise or would be more of a hassle than it'd be worth to change.
-    - path: layer.go
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    - path: hcsshim.go
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    - path: cmd\\ncproxy\\nodenetsvc\\
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    - path: cmd\\ncproxy_mock\\
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    - path: internal\\hcs\\schema2\\
-      linters:
-        - stylecheck
-        - gofmt
-
-    - path: internal\\wclayer\\
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    - path: hcn\\
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    - path: internal\\hcs\\schema1\\
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    - path: internal\\hns\\
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    - path: ext4\\internal\\compactext4\\
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    - path: ext4\\internal\\format\\
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    - path: internal\\guestrequest\\
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    - path: internal\\guest\\prot\\
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    - path: internal\\windevice\\
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    - path: internal\\winapi\\
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    - path: internal\\vmcompute\\
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    - path: internal\\regstate\\
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    - path: internal\\hcserror\\
-      linters:
-        - stylecheck
-      text: "ST1003:"
-
-    # v0 APIs are deprecated, but still retained for backwards compatability
-    - path: cmd\\ncproxy\\
-      linters:
-        - staticcheck
-      text: "^SA1019: .*(ncproxygrpc|nodenetsvc)[/]?v0"
-
-    - path: internal\\tools\\networkagent
-      linters:
-        - staticcheck
-      text: "^SA1019: .*nodenetsvc[/]?v0"
-
-    - path: internal\\vhdx\\info
-      linters:
-        - stylecheck
-      text: "ST1003:"
+  settings:
+    govet:
+      disable:
+        # struct order is often for Win32 compat
+        # also, ignore pointer bytes/GC issues for now until performance becomes an issue
+        - fieldalignment
+      enable-all: true
+
+    staticcheck:
+      # https://staticcheck.io/docs/checks
+      checks:
+        - all
+
+  exclusions:
+    generated: lax
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
+    rules:
+      # err is very often shadowed in nested scopes
+      - linters:
+          - govet
+        text: '^shadow: declaration of "err" shadows declaration'
+
+      # path is relative to module root, which is ./test/
+      - linters:
+          - staticcheck
+        path: cri-containerd
+        text: "^ST1003: should not use underscores in package names$"
+        source: ^package cri_containerd$
+
+      # don't bother with propper error wrapping in test code
+      - linters:
+          - errorlint
+        path: cri-containerd
+        text: non-wrapping format verb for fmt.Errorf
+
+      # static check doesn't recognize `t.Fatal[f]?` when checking for potential `nil` pointer dereference
+      - linters:
+          - staticcheck
+        path: ".*_test.go$"
+        text: "^SA5011"
+
+      # This repo has a LOT of generated schema files, operating system bindings, and other
+      # things that ST1003 from stylecheck won't like (screaming case Windows api constants for example).
+      # There's also some structs that we *could* change the initialisms to be Go friendly
+      # (Id -> ID) but they're exported and it would be a breaking change.
+      # This makes it so that most new code, code that isn't supposed to be a pretty faithful
+      # mapping to an OS call/constants, or non-generated code still checks if we're following idioms,
+      # while ignoring the things that are just noise or would be more of a hassle than it'd be worth to change.
+      - linters:
+          - staticcheck
+        path: layer.go
+        text: "ST1003:"
+      - linters:
+          - staticcheck
+        path: hcsshim.go
+        text: "ST1003:"
+      - linters:
+          - staticcheck
+        path: cmd/ncproxy/nodenetsvc/
+        text: "ST1003:"
+      - linters:
+          - staticcheck
+        path: cmd/ncproxy_mock/
+        text: "ST1003:"
+      - linters:
+          - staticcheck
+        path: internal/hcs/schema2/
+      - linters:
+          - staticcheck
+        path: internal/wclayer/
+        text: "ST1003:"
+      - linters:
+          - staticcheck
+        path: hcn/
+        text: "ST1003:"
+      - linters:
+          - staticcheck
+        path: internal/hcs/schema1/
+        text: "ST1003:"
+      - linters:
+          - staticcheck
+        path: internal/hns/
+        text: "ST1003:"
+      - linters:
+          - staticcheck
+        path: ext4/internal/compactext4/
+        text: "ST1003:"
+      - linters:
+          - staticcheck
+        path: ext4/internal/format/
+        text: "ST1003:"
+      - linters:
+          - staticcheck
+        path: internal/guestrequest/
+        text: "ST1003:"
+      - linters:
+          - staticcheck
+        path: internal/guest/prot/
+        text: "ST1003:"
+      - linters:
+          - staticcheck
+        path: internal/windevice/
+        text: "ST1003:"
+      - linters:
+          - staticcheck
+        path: internal/winapi/
+        text: "ST1003:"
+      - linters:
+          - staticcheck
+        path: internal/vmcompute/
+        text: "ST1003:"
+      - linters:
+          - staticcheck
+        path: internal/regstate/
+        text: "ST1003:"
+      - linters:
+          - staticcheck
+        path: internal/hcserror/
+        text: "ST1003:"
+
+      # v0 APIs are deprecated, but still retained for backwards compatability
+      - linters:
+          - staticcheck
+        path: cmd/ncproxy/
+        text: "^SA1019: .*(ncproxygrpc|nodenetsvc)[/]?v0"
+      - linters:
+          - staticcheck
+        path: internal/tools/networkagent
+        text: "^SA1019: .*nodenetsvc[/]?v0"
+      - linters:
+          - staticcheck
+        path: internal/vhdx/info
+        text: "ST1003:"
+
+    paths:
+      # paths are relative to module root
+      - cri-containerd/test-images
+
+formatters:
+  enable:
+    - gofmt
+  exclusions:
+    generated: lax
+    paths:
+      - cri-containerd/test-images
+      - internal/hcs/schema2/

hcn/hcnerrors.go

@@ -23,11 +23,7 @@ var (
 )
 
 func checkForErrors(methodName string, hr error, resultBuffer *uint16) error {
-	errorFound := false
-
-	if hr != nil {
-		errorFound = true
-	}
+	errorFound := hr != nil
 
 	result := ""
 	if resultBuffer != nil {

internal/cmd/io_npipe.go

@@ -114,7 +114,7 @@ func (nprw *nPipeRetryWriter) Write(p []byte) (n int, err error) {
 				}).Error("Named pipe disconnected, retrying dial")
 
 				// Close the old conn first.
-				nprw.Conn.Close()
+				nprw.Close()
 				newConn, retryErr := nprw.retryDialPipe()
 				if retryErr == nil {
 					log.G(nprw.ctx).WithField("address", nprw.pipePath).Info("Succeeded in reconnecting to named pipe")

internal/exec/exec.go

@@ -113,7 +113,7 @@ func (e *Exec) Start() error {
 	pi := new(windows.ProcessInformation)
 
 	// Need EXTENDED_STARTUPINFO_PRESENT as we're making use of the attribute list field.
-	flags := uint32(windows.CREATE_UNICODE_ENVIRONMENT) | windows.EXTENDED_STARTUPINFO_PRESENT | e.execConfig.processFlags
+	flags := uint32(windows.CREATE_UNICODE_ENVIRONMENT) | windows.EXTENDED_STARTUPINFO_PRESENT | e.processFlags
 
 	// Allocate an attribute list that's large enough to do the operations we care about
 	// 1. Assigning to a job object at creation time
@@ -177,9 +177,9 @@ func (e *Exec) Start() error {
 
 	siEx.ProcThreadAttributeList = attrList.List()
 	siEx.Cb = uint32(unsafe.Sizeof(*siEx))
-	if e.execConfig.token != 0 {
+	if e.token != 0 {
 		err = windows.CreateProcessAsUser(
-			e.execConfig.token,
+			e.token,
 			argv0p,
 			argvp,
 			pSec,

internal/guest/network/netns.go

@@ -247,7 +247,7 @@ func configureLink(ctx context.Context,
 
 		// dst will be nil when setting default gateways
 		var dst *net.IPNet
-		if !(r.DestinationPrefix == ipv4GwDestination || r.DestinationPrefix == ipv6GwDestination) {
+		if r.DestinationPrefix != ipv4GwDestination && r.DestinationPrefix != ipv6GwDestination {
 			dstIP, dstAddr, err := net.ParseCIDR(r.DestinationPrefix)
 			if err != nil {
 				return fmt.Errorf("parsing route dst address %s failed: %w", r.DestinationPrefix, err)