From 15a2b001b1394d9c82916e53d00e5ddb8abb2034 Mon Sep 17 00:00:00 2001 From: Alex Russell Date: Thu, 6 Nov 2025 09:51:18 -0700 Subject: [PATCH] transition to golangci-lint v2 and fixup linter warnings --- .golangci.yml | 167 +++++++++++------- cmd/nginx-loadbalancer-kubernetes/main.go | 3 +- .../application/application_common_test.go | 1 + .../nginx_http_border_client_test.go | 4 - .../nginx_stream_border_client_test.go | 2 - internal/communication/factory.go | 1 + internal/communication/factory_test.go | 1 - internal/observation/watcher.go | 4 + internal/probation/server.go | 2 +- internal/synchronization/synchronizer.go | 6 +- internal/translation/translator.go | 1 + internal/translation/translator_test.go | 14 -- pkg/pointer/pointer.go | 1 - 13 files changed, 120 insertions(+), 87 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 268e39ff..a9003e0d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,90 +1,135 @@ # GolangCI-Lint settings - +version: "2" # Disable all linters and enable the required ones linters: - disable-all: true + default: none # Supported linters: https://golangci-lint.run/usage/linters/ enable: - - errcheck - - gosimple - - govet - - ineffassign - - staticcheck - - typecheck - - unused + - asasalint + - asciicheck + - bidichk - bodyclose + - containedctx + - copyloopvar + - decorder - dupl + - dupword + - durationcheck + - errcheck + - errchkjson + - errname + - errorlint + - exptostd + - fatcontext + - forbidigo + - forcetypeassert + - ginkgolinter + - gocheckcompilerdirectives - gochecknoinits + - gochecksumtype + - gocognit - goconst - gocritic - gocyclo - - gofmt - - goimports + - goprintffuncname - gosec + - govet + - grouper + - iface + - importas + - ineffassign + - interfacebloat - lll + - loggercheck + - makezero + - mirror - misspell + - musttag - nakedret + - nestif + - nilerr + - nilnesserr + - noctx + - nolintlint + - nosprintfhostport - prealloc - - stylecheck + - predeclared + - reassign + - rowserrcheck + - sqlclosecheck + - staticcheck + - testableexamples + - tparallel - unconvert - unparam - - paralleltest - - forbidigo - fast: false + - unused + - usestdlibvars + - usetesting + - wastedassign + - zerologlint + + # Specific linter settings + settings: + gocognit: + # Minimal code complexity to report + min-complexity: 16 + gocyclo: + # Minimal code complexity to report + min-complexity: 16 + misspell: + # Correct spellings using locale preferences for US + locale: US + staticcheck: + # Default list with custom silencing of: + + checks: [ + "all", + # Disable the checks that staticcheck disables by default + "-ST1000", + "-ST1003", + "-ST1016", + "-ST1020", + "-ST1021", + "-ST1022", + "-QF1008", # Omit embedded fields from selector expression. + ] + exclusions: + rules: + # Exclude gochecknoinits and gosec from running on tests files + - path: _test\.go + linters: + - dupl + - gochecknoinits + - gosec + - errcheck + - gocognit + - musttag + - noctx + - usestdlibvars + - exhaustruct + - copyloopvar + - goconst + - path: test/* + linters: + - gochecknoinits + - gosec + - gocognit + # Exclude lll issues for long lines with go:generate + - linters: + - lll + source: "^//go:generate " + paths: + - .go/pkg/mod + - vendor-fork + - internal/logreporter/api/loki # Run options run: # 10 minute timeout for analysis timeout: 10m -# Specific linter settings -linters-settings: - gocyclo: - # Minimal code complexity to report - min-complexity: 16 - govet: - disable-all: true - enable: - # Report shadowed variables - - shadow - - misspell: - # Correct spellings using locale preferences for US - locale: US - goimports: - # Put imports beginning with prefix after 3rd-party packages - local-prefixes: gitswarm.f5net.com/indigo,gitlab.com/f5 - exhaustruct: - # List of regular expressions to match struct packages and names. - # If this list is empty, all structs are tested. - # Default: [] - include: - - "gitlab.com/f5/nginx/nginxazurelb/azure-resource-provider/pkg/token.TokenID" - - "gitlab.com/f5/nginx/nginxazurelb/azure-resource-provider/internal/dpo/agent/certificates.CertGetRequest" issues: - exclude-dirs: - - .go/pkg/mod - # Exclude configuration - exclude-rules: - # Exclude gochecknoinits and gosec from running on tests files - - path: _test\.go - linters: - - gochecknoinits - - gosec - - path: test/* - linters: - - gochecknoinits - - gosec - # Exclude lll issues for long lines with go:generate - - linters: - - lll - source: "^//go:generate " - # Exclude false positive paralleltest error : Range statement for test case does not use range value in test Run - - linters: - - paralleltest - text: "does not use range value in test Run" - # Disable maximum issues count per one linter max-issues-per-linter: 0 diff --git a/cmd/nginx-loadbalancer-kubernetes/main.go b/cmd/nginx-loadbalancer-kubernetes/main.go index 76fb8c25..cf12d593 100644 --- a/cmd/nginx-loadbalancer-kubernetes/main.go +++ b/cmd/nginx-loadbalancer-kubernetes/main.go @@ -7,6 +7,7 @@ package main import ( "context" + "errors" "fmt" "log/slog" "os" @@ -119,7 +120,7 @@ func initializeLogger(logLevel string) { func buildKubernetesClient() (*kubernetes.Clientset, error) { slog.Debug("Watcher::buildKubernetesClient") k8sConfig, err := rest.InClusterConfig() - if err == rest.ErrNotInCluster { + if errors.Is(err, rest.ErrNotInCluster) { return nil, fmt.Errorf(`not running in a Cluster: %w`, err) } else if err != nil { return nil, fmt.Errorf(`error occurred getting the Cluster config: %w`, err) diff --git a/internal/application/application_common_test.go b/internal/application/application_common_test.go index ccbb8da0..1bab26e2 100644 --- a/internal/application/application_common_test.go +++ b/internal/application/application_common_test.go @@ -20,6 +20,7 @@ const ( ) func buildTerrorizingBorderClient(clientType string) (Interface, error) { + //nolint:dupword // dupword doesn't understand jokes nginxClient := mocks.NewErroringMockClient(errors.New(`something went horribly horribly wrong`)) bc, err := NewBorderClient(clientType, nginxClient) diff --git a/internal/application/nginx_http_border_client_test.go b/internal/application/nginx_http_border_client_test.go index 7a972068..ab3386cf 100644 --- a/internal/application/nginx_http_border_client_test.go +++ b/internal/application/nginx_http_border_client_test.go @@ -2,10 +2,6 @@ * Copyright 2023 F5 Inc. All rights reserved. * Use of this source code is governed by the Apache License that can be found in the LICENSE file. */ - -// dupl complains about duplicates with nginx_stream_border_client_test.go -// -//nolint:dupl package application import ( diff --git a/internal/application/nginx_stream_border_client_test.go b/internal/application/nginx_stream_border_client_test.go index cf4d302d..2198190a 100644 --- a/internal/application/nginx_stream_border_client_test.go +++ b/internal/application/nginx_stream_border_client_test.go @@ -2,8 +2,6 @@ * Copyright 2023 F5 Inc. All rights reserved. * Use of this source code is governed by the Apache License that can be found in the LICENSE file. */ -// dupl complains about duplicates with nginx_http_border_client_test.go -//nolint:dupl package application import ( diff --git a/internal/communication/factory.go b/internal/communication/factory.go index f2affddc..7e5d7e29 100644 --- a/internal/communication/factory.go +++ b/internal/communication/factory.go @@ -46,6 +46,7 @@ func NewHeaders(apiKey string) []string { // NewTransport is a factory method to create a new basic Http Transport. func NewTransport(skipVerify bool) *netHttp.Transport { + //nolint:forcetypeassert transport := netHttp.DefaultTransport.(*netHttp.Transport).Clone() transport.TLSClientConfig.InsecureSkipVerify = skipVerify diff --git a/internal/communication/factory_test.go b/internal/communication/factory_test.go index 8c637a8a..00660e44 100644 --- a/internal/communication/factory_test.go +++ b/internal/communication/factory_test.go @@ -24,7 +24,6 @@ func TestNewHTTPClient(t *testing.T) { } } -//nolint:goconst func TestNewHeaders(t *testing.T) { t.Parallel() headers := NewHeaders("fakeKey") diff --git a/internal/observation/watcher.go b/internal/observation/watcher.go index 0244bb2e..e6149d88 100644 --- a/internal/observation/watcher.go +++ b/internal/observation/watcher.go @@ -211,6 +211,7 @@ func (w *Watcher) buildEndpointSlicesEventHandlerForDelete() func(interface{}) { func (w *Watcher) buildServiceEventHandlerForAdd() func(interface{}) { slog.Info("Watcher::buildServiceEventHandlerForAdd") return func(obj interface{}) { + //nolint:forcetypeassert service := obj.(*v1.Service) if !w.isDesiredService(service) { return @@ -228,6 +229,7 @@ func (w *Watcher) buildServiceEventHandlerForAdd() func(interface{}) { func (w *Watcher) buildServiceEventHandlerForDelete() func(interface{}) { slog.Info("Watcher::buildServiceEventHandlerForDelete") return func(obj interface{}) { + //nolint:forcetypeassert service := obj.(*v1.Service) if !w.isDesiredService(service) { return @@ -245,7 +247,9 @@ func (w *Watcher) buildServiceEventHandlerForDelete() func(interface{}) { func (w *Watcher) buildServiceEventHandlerForUpdate() func(interface{}, interface{}) { slog.Info("Watcher::buildServiceEventHandlerForUpdate") return func(previous, updated interface{}) { + //nolint:forcetypeassert previousService := previous.(*v1.Service) + //nolint:forcetypeassert service := updated.(*v1.Service) if w.isDesiredService(previousService) && !w.isDesiredService(service) { diff --git a/internal/probation/server.go b/internal/probation/server.go index c3328c70..c946d985 100644 --- a/internal/probation/server.go +++ b/internal/probation/server.go @@ -60,7 +60,7 @@ func (hs *HealthServer) Start() { mux.HandleFunc("/readyz", hs.HandleReady) mux.HandleFunc("/startupz", hs.HandleStartup) - listener, err := net.Listen("tcp", address) + listener, err := net.Listen("tcp", address) //nolint:noctx if err != nil { slog.Error("failed to listen", "error", err) return diff --git a/internal/synchronization/synchronizer.go b/internal/synchronization/synchronizer.go index 3c71d463..b273691e 100644 --- a/internal/synchronization/synchronizer.go +++ b/internal/synchronization/synchronizer.go @@ -133,12 +133,12 @@ func (s *Synchronizer) buildBorderClient(event *core.ServerUpdateEvent) (applica var err error httpClient, err := communication.NewHTTPClient(s.settings.APIKey, s.settings.SkipVerifyTLS) if err != nil { - return nil, fmt.Errorf(`error creating HTTP client: %v`, err) + return nil, fmt.Errorf(`error creating HTTP client: %w`, err) } ngxClient, err := nginxClient.NewNginxClient(event.NginxHost, nginxClient.WithHTTPClient(httpClient)) if err != nil { - return nil, fmt.Errorf(`error creating Nginx Plus client: %v`, err) + return nil, fmt.Errorf(`error creating Nginx Plus client: %w`, err) } return application.NewBorderClient(event.ClientType, ngxClient) @@ -164,6 +164,8 @@ func (s *Synchronizer) fanOutEventToHosts(event core.ServerUpdateEvents) core.Se // handleServiceEvent gets the latest state for the service from the shared // informer cache, translates the service event into server update events and // dispatches these events to the proper handler function. +// +//nolint:gocognit func (s *Synchronizer) handleServiceEvent(ctx context.Context, key ServiceKey) (err error) { logger := slog.With("service", key) logger.Debug(`Synchronizer::handleServiceEvent`) diff --git a/internal/translation/translator.go b/internal/translation/translator.go index 6f01ad15..30224c87 100644 --- a/internal/translation/translator.go +++ b/internal/translation/translator.go @@ -99,6 +99,7 @@ func (t *Translator) buildLoadBalancerEvents(event *core.Event) (events core.Ser return events, nil } +//nolint:gocognit func (t *Translator) buildClusterIPEvents(event *core.Event) (events core.ServerUpdateEvents, err error) { namespace := event.Service.GetObjectMeta().GetNamespace() serviceName := event.Service.Name diff --git a/internal/translation/translator_test.go b/internal/translation/translator_test.go index a241cebd..49811cdb 100644 --- a/internal/translation/translator_test.go +++ b/internal/translation/translator_test.go @@ -110,7 +110,6 @@ func TestCreatedTranslateNoInterestingPorts(t *testing.T) { } } -//nolint:dupl func TestCreatedTranslateOneInterestingPort(t *testing.T) { t.Parallel() testcases := map[string]struct { @@ -165,7 +164,6 @@ func TestCreatedTranslateOneInterestingPort(t *testing.T) { } } -//nolint:dupl func TestCreatedTranslateManyInterestingPorts(t *testing.T) { t.Parallel() testcases := map[string]struct { @@ -220,7 +218,6 @@ func TestCreatedTranslateManyInterestingPorts(t *testing.T) { } } -//nolint:dupl func TestCreatedTranslateManyMixedPorts(t *testing.T) { t.Parallel() @@ -492,7 +489,6 @@ func TestUpdatedTranslateOneInterestingPort(t *testing.T) { } } -//nolint:dupl func TestUpdatedTranslateManyInterestingPorts(t *testing.T) { t.Parallel() @@ -547,7 +543,6 @@ func TestUpdatedTranslateManyInterestingPorts(t *testing.T) { } } -//nolint:dupl func TestUpdatedTranslateManyMixedPorts(t *testing.T) { t.Parallel() @@ -603,7 +598,6 @@ func TestUpdatedTranslateManyMixedPorts(t *testing.T) { } } -//nolint:dupl func TestUpdatedTranslateManyMixedPortsAndManyNodes(t *testing.T) { t.Parallel() @@ -756,7 +750,6 @@ func TestDeletedTranslateNoInterestingPortsAndNoNodes(t *testing.T) { } } -//nolint:dupl func TestDeletedTranslateOneInterestingPortAndNoNodes(t *testing.T) { t.Parallel() @@ -806,7 +799,6 @@ func TestDeletedTranslateOneInterestingPortAndNoNodes(t *testing.T) { } } -//nolint:dupl func TestDeletedTranslateManyInterestingPortsAndNoNodes(t *testing.T) { t.Parallel() @@ -905,7 +897,6 @@ func TestDeletedTranslateManyMixedPortsAndNoNodes(t *testing.T) { } } -//nolint:dupl func TestDeletedTranslateNoPortsAndOneNode(t *testing.T) { t.Parallel() @@ -1005,7 +996,6 @@ func TestDeletedTranslateNoInterestingPortsAndOneNode(t *testing.T) { } } -//nolint:dupl func TestDeletedTranslateOneInterestingPortAndOneNode(t *testing.T) { t.Parallel() @@ -1057,7 +1047,6 @@ func TestDeletedTranslateOneInterestingPortAndOneNode(t *testing.T) { } } -//nolint:dupl func TestDeletedTranslateManyInterestingPortsAndOneNode(t *testing.T) { t.Parallel() @@ -1160,7 +1149,6 @@ func TestDeletedTranslateManyMixedPortsAndOneNode(t *testing.T) { } } -//nolint:dupl func TestDeletedTranslateNoPortsAndManyNodes(t *testing.T) { t.Parallel() @@ -1260,7 +1248,6 @@ func TestDeletedTranslateNoInterestingPortsAndManyNodes(t *testing.T) { } } -//nolint:dupl func TestDeletedTranslateOneInterestingPortAndManyNodes(t *testing.T) { t.Parallel() @@ -1311,7 +1298,6 @@ func TestDeletedTranslateOneInterestingPortAndManyNodes(t *testing.T) { } } -//nolint:dupl func TestDeletedTranslateManyInterestingPortsAndManyNodes(t *testing.T) { t.Parallel() diff --git a/pkg/pointer/pointer.go b/pkg/pointer/pointer.go index 9fd6c696..1855b7c1 100644 --- a/pkg/pointer/pointer.go +++ b/pkg/pointer/pointer.go @@ -24,7 +24,6 @@ func ToSlice[T any](values []T) []*T { } ret := make([]*T, 0, len(values)) for _, v := range values { - v := v ret = append(ret, &v) } return ret