🌱 Bump golangci to v1.60.2 (#11132)

* Bump golangci to v1.60.2

* Fix lint

* Improve disabling of rules

* Fix lint and remove duplicate config

Author: mcbenjemaa

.github/workflows/pr-golangci-lint.yaml

@@ -30,6 +30,6 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@aaa42aa0628b4ae2578232a66b541047968fac86 # tag=v6.1.0
         with:
-          version: v1.59.0
+          version: v1.60.2
           args: --out-format=colored-line-number
           working-directory: ${{matrix.working-directory}}

.golangci.yml

@@ -55,6 +55,10 @@ linters:
     - whitespace # unnecessary newlines
 
 linters-settings:
+  gosec:
+    excludes:
+    # integer overflow conversion int -> int32
+      - G115
   gci:
     sections:
       - standard # Standard section: captures all standard packages.
@@ -348,3 +352,7 @@ issues:
     - gocritic
     text: "deferInLoop: Possible resource leak, 'defer' is called in the 'for' loop"
     path: _test\.go
+    # Ignore non-constant format string in call to condition utils
+  - linters:
+      - govet
+    text: "non-constant format string in call to sigs\\.k8s\\.io\\/cluster-api\\/util\\/conditions\\."

cmd/clusterctl/cmd/describe_cluster.go

@@ -338,7 +338,7 @@ func getRowName(obj ctrlclient.Object) string {
 
 	name := objName
 	if objectPrefix := tree.GetMetaName(obj); objectPrefix != "" {
-		name = fmt.Sprintf("%s - %s", objectPrefix, gray.Sprintf(name))
+		name = fmt.Sprintf("%s - %s", objectPrefix, gray.Sprintf("%s", name))
 	}
 
 	if !obj.GetDeletionTimestamp().IsZero() {

internal/controllers/machinedeployment/mdutil/util.go

@@ -120,16 +120,16 @@ func SetDeploymentRevision(deployment *clusterv1.MachineDeployment, revision str
 func MaxRevision(ctx context.Context, allMSs []*clusterv1.MachineSet) int64 {
 	log := ctrl.LoggerFrom(ctx)
 
-	max := int64(0)
+	maxVal := int64(0)
 	for _, ms := range allMSs {
 		if v, err := Revision(ms); err != nil {
 			// Skip the machine sets when it failed to parse their revision information
 			log.Error(err, fmt.Sprintf("Couldn't parse revision for MachineSet %s, deployment controller will skip it when reconciling revisions", ms.Name))
-		} else if v > max {
-			max = v
+		} else if v > maxVal {
+			maxVal = v
 		}
 	}
-	return max
+	return maxVal
 }
 
 // Revision returns the revision number of the input object.

internal/controllers/machinehealthcheck/machinehealthcheck_controller.go

@@ -580,13 +580,13 @@ func isAllowedRemediation(mhc *clusterv1.MachineHealthCheck) (bool, int32, error
 	var remediationAllowed bool
 	var remediationCount int32
 	if mhc.Spec.UnhealthyRange != nil {
-		min, max, err := getUnhealthyRange(mhc)
+		minVal, maxVal, err := getUnhealthyRange(mhc)
 		if err != nil {
 			return false, 0, err
 		}
 		unhealthyMachineCount := unhealthyMachineCount(mhc)
-		remediationAllowed = unhealthyMachineCount >= min && unhealthyMachineCount <= max
-		remediationCount = int32(max - unhealthyMachineCount)
+		remediationAllowed = unhealthyMachineCount >= minVal && unhealthyMachineCount <= maxVal
+		remediationCount = int32(maxVal - unhealthyMachineCount)
 		return remediationAllowed, remediationCount, nil
 	}
 
@@ -610,21 +610,21 @@ func getUnhealthyRange(mhc *clusterv1.MachineHealthCheck) (int, int, error) {
 
 	parts := strings.Split(unhealthyRange, "-")
 
-	min, err := strconv.ParseUint(parts[0], 10, 32)
+	minVal, err := strconv.ParseUint(parts[0], 10, 32)
 	if err != nil {
 		return 0, 0, err
 	}
 
-	max, err := strconv.ParseUint(parts[1], 10, 32)
+	maxVal, err := strconv.ParseUint(parts[1], 10, 32)
 	if err != nil {
 		return 0, 0, err
 	}
 
-	if max < min {
-		return 0, 0, errors.Errorf("max value %d cannot be less than min value %d for unhealthyRange", max, min)
+	if maxVal < minVal {
+		return 0, 0, errors.Errorf("max value %d cannot be less than min value %d for unhealthyRange", maxVal, minVal)
 	}
 
-	return int(min), int(max), nil
+	return int(minVal), int(maxVal), nil
 }
 
 func getMaxUnhealthy(mhc *clusterv1.MachineHealthCheck) (int, error) {

internal/controllers/machinehealthcheck/machinehealthcheck_targets_test.go

@@ -406,7 +406,7 @@ func TestHealthCheckTargets(t *testing.T) {
 	machineFailureMsgCondition := newFailedHealthCheckCondition(clusterv1.MachineHasFailureReason, "FailureMessage: %s", failureMsg)
 
 	// Target for when the machine has the remediate machine annotation
-	annotationRemediationMsg := "Marked for remediation via remediate-machine annotation"
+	const annotationRemediationMsg = "Marked for remediation via remediate-machine annotation"
 	testMachineAnnotationRemediation := testMachine.DeepCopy()
 	testMachineAnnotationRemediation.Annotations = map[string]string{clusterv1.RemediateMachineAnnotation: ""}
 	machineAnnotationRemediation := healthCheckTarget{

test/framework/docker_logcollector.go

@@ -147,7 +147,7 @@ func (k DockerLogCollector) collectLogsFromNode(ctx context.Context, outputPath
 				"tar", "--hard-dereference", "--dereference", "--directory", containerDir, "--create", "--file", "-", ".",
 			)
 			if err != nil {
-				return errors.Wrapf(err, execErr)
+				return errors.Wrap(err, execErr)
 			}
 
 			err = os.MkdirAll(outputDir, 0750)

test/infrastructure/docker/internal/docker/loadbalancer.go

@@ -65,7 +65,10 @@ func NewLoadBalancer(ctx context.Context, cluster *clusterv1.Cluster, dockerClus
 		return nil, err
 	}
 
-	ipFamily, err := cluster.GetIPFamily() //nolint:staticcheck // We tolerate this until removal; after removal IPFamily will become an internal CAPD concept. See https://github.com/kubernetes-sigs/cluster-api/issues/7521.
+	// We tolerate this until removal;
+	// after removal IPFamily will become an internal CAPD concept.
+	// See https://github.com/kubernetes-sigs/cluster-api/issues/7521.
+	ipFamily, err := cluster.GetIPFamily() //nolint:staticcheck
 	if err != nil {
 		return nil, fmt.Errorf("create load balancer: %s", err)
 	}

test/infrastructure/docker/internal/docker/machine.go

@@ -92,7 +92,10 @@ func NewMachine(ctx context.Context, cluster *clusterv1.Cluster, machine string,
 		return nil, err
 	}
 
-	ipFamily, err := cluster.GetIPFamily() //nolint:staticcheck // We tolerate this until removal; after removal IPFamily will become an internal CAPD concept. See https://github.com/kubernetes-sigs/cluster-api/issues/7521.
+	// We tolerate this until removal;
+	// after removal IPFamily will become an internal CAPD concept.
+	// See https://github.com/kubernetes-sigs/cluster-api/issues/7521.
+	ipFamily, err := cluster.GetIPFamily() //nolint:staticcheck
 	if err != nil {
 		return nil, fmt.Errorf("create docker machine: %s", err)
 	}
@@ -126,7 +129,10 @@ func ListMachinesByCluster(ctx context.Context, cluster *clusterv1.Cluster, labe
 		return nil, err
 	}
 
-	ipFamily, err := cluster.GetIPFamily() //nolint:staticcheck // We tolerate this until removal; after removal IPFamily will become an internal CAPD concept. See https://github.com/kubernetes-sigs/cluster-api/issues/7521.
+	// We tolerate this until removal;
+	// after removal IPFamily will become an internal CAPD concept.
+	// See https://github.com/kubernetes-sigs/cluster-api/issues/7521 .
+	ipFamily, err := cluster.GetIPFamily() //nolint:staticcheck
 	if err != nil {
 		return nil, fmt.Errorf("list docker machines by cluster: %s", err)
 	}

test/infrastructure/inmemory/pkg/runtime/cache/client.go

@@ -212,9 +212,8 @@ func (c *cache) store(resourceGroup string, obj client.Object, replaceExisting b
 				return apierrors.NewConflict(unsafeGuessGroupVersionResource(objGVK).GroupResource(), objKey.String(), fmt.Errorf("object has been modified"))
 			}
 
-			if err := c.beforeUpdate(resourceGroup, trackedObj, obj); err != nil {
-				return err
-			}
+			c.beforeUpdate(resourceGroup, trackedObj, obj)
+
 			tracker.objects[objGVK][objKey] = obj.DeepCopyObject().(client.Object)
 			updateTrackerOwnerReferences(tracker, trackedObj, obj, objRef)
 			c.afterUpdate(resourceGroup, trackedObj, obj)
@@ -227,9 +226,8 @@ func (c *cache) store(resourceGroup string, obj client.Object, replaceExisting b
 		return apierrors.NewNotFound(unsafeGuessGroupVersionResource(objGVK).GroupResource(), objKey.String())
 	}
 
-	if err := c.beforeCreate(resourceGroup, obj); err != nil {
-		return err
-	}
+	c.beforeCreate(resourceGroup, obj)
+
 	tracker.objects[objGVK][objKey] = obj.DeepCopyObject().(client.Object)
 	updateTrackerOwnerReferences(tracker, nil, obj, objRef)
 	c.afterCreate(resourceGroup, obj)
@@ -424,9 +422,7 @@ func (c *cache) doTryDeleteLocked(resourceGroup string, tracker *resourceGroupTr
 		oldObj := obj.DeepCopyObject().(client.Object)
 		now := metav1.Time{Time: time.Now().UTC()}
 		obj.SetDeletionTimestamp(&now)
-		if err := c.beforeUpdate(resourceGroup, oldObj, obj); err != nil {
-			return false, apierrors.NewBadRequest(err.Error())
-		}
+		c.beforeUpdate(resourceGroup, oldObj, obj)
 
 		objects[objKey] = obj
 		c.afterUpdate(resourceGroup, oldObj, obj)