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>
(cherry picked from commit 182d147)

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,37 @@ 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 _, statusCond := range policy.Status.Conditions {
+		if strings.Contains(statusCond.Message, cnfPrefix) {
+			policyMessage = statusCond.Message
+		}
+	}
+
+	if policyMessage == "" || !strings.Contains(cond.Message, cnfPrefix) {
+		return false
+	}
+
+	policyMessage = strings.TrimPrefix(policyMessage, cnfPrefix)
+	subMessage = strings.TrimPrefix(cond.Message, 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'

test/e2e/case38_install_operator_test.go

@@ -3,7 +3,10 @@ package e2e
 import (
 	"encoding/json"
 	"fmt"
+	"reflect"
 	"regexp"
+	"slices"
+	"strings"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -84,11 +87,30 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun
 			}
 
 			idx, actualCondition := policy.Status.GetCondition(expectedCondition.Type)
+
 			g.Expect(idx).NotTo(Equal(-1))
 			g.Expect(actualCondition.Status).To(Equal(expectedCondition.Status))
 			g.Expect(actualCondition.Reason).To(Equal(expectedCondition.Reason))
-			g.Expect(actualCondition.Message).To(MatchRegexp(
-				fmt.Sprintf(".*%v.*", regexp.QuoteMeta(expectedCondition.Message))))
+
+			const cnfPrefix = "constraints not satisfiable: "
+			if strings.Contains(actualCondition.Message, cnfPrefix) &&
+				strings.Contains(expectedCondition.Message, cnfPrefix) {
+				// need to sort message before checking
+
+				expectedMessage := strings.TrimPrefix(expectedCondition.Message, cnfPrefix)
+				actualMessage := strings.TrimPrefix(actualCondition.Message, cnfPrefix)
+
+				expectedMessageSlice := strings.Split(expectedMessage, ", ")
+				slices.Sort(expectedMessageSlice)
+
+				actualMessageSlice := strings.Split(actualMessage, ", ")
+				slices.Sort(actualMessageSlice)
+
+				g.Expect(reflect.DeepEqual(expectedMessageSlice, actualMessageSlice)).To(BeTrue())
+			} else {
+				g.Expect(actualCondition.Message).To(MatchRegexp(
+					fmt.Sprintf(".*%v.*", regexp.QuoteMeta(expectedCondition.Message))))
+			}
 
 			events := utils.GetMatchingEvents(
 				clientManaged, opPolTestNS, parentPolicyName, "", expectedEventMsgSnippet, eventuallyTimeout,