Do not skip compliant policies containing status fields (#420)

controllers/clustergroupupgrade_controller.go

@@ -990,22 +990,23 @@ func (r *ClusterGroupUpgradeReconciler) doManagedPoliciesExist(
 			}
 
 			// If the parent policy is not valid due to missing field, add its name to the list of invalid policies.
-			policyErr := utils.VerifyPolicyObjects(foundPolicy)
+			containsStatus, policyErr := utils.InspectPolicyObjects(foundPolicy)
 			if policyErr != nil {
 				r.Log.Error(policyErr, "Policy is invalid")
 				managedPoliciesInfo.invalidPolicies = append(managedPoliciesInfo.invalidPolicies, managedPolicyName)
 				continue
 			}
 
-			// Check the policy has at least one of the clusters from the CR in NonCompliant state.
-			clustersNonCompliantWithPolicy := r.getClustersNonCompliantWithPolicy(clusters, foundPolicy)
+			if !containsStatus {
+				// Check the policy has at least one of the clusters from the CR in NonCompliant state.
+				clustersNonCompliantWithPolicy := r.getClustersNonCompliantWithPolicy(clusters, foundPolicy)
 
-			if len(clustersNonCompliantWithPolicy) == 0 {
-				managedPoliciesCompliantBeforeUpgrade = append(managedPoliciesCompliantBeforeUpgrade, foundPolicy.GetName())
-				managedPoliciesInfo.compliantPolicies = append(managedPoliciesInfo.compliantPolicies, foundPolicy)
-				continue
+				if len(clustersNonCompliantWithPolicy) == 0 {
+					managedPoliciesCompliantBeforeUpgrade = append(managedPoliciesCompliantBeforeUpgrade, foundPolicy.GetName())
+					managedPoliciesInfo.compliantPolicies = append(managedPoliciesInfo.compliantPolicies, foundPolicy)
+					continue
+				}
 			}
-
 			// Update the info on the policies used in the upgrade.
 			newPolicyInfo := ranv1alpha1.ManagedPolicyForUpgrade{Name: managedPolicyName, Namespace: managedPolicyNamespace}
 			managedPoliciesForUpgrade = append(managedPoliciesForUpgrade, newPolicyInfo)

controllers/utils/policy_util.go

@@ -127,8 +127,11 @@ func GetParentPolicyNameAndNamespace(childPolicyName string) []string {
 	return strings.SplitN(childPolicyName, ".", 2)
 }
 
-// VerifyPolicyObjects validates the policy objects and return error if the policy is invalid
-func VerifyPolicyObjects(policy *unstructured.Unstructured) error {
+// InspectPolicyObjects validates the policy objects, checks if it contains a status section in any object templates
+// and return error if the policy is invalid
+func InspectPolicyObjects(policy *unstructured.Unstructured) (bool, error) {
+
+	containsStatus := false
 	policyName := policy.GetName()
 	policySpec := policy.Object["spec"].(map[string]interface{})
 
@@ -139,34 +142,38 @@ func VerifyPolicyObjects(policy *unstructured.Unstructured) error {
 	for _, plcTmpl := range policyTemplates {
 		// Make sure the objectDefinition of the policy template exists.
 		if plcTmpl.(map[string]interface{})["objectDefinition"] == nil {
-			return &PolicyErr{policyName, PlcMissTmplDef}
+			return containsStatus, &PolicyErr{policyName, PlcMissTmplDef}
 		}
 		plcTmplDef := plcTmpl.(map[string]interface{})["objectDefinition"].(map[string]interface{})
 
 		// Make sure the ConfigurationPolicy metadata exists.
 		if plcTmplDef["metadata"] == nil {
-			return &PolicyErr{policyName, PlcMissTmplDefMeta}
+			return containsStatus, &PolicyErr{policyName, PlcMissTmplDefMeta}
 		}
 
 		// Make sure the ConfigurationPolicy spec exists.
 		if plcTmplDef["spec"] == nil {
-			return &PolicyErr{policyName, PlcMissTmplDefSpec}
+			return containsStatus, &PolicyErr{policyName, PlcMissTmplDefSpec}
 		}
 		plcTmplDefSpec := plcTmplDef["spec"].(map[string]interface{})
 
 		// Make sure the ConfigurationPolicy object-templates exists.
 		if plcTmplDefSpec["object-templates"] == nil {
-			return &PolicyErr{policyName, ConfigPlcMissObjTmpl}
+			return containsStatus, &PolicyErr{policyName, ConfigPlcMissObjTmpl}
 		}
 		configPlcTmpls := plcTmplDefSpec["object-templates"].([]interface{})
 
 		// Go through the ConfigurationPolicy object-templates.
 		for _, configPlcTmpl := range configPlcTmpls {
 			// Make sure the objectDefinition of the ConfigurationPolicy object template exists.
 			if configPlcTmpl.(map[string]interface{})["objectDefinition"] == nil {
-				return &PolicyErr{policyName, ConfigPlcMissObjTmplDef}
+				return containsStatus, &PolicyErr{policyName, ConfigPlcMissObjTmplDef}
+			}
+			objectDefinition := configPlcTmpl.(map[string]interface{})["objectDefinition"].(map[string]interface{})
+			if objectDefinition["status"] != nil {
+				containsStatus = true
 			}
 		}
 	}
-	return nil
+	return containsStatus, nil
 }

deploy/acm/policies/all_policies/policy0-common-config-policy.yaml

@@ -35,9 +35,6 @@ spec:
               image: e27-h01-000-r650.rdu2.scalelab.redhat.com:5000/olm-mirror/redhat-operator-index:v4.11
               publisher: Red Hat
               sourceType: grpc
-            status:
-              connectionState:
-                lastObservedState: READY
         remediationAction: inform
         severity: low
   remediationAction: inform

deploy/acm/policies/all_policies/policy2-common-pao-sub-with-status-policy.yaml

@@ -0,0 +1,57 @@
+apiVersion: policy.open-cluster-management.io/v1
+kind: Policy
+metadata:
+  annotations:
+    policy.open-cluster-management.io/categories: CM Configuration Management
+    policy.open-cluster-management.io/controls: CM-2 Baseline Configuration
+    policy.open-cluster-management.io/standards: NIST SP 800-53
+  name: policy2-common-pao-sub-policy
+spec:
+  disabled: false
+  policy-templates:
+  - objectDefinition:
+      apiVersion: policy.open-cluster-management.io/v1
+      kind: ConfigurationPolicy
+      metadata:
+        name: common-pao-sub-policy-config
+      spec:
+        namespaceselector:
+          exclude:
+          - kube-*
+          include:
+          - '*'
+        object-templates:
+        - complianceType: musthave
+          objectDefinition:
+            apiVersion: operators.coreos.com/v1alpha1
+            kind: Subscription
+            metadata:
+              name: performance-addon-operator
+              namespace: openshift-performance-addon-operator
+            spec:
+              channel: "4.9"
+              name: performance-addon-operator
+              source: redhat-operators
+              sourceNamespace: openshift-marketplace
+            status:
+              state: AtLatestKnown
+        - complianceType: musthave
+          objectDefinition:
+            apiVersion: v1
+            kind: Namespace
+            metadata:
+              annotations:
+                workload.openshift.io/allowed: management
+              labels:
+                openshift.io/cluster-monitoring: "true"
+              name: openshift-performance-addon-operator
+        - complianceType: musthave
+          objectDefinition:
+            apiVersion: operators.coreos.com/v1
+            kind: OperatorGroup
+            metadata:
+              name: performance-addon-operator
+              namespace: openshift-performance-addon-operator
+        remediationAction: inform
+        severity: low
+  remediationAction: inform

tests/kuttl/tests/do-not-skip-compliant-policies-with-status/00-assert.yaml

@@ -0,0 +1,102 @@
+apiVersion: ran.openshift.io/v1alpha1
+kind: ClusterGroupUpgrade
+metadata:
+  name: cgu
+  namespace: default
+spec:
+  clusters:
+  - spoke6
+  - spoke2
+  - spoke1
+  - spoke5
+  enable: true
+  managedPolicies:
+  - policy1-common-cluster-version-policy
+  - policy2-common-pao-sub-policy
+  - policy3-common-ptp-sub-policy
+  - policy4-common-sriov-sub-policy
+  remediationStrategy:
+    maxConcurrency: 4
+    timeout: 240
+status:
+  computedMaxConcurrency: 4
+  conditions:
+  - message: All selected clusters are valid
+    reason: ClusterSelectionCompleted
+    status: "True"
+    type: ClustersSelected
+  - message: Completed validation
+    reason: ValidationCompleted
+    status: "True"
+    type: Validated
+  - message: Remediating non-compliant policies
+    reason: InProgress
+    status: "True"
+    type: Progressing
+  copiedPolicies:
+  - cgu-policy1-common-cluster-version-policy-kuttl
+  - cgu-policy2-common-pao-sub-policy-kuttl
+  - cgu-policy3-common-ptp-sub-policy-kuttl
+  - cgu-policy4-common-sriov-sub-policy-kuttl
+  managedPoliciesContent:
+    policy2-common-pao-sub-policy: '[{"kind":"Subscription","name":"performance-addon-operator","apiVersion":"operators.coreos.com/v1alpha1","namespace":"openshift-performance-addon-operator"}]'
+    policy3-common-ptp-sub-policy: '[{"kind":"Subscription","name":"ptp-operator-subscription","apiVersion":"operators.coreos.com/v1alpha1","namespace":"openshift-ptp"}]'
+    policy4-common-sriov-sub-policy: '[{"kind":"Subscription","name":"sriov-network-operator-subscription","apiVersion":"operators.coreos.com/v1alpha1","namespace":"openshift-sriov-network-operator"}]'
+  managedPoliciesForUpgrade:
+  - name: policy1-common-cluster-version-policy
+    namespace: default
+  - name: policy2-common-pao-sub-policy
+    namespace: default
+  - name: policy3-common-ptp-sub-policy
+    namespace: default
+  - name: policy4-common-sriov-sub-policy
+    namespace: default
+  managedPoliciesNs:
+    policy1-common-cluster-version-policy: default
+    policy2-common-pao-sub-policy: default
+    policy3-common-ptp-sub-policy: default
+    policy4-common-sriov-sub-policy: default
+  placementBindings:
+  - cgu-policy1-common-cluster-version-policy-placement-kuttl
+  - cgu-policy2-common-pao-sub-policy-placement-kuttl
+  - cgu-policy3-common-ptp-sub-policy-placement-kuttl
+  - cgu-policy4-common-sriov-sub-policy-placement-kuttl
+  placementRules:
+  - cgu-policy1-common-cluster-version-policy-placement-kuttl
+  - cgu-policy2-common-pao-sub-policy-placement-kuttl
+  - cgu-policy3-common-ptp-sub-policy-placement-kuttl
+  - cgu-policy4-common-sriov-sub-policy-placement-kuttl
+  remediationPlan:
+  - - spoke6
+    - spoke2
+    - spoke1
+    - spoke5
+  safeResourceNames:
+    cgu-common-cluster-version-policy-config: cgu-common-cluster-version-policy-config-kuttl
+    cgu-common-pao-sub-policy-config: cgu-common-pao-sub-policy-config-kuttl
+    cgu-common-ptp-sub-policy-config: cgu-common-ptp-sub-policy-config-kuttl
+    cgu-common-sriov-sub-policy-config: cgu-common-sriov-sub-policy-config-kuttl
+    cgu-default-subscription-ptp-operator-subscription: cgu-default-subscription-ptp-operator-subscription-kuttl
+    cgu-default-subscription-sriov-network-operator-subscription: cgu-default-subscription-sriov-network-operator-subscription-kuttl
+    cgu-policy1-common-cluster-version-policy: cgu-policy1-common-cluster-version-policy-kuttl
+    cgu-policy1-common-cluster-version-policy-placement: cgu-policy1-common-cluster-version-policy-placement-kuttl
+    cgu-policy2-common-pao-sub-policy: cgu-policy2-common-pao-sub-policy-kuttl
+    cgu-policy2-common-pao-sub-policy-placement: cgu-policy2-common-pao-sub-policy-placement-kuttl
+    cgu-policy3-common-ptp-sub-policy: cgu-policy3-common-ptp-sub-policy-kuttl
+    cgu-policy3-common-ptp-sub-policy-placement: cgu-policy3-common-ptp-sub-policy-placement-kuttl
+    cgu-policy4-common-sriov-sub-policy: cgu-policy4-common-sriov-sub-policy-kuttl
+  status:
+    currentBatch: 1
+    currentBatchRemediationProgress:
+      spoke1:
+        policyIndex: 0
+        state: InProgress
+      spoke2:
+        policyIndex: 2
+        state: InProgress
+      spoke5:
+        policyIndex: 3
+        state: InProgress
+      spoke6:
+        policyIndex: 0
+        state: InProgress

tests/kuttl/tests/do-not-skip-compliant-policies-with-status/00-setup.yaml

@@ -0,0 +1,48 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+
+commands:
+  # Create all the managed inform policies
+  - command: oc apply -f ../../../../deploy/acm/policies/all_policies/policy1-common-cluster-version-policy.yaml
+    namespaced: true
+  - command: oc apply -f ../../../../deploy/acm/policies/all_policies/policy2-common-pao-sub-with-status-policy.yaml
+    namespaced: true
+  - command: oc apply -f ../../../../deploy/acm/policies/all_policies/policy3-common-ptp-sub-policy.yaml
+    namespaced: true
+  - command: oc apply -f ../../../../deploy/acm/policies/all_policies/policy4-common-sriov-sub-policy.yaml
+    namespaced: true
+
+  # Patch the inform policies to reflect the compliance status. Make policy 2 compliant with all the clusters.
+  - command: ../../../../deploy/upgrades/skip-compliant-policies/patch-policies-status.sh default default default default
+    ignoreFailure: false
+
+  # Create all the child policies to map the inform policies above.
+  - command: oc apply --namespace=spoke1 -f ../../../../deploy/acm/policies/all_policies/child-policy1-common-cluster-version-policy.yaml
+    namespaced: true
+  - command: oc apply --namespace=spoke4 -f ../../../../deploy/acm/policies/all_policies/child-policy1-common-cluster-version-policy.yaml
+    namespaced: true
+  - command: oc apply --namespace=spoke6 -f ../../../../deploy/acm/policies/all_policies/child-policy1-common-cluster-version-policy.yaml
+    namespaced: true
+
+  - command: oc apply --namespace=spoke1 -f ../../../../deploy/acm/policies/all_policies/child-policy2-common-pao-sub-policy.yaml
+    namespaced: true
+  - command: oc apply --namespace=spoke2 -f ../../../../deploy/acm/policies/all_policies/child-policy2-common-pao-sub-policy.yaml
+    namespaced: true
+  - command: oc apply --namespace=spoke4 -f ../../../../deploy/acm/policies/all_policies/child-policy2-common-pao-sub-policy.yaml
+    namespaced: true
+
+  - command: oc apply --namespace=spoke2 -f ../../../../deploy/acm/policies/all_policies/child-policy3-common-ptp-sub-policy.yaml
+    namespaced: true
+  - command: oc apply --namespace=spoke4 -f ../../../../deploy/acm/policies/all_policies/child-policy3-common-ptp-sub-policy.yaml
+    namespaced: true
+
+  - command: oc apply --namespace=spoke4 -f ../../../../deploy/acm/policies/all_policies/child-policy4-common-sriov-sub-policy.yaml
+    namespaced: true
+  - command: oc apply --namespace=spoke5 -f ../../../../deploy/acm/policies/all_policies/child-policy4-common-sriov-sub-policy.yaml
+    namespaced: true
+  - command: oc apply --namespace=spoke6 -f ../../../../deploy/acm/policies/all_policies/child-policy4-common-sriov-sub-policy.yaml
+    namespaced: true
+
+  # Apply the UOCR.
+  - command: oc apply -f ../../../../deploy/upgrades/skip-compliant-policies/skip-compliant-policies.yaml
+    namespaced: true

tests/kuttl/tests/do-not-skip-compliant-policies-with-status/01-cleanup.yaml

@@ -0,0 +1,44 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+
+commands:
+  # Delete all the managed inform policies
+  - command: oc delete -f ../../../../deploy/acm/policies/all_policies/policy1-common-cluster-version-policy.yaml
+    namespaced: true
+  - command: oc delete -f ../../../../deploy/acm/policies/all_policies/policy2-common-pao-sub-policy.yaml
+    namespaced: true
+  - command: oc delete -f ../../../../deploy/acm/policies/all_policies/policy3-common-ptp-sub-policy.yaml
+    namespaced: true
+  - command: oc delete -f ../../../../deploy/acm/policies/all_policies/policy4-common-sriov-sub-policy.yaml
+    namespaced: true
+
+  # Delete all the child policies to map the inform policies above.
+  - command: oc delete --namespace=spoke1 -f ../../../../deploy/acm/policies/all_policies/child-policy1-common-cluster-version-policy.yaml
+    namespaced: true
+  - command: oc delete --namespace=spoke4 -f ../../../../deploy/acm/policies/all_policies/child-policy1-common-cluster-version-policy.yaml
+    namespaced: true
+  - command: oc delete --namespace=spoke6 -f ../../../../deploy/acm/policies/all_policies/child-policy1-common-cluster-version-policy.yaml
+    namespaced: true
+
+  - command: oc delete --namespace=spoke1 -f ../../../../deploy/acm/policies/all_policies/child-policy2-common-pao-sub-policy.yaml
+    namespaced: true
+  - command: oc delete --namespace=spoke2 -f ../../../../deploy/acm/policies/all_policies/child-policy2-common-pao-sub-policy.yaml
+    namespaced: true
+  - command: oc delete --namespace=spoke4 -f ../../../../deploy/acm/policies/all_policies/child-policy2-common-pao-sub-policy.yaml
+    namespaced: true
+
+  - command: oc delete --namespace=spoke2 -f ../../../../deploy/acm/policies/all_policies/child-policy3-common-ptp-sub-policy.yaml
+    namespaced: true
+  - command: oc delete --namespace=spoke4 -f ../../../../deploy/acm/policies/all_policies/child-policy3-common-ptp-sub-policy.yaml
+    namespaced: true
+
+  - command: oc delete --namespace=spoke4 -f ../../../../deploy/acm/policies/all_policies/child-policy4-common-sriov-sub-policy.yaml
+    namespaced: true
+  - command: oc delete --namespace=spoke5 -f ../../../../deploy/acm/policies/all_policies/child-policy4-common-sriov-sub-policy.yaml
+    namespaced: true
+  - command: oc delete --namespace=spoke6 -f ../../../../deploy/acm/policies/all_policies/child-policy4-common-sriov-sub-policy.yaml
+    namespaced: true
+
+  # Delete the UOCR.
+  - command: oc delete -f ../../../../deploy/upgrades/skip-compliant-policies/skip-compliant-policies.yaml
+    namespaced: true

tests/kuttl/tests/skip-compliant-policies/00-setup.yaml

@@ -12,7 +12,7 @@ commands:
   - command: oc apply -f ../../../../deploy/acm/policies/all_policies/policy4-common-sriov-sub-policy.yaml
     namespaced: true
 
-  # Patch the inform policies to reflect the compliance status. Make policy 3 compliant with all the clusters.
+  # Patch the inform policies to reflect the compliance status. Make policy 2 compliant with all the clusters.
   - command: ../../../../deploy/upgrades/skip-compliant-policies/patch-policies-status.sh default default default default
     ignoreFailure: false