upadte golangci-lint rules and fix lints

Signed-off-by: cpanato <[email protected]>

Author: cpanato

.github/workflows/lint.yml

@@ -23,5 +23,4 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@a4f60bb28d35aeee14e6880718e0c85ff1882e64 # v6.0.1
         with:
-          timeout: 10m
-          version: v1.57
+          version: v1.58

.golangci.yml

@@ -1,41 +1,111 @@
+---
 linters:
   disable-all: true
   enable:
-  - asciicheck
-  - bodyclose
-  - depguard
-  - dogsled
-  - errcheck
-  - exportloopref
-  - goconst
-  - gocritic
-  - gocyclo
-  - godot
-  - gofmt
-  - goimports
-  - goprintffuncname
-  - gosec
-  - gosimple
-  - govet
-  - importas
-  - ineffassign
-  - misspell
-  - nakedret
-  - nilerr
-  - noctx
-  - nolintlint
-  - prealloc
-  - predeclared
-  - revive
-  - rowserrcheck
-  - staticcheck
-  - stylecheck
-  - thelper
-  - typecheck
-  - unconvert
-  - unparam
-  - unused
-  - whitespace
+    - asasalint
+    - asciicheck
+    - bidichk
+    - bodyclose
+    - canonicalheader
+    # - copyloopvar // only on go1.22
+    - decorder
+    - dogsled
+    - durationcheck
+    - errcheck
+    - errchkjson
+    - errname
+    - exportloopref
+    - fatcontext
+    - ginkgolinter
+    - gocheckcompilerdirectives
+    - gochecksumtype
+    - goconst
+    - gocritic
+    - gocyclo
+    - godox
+    - gofmt
+    - gofumpt
+    - goheader
+    - goimports
+    - gomodguard
+    - goprintffuncname
+    - gosec
+    - gosimple
+    - gosmopolitan
+    - govet
+    - grouper
+    - importas
+    - ineffassign
+    # - intrange  // only on go1.22
+    - loggercheck
+    - makezero
+    - mirror
+    - misspell
+    - musttag
+    - nakedret
+    - nolintlint
+    - nosprintfhostport
+    - perfsprint
+    - prealloc
+    - predeclared
+    - promlinter
+    - protogetter
+    - reassign
+    - revive
+    - rowserrcheck
+    - sloglint
+    - spancheck
+    - sqlclosecheck
+    - staticcheck
+    - stylecheck
+    - tagalign
+    - tenv
+    - testableexamples
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - usestdlibvars
+    - wastedassign
+    - whitespace
+    - zerologlint
+    # - containedctx
+    # - cyclop
+    # - depguard
+    # - dupword
+    # - err113
+    # - errorlint
+    # - exhaustive
+    # - exhaustruct
+    # - forbidigo
+    # - forcetypeassert
+    # - funlen
+    # - gci
+    # - gochecknoglobals
+    # - gochecknoinits
+    # - gocognit
+    # - godot
+    # - inamedparam
+    # - interfacebloat
+    # - ireturn
+    # - lll
+    # - maintidx
+    # - mnd
+    # - nestif
+    # - nilerr
+    # - nilnil
+    # - nlreturn
+    # - noctx
+    # - nonamedreturns
+    # - paralleltest
+    # - tagliatelle
+    # - testifylint
+    # - testpackage
+    # - thelper
+    # - tparallel
+    # - varnamelen
+    # - wrapcheck
+    # - wsl
 
 linters-settings:
   importas:
@@ -55,84 +125,72 @@ linters-settings:
       # Controller Runtime
       - pkg: sigs.k8s.io/controller-runtime
         alias: ctrl
-  staticcheck:
-    go: "1.18"
-  stylecheck:
-    go: "1.18"
-  depguard:
-    rules:
-      main:
-        deny:
-          - pkg: "io/ioutil"
-            desc: "ioutil is deprecated starting with Go 1.16"
-
 
 issues:
   max-same-issues: 0
   max-issues-per-linter: 0
   # We are disabling default golangci exclusions because we want to help reviewers to focus on reviewing the most relevant
   # changes in PRs and avoid nitpicking.
   exclude-use-default: false
+  exclude-files:
+    - "zz_generated.*\\.go$"
+    - ".*conversion.*\\.go$"
+  exclude-dirs:
+    - mock*
   # List of regexps of issue texts to exclude, empty list by default.
   exclude:
-  # The following are being worked on to remove their exclusion. This list should be reduced or go away all together over time.
-  # If it is decided they will not be addressed they should be moved above this comment.
-  - Subprocess launch(ed with variable|ing should be audited)
-  - (Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)
-  - (G104|G307)
+    # The following are being worked on to remove their exclusion. This list should be reduced or go away all together over time.
+    # If it is decided they will not be addressed they should be moved above this comment.
+    - Subprocess launch(ed with variable|ing should be audited)
+    - (Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)
+    - (G104|G307)
   exclude-rules:
-  - linters:
-    - gosec
-    text: "G108: Profiling endpoint is automatically exposed on /debug/pprof"
-  - linters:
-    - revive
-    text: "exported: exported method .*\\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported"
-  - linters:
-    - errcheck
-    text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
-  # With Go 1.16, the new embed directive can be used with an un-named import,
-  # revive (previously, golint) only allows these to be imported in a main.go, which wouldn't work for us.
-  # This directive allows the embed package to be imported with an underscore everywhere.
-  - linters:
-    - revive
-    source: _ "embed"
-  # Exclude some packages or code to require comments, for example test code, or fake clients.
-  - linters:
-    - revive
-    text: exported (method|function|type|const) (.+) should have comment or be unexported
-    source: (func|type).*Fake.*
-  - linters:
-    - revive
-    text: exported (method|function|type|const) (.+) should have comment or be unexported
-    path: fake_\.go
-  - linters:
-    - revive
-    text: exported (method|function|type|const) (.+) should have comment or be unexported
-    path: "(framework|e2e|infrastructure/docker)/.*.go"
-  # Disable unparam "always receives" which might not be really
-  # useful when building libraries.
-  - linters:
-    - unparam
-    text: always receives
-  # Dot imports for gomega or ginkgo are allowed
-  # within test files.
-  - path: _test\.go
-    text: should not use dot imports
-  - path: (framework|e2e)/.*.go
-    text: should not use dot imports
-  - path: _test\.go
-    text: cyclomatic complexity
-  # Append should be able to assign to a different var/slice.
-  - linters:
-    - gocritic
-    text: "appendAssign: append result not assigned to the same slice"
-
+    - linters:
+        - gosec
+      text: "G108: Profiling endpoint is automatically exposed on /debug/pprof"
+    - linters:
+        - revive
+      text: "exported: exported method .*\\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported"
+    - linters:
+        - errcheck
+      text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
+    # With Go 1.16, the new embed directive can be used with an un-named import,
+    # revive (previously, golint) only allows these to be imported in a main.go, which wouldn't work for us.
+    # This directive allows the embed package to be imported with an underscore everywhere.
+    - linters:
+        - revive
+      source: _ "embed"
+    # Exclude some packages or code to require comments, for example test code, or fake clients.
+    - linters:
+        - revive
+      text: exported (method|function|type|const) (.+) should have comment or be unexported
+      source: (func|type).*Fake.*
+    - linters:
+        - revive
+      text: exported (method|function|type|const) (.+) should have comment or be unexported
+      path: fake_\.go
+    - linters:
+        - revive
+      text: exported (method|function|type|const) (.+) should have comment or be unexported
+      path: "(framework|e2e|infrastructure/docker)/.*.go"
+    # Disable unparam "always receives" which might not be really
+    # useful when building libraries.
+    - linters:
+        - unparam
+      text: always receives
+    # Dot imports for gomega or ginkgo are allowed
+    # within test files.
+    - path: _test\.go
+      text: should not use dot imports
+    - path: (framework|e2e)/.*.go
+      text: should not use dot imports
+    - path: _test\.go
+      text: cyclomatic complexity
+    # Append should be able to assign to a different var/slice.
+    - linters:
+        - gocritic
+      text: "appendAssign: append result not assigned to the same slice"
 
 run:
   timeout: 10m
-  skip-files:
-  - "zz_generated.*\\.go$"
-  - ".*conversion.*\\.go$"
-  skip-dirs:
-  - mock*
   allow-parallel-runners: true

api/v1beta1/gcpcluster_webhook.go

@@ -41,8 +41,10 @@ func (c *GCPCluster) SetupWebhookWithManager(mgr ctrl.Manager) error {
 // +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-gcpcluster,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=gcpclusters,versions=v1beta1,name=validation.gcpcluster.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1
 // +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1beta1-gcpcluster,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=gcpclusters,versions=v1beta1,name=default.gcpcluster.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1
 
-var _ webhook.Validator = &GCPCluster{}
-var _ webhook.Defaulter = &GCPCluster{}
+var (
+	_ webhook.Validator = &GCPCluster{}
+	_ webhook.Defaulter = &GCPCluster{}
+)
 
 // Default implements webhook.Defaulter so a webhook will be registered for the type.
 func (c *GCPCluster) Default() {

api/v1beta1/gcpclustertemplate_webhook_test.go

@@ -39,7 +39,8 @@ func TestGCPClusterTemplate_ValidateUpdate(t *testing.T) {
 						Spec: GCPClusterSpec{
 							Project: "test-gcp-cluster",
 							Region:  "ap-south-1",
-						}},
+						},
+					},
 				},
 			},
 			oldTemplate: &GCPClusterTemplate{
@@ -48,7 +49,8 @@ func TestGCPClusterTemplate_ValidateUpdate(t *testing.T) {
 						Spec: GCPClusterSpec{
 							Project: "test-gcp-cluster",
 							Region:  "ap-south-1",
-						}},
+						},
+					},
 				},
 			},
 			wantErr: false,
@@ -61,7 +63,8 @@ func TestGCPClusterTemplate_ValidateUpdate(t *testing.T) {
 						Spec: GCPClusterSpec{
 							Project: "test-gcp-cluster",
 							Region:  "ap-south-1",
-						}},
+						},
+					},
 				},
 			},
 			oldTemplate: &GCPClusterTemplate{
@@ -70,7 +73,8 @@ func TestGCPClusterTemplate_ValidateUpdate(t *testing.T) {
 						Spec: GCPClusterSpec{
 							Project: "test-gcp-cluster",
 							Region:  "ap-east-1",
-						}},
+						},
+					},
 				},
 			},
 			wantErr: true,

api/v1beta1/gcpmachine_webhook.go

@@ -126,14 +126,14 @@ func checkKeyType(key *CustomerEncryptionKey) error {
 	switch key.KeyType {
 	case CustomerManagedKey:
 		if key.ManagedKey == nil || key.SuppliedKey != nil {
-			return fmt.Errorf("CustomerEncryptionKey KeyType of Managed requires only ManagedKey to be set")
+			return errors.New("CustomerEncryptionKey KeyType of Managed requires only ManagedKey to be set")
 		}
 	case CustomerSuppliedKey:
 		if key.SuppliedKey == nil || key.ManagedKey != nil {
-			return fmt.Errorf("CustomerEncryptionKey KeyType of Supplied requires only SuppliedKey to be set")
+			return errors.New("CustomerEncryptionKey KeyType of Supplied requires only SuppliedKey to be set")
 		}
 		if len(key.SuppliedKey.RawKey) > 0 && len(key.SuppliedKey.RSAEncryptedKey) > 0 {
-			return fmt.Errorf("CustomerEncryptionKey KeyType of Supplied requires either RawKey or RSAEncryptedKey to be set, not both")
+			return errors.New("CustomerEncryptionKey KeyType of Supplied requires either RawKey or RSAEncryptedKey to be set, not both")
 		}
 	default:
 		return fmt.Errorf("invalid value for CustomerEncryptionKey KeyType %s", key.KeyType)