Fix objectSelector polling

The objectSelector was always running in watch
mode regardless of the evaluationInterval.

ref: https://issues.redhat.com/browse/ACM-15952
Signed-off-by: Dale Haiducek <[email protected]>

Author: dhaiducek

controllers/configurationpolicy_controller.go

@@ -1422,9 +1422,30 @@ func (r *ConfigurationPolicyReconciler) determineDesiredObjects(
 			return nil, &scopedGVR, errEvent, err
 		}
 
+		usingWatch := currentlyUsingWatch(plc)
+		listOpts := metav1.ListOptions{
+			LabelSelector: objSelector.String(),
+		}
+
 		// Has a valid objectSelector, so list the names for each namespace using the objectSelector
 		for ns := range relevantNsNames {
-			filteredObjects, err := r.DynamicWatcher.List(plc.ObjectIdentifier(), objGVK, ns, objSelector)
+			var filteredObjects []unstructured.Unstructured
+			var err error
+
+			// If watch is enabled, use the dynamic watcher, otherwise use the controller dynamic client
+			if usingWatch {
+				filteredObjects, err = r.DynamicWatcher.List(plc.ObjectIdentifier(), objGVK, ns, objSelector)
+			} else {
+				var filteredObjectList *unstructured.UnstructuredList
+				filteredObjectList, err = r.TargetK8sDynamicClient.Resource(
+					scopedGVR.GroupVersionResource,
+				).Namespace(ns).List(context.TODO(), listOpts)
+
+				if err == nil {
+					filteredObjects = filteredObjectList.Items
+				}
+			}
+
 			if err != nil {
 				log.Error(err, "Failed to fetch the resources",
 					"objectSelector", fmt.Sprint(objectT.ObjectSelector.String()))

test/e2e/case42_resource_selection_test.go

@@ -8,6 +8,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
 	"open-cluster-management.io/config-policy-controller/test/utils"
 )
@@ -238,3 +239,111 @@ var _ = Describe("Test behavior of resource selection as resources change", Orde
 			"fakeapis [case42-3-e2e] found as specified in namespace %s", targetNs)))
 	})
 })
+
+var _ = Describe("Test evaluation of resource selection", Ordered, func() {
+	const (
+		targetNs   = "case42-e2e-3"
+		prereqYaml = "../resources/case42_resource_selector/case42_eval_prereq.yaml"
+		policyYaml = "../resources/case42_resource_selector/case42_eval_policy.yaml"
+		policyName = "case42-selector-eval-e2e"
+	)
+
+	BeforeAll(func() {
+		By("Applying prerequisites")
+		utils.Kubectl("apply", "-n", targetNs, "-f", prereqYaml)
+		DeferCleanup(func() {
+			utils.KubectlDelete("-n", targetNs, "-f", prereqYaml)
+		})
+
+		utils.Kubectl("apply", "-f", policyYaml, "-n", testNamespace)
+		DeferCleanup(func() {
+			utils.KubectlDelete("-f", policyYaml, "-n", testNamespace)
+		})
+
+		By("Verifying initial compliance message")
+		Eventually(func() interface{} {
+			managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+				policyName, testNamespace, true, defaultTimeoutSeconds)
+
+			return utils.GetStatusMessage(managedPlc)
+		}, defaultTimeoutSeconds, 1).Should(Equal(fmt.Sprintf(
+			"fakeapis [case42-1-e2e] found but not as specified in namespace %s", targetNs)))
+	})
+
+	It("does not re-evaluate when the evaluation interval is in watch", func() {
+		managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+			policyName, testNamespace, true, defaultTimeoutSeconds)
+
+		evalTime, found, err := unstructured.NestedString(managedPlc.Object, "status", "lastEvaluated")
+		Expect(evalTime).ToNot(BeEmpty())
+		Expect(found).To(BeTrue())
+		Expect(err).ToNot(HaveOccurred())
+
+		By("Verifying it does not evaluate again")
+		Consistently(func() interface{} {
+			managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+				policyName, testNamespace, true, defaultTimeoutSeconds)
+
+			newestEvalTime, found, err := unstructured.NestedString(managedPlc.Object, "status", "lastEvaluated")
+			Expect(newestEvalTime).ToNot(BeEmpty())
+			Expect(found).To(BeTrue())
+			Expect(err).ToNot(HaveOccurred())
+
+			return newestEvalTime
+		}, defaultConsistentlyDuration, "3s").Should(Equal(evalTime))
+	})
+
+	FIt("re-evaluates when the evaluation interval is set to a duration", func() {
+		managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+			policyName, testNamespace, true, defaultTimeoutSeconds)
+
+		By("Fetching the current evaluation timestamp")
+		currentEvalTime, found, err := unstructured.NestedString(managedPlc.Object, "status", "lastEvaluated")
+		Expect(currentEvalTime).ToNot(BeEmpty())
+		Expect(found).To(BeTrue())
+		Expect(err).ToNot(HaveOccurred())
+
+		By("Updating the evaluationInterval to 5s")
+		utils.Kubectl("patch", "--namespace=managed", "configurationpolicy", policyName, "--type=json",
+			`--patch=[{
+				"op": "replace",
+				"path": "/spec/evaluationInterval",
+				"value": {
+					"compliant": "5s",
+					"noncompliant": "5s",
+				}
+			}]`,
+		)
+
+		var nextEvalTime string
+
+		By("Waiting for the first evaluation after the spec update")
+		Eventually(func() interface{} {
+			managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+				policyName, testNamespace, true, defaultTimeoutSeconds)
+
+			nextEvalTime, found, err := unstructured.NestedString(managedPlc.Object, "status", "lastEvaluated")
+			Expect(nextEvalTime).ToNot(BeEmpty())
+			Expect(found).To(BeTrue())
+			Expect(err).ToNot(HaveOccurred())
+
+			return nextEvalTime
+		}, defaultTimeoutSeconds, 1).ShouldNot(Equal(currentEvalTime))
+
+		By("Verifying it evaluates each interval")
+		Consistently(func() interface{} {
+			currentEvalTime = nextEvalTime
+			managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+				policyName, testNamespace, true, defaultTimeoutSeconds)
+
+			nextEvalTime, found, err := unstructured.NestedString(managedPlc.Object, "status", "lastEvaluated")
+			Expect(nextEvalTime).ToNot(BeEmpty())
+			Expect(found).To(BeTrue())
+			Expect(err).ToNot(HaveOccurred())
+			Expect(utils.GetStatusMessage(managedPlc)).To(Equal(fmt.Sprintf(
+				"fakeapis [case42-1-e2e] found but not as specified in namespace %s", targetNs)))
+
+			return nextEvalTime
+		}, defaultConsistentlyDuration*2, "10s").ShouldNot(Equal(currentEvalTime))
+	})
+})

test/resources/case42_resource_selector/case42_eval_policy.yaml

@@ -0,0 +1,22 @@
+apiVersion: policy.open-cluster-management.io/v1
+kind: ConfigurationPolicy
+metadata:
+  name: case42-selector-eval-e2e
+spec:
+  evaluationInterval:
+    compliant: watch
+    noncompliant: watch
+  remediationAction: inform
+  object-templates:
+    - complianceType: musthave
+      objectSelector:
+        matchExpressions:
+          - key: case42
+            operator: Exists
+      objectDefinition:
+        apiVersion: config-policy-controller.io/v1
+        kind: FakeAPI
+        metadata:
+          labels:
+            selected: "true"
+          namespace: case42-e2e-3

test/resources/case42_resource_selector/case42_eval_prereq.yaml

@@ -0,0 +1,11 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: case42-e2e-3
+---
+apiVersion: config-policy-controller.io/v1
+kind: FakeAPI
+metadata:
+  labels:
+    case42: case42-1-e2e
+  name: case42-1-e2e