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"
@@ -517,8 +519,7 @@ func (r *OperatorPolicyReconciler) handleOpGroup(
  merged := opGroup.DeepCopy() // Copy it so that the value in the cache is not changed
 
  updateNeeded, skipUpdate, err := r.mergeObjects(
- ctx, desiredUnstruct, merged, string(policy.Spec.ComplianceType),
- )
+ ctx, desiredUnstruct, merged, string(policy.Spec.ComplianceType))
  if err != nil {
  return nil, false, fmt.Errorf("error checking if the OperatorGroup needs an update: %w", err)
  }
@@ -624,7 +625,9 @@ func (r *OperatorPolicyReconciler) handleSubscription(
 
  merged := foundSub.DeepCopy() // Copy it so that the value in the cache is not changed
 
- updateNeeded, skipUpdate, err := r.mergeObjects(ctx, desiredUnstruct, merged, string(policy.Spec.ComplianceType))
+ updateNeeded, skipUpdate, err := r.mergeObjects(
+ ctx, desiredUnstruct, merged, string(policy.Spec.ComplianceType),
+ )
  if err != nil {
  return nil, nil, false, fmt.Errorf("error checking if the Subscription needs an update: %w", err)
  }
@@ -653,19 +656,17 @@ func (r *OperatorPolicyReconciler) handleSubscription(
  }
 
  if includesSubscription {
- cond := metav1.Condition{
- Type: subConditionType,
- Status: metav1.ConditionFalse,
- Reason: subResFailed.Reason,
- Message: subResFailed.Message,
- }
-
- if subResFailed.LastTransitionTime != nil {
- cond.LastTransitionTime = *subResFailed.LastTransitionTime
+ // 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
  }
 
- 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 +728,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'