[FIX] recent sonar issues (#137)

CHANGES: - refactor version monitoring related methods to reduce cognitive complexity
SAP · Sep 25, 2024 · de12fe7 · de12fe7
1 parent 25b3460
commit de12fe7
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 47 deletions.
diff --git a/internal/controller/reconcile-capapplicationversion.go b/internal/controller/reconcile-capapplicationversion.go
@@ -466,32 +466,27 @@ func (c *Controller) updateServiceMonitors(ctx context.Context, ca *v1alpha1.CAP
 		return nil
 	}
 
+	isWorkloadPort := func(wlPorts []corev1.ServicePort, scrapePort string) bool {
+		for j := range wlPorts {
+			if wlPorts[j].Name == scrapePort {
+				return true
+			}
+		}
+		return false
+	}
+
 	for i := range cav.Spec.Workloads {
 		wl := cav.Spec.Workloads[i]
 		if wl.DeploymentDefinition == nil || wl.DeploymentDefinition.Monitoring == nil || wl.DeploymentDefinition.Monitoring.ScrapeConfig == nil {
 			continue // do not reconcile service monitors
 		}
 
-		var wlPortInfos *servicePortInfo
-		for j := range workloadServicePortInfos {
-			item := workloadServicePortInfos[j]
-			if item.WorkloadName == getWorkloadName(cav.Name, wl.Name) {
-				wlPortInfos = &item
-				break
-			}
-		}
+		wlPortInfos := getServicePortInfoByWorkloadName(workloadServicePortInfos, cav.Name, wl.Name)
 		if wlPortInfos == nil {
 			return fmt.Errorf("could not identify workload port information for workload %s in version %s", wl.Name, cav.Name)
 		}
 
-		portVerified := false
-		for j := range wlPortInfos.Ports {
-			if wlPortInfos.Ports[j].Name == wl.DeploymentDefinition.Monitoring.ScrapeConfig.WorkloadPort {
-				portVerified = true
-				break
-			}
-		}
-		if !portVerified {
+		if portVerified := isWorkloadPort(wlPortInfos.Ports, wl.DeploymentDefinition.Monitoring.ScrapeConfig.WorkloadPort); !portVerified {
 			return fmt.Errorf("invalid port reference in workload %s monitoring config of version %s", wl.Name, cav.Name)
 		}
 

diff --git a/internal/controller/reconcile.go b/internal/controller/reconcile.go
@@ -560,6 +560,16 @@ func updateWorkloadPortInfo(cavName string, workloadName string, deploymentType
 	return workloadPortInfo
 }
 
+func getServicePortInfoByWorkloadName(items []servicePortInfo, cavName string, workloadName string) *servicePortInfo {
+	for i := range items {
+		current := items[i]
+		if current.WorkloadName == getWorkloadName(cavName, workloadName) {
+			return &current
+		}
+	}
+	return nil
+}
+
 func (c *Controller) getRouterServicePortInfo(cavName string, namespace string) (*servicePortInfo, error) {
 	cav, err := c.crdInformerFactory.Sme().V1alpha1().CAPApplicationVersions().Lister().CAPApplicationVersions(namespace).Get(cavName)
 	if err != nil {

diff --git a/internal/controller/version-monitoring.go b/internal/controller/version-monitoring.go
@@ -274,36 +274,7 @@ func (c *Controller) evaluateVersionForCleanup(ctx context.Context, item Namespa
 	cleanup := true
 	for i := range cav.Spec.Workloads {
 		wl := cav.Spec.Workloads[i]
-		workloadEvaluation := true
-		if wl.DeploymentDefinition != nil && wl.DeploymentDefinition.Monitoring != nil && wl.DeploymentDefinition.Monitoring.DeletionRules != nil {
-			if wl.DeploymentDefinition.Monitoring.DeletionRules.ScalarExpression != nil { // evaluate provided expression
-				expr := strings.TrimSpace(*wl.DeploymentDefinition.Monitoring.DeletionRules.ScalarExpression)
-				if expr == "" {
-					workloadEvaluation = false
-				} else {
-					isRelevantForCleanup, err := evaluateExpression(ctx, expr, promapi)
-					if err != nil || !isRelevantForCleanup {
-						if err != nil {
-							klog.ErrorS(err, "could not evaluate PromQL expression for workload", "workload", wl.Name, "version", cav.Name)
-						}
-						workloadEvaluation = false
-					}
-				}
-			} else {
-				for j := range wl.DeploymentDefinition.Monitoring.DeletionRules.Metrics {
-					rule := wl.DeploymentDefinition.Monitoring.DeletionRules.Metrics[j]
-					isRelevantForCleanup, err := evaluateMetric(ctx, &rule, fmt.Sprintf("%s%s", getWorkloadName(cav.Name, wl.Name), ServiceSuffix), cav.Namespace, promapi)
-					if err != nil || !isRelevantForCleanup {
-						if err != nil {
-							klog.ErrorS(err, "could not evaluate metric for workload", "workload", wl.Name, "version", cav.Name)
-						}
-						workloadEvaluation = false
-						break
-					}
-				}
-			}
-		}
-		if !workloadEvaluation {
+		if workloadEvaluation := evaluateWorkloadForCleanup(ctx, item, &wl, promapi); !workloadEvaluation {
 			cleanup = false
 			break
 		}
@@ -321,6 +292,35 @@ func (c *Controller) evaluateVersionForCleanup(ctx context.Context, item Namespa
 	return nil
 }
 
+func evaluateWorkloadForCleanup(ctx context.Context, cav NamespacedResourceKey, wl *v1alpha1.WorkloadDetails, promapi promv1.API) bool {
+	if wl.DeploymentDefinition == nil || wl.DeploymentDefinition.Monitoring == nil || wl.DeploymentDefinition.Monitoring.DeletionRules == nil {
+		return true // if there are no rules - the workload is automatically eligible for cleanup
+	}
+
+	if wl.DeploymentDefinition.Monitoring.DeletionRules.ScalarExpression != nil { // evaluate provided expression
+		isRelevantForCleanup, err := evaluateExpression(ctx, *wl.DeploymentDefinition.Monitoring.DeletionRules.ScalarExpression, promapi)
+		if err != nil {
+			klog.ErrorS(err, "could not evaluate PromQL expression for workload", "workload", wl.Name, "version", cav.Name)
+			return false
+		}
+		return isRelevantForCleanup
+	}
+
+	// evaluate rules based on metric type
+	for j := range wl.DeploymentDefinition.Monitoring.DeletionRules.Metrics {
+		rule := wl.DeploymentDefinition.Monitoring.DeletionRules.Metrics[j]
+		isRelevantForCleanup, err := evaluateMetric(ctx, &rule, fmt.Sprintf("%s%s", getWorkloadName(cav.Name, wl.Name), ServiceSuffix), cav.Namespace, promapi)
+		if err != nil {
+			klog.ErrorS(err, "could not evaluate metric for workload", "workload", wl.Name, "version", cav.Name)
+			return false
+		}
+		if !isRelevantForCleanup {
+			return false
+		}
+	}
+	return true
+}
+
 func executePromQL(ctx context.Context, promapi promv1.API, query string) (prommodel.Value, error) {
 	// klog.InfoS("executing prometheus query", "query", query)
 	result, warnings, err := promapi.Query(ctx, query, time.Now())
@@ -335,7 +335,12 @@ func executePromQL(ctx context.Context, promapi promv1.API, query string) (promm
 	return result, nil
 }
 
-func evaluateExpression(ctx context.Context, expr string, promapi promv1.API) (bool, error) {
+func evaluateExpression(ctx context.Context, rawExpr string, promapi promv1.API) (bool, error) {
+	expr := strings.TrimSpace(rawExpr)
+	if expr == "" {
+		return false, fmt.Errorf("encountered empty expression")
+	}
+
 	result, err := executePromQL(ctx, promapi, expr)
 	if err != nil {
 		return false, err