diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go index 3a3a75eb..28ef5637 100644 --- a/controllers/operatorpolicy_controller.go +++ b/controllers/operatorpolicy_controller.go @@ -476,8 +476,8 @@ func (r *OperatorPolicyReconciler) checkSubOverlap( statusChanged = true } - if resolvedSubLabel == "" { - // No possible overlap if the subscription could not be determined + // No overlap if the subscription could not be determined or if the policy is in inform mode + if resolvedSubLabel == "" || policy.Spec.RemediationAction.IsInform() { if len(policy.Status.OverlappingPolicies) != 0 { policy.Status.OverlappingPolicies = []string{} statusChanged = true @@ -504,6 +504,10 @@ func (r *OperatorPolicyReconciler) checkSubOverlap( for _, otherPolicy := range opList.Items { if otherPolicy.Status.ResolvedSubscriptionLabel == resolvedSubLabel { + if otherPolicy.Spec.RemediationAction.IsInform() { + continue + } + if !(otherPolicy.Name == policy.Name && otherPolicy.Namespace == policy.Namespace) { overlappers = append(overlappers, otherPolicy.Name+"."+otherPolicy.Namespace) } @@ -522,7 +526,7 @@ func (r *OperatorPolicyReconciler) checkSubOverlap( slices.Sort(overlappers) - overlapError := fmt.Errorf("the specified operator is managed by multiple policies (%v)", + overlapError := fmt.Errorf("the specified operator is managed by multiple enforced policies (%v)", strings.Join(overlappers, ", ")) if !slices.Equal(overlappers, policy.Status.OverlappingPolicies) { diff --git a/test/e2e/case38_install_operator_test.go b/test/e2e/case38_install_operator_test.go index 3cf97e0b..dafe86ee 100644 --- a/test/e2e/case38_install_operator_test.go +++ b/test/e2e/case38_install_operator_test.go @@ -3120,32 +3120,30 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu mustnothaveYAML, testNamespace, gvrPolicy, gvrOperatorPolicy) }) - It("Should display a validation error in both", func() { + It("Should not display a validation error when both are in inform mode", func() { check( mustnothaveName, - true, + false, []policyv1.RelatedObject{}, metav1.Condition{ - Type: "ValidPolicySpec", - Status: metav1.ConditionFalse, - Reason: "InvalidPolicySpec", - Message: `the specified operator is managed by multiple policies ` + - `(oppol-mustnothave.` + testNamespace + `, oppol-no-group.` + testNamespace + `)`, + Type: "ValidPolicySpec", + Status: metav1.ConditionTrue, + Reason: "PolicyValidated", + Message: `the policy spec is valid`, }, - `the specified operator is managed by multiple policies`, + `the policy spec is valid`, ) check( musthaveName, - true, + false, []policyv1.RelatedObject{}, metav1.Condition{ - Type: "ValidPolicySpec", - Status: metav1.ConditionFalse, - Reason: "InvalidPolicySpec", - Message: `the specified operator is managed by multiple policies ` + - `(oppol-mustnothave.` + testNamespace + `, oppol-no-group.` + testNamespace + `)`, + Type: "ValidPolicySpec", + Status: metav1.ConditionTrue, + Reason: "PolicyValidated", + Message: `the policy spec is valid`, }, - `the specified operator is managed by multiple policies`, + `the policy spec is valid`, ) }) @@ -3181,20 +3179,38 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu }, "10s", "1s").Should(Equal(totalReconciles)) }) - It("Should remove the validation error when the overlapping policy is removed", func() { - utils.Kubectl("delete", "operatorpolicy", musthaveName, "-n", testNamespace) + It("Should display a validation error when both are enforced", func() { + // enforce the policies + utils.Kubectl("patch", "operatorpolicy", mustnothaveName, "-n", testNamespace, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/remediationAction", "value": "enforce"}]`) + utils.Kubectl("patch", "operatorpolicy", musthaveName, "-n", testNamespace, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/remediationAction", "value": "enforce"}]`) check( mustnothaveName, - false, + true, []policyv1.RelatedObject{}, metav1.Condition{ - Type: "ValidPolicySpec", - Status: metav1.ConditionTrue, - Reason: "PolicyValidated", - Message: `the policy spec is valid`, + Type: "ValidPolicySpec", + Status: metav1.ConditionFalse, + Reason: "InvalidPolicySpec", + Message: `the specified operator is managed by multiple enforced policies ` + + `(oppol-mustnothave.` + testNamespace + `, oppol-no-group.` + testNamespace + `)`, }, - `the policy spec is valid`, + `the specified operator is managed by multiple enforced policies`, + ) + check( + musthaveName, + true, + []policyv1.RelatedObject{}, + metav1.Condition{ + Type: "ValidPolicySpec", + Status: metav1.ConditionFalse, + Reason: "InvalidPolicySpec", + Message: `the specified operator is managed by multiple enforced policies ` + + `(oppol-mustnothave.` + testNamespace + `, oppol-no-group.` + testNamespace + `)`, + }, + `the specified operator is managed by multiple enforced policies`, ) }) @@ -3203,15 +3219,6 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu // which would slow down the suite. As long as no other test files use OperatorPolicy, the // Ordered property on this file should ensure this is stable. It("Should not cause an infinite reconcile loop when enforced", func() { - createObjWithParent(parentPolicyYAML, parentPolicyName, - musthaveYAML, testNamespace, gvrPolicy, gvrOperatorPolicy) - - // enforce the policies - utils.Kubectl("patch", "operatorpolicy", mustnothaveName, "-n", testNamespace, "--type=json", "-p", - `[{"op": "replace", "path": "/spec/remediationAction", "value": "enforce"}]`) - utils.Kubectl("patch", "operatorpolicy", musthaveName, "-n", testNamespace, "--type=json", "-p", - `[{"op": "replace", "path": "/spec/remediationAction", "value": "enforce"}]`) - check( mustnothaveName, true, @@ -3220,10 +3227,10 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu Type: "ValidPolicySpec", Status: metav1.ConditionFalse, Reason: "InvalidPolicySpec", - Message: `the specified operator is managed by multiple policies ` + + Message: `the specified operator is managed by multiple enforced policies ` + `(oppol-mustnothave.` + testNamespace + `, oppol-no-group.` + testNamespace + `)`, }, - `the specified operator is managed by multiple policies`, + `the specified operator is managed by multiple enforced policies`, ) recMetrics := utils.GetMetrics("controller_runtime_reconcile_total", @@ -3252,6 +3259,23 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu return loopReconciles }, "10s", "1s").Should(Equal(totalReconciles)) }) + + It("Should remove the validation error when an overlapping policy is removed", func() { + utils.Kubectl("delete", "operatorpolicy", musthaveName, "-n", testNamespace) + + check( + mustnothaveName, + false, + []policyv1.RelatedObject{}, + metav1.Condition{ + Type: "ValidPolicySpec", + Status: metav1.ConditionTrue, + Reason: "PolicyValidated", + Message: `the policy spec is valid`, + }, + `the policy spec is valid`, + ) + }) }) Describe("Testing templates in an OperatorPolicy", Ordered, func() { const (