Skip to content

Commit

Permalink
Set parent policy label on template when updating
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
willkutler committed Mar 13, 2023
1 parent 0a2229c commit 78a1dfa
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 3 deletions.
54 changes: 51 additions & 3 deletions controllers/templatesync/template_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -666,20 +666,24 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
log.Error(err, "Failed to apply defaults to the ConstraintTemplate for comparison. Continuing.")
}
}
// 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{})
Expand Down Expand Up @@ -806,6 +810,50 @@ 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(
Expand Down
21 changes: 21 additions & 0 deletions test/e2e/case15_template_cleanup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,5 +101,26 @@ var _ = Describe("Test template sync", func() {
cfgplc = utils.GetWithTimeout(clientManagedDynamic, gvrConfigurationPolicy, case15ConfigPolicyRenamed,
clusterNamespace, false, defaultTimeoutSeconds*2)
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))
})
})
3 changes: 3 additions & 0 deletions test/resources/case15_template_cleanup/case15-patchlabel.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
metadata:
labels:
policy.open-cluster-management.io/policy: policy-doesnotexist

0 comments on commit 78a1dfa

Please sign in to comment.