Fix spec fetching for Compliance API

This was also incorrectly setting the replicated policy annotations on
the policy template's annotations.

Co-authored-by: Dale Haiducek <[email protected]>
Signed-off-by: mprahl <[email protected]>
(cherry picked from commit 4f01a51)

controllers/complianceeventsapi/types.go

@@ -390,7 +390,7 @@ func PolicyFromUnstructured(obj unstructured.Unstructured) *Policy {
 
 	policy.Name = obj.GetName()
 
-	spec, ok, _ := unstructured.NestedStringMap(obj.Object, "spec")
+	spec, ok, _ := unstructured.NestedMap(obj.Object, "spec")
 	if ok {
 		typedSpec := JSONMap{}
 
@@ -402,7 +402,9 @@ func PolicyFromUnstructured(obj unstructured.Unstructured) *Policy {
 	}
 
 	if severity, ok := spec["severity"]; ok {
-		policy.Severity = &severity
+		if sevString, ok := severity.(string); ok {
+			policy.Severity = &sevString
+		}
 	}
 
 	return policy

controllers/propagator/replicatedpolicy_controller.go

@@ -522,6 +522,7 @@ func (r *ReplicatedPolicyReconciler) setDBAnnotations(
 		}
 	} else {
 		annotations[ParentPolicyIDAnnotation] = strconv.FormatInt(int64(parentPolicyID), 10)
+		replicatedPolicy.SetAnnotations(annotations)
 	}
 
 	for i, plcTemplate := range replicatedPolicy.Spec.PolicyTemplates {
@@ -546,18 +547,23 @@ func (r *ReplicatedPolicyReconciler) setDBAnnotations(
 			}
 
 			requeueForDB = true
-			annotations := plcTmplUnstruct.GetAnnotations()
+			tmplAnnotations := plcTmplUnstruct.GetAnnotations()
 
-			if annotations[PolicyIDAnnotation] == "" {
+			if tmplAnnotations[PolicyIDAnnotation] == "" {
 				continue
 			}
 
 			// Remove it if the user accidentally provided the annotation
-			delete(annotations, PolicyIDAnnotation)
-			plcTmplUnstruct.SetAnnotations(annotations)
+			delete(tmplAnnotations, PolicyIDAnnotation)
+			plcTmplUnstruct.SetAnnotations(tmplAnnotations)
 		} else {
-			annotations[PolicyIDAnnotation] = strconv.FormatInt(int64(policyID), 10)
-			plcTmplUnstruct.SetAnnotations(annotations)
+			tmplAnnotations := plcTmplUnstruct.GetAnnotations()
+			if tmplAnnotations == nil {
+				tmplAnnotations = map[string]string{}
+			}
+
+			tmplAnnotations[PolicyIDAnnotation] = strconv.FormatInt(int64(policyID), 10)
+			plcTmplUnstruct.SetAnnotations(tmplAnnotations)
 		}
 
 		updatedTemplate, err := plcTmplUnstruct.MarshalJSON()

controllers/propagator/replicatedpolicy_controller_test.go

@@ -126,6 +126,10 @@ func TestSetDBAnnotationsNoDB(t *testing.T) {
 		t.Fatalf("Expected the policy ID of 56 but got: %s", templateAnnotations[PolicyIDAnnotation])
 	}
 
+	if len(templateAnnotations) != 1 {
+		t.Fatalf("Expected 1 policy annotation but got: %d", len(templateAnnotations))
+	}
+
 	// Test a cache hit from the last run using the policies from the first run
 	reconciler.setDBAnnotations(context.TODO(), rootPolicy, replicatedPolicy, existingReplicatedPolicy)
 
@@ -142,4 +146,8 @@ func TestSetDBAnnotationsNoDB(t *testing.T) {
 	if templateAnnotations[PolicyIDAnnotation] != "56" {
 		t.Fatalf("Expected the policy ID of 56 but got: %s", templateAnnotations[PolicyIDAnnotation])
 	}
+
+	if len(templateAnnotations) != 1 {
+		t.Fatalf("Expected 1 policy annotation but got: %d", len(templateAnnotations))
+	}
 }

test/e2e/case14_root_policy_metrics_test.go

@@ -64,7 +64,7 @@ var _ = Describe("Test root policy metrics", Ordered, func() {
 			Expect(plc).ToNot(BeNil())
 
 			yamlPlc := utils.ParseYaml(case9ReplicatedPolicyYamlM1)
-			Eventually(func() interface{} {
+			Eventually(func(g Gomega) interface{} {
 				replicatedPlc := utils.GetWithTimeout(
 					clientHubDynamic,
 					gvrPolicy,
@@ -74,6 +74,9 @@ var _ = Describe("Test root policy metrics", Ordered, func() {
 					defaultTimeoutSeconds,
 				)
 
+				err := utils.RemovePolicyTemplateDBAnnotations(replicatedPlc)
+				g.Expect(err).ToNot(HaveOccurred())
+
 				return replicatedPlc.Object["spec"]
 			}, defaultTimeoutSeconds, 1).Should(utils.SemanticEqual(yamlPlc.Object["spec"]))
 		})

test/e2e/case1_propagation_test.go

@@ -488,7 +488,7 @@ var _ = Describe("Test policy propagation", func() {
 				context.TODO(), rootPlc, metav1.UpdateOptions{},
 			)
 			Expect(err).ToNot(HaveOccurred())
-			Eventually(func() interface{} {
+			Eventually(func(g Gomega) interface{} {
 				replicatedPlc := utils.GetWithTimeout(
 					clientHubDynamic,
 					gvrPolicy,
@@ -498,6 +498,9 @@ var _ = Describe("Test policy propagation", func() {
 					defaultTimeoutSeconds,
 				)
 
+				err := utils.RemovePolicyTemplateDBAnnotations(replicatedPlc)
+				g.Expect(err).ToNot(HaveOccurred())
+
 				return replicatedPlc.Object["spec"]
 			}, defaultTimeoutSeconds, 1).Should(utils.SemanticEqual(rootPlc.Object["spec"]))
 		})
@@ -560,7 +563,7 @@ var _ = Describe("Test policy propagation", func() {
 			)
 			Expect(rootPlc).NotTo(BeNil())
 			yamlPlc := utils.ParseYaml("../resources/case1_propagation/case1-test-policy2.yaml")
-			Eventually(func() interface{} {
+			Eventually(func(g Gomega) interface{} {
 				replicatedPlc := utils.GetWithTimeout(
 					clientHubDynamic,
 					gvrPolicy,
@@ -570,6 +573,9 @@ var _ = Describe("Test policy propagation", func() {
 					defaultTimeoutSeconds,
 				)
 
+				err := utils.RemovePolicyTemplateDBAnnotations(replicatedPlc)
+				g.Expect(err).ToNot(HaveOccurred())
+
 				return replicatedPlc.Object["spec"]
 			}, defaultTimeoutSeconds, 1).Should(utils.SemanticEqual(yamlPlc.Object["spec"]))
 		})

test/e2e/case20_compliance_api_controller_test.go

@@ -192,7 +192,9 @@ func bringDownDBConnection(ctx context.Context) {
 		g.Expect(err).ToNot(HaveOccurred())
 
 		resp, err := httpClient.Do(req)
-		g.Expect(err).ToNot(HaveOccurred())
+		if err != nil {
+			return
+		}
 
 		defer resp.Body.Close()
 

test/e2e/case3_mutation_recovery_test.go

@@ -136,11 +136,14 @@ var _ = Describe("Test unexpected policy mutation", func() {
 		rootPlc := utils.GetWithTimeout(
 			clientHubDynamic, gvrPolicy, case3PolicyName, testNamespace, true, defaultTimeoutSeconds,
 		)
-		Eventually(func() interface{} {
+		Eventually(func(g Gomega) interface{} {
 			plc = utils.GetWithTimeout(
 				clientHubDynamic, gvrPolicy, testNamespace+"."+case3PolicyName, "managed2", true, defaultTimeoutSeconds,
 			)
 
+			err := utils.RemovePolicyTemplateDBAnnotations(plc)
+			g.Expect(err).ToNot(HaveOccurred())
+
 			return plc.Object["spec"]
 		}, defaultTimeoutSeconds, 1).Should(utils.SemanticEqual(rootPlc.Object["spec"]))
 	})

test/e2e/case6_placement_propagation_test.go

@@ -427,7 +427,7 @@ var _ = Describe("Test policy propagation", func() {
 				context.TODO(), rootPlc, metav1.UpdateOptions{},
 			)
 			Expect(err).ToNot(HaveOccurred())
-			Eventually(func() interface{} {
+			Eventually(func(g Gomega) interface{} {
 				replicatedPlc := utils.GetWithTimeout(
 					clientHubDynamic,
 					gvrPolicy,
@@ -437,6 +437,9 @@ var _ = Describe("Test policy propagation", func() {
 					defaultTimeoutSeconds,
 				)
 
+				err := utils.RemovePolicyTemplateDBAnnotations(replicatedPlc)
+				g.Expect(err).ToNot(HaveOccurred())
+
 				return replicatedPlc.Object["spec"]
 			}, defaultTimeoutSeconds, 1).Should(utils.SemanticEqual(rootPlc.Object["spec"]))
 		})
@@ -508,7 +511,7 @@ var _ = Describe("Test policy propagation", func() {
 			)
 			Expect(rootPlc).NotTo(BeNil())
 			yamlPlc := utils.ParseYaml("../resources/case6_placement_propagation/case6-test-policy2.yaml")
-			Eventually(func() interface{} {
+			Eventually(func(g Gomega) interface{} {
 				replicatedPlc := utils.GetWithTimeout(
 					clientHubDynamic,
 					gvrPolicy,
@@ -518,6 +521,9 @@ var _ = Describe("Test policy propagation", func() {
 					defaultTimeoutSeconds,
 				)
 
+				err := utils.RemovePolicyTemplateDBAnnotations(replicatedPlc)
+				g.Expect(err).ToNot(HaveOccurred())
+
 				return replicatedPlc.Object["spec"]
 			}, defaultTimeoutSeconds, 1).Should(utils.SemanticEqual(yamlPlc.Object["spec"]))
 		})

test/e2e/case9_templates_test.go

@@ -67,7 +67,7 @@ var _ = Describe("Test policy templates", func() {
 			Expect(plc).ToNot(BeNil())
 
 			yamlPlc := utils.ParseYaml(case9ReplicatedPolicyYamlM1)
-			Eventually(func() interface{} {
+			Eventually(func(g Gomega) interface{} {
 				replicatedPlc := utils.GetWithTimeout(
 					clientHubDynamic,
 					gvrPolicy,
@@ -77,6 +77,9 @@ var _ = Describe("Test policy templates", func() {
 					defaultTimeoutSeconds,
 				)
 
+				err := utils.RemovePolicyTemplateDBAnnotations(replicatedPlc)
+				g.Expect(err).ToNot(HaveOccurred())
+
 				return replicatedPlc.Object["spec"]
 			}, defaultTimeoutSeconds, 1).Should(utils.SemanticEqual(yamlPlc.Object["spec"]))
 		})
@@ -87,7 +90,7 @@ var _ = Describe("Test policy templates", func() {
 
 			By("Verifying the policy is updated")
 			yamlPlc := utils.ParseYaml(case9ReplicatedPolicyYamlM1Update)
-			Eventually(func() interface{} {
+			Eventually(func(g Gomega) interface{} {
 				replicatedPlc := utils.GetWithTimeout(
 					clientHubDynamic,
 					gvrPolicy,
@@ -97,6 +100,9 @@ var _ = Describe("Test policy templates", func() {
 					defaultTimeoutSeconds,
 				)
 
+				err := utils.RemovePolicyTemplateDBAnnotations(replicatedPlc)
+				g.Expect(err).ToNot(HaveOccurred())
+
 				return replicatedPlc.Object["spec"]
 			}, defaultTimeoutSeconds, 1).Should(utils.SemanticEqual(yamlPlc.Object["spec"]))
 		})
@@ -118,7 +124,7 @@ var _ = Describe("Test policy templates", func() {
 			Expect(plc).ToNot(BeNil())
 
 			yamlPlc := utils.ParseYaml(case9ReplicatedPolicyYamlM2)
-			Eventually(func() interface{} {
+			Eventually(func(g Gomega) interface{} {
 				replicatedPlc := utils.GetWithTimeout(
 					clientHubDynamic,
 					gvrPolicy,
@@ -128,6 +134,9 @@ var _ = Describe("Test policy templates", func() {
 					defaultTimeoutSeconds,
 				)
 
+				err := utils.RemovePolicyTemplateDBAnnotations(replicatedPlc)
+				g.Expect(err).ToNot(HaveOccurred())
+
 				return replicatedPlc.Object["spec"]
 			}, defaultTimeoutSeconds, 1).Should(utils.SemanticEqual(yamlPlc.Object["spec"]))
 		})
@@ -210,7 +219,7 @@ var _ = Describe("Test encrypted policy templates", func() {
 
 				By("Verifying the replicated policy against a snapshot")
 				yamlPlc := utils.ParseYaml(case9PolicyYamlEncryptedRepl + managedCluster + ".yaml")
-				Eventually(func() interface{} {
+				Eventually(func(g Gomega) interface{} {
 					replicatedPlc = utils.GetWithTimeout(
 						clientHubDynamic,
 						gvrPolicy,
@@ -220,6 +229,9 @@ var _ = Describe("Test encrypted policy templates", func() {
 						defaultTimeoutSeconds,
 					)
 
+					err := utils.RemovePolicyTemplateDBAnnotations(replicatedPlc)
+					g.Expect(err).ToNot(HaveOccurred())
+
 					return replicatedPlc.Object["spec"]
 				}, defaultTimeoutSeconds, 1).Should(utils.SemanticEqual(yamlPlc.Object["spec"]))
 			})
@@ -361,7 +373,7 @@ var _ = Describe("Test encrypted policy templates with secret copy", func() {
 
 				By("Verifying the replicated policy against a snapshot")
 				yamlPlc := utils.ParseYaml(case9PolicyYamlCopiedRepl + managedCluster + ".yaml")
-				Eventually(func() interface{} {
+				Eventually(func(g Gomega) interface{} {
 					replicatedPlc = utils.GetWithTimeout(
 						clientHubDynamic,
 						gvrPolicy,
@@ -371,6 +383,9 @@ var _ = Describe("Test encrypted policy templates with secret copy", func() {
 						defaultTimeoutSeconds,
 					)
 
+					err := utils.RemovePolicyTemplateDBAnnotations(replicatedPlc)
+					g.Expect(err).ToNot(HaveOccurred())
+
 					return replicatedPlc.Object["spec"]
 				}, defaultTimeoutSeconds, 1).Should(utils.SemanticEqual(yamlPlc.Object["spec"]))
 			})

test/utils/utils.go

@@ -24,6 +24,8 @@ import (
 	"k8s.io/client-go/kubernetes"
 	clusterv1beta1 "open-cluster-management.io/api/cluster/v1beta1"
 	appsv1 "open-cluster-management.io/multicloud-operators-subscription/pkg/apis/apps/placementrule/v1"
+
+	"open-cluster-management.io/governance-policy-propagator/controllers/propagator"
 )
 
 // GeneratePlrStatus generate plr status with given clusters
@@ -54,6 +56,56 @@ func GeneratePldStatus(
 	return &clusterv1beta1.PlacementDecisionStatus{Decisions: plrDecision}
 }
 
+func RemovePolicyTemplateDBAnnotations(plc *unstructured.Unstructured) error {
+	// Remove the database annotation since this can be an inconsistent value
+	templates, _, _ := unstructured.NestedSlice(plc.Object, "spec", "policy-templates")
+
+	updated := false
+
+	for i, template := range templates {
+		template := template.(map[string]interface{})
+
+		annotations, ok, _ := unstructured.NestedMap(
+			template, "objectDefinition", "metadata", "annotations",
+		)
+		if !ok {
+			continue
+		}
+
+		annotationVal, ok := annotations[propagator.PolicyIDAnnotation].(string)
+		if !ok {
+			continue
+		}
+
+		if annotationVal != "" {
+			delete(annotations, propagator.PolicyIDAnnotation)
+
+			if len(annotations) == 0 {
+				unstructured.RemoveNestedField(template, "objectDefinition", "metadata", "annotations")
+			} else {
+				err := unstructured.SetNestedField(
+					template, annotations, "objectDefinition", "metadata", "annotations",
+				)
+				if err != nil {
+					return err
+				}
+			}
+
+			templates[i] = template
+			updated = true
+		}
+	}
+
+	if updated {
+		err := unstructured.SetNestedField(plc.Object, templates, "spec", "policy-templates")
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // Pause sleep for given seconds
 func Pause(s uint) {
 	if s < 1 {