diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go index 1e01b80c..df016237 100644 --- a/controllers/operatorpolicy_controller.go +++ b/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) { diff --git a/controllers/operatorpolicy_controller_test.go b/controllers/operatorpolicy_controller_test.go index c183d838..55ef19bb 100644 --- a/controllers/operatorpolicy_controller_test.go +++ b/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) +} diff --git a/controllers/operatorpolicy_status.go b/controllers/operatorpolicy_status.go index 6d1f70f8..20ed84f3 100644 --- a/controllers/operatorpolicy_status.go +++ b/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'