chore(build): migrate to golangci-lint@v2 and golangci-lint-action@v7

.github/workflows/ci-golang-sbom.yml

@@ -27,9 +27,9 @@ jobs:
         make generate
 
     - name: Lint
-      uses: golangci/golangci-lint-action@v6
+      uses: golangci/golangci-lint-action@v7
       with:
-        version: v1.63.1
+        version: v2.0.2
         skip-pkg-cache: true
         skip-build-cache: true
         args: --config=./.golangci.yml --verbose

.golangci.yml

@@ -1,34 +1,66 @@
-run:
-  # timeout for analysis, e.g. 30s, 5m, default is 1m
-  timeout: 10m
-
+version: "2"
 linters:
   enable:
-    - megacheck
+    - asasalint
+    - asciicheck
+    - bidichk
+    - bodyclose
+    - durationcheck
+    - errchkjson
+    - errorlint
+    - exhaustive
+    - gocheckcompilerdirectives
+    - gochecksumtype
     - gocyclo
-    - gofmt
-    - revive
+    - gosec
+    - gosmopolitan
+    - loggercheck
+    - makezero
     - misspell
-  presets: # groups of linters. See https://golangci-lint.run/usage/linters/
-    - bugs
-    - unused
-  disable: 
-    - golint # deprecated, use 'revive'
-    - scopelint # deprecated, use 'exportloopref'
-    - contextcheck # too many false-positives
-    - noctx # not needed
-
-# all available settings of specific linters
-linters-settings:
-  unparam:
-    # Inspect exported functions, default is false. Set to true if no external program/library imports your code.
-    # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors:
-    # if it's called for subdir of a project it can't find external interfaces. All text editor integrations
-    # with golangci-lint call it on a directory with the changed file.
-    check-exported: true
-  revive:
-    rules:
-      - name: dot-imports
-        disabled: true # we allow for dot-imports
-
-
+    - musttag
+    - nilerr
+    - nilnesserr
+    - protogetter
+    - reassign
+    - recvcheck
+    - revive
+    - rowserrcheck
+    - spancheck
+    - sqlclosecheck
+    - testifylint
+    - unparam
+    - zerologlint
+  disable:
+    - contextcheck
+    - noctx
+  settings:
+    revive:
+      rules:
+        - name: dot-imports
+          disabled: true
+    staticcheck:
+      # see https://golangci-lint.run/usage/linters/#staticcheck
+      # we allow for dot-imports
+      checks: ["all", "-ST1000", "-ST1001", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022"]
+    unparam:
+      check-exported: true
+  exclusions:
+    generated: lax
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
+formatters:
+  enable:
+    - gofmt
+  exclusions:
+    generated: lax
+    paths:
+      - third_party$
+      - builtin$
+      - examples$

cmd/main.go

@@ -515,15 +515,16 @@ func (kw klogWriter) Write(p []byte) (n int, err error) {
 		klogv2.InfoDepth(OutputCallDepth, string(p))
 		return len(p), nil
 	}
-	if p[0] == 'I' {
+	switch p[0] {
+	case 'I':
 		klogv2.InfoDepth(OutputCallDepth, string(p[DefaultPrefixLength:]))
-	} else if p[0] == 'W' {
+	case 'W':
 		klogv2.WarningDepth(OutputCallDepth, string(p[DefaultPrefixLength:]))
-	} else if p[0] == 'E' {
+	case 'E':
 		klogv2.ErrorDepth(OutputCallDepth, string(p[DefaultPrefixLength:]))
-	} else if p[0] == 'F' {
+	case 'F':
 		klogv2.FatalDepth(OutputCallDepth, string(p[DefaultPrefixLength:]))
-	} else {
+	default:
 		klogv2.InfoDepth(OutputCallDepth, string(p[DefaultPrefixLength:]))
 	}
 	return len(p), nil

controllers/deactivation/deactivation_controller.go

@@ -248,7 +248,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 	// in some rare circumstances the deactivating notification/status may fail to be set due to cluster downtime or
 	// other reasons.  Because of this, the scheduled deactivation time that is set in the UserSignup.Status should be
 	// treated as informational only.
-	deactivationDueTime := deactivatingCondition.LastTransitionTime.Time.Add(time.Duration(deactivatingNotificationDays*24) * time.Hour)
+	deactivationDueTime := deactivatingCondition.LastTransitionTime.Add(time.Duration(deactivatingNotificationDays*24) * time.Hour)
 
 	if time.Now().Before(deactivationDueTime) {
 		// Update the ScheduledDeactivationTimestamp to the recalculated time base on when the deactivating notification was sent

controllers/deactivation/deactivation_controller_test.go

@@ -80,7 +80,7 @@ func TestReconcile(t *testing.T) {
 				case *toolchainv1alpha1.UserSignup:
 					return errors.New("mock error")
 				default:
-					return cl.Client.Status().Update(ctx, obj)
+					return cl.Status().Update(ctx, obj)
 				}
 			}
 
@@ -169,7 +169,7 @@ func TestReconcile(t *testing.T) {
 				case *toolchainv1alpha1.UserSignup:
 					return errors.New("mock error")
 				default:
-					return fakeClient.Client.Status().Update(ctx, obj)
+					return fakeClient.Status().Update(ctx, obj)
 				}
 			}
 
@@ -266,7 +266,7 @@ func TestReconcile(t *testing.T) {
 				case *toolchainv1alpha1.UserSignup:
 					return errors.New("mock error")
 				default:
-					return cl.Client.Status().Update(ctx, obj)
+					return cl.Status().Update(ctx, obj)
 				}
 			}
 
@@ -315,7 +315,7 @@ func TestReconcile(t *testing.T) {
 				case *toolchainv1alpha1.UserSignup:
 					return errors.New("mock error")
 				default:
-					return cl.Client.Status().Update(ctx, obj)
+					return cl.Status().Update(ctx, obj)
 				}
 			}
 
@@ -355,7 +355,7 @@ func TestReconcile(t *testing.T) {
 					case *toolchainv1alpha1.UserSignup:
 						return errors.New("mock error")
 					default:
-						return cl.Client.Status().Update(ctx, obj)
+						return cl.Status().Update(ctx, obj)
 					}
 				}
 
@@ -410,7 +410,7 @@ func TestReconcile(t *testing.T) {
 						case *toolchainv1alpha1.UserSignup:
 							return errors.New("mock error")
 						default:
-							return cl.Client.Status().Update(ctx, obj)
+							return cl.Status().Update(ctx, obj)
 						}
 					}
 
@@ -664,7 +664,7 @@ func TestReconcile(t *testing.T) {
 					case *toolchainv1alpha1.UserSignup:
 						return errors.New("mock error")
 					default:
-						return fakeClient.Client.Status().Update(ctx, obj)
+						return fakeClient.Status().Update(ctx, obj)
 					}
 				}
 

controllers/masteruserrecord/masteruserrecord_controller_test.go

@@ -845,7 +845,7 @@ func TestCreateSynchronizeOrDeleteUserAccountFailed(t *testing.T) {
 			if ua, ok := obj.(*toolchainv1alpha1.UserAccount); ok {
 				return fmt.Errorf("unable to update user account %s", ua.Name)
 			}
-			return memberClient.Client.Update(ctx, obj, opts...)
+			return memberClient.Update(ctx, obj, opts...)
 		}
 		otherTier := tiertest.OtherTier(t)
 		modifiedMur := murtest.NewMasterUserRecord(t, "john",
@@ -1257,7 +1257,7 @@ func TestDeleteUserAccountViaMasterUserRecordBeingDeleted(t *testing.T) {
 
 		// remove finalizer on UserAcc to delete
 		userAcc.SetFinalizers(nil)
-		err = memberClient.Client.Update(context.TODO(), userAcc)
+		err = memberClient.Update(context.TODO(), userAcc)
 		require.NoError(t, err)
 
 		result2, err2 := cntrl.Reconcile(context.TODO(), newMurRequest(mur))
@@ -1445,7 +1445,7 @@ func TestDisablingMasterUserRecord(t *testing.T) {
 
 func newMurRequest(mur *toolchainv1alpha1.MasterUserRecord) reconcile.Request {
 	return reconcile.Request{
-		NamespacedName: namespacedName(mur.ObjectMeta.Namespace, mur.ObjectMeta.Name),
+		NamespacedName: namespacedName(mur.Namespace, mur.Name),
 	}
 }
 

controllers/masteruserrecord/sync_test.go

@@ -565,7 +565,7 @@ func TestAlignReadiness(t *testing.T) {
 		// then
 		require.NoError(t, err)
 		assert.True(t, ready)
-		require.True(t, preAlignTime.Time.After(mur.Status.ProvisionedTime.Time), "the timestamp just before syncing should be after the ProvisionedTime because this is simulating the case where the record was already provisioned before")
+		require.True(t, preAlignTime.After(mur.Status.ProvisionedTime.Time), "the timestamp just before syncing should be after the ProvisionedTime because this is simulating the case where the record was already provisioned before")
 		assert.Equal(t, provisionTime.Time, mur.Status.ProvisionedTime.Time) // timestamp should be the same
 		murtest.AssertThatMasterUserRecord(t, "john", test.NewFakeClient(t, mur)).
 			HasConditions(toBeProvisioned(), toBeProvisionedNotificationCreated()).

controllers/nstemplatetier/nstemplatetier_controller_test.go

@@ -48,7 +48,7 @@ func TestReconcile(t *testing.T) {
 					if _, ok := obj.(*toolchainv1alpha1.NSTemplateTier); ok {
 						return errors.NewNotFound(schema.GroupResource{}, key.Name)
 					}
-					return cl.Client.Get(ctx, key, obj, opts...)
+					return cl.Get(ctx, key, obj, opts...)
 				}
 				// when
 				res, err := r.Reconcile(context.TODO(), req)
@@ -65,7 +65,7 @@ func TestReconcile(t *testing.T) {
 					if _, ok := obj.(*toolchainv1alpha1.NSTemplateTier); ok {
 						return fmt.Errorf("mock error")
 					}
-					return cl.Client.Get(ctx, key, obj, opts...)
+					return cl.Get(ctx, key, obj, opts...)
 				}
 				// when
 				res, err := r.Reconcile(context.TODO(), req)
@@ -302,7 +302,7 @@ func TestReconcile(t *testing.T) {
 					return fmt.Errorf("should not call Get more than once")
 				}
 				called = true
-				return cl.Client.Get(ctx, key, obj, opts...)
+				return cl.Get(ctx, key, obj, opts...)
 			}
 			cl.MockCreate = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.CreateOption) error {
 				return fmt.Errorf("should not call Create")

controllers/nstemplatetierrevisioncleanup/nstemplatetier_revision_cleanup_controller_test.go

@@ -6,7 +6,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/codeready-toolchain/api/api/v1alpha1"
 	toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
 	"github.com/codeready-toolchain/host-operator/controllers/nstemplatetierrevisioncleanup"
 	"github.com/codeready-toolchain/host-operator/pkg/apis"
@@ -48,7 +47,7 @@ func TestTTRDeletionReconcile(t *testing.T) {
 		ttr.DeletionTimestamp = &metav1.Time{
 			Time: time.Now(),
 		}
-		controllerutil.AddFinalizer(ttr, v1alpha1.FinalizerName)
+		controllerutil.AddFinalizer(ttr, toolchainv1alpha1.FinalizerName)
 		r, req, cl := prepareReconcile(t, ttr.Name, ttr, s, nsTemplateTier)
 		//when
 		res, err := r.Reconcile(context.TODO(), req)
@@ -165,7 +164,7 @@ func TestTTRDeletionReconcile(t *testing.T) {
 				if _, ok := obj.(*toolchainv1alpha1.NSTemplateTier); ok {
 					return fmt.Errorf("mock error")
 				}
-				return cl.Client.Get(ctx, key, obj, opts...)
+				return cl.Get(ctx, key, obj, opts...)
 			}
 			//when
 			_, err := r.Reconcile(context.TODO(), req)

controllers/socialevent/socialevent_controller.go

@@ -49,7 +49,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 
 	// fetch the SocialEvent resource
 	event := &toolchainv1alpha1.SocialEvent{}
-	if err := r.Client.Get(ctx, request.NamespacedName, event); err != nil {
+	if err := r.Get(ctx, request.NamespacedName, event); err != nil {
 		if errors.IsNotFound(err) {
 			logger.Info("SocialEvent not found")
 			return reconcile.Result{}, nil
@@ -65,7 +65,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 	// count the number of UserSignups with the `toolchain.dev.openshift.com/social-event` label matching the SocialEvent.Name
 	// and update the SocialEvent.Status.ActivationCount accordingly
 	usersignups := &toolchainv1alpha1.UserSignupList{}
-	if err := r.Client.List(ctx, usersignups,
+	if err := r.List(ctx, usersignups,
 		runtimeclient.InNamespace(r.Namespace),
 		runtimeclient.MatchingLabels{
 			toolchainv1alpha1.SocialEventUserSignupLabelKey: event.Name,
@@ -75,7 +75,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 	}
 	logger.Info("approved usersignups with activation code", "count", len(usersignups.Items))
 	event.Status.ActivationCount = len(usersignups.Items)
-	if err := r.Client.Status().Update(ctx, event); err != nil {
+	if err := r.Status().Update(ctx, event); err != nil {
 		return reconcile.Result{}, errs.Wrap(err, "unable to update status with activation count")
 	}
 	return reconcile.Result{}, nil
@@ -84,7 +84,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 func (r *Reconciler) checkTier(ctx context.Context, event *toolchainv1alpha1.SocialEvent) error {
 	// fetch the UserTier specified in the SocialEvent
 	userTier := &toolchainv1alpha1.UserTier{}
-	if err := r.Client.Get(ctx, types.NamespacedName{
+	if err := r.Get(ctx, types.NamespacedName{
 		Namespace: event.Namespace,
 		Name:      event.Spec.UserTier,
 	}, userTier); err != nil {
@@ -97,7 +97,7 @@ func (r *Reconciler) checkTier(ctx context.Context, event *toolchainv1alpha1.Soc
 
 	// fetch the NSTemplateTier specified in the SocialEvent
 	spaceTier := &toolchainv1alpha1.NSTemplateTier{}
-	if err := r.Client.Get(ctx, types.NamespacedName{
+	if err := r.Get(ctx, types.NamespacedName{
 		Namespace: event.Namespace,
 		Name:      event.Spec.SpaceTier,
 	}, spaceTier); err != nil {

controllers/socialevent/socialevent_controller_test.go

@@ -3,9 +3,10 @@ package socialevent_test
 import (
 	"context"
 	"fmt"
-	commontier "github.com/codeready-toolchain/toolchain-common/pkg/test/tier"
 	"testing"
 
+	commontier "github.com/codeready-toolchain/toolchain-common/pkg/test/tier"
+
 	toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
 	"github.com/codeready-toolchain/host-operator/controllers/socialevent"
 	"github.com/codeready-toolchain/host-operator/pkg/apis"
@@ -95,7 +96,7 @@ func TestReconcileSocialEvent(t *testing.T) {
 				if _, ok := obj.(*toolchainv1alpha1.UserTier); ok && key.Name == "notfound" {
 					return fmt.Errorf("mock error")
 				}
-				return hostClient.Client.Get(ctx, key, obj, opts...)
+				return hostClient.Get(ctx, key, obj, opts...)
 			}
 			ctrl := newReconciler(hostClient)
 
@@ -144,7 +145,7 @@ func TestReconcileSocialEvent(t *testing.T) {
 				if _, ok := obj.(*toolchainv1alpha1.NSTemplateTier); ok && key.Name == "notfound" {
 					return fmt.Errorf("mock error")
 				}
-				return hostClient.Client.Get(ctx, key, obj, opts...)
+				return hostClient.Get(ctx, key, obj, opts...)
 			}
 			ctrl := newReconciler(hostClient)