Add the spec.copyPolicyMetadata field

This new field allows a replicated policy to not have labels and
annotations that are not managed by the policy framework. This is useful
in the case where policies are deployed with ArgoCD and you don't want
them to show up in the ArgoCD UI.

Additionally, the argocd.argoproj.io/compare-options annotation is now
always set to IgnoreExtraneous on the replicated policies to avoid
ArgoCD trying to manage the replicated policies.

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

Signed-off-by: mprahl <mprahl@users.noreply.github.com>

Author: mprahl

api/v1/policy_types.go

@@ -69,6 +69,12 @@ type PolicySpec struct {
 	// This provides the ability to enable and disable your policies.
 	Disabled bool `json:"disabled"`
 
+	// If set to true (default), all the policy's labels and annotations will be copied to the replicated policy.
+	// If set to false, only the policy framework specific policy labels and annotations will be copied to the
+	// replicated policy.
+	// +kubebuilder:validation:Optional
+	CopyPolicyMetadata *bool `json:"copyPolicyMetadata,omitempty"`
+
 	// This value (Enforce or Inform) will override the remediationAction on each template
 	RemediationAction RemediationAction `json:"remediationAction,omitempty"`
 

controllers/propagator/replication.go

@@ -16,6 +16,8 @@ import (
 	"open-cluster-management.io/governance-policy-propagator/controllers/common"
 )
 
+const argoCDCompareOptionsAnnotation = "argocd.argoproj.io/compare-options"
+
 // equivalentReplicatedPolicies compares replicated policies. Returns true if they match.
 func equivalentReplicatedPolicies(plc1 *policiesv1.Policy, plc2 *policiesv1.Policy) bool {
 	// Compare annotations
@@ -52,13 +54,43 @@ func (r *PolicyReconciler) buildReplicatedPolicy(
 		labels = map[string]string{}
 	}
 
+	if root.Spec.CopyPolicyMetadata != nil && !*root.Spec.CopyPolicyMetadata {
+		originalLabels := replicated.GetLabels()
+
+		for label := range originalLabels {
+			if !strings.HasPrefix(label, policiesv1.GroupVersion.Group+"/") {
+				delete(labels, label)
+			}
+		}
+	}
+
 	// Extra labels on replicated policies
 	labels[common.ClusterNameLabel] = decision.ClusterName
 	labels[common.ClusterNamespaceLabel] = decision.ClusterNamespace
 	labels[common.RootPolicyLabel] = replicatedName
 
 	replicated.SetLabels(labels)
 
+	annotations := replicated.GetAnnotations()
+	if annotations == nil {
+		annotations = map[string]string{}
+	}
+
+	if root.Spec.CopyPolicyMetadata != nil && !*root.Spec.CopyPolicyMetadata {
+		originalAnnotations := replicated.GetAnnotations()
+
+		for annotation := range originalAnnotations {
+			if !strings.HasPrefix(annotation, policiesv1.GroupVersion.Group+"/") {
+				delete(annotations, annotation)
+			}
+		}
+	}
+
+	// Always set IgnoreExtraneous to avoid ArgoCD managing the replicated policy.
+	annotations[argoCDCompareOptionsAnnotation] = "IgnoreExtraneous"
+
+	replicated.SetAnnotations(annotations)
+
 	var err error
 
 	replicated.Spec.Dependencies, err = r.canonicalizeDependencies(replicated.Spec.Dependencies, root.Namespace)

deploy/crds/kustomize/policy.open-cluster-management.io_policies.yaml

@@ -48,6 +48,12 @@ spec:
           spec:
             description: PolicySpec defines the desired state of Policy
             properties:
+              copyPolicyMetadata:
+                description: If set to true (default), all the policy's labels and
+                  annotations will be copied to the replicated policy. If set to false,
+                  only the policy framework specific policy labels and annotations
+                  will be copied to the replicated policy.
+                type: boolean
               dependencies:
                 description: PolicyDependencies that apply to each template in this
                   Policy

deploy/crds/policy.open-cluster-management.io_policies.yaml

@@ -48,6 +48,12 @@ spec:
           spec:
             description: PolicySpec defines the desired state of Policy
             properties:
+              copyPolicyMetadata:
+                description: If set to true (default), all the policy's labels and
+                  annotations will be copied to the replicated policy. If set to false,
+                  only the policy framework specific policy labels and annotations
+                  will be copied to the replicated policy.
+                type: boolean
               dependencies:
                 description: PolicyDependencies that apply to each template in this
                   Policy

test/e2e/case1_propagation_test.go

@@ -8,8 +8,11 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/dynamic"
 	appsv1 "open-cluster-management.io/multicloud-operators-subscription/pkg/apis/apps/placementrule/v1"
 
 	policiesv1 "open-cluster-management.io/governance-policy-propagator/api/v1"
@@ -536,4 +539,210 @@ var _ = Describe("Test policy propagation", func() {
 			utils.ListWithTimeout(clientHubDynamic, gvrPolicy, opt, 0, false, 10)
 		})
 	})
+
+	Describe("Test spec.copyPolicyMetadata", Ordered, func() {
+		const policyName = "case1-metadata"
+		policyClient := func() dynamic.ResourceInterface {
+			return clientHubDynamic.Resource(gvrPolicy).Namespace(testNamespace)
+		}
+
+		getPolicy := func() *policiesv1.Policy {
+			return &policiesv1.Policy{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       policiesv1.Kind,
+					APIVersion: policiesv1.GroupVersion.String(),
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      policyName,
+					Namespace: testNamespace,
+				},
+				Spec: policiesv1.PolicySpec{
+					Disabled:        false,
+					PolicyTemplates: []*policiesv1.PolicyTemplate{},
+				},
+			}
+		}
+
+		BeforeAll(func() {
+			By("Creating a PlacementRule")
+			plr := &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"apiVersion": "apps.open-cluster-management.io/v1",
+					"kind":       "PlacementRule",
+					"metadata": map[string]interface{}{
+						"name":      policyName,
+						"namespace": testNamespace,
+					},
+					"spec": map[string]interface{}{},
+				},
+			}
+			var err error
+			plr, err = clientHubDynamic.Resource(gvrPlacementRule).Namespace(testNamespace).Create(
+				context.TODO(), plr, metav1.CreateOptions{},
+			)
+			Expect(err).To(BeNil())
+
+			plr.Object["status"] = utils.GeneratePlrStatus("managed1")
+			_, err = clientHubDynamic.Resource(gvrPlacementRule).Namespace(testNamespace).UpdateStatus(
+				context.TODO(), plr, metav1.UpdateOptions{},
+			)
+			Expect(err).To(BeNil())
+
+			By("Creating a PlacementBinding")
+			plb := &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"apiVersion": policiesv1.GroupVersion.String(),
+					"kind":       "PlacementBinding",
+					"metadata": map[string]interface{}{
+						"name":      policyName,
+						"namespace": testNamespace,
+					},
+					"placementRef": map[string]interface{}{
+						"apiGroup": gvrPlacementRule.Group,
+						"kind":     "PlacementRule",
+						"name":     policyName,
+					},
+					"subjects": []interface{}{
+						map[string]interface{}{
+							"apiGroup": policiesv1.GroupVersion.Group,
+							"kind":     policiesv1.Kind,
+							"name":     policyName,
+						},
+					},
+				},
+			}
+
+			_, err = clientHubDynamic.Resource(gvrPlacementBinding).Namespace(testNamespace).Create(
+				context.TODO(), plb, metav1.CreateOptions{},
+			)
+			Expect(err).To(BeNil())
+		})
+
+		AfterEach(func() {
+			By("Removing the policy")
+			err := policyClient().Delete(context.TODO(), policyName, metav1.DeleteOptions{})
+			if !errors.IsNotFound(err) {
+				Expect(err).To(BeNil())
+			}
+
+			replicatedPolicy := utils.GetWithTimeout(
+				clientHubDynamic,
+				gvrPolicy,
+				testNamespace+"."+policyName,
+				"managed1",
+				false,
+				defaultTimeoutSeconds,
+			)
+			Expect(replicatedPolicy).To(BeNil())
+		})
+
+		AfterAll(func() {
+			By("Removing the PlacementRule")
+			err := clientHubDynamic.Resource(gvrPlacementRule).Namespace(testNamespace).Delete(
+				context.TODO(), policyName, metav1.DeleteOptions{},
+			)
+			if !errors.IsNotFound(err) {
+				Expect(err).To(BeNil())
+			}
+
+			By("Removing the PlacementBinding")
+			err = clientHubDynamic.Resource(gvrPlacementBinding).Namespace(testNamespace).Delete(
+				context.TODO(), policyName, metav1.DeleteOptions{},
+			)
+			if !errors.IsNotFound(err) {
+				Expect(err).To(BeNil())
+			}
+		})
+
+		It("Verifies that spec.copyPolicyMetadata defaults to unset", func() {
+			By("Creating an empty policy")
+			policy := getPolicy()
+			policyMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(policy)
+			Expect(err).To(BeNil())
+
+			policyRV, err := policyClient().Create(
+				context.TODO(), &unstructured.Unstructured{Object: policyMap}, metav1.CreateOptions{},
+			)
+			Expect(err).To(BeNil())
+
+			_, found, _ := unstructured.NestedBool(policyRV.Object, "spec", "copyPolicyMetadata")
+			Expect(found).To(BeFalse())
+		})
+
+		It("verifies that the labels and annotations are copied with spec.copyPolicyMetadata=true", func() {
+			By("Creating a policy with labels and annotations")
+			policy := getPolicy()
+			copyPolicyMetadata := true
+			policy.Spec.CopyPolicyMetadata = &copyPolicyMetadata
+			policy.SetAnnotations(map[string]string{"do": "copy", "please": "do-copy"})
+			policy.SetLabels(map[string]string{"do": "copy", "please": "do-copy"})
+
+			policyMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(policy)
+			Expect(err).To(BeNil())
+
+			_, err = policyClient().Create(
+				context.TODO(), &unstructured.Unstructured{Object: policyMap}, metav1.CreateOptions{},
+			)
+			Expect(err).To(BeNil())
+
+			Eventually(func(g Gomega) {
+				replicatedPlc := utils.GetWithTimeout(
+					clientHubDynamic,
+					gvrPolicy,
+					testNamespace+"."+policyName,
+					"managed1",
+					true,
+					defaultTimeoutSeconds,
+				)
+
+				annotations := replicatedPlc.GetAnnotations()
+				g.Expect(annotations["do"]).To(Equal("copy"))
+				g.Expect(annotations["please"]).To(Equal("do-copy"))
+				// This annotation is always set.
+				g.Expect(annotations["argocd.argoproj.io/compare-options"]).To(Equal("IgnoreExtraneous"))
+
+				labels := replicatedPlc.GetLabels()
+				g.Expect(labels["do"]).To(Equal("copy"))
+				g.Expect(labels["please"]).To(Equal("do-copy"))
+			}, defaultTimeoutSeconds, 1).Should(Succeed())
+		})
+
+		It("verifies that the labels and annotations are not copied with spec.copyPolicyMetadata=false", func() {
+			By("Creating a policy with labels and annotations")
+			policy := getPolicy()
+			copyPolicyMetadata := false
+			policy.Spec.CopyPolicyMetadata = &copyPolicyMetadata
+			policy.SetAnnotations(map[string]string{"do-not": "copy", "please": "do-not-copy"})
+			policy.SetLabels(map[string]string{"do-not": "copy", "please": "do-not-copy"})
+
+			policyMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(policy)
+			Expect(err).To(BeNil())
+
+			_, err = policyClient().Create(
+				context.TODO(), &unstructured.Unstructured{Object: policyMap}, metav1.CreateOptions{},
+			)
+			Expect(err).To(BeNil())
+
+			Eventually(func(g Gomega) {
+				replicatedPlc := utils.GetWithTimeout(
+					clientHubDynamic,
+					gvrPolicy,
+					testNamespace+"."+policyName,
+					"managed1",
+					true,
+					defaultTimeoutSeconds,
+				)
+
+				annotations := replicatedPlc.GetAnnotations()
+				g.Expect(annotations["do-not"]).To(Equal(""))
+				g.Expect(annotations["please"]).To(Equal(""))
+				// This annotation is always set.
+				g.Expect(annotations["argocd.argoproj.io/compare-options"]).To(Equal("IgnoreExtraneous"))
+
+				labels := replicatedPlc.GetLabels()
+				g.Expect(labels["do-not"]).To(Equal(""))
+				g.Expect(labels["please"]).To(Equal(""))
+			}, defaultTimeoutSeconds, 1).Should(Succeed())
+		})
+	})
 })