Fix status for invalid object

Previously, a config policy with an invalid field in the object definition would cause the policy status to flip between "object not found" and "object could not be created/updated" indefinitely, which became an issue at scale. This change makes it so that an invalid template will only cause one violation event, which lines up with the single event sent out when a policy template has an invalid field.

ref: https://issues.redhat.com/browse/ACM-3465?filter=-1

Signed-off-by: Will Kutler <wkutler@redhat.com>

controllers/configurationpolicy_controller.go

@@ -1568,24 +1568,29 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj(
 	if !exists && obj.shouldExist {
 		// it is a musthave and it does not exist, so it must be created
 		if strings.EqualFold(string(remediation), string(policyv1.Enforce)) {
-			// object is missing, so send noncompliant event
-			_ = createStatus("", obj.gvr.Resource, compliantObject, obj.namespaced, obj.policy,
-				obj.index, false, true)
-			obj.policy.Status.ComplianceState = policyv1.NonCompliant
-			statusStr := convertPolicyStatusToString(obj.policy)
-			objLog.Info("Sending an update policy status event", "policy", obj.policy.Name, "status", statusStr)
-			r.Recorder.Event(obj.policy, eventWarning, fmt.Sprintf(eventFmtStr, obj.policy.GetName(), obj.name),
-				statusStr)
-			// update parent policy status
-			r.addForUpdate(obj.policy, true)
-
 			var uid string
 			statusUpdateNeeded, uid, err = r.enforceByCreatingOrDeleting(obj)
 
 			if err != nil {
 				// violation created for handling error
 				objLog.Error(err, "Could not handle missing musthave object")
 			} else {
+				// object is missing and will be created, so send noncompliant "does not exist" event first
+				// (this check has already happened, but we send the event here to avoid the status flipping on an
+				// error)
+				_ = createStatus("", obj.gvr.Resource, compliantObject, obj.namespaced, obj.policy,
+					obj.index, false, true)
+				obj.policy.Status.ComplianceState = policyv1.NonCompliant
+				statusStr := convertPolicyStatusToString(obj.policy)
+				objLog.Info("Sending a noncompliant status event (object missing)", "policy", obj.policy.Name, "status",
+					statusStr)
+				r.Recorder.Event(obj.policy, eventWarning, fmt.Sprintf(eventFmtStr, obj.policy.GetName(), obj.name),
+					statusStr)
+				// update parent policy status
+				r.addForUpdate(obj.policy, true)
+
+				fmt.Println("CREATED INFORM STATUS")
+
 				created := true
 				creationInfo = &policyv1.ObjectProperties{
 					CreatedByPolicy: &created,
@@ -1638,7 +1643,8 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj(
 		throwSpecViolation, msg, pErr, triedUpdate, updatedObj := r.checkAndUpdateResource(obj, compType, mdCompType,
 			remediation)
 
-		if triedUpdate {
+		if triedUpdate && !strings.Contains(msg, "Error validating the object") {
+			fmt.Println("UPDATING STATUS")
 			// object has a mismatch and needs an update to be enforced, throw violation for mismatch
 			_ = createStatus("", obj.gvr.Resource, compliantObject, obj.namespaced, obj.policy, obj.index,
 				false, true)

test/e2e/case23_invalid_field_test.go

@@ -4,6 +4,7 @@ package e2e
 
 import (
 	"context"
+	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -39,6 +40,23 @@ var _ = Describe("Test an objectDefinition with an invalid field", Ordered, func
 			g.Expect(utils.GetStatusMessage(managedPlc)).To(Equal(expectedMsg))
 		}, defaultTimeoutSeconds, 1).Should(Succeed())
 
+		By("Verifying events do not continue to be created after the first violation for existing objects")
+		startTime := metav1.NewMicroTime(time.Now())
+
+		Consistently(func() interface{} {
+			compPlcEvents := utils.GetMatchingEvents(clientManaged, testNamespace,
+				policyName,
+				"",
+				"unknown field \"invalid\" in io.k8s.api.core.v1.ConfigMap",
+				defaultTimeoutSeconds)
+
+			if len(compPlcEvents) == 0 {
+				return false
+			}
+
+			return compPlcEvents[0].EventTime.Before(&startTime)
+		}, defaultTimeoutSeconds, 1).Should(BeTrue())
+
 		By("Verifying the message is correct when the " + configMapName + " ConfigMap already exists")
 		configmap := &corev1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
@@ -57,6 +75,23 @@ var _ = Describe("Test an objectDefinition with an invalid field", Ordered, func
 
 			return utils.GetStatusMessage(managedPlc)
 		}, defaultTimeoutSeconds, 1).Should(Equal(expectedMsg))
+
+		By("Verifying events do not continue to be created after the first violation for existing objects")
+		alreadyExistsStartTime := metav1.NewMicroTime(time.Now())
+
+		Consistently(func() interface{} {
+			compPlcEvents := utils.GetMatchingEvents(clientManaged, testNamespace,
+				policyName,
+				"",
+				"unknown field \"invalid\" in io.k8s.api.core.v1.ConfigMap",
+				defaultTimeoutSeconds)
+
+			if len(compPlcEvents) == 0 {
+				return false
+			}
+
+			return compPlcEvents[0].EventTime.Before(&alreadyExistsStartTime)
+		}, defaultTimeoutSeconds, 1).Should(BeTrue())
 	})
 
 	AfterAll(func() {