Upgrade golangci-lint to 1.64.8

- All above these version: github.com/golangci/golangci-lint/cmd/golangci-lint@v1.55.1 github.com/daixiang0/gci@v0.11.2 sigs.k8s.io/kustomize/kustomize/v5@v5.2.1 gosec => 2.22.2 kustomize => 5.6.
- Upgrade golang to 1.23
ref: https://issues.redhat.com/browse/ACM-8341
Signed-off-by: yiraeChristineKim <yikim@redhat.com>

Author: yiraeChristineKim

api/v1/policy_webhook.go

@@ -100,7 +100,7 @@ func (r *Policy) validateName() error {
 
 	// 1 character for "."
 	if (utf8.RuneCountInString(r.Name) + utf8.RuneCountInString(r.Namespace)) > 62 {
-		log.Info(fmt.Sprintf("Invalid policy name/namespace: %s", errName.Error()))
+		log.Info("Invalid policy name/namespace: " + errName.Error())
 
 		return errName
 	}
@@ -126,7 +126,7 @@ func (r *Policy) validateRemediationAction() error {
 		if objUnstruct.GroupVersionKind().Kind == "ConfigurationPolicy" {
 			_, found, _ := unstructured.NestedString(objUnstruct.Object, "spec", "remediationAction")
 			if !found {
-				log.Info(fmt.Sprintf("Invalid remediationAction configuration: %s", errRemediation.Error()))
+				log.Info("Invalid remediationAction configuration: " + errRemediation.Error())
 
 				return errRemediation
 			}

api/v1beta1/policyautomation_types.go

@@ -134,7 +134,7 @@ func init() {
 // ReplicatedComplianceHistory defines the replicated policy compliance details history.
 type ReplicatedComplianceHistory struct {
 	LastTimestamp metav1.Time `json:"lastTimestamp,omitempty" protobuf:"bytes,7,opt,name=lastTimestamp"`
-	Message       string      `json:"message,omitempty" protobuf:"bytes,4,opt,name=message"`
+	Message       string      `json:"message,omitempty"       protobuf:"bytes,4,opt,name=message"`
 }
 
 // ReplicatedDetailsPerTemplate defines the replicated policy compliance details and history.
@@ -153,10 +153,10 @@ type ReplicatedPolicyStatus struct {
 // ViolationContext defines the noncompliant replicated policy information that is sent to the
 // AnsibleJob through the extra_vars parameter.
 type ViolationContext struct {
-	TargetClusters   []string                          `json:"targetClusters" ansibleJob:"target_clusters"`
-	PolicyName       string                            `json:"policyName" ansibleJob:"policy_name"`
-	PolicyNamespace  string                            `json:"policyNamespace" ansibleJob:"policy_namespace"`
-	HubCluster       string                            `json:"hubCluster" ansibleJob:"hub_cluster"`
-	PolicySets       []string                          `json:"policySets" ansibleJob:"policy_sets"`
-	PolicyViolations map[string]ReplicatedPolicyStatus `json:"policyViolations" ansibleJob:"policy_violations"`
+	TargetClusters   []string                          `ansibleJob:"target_clusters"   json:"targetClusters"`
+	PolicyName       string                            `ansibleJob:"policy_name"       json:"policyName"`
+	PolicyNamespace  string                            `ansibleJob:"policy_namespace"  json:"policyNamespace"`
+	HubCluster       string                            `ansibleJob:"hub_cluster"       json:"hubCluster"`
+	PolicySets       []string                          `ansibleJob:"policy_sets"       json:"policySets"`
+	PolicyViolations map[string]ReplicatedPolicyStatus `ansibleJob:"policy_violations" json:"policyViolations"`
 }

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,7 @@ run:
 linters:
   enable-all: true
   disable:
+    - mnd # Disabled as tech debt: Magic number detection
     - bodyclose
     - contextcheck # New linter to consider
     - cyclop

controllers/automation/PolicyAutomationPredicate.go

@@ -34,7 +34,7 @@ var policyAuomtationPredicateFuncs = predicate.Funcs{
 
 		return policyAutomationNew.Spec.PolicyRef != ""
 	},
-	DeleteFunc: func(e event.DeleteEvent) bool {
+	DeleteFunc: func(_ event.DeleteEvent) bool {
 		return false
 	},
 }

controllers/automation/ansible.go

@@ -122,16 +122,18 @@ func CreateAnsibleJob(policyAutomation *policyv1beta1.PolicyAutomation,
 	typesOf := values.Type()
 	// add every violationContext fields into mapExtraVars as well as the empty values,
 	// or when the whole violationContext is empty
-	for i := 0; i < values.NumField(); i++ {
+	for i := range values.NumField() {
 		tag := typesOf.Field(i).Tag
 		value := values.Field(i).Interface()
 
 		var fieldName string
-		if tag.Get("ansibleJob") != "" {
+
+		switch {
+		case tag.Get("ansibleJob") != "":
 			fieldName = tag.Get("ansibleJob")
-		} else if tag.Get("json") != "" {
+		case tag.Get("json") != "":
 			fieldName = strings.SplitN(tag.Get("json"), ",", 2)[0]
-		} else {
+		default:
 			fieldName = typesOf.Field(i).Name
 		}
 

controllers/automation/policyPredicate.go

@@ -24,10 +24,10 @@ var policyPredicateFuncs = predicate.Funcs{
 
 		return !cmp.Equal(plcObjNew.Status.Status, plcObjOld.Status.Status)
 	},
-	CreateFunc: func(e event.CreateEvent) bool {
+	CreateFunc: func(_ event.CreateEvent) bool {
 		return false
 	},
-	DeleteFunc: func(e event.DeleteEvent) bool {
+	DeleteFunc: func(_ event.DeleteEvent) bool {
 		return false
 	},
 }

controllers/automation/policyautomation_controller.go

@@ -5,7 +5,6 @@ package automation
 
 import (
 	"context"
-	"fmt"
 	"strconv"
 	"time"
 
@@ -83,7 +82,7 @@ func (r *PolicyAutomationReconciler) setOwnerReferences(
 	}
 
 	if !policyOwnerRefFound {
-		log.V(3).Info(fmt.Sprintf("Setting the owner reference on the PolicyAutomation %s", policyAutomation.GetName()))
+		log.V(3).Info("Setting the owner reference on the PolicyAutomation " + policyAutomation.GetName())
 		policyAutomation.SetOwnerReferences([]metav1.OwnerReference{
 			*metav1.NewControllerRef(policy, policy.GroupVersionKind()),
 		})
@@ -232,8 +231,8 @@ func (r *PolicyAutomationReconciler) getViolationContext(
 		if contextLimit > 0 && len(violationContext.PolicyViolations) == contextLimit {
 			log.V(2).Info(
 				"PolicyViolationsLimit is %s so skipping %s remaining replicated policies violations.",
-				fmt.Sprint(contextLimit),
-				fmt.Sprint(len(replicatedPlcList.Items)-contextLimit),
+				strconv.Itoa(contextLimit),
+				strconv.Itoa(len(replicatedPlcList.Items)-contextLimit),
 			)
 
 			break
@@ -301,9 +300,10 @@ func (r *PolicyAutomationReconciler) Reconcile(
 		return reconcile.Result{}, err
 	}
 
-	if policyAutomation.Annotations["policy.open-cluster-management.io/rerun"] == "true" {
-		AjExist, err := MatchPAResouceV(policyAutomation,
-			r.DynamicClient, policyAutomation.GetResourceVersion())
+	switch {
+	case policyAutomation.Annotations["policy.open-cluster-management.io/rerun"] == "true":
+		// Rerun logic
+		AjExist, err := MatchPAResouceV(policyAutomation, r.DynamicClient, policyAutomation.GetResourceVersion())
 		if err != nil {
 			log.Error(err, "Failed to compare Ansible job's resourceVersion")
 
@@ -317,24 +317,18 @@ func (r *PolicyAutomationReconciler) Reconcile(
 		}
 
 		targetList := common.FindNonCompliantClustersForPolicy(policy)
-		log.Info(
-			"Creating an Ansible job", "mode", "manual",
-			"clusterCount", strconv.Itoa(len(targetList)))
+		log.Info("Creating an Ansible job", "mode", "manual", "clusterCount", strconv.Itoa(len(targetList)))
 
 		violationContext, _ := r.getViolationContext(ctx, policy, targetList, policyAutomation)
 
-		err = CreateAnsibleJob(
-			policyAutomation,
-			r.DynamicClient,
-			"manual",
-			violationContext,
-		)
+		err = CreateAnsibleJob(policyAutomation, r.DynamicClient, "manual", violationContext)
 		if err != nil {
 			log.Error(err, "Failed to create the Ansible job", "mode", "manual")
 
 			return reconcile.Result{}, err
 		}
-		// manual run succeeded, remove annotation
+
+		// Manual run succeeded, remove annotation
 		delete(policyAutomation.Annotations, "policy.open-cluster-management.io/rerun")
 
 		err = r.Update(ctx, policyAutomation, &client.UpdateOptions{})
@@ -345,19 +339,17 @@ func (r *PolicyAutomationReconciler) Reconcile(
 		}
 
 		return reconcile.Result{}, nil
-	} else if policyAutomation.Spec.Mode == policyv1beta1.Disabled {
-		log.Info("Automation is disabled, doing nothing")
 
-		return reconcile.Result{}, nil
-	} else {
-		if policy.Spec.Disabled {
-			log.Info("The policy is disabled. Doing nothing.")
+	case policy.Spec.Disabled:
+		log.Info("The policy is disabled. Doing nothing.")
 
-			return reconcile.Result{}, nil
-		}
+		return reconcile.Result{}, nil
 
-		if policyAutomation.Spec.Mode == "scan" {
+	default:
+		switch policyAutomation.Spec.Mode {
+		case "scan":
 			log := log.WithValues("mode", "scan")
+
 			log.V(2).Info("Triggering scan mode")
 
 			requeueAfter, err := time.ParseDuration(policyAutomation.Spec.RescanAfter)
@@ -373,45 +365,42 @@ func (r *PolicyAutomationReconciler) Reconcile(
 			if len(targetList) > 0 {
 				log.Info("Creating An Ansible job", "targetList", targetList)
 				violationContext, _ := r.getViolationContext(ctx, policy, targetList, policyAutomation)
-				err = CreateAnsibleJob(policyAutomation, r.DynamicClient, "scan",
-					violationContext)
+
+				err = CreateAnsibleJob(policyAutomation, r.DynamicClient, "scan", violationContext)
 				if err != nil {
 					return reconcile.Result{RequeueAfter: requeueAfter}, err
 				}
 			} else {
 				log.Info("All clusters are compliant. Doing nothing.")
 			}
-
 			// no violations found, doing nothing
 			r.counter++
-			log.V(2).Info(
-				"RequeueAfter.", "RequeueAfter", requeueAfter.String(), "Counter", fmt.Sprintf("%d", r.counter),
-			)
+
+			log.V(2).Info("RequeueAfter.", "RequeueAfter", requeueAfter.String(), "Counter", strconv.Itoa(r.counter))
 
 			return reconcile.Result{RequeueAfter: requeueAfter}, nil
-		} else if policyAutomation.Spec.Mode == policyv1beta1.Once {
+
+		case policyv1beta1.Once:
 			log := log.WithValues("mode", string(policyv1beta1.Once))
 			targetList := common.FindNonCompliantClustersForPolicy(policy)
+
 			if len(targetList) > 0 {
 				log.Info("Creating an Ansible job", "targetList", targetList)
 
-				AjExist, err := MatchPAGeneration(policyAutomation,
-					r.DynamicClient, policyAutomation.GetGeneration())
+				AjExist, err := MatchPAGeneration(policyAutomation, r.DynamicClient, policyAutomation.GetGeneration())
 				if err != nil {
 					log.Error(err, "Failed to get Ansible job's generation")
 
 					return reconcile.Result{}, err
 				}
+
 				if AjExist {
 					return reconcile.Result{}, nil
 				}
+
 				violationContext, _ := r.getViolationContext(ctx, policy, targetList, policyAutomation)
-				err = CreateAnsibleJob(
-					policyAutomation,
-					r.DynamicClient,
-					string(policyv1beta1.Once),
-					violationContext,
-				)
+
+				err = CreateAnsibleJob(policyAutomation, r.DynamicClient, string(policyv1beta1.Once), violationContext)
 				if err != nil {
 					log.Error(err, "Failed to create the Ansible job")
 
@@ -429,7 +418,8 @@ func (r *PolicyAutomationReconciler) Reconcile(
 			} else {
 				log.Info("All clusters are compliant. Doing nothing.")
 			}
-		} else if policyAutomation.Spec.Mode == policyv1beta1.EveryEvent {
+
+		case policyv1beta1.EveryEvent:
 			log := log.WithValues("mode", string(policyv1beta1.EveryEvent))
 			targetList := common.FindNonCompliantClustersForPolicy(policy)
 			targetListMap := getTargetListMap(targetList)
@@ -441,6 +431,7 @@ func (r *PolicyAutomationReconciler) Reconcile(
 			requeueFlag := false
 			// Automation event time grouped by the cluster name
 			eventMap := map[string]policyv1beta1.ClusterEvent{}
+
 			if len(policyAutomation.Status.ClustersWithEvent) > 0 {
 				eventMap = policyAutomation.Status.ClustersWithEvent
 			}
@@ -460,7 +451,6 @@ func (r *PolicyAutomationReconciler) Reconcile(
 					log.Error(err, "Failed to retrieve EventTime in ClustersWithEvent")
 					delete(eventMap, clusterName)
 				}
-
 				// The time that delayAfterRunSeconds setting expires
 				delayUntil := originalStartTime.Add(time.Duration(delayAfterRunSeconds) * time.Second)
 
@@ -479,22 +469,23 @@ func (r *PolicyAutomationReconciler) Reconcile(
 						} else {
 							requeueFlag = true
 							// Within the delay period and use the earliest requeueDuration to requeue
-							if (requeueDuration == 0) || (requeueDuration > int(delayUntil.Sub(now)+1)) {
+							if requeueDuration == 0 || requeueDuration > int(delayUntil.Sub(now)+1) {
 								requeueDuration = int(delayUntil.Sub(now) + 1)
 							}
 							// keep the event and update eventTime
 							clusterEvent.EventTime = nowStr
-							// new event from compliant to non-compliant
 							eventMap[clusterName] = clusterEvent
 						}
-					} // Otherwise, the policy keeps non-compliant since originalStartTime, do nothing
-				} else { // The policy is compliant with the target cluster
+					}
+				} else {
+					// Otherwise, the policy keeps non-compliant since originalStartTime, do nothing
+					// The policy is compliant with the target cluster
 					if delayAfterRunSeconds > 0 && now.Before(delayUntil) {
 						// Within the delay period, keep the event and update eventTime
 						clusterEvent.EventTime = nowStr
-						// new event from non-compliant to compliant
 						eventMap[clusterName] = clusterEvent
-					} else { // No delay period or it is expired, remove the event
+					} else {
+						// No delay period or it is expired, remove the event
 						delete(eventMap, clusterName)
 					}
 				}
@@ -514,14 +505,12 @@ func (r *PolicyAutomationReconciler) Reconcile(
 				for clusterName := range trimmedTargetMap {
 					trimmedTargetList = append(trimmedTargetList, clusterName)
 				}
+
 				log.Info("Creating An Ansible job", "trimmedTargetList", trimmedTargetList)
 				violationContext, _ := r.getViolationContext(ctx, policy, trimmedTargetList, policyAutomation)
-				err = CreateAnsibleJob(
-					policyAutomation,
-					r.DynamicClient,
-					string(policyv1beta1.EveryEvent),
-					violationContext,
-				)
+
+				err = CreateAnsibleJob(policyAutomation, r.DynamicClient,
+					string(policyv1beta1.EveryEvent), violationContext)
 				if err != nil {
 					log.Error(err, "Failed to create the Ansible job")
 
@@ -542,22 +531,28 @@ func (r *PolicyAutomationReconciler) Reconcile(
 
 			policyAutomation.Status.ClustersWithEvent = eventMap
 			// use StatusWriter to update status subresource of a Kubernetes object
-			err = r.Status().Update(ctx, policyAutomation)
+			err := r.Status().Update(ctx, policyAutomation)
 			if err != nil {
 				log.Error(err, "Failed to update ClustersWithEvent in policyAutomation status")
 
 				return reconcile.Result{}, err
 			}
 
 			if requeueFlag {
-				log.Info(
-					"Requeue for the new non-compliant event during the delay period",
-					"Delay in seconds", delayAfterRunSeconds,
-					"Requeue After", requeueDuration,
-				)
+				log.Info("Requeue for the new non-compliant event during the delay period", "Delay in seconds",
+					delayAfterRunSeconds, "Requeue After", requeueDuration)
 
 				return reconcile.Result{RequeueAfter: time.Duration(requeueDuration)}, nil
 			}
+
+		case policyv1beta1.Disabled:
+			log.Info("Automation is disabled, doing nothing")
+
+			return reconcile.Result{}, nil
+		default:
+			log.Info("Unknown mode. No action taken.")
+
+			return reconcile.Result{}, nil
 		}
 	}
 

controllers/common/common_test.go

@@ -30,9 +30,11 @@ func TestParseRootPolicyLabel(t *testing.T) {
 			if (err != nil) != expected.shouldErr {
 				t.Fatal("expected error, got nil")
 			}
+
 			if name != expected.name {
 				t.Fatalf("expected name '%v', got '%v'", expected.name, name)
 			}
+
 			if namespace != expected.namespace {
 				t.Fatalf("expected namespace '%v', got '%v'", expected.namespace, namespace)
 			}

controllers/common/handler.go

@@ -59,4 +59,4 @@ func (e *EnqueueRequestsFromMapFunc) mapAndEnqueue(
 	}
 }
 
-var NeverEnqueue = predicate.NewPredicateFuncs(func(o client.Object) bool { return false })
+var NeverEnqueue = predicate.NewPredicateFuncs(func(_ client.Object) bool { return false })