Test related object properties present for any remediation action

ref: https://issues.redhat.com/browse/ACM-14577

Tests scenarios where the policy reports a mismatched object,
but the dry run validation reports a compliant object due to
the apiserver adding or modifying fields in the request body.

This commit also prevents timeouts in case40 by wrapping assertions with
Eventually.

Signed-off-by: Janelle Law <[email protected]>

Author: jan-law

controllers/configurationpolicy_controller_test.go

@@ -705,6 +705,75 @@ func TestUpdatedRelatedObjects(t *testing.T) {
 	assert.Equal(t, "bar", relatedList[0].Object.Metadata.Name)
 }
 
+func TestAddRelatedObjectProperties(t *testing.T) {
+	compliant := true
+	scopedGVR := depclient.ScopedGVR{
+		GroupVersionResource: policyv1.SchemeBuilder.GroupVersion.WithResource("ConfigurationPolicy"),
+		Namespaced:           true,
+	}
+	namespace := "default"
+	name := "foo"
+	reason := "reason"
+
+	// Create test data for ObjectProperties
+	createdByPolicy := true
+	testUID := "test-uid-12345"
+	testDiff := "test diff content"
+	dryRunMatches := true
+
+	creationInfo := &policyv1.ObjectProperties{
+		CreatedByPolicy:    &createdByPolicy,
+		UID:                testUID,
+		Diff:               testDiff,
+		MatchesAfterDryRun: dryRunMatches,
+	}
+
+	relatedList := addRelatedObjects(
+		compliant, scopedGVR, "ConfigurationPolicy", namespace, []string{name}, reason, creationInfo,
+	)
+	related := relatedList[0]
+
+	// Validate that the properties are correctly set
+	assert.NotNil(t, related.Properties, "Properties should not be nil when creationInfo is provided")
+	assert.NotNil(t, related.Properties.CreatedByPolicy, "CreatedByPolicy should not be nil")
+	assert.Equal(t, createdByPolicy, *related.Properties.CreatedByPolicy)
+	assert.Equal(t, testUID, related.Properties.UID)
+	assert.Equal(t, testDiff, related.Properties.Diff)
+	assert.Equal(t, dryRunMatches, related.Properties.MatchesAfterDryRun)
+
+	// add the same object and make sure the existing one is overwritten
+	reason = "new"
+	compliant = false
+
+	// Create new test data for ObjectProperties with different values
+	newCreatedByPolicy := false
+	newTestDiff := "updated diff content"
+	newDryRunMatches := false
+
+	newCreationInfo := &policyv1.ObjectProperties{
+		CreatedByPolicy:    &newCreatedByPolicy,
+		UID:                testUID,
+		Diff:               newTestDiff,
+		MatchesAfterDryRun: newDryRunMatches,
+	}
+
+	relatedList = addRelatedObjects(
+		compliant, scopedGVR, "ConfigurationPolicy", namespace, []string{name}, reason, newCreationInfo)
+	related = relatedList[0]
+
+	assert.Len(t, relatedList, 1)
+	assert.Equal(t, string(policyv1.NonCompliant), related.Compliant)
+	assert.Equal(t, "new", related.Reason)
+
+	// Validate that the properties were updated with new values
+	assert.NotNil(t, related.Properties, "Properties should not be nil when newCreationInfo is provided")
+	assert.NotNil(t, related.Properties.CreatedByPolicy, "CreatedByPolicy should not be nil")
+	assert.Equal(t, newCreatedByPolicy, *related.Properties.CreatedByPolicy)
+	assert.Equal(t, testUID, related.Properties.UID)
+	assert.Equal(t, newTestDiff, related.Properties.Diff)
+	assert.Equal(t, newDryRunMatches, related.Properties.MatchesAfterDryRun)
+}
+
 func TestCreateStatus(t *testing.T) {
 	testcases := []struct {
 		testName          string

test/e2e/case20_delete_objects_test.go

@@ -130,26 +130,40 @@ var _ = Describe("Test Object deletion", Ordered, func() {
 				return properties["uid"]
 			}, defaultTimeoutSeconds, 1).ShouldNot(BeEmpty())
 		})
-		It("should not update status field for inform policies", func() {
+		It("should update status field for inform policies", func() {
 			By("Creating " + case20ConfigPolicyNameInform + " on managed")
 			utils.Kubectl("apply", "-f", case20PolicyYamlInform, "-n", testNamespace)
 			plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
 				case20ConfigPolicyNameInform, testNamespace, true, defaultTimeoutSeconds)
 			Expect(plc).NotTo(BeNil())
+
+			var managedPlc *unstructured.Unstructured
+
 			Eventually(func(g Gomega) {
-				managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+				managedPlc = utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
 					case20ConfigPolicyNameInform, testNamespace, true, defaultTimeoutSeconds)
 
 				utils.CheckComplianceStatus(g, managedPlc, "Compliant")
 			}, defaultTimeoutSeconds, 1).Should(Succeed())
-			Eventually(func() interface{} {
-				managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
-					case20ConfigPolicyNameInform, testNamespace, true, defaultTimeoutSeconds)
-				relatedObj := managedPlc.Object["status"].(map[string]interface{})["relatedObjects"].([]interface{})[0]
-				properties := relatedObj.(map[string]interface{})["properties"]
 
-				return properties
-			}, defaultTimeoutSeconds, 1).Should(BeNil())
+			By("Verifying the related object properties")
+			relatedObjects, _, err := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(relatedObjects).To(HaveLen(1))
+
+			relatedObj := relatedObjects[0].(map[string]interface{})
+
+			createdByPolicy, found, _ := unstructured.NestedBool(relatedObj, "properties", "createdByPolicy")
+			Expect(found).To(BeTrue())
+			Expect(createdByPolicy).To(BeFalse())
+
+			uid, found, _ := unstructured.NestedString(relatedObj, "properties", "uid")
+			Expect(found).To(BeTrue())
+			Expect(uid).ToNot(BeEmpty())
+
+			matchesAfterDryRun, found, _ := unstructured.NestedBool(relatedObj, "properties", "matchesAfterDryRun")
+			Expect(found).To(BeTrue())
+			Expect(matchesAfterDryRun).To(BeTrue())
 		})
 		AfterAll(func() {
 			policies := []string{

test/e2e/case40_recreate_option_test.go

@@ -117,7 +117,7 @@ var _ = Describe("Recreate options", Ordered, func() {
 
 		By("Verifying the ConfigurationPolicy is Compliant")
 		var managedPlc *unstructured.Unstructured
-
+		var propsUID string
 		Eventually(func(g Gomega) {
 			managedPlc = utils.GetWithTimeout(
 				clientManagedDynamic,
@@ -129,37 +129,39 @@ var _ = Describe("Recreate options", Ordered, func() {
 			)
 
 			utils.CheckComplianceStatus(g, managedPlc, "Compliant")
-		}, defaultTimeoutSeconds, 1).Should(Succeed())
 
-		By("Verifying the diff is not set")
-		relatedObjects, _, err := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects")
-		Expect(err).ToNot(HaveOccurred())
-		Expect(relatedObjects).To(HaveLen(1))
+			relatedObjects, _, err := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects")
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(relatedObjects).To(HaveLen(1))
 
-		relatedObject := relatedObjects[0].(map[string]interface{})
+			relatedObject := relatedObjects[0].(map[string]interface{})
 
-		diff, _, _ := unstructured.NestedString(relatedObject, "properties", "diff")
-		Expect(diff).To(BeEmpty())
+			diff, _, _ := unstructured.NestedString(relatedObject, "properties", "diff")
+			g.Expect(diff).To(BeEmpty())
 
-		propsUID, _, _ := unstructured.NestedString(relatedObject, "properties", "uid")
-		Expect(propsUID).ToNot(BeEmpty())
+			propsUID, _, _ = unstructured.NestedString(relatedObject, "properties", "uid")
+			g.Expect(propsUID).ToNot(BeEmpty())
 
-		createdByPolicy, _, _ := unstructured.NestedBool(relatedObject, "properties", "createdByPolicy")
-		Expect(createdByPolicy).To(BeTrue())
+			createdByPolicy, _, _ := unstructured.NestedBool(relatedObject, "properties", "createdByPolicy")
+			g.Expect(createdByPolicy).To(BeTrue())
 
-		deployment, err = clientManagedDynamic.Resource(gvrDeployment).Namespace("default").Get(
-			ctx, "case40", metav1.GetOptions{},
-		)
-		Expect(err).ToNot(HaveOccurred())
+			deployment, err = clientManagedDynamic.Resource(gvrDeployment).Namespace("default").Get(
+				ctx, "case40", metav1.GetOptions{},
+			)
+			g.Expect(err).ToNot(HaveOccurred())
+		}, defaultTimeoutSeconds, 1).Should(Succeed())
 
 		By("Verifying the Deployment was recreated")
-		Expect(deployment.GetUID()).ToNot(Equal(uid), "Expected a new UID on the Deployment after it got recreated")
-		Expect(propsUID).To(
-			BeEquivalentTo(deployment.GetUID()), "Expect the object properties UID to match the new Deployment",
-		)
+		Eventually(func(g Gomega) {
+			g.Expect(deployment.GetUID()).ToNot(
+				Equal(uid), "Expected a new UID on the Deployment after it got recreated")
+			g.Expect(propsUID).To(
+				BeEquivalentTo(deployment.GetUID()), "Expect the object properties UID to match the new Deployment",
+			)
 
-		selector, _, _ := unstructured.NestedString(deployment.Object, "spec", "selector", "matchLabels", "app")
-		Expect(selector).To(Equal("case40-2"))
+			selector, _, _ := unstructured.NestedString(deployment.Object, "spec", "selector", "matchLabels", "app")
+			g.Expect(selector).To(Equal("case40-2"))
+		}, defaultTimeoutSeconds, 1).Should(Succeed())
 
 		deleteConfigPolicies([]string{"case40"})
 	})

test/e2e/case8_status_check_test.go

@@ -171,3 +171,133 @@ var _ = Describe("Test pod obj template handling", func() {
 		})
 	})
 })
+
+var _ = Describe("Test related object property status", Ordered, func() {
+	Describe("Create a policy missing a field added by kubernetes", Ordered, func() {
+		const (
+			policyNoType       = "policy-service-no-type"
+			policyWithType     = "policy-service-with-type"
+			serviceName        = "grc-policy-propagator-metrics"
+			policyNoTypeYAML   = "../resources/case8_status_check/case8_service_no_type_inform.yaml"
+			policyWithTypeYAML = "../resources/case8_status_check/case8_service_with_type_inform.yaml"
+			serviceYAML        = "../resources/case8_status_check/case8_service_with_type.yaml"
+		)
+
+		It("Should be compliant when the inform policy omits a field added by the apiserver", func() {
+			By("Creating a Service with explicit type: ClusterIP")
+			utils.Kubectl("apply", "-f", serviceYAML)
+
+			By("Creating the " + policyNoType + " policy that omits the type field)")
+			utils.Kubectl("apply", "-f", policyNoTypeYAML, "-n", testNamespace)
+
+			By("Verifying that the " + policyNoType + " policy is compliant")
+			var managedPlc *unstructured.Unstructured
+
+			Eventually(func(g Gomega) {
+				managedPlc = utils.GetWithTimeout(
+					clientManagedDynamic, gvrConfigPolicy, policyNoType, testNamespace, true, defaultTimeoutSeconds,
+				)
+
+				utils.CheckComplianceStatus(g, managedPlc, "Compliant")
+			}, defaultTimeoutSeconds, 1).Should(Succeed())
+
+			By("Verifying that matchesAfterDryRun is false")
+			relatedObjects, _, err := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(relatedObjects).To(HaveLen(1))
+
+			relatedObj := relatedObjects[0].(map[string]interface{})
+
+			matchesAfterDryRun, _, _ := unstructured.NestedBool(relatedObj, "properties", "matchesAfterDryRun")
+
+			Expect(matchesAfterDryRun).To(BeFalse())
+		})
+
+		It("Should be compliant when the inform policy includes all fields", func() {
+			By("Creating the " + policyWithType + " policy that includes the type field)")
+			utils.Kubectl("apply", "-f", policyWithTypeYAML, "-n", testNamespace)
+
+			By("Verifying that the " + policyWithType + " policy is compliant")
+			var managedPlc *unstructured.Unstructured
+
+			Eventually(func(g Gomega) {
+				managedPlc = utils.GetWithTimeout(
+					clientManagedDynamic, gvrConfigPolicy, policyWithType, testNamespace, true, defaultTimeoutSeconds,
+				)
+
+				utils.CheckComplianceStatus(g, managedPlc, "Compliant")
+			}, defaultTimeoutSeconds, 1).Should(Succeed())
+
+			By("Verifying that matchesAfterDryRun is true")
+			relatedObjects, _, err := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(relatedObjects).To(HaveLen(1))
+
+			relatedObj := relatedObjects[0].(map[string]interface{})
+
+			matchesAfterDryRun, _, _ := unstructured.NestedBool(relatedObj, "properties", "matchesAfterDryRun")
+
+			Expect(matchesAfterDryRun).To(BeTrue())
+		})
+
+		It("Should be compliant when the enforce policy omits a field added by the apiserver", func() {
+			By("Changing the policy to enforce mode")
+			utils.EnforceConfigurationPolicy(policyNoType, testNamespace)
+
+			By("Verifying that the " + policyNoType + " policy is compliant")
+			var managedPlc *unstructured.Unstructured
+
+			Eventually(func(g Gomega) {
+				managedPlc = utils.GetWithTimeout(
+					clientManagedDynamic, gvrConfigPolicy, policyNoType, testNamespace, true, defaultTimeoutSeconds,
+				)
+
+				utils.CheckComplianceStatus(g, managedPlc, "Compliant")
+			}, defaultTimeoutSeconds, 1).Should(Succeed())
+
+			By("Verifying that matchesAfterDryRun is false")
+			relatedObjects, _, err := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(relatedObjects).To(HaveLen(1))
+
+			relatedObj := relatedObjects[0].(map[string]interface{})
+
+			matchesAfterDryRun, _, _ := unstructured.NestedBool(relatedObj, "properties", "matchesAfterDryRun")
+
+			Expect(matchesAfterDryRun).To(BeFalse())
+		})
+
+		It("Should be compliant when the enforce policy includes all fields", func() {
+			By("Changing the policy to enforce mode")
+			utils.EnforceConfigurationPolicy(policyWithType, testNamespace)
+
+			By("Verifying that the " + policyWithType + " policy is compliant")
+			var managedPlc *unstructured.Unstructured
+
+			Eventually(func(g Gomega) {
+				managedPlc = utils.GetWithTimeout(
+					clientManagedDynamic, gvrConfigPolicy, policyWithType, testNamespace, true, defaultTimeoutSeconds,
+				)
+
+				utils.CheckComplianceStatus(g, managedPlc, "Compliant")
+			}, defaultTimeoutSeconds, 1).Should(Succeed())
+
+			By("Verifying that matchesAfterDryRun is true")
+			relatedObjects, _, err := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(relatedObjects).To(HaveLen(1))
+
+			relatedObj := relatedObjects[0].(map[string]interface{})
+
+			matchesAfterDryRun, _, _ := unstructured.NestedBool(relatedObj, "properties", "matchesAfterDryRun")
+
+			Expect(matchesAfterDryRun).To(BeTrue())
+		})
+
+		AfterAll(func() {
+			deleteConfigPolicies([]string{policyNoType, policyWithType})
+
+			utils.KubectlDelete("service", serviceName, "-n", "managed")
+		})
+	})
+})

test/resources/case8_status_check/case8_service_no_type_inform.yaml

@@ -0,0 +1,36 @@
+apiVersion: policy.open-cluster-management.io/v1
+kind: ConfigurationPolicy
+metadata:
+  name: policy-service-no-type
+  namespace: managed
+spec:
+  remediationAction: inform
+  severity: low
+  object-templates:
+  - complianceType: mustonlyhave
+    metadataComplianceType: musthave
+    objectDefinition:
+      apiVersion: v1
+      kind: Service
+      metadata:
+        name: grc-policy-propagator-metrics
+        namespace: managed
+      spec: # Intentionally omitting 'type' field - K8s will default to ClusterIP 
+        clusterIP: '{{ (lookup "v1" "Service" "managed" "grc-policy-propagator-metrics").spec.clusterIP }}'
+        clusterIPs:
+        - '{{ (lookup "v1" "Service" "managed" "grc-policy-propagator-metrics").spec.clusterIP }}'
+        internalTrafficPolicy: Cluster
+        ipFamilies:
+        - IPv4
+        ipFamilyPolicy: SingleStack
+        ports:
+        - name: metrics-https
+          port: 8443
+          protocol: TCP
+          targetPort: 8443
+        selector:
+          app: grc
+          component: ocm-policy-propagator
+          release: grc
+        sessionAffinity: None
+

test/resources/case8_status_check/case8_service_with_type.yaml

@@ -0,0 +1,23 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: grc-policy-propagator-metrics
+  namespace: managed
+  annotations:
+    test: test
+spec:
+  internalTrafficPolicy: Cluster
+  ipFamilies:
+  - IPv4
+  ipFamilyPolicy: SingleStack
+  ports:
+  - name: metrics-https
+    port: 8443
+    protocol: TCP
+    targetPort: 8443
+  selector:
+    app: grc
+    component: ocm-policy-propagator
+    release: grc
+  sessionAffinity: None
+  type: ClusterIP