Clear compliancyDetails on template error

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 }}

test/utils/utils.go

@@ -59,7 +59,7 @@ func GetClusterLevelWithTimeout(
  }
  var obj *unstructured.Unstructured
 
- Eventually(func() error {
+ EventuallyWithOffset(1, func() error {
  var err error
  namespace := clientHubDynamic.Resource(gvr)
  obj, err = namespace.Get(context.TODO(), name, metav1.GetOptions{})
@@ -74,7 +74,7 @@ func GetClusterLevelWithTimeout(
  }
 
  return nil
- }, timeout, 1).Should(BeNil())
+ }, timeout, 1).ShouldNot(HaveOccurred())
 
  if wantFound {
  return obj
@@ -97,7 +97,7 @@ func GetWithTimeout(
  }
  var obj *unstructured.Unstructured
 
- Eventually(func() error {
+ EventuallyWithOffset(1, func() error {
  var err error
  namespace := clientHubDynamic.Resource(gvr).Namespace(namespace)
  obj, err = namespace.Get(context.TODO(), name, metav1.GetOptions{})
@@ -112,7 +112,7 @@ func GetWithTimeout(
  }
 
  return nil
- }, timeout, 1).Should(BeNil())
+ }, timeout, 1).ShouldNot(HaveOccurred())
 
  if wantFound {
  return obj
@@ -136,17 +136,13 @@ func ListWithTimeout(
  }
  var list *unstructured.UnstructuredList
 
- Eventually(func() error {
+ EventuallyWithOffset(1, func(g Gomega) []unstructured.Unstructured {
  var err error
  list, err = clientHubDynamic.Resource(gvr).List(context.TODO(), opts)
- if err != nil {
- return err
- } else if len(list.Items) != size {
- return fmt.Errorf("list size doesn't match, expected %d actual %d", size, len(list.Items))
- }
+ g.Expect(err).ToNot(HaveOccurred())
 
- return nil
- }, timeout, 1).Should(BeNil())
+ return list.Items
+ }, timeout, 1).Should(HaveLen(size))
 
  if wantFound {
  return list
@@ -160,12 +156,12 @@ func GetMatchingEvents(
 ) []corev1.Event {
  var eventList *corev1.EventList
 
- Eventually(func() error {
+ EventuallyWithOffset(1, func() error {
  var err error
  eventList, err = client.CoreV1().Events(namespace).List(context.TODO(), metav1.ListOptions{})
 
  return err
- }, timeout, 1).Should(BeNil())
+ }, timeout, 1).ShouldNot(HaveOccurred())
 
  matchingEvents := make([]corev1.Event, 0)
  msgMatcher := regexp.MustCompile(msgRegex)
@@ -331,10 +327,9 @@ func DoConfigPolicyMessageTest(clientHubDynamic dynamic.Interface,
  gvrConfigPolicy schema.GroupVersionResource, namespace string, policyName string,
  templateIdx int, timeout int, expectedMsg string,
 ) {
- Eventually(func(g Gomega) string {
+ EventuallyWithOffset(1, func(g Gomega) string {
  message, _, err := getConfigPolicyStatusMessages(clientHubDynamic,
  gvrConfigPolicy, namespace, policyName, templateIdx)
-
  g.Expect(err).ShouldNot(HaveOccurred())
 
  return message