diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go index 1e01b80c..ae8c7eac 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" @@ -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) { 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' diff --git a/test/e2e/case38_install_operator_test.go b/test/e2e/case38_install_operator_test.go index db5d0905..835823ff 100644 --- a/test/e2e/case38_install_operator_test.go +++ b/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,