multi namepace message changed

ConfigurationPolicy message for enforce omits objects when multiple namespaces are specified When set to inform, the full list of objects is shown. But when set to enforce (the first two messages displayed), only the last object message is shown. In multiple namespaces, the message should be merged. Should be fixed like this example: pod [pod1] in namespace test1 found; [pod1] in namespace test2 found as specified, therefore this Object template is compliant Ref: https://issues.redhat.com/browse/ACM-2604 Signed-off-by: Yi Rae Kim <yikim@redhat.com>
open-cluster-management-io · Apr 3, 2023 · d7e709f · d7e709f
1 parent b70748b
commit d7e709f
Show file tree

Hide file tree

Showing 10 changed files with 452 additions and 66 deletions.
diff --git a/controllers/configurationpolicy_controller.go b/controllers/configurationpolicy_controller.go
@@ -1041,9 +1041,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:
@@ -1057,10 +1057,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(
@@ -1079,20 +1082,32 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
  }
 
  if objKind != "" {
+ // object template enforced, objKind is empty
  kind = objKind
  }
 
+ if mergeMessageEnforce {
+ names = append([]string{}, templateObjs[indx].name)
+ }
+
+ // object template enforced, already handled in handleObjects
  if names == nil {
- // object template enforced, already handled in handleObjects
  handled = true
  } else {
  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 mergeMessageEnforce || (!handled && !enforce) {
  if !compliant {
  if len(names) == 0 {
  numNonCompliant++
  } else {
  numNonCompliant += len(names)
  }
+
  nonCompliantObjects[ns] = map[string]interface{}{
  "names": names,
  "reason": reason,
@@ -1110,28 +1125,75 @@ 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 := map[string]interface{}{
- "indx": indx,
- "kind": kind,
- "desiredName": templateObjs[indx].name,
- "namespaced": templateObjs[indx].isNamespaced,
- }
 
- statusUpdateNeeded := createInformStatus(
+ objData := map[string]interface{}{
+ "indx": indx,
+ "kind": kind,
+ "desiredName": templateObjs[indx].name,
+ "namespaced": templateObjs[indx].isNamespaced,
+ }
+ if mergeMessageEnforce || (!handled && !enforce) {
+ statusUpdateNeeded := createMergedStatus(
  objShouldExist, numCompliant, numNonCompliant, compliantObjects, nonCompliantObjects, &plc, objData,
  )
+
  if statusUpdateNeeded {
  parentStatusUpdateNeeded = true
  }
+
+ if mergeMessageEnforce {
+ r.Recorder.Event(&plc, eventNormal, fmt.Sprintf(plcFmtStr, plc.GetName()),
+ convertPolicyStatusToString(&plc))
+ }
  }
  }
 
  r.checkRelatedAndUpdate(plc, relatedObjects, oldRelated, parentStatusUpdateNeeded)
 }
 
+// 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
+// Or multiple namespaces and enforce use this
+func createMergedStatus(
+ objShouldExist bool,
+ numCompliant,
+ numNonCompliant int,
+ compliantObjects,
+ nonCompliantObjects map[string]map[string]interface{},
+ plc *policyv1.ConfigurationPolicy,
+ objData map[string]interface{},
+) bool {
+ //nolint:forcetypeassert
+ desiredName := objData["desiredName"].(string)
+ //nolint:forcetypeassert
+ indx := objData["indx"].(int)
+ //nolint:forcetypeassert
+ kind := objData["kind"].(string)
+ //nolint:forcetypeassert
+ namespaced := objData["namespaced"].(bool)
+
+ if kind == "" {
+ return false
+ }
+
+ var compObjs map[string]map[string]interface{}
+ var compliant bool
+
+ if numNonCompliant > 0 {
+ compliant = false
+ compObjs = nonCompliantObjects
+ } else if objShouldExist && numCompliant == 0 {
+ // Special case: No resources found is NonCompliant
+ compliant = false
+ compObjs = nonCompliantObjects
+ } else {
+ compliant = true
+ compObjs = compliantObjects
+ }
+
+ return createStatus(desiredName, kind, compObjs, namespaced, plc, indx, compliant, objShouldExist)
+}
+
 // checkRelatedAndUpdate checks the related objects field and triggers an update on the ConfigurationPolicy
 func (r *ConfigurationPolicyReconciler) checkRelatedAndUpdate(
  plc policyv1.ConfigurationPolicy, related, oldRelated []policyv1.RelatedObject, sendEvent bool,
@@ -1294,48 +1356,6 @@ func addConditionToStatus(
  return updateNeeded
 }
 
-// createInformStatus 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(
- objShouldExist bool,
- numCompliant,
- numNonCompliant int,
- compliantObjects,
- nonCompliantObjects map[string]map[string]interface{},
- plc *policyv1.ConfigurationPolicy,
- objData map[string]interface{},
-) bool {
- //nolint:forcetypeassert
- desiredName := objData["desiredName"].(string)
- //nolint:forcetypeassert
- indx := objData["indx"].(int)
- //nolint:forcetypeassert
- kind := objData["kind"].(string)
- //nolint:forcetypeassert
- namespaced := objData["namespaced"].(bool)
-
- if kind == "" {
- return false
- }
-
- var compObjs map[string]map[string]interface{}
- var compliant bool
-
- if numNonCompliant > 0 {
- compliant = false
- compObjs = nonCompliantObjects
- } else if objShouldExist && numCompliant == 0 {
- // Special case: No resources found is NonCompliant
- compliant = false
- compObjs = nonCompliantObjects
- } else {
- compliant = true
- compObjs = compliantObjects
- }
-
- return createStatus(desiredName, kind, compObjs, namespaced, plc, indx, compliant, objShouldExist)
-}
-
 // handleObjects controls the processing of each individual object template within a configurationpolicy
 func (r *ConfigurationPolicyReconciler) handleObjects(
  objectT *policyv1.ObjectTemplate,

diff --git a/controllers/configurationpolicy_controller_test.go b/controllers/configurationpolicy_controller_test.go
@@ -435,7 +435,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",
@@ -473,7 +473,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)
 
@@ -484,7 +484,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)
 
@@ -493,7 +493,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)
 
@@ -509,7 +509,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)
 
@@ -523,7 +523,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)
 }

diff --git a/controllers/configurationpolicy_utils.go b/controllers/configurationpolicy_utils.go
@@ -346,7 +346,16 @@ func isAutogenerated(key string) (result bool) {
 func formatTemplate(unstruct unstructured.Unstructured, key string) (obj interface{}) {
  if key == "metadata" {
  //nolint:forcetypeassert
- metadata := unstruct.Object[key].(map[string]interface{})
+ var metadata map[string]interface{}
+ if unstruct.Object[key] != nil {
+ metadata = unstruct.Object[key].(map[string]interface{})
+ } else {
+ log.Info("metadata was nil, add minimum to avoid panic")
+
+ metadata = map[string]interface{}{
+ "name": "",
+ }
+ }
 
  return formatMetadata(metadata)
  }