Adjust CompliancyDetails array properly

- Clear compliancyDetails on policy-wide errors
- Reduce the length of the array if the number of
  object-templates is lower

ref: https://issues.redhat.com/browse/ACM-5512
Signed-off-by: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com>

controllers/configurationpolicy_controller.go

@@ -466,7 +466,7 @@ func (r *ConfigurationPolicyReconciler) getObjectTemplateDetails(
 				reason := "namespaceSelector error"
 				msg := fmt.Sprintf(
 					"%s: %s", errMsg, err.Error())
-				statusChanged := addConditionToStatus(&plc, 0, false, reason, msg)
+				statusChanged := addConditionToStatus(&plc, -1, false, reason, msg)
 				if statusChanged {
 					r.Recorder.Event(
 						&plc,
@@ -682,7 +682,7 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
 	if validationErr != "" {
 		message := validationErr
 		log.Info(message)
-		statusChanged := addConditionToStatus(&plc, 0, false, "Invalid spec", message)
+		statusChanged := addConditionToStatus(&plc, -1, false, "Invalid spec", message)
 
 		if statusChanged {
 			r.Recorder.Event(&plc, eventWarning,
@@ -768,7 +768,7 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
 
 				statusChanged := addConditionToStatus(
 					&plc,
-					0,
+					-1,
 					false,
 					reasonCleanupError,
 					"Failed to delete objects: "+strings.Join(failures, ", "))
@@ -808,7 +808,7 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
 			reason = "Error processing template"
 		}
 
-		statusChanged := addConditionToStatus(&plc, 0, false, reason, msg)
+		statusChanged := addConditionToStatus(&plc, -1, false, reason, msg)
 		if statusChanged {
 			parentStatusUpdateNeeded = true
 
@@ -1033,6 +1033,12 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
 
 	templateObjs, selectedNamespaces, objTmplStatusChangeNeeded, err = r.getObjectTemplateDetails(plc)
 
+	// Set the CompliancyDetails array length accordingly in case the number of
+	// object-templates was reduced (the status update will handle if it's longer)
+	if len(templateObjs) < len(plc.Status.CompliancyDetails) {
+		plc.Status.CompliancyDetails = plc.Status.CompliancyDetails[:len(templateObjs)]
+	}
+
 	if objTmplStatusChangeNeeded {
 		parentStatusUpdateNeeded = true
 	}
@@ -1050,7 +1056,7 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
 		msg := fmt.Sprintf("%v contains no object templates to check, and thus has no violations",
 			plc.GetName())
 
-		statusUpdateNeeded := addConditionToStatus(&plc, 0, true, reason, msg)
+		statusUpdateNeeded := addConditionToStatus(&plc, -1, true, reason, msg)
 
 		if statusUpdateNeeded {
 			eventType := eventNormal
@@ -1288,6 +1294,7 @@ func (r *ConfigurationPolicyReconciler) deleteDetachedObj(plc policyv1.Configura
 }
 
 // helper function that appends a condition (violation or compliant) to the status of a configurationpolicy
+// Set the index to -1 to signal that the status should be cleared.
 func addConditionToStatus(
 	plc *policyv1.ConfigurationPolicy, index int, compliant bool, reason string, message string,
 ) (updateNeeded bool) {
@@ -1324,6 +1331,15 @@ func addConditionToStatus(
 		cond.Message += fmt.Sprintf(". %s.", msg)
 	}
 
+	// Set a boolean to clear the details array if the index is -1, but set
+	// the index to zero to determine whether a status update is required
+	clearStatus := false
+
+	if index == -1 {
+		clearStatus = true
+		index = 0
+	}
+
 	if len(plc.Status.CompliancyDetails) <= index {
 		plc.Status.CompliancyDetails = append(plc.Status.CompliancyDetails, policyv1.TemplateStatus{
 			ComplianceState: complianceState,
@@ -1344,6 +1360,14 @@ func addConditionToStatus(
 		updateNeeded = true
 	}
 
+	// Clear the details array if the index provided was -1
+	if clearStatus {
+		plc.Status.CompliancyDetails = []policyv1.TemplateStatus{{
+			ComplianceState: complianceState,
+			Conditions:      []policyv1.Condition{},
+		}}
+	}
+
 	if updateNeeded {
 		log.Info("Will update the policy status")
 	}

test/e2e/case30_multiline_templatization_test.go

@@ -10,6 +10,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
 	"open-cluster-management.io/config-policy-controller/test/utils"
 )
@@ -110,17 +111,56 @@ var _ = Describe("Test multiline templatization", Ordered, func() {
 		})
 	})
 	Describe("Test invalid multiline templates", func() {
+		It("configmap should be created properly on the managed cluster", func() {
+			By("Creating config maps on managed")
+			utils.Kubectl("apply", "-f", case30ConfigMapsYaml, "-n", "default")
+			for _, cfgMapName := range []string{"30config1", "30config2"} {
+				cfgmap := utils.GetWithTimeout(clientManagedDynamic, gvrConfigMap,
+					cfgMapName, "default", true, defaultTimeoutSeconds)
+				Expect(cfgmap).NotTo(BeNil())
+			}
+		})
+		It("should create compliant policy", func() {
+			By("Creating policy with range template on managed")
+			utils.Kubectl("apply", "-f", case30RangePolicyYaml, "-n", testNamespace)
+			plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+				case30RangePolicyName, testNamespace, true, defaultTimeoutSeconds)
+			Expect(plc).NotTo(BeNil())
+
+			By("Verifying that the " + case30RangePolicyName + " policy is compliant")
+			Eventually(func(g Gomega) interface{} {
+				managedPlc := utils.GetWithTimeout(
+					clientManagedDynamic,
+					gvrConfigPolicy,
+					case30RangePolicyName,
+					testNamespace,
+					true,
+					defaultTimeoutSeconds,
+				)
+
+				details, _, err := unstructured.NestedSlice(managedPlc.Object, "status", "compliancyDetails")
+				g.Expect(err).ToNot(HaveOccurred())
+				g.Expect(details).To(HaveLen(2))
+
+				return utils.GetComplianceState(managedPlc)
+			}, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
+		})
+
 		It("should generate noncompliant for invalid template strings", func() {
 			By("Creating policies on managed")
 			// create policy with unterminated template
 			utils.Kubectl("apply", "-f", case30UnterminatedYaml, "-n", testNamespace)
 			plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
 				case30Unterminated, testNamespace, true, defaultTimeoutSeconds)
 			Expect(plc).NotTo(BeNil())
-			Eventually(func() interface{} {
+			Eventually(func(g Gomega) interface{} {
 				managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
 					case30Unterminated, testNamespace, true, defaultTimeoutSeconds)
 
+				details, _, err := unstructured.NestedSlice(managedPlc.Object, "status", "compliancyDetails")
+				g.Expect(err).ToNot(HaveOccurred())
+				g.Expect(details).To(HaveLen(1))
+
 				return utils.GetComplianceState(managedPlc)
 			}, defaultTimeoutSeconds, 1).Should(Equal("NonCompliant"))
 			Eventually(func() interface{} {

test/resources/case30_multiline_templatization/case30_configmaps.yaml

@@ -2,12 +2,16 @@ apiVersion: v1
 kind: ConfigMap
 metadata:
   name: 30config1
+  labels:
+    testcase: "30"
 data:
   name: testvalue1
 ---
 apiVersion: v1
 kind: ConfigMap
 metadata:
   name: 30config2
+  labels:
+    testcase: "30"
 data:
   name: testvalue2

test/resources/case30_multiline_templatization/case30_policy.yaml

@@ -6,7 +6,7 @@ spec:
   remediationAction: enforce
   severity: low
   object-templates-raw: |
-    {{ range (lookup "v1" "ConfigMap" "default" "").items }}
+    {{ range (lookup "v1" "ConfigMap" "default" "" "testcase=30").items }}
       - complianceType: musthave
         objectDefinition:
           apiVersion: v1