Restrict reported overlaps to enforced policies

Previously, informed OperatorPolicies could report that they were
overlapping, but in those cases it is not necessary to prevent the
policies from operating normally. Now, only overlapping *enforced*
policies will be considered invalid.

Refs:
 - https://issues.redhat.com/browse/ACM-12207

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
(cherry picked from commit 9a2ada5)

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) {

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 (