chore(lint): bump golangci lint config to v2, address lint findings

.github/workflows/golangci-lint.yml

@@ -45,10 +45,10 @@ jobs:
 
       - uses: actions/checkout@v3
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v6
+        uses: golangci/golangci-lint-action@v8
         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: v1.64
+          version: v2.7.2
           args: --timeout=5m
 
           # Optional: working directory, useful for monorepos

.gitignore

@@ -52,4 +52,7 @@ node_modules/
 
 .cursor
 .envrc
+
+docker-compose.yaml
 mise.toml
+.jj/

.golangci.yml

@@ -1,4 +1,4 @@
-# Copyright 2024 StreamNative
+# Copyright 2025 StreamNative
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -12,26 +12,26 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+version: "2"
+
+run:
+  go: "1.24"
+  build-tags:
+    - tools
+    - e2e
+  allow-parallel-runners: true
+
 linters:
-  disable-all: true
+  default: none
   enable:
     - asciicheck
     - bodyclose
-    - unused
-    # - deadcode
-    # - depguard
+    - copyloopvar
     - dogsled
     - errcheck
-    - copyloopvar
-    # - gci
     - gocritic
-    #   - gocyclo
-    # - godot
-    - gofmt
-    - goimports
     - goprintffuncname
     - gosec
-    - gosimple
     - govet
     - importas
     - ineffassign
@@ -42,167 +42,137 @@ linters:
     - prealloc
     - predeclared
     - rowserrcheck
+    - revive
     - staticcheck
-    # - structcheck
-    - stylecheck
     - thelper
-    - typecheck
     - unconvert
     - unparam
-  # - varcheck
-
-linters-settings:
-  godot:
-    #   declarations - for top level declaration comments (default);
-    #   toplevel     - for top level comments;
-    #   all          - for all comments.
-    scope: toplevel
-    exclude:
-      - '^ \+.*'
-      - "^ ANCHOR.*"
-  gci:
-    sections:
-      - prefix(github.com/streamnative)
+    - unused
+  settings:
+    gocritic:
+      disabled-checks:
+        - appendAssign
+        - dupImport
+        - evalOrder
+        - ifElseChain
+        - octalLiteral
+        - regexpSimplify
+        - sloppyReassign
+        - truncateCmp
+        - typeDefFirst
+        - unnamedResult
+        - unnecessaryDefer
+        - whyNoLint
+        - wrapperFunc
+      enabled-tags:
+        - experimental
+    gosec:
+      excludes:
+        - G307
+        - G108
+    importas:
+      alias:
+        - pkg: k8s.io/api/core/v1
+          alias: corev1
+        - pkg: k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1
+          alias: apiextensionsv1
+        - pkg: k8s.io/apimachinery/pkg/apis/meta/v1
+          alias: metav1
+        - pkg: k8s.io/apimachinery/pkg/api/errors
+          alias: apierrors
+        - pkg: k8s.io/apimachinery/pkg/util/errors
+          alias: kerrors
+        - pkg: sigs.k8s.io/controller-runtime
+          alias: ctrl
+      no-unaliased: true
+    nolintlint:
+      require-specific: true
+      allow-unused: false
+  exclusions:
+    generated: lax
+    rules:
+      - 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
+      - linters:
+          - revive
+        text: exported (method|function|type|const) (.+) should have comment or be unexported
+        source: (func|type).*Fake.*
+      - linters:
+          - revive
+        path: fake_\.go
+        text: exported (method|function|type|const) (.+) should have comment or be unexported
+      - linters:
+          - revive
+        path: cmd/clusterctl/internal/test/providers.*.go
+        text: exported (method|function|type|const) (.+) should have comment or be unexported
+      - linters:
+          - revive
+        path: (framework|e2e)/.*.go
+        text: exported (method|function|type|const) (.+) should have comment or be unexported
+      - linters:
+          - unparam
+        text: always receives
+      - 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
+      - linters:
+          - gocritic
+        text: 'appendAssign: append result not assigned to the same slice'
+      - linters:
+          - staticcheck
+        path: .*(api|types)\/.*\/conversion.*\.go$
+        text: 'SA1019: in.(.+) is deprecated'
+      - linters:
+          - revive
+        path: .*(api|types)\/.*\/conversion.*\.go$
+        text: exported (method|function|type|const) (.+) should have comment or be unexported
+      - linters:
+          - revive
+        path: .*(api|types)\/.*\/conversion.*\.go$
+        text: 'var-naming: don''t use underscores in Go names;'
+      - linters:
+          - revive
+        path: .*(api|types)\/.*\/conversion.*\.go$
+        text: 'receiver-naming: receiver name'
+      - linters:
+          - staticcheck
+        path: .*(api|types)\/.*\/conversion.*\.go$
+        text: 'ST1003: should not use underscores in Go names;'
+      - linters:
+          - staticcheck
+        path: .*(api|types)\/.*\/conversion.*\.go$
+        text: 'ST1016: methods on the same type should have the same receiver name'
+    paths:
+      - zz_generated.*\.go$
+      - third_party
+      - third_party$
+      - builtin$
+      - examples$
 
-  importas:
-    no-unaliased: true
-    alias:
-      # Kubernetes
-      - pkg: k8s.io/api/core/v1
-        alias: corev1
-      - pkg: k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1
-        alias: apiextensionsv1
-      - pkg: k8s.io/apimachinery/pkg/apis/meta/v1
-        alias: metav1
-      - pkg: k8s.io/apimachinery/pkg/api/errors
-        alias: apierrors
-      - pkg: k8s.io/apimachinery/pkg/util/errors
-        alias: kerrors
-      # Controller Runtime
-      - pkg: sigs.k8s.io/controller-runtime
-        alias: ctrl
-  nolintlint:
-    allow-unused: false
-    require-specific: true
-  gosec:
-    excludes:
-      - G307 # Deferring unsafe method "Close" on type "\*os.File"
-      - G108 # Profiling endpoint is automatically exposed on /debug/pprof
-  gocritic:
-    enabled-tags:
-      - experimental
-    disabled-checks:
-      - appendAssign
-      - dupImport # https://github.com/go-critic/go-critic/issues/845
-      - evalOrder
-      - ifElseChain
-      - octalLiteral
-      - regexpSimplify
-      - sloppyReassign
-      - truncateCmp
-      - typeDefFirst
-      - unnamedResult
-      - unnecessaryDefer
-      - whyNoLint
-      - wrapperFunc
 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$"
-  exclude-dirs:
-    - third_party
-  exclude-rules:
-    - 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
-    # 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: cmd/clusterctl/internal/test/providers.*.go
-    - linters:
-        - revive
-      text: exported (method|function|type|const) (.+) should have comment or be unexported
-      path: "(framework|e2e)/.*.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"
-    # ifshort flags variables that are only used in the if-statement even though there is
-    # already a SimpleStmt being used in the if-statement in question.
-    - linters:
-        - ifshort
-      text: "variable .* is only used in the if-statement"
-      path: controllers/mdutil/util.go
-    # Disable linters for conversion
-    - linters:
-        - staticcheck
-      text: "SA1019: in.(.+) is deprecated"
-      path: .*(api|types)\/.*\/conversion.*\.go$
-    - linters:
-        - revive
-      text: exported (method|function|type|const) (.+) should have comment or be unexported
-      path: .*(api|types)\/.*\/conversion.*\.go$
-    - linters:
-        - revive
-      text: "var-naming: don't use underscores in Go names;"
-      path: .*(api|types)\/.*\/conversion.*\.go$
-    - linters:
-        - revive
-      text: "receiver-naming: receiver name"
-      path: .*(api|types)\/.*\/conversion.*\.go$
-    - linters:
-        - stylecheck
-      text: "ST1003: should not use underscores in Go names;"
-      path: .*(api|types)\/.*\/conversion.*\.go$
-    - linters:
-        - stylecheck
-      text: "ST1016: methods on the same type should have the same receiver name"
-      path: .*(api|types)\/.*\/conversion.*\.go$
-    # hack/tools
-    - linters:
-        - typecheck
-      text: import (".+") is a program, not an importable package
-      path: ^tools\.go$
-    # Ignore ifshort false positive
-    # TODO(sbueringer) false positive: https://github.com/esimonov/ifshort/issues/23
-    - linters:
-        - ifshort
-      text: "variable 'isDeleteNodeAllowed' is only used in the if-statement.*"
-      path: ^controllers/machine_controller\.go$
+  max-same-issues: 0
 
-run:
-  timeout: 10m
-  build-tags:
-    - tools
-    - e2e
-  allow-parallel-runners: true
-  go: "1.24"
+formatters:
+  enable:
+    - gofmt
+    - goimports
+  settings:
+    gci:
+      sections:
+        - prefix(github.com/streamnative)
+  exclusions:
+    generated: lax
+    paths:
+      - zz_generated.*\.go$
+      - third_party
+      - third_party$
+      - builtin$
+      - examples$

controllers/secret_controller.go

@@ -136,7 +136,7 @@ func (r *SecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 
 	finalizerName := cloudapi.SecretFinalizer
 	// Handle deletion
-	if !secretCR.ObjectMeta.DeletionTimestamp.IsZero() {
+	if !secretCR.DeletionTimestamp.IsZero() {
 		if controllerutil.ContainsFinalizer(secretCR, finalizerName) {
 			if err := secretClient.DeleteSecret(ctx, secretCR); err != nil {
 				// If the remote secret is already gone, that's okay.

controllers/serviceaccountbinding_controller.go

@@ -237,7 +237,7 @@ func (r *ServiceAccountBindingReconciler) Reconcile(ctx context.Context, req ctr
 			}
 		} else {
 			// Remote binding exists.
-			logger.Info("Remote ServiceAccountBinding already exists", "bindingName", remoteName, "poolMemberRef", poolMemberRef, "existingRemoteName", existingRemoteBinding.ObjectMeta.Name)
+			logger.Info("Remote ServiceAccountBinding already exists", "bindingName", remoteName, "poolMemberRef", poolMemberRef, "existingRemoteName", existingRemoteBinding.Name)
 			// TODO: Implement update logic if necessary.
 			// Compare existingRemoteBinding.Spec with what payloadForClient would generate via conversion.
 			// For now, we assume if it exists, it's correctly configured or updates are not handled here.

pkg/admin/impl.go

@@ -223,10 +223,7 @@ func (p *PulsarAdminClient) DeleteTopic(name string) error {
 	if err != nil {
 		return err
 	}
-	nonPartitioned := true
-	if topicMeta.Partitions > 0 {
-		nonPartitioned = false
-	}
+	nonPartitioned := topicMeta.Partitions < 1
 	if err := p.adminClient.Topics().Delete(*topic, true, nonPartitioned); err != nil {
 		return err
 	}

scripts/lint.sh

@@ -28,7 +28,7 @@ export POP_HOME=`cd $BINDIR/..;pwd`
 
 if [ ! -f ${POP_HOME}/bin/golangci-lint ]; then
     cd ${POP_HOME}
-    wget -O - -q https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s v1.55.2
+    wget -O - -q https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s v2.7.2
     cd -
 fi
 ${POP_HOME}/bin/golangci-lint --version