Upgrade golangci-lint to v1.64.8

api/v1/configurationpolicy_types_test.go

@@ -63,8 +63,6 @@ func TestRecordDiffWithDefault(t *testing.T) {
 	}
 
 	for testName, test := range tests {
-		test := test
-
 		t.Run(
 			testName,
 			func(t *testing.T) {

build/common/Makefile.common.mk

@@ -5,15 +5,15 @@
 # https://github.com/kubernetes-sigs/controller-tools/releases/latest
 CONTROLLER_GEN_VERSION := v0.16.3
 # https://github.com/kubernetes-sigs/kustomize/releases/latest
-KUSTOMIZE_VERSION := v5.4.3
+KUSTOMIZE_VERSION := v5.6.0
 # https://github.com/golangci/golangci-lint/releases/latest
-GOLANGCI_VERSION := v1.52.2
+GOLANGCI_VERSION := v1.64.8
 # https://github.com/mvdan/gofumpt/releases/latest
 GOFUMPT_VERSION := v0.7.0
 # https://github.com/daixiang0/gci/releases/latest
 GCI_VERSION := v0.13.5
 # https://github.com/securego/gosec/releases/latest
-GOSEC_VERSION := v2.21.3
+GOSEC_VERSION := v2.22.2
 # https://github.com/kubernetes-sigs/kubebuilder/releases/latest
 KBVERSION := 3.15.1
 # https://github.com/kubernetes/kubernetes/releases/latest

build/common/config/.golangci.yml

@@ -24,6 +24,8 @@ run:
 linters:
   enable-all: true
   disable:
+    - mnd  # Disabled as tech debt: Magic number detection
+    - recvcheck # Disable linter checks for pointer vs. non-pointer receiver usage.
     - bodyclose
     # Disable contextcheck since we don't want to pass the context passed to PeriodicallyExecConfigPolicies to functions
     # called within it or else cleanup won't occur
@@ -193,6 +195,7 @@ issues:
     - return statements should not be cuddled if block has more than two lines
     - declarations should never be cuddled
     - don't use leading k in Go names
+    - "require-error: for error assertions use require"
 
   exclude-rules:
     # Allow dot imports in the tests.

controllers/configurationpolicy_controller.go

@@ -473,23 +473,26 @@ func (r *ConfigurationPolicyReconciler) shouldEvaluatePolicy(
 	var interval time.Duration
 	var getIntervalErr error
 
-	if policy.Status.ComplianceState == policyv1.Compliant {
+	switch policy.Status.ComplianceState {
+	case policyv1.Compliant:
 		interval, getIntervalErr = policy.Spec.EvaluationInterval.GetCompliantInterval()
-	} else if policy.Status.ComplianceState == policyv1.NonCompliant {
+	case policyv1.NonCompliant:
 		interval, getIntervalErr = policy.Spec.EvaluationInterval.GetNonCompliantInterval()
-	} else {
+	case policyv1.UnknownCompliancy, policyv1.Terminating:
 		log.V(1).Info("The policy has an unknown compliance. Will evaluate it now.")
 
 		return true, 0
 	}
 
 	now := time.Now().UTC()
 
-	if errors.Is(getIntervalErr, policyv1.ErrIsNever) {
+	switch {
+	case errors.Is(getIntervalErr, policyv1.ErrIsNever):
 		log.V(1).Info("Skipping the policy evaluation due to the spec.evaluationInterval value being set to never")
 
 		return false, 0
-	} else if errors.Is(getIntervalErr, policyv1.ErrIsWatch) {
+
+	case errors.Is(getIntervalErr, policyv1.ErrIsWatch):
 		minNextEval := lastEvaluated.Add(time.Second * time.Duration(r.EvalBackoffSeconds))
 		durationLeft := minNextEval.Sub(now)
 
@@ -507,7 +510,7 @@ func (r *ConfigurationPolicyReconciler) shouldEvaluatePolicy(
 		log.V(1).Info("The policy evaluation is configured for a watch event. Will evaluate now.")
 
 		return true, 0
-	} else if getIntervalErr != nil {
+	case getIntervalErr != nil:
 		log.Error(
 			getIntervalErr,
 			"The policy has an invalid spec.evaluationInterval value; defaulting to watch",
@@ -631,7 +634,7 @@ func (r *ConfigurationPolicyReconciler) cleanUpChildObjects(
 			// if object has already been deleted and is stuck, no need to redo delete request
 			_, deletionTimeFound, _ := unstructured.NestedString(existing.Object, "metadata", "deletionTimestamp")
 			if deletionTimeFound {
-				log.Error(fmt.Errorf("tried to delete object, but delete is hanging"), "Error")
+				log.Error(errors.New("tried to delete object, but delete is hanging"), "Error")
 
 				deletionFailures = append(deletionFailures, gvk.String()+fmt.Sprintf(` "%s" in namespace %s`,
 					object.Object.Metadata.Name, object.Object.Metadata.Namespace))
@@ -693,11 +696,12 @@ func (r *ConfigurationPolicyReconciler) cleanupImmediately() (beingUninstalled b
 
 	crdDeleting, defErr = r.definitionIsDeleting()
 
-	if beingUninstalledErr != nil && defErr != nil {
+	switch {
+	case beingUninstalledErr != nil && defErr != nil:
 		err = fmt.Errorf("%w; %w", beingUninstalledErr, defErr)
-	} else if beingUninstalledErr != nil {
+	case beingUninstalledErr != nil:
 		err = beingUninstalledErr
-	} else if defErr != nil {
+	case defErr != nil:
 		err = defErr
 	}
 
@@ -1360,16 +1364,14 @@ func (r *ConfigurationPolicyReconciler) determineDesiredObjects(
 	// The object is namespaced and either has no namespace specified or it is
 	// templated in the object definition. Fetch and filter namespaces using
 	// provided namespaceSelector.
-	if scopedGVR.Namespaced && desiredNs == "" {
+	switch {
+	case scopedGVR.Namespaced && desiredNs == "":
 		nsSelector := plc.Spec.NamespaceSelector
 
 		selectedNamespaces, err := r.SelectorReconciler.Get(plc.Namespace, plc.Name, nsSelector)
 		if err != nil {
-			log.Error(err, "Failed to select the namespaces",
-				"namespaceSelector", nsSelector.String())
-
+			log.Error(err, "Failed to select the namespaces", "namespaceSelector", nsSelector.String())
 			msg := fmt.Sprintf("Error filtering namespaces with provided namespaceSelector: %v", err)
-
 			errEvent := &objectTmplEvalEvent{
 				compliant: false,
 				reason:    "namespaceSelector error",
@@ -1384,10 +1386,12 @@ func (r *ConfigurationPolicyReconciler) determineDesiredObjects(
 				relevantNsNames[ns] = defaultNamesPerNs
 			}
 		}
-	} else if scopedGVR.Namespaced {
+
+	case scopedGVR.Namespaced:
 		// Namespaced, but a namespace was provided
 		relevantNsNames[desiredNs] = defaultNamesPerNs
-	} else {
+
+	default:
 		// Cluster scoped
 		relevantNsNames[""] = defaultNamesPerNs
 	}
@@ -1421,7 +1425,7 @@ func (r *ConfigurationPolicyReconciler) determineDesiredObjects(
 		objSelector, err := metav1.LabelSelectorAsSelector(objectSelector)
 		if err != nil {
 			log.Error(err, "Failed to select the resources",
-				"objectSelector", fmt.Sprint(objectT.ObjectSelector.String()))
+				"objectSelector", objectT.ObjectSelector.String())
 
 			msg := fmt.Sprintf(
 				"Error parsing provided objectSelector in the object-template at index [%d]: %v",
@@ -1463,7 +1467,7 @@ func (r *ConfigurationPolicyReconciler) determineDesiredObjects(
 
 			if err != nil {
 				log.Error(err, "Failed to fetch the resources",
-					"objectSelector", fmt.Sprint(objectT.ObjectSelector.String()))
+					"objectSelector", objectT.ObjectSelector.String())
 
 				msg := fmt.Sprintf(
 					"Error listing resources with provided objectSelector in the object-template at index [%d]: %v",
@@ -1536,24 +1540,27 @@ func (r *ConfigurationPolicyReconciler) determineDesiredObjects(
 			//   used but ObjectNamespace is
 			// - Only on the first namespace (outer) loop if the ObjectNamespace
 			//   template variable isn't used
-			if tmplResolver != nil && hasTemplate && desiredObj == nil {
+			if tmplResolver != nil && hasTemplate && desiredObj == nil { //nolint:gocritic
 				r.processedPolicyCache.Delete(plc.GetUID())
 
 				var templateContext interface{}
 
 				// Only populate context variables as they are available:
 				// - Namespaced object with metadata.name or objectSelector
-				if name != "" && ns != "" {
+				switch {
+				case name != "" && ns != "":
 					templateContext = struct {
 						ObjectNamespace string
 						ObjectName      string
 					}{ObjectNamespace: ns, ObjectName: name}
-				} else if name != "" {
+
+				case name != "":
 					// - Cluster-scoped object with metadata.name or objectSelector
 					templateContext = struct {
 						ObjectName string
-					}{ObjectName: ns}
-				} else if ns != "" {
+					}{ObjectName: name}
+
+				case ns != "":
 					// - Unnamed namespaced object
 					templateContext = struct {
 						ObjectNamespace string
@@ -1685,11 +1692,12 @@ func (r *ConfigurationPolicyReconciler) determineDesiredObjects(
 
 		var msg string
 
-		if skipObjectCalled {
+		switch {
+		case skipObjectCalled:
 			msg = "All objects of kind %s were skipped by the `skipObject` template function"
-		} else if objectSelector != nil {
+		case objectSelector != nil:
 			msg = "No objects of kind %s were matched from the policy objectSelector"
-		} else {
+		default:
 			msg = "No objects of kind %s were matched from the objectDefinition metadata"
 		}
 
@@ -1773,6 +1781,7 @@ func (r *ConfigurationPolicyReconciler) updatedRelatedObjects(
 		if related[i].Object.Kind != related[j].Object.Kind {
 			return related[i].Object.Kind < related[j].Object.Kind
 		}
+
 		if related[i].Object.Metadata.Namespace != related[j].Object.Metadata.Namespace {
 			return related[i].Object.Metadata.Namespace < related[j].Object.Metadata.Namespace
 		}
@@ -1815,13 +1824,14 @@ func addConditionToStatus(
 
 	var complianceState policyv1.ComplianceState
 
-	if reason == reasonCleanupError {
+	switch {
+	case reason == reasonCleanupError:
 		complianceState = policyv1.Terminating
 		newCond.Type = "violation"
-	} else if compliant {
+	case compliant:
 		complianceState = policyv1.Compliant
 		newCond.Type = "notification"
-	} else {
+	default:
 		complianceState = policyv1.NonCompliant
 		newCond.Type = "violation"
 	}
@@ -1953,6 +1963,7 @@ func (r *ConfigurationPolicyReconciler) handleObjects(
 		log.V(1).Info(
 			"The object template does not specify a name. Will search for matching objects in the namespace.",
 		)
+
 		objNames, allResourceNames = r.getMatchingNames(policy, desiredObj, scopedGVR, objectT)
 
 		// we do not support enforce on unnamed templates
@@ -1962,6 +1973,7 @@ func (r *ConfigurationPolicyReconciler) handleObjects(
 				"oldRemediationAction", remediation,
 			)
 		}
+
 		remediation = "inform"
 
 		if len(objNames) == 0 {
@@ -2011,6 +2023,7 @@ func (r *ConfigurationPolicyReconciler) handleObjects(
 		}
 	} else { // This case only occurs when the desired object is not named
 		resultEvent := objectTmplEvalEvent{}
+
 		if objShouldExist {
 			if exists {
 				resultEvent.compliant = true
@@ -2021,6 +2034,7 @@ func (r *ConfigurationPolicyReconciler) handleObjects(
 				// Length of objNames = 0, complianceType == musthave or mustonlyhave
 				// Find Noncompliant resources to add to the status.relatedObjects for debugging purpose
 				shouldAddCondensedRelatedObj = true
+
 				if desiredObjKind != "" && desiredObjName == "" {
 					// Change reason to Resource found but does not match
 					if len(allResourceNames) > 0 {
@@ -2362,19 +2376,16 @@ func (r *ConfigurationPolicyReconciler) getMatchingNames(
 		sel = labels.Nothing()
 	}
 
-	if currentlyUsingWatch(plc) {
+	switch {
+	case currentlyUsingWatch(plc):
 		var returnedItems []unstructured.Unstructured
-
 		returnedItems, err = r.DynamicWatcher.List(plc.ObjectIdentifier(), desiredObj.GroupVersionKind(), ns, sel)
-
 		resList = &unstructured.UnstructuredList{Items: returnedItems}
-	} else if scopedGVR.Namespaced {
+	case scopedGVR.Namespaced:
 		res := r.TargetK8sDynamicClient.Resource(scopedGVR.GroupVersionResource).Namespace(ns)
-
 		resList, err = res.List(context.TODO(), metav1.ListOptions{LabelSelector: sel.String()})
-	} else {
+	default:
 		res := r.TargetK8sDynamicClient.Resource(scopedGVR.GroupVersionResource)
-
 		resList, err = res.List(context.TODO(), metav1.ListOptions{LabelSelector: sel.String()})
 	}
 
@@ -2447,6 +2458,7 @@ func (r *ConfigurationPolicyReconciler) enforceByCreating(obj singleObject) (
 		log.V(2).Info(
 			"Created missing must have object", "resource", obj.scopedGVR.Resource, "name", obj.name,
 		)
+
 		reason = reasonWantFoundCreated
 		msg = fmt.Sprintf("%v %v was created successfully", obj.scopedGVR.Resource, idStr)
 
@@ -2782,7 +2794,7 @@ func mergeArrays(
 		// if an item in the existing object cannot be found in the template, we add it to the template array
 		// to produce the merged array
 		if count < oldItemSet[key].count {
-			for i := 0; i < (oldItemSet[key].count - count); i++ {
+			for range oldItemSet[key].count - count {
 				desiredArr = append(desiredArr, val2)
 			}
 		}
@@ -2901,7 +2913,7 @@ func handleSingleKey(
 			decoded, err := base64.StdEncoding.DecodeString(encoded)
 			if err != nil {
 				secretName := existingObj.GetName()
-				message := fmt.Sprintf("Error decoding secret: %s", secretName)
+				message := "Error decoding secret: " + secretName
 
 				return message, false, mergedValue, false
 			}
@@ -3111,7 +3123,7 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource(
 			recreateOption := objectT.RecreateOption
 
 			if isInform || !(recreateOption == policyv1.Always || recreateOption == policyv1.IfRequired) {
-				log.Info(fmt.Sprintf("Dry run update failed with error: %s", err.Error()))
+				log.Info("Dry run update failed with error: " + err.Error())
 
 				// Remove noisy fields such as managedFields from the diff
 				removeFieldsForComparison(existingObjectCopy)
@@ -3255,7 +3267,7 @@ func getMsgPrefix(obj *singleObject) string {
 	var namespaceMsg string
 
 	if obj.scopedGVR.Namespaced {
-		namespaceMsg = fmt.Sprintf(" in namespace %s", obj.namespace)
+		namespaceMsg = " in namespace " + obj.namespace
 	}
 
 	return fmt.Sprintf(`%s [%s]%s`, obj.scopedGVR.Resource, obj.name, namespaceMsg)
@@ -3478,13 +3490,14 @@ func (r *ConfigurationPolicyReconciler) addForUpdate(policy *policyv1.Configurat
 
 	previousComplianceState := policy.Status.ComplianceState
 
-	if policy.ObjectMeta.DeletionTimestamp != nil {
+	switch {
+	case policy.ObjectMeta.DeletionTimestamp != nil:
 		policy.Status.ComplianceState = policyv1.Terminating
-	} else if len(policy.Status.CompliancyDetails) == 0 {
+	case len(policy.Status.CompliancyDetails) == 0:
 		policy.Status.ComplianceState = policyv1.UnknownCompliancy
-	} else if compliant {
+	case compliant:
 		policy.Status.ComplianceState = policyv1.Compliant
-	} else {
+	default:
 		policy.Status.ComplianceState = policyv1.NonCompliant
 	}
 
@@ -3504,9 +3517,12 @@ func (r *ConfigurationPolicyReconciler) addForUpdate(policy *policyv1.Configurat
 	policyLog := log.WithValues("name", policy.Name, "namespace", policy.Namespace)
 
 	err := r.updatePolicyStatus(policy, sendEvent)
-	if k8serrors.IsConflict(err) {
+
+	switch {
+	case k8serrors.IsConflict(err):
 		policyLog.Error(err, "Tried to re-update status before previous update could be applied, retrying next loop")
-	} else if err != nil {
+
+	case err != nil:
 		policyLog.Error(err, "Could not update status, will retry")
 
 		parent := ""
@@ -3515,7 +3531,8 @@ func (r *ConfigurationPolicyReconciler) addForUpdate(policy *policyv1.Configurat
 		}
 
 		policySystemErrorsCounter.WithLabelValues(parent, policy.GetName(), "status-update-failed").Add(1)
-	} else {
+
+	default:
 		r.lastEvaluatedCache.Store(policy.UID, policy.Status.LastEvaluated)
 	}
 }
@@ -3602,7 +3619,7 @@ func (r *ConfigurationPolicyReconciler) updatePolicyStatus(
 			policy,
 			eventType,
 			"Policy updated",
-			fmt.Sprintf("Policy status is %s", eventMessage),
+			"Policy status is "+eventMessage,
 		)
 	}