Set parent policy label on template when updating

Currently, the parent policy label on templates, which is used for #28 , is only set on template creation, which could cause issues with policies that have already been created. This fix verifies the necessary template labels are set when updating the template.

Signed-off-by: Will Kutler <wkutler@redhat.com>

controllers/templatesync/template_sync.go

@@ -516,19 +516,24 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 			continue
 		}
 
+		// verify parent policy label is set properly
+		tObjectUnstructured.SetLabels(r.setDefaultTemplateLabels(instance, tObjectUnstructured.GetLabels()))
+
 		overrideRemediationAction(instance, tObjectUnstructured)
+
 		// got object, need to compare both spec and annotation and update
 		eObjectUnstructured := eObject.UnstructuredContent()
-		if (!equality.Semantic.DeepEqual(eObjectUnstructured["spec"], tObjectUnstructured.Object["spec"])) ||
-			(!equality.Semantic.DeepEqual(eObject.GetAnnotations(), tObjectUnstructured.GetAnnotations())) ||
-			(!equality.Semantic.DeepEqual(eObject.GetOwnerReferences(), tObjectUnstructured.GetOwnerReferences())) {
+
+		if !equivalentTemplates(eObject, tObjectUnstructured) {
 			// doesn't match
 			tLogger.Info("Existing object and template didn't match, will update")
 
 			eObjectUnstructured["spec"] = tObjectUnstructured.Object["spec"]
 
 			eObject.SetAnnotations(tObjectUnstructured.GetAnnotations())
 
+			eObject.SetLabels(tObjectUnstructured.GetLabels())
+
 			eObject.SetOwnerReferences(tObjectUnstructured.GetOwnerReferences())
 
 			_, err = res.Update(ctx, eObject, metav1.UpdateOptions{})
@@ -605,6 +610,49 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 	return reconcile.Result{}, resultError
 }
 
+// equivalentTemplates determines whether the template existing on the cluster and the policy template are the same
+func equivalentTemplates(eObject *unstructured.Unstructured, tObject *unstructured.Unstructured) bool {
+	if !equality.Semantic.DeepEqual(eObject.UnstructuredContent()["spec"], tObject.Object["spec"]) {
+		return false
+	}
+
+	if !equality.Semantic.DeepEqual(eObject.GetAnnotations(), tObject.GetAnnotations()) {
+		return false
+	}
+
+	if !equality.Semantic.DeepEqual(eObject.GetLabels(), tObject.GetLabels()) {
+		return false
+	}
+
+	if !equality.Semantic.DeepEqual(eObject.GetOwnerReferences(), tObject.GetOwnerReferences()) {
+		return false
+	}
+
+	return true
+}
+
+// setDefaultTemplateLabels ensures the template contains all necessary labels for processing
+func (r *PolicyReconciler) setDefaultTemplateLabels(instance *policiesv1.Policy,
+	labels map[string]string) map[string]string {
+	if labels == nil {
+		labels = map[string]string{}
+	}
+
+	desiredLabels := map[string]string{
+		parentPolicyLabel:            instance.GetName(),
+		"cluster-name":               instance.GetLabels()[common.ClusterNameLabel],
+		common.ClusterNameLabel:      instance.GetLabels()[common.ClusterNameLabel],
+		"cluster-namespace":          r.ClusterNamespace,
+		common.ClusterNamespaceLabel: instance.GetLabels()[common.ClusterNamespaceLabel],
+	}
+
+	for key, label := range desiredLabels {
+		labels[key] = label
+	}
+
+	return labels
+}
+
 // cleanUpExcessTemplates compares existing policy templates on the cluster to those contained in the policy,
 // and deletes those that have been renamed or removed from the parent policy
 func (r *PolicyReconciler) cleanUpExcessTemplates(

test/e2e/case15_template_cleanup_test.go

@@ -101,5 +101,26 @@ var _ = Describe("Test template sync", func() {
 		cfgplc = utils.GetWithTimeout(clientManagedDynamic, gvrConfigurationPolicy, case15ConfigPolicyRenamed,
 			clusterNamespace, false, defaultTimeoutSeconds)
 		Expect(cfgplc).To(BeNil())
+
+		By("Verifying parent label can be set after template creation")
+		_, err = kubectlManaged("patch", "configurationpolicy", case15ConfigPolicyNameStable, "-n", clusterNamespace,
+			"--type", "merge", "--patch-file", "../resources/case15_template_cleanup/case15-patchlabel.yaml")
+		Expect(err).Should(BeNil())
+
+		Eventually(func() interface{} {
+			configPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigurationPolicy,
+				case15ConfigPolicyNameStable, clusterNamespace, true, defaultTimeoutSeconds)
+
+			md, ok := configPlc.Object["metadata"].(map[string]interface{})
+			if !ok {
+				return nil
+			}
+			labels, ok := md["labels"]
+			if !ok {
+				return nil
+			}
+
+			return labels.(map[string]interface{})["policy.open-cluster-management.io/policy"].(string)
+		}, defaultTimeoutSeconds, 1).Should(utils.SemanticEqual(case15PolicyName))
 	})
 })

test/resources/case15_template_cleanup/case15-patchlabel.yaml

@@ -0,0 +1,3 @@
+metadata:
+  labels:
+    policy.open-cluster-management.io/policy: policy-doesnotexist