Set the parent policy to NonCompliant when there is a duplicate name

The noncompliant event was sent in this case but the status-sync did not
pick it up because it specified the policy template name using the array
index.

Relates:
https://issues.redhat.com/browse/ACM-11806

Signed-off-by: mprahl <[email protected]>

Author: mprahl

controllers/templatesync/template_sync.go

@@ -177,13 +177,13 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 	}
 
 	// Check duplicate names in configuration-policies
-	has := hasDupName(instance)
-	if has {
-		msg := "There are duplicate names in configurationpolicies, please check the policy"
+	dupName := getDupName(instance)
+	if dupName != "" {
+		msg := "All policy-template names must be unique within a policy"
 		reqLogger.Info(msg)
 
 		for tIndex := range instance.Spec.PolicyTemplates {
-			_ = r.emitTemplateError(ctx, instance, tIndex, fmt.Sprintf("template-%v", tIndex), false, msg)
+			_ = r.emitTemplateError(ctx, instance, tIndex, dupName, false, msg)
 		}
 
 		return reconcile.Result{}, nil
@@ -1693,27 +1693,27 @@ func (r *PolicyReconciler) setCreatedGkConstraint(b bool) {
 	r.createdGkConstraint = &b
 }
 
-// Check duplicate names in policy-templates(configurationPolicies)
-func hasDupName(pol *policiesv1.Policy) bool {
+// Check duplicate names in policy-templates and return the first duplicate name found, if any.
+func getDupName(pol *policiesv1.Policy) string {
 	templates := pol.Spec.PolicyTemplates
 
-	foundNames := make(map[string]struct{})
+	foundNames := make(map[string]bool, len(templates))
 
 	for _, v := range templates {
 		unstructured, err := unmarshalFromJSON(v.ObjectDefinition.Raw)
 		if err != nil {
 			// Skip unmarshal error here, template error should appear later
-			return false
+			return ""
 		}
 
 		name := unstructured.GetName()
 
-		if _, has := foundNames[name]; has {
-			return true
+		if foundNames[name] {
+			return name
 		}
 
-		foundNames[name] = struct{}{}
+		foundNames[name] = true
 	}
 
-	return false
+	return ""
 }

controllers/templatesync/template_sync_test.go

@@ -93,7 +93,7 @@ func TestHandleSyncSuccessNoDoubleRemoveStatus(t *testing.T) {
 	}
 }
 
-func TestHasDuplicateNames(t *testing.T) {
+func TestGetDupName(t *testing.T) {
 	policy := policiesv1.Policy{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "Policy",
@@ -131,8 +131,8 @@ func TestHasDuplicateNames(t *testing.T) {
 
 	policy.Spec.PolicyTemplates = append(policy.Spec.PolicyTemplates, &x)
 
-	has := hasDupName(&policy)
-	if has {
+	dupName := getDupName(&policy)
+	if dupName != "" {
 		t.Fatal("Unexpected duplicate policy template names")
 	}
 
@@ -160,8 +160,8 @@ func TestHasDuplicateNames(t *testing.T) {
 
 	policy.Spec.PolicyTemplates = append(policy.Spec.PolicyTemplates, &y)
 
-	has = hasDupName(&policy)
-	if !has {
+	dupName = getDupName(&policy)
+	if dupName != "test-configpolicy" {
 		t.Fatal("Duplicate names for templates not detected")
 	}
 
@@ -189,8 +189,8 @@ func TestHasDuplicateNames(t *testing.T) {
 
 	policy.Spec.PolicyTemplates = append(policy.Spec.PolicyTemplates, &z)
 
-	has = hasDupName(&policy)
-	if !has {
+	dupName = getDupName(&policy)
+	if dupName != "test-configpolicy" {
 		t.Fatal("Duplicate names for templates not detected")
 	}
 
@@ -208,8 +208,8 @@ func TestHasDuplicateNames(t *testing.T) {
 
 	policy.Spec.PolicyTemplates = append(policy.Spec.PolicyTemplates, &x2)
 
-	has = hasDupName(&policy)
-	if !has { // expect duplicate detection to return true
+	dupName = getDupName(&policy)
+	if dupName != "test-configpolicy" { // expect duplicate detection to return true
 		t.Fatal("Duplicate name not detected")
 	}
 }

test/e2e/case10_error_test.go

@@ -42,12 +42,27 @@ var _ = Describe("Test error handling", func() {
 
 		By("Should generate warning events")
 		Eventually(
-			checkForEvent(dupNamePolicyName,
-				"There are duplicate names in configurationpolicies, please check the policy"),
+			checkForEvent(dupNamePolicyName, "All policy-template names must be unique within a policy"),
 			defaultTimeoutSeconds,
 			1,
 		).Should(BeTrue())
 
+		By("Checking if the policy status is NonCompliant")
+		Eventually(func(g Gomega) string {
+			hubPlc := utils.GetWithTimeout(
+				clientHubDynamic,
+				gvrPolicy,
+				dupNamePolicyName,
+				clusterNamespaceOnHub,
+				true,
+				defaultTimeoutSeconds,
+			)
+
+			compliant, _, _ := unstructured.NestedString(hubPlc.Object, "status", "compliant")
+
+			return compliant
+		}, defaultTimeoutSeconds, 1).Should(Equal("NonCompliant"))
+
 		By("Should not create any configuration policy")
 		Consistently(func() interface{} {
 			return utils.GetWithTimeout(clientManagedDynamic, gvrConfigurationPolicy, dupConfigPolicyName,