Bug: ConfigurationPolicy message for enforce omits objects when multiple namespaces are specified

controllers/configurationpolicy_controller.go

@@ -1068,9 +1068,9 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
  nonCompliantObjects := map[string]map[string]interface{}{}
  compliantObjects := map[string]map[string]interface{}{}
  enforce := strings.EqualFold(string(plc.Spec.RemediationAction), string(policyv1.Enforce))
- kind := ""
+ kind := templateObjs[indx].kind
  objShouldExist := !strings.EqualFold(string(objectT.ComplianceType), string(policyv1.MustNotHave))
-
+ mergeMessageEnforce := false
  // If the object does not have a namespace specified, use the previously retrieved namespaces
  // from the NamespaceSelector. If no namespaces are found/specified, use the value from the
  // object so that the objectTemplate is processed:
@@ -1084,10 +1084,13 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
  relevantNamespaces = []string{templateObjs[indx].namespace}
  }
 
+ if enforce && len(relevantNamespaces) > 1 {
+ mergeMessageEnforce = true
+ }
+
  numCompliant := 0
  numNonCompliant := 0
  handled := false
-
  // iterate through all namespaces the configurationpolicy is set on
  for _, ns := range relevantNamespaces {
  log.Info(
@@ -1106,20 +1109,33 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
  }
 
  if objKind != "" {
+ // object template enforced, objKind is empty
  kind = objKind
  }
 
+ if mergeMessageEnforce {
+ names = []string{templateObjs[indx].name}
+ }
+
+ // object template enforced, already handled in handleObjects
  if names == nil {
- // object template enforced, already handled in handleObjects
  handled = true
  } else {
+ // when multiple messages and enforce mode, it doesn't mean remediactionAction is changed
  enforce = false
+ }
+
+ // violations for enforce configurationpolicies are already handled in handleObjects,
+ // so we only need to generate a violation if the remediationAction is set to inform
+ // Or multiple namespaces and enforce use this
+ if !handled && !enforce {
  if !compliant {
  if len(names) == 0 {
  numNonCompliant++
  } else {
  numNonCompliant += len(names)
  }
+
  nonCompliantObjects[ns] = map[string]interface{}{
  "names": names,
  "reason": reason,
@@ -1137,8 +1153,7 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
  relatedObjects = updateRelatedObjectsStatus(relatedObjects, object)
  }
  }
- // violations for enforce configurationpolicies are already handled in handleObjects,
- // so we only need to generate a violation if the remediationAction is set to inform
+
  if !handled && !enforce {
  objData := templateIdentifier{
  index: indx,
@@ -1147,9 +1162,10 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
  namespaced: templateObjs[indx].isNamespaced,
  }
 
- statusUpdateNeeded := createInformStatus(
+ statusUpdateNeeded := createMergedStatus(
  objShouldExist, numCompliant, numNonCompliant, compliantObjects, nonCompliantObjects, &plc, objData,
  )
+
  if statusUpdateNeeded {
  parentStatusUpdateNeeded = true
  }
@@ -1342,9 +1358,10 @@ type templateIdentifier struct {
  namespaced bool
 }
 
-// createInformStatus updates the status field for a configurationpolicy with remediationAction=inform
+// createMergedStatus updates the status field for a configurationpolicy with remediationAction=inform
 // based on how many compliant/noncompliant objects are found when processing the templates in the configurationpolicy
-func createInformStatus(
+// Or multiple namespaces and enforce use this
+func createMergedStatus(
  objShouldExist bool,
  numCompliant,
  numNonCompliant int,

controllers/configurationpolicy_controller_test.go

@@ -437,7 +437,7 @@ func TestSortRelatedObjectsAndUpdate(t *testing.T) {
  assert.True(t, relatedList[0].Object.Metadata.Name == "bar")
 }
 
-func TestCreateInformStatus(t *testing.T) {
+func TestCreateMergedStatus(t *testing.T) {
  policy := &policyv1.ConfigurationPolicy{
  ObjectMeta: metav1.ObjectMeta{
  Name: "foo",
@@ -475,7 +475,7 @@ func TestCreateInformStatus(t *testing.T) {
  }
 
  // Test 1 NonCompliant resource
- createInformStatus(!mustNotHave, numCompliant, numNonCompliant,
+ createMergedStatus(!mustNotHave, numCompliant, numNonCompliant,
  compliantObjects, nonCompliantObjects, policy, objData)
  assert.True(t, policy.Status.CompliancyDetails[0].ComplianceState == policyv1.NonCompliant)
 
@@ -486,7 +486,7 @@ func TestCreateInformStatus(t *testing.T) {
  numNonCompliant = 2
 
  // Test 2 NonCompliant resources
- createInformStatus(!mustNotHave, numCompliant, numNonCompliant,
+ createMergedStatus(!mustNotHave, numCompliant, numNonCompliant,
  compliantObjects, nonCompliantObjects, policy, objData)
  assert.True(t, policy.Status.CompliancyDetails[0].ComplianceState == policyv1.NonCompliant)
 
@@ -495,7 +495,7 @@ func TestCreateInformStatus(t *testing.T) {
 
  // Test 0 resources
  numNonCompliant = 0
- createInformStatus(!mustNotHave, numCompliant, numNonCompliant,
+ createMergedStatus(!mustNotHave, numCompliant, numNonCompliant,
  compliantObjects, nonCompliantObjects, policy, objData)
  assert.True(t, policy.Status.CompliancyDetails[0].ComplianceState == policyv1.NonCompliant)
 
@@ -511,7 +511,7 @@ func TestCreateInformStatus(t *testing.T) {
  numNonCompliant = 1
 
  // Test 1 compliant and 1 noncompliant resource NOTE: This use case is the new behavior change!
- createInformStatus(!mustNotHave, numCompliant, numNonCompliant,
+ createMergedStatus(!mustNotHave, numCompliant, numNonCompliant,
  compliantObjects, nonCompliantObjects, policy, objData)
  assert.True(t, policy.Status.CompliancyDetails[0].ComplianceState == policyv1.NonCompliant)
 
@@ -525,7 +525,7 @@ func TestCreateInformStatus(t *testing.T) {
  delete(nonCompliantObjects, "test2")
 
  // Test 2 compliant resources
- createInformStatus(!mustNotHave, numCompliant, numNonCompliant,
+ createMergedStatus(!mustNotHave, numCompliant, numNonCompliant,
  compliantObjects, nonCompliantObjects, policy, objData)
  assert.True(t, policy.Status.CompliancyDetails[0].ComplianceState == policyv1.Compliant)
 }

test/e2e/case5_multi_test.go

@@ -87,4 +87,134 @@ var _ = Describe("Test multiple obj template handling", func() {
  deleteConfigPolicies(policies)
  })
  })
+
+ Describe("Check messages when it is multiple namesapces and multiple obj-templates", Ordered, func() {
+ const (
+ case5MultiNamespace1 string = "n1"
+ case5MultiNamespace2 string = "n2"
+ case5MultiNamespace3 string = "n3"
+ case5MultiNSConfigPolicyName string = "policy-multi-namespace-enforce"
+ case5MultiNSInformConfigPolicyName string = "policy-multi-namespace-inform"
+ case5MultiObjNSConfigPolicyName string = "policy-pod-multi-obj-temp-enforce"
+ case5InformYaml string = "../resources/case5_multi/case5_multi_namespace_inform.yaml"
+ case5EnforceYaml string = "../resources/case5_multi/case5_multi_namespace_enforce.yaml"
+ case5MultiObjTmpYaml string = "../resources/case5_multi/case5_multi_obj_template_enforce.yaml"
+ case5MultiEnforceErrYaml string = "../resources/case5_multi/" +
+ "case5_multi_namespace_enforce_error.yaml"
+ case5KindMissPlcName string = "policy-multi-namespace-enforce-kind-missing"
+ case5KindNameMissPlcName string = "policy-multi-namespace-enforce-both-missing"
+ case5NameMissPlcName string = "policy-multi-namespace-enforce-name-missing"
+ )
+
+ BeforeAll(func() {
+ nss := []string{
+ case5MultiNamespace1,
+ case5MultiNamespace2,
+ case5MultiNamespace3,
+ }
+
+ for _, ns := range nss {
+ utils.Kubectl("create", "ns", ns)
+ }
+ })
+ It("Should handle when kind or name missing without any panic", func() {
+ utils.Kubectl("apply", "-f", case5MultiEnforceErrYaml)
+
+ By("Kind missing")
+ kindErrMsg := "Decoding error, please check your policy file! Aborting handling the " +
+ "object template at index [0] in policy `policy-multi-namespace-enforce-kind-missing` with " +
+ "error = `Object 'Kind' is missing in '{\"apiVersion\":\"v1\",\"metadata\":" +
+ "{\"name\":\"case5-multi-namespace-enforce-kind-missing-pod\"}," +
+ "\"spec\":{\"containers\":[{\"image\":\"nginx:1.7.9\",\"imagePullPolicy\":\"Never\",\"name\":" +
+ "\"nginx\",\"ports\":[{\"containerPort\":80}]}]}}'`"
+ utils.DoConfigPolicyMessageTest(clientManagedDynamic, gvrConfigPolicy, testNamespace,
+ case5KindMissPlcName, 0, defaultTimeoutSeconds, kindErrMsg)
+
+ By("Kind and Name missing")
+ kindNameErrMsg := "Decoding error, please check your policy file! Aborting handling the " +
+ "object template at index [0] in policy `policy-multi-namespace-enforce-both-missing` " +
+ "with error = `Object 'Kind' is missing in " +
+ "'{\"apiVersion\":\"v1\",\"metadata\":{\"name\":\"\"},\"spec\":{\"containers\":" +
+ "[{\"image\":\"nginx:1.7.9\",\"imagePullPolicy\":\"Never\"," +
+ "\"name\":\"nginx\",\"ports\":[{\"containerPort\":80}]}]}}'`"
+ utils.DoConfigPolicyMessageTest(clientManagedDynamic, gvrConfigPolicy, testNamespace,
+ case5KindNameMissPlcName, 0, defaultTimeoutSeconds, kindNameErrMsg)
+
+ By("Name missing")
+ nameErrMsg := "No instances of `pods` found as specified in namespaces: n1, n2, n3"
+ utils.DoConfigPolicyMessageTest(clientManagedDynamic, gvrConfigPolicy, testNamespace,
+ case5NameMissPlcName, 0, defaultTimeoutSeconds, nameErrMsg)
+ })
+ It("Should show merged Noncompliant messages when it is multiple namespaces and inform", func() {
+ expectedMsg := "pods not found: [case5-multi-namespace-inform-pod] " +
+ "in namespace n1 missing; [case5-multi-namespace-inform-pod] " +
+ "in namespace n2 missing; [case5-multi-namespace-inform-pod] " +
+ "in namespace n3 missing"
+ utils.Kubectl("apply", "-f", case5InformYaml)
+ utils.DoConfigPolicyMessageTest(clientManagedDynamic, gvrConfigPolicy, testNamespace,
+ case5MultiNSInformConfigPolicyName, 0, defaultTimeoutSeconds, expectedMsg)
+ })
+ It("Should show merged messages when it is multiple namespaces", func() {
+ expectedMsg := "Pod [case5-multi-namespace-enforce-pod] in namespace n1 found; " +
+ "[case5-multi-namespace-enforce-pod] in namespace n2 found; " +
+ "[case5-multi-namespace-enforce-pod] in namespace n3 found " +
+ "as specified, therefore this Object template is compliant"
+ utils.Kubectl("apply", "-f", case5EnforceYaml)
+ utils.DoConfigPolicyMessageTest(clientManagedDynamic, gvrConfigPolicy, testNamespace,
+ case5MultiNSConfigPolicyName, 0, defaultTimeoutSeconds, expectedMsg)
+ })
+ It("Should show 3 merged messages when it is multiple namespaces and multiple obj-template", func() {
+ firstMsg := "Pod [case5-multi-obj-temp-pod-11] in namespace n1 found; " +
+ "[case5-multi-obj-temp-pod-11] in namespace n2 found; " +
+ "[case5-multi-obj-temp-pod-11] in namespace n3 found " +
+ "as specified, therefore this Object template is compliant"
+ secondMsg := "Pod [case5-multi-obj-temp-pod-22] in namespace n1 found; " +
+ "[case5-multi-obj-temp-pod-22] in namespace n2 found; " +
+ "[case5-multi-obj-temp-pod-22] in namespace n3 found " +
+ "as specified, therefore this Object template is compliant"
+ thirdMsg := "Pod [case5-multi-obj-temp-pod-33] in namespace n1 found; " +
+ "[case5-multi-obj-temp-pod-33] in namespace n2 found; " +
+ "[case5-multi-obj-temp-pod-33] in namespace n3 found " +
+ "as specified, therefore this Object template is compliant"
+ utils.Kubectl("apply", "-f", case5MultiObjTmpYaml)
+ utils.DoConfigPolicyMessageTest(clientManagedDynamic, gvrConfigPolicy, testNamespace,
+ case5MultiObjNSConfigPolicyName, 0, defaultTimeoutSeconds, firstMsg)
+ utils.DoConfigPolicyMessageTest(clientManagedDynamic, gvrConfigPolicy, testNamespace,
+ case5MultiObjNSConfigPolicyName, 1, defaultTimeoutSeconds, secondMsg)
+ utils.DoConfigPolicyMessageTest(clientManagedDynamic, gvrConfigPolicy, testNamespace,
+ case5MultiObjNSConfigPolicyName, 2, defaultTimeoutSeconds, thirdMsg)
+
+ By("Configuration policy name " + case5NameMissPlcName +
+ " should be compliant after apply " + case5MultiObjTmpYaml)
+
+ Eventually(func() interface{} {
+ plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+ case5NameMissPlcName, testNamespace, true, defaultTimeoutSeconds)
+
+ return utils.GetComplianceState(plc)
+ }, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
+ })
+ cleanup := func() {
+ policies := []string{
+ case5MultiNSConfigPolicyName,
+ case5MultiNSInformConfigPolicyName,
+ case5MultiObjNSConfigPolicyName,
+ case5NameMissPlcName,
+ case5KindNameMissPlcName,
+ case5KindMissPlcName,
+ }
+
+ deleteConfigPolicies(policies)
+ nss := []string{
+ case5MultiNamespace1,
+ case5MultiNamespace2,
+ case5MultiNamespace3,
+ }
+
+ for _, ns := range nss {
+ utils.Kubectl("delete", "ns", ns, "--ignore-not-found")
+ }
+ }
+ AfterAll(cleanup)
+ })
 })

test/e2e/case8_status_check_test.go

@@ -25,7 +25,7 @@ const (
 )
 
 var _ = Describe("Test pod obj template handling", func() {
- Describe("Create a policy on managed cluster in ns:"+testNamespace, func() {
+ Describe("Create a policy on managed cluster in ns:"+testNamespace, Ordered, func() {
  It("should create a policy properly on the managed cluster", func() {
  By("Creating " + case8ConfigPolicyNamePod + " on managed")
  utils.Kubectl("apply", "-f", case8PolicyYamlPod, "-n", testNamespace)
@@ -89,7 +89,7 @@ var _ = Describe("Test pod obj template handling", func() {
  deleteConfigPolicies(policies)
  })
  })
- Describe("Create a policy with status on managed cluster in ns:"+testNamespace, func() {
+ Describe("Create a policy with status on managed cluster in ns:"+testNamespace, Ordered, func() {
  It("should create a policy properly on the managed cluster", func() {
  By("Creating " + case8ConfigPolicyStatusPod + " on managed")
  utils.Kubectl("apply", "-f", case8PolicyYamlBadPod, "-n", testNamespace)
@@ -140,11 +140,13 @@ var _ = Describe("Test pod obj template handling", func() {
  return utils.GetComplianceState(managedPlc)
  }, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
  })
- It("Cleans up", func() {
+ AfterAll(func() {
  policies := []string{
  case8ConfigPolicyStatusPod,
  }
  deleteConfigPolicies(policies)
+
+ utils.Kubectl("delete", "pod", "nginx-badpod-e2e-8", "-n", "default", "--ignore-not-found")
  })
  })
 })

test/resources/case5_multi/case5_multi_namespace_enforce.yaml

@@ -0,0 +1,25 @@
+apiVersion: policy.open-cluster-management.io/v1
+kind: ConfigurationPolicy
+metadata:
+ name: policy-multi-namespace-enforce
+ namespace: managed
+spec:
+ remediationAction: enforce
+ pruneObjectBehavior: DeleteAll
+ namespaceSelector:
+ exclude: ["kube-*"]
+ include: ["n1","n2","n3"]
+ object-templates:
+ - complianceType: musthave
+ objectDefinition:
+ apiVersion: v1
+ kind: Pod
+ metadata:
+ name: case5-multi-namespace-enforce-pod
+ spec:
+ containers:
+ - image: nginx:1.7.9
+ imagePullPolicy: Never
+ name: nginx
+ ports:
+ - containerPort: 80