NO-JIRA: (refactor) reduce cognitive and adding unit tests (#889)

* refactor: reduce configobserver config cognitive

* refactor: conditions checks and tests

* test: testing conditions

* style: adjust code style

Author: Ricardo Lüders

pkg/config/configobserver/config.go

@@ -76,52 +76,12 @@ func (c *Config) loadHTTP(data map[string][]byte) {
 }
 
 func (c *Config) loadReport(data map[string][]byte) {
-	if enableGlobalObfuscation, ok := data["enableGlobalObfuscation"]; ok {
-		c.EnableGlobalObfuscation = strings.EqualFold(string(enableGlobalObfuscation), "true")
-	}
-
-	if reportEndpoint, ok := data["reportEndpoint"]; ok {
-		c.ReportEndpoint = string(reportEndpoint)
-	}
-	if reportPullingDelay, ok := data["reportPullingDelay"]; ok {
-		if v, err := time.ParseDuration(string(reportPullingDelay)); err == nil {
-			c.ReportPullingDelay = v
-		} else {
-			klog.Warningf(
-				"reportPullingDelay secret contains an invalid value (%s). Using previous value",
-				reportPullingDelay,
-			)
-		}
-	} else {
-		c.ReportPullingDelay = time.Duration(-1)
-	}
-
-	if reportPullingTimeout, ok := data["reportPullingTimeout"]; ok {
-		if v, err := time.ParseDuration(string(reportPullingTimeout)); err == nil {
-			c.ReportPullingTimeout = v
-		} else {
-			klog.Warningf(
-				"reportPullingTimeout secret contains an invalid value (%s). Using previous value",
-				reportPullingTimeout,
-			)
-		}
-	}
-
-	if reportMinRetryTime, ok := data["reportMinRetryTime"]; ok {
-		if v, err := time.ParseDuration(string(reportMinRetryTime)); err == nil {
-			c.ReportMinRetryTime = v
-		} else {
-			klog.Warningf(
-				"reportMinRetryTime secret contains an invalid value (%s). Using previous value",
-				reportMinRetryTime,
-			)
-		}
-	}
-
-	c.Report = len(c.Endpoint) > 0
-	if disableInsightsAlerts, ok := data["disableInsightsAlerts"]; ok {
-		c.DisableInsightsAlerts = strings.EqualFold(string(disableInsightsAlerts), "true")
-	}
+	c.loadEnableGlobalObfuscation(data)
+	c.loadReportEndpoint(data)
+	c.loadReportPullingDelay(data)
+	c.loadReportPullingTimeout(data)
+	c.loadReportMinRetryTime(data)
+	c.loadReportSettings(data)
 }
 
 func (c *Config) loadOCM(data map[string][]byte) {
@@ -168,3 +128,48 @@ func (c *Config) loadReportEndpointTechPreview(data map[string][]byte) {
 		c.ReportEndpointTechPreview = string(endpoint)
 	}
 }
+
+func (c *Config) loadEnableGlobalObfuscation(data map[string][]byte) {
+	if enableGlobalObfuscation, ok := data["enableGlobalObfuscation"]; ok {
+		c.EnableGlobalObfuscation = strings.EqualFold(string(enableGlobalObfuscation), "true")
+	}
+}
+
+func (c *Config) loadReportEndpoint(data map[string][]byte) {
+	if reportEndpoint, ok := data["reportEndpoint"]; ok {
+		c.ReportEndpoint = string(reportEndpoint)
+	}
+}
+
+func (c *Config) loadReportPullingDelay(data map[string][]byte) {
+	c.loadReportDurationSetting(data, "reportPullingDelay", &c.ReportPullingDelay)
+}
+
+func (c *Config) loadReportPullingTimeout(data map[string][]byte) {
+	c.loadReportDurationSetting(data, "reportPullingTimeout", &c.ReportPullingTimeout)
+}
+
+func (c *Config) loadReportMinRetryTime(data map[string][]byte) {
+	c.loadReportDurationSetting(data, "reportMinRetryTime", &c.ReportMinRetryTime)
+}
+
+func (c *Config) loadReportDurationSetting(data map[string][]byte, key string, target *time.Duration) {
+	if value, ok := data[key]; ok {
+		if v, err := time.ParseDuration(string(value)); err == nil {
+			*target = v
+		} else {
+			klog.Warningf("%s secret contains an invalid value (%s). Using previous value", key, value)
+		}
+	} else {
+		if key == "reportPullingDelay" {
+			*target = time.Duration(-1)
+		}
+	}
+}
+
+func (c *Config) loadReportSettings(data map[string][]byte) {
+	c.Report = len(c.Endpoint) > 0
+	if disableInsightsAlerts, ok := data["disableInsightsAlerts"]; ok {
+		c.DisableInsightsAlerts = strings.EqualFold(string(disableInsightsAlerts), "true")
+	}
+}

pkg/gatherers/conditional/conditions.go

@@ -53,22 +53,15 @@ type ClusterVersionMatchesConditionParams struct {
 // areAllConditionsSatisfied returns true if all the conditions are satisfied, for example if the condition is
 // to check if a metric is firing, it will look at that metric and return the result according to that
 func (g *Gatherer) areAllConditionsSatisfied(conditions []ConditionWithParams) (bool, error) {
-	for _, condition := range conditions {
+	for k := range conditions {
+		condition := conditions[k]
 		switch condition.Type {
 		case AlertIsFiring:
-			if condition.Alert == nil {
-				return false, fmt.Errorf("alert field should not be nil")
-			}
-
-			if firing, err := g.isAlertFiring(condition.Alert.Name); !firing || err != nil {
+			if ok, err := g.checkAlertIsFiring(condition.Alert); !ok || err != nil {
 				return false, err
 			}
 		case ClusterVersionMatches:
-			if condition.ClusterVersionMatches == nil {
-				return false, fmt.Errorf("cluster_version_matches field should not be nil")
-			}
-
-			if doesMatch, err := g.doesClusterVersionMatch(condition.ClusterVersionMatches.Version); !doesMatch || err != nil {
+			if ok, err := g.checkClusterVersionMatches(condition.ClusterVersionMatches); !ok || err != nil {
 				return false, err
 			}
 		default:
@@ -79,6 +72,26 @@ func (g *Gatherer) areAllConditionsSatisfied(conditions []ConditionWithParams) (
 	return true, nil
 }
 
+// checkAlertIsFiring verifies whether an alert is currently in a firing state. This function is used to
+// evaluate the AlertIsFiring condition.
+func (g *Gatherer) checkAlertIsFiring(alert *AlertConditionParams) (bool, error) {
+	if alert == nil {
+		return false, fmt.Errorf("alert field should not be nil")
+	}
+
+	return g.isAlertFiring(alert.Name)
+}
+
+// checkClusterVersionMatches verifies if the OpenShift cluster version matches the specified criteria. This function
+// is employed to evaluate the ClusterVersionMatches condition.
+func (g *Gatherer) checkClusterVersionMatches(clusterVersion *ClusterVersionMatchesConditionParams) (bool, error) {
+	if clusterVersion == nil {
+		return false, fmt.Errorf("cluster_version_matches field should not be nil")
+	}
+
+	return g.doesClusterVersionMatch(clusterVersion.Version)
+}
+
 // isAlertFiring using the cache it returns true if the alert is firing
 func (g *Gatherer) isAlertFiring(alertName string) (bool, error) {
 	if g.firingAlerts == nil {