chore: update golangci-lint and fix all issues (#3575)

* chore: Update golangci-lint

Signed-off-by: Lars Francke <git@lars-francke.de>

* chore: fix all golangci-lint issues

- Remove deprecated `// +build` tags
- Fix godoclint, modernize, wsl_v5, govet, lll, gci, noctx issues
- Update linter configuration
- Modernize code to use Go 1.22+ features (for range N, slices.Contains, etc.)
- Update make check lint the privileged tests

Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>

---------

Signed-off-by: Lars Francke <git@lars-francke.de>
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
Co-authored-by: Lars Francke <git@lars-francke.de>

Author: andaaron

.github/workflows/golangci-lint.yaml

@@ -24,14 +24,14 @@ jobs:
         uses: golangci/golangci-lint-action@v9
         with:
           # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
-          version: v2.1.0
+          version: v2.6.2
 
           # Optional: working directory, useful for monorepos
           # working-directory: somedir
 
           # Optional: golangci-lint command line arguments.
           # args: --issues-exit-code=0
-          args: --config ./golangcilint.yaml --build-tags debug,needprivileges,sync,scrub,search,userprefs,metrics,lint,mgmt,imagetrust ./cmd/... ./pkg/...
+          args: --build-tags debug,needprivileges,sync,scrub,search,userprefs,metrics,lint,mgmt,imagetrust ./cmd/... ./pkg/...
 
           # Optional: show only new issues if it's a pull request. The default value is `false`.
           # only-new-issues: true

golangcilint.yaml .golangci.yamlgolangcilint.yaml renamed to .golangci.yaml

@@ -19,6 +19,7 @@ linters:
     - maintidx
     - mnd
     - musttag
+    - noinlineerr
     - paralleltest
     - promlinter
     - protogetter
@@ -29,6 +30,7 @@ linters:
     - staticcheck
     - usetesting
     - wrapcheck
+    - wsl
   settings:
     cyclop:
       max-complexity: 40
@@ -68,9 +70,20 @@ linters:
         - l *ldap.Conn
         - w http.ResponseWriter
         - r *http.Request
-    wsl:
-      allow-assign-and-anything: true
-      force-err-cuddling: true
+    wsl_v5:
+      allow-first-in-block: true
+      allow-whole-block: false
+      branch-max-lines: 2
+      enable:
+        - err
+      disable:
+        - assign
+        - defer
+        - if
+        - send
+        - expr
+        - select
+        - switch
   exclusions:
     generated: lax
     presets:

Makefile

@@ -11,7 +11,7 @@ TOOLSDIR := $(shell pwd)/hack/tools
 PATH := bin:$(TOOLSDIR)/bin:$(PATH)
 STACKER := $(shell which stacker)
 GOLINTER := $(TOOLSDIR)/bin/golangci-lint
-GOLINTER_VERSION := v2.1.0
+GOLINTER_VERSION := v2.6.2
 NOTATION := $(TOOLSDIR)/bin/notation
 NOTATION_VERSION := 1.3.2
 COSIGN := $(TOOLSDIR)/bin/cosign
@@ -331,24 +331,29 @@ covhtml:
 	gocovmerge coverage*.txt > coverage.txt
 	go tool cover -html=coverage.txt -o coverage.html
 
-$(GOLINTER):
+$(GOLINTER): $(TOOLSDIR)/.golangci-lint-$(GOLINTER_VERSION)
+
+$(TOOLSDIR)/.golangci-lint-$(GOLINTER_VERSION):
 	mkdir -p $(TOOLSDIR)/bin
+	rm -f $(TOOLSDIR)/.golangci-lint-*
 	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(TOOLSDIR)/bin $(GOLINTER_VERSION)
 	$(GOLINTER) version
+	touch $@
 
 .PHONY: check-logs
 check-logs:
 	@./scripts/check_logs.sh
 
 .PHONY: check
 check: $(if $(findstring ui,$(BUILD_LABELS)), ui)
-check: ./golangcilint.yaml $(GOLINTER)
+check: ./.golangci.yaml $(GOLINTER)
 	mkdir -p pkg/extensions/build; touch pkg/extensions/build/.empty
-	$(GOLINTER) --config ./golangcilint.yaml run --output.text.colors --build-tags ./...
-	$(GOLINTER) --config ./golangcilint.yaml run --output.text.colors --build-tags $(BUILD_LABELS)  ./...
-	$(GOLINTER) --config ./golangcilint.yaml run --output.text.colors --build-tags debug  ./pkg/debug/swagger/ ./pkg/debug/gqlplayground
-	$(GOLINTER) --config ./golangcilint.yaml run --output.text.colors --build-tags dev ./pkg/test/inject/
-	$(GOLINTER) --config ./golangcilint.yaml run --output.text.colors --build-tags stress ./pkg/cli/server/
+	$(GOLINTER) run --output.text.colors --build-tags ./...
+	$(GOLINTER) run --output.text.colors --build-tags $(BUILD_LABELS)  ./...
+	$(GOLINTER) run --output.text.colors --build-tags debug  ./pkg/debug/swagger/ ./pkg/debug/gqlplayground
+	$(GOLINTER) run --output.text.colors --build-tags dev ./pkg/test/inject/
+	$(GOLINTER) run --output.text.colors --build-tags stress ./pkg/cli/server/
+	$(GOLINTER) run --output.text.colors --build-tags needprivileges,$(BUILD_LABELS) ./pkg/cli/client/ ./pkg/storage/local/ ./pkg/api/config/
 	rm pkg/extensions/build/.empty
 
 .PHONY: swagger

cmd/zb/helper.go

@@ -25,7 +25,7 @@ import (
 	"zotregistry.dev/zot/v2/pkg/common"
 )
 
-func makeHTTPGetRequest(url string, resultPtr interface{}, client *resty.Client) (http.Header, error) {
+func makeHTTPGetRequest(url string, resultPtr any, client *resty.Client) (http.Header, error) {
 	resp, err := client.R().Get(url)
 	if err != nil {
 		return http.Header{}, err

cmd/zb/main.go

@@ -10,7 +10,7 @@ import (
 	"zotregistry.dev/zot/v2/pkg/log"
 )
 
-// "zb" - performance benchmark and stress.
+// NewPerfRootCmd creates the root command for "zb" - performance benchmark and stress.
 func NewPerfRootCmd() *cobra.Command {
 	showVersion := false
 

cmd/zb/perf.go

@@ -196,10 +196,10 @@ func updateStats(summary *statsSummary, record statsRecord) {
 }
 
 type cicdTestSummary struct {
-	Name  string      `json:"name"`
-	Unit  string      `json:"unit"`
-	Value interface{} `json:"value"`
-	Range string      `json:"range,omitempty"`
+	Name  string `json:"name"`
+	Unit  string `json:"unit"`
+	Value any    `json:"value"`
+	Range string `json:"range,omitempty"`
 }
 
 type manifestStruct struct {
@@ -299,15 +299,15 @@ func GetCatalog(
 
 	statusRequests = sync.Map{}
 
-	for count := 0; count < requests; count++ {
+	for range requests {
 		// Push random blob
 		_, repos, err = pushMonolithImage(workdir, url, repo, repos, config, client)
 		if err != nil {
 			return err
 		}
 	}
 
-	for count := 0; count < requests; count++ {
+	for range requests {
 		func() {
 			start := time.Now()
 
@@ -376,7 +376,7 @@ func PushMonolithStreamed(
 		statusRequests = sync.Map{}
 	}
 
-	for count := 0; count < requests; count++ {
+	for count := range requests {
 		repos = pushMonolithAndCollect(workdir, url, trepo, count,
 			repos, config, client, statsCh)
 	}
@@ -406,7 +406,7 @@ func PushChunkStreamed(
 		statusRequests = sync.Map{}
 	}
 
-	for count := 0; count < requests; count++ {
+	for count := range requests {
 		repos = pushChunkAndCollect(workdir, url, trepo, count,
 			repos, config, client, statsCh)
 	}
@@ -493,7 +493,7 @@ func Pull(
 	}
 
 	// download image
-	for count := 0; count < requests; count++ {
+	for range requests {
 		repos = pullAndCollect(url, repos, manifestItem, config, client, statsCh)
 	}
 
@@ -530,7 +530,7 @@ func MixedPullAndPush(
 		manifestHash: manifestHash,
 	}
 
-	for count := 0; count < requests; count++ {
+	for count := range requests {
 		idx := flipFunc(config.probabilityRange)
 
 		readTestIdx := 0
@@ -667,6 +667,16 @@ func Perf(
 	outFmt string, srcIPs string, srcCIDR string, skipCleanup bool,
 ) {
 	json := jsoniter.ConfigCompatibleWithStandardLibrary
+
+	// fatalWithCleanup calls teardown then logs fatal, ensuring cleanup happens before exit.
+	// Uses sync.Once to ensure teardown is only called once, even from goroutines.
+	var teardownOnce sync.Once
+	fatalWithCleanup := func(err error) {
+		teardownOnce.Do(func() {
+			teardown(workdir)
+		})
+		log.Fatal(err)
+	}
 	// logging
 	log.SetFlags(0)
 	log.SetOutput(tabwriter.NewWriter(os.Stdout, 0, 0, 1, ' ', tabwriter.TabIndent))
@@ -694,7 +704,6 @@ func Perf(
 	log.Printf("Preparing test data ...\n")
 
 	setup(workdir)
-	defer teardown(workdir)
 
 	log.Printf("Starting tests ...\n")
 
@@ -709,7 +718,7 @@ func Perf(
 	} else if len(srcCIDR) > 0 {
 		ips, err = getIPsFromCIDR(srcCIDR, maxSourceIPs)
 		if err != nil {
-			log.Fatal(err) //nolint: gocritic
+			fatalWithCleanup(err)
 		}
 	}
 
@@ -722,7 +731,7 @@ func Perf(
 
 		start := time.Now()
 
-		for c := 0; c < concurrency; c++ {
+		for range concurrency {
 			// parallelize with clients
 			wg.Add(1)
 
@@ -731,12 +740,12 @@ func Perf(
 
 				httpClient, err := getRandomClientIPs(auth, url, ips)
 				if err != nil {
-					log.Fatal(err)
+					fatalWithCleanup(err)
 				}
 
 				err = tconfig.tfunc(workdir, url, repo, requests/concurrency, tconfig, statsCh, httpClient, skipCleanup)
 				if err != nil {
-					log.Fatal(err)
+					fatalWithCleanup(err)
 				}
 			}()
 		}
@@ -754,7 +763,7 @@ func Perf(
 			summary.mixedType = true
 		}
 
-		for count := 0; count < requests; count++ {
+		for range requests {
 			record := <-statsCh
 			updateStats(&summary, record)
 		}
@@ -771,14 +780,19 @@ func Perf(
 	if outFmt == cicdFmt {
 		jsonOut, err := json.Marshal(cicdSummary)
 		if err != nil {
-			log.Fatal(err) // file closed on exit
+			fatalWithCleanup(err)
 		}
 
 		if err := os.WriteFile(outFmt+".json", jsonOut, defaultFilePerms); err != nil {
-			log.Fatal(err)
+			fatalWithCleanup(err)
 		}
 	}
 
+	// Cleanup before exit (sync.Once ensures it only runs once, even if fatalWithCleanup was called)
+	teardownOnce.Do(func() {
+		teardown(workdir)
+	})
+
 	if zbError {
 		os.Exit(1)
 	}

cmd/zli/main.go

@@ -1,5 +1,4 @@
 //go:build search
-// +build search
 
 package main
 

cmd/zli/main_test.go

@@ -1,5 +1,4 @@
 //go:build search
-// +build search
 
 package main_test
 

cmd/zxp/main.go

@@ -1,5 +1,4 @@
 //go:build !metrics
-// +build !metrics
 
 package main
 

pkg/api/authn.go

@@ -13,6 +13,7 @@ import (
 	"net"
 	"net/http"
 	"os"
+	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -657,7 +658,7 @@ func getRelyingPartyArgs(cfg *config.Config, provider string, hashKey, encryptKe
 
 	scopes := providerConfig.Scopes
 	// openid scope must be the first one in list
-	if !zcommon.Contains(scopes, oidc.ScopeOpenID) && config.IsOpenIDSupported(provider) {
+	if !slices.Contains(scopes, oidc.ScopeOpenID) && config.IsOpenIDSupported(provider) {
 		scopes = append([]string{oidc.ScopeOpenID}, scopes...)
 	}