Bump linter to v1.55 (#258)

* format github workflow codequality.yml

Replace doublequotes with singlequotes in codequality.yml.

* bump golangcli-lint version

Bump the golangcli-lint version from v1.52 to v1.55 for increased stability. https://github.com/golangci/golangci-lint/releases/tag/v1.55.2

* fix comments

Fix some formating issues and typos in the comments.

* regenerate mocks

* move message to const

* whitelist dotimports and set lll to warning

Allow dotimports for ginkgo and gomega.

* fix lll

.github/workflows/codequality.yml

@@ -4,16 +4,16 @@ on:
     branches:
       - main
     paths-ignore:
-      - 'docs/**'
-      - '**.md'
-      - 'sec-scanners-config.yaml'
+      - "docs/**"
+      - "**.md"
+      - "sec-scanners-config.yaml"
   push:
     branches:
       - main
     paths-ignore:
-      - 'docs/**'
-      - '**.md'
-      - 'sec-scanners-config.yaml'
+      - "docs/**"
+      - "**.md"
+      - "sec-scanners-config.yaml"
 
 permissions:
   contents: read
@@ -27,12 +27,12 @@ jobs:
     steps:
       - uses: actions/setup-go@v5
         with:
-          go-version: '1.21'
+          go-version: "1.21"
           cache: false
       - uses: actions/checkout@v4
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v3
         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.52
+          version: v1.55
           args: --timeout=5m

.golangci.yaml

@@ -11,10 +11,26 @@ run:
   # Default: 1m
   timeout: 3m
 
-
 # This file contains only configs which differ from defaults.
 # All possible options can be found here https://github.com/golangci/golangci-lint/blob/master/.golangci.reference.yml
 linters-settings:
+  stylecheck:
+    dot-import-whitelist:
+      - github.com/onsi/ginkgo/v2
+      - github.com/onsi/gomega
+  revive:
+    enable-all-rules: false
+    severity: error
+    rules:
+      - name: comment-spacings
+        disabled: true
+      - name: dot-imports
+        severity: warning
+        disabled: true
+      - name: line-length-limit
+        severity: warning
+        disabled: true
+        arguments: [120]
   cyclop:
     # The maximal code complexity to report.
     # Default: 10
@@ -121,7 +137,7 @@ linters-settings:
   nolintlint:
     # Exclude following linters from requiring an explanation.
     # Default: []
-    allow-no-explanation: [ funlen, gocognit, lll ]
+    allow-no-explanation: [funlen, gocognit, lll]
     # Enable to require an explanation of nonzero length after each nolint directive.
     # Default: false
     require-explanation: true
@@ -141,7 +157,6 @@ linters-settings:
     # Default: false
     all: true
 
-
 linters:
   disable-all: true
   enable:
@@ -252,7 +267,6 @@ linters:
     #- structcheck # [deprecated, replaced by unused] finds unused struct fields
     #- varcheck # [deprecated, replaced by unused] finds unused global variables and constants
 
-
 issues:
   # Maximum count of issues with the same text.
   # Set to 0 to disable.
@@ -261,13 +275,13 @@ issues:
 
   exclude-rules:
     - source: "^//\\s*go:generate\\s"
-      linters: [ lll ]
+      linters: [lll]
     - source: "(noinspection|TODO)"
-      linters: [ godot ]
+      linters: [godot]
     - source: "//noinspection"
-      linters: [ gocritic ]
+      linters: [gocritic]
     - source: "^\\s+if _, ok := err\\.\\([^.]+\\.InternalError\\); ok {"
-      linters: [ errorlint ]
+      linters: [errorlint]
     - path: "_test\\.go"
       linters:
         - bodyclose

internal/controller/nats/controller.go

@@ -42,13 +42,15 @@ import (
 )
 
 const (
-	NATSFinalizerName   = "nats.operator.kyma-project.io/finalizer"
-	ControllerName      = "nats-manager"
-	ManagedByLabelKey   = "app.kubernetes.io/managed-by"
-	ManagedByLabelValue = ControllerName
+	NATSFinalizerName     = "nats.operator.kyma-project.io/finalizer"
+	ControllerName        = "nats-manager"
+	ManagedByLabelKey     = "app.kubernetes.io/managed-by"
+	ManagedByLabelValue   = ControllerName
+	CreationNotAllowedMsg = "Only a single NATS CR with name: %s and namespace: " +
+		"%s is allowed to be created in a Kyma cluster."
 )
 
-// Reconciler reconciles a Nats object.
+// Reconciler reconciles a NATS object.
 //
 //go:generate go run github.com/vektra/mockery/v2 --name=Controller --dir=../../../vendor/sigs.k8s.io/controller-runtime/pkg/controller --outpkg=mocks --case=underscore
 //go:generate go run github.com/vektra/mockery/v2 --name=Manager --dir=../../../vendor/sigs.k8s.io/controller-runtime/pkg/manager --outpkg=mocks --case=underscore
@@ -125,44 +127,44 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	// copy the object, so we don't modify the source object
+	// Copy the object, so we don't modify the source object.
 	nats := currentNats.DeepCopy()
 
-	// logger with nats details
+	// Create a logger with NATS details.
 	log := r.loggerWithNATS(nats)
 
-	// check if nats is in deletion state
+	// Check if nats is in deletion state.
 	if nats.IsInDeletion() {
 		return r.handleNATSDeletion(ctx, nats, log)
 	}
 
-	// check if the NATS CR is allowed to be created.
+	// Check if the NATS CR is allowed to be created.
 	if r.allowedNATSCR != nil {
 		if result, err := r.handleNATSCRAllowedCheck(ctx, nats, log); !result || err != nil {
 			return ctrl.Result{}, err
 		}
 	}
 
-	// handle reconciliation
+	// Handle reconciliation.
 	return r.handleNATSReconcile(ctx, nats, log)
 }
 
 // handleNATSCRAllowedCheck checks if NATS CR is allowed to be created or not.
 // returns true if the NATS CR is allowed.
 func (r *Reconciler) handleNATSCRAllowedCheck(ctx context.Context, nats *natsv1alpha1.NATS,
-	log *zap.SugaredLogger) (bool, error) {
-	// if the name and namespace matches with allowed NATS CR then allow the CR to be reconciled.
+	log *zap.SugaredLogger,
+) (bool, error) {
+	// If the name and namespace matches with allowed NATS CR then allow the CR to be reconciled.
 	if nats.Name == r.allowedNATSCR.Name && nats.Namespace == r.allowedNATSCR.Namespace {
 		return true, nil
 	}
 
-	// set error state in status.
+	// Set error state in status.
 	nats.Status.SetStateError()
-	// update conditions in status.
+	// Update conditions in status.
 	nats.Status.UpdateConditionStatefulSet(metav1.ConditionFalse,
 		natsv1alpha1.ConditionReasonForbidden, "")
-	errorMessage := fmt.Sprintf("Only a single NATS CR with name: %s and namespace: %s "+
-		"is allowed to be created in a Kyma cluster.", r.allowedNATSCR.Name, r.allowedNATSCR.Namespace)
+	errorMessage := fmt.Sprintf(CreationNotAllowedMsg, r.allowedNATSCR.Name, r.allowedNATSCR.Namespace)
 	nats.Status.UpdateConditionAvailable(metav1.ConditionFalse,
 		natsv1alpha1.ConditionReasonForbidden, errorMessage)
 	events.Warn(r.recorder, nats, natsv1alpha1.ConditionReasonForbidden, errorMessage)
@@ -173,7 +175,7 @@ func (r *Reconciler) handleNATSCRAllowedCheck(ctx context.Context, nats *natsv1a
 // generateNatsResources renders the NATS chart with provided overrides.
 // It puts results into ReleaseInstance.
 func (r *Reconciler) generateNatsResources(nats *natsv1alpha1.NATS, instance *chart.ReleaseInstance) error {
-	// generate Nats resources from chart
+	// Generate Nats resources from chart.
 	natsResources, err := r.natsManager.GenerateNATSResources(
 		instance,
 		manager.WithOwnerReference(*nats), // add owner references to all resources
@@ -183,15 +185,16 @@ func (r *Reconciler) generateNatsResources(nats *natsv1alpha1.NATS, instance *ch
 		return err
 	}
 
-	// update manifests in instance
+	// Update manifests in instance.
 	instance.SetRenderedManifests(*natsResources)
 	return nil
 }
 
 // initNATSInstance initializes a new NATS release instance based on NATS CR.
 func (r *Reconciler) initNATSInstance(ctx context.Context, nats *natsv1alpha1.NATS,
-	log *zap.SugaredLogger) (*chart.ReleaseInstance, error) {
-	// Check if istio is enabled in cluster
+	log *zap.SugaredLogger,
+) (*chart.ReleaseInstance, error) {
+	// Check if istio is enabled in cluster.
 	istioExists, err := r.kubeClient.DestinationRuleCRDExists(ctx)
 	if err != nil {
 		return nil, err
@@ -208,11 +211,11 @@ func (r *Reconciler) initNATSInstance(ctx context.Context, nats *natsv1alpha1.NA
 	}
 	log.Infof("NATS account secret (name: %s) exists: %t", accountSecretName, accountSecret != nil)
 
-	// generate overrides for helm chart
+	// Generate overrides for helm chart.
 	overrides := r.natsManager.GenerateOverrides(&nats.Spec, istioExists, accountSecret == nil)
 	log.Debugw("using overrides", "overrides", overrides)
 
-	// Init a release instance
+	// Init a release instance.
 	instance := chart.NewReleaseInstance(nats.Name, nats.Namespace, istioExists, overrides)
 
 	if err = r.generateNatsResources(nats, instance); err != nil {

internal/controller/nats/controller_test.go

@@ -211,17 +211,14 @@ func Test_handleNATSCRAllowedCheck(t *testing.T) {
 						Status:             metav1.ConditionFalse,
 						LastTransitionTime: metav1.Now(),
 						Reason:             string(natsv1alpha1.ConditionReasonForbidden),
-						Message: fmt.Sprintf("Only a single NATS CR with name: %s and namespace: %s "+
-							"is allowed to be created in a Kyma cluster.", givenAllowedNATS.Name,
-							givenAllowedNATS.Namespace),
+						Message:            fmt.Sprintf(CreationNotAllowedMsg, givenAllowedNATS.Name, givenAllowedNATS.Namespace),
 					},
 				}
 				require.True(t, natsv1alpha1.ConditionsEquals(wantConditions, gotNATS.Status.Conditions))
 
+				wantK8sEventMsg := fmt.Sprintf("Warning Forbidden %s", CreationNotAllowedMsg)
 				wantK8sEvent := []string{
-					fmt.Sprintf("Warning Forbidden Only a single NATS CR with name: %s and namespace: %s "+
-						"is allowed to be created in a Kyma cluster.", givenAllowedNATS.Name,
-						givenAllowedNATS.Namespace),
+					fmt.Sprintf(wantK8sEventMsg, givenAllowedNATS.Name, givenAllowedNATS.Namespace),
 				}
 
 				// check k8s events