Fix constraints not satisfiable message causing compliance flood

ConstraintsNotSatisfiable message types in the subscription
status are formatted "constraints not satisfiable: clause1, clause2,
clause3 ...". However, the order of the clauses is nondeterministic,
which causes compliance event flapping as each update leads to a
different ordering of messages, which in turn triggers another update.
This was fixed by sorting the message prior to evaluation.

Ref: https://issues.redhat.com/browse/ACM-10204

Signed-off-by: Jeffrey Luo <jeluo@redhat.com>

controllers/operatorpolicy_controller.go

@@ -11,6 +11,8 @@ import (
 	"fmt"
 	"reflect"
 	"regexp"
+	"slices"
+	"strings"
 
 	operatorv1 "github.com/operator-framework/api/pkg/operators/v1"
 	operatorv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
@@ -653,19 +655,17 @@ func (r *OperatorPolicyReconciler) handleSubscription(
 			}
 
 			if includesSubscription {
-				cond := metav1.Condition{
-					Type:    subConditionType,
-					Status:  metav1.ConditionFalse,
-					Reason:  subResFailed.Reason,
-					Message: subResFailed.Message,
+				// a "constraints not satisfiable" message has nondeterministic clauses, and thus
+				// need to be sorted in order to check that they are not duplicates of the current message.
+				if constraintMessageMatch(policy, &subResFailed) {
+					return mergedSub, nil, false, nil
 				}
 
-				if subResFailed.LastTransitionTime != nil {
-					cond.LastTransitionTime = *subResFailed.LastTransitionTime
-				}
-
-				return mergedSub, nil, updateStatus(policy, cond, nonCompObj(foundSub, subResFailed.Reason)), nil
+				return mergedSub, nil, updateStatus(
+					policy, subResFailedCond(subResFailed), nonCompObj(foundSub, subResFailed.Reason)), nil
 			}
+
+			return mergedSub, nil, false, nil
 		}
 
 		return mergedSub, nil, updateStatus(policy, matchesCond("Subscription"), matchedObj(foundSub)), nil
@@ -727,6 +727,43 @@ func messageIncludesSubscription(subscription *operatorv1alpha1.Subscription, me
 	return regexp.MatchString(regex, message)
 }
 
+// constraintMessageMatch checks if the ConstraintsNotSatisfiable message is actually different
+// from the old one by sorting the clauses of the message
+func constraintMessageMatch(policy *policyv1beta1.OperatorPolicy, cond *operatorv1alpha1.SubscriptionCondition) bool {
+	const cnfPrefix = "constraints not satisfiable: "
+
+	var policyMessage, subMessage string
+
+	for _, cond := range policy.Status.Conditions {
+		if strings.Contains(cond.Message, cnfPrefix) {
+			policyMessage = cond.Message
+		}
+	}
+
+	if policyMessage == "" {
+		return false
+	}
+
+	if strings.Contains(cond.Message, cnfPrefix) {
+		subMessage = cond.Message
+	} else {
+		return false
+	}
+
+	policyMessage = strings.TrimPrefix(policyMessage, cnfPrefix)
+	subMessage = strings.TrimPrefix(subMessage, cnfPrefix)
+
+	// The ConstraintsNotSatisfiable message is always formatted as follows:
+	// constraints not satisfiable: clause1, clause2, clause3 ...
+	policyMessageSlice := strings.Split(policyMessage, ", ")
+	slices.Sort(policyMessageSlice)
+
+	subMessageSlice := strings.Split(subMessage, ", ")
+	slices.Sort(subMessageSlice)
+
+	return reflect.DeepEqual(policyMessageSlice, subMessageSlice)
+}
+
 func (r *OperatorPolicyReconciler) handleInstallPlan(
 	ctx context.Context, policy *policyv1beta1.OperatorPolicy, sub *operatorv1alpha1.Subscription,
 ) (bool, error) {

controllers/operatorpolicy_controller_test.go

@@ -199,3 +199,62 @@ func TestMessageIncludesSubscription(t *testing.T) {
 		)
 	}
 }
+
+func TestMessageContentOrderMatching(t *testing.T) {
+	t.Parallel()
+
+	testPolicy := &policyv1beta1.OperatorPolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-policy",
+			Namespace: "default",
+		},
+		Spec: policyv1beta1.OperatorPolicySpec{
+			Severity:          "low",
+			RemediationAction: "enforce",
+			ComplianceType:    "musthave",
+			Subscription: runtime.RawExtension{
+				Raw: []byte(`{
+					"source": "my-catalog",
+					"sourceNamespace": "my-ns",
+					"name": "my-operator",
+					"channel": "stable",
+					"startingCSV": "my-operator-v1",
+					"installPlanApproval": "Automatic"
+				}`),
+			},
+		},
+		Status: policyv1beta1.OperatorPolicyStatus{
+			ComplianceState: "NonCompliant",
+			Conditions: []metav1.Condition{
+				{
+					Type:               "SubscriptionCompliant",
+					Status:             "False",
+					ObservedGeneration: 0,
+					LastTransitionTime: metav1.Now(),
+					Reason:             "ConstraintsNotSatisfiable",
+					Message: "constraints not satisfiable: " +
+						"no operators found in package gatekeeper-operator-product " +
+						"in the catalog referenced by subscription gatekeeper-operator-product, " +
+						"clusterserviceversion gatekeeper-operator-product.v3.11.1 exists " +
+						"and is not referenced by a subscription, " +
+						"subscription gatekeeper-operator-product4 exists",
+				},
+			},
+		},
+	}
+
+	testCond := &operatorv1alpha1.SubscriptionCondition{
+		Type:   "ResolutionFailed",
+		Status: "True",
+		Reason: "ConstraintsNotSatisfiable",
+		Message: "constraints not satisfiable: " +
+			"subscription gatekeeper-operator-product4 exists, " +
+			"no operators found in package gatekeeper-operator-product " +
+			"in the catalog referenced by subscription gatekeeper-operator-product, " +
+			"clusterserviceversion gatekeeper-operator-product.v3.11.1 exists " +
+			"and is not referenced by a subscription",
+	}
+
+	ret := constraintMessageMatch(testPolicy, testCond)
+	assert.Equal(t, true, ret)
+}

controllers/operatorpolicy_status.go

@@ -473,6 +473,22 @@ func validationCond(validationErrors []error) metav1.Condition {
 	}
 }
 
+// subResFailedCond takes a failed SubscriptionCondition and converts it to a generic Condition
+func subResFailedCond(subFailedCond operatorv1alpha1.SubscriptionCondition) metav1.Condition {
+	cond := metav1.Condition{
+		Type:    subConditionType,
+		Status:  metav1.ConditionFalse,
+		Reason:  subFailedCond.Reason,
+		Message: subFailedCond.Message,
+	}
+
+	if subFailedCond.LastTransitionTime != nil {
+		cond.LastTransitionTime = *subFailedCond.LastTransitionTime
+	}
+
+	return cond
+}
+
 // opGroupPreexistingCond is a Compliant condition with Reason 'PreexistingOperatorGroupFound',
 // and Message 'the policy does not specify an OperatorGroup but one already exists in the
 // namespace - assuming that OperatorGroup is correct'