From 1596e01654fc45de75103aa2d85d53dd6b0a53bc Mon Sep 17 00:00:00 2001 From: Matt Conway Date: Sat, 27 Dec 2025 19:13:41 -0500 Subject: [PATCH 1/5] chore(lint): bump golangci lint config to v2, address lint findings --- .gitignore | 4 + .golangci.yml | 318 +++++++++++++++++++++++------------------------- go.sum | 1 + scripts/lint.sh | 2 +- 4 files changed, 156 insertions(+), 169 deletions(-) diff --git a/.gitignore b/.gitignore index 9609ee3f..c67deebe 100644 --- a/.gitignore +++ b/.gitignore @@ -52,3 +52,7 @@ node_modules/ .cursor .envrc + +docker-compose.yaml +mise.toml +.jj/ diff --git a/.golangci.yml b/.golangci.yml index 4486f42d..8ff54222 100644 --- a/.golangci.yml +++ b/.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 @@ -43,166 +43,148 @@ linters: - predeclared - rowserrcheck - 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 + godot: + scope: toplevel + exclude: + - ^ \+.* + - ^ ANCHOR.* + 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: + - ifshort + path: controllers/mdutil/util.go + text: variable .* is only used in the if-statement + - 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' + - linters: + - ifshort + path: ^controllers/machine_controller\.go$ + text: variable 'isDeleteNodeAllowed' is only used in the if-statement.* + 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$ diff --git a/go.sum b/go.sum index d11a820f..3404aa0e 100644 --- a/go.sum +++ b/go.sum @@ -110,6 +110,7 @@ github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XL github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= github.com/cncf/xds/go v0.0.0-20250121191232-2f005788dc42 h1:Om6kYQYDUk5wWbT0t0q6pvyM49i9XZAv9dDrkDA7gjk= +github.com/cncf/xds/go v0.0.0-20250121191232-2f005788dc42/go.mod h1:W+zGtBO5Y1IgJhy4+A9GOqVhqLpfZi+vwmdNXUehLA8= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/danieljoos/wincred v1.1.2 h1:QLdCxFs1/Yl4zduvBdcHB8goaYk9RARS2SgLLRuAyr0= github.com/danieljoos/wincred v1.1.2/go.mod h1:GijpziifJoIBfYh+S7BbkdUTU4LfM+QnGqR5Vl2tAx0= diff --git a/scripts/lint.sh b/scripts/lint.sh index 4c31eaef..1fd69a7d 100755 --- a/scripts/lint.sh +++ b/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 From d9d937968abe40cc695cc1bdd921d124297f6836 Mon Sep 17 00:00:00 2001 From: Matt Conway Date: Sat, 27 Dec 2025 19:13:41 -0500 Subject: [PATCH 2/5] chore(lint): address QF1007 staticcheck rule --- pkg/admin/impl.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/admin/impl.go b/pkg/admin/impl.go index 88806cf0..c196ab00 100644 --- a/pkg/admin/impl.go +++ b/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 } From 608024cdd8d03e09be5a9cc6805fe1b730078659 Mon Sep 17 00:00:00 2001 From: Matt Conway Date: Sat, 27 Dec 2025 19:13:41 -0500 Subject: [PATCH 3/5] chore(lint): address QF1008 staticcheck rule --- controllers/secret_controller.go | 2 +- controllers/serviceaccountbinding_controller.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/secret_controller.go b/controllers/secret_controller.go index 3f6b6410..815e3297 100644 --- a/controllers/secret_controller.go +++ b/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. diff --git a/controllers/serviceaccountbinding_controller.go b/controllers/serviceaccountbinding_controller.go index b51fa5e9..3b09485b 100644 --- a/controllers/serviceaccountbinding_controller.go +++ b/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. From cc71d16a99e18429b1a308275de144236267e519 Mon Sep 17 00:00:00 2001 From: Matt Conway Date: Sun, 28 Dec 2025 02:55:46 -0500 Subject: [PATCH 4/5] chore(lint): bump golangci-lint version in CI --- .github/workflows/golangci-lint.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 70fcd944..75d3dd3e 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.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 From 52372c353df21828ebcc7a6d1ccee365336a359f Mon Sep 17 00:00:00 2001 From: Matt Conway Date: Tue, 6 Jan 2026 21:37:46 -0500 Subject: [PATCH 5/5] chore: cleanup golangci lints --- .golangci.yml | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 8ff54222..40da2a55 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -42,6 +42,7 @@ linters: - prealloc - predeclared - rowserrcheck + - revive - staticcheck - thelper - unconvert @@ -65,11 +66,6 @@ linters: - wrapperFunc enabled-tags: - experimental - godot: - scope: toplevel - exclude: - - ^ \+.* - - ^ ANCHOR.* gosec: excludes: - G307 @@ -129,10 +125,6 @@ linters: - linters: - gocritic text: 'appendAssign: append result not assigned to the same slice' - - linters: - - ifshort - path: controllers/mdutil/util.go - text: variable .* is only used in the if-statement - linters: - staticcheck path: .*(api|types)\/.*\/conversion.*\.go$ @@ -157,10 +149,6 @@ linters: - staticcheck path: .*(api|types)\/.*\/conversion.*\.go$ text: 'ST1016: methods on the same type should have the same receiver name' - - linters: - - ifshort - path: ^controllers/machine_controller\.go$ - text: variable 'isDeleteNodeAllowed' is only used in the if-statement.* paths: - zz_generated.*\.go$ - third_party