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,11 @@ func addConditionToStatus(
  updateNeeded = true
  }
 
+ // Clear the details array if the index provided was -1
+ if clearStatus {
+ plc.Status.CompliancyDetails = plc.Status.CompliancyDetails[0:1]
+ }
+
  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"
 )
@@ -23,7 +24,7 @@ const (
 )
 
 const (
- case30Unterminated string = "policy-pod-create-unterminated"
+ case30Unterminated string = "case30-configpolicy"
  case30UnterminatedYaml string = "../resources/case30_multiline_templatization/case30_unterminated.yaml"
  case30WrongArgs string = "policy-pod-create-wrong-args"
  case30WrongArgsYaml string = "../resources/case30_multiline_templatization/case30_wrong_args.yaml"
@@ -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

test/resources/case30_multiline_templatization/case30_unterminated.yaml

@@ -1,22 +1,21 @@
 apiVersion: policy.open-cluster-management.io/v1
 kind: ConfigurationPolicy
 metadata:
- name: policy-pod-create-unterminated
+ name: case30-configpolicy
 spec:
  remediationAction: enforce
  namespaceSelector:
  exclude: ["kube-*"]
  include: ["default"]
- object-templates-raw: |
- - complianceType: musthave
- objectDefinition:
- apiVersion: v1
- kind: Pod
- metadata:
- name: '{{'
- spec:
- containers:
- - image: nginx:1.7.9
- name: nginx
- ports:
- - containerPort: 80
+ object-templates-raw: |
+ {{ range (lookup "v1" "ConfigMap" "default" "" "testcase=30").items }}
+ - complianceType: musthave
+ objectDefinition:
+ apiVersion: v1
+ kind: ConfigMap
+ metadata:
+ name: '{{'
+ namespace: default
+ data:
+ extraData: exists!
+ {{ end }}