diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 25fa17c854..048ecdac27 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -32,8 +32,8 @@ jobs: with: go-version: ${{ steps.vars.outputs.go_version }} - name: golangci-lint - uses: golangci/golangci-lint-action@4696ba8babb6127d732c3c6dde519db15edab9ea # tag=v6.5.1 + uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # tag=v8.0.0 with: - version: v1.64.6 - args: --out-format=colored-line-number + version: v2.1.6 + args: --output.text.print-linter-name=true --output.text.colors=true --timeout 10m working-directory: ${{matrix.working-directory}} diff --git a/.golangci.yml b/.golangci.yml index cc00a1903b..7390d2024b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,5 +1,10 @@ +version: "2" +run: + go: "1.24" + timeout: 10m + allow-parallel-runners: true linters: - disable-all: true + default: none enable: - asasalint - asciicheck @@ -16,10 +21,7 @@ linters: - goconst - gocritic - gocyclo - - gofmt - - goimports - goprintffuncname - - gosimple - govet - importas - ineffassign @@ -31,150 +33,152 @@ linters: - prealloc - revive - staticcheck - - stylecheck - tagliatelle - - typecheck - unconvert - unparam - unused - whitespace - -linters-settings: - govet: - enable-all: true - disable: - - fieldalignment - - shadow - 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 - revive: + settings: + govet: + disable: + - fieldalignment + - shadow + enable-all: true + 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 + revive: + rules: + # The following rules are recommended https://github.com/mgechev/revive#recommended-configuration + - name: blank-imports + - name: context-as-argument + - name: context-keys-type + - name: dot-imports + - name: error-return + - name: error-strings + - name: error-naming + - name: exported + - name: if-return + - name: increment-decrement + - name: var-naming + - name: var-declaration + - name: range + - name: receiver-naming + - name: time-naming + - name: unexported-return + - name: indent-error-flow + - name: errorf + - name: superfluous-else + - name: unreachable-code + - name: redefines-builtin-id + # + # Rules in addition to the recommended configuration above. + # + - name: bool-literal-in-expr + - name: constant-logical-expr + exclusions: + generated: strict + paths: + - zz_generated.*\.go$ + - .*conversion.*\.go$ rules: - # The following rules are recommended https://github.com/mgechev/revive#recommended-configuration - - name: blank-imports - - name: context-as-argument - - name: context-keys-type - - name: dot-imports - - name: error-return - - name: error-strings - - name: error-naming - - name: exported - - name: if-return - - name: increment-decrement - - name: var-naming - - name: var-declaration - - name: range - - name: receiver-naming - - name: time-naming - - name: unexported-return - - name: indent-error-flow - - name: errorf - - name: superfluous-else - - name: unreachable-code - - name: redefines-builtin-id - # - # Rules in addition to the recommended configuration above. - # - - name: bool-literal-in-expr - - name: constant-logical-expr - + - 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 + - linters: + - staticcheck + text: 'SA1019: .*The component config package has been deprecated and will be removed in a future release.' + # 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 + path: fake_\.go + text: exported (method|function|type|const) (.+) should have comment or be unexported + # Disable unparam "always receives" which might not be really + # useful when building libraries. + - linters: + - unparam + text: always receives + # Dot imports for gomega and ginkgo are allowed + # within test files. + - path: _test\.go + text: should not use dot imports + - path: _test\.go + text: cyclomatic complexity + - path: _test\.go + text: 'G107: Potential HTTP request made with variable url' + # Append should be able to assign to a different var/slice. + - linters: + - gocritic + text: 'appendAssign: append result not assigned to the same slice' + - linters: + - gocritic + text: 'singleCaseSwitch: should rewrite switch statement to if statement' + # It considers all file access to a filename that comes from a variable problematic, + # which is naiv at best. + - linters: + - gosec + text: 'G304: Potential file inclusion via variable' + - linters: + - dupl + path: _test\.go + - linters: + - revive + path: .*/internal/.* + - linters: + - unused + # Seems to incorrectly trigger on the two implementations that are only + # used through an interface and not directly..? + # Likely same issue as https://github.com/dominikh/go-tools/issues/1616 + path: pkg/controller/priorityqueue/metrics\.go + # 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. + - path: (.+)\.go$ + text: Subprocess launch(ed with variable|ing should be audited) + - linters: + - gosec + path: (.+)\.go$ + text: (G204|G104|G307) + - linters: + - staticcheck + path: (.+)\.go$ + text: (ST1000|QF1008) 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 - # 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) - - (G204|G104|G307) - - "ST1000: at least one file in a package should have a package comment" - exclude-files: - - "zz_generated.*\\.go$" - - ".*conversion.*\\.go$" - 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 - - linters: - - staticcheck - text: "SA1019: .*The component config package has been deprecated and will be removed in a future release." - # 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 - # Disable unparam "always receives" which might not be really - # useful when building libraries. - - linters: - - unparam - text: always receives - # Dot imports for gomega and ginkgo are allowed - # within test files. - - path: _test\.go - text: should not use dot imports - - path: _test\.go - text: cyclomatic complexity - - path: _test\.go - text: "G107: Potential HTTP request made with variable url" - # Append should be able to assign to a different var/slice. - - linters: - - gocritic - text: "appendAssign: append result not assigned to the same slice" - - linters: - - gocritic - text: "singleCaseSwitch: should rewrite switch statement to if statement" - # It considers all file access to a filename that comes from a variable problematic, - # which is naiv at best. - - linters: - - gosec - text: "G304: Potential file inclusion via variable" - - linters: - - dupl - path: _test\.go - - linters: - - revive - path: .*/internal/.* - - linters: - - unused - # Seems to incorrectly trigger on the two implementations that are only - # used through an interface and not directly..? - # Likely same issue as https://github.com/dominikh/go-tools/issues/1616 - path: pkg/controller/priorityqueue/metrics\.go - -run: - go: "1.24" - timeout: 10m - allow-parallel-runners: true + max-same-issues: 0 +formatters: + enable: + - gofmt + - goimports + exclusions: + generated: strict + paths: + - zz_generated.*\.go$ + - .*conversion.*\.go$ diff --git a/Makefile b/Makefile index b656fa0175..b8e9cfa877 100644 --- a/Makefile +++ b/Makefile @@ -99,7 +99,7 @@ $(CONTROLLER_GEN): # Build controller-gen from tools folder. GOLANGCI_LINT_BIN := golangci-lint GOLANGCI_LINT_VER := $(shell cat .github/workflows/golangci-lint.yml | grep [[:space:]]version: | sed 's/.*version: //') GOLANGCI_LINT := $(abspath $(TOOLS_BIN_DIR)/$(GOLANGCI_LINT_BIN)-$(GOLANGCI_LINT_VER)) -GOLANGCI_LINT_PKG := github.com/golangci/golangci-lint/cmd/golangci-lint +GOLANGCI_LINT_PKG := github.com/golangci/golangci-lint/v2/cmd/golangci-lint $(GOLANGCI_LINT): # Build golangci-lint from tools folder. GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) $(GOLANGCI_LINT_PKG) $(GOLANGCI_LINT_BIN) $(GOLANGCI_LINT_VER) diff --git a/examples/builtins/controller.go b/examples/builtins/controller.go index 6c8c5d935f..443283140a 100644 --- a/examples/builtins/controller.go +++ b/examples/builtins/controller.go @@ -21,7 +21,7 @@ import ( "fmt" appsv1 "k8s.io/api/apps/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -43,13 +43,13 @@ func (r *reconcileReplicaSet) Reconcile(ctx context.Context, request reconcile.R // Fetch the ReplicaSet from the cache rs := &appsv1.ReplicaSet{} err := r.client.Get(ctx, request.NamespacedName, rs) - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { log.Error(nil, "Could not find ReplicaSet") return reconcile.Result{}, nil } if err != nil { - return reconcile.Result{}, fmt.Errorf("could not fetch ReplicaSet: %+v", err) + return reconcile.Result{}, fmt.Errorf("could not fetch ReplicaSet: %+w", err) } // Print the ReplicaSet @@ -67,7 +67,7 @@ func (r *reconcileReplicaSet) Reconcile(ctx context.Context, request reconcile.R rs.Labels["hello"] = "world" err = r.client.Update(ctx, rs) if err != nil { - return reconcile.Result{}, fmt.Errorf("could not write ReplicaSet: %+v", err) + return reconcile.Result{}, fmt.Errorf("could not write ReplicaSet: %+w", err) } return reconcile.Result{}, nil diff --git a/examples/crd/main.go b/examples/crd/main.go index 1f6cd5fac2..0bf65c9890 100644 --- a/examples/crd/main.go +++ b/examples/crd/main.go @@ -65,7 +65,7 @@ func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu if podFound { shouldStop := chaospod.Spec.NextStop.Time.Before(time.Now()) if !shouldStop { - return ctrl.Result{RequeueAfter: chaospod.Spec.NextStop.Sub(time.Now()) + 1*time.Second}, nil + return ctrl.Result{RequeueAfter: time.Until(chaospod.Spec.NextStop.Time) + 1*time.Second}, nil } if err := r.Delete(ctx, &pod); err != nil { diff --git a/examples/crd/pkg/groupversion_info.go b/examples/crd/pkg/groupversion_info.go index 04953dd939..31dfbbc779 100644 --- a/examples/crd/pkg/groupversion_info.go +++ b/examples/crd/pkg/groupversion_info.go @@ -20,13 +20,10 @@ package pkg import ( "k8s.io/apimachinery/pkg/runtime/schema" - logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/scheme" ) var ( - log = logf.Log.WithName("chaospod-resource") - // SchemeGroupVersion is group version used to register these objects SchemeGroupVersion = schema.GroupVersion{Group: "chaosapps.metamagical.io", Version: "v1"} diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index 9efd04877c..64cf207570 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -1917,7 +1917,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca It("should error when starting the cache a second time", func() { err := informerCache.Start(context.Background()) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("Informer already started")) + Expect(err.Error()).To(ContainSubstring("informer already started")) }) Context("with structured objects", func() { diff --git a/pkg/cache/internal/informers.go b/pkg/cache/internal/informers.go index b21a9c6345..4bf832b2d9 100644 --- a/pkg/cache/internal/informers.go +++ b/pkg/cache/internal/informers.go @@ -201,7 +201,7 @@ func (ip *Informers) Start(ctx context.Context) error { defer ip.mu.Unlock() if ip.started { - return errors.New("Informer already started") //nolint:stylecheck + return errors.New("informer already started") //nolint:stylecheck } // Set the context so it can be passed to informers that are added later diff --git a/pkg/client/config/config_test.go b/pkg/client/config/config_test.go index 76d42b318f..bbaeb2e2bd 100644 --- a/pkg/client/config/config_test.go +++ b/pkg/client/config/config_test.go @@ -52,7 +52,7 @@ var _ = Describe("Config", func() { }) AfterEach(func() { - os.Unsetenv(clientcmd.RecommendedConfigPathEnvVar) + _ = os.Unsetenv(clientcmd.RecommendedConfigPathEnvVar) kubeconfig = "" clientcmd.RecommendedHomeFile = origRecommendedHomeFile diff --git a/pkg/client/fake/client.go b/pkg/client/fake/client.go index 99124e2f03..39482aedee 100644 --- a/pkg/client/fake/client.go +++ b/pkg/client/fake/client.go @@ -411,12 +411,9 @@ func (t versionedTracker) Patch(gvr schema.GroupVersionResource, obj runtime.Obj return err } - isStatus := false // We apply patches using a client-go reaction that ends up calling the trackers Patch. As we can't change // that reaction, we use the callstack to figure out if this originated from the status client. - if bytes.Contains(debug.Stack(), []byte("sigs.k8s.io/controller-runtime/pkg/client/fake.(*fakeSubResourceClient).statusPatch")) { - isStatus = true - } + isStatus := bytes.Contains(debug.Stack(), []byte("sigs.k8s.io/controller-runtime/pkg/client/fake.(*fakeSubResourceClient).statusPatch")) obj, err = t.updateObject(gvr, obj, ns, isStatus, false, patchOptions.DryRun) if err != nil { diff --git a/pkg/envtest/webhook.go b/pkg/envtest/webhook.go index f6bfe95cc6..a6961bf7c6 100644 --- a/pkg/envtest/webhook.go +++ b/pkg/envtest/webhook.go @@ -419,8 +419,8 @@ func readWebhooks(path string) ([]*admissionv1.MutatingWebhookConfiguration, []* const ( admissionregv1 = "admissionregistration.k8s.io/v1" ) - switch { - case generic.Kind == "MutatingWebhookConfiguration": + switch generic.Kind { + case "MutatingWebhookConfiguration": if generic.APIVersion != admissionregv1 { return nil, nil, fmt.Errorf("only v1 is supported right now for MutatingWebhookConfiguration (name: %s)", generic.Name) } @@ -429,7 +429,7 @@ func readWebhooks(path string) ([]*admissionv1.MutatingWebhookConfiguration, []* return nil, nil, err } mutHooks = append(mutHooks, hook) - case generic.Kind == "ValidatingWebhookConfiguration": + case "ValidatingWebhookConfiguration": if generic.APIVersion != admissionregv1 { return nil, nil, fmt.Errorf("only v1 is supported right now for ValidatingWebhookConfiguration (name: %s)", generic.Name) }