Fix setting manifest overrides correctly

Since the manifest configuration wasn't being set when consolidateManifests was true, it wasn't being propagated to things like expanded policies and policy template manifests provided directly. This fixes it to set manifest configurations and then compare manifest and policy configurations to make sure they match when consolidateManifests is true. ref: https://issues.redhat.com/browse/ACM-9131 Signed-off-by: Dale Haiducek <[email protected]>
open-cluster-management-io · Dec 18, 2023 · 66195ab · 66195ab
1 parent 62f7f83
commit 66195ab
Show file tree

Hide file tree

Showing 3 changed files with 178 additions and 18 deletions.
diff --git a/internal/ordering_test.go b/internal/ordering_test.go
@@ -667,6 +667,33 @@ policies:
 			wantFile: "testdata/ordering/manifest-extradeps-configpolicy.yaml",
 			wantErr:  "",
 		},
+		"manifest extraDependencies are handled with ConfigurationPolicy manifests when defaults are set": {
+			tmpDir: tmpDir,
+			generator: `
+apiVersion: policy.open-cluster-management.io/v1
+kind: PolicyGenerator
+metadata:
+  name: test
+policyDefaults:
+  namespace: my-policies
+  placement:
+    clusterSelector:
+      matchExpressions: []
+  extraDependencies:
+  - kind: IamPolicy
+    name: manifestextra
+policies:
+- name: one
+  manifests:
+  - path: {{printf "%v/%v" .Dir "configpolicy.yaml"}}
+  - path: {{printf "%v/%v" .Dir "configmap.yaml"}}
+- name: two
+  manifests:
+  - path: {{printf "%v/%v" .Dir "configmap.yaml"}}
+`,
+			wantFile: "testdata/ordering/manifest-extradeps-configpolicy-defaults.yaml",
+			wantErr:  "",
+		},
 		"extraDependencies defaults can be overwritten": {
 			tmpDir: tmpDir,
 			generator: `

diff --git a/internal/plugin.go b/internal/plugin.go
@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"reflect"
 	"sort"
 	"strings"
 	"time"
@@ -671,6 +672,8 @@ func (p *Plugin) applyDefaults(unmarshaledConfig map[string]interface{}) {
 		for j := range policy.Manifests {
 			manifest := &policy.Manifests[j]
 
+			// Only use the policy's ConfigurationPolicyOptions values when they're not explicitly set in
+			// the manifest.
 			if manifest.ComplianceType == "" {
 				manifest.ComplianceType = policy.ComplianceType
 			}
@@ -679,13 +682,6 @@ func (p *Plugin) applyDefaults(unmarshaledConfig map[string]interface{}) {
 				manifest.MetadataComplianceType = policy.MetadataComplianceType
 			}
 
-			// If the manifests are consolidated to a single ConfigurationPolicy object, don't set
-			// ConfigurationPolicy options per manifest.
-			if policy.ConsolidateManifests {
-				continue
-			}
-
-			// Only use the policy's ConfigurationPolicyOptions values when they're not explicitly set in the manifest.
 			if manifest.EvaluationInterval.Compliant == "" {
 				set := isEvaluationIntervalSetManifest(unmarshaledConfig, i, j, "compliant")
 				if !set {
@@ -1006,41 +1002,38 @@ func (p *Plugin) assertValidConfig() error {
 
 			evalInterval := manifest.EvaluationInterval
 
-			// Verify that consolidated manifests don't specify fields
-			// that can't be overridden at the objectTemplate level
+			// Verify that consolidated manifests fields match that of the policy configuration.
 			if policy.ConsolidateManifests {
 				errorMsgFmt := fmt.Sprintf(
 					"the policy %s has the %%s value set on manifest[%d] but consolidateManifests is true",
 					policy.Name, j,
 				)
 
-				if evalInterval.Compliant != "" || evalInterval.NonCompliant != "" {
+				if !reflect.DeepEqual(evalInterval, policy.EvaluationInterval) {
 					return fmt.Errorf(errorMsgFmt, "evaluationInterval")
 				}
 
-				selector := manifest.NamespaceSelector
-				if selector.Exclude != nil || selector.Include != nil ||
-					selector.MatchLabels != nil || selector.MatchExpressions != nil {
+				if !reflect.DeepEqual(manifest.NamespaceSelector, policy.NamespaceSelector) {
 					return fmt.Errorf(errorMsgFmt, "namespaceSelector")
 				}
 
-				if manifest.PruneObjectBehavior != "" {
+				if manifest.PruneObjectBehavior != policy.PruneObjectBehavior {
 					return fmt.Errorf(errorMsgFmt, "pruneObjectBehavior")
 				}
 
-				if manifest.RemediationAction != "" {
+				if manifest.RemediationAction != policy.RemediationAction {
 					return fmt.Errorf(errorMsgFmt, "remediationAction")
 				}
 
-				if manifest.Severity != "" {
+				if manifest.Severity != policy.Severity {
 					return fmt.Errorf(errorMsgFmt, "severity")
 				}
 
-				if len(manifest.ExtraDependencies) != 0 {
+				if !reflect.DeepEqual(manifest.ExtraDependencies, policy.ExtraDependencies) {
 					return fmt.Errorf(errorMsgFmt, "extraDependencies")
 				}
 
-				if manifest.IgnorePending {
+				if manifest.IgnorePending != policy.IgnorePending {
 					return fmt.Errorf(errorMsgFmt, "ignorePending")
 				}
 			}

diff --git a/internal/testdata/ordering/manifest-extradeps-configpolicy-defaults.yaml b/internal/testdata/ordering/manifest-extradeps-configpolicy-defaults.yaml
@@ -0,0 +1,140 @@
+---
+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: one
+  namespace: my-policies
+spec:
+  disabled: false
+  policy-templates:
+    - extraDependencies:
+      - apiVersion: policy.open-cluster-management.io/v1
+        compliance: Compliant
+        kind: IamPolicy
+        name: manifestextra
+      objectDefinition:
+        apiVersion: policy.open-cluster-management.io/v1
+        kind: ConfigurationPolicy
+        metadata:
+          name: configpolicy-my-configmap
+        spec:
+          object-templates:
+            - complianceType: musthave
+              objectDefinition:
+                apiVersion: v1
+                data:
+                  game.properties: enemies=potato
+                kind: ConfigMap
+                metadata:
+                  name: my-configmap
+          remediationAction: inform
+          severity: low
+    - extraDependencies:
+      - apiVersion: policy.open-cluster-management.io/v1
+        compliance: Compliant
+        kind: IamPolicy
+        name: manifestextra
+      objectDefinition:
+        apiVersion: policy.open-cluster-management.io/v1
+        kind: ConfigurationPolicy
+        metadata:
+          name: one
+        spec:
+          object-templates:
+            - complianceType: musthave
+              objectDefinition:
+                apiVersion: v1
+                data:
+                  game.properties: enemies=potato
+                kind: ConfigMap
+                metadata:
+                  name: my-configmap
+          remediationAction: inform
+          severity: low
+  remediationAction: inform
+---
+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: two
+  namespace: my-policies
+spec:
+  disabled: false
+  policy-templates:
+    - extraDependencies:
+      - apiVersion: policy.open-cluster-management.io/v1
+        compliance: Compliant
+        kind: IamPolicy
+        name: manifestextra
+      objectDefinition:
+        apiVersion: policy.open-cluster-management.io/v1
+        kind: ConfigurationPolicy
+        metadata:
+          name: two
+        spec:
+          object-templates:
+            - complianceType: musthave
+              objectDefinition:
+                apiVersion: v1
+                data:
+                  game.properties: enemies=potato
+                kind: ConfigMap
+                metadata:
+                  name: my-configmap
+          remediationAction: inform
+          severity: low
+  remediationAction: inform
+---
+apiVersion: apps.open-cluster-management.io/v1
+kind: PlacementRule
+metadata:
+  name: placement-one
+  namespace: my-policies
+spec:
+  clusterSelector:
+    matchExpressions: []
+---
+apiVersion: apps.open-cluster-management.io/v1
+kind: PlacementRule
+metadata:
+  name: placement-two
+  namespace: my-policies
+spec:
+  clusterSelector:
+    matchExpressions: []
+---
+apiVersion: policy.open-cluster-management.io/v1
+kind: PlacementBinding
+metadata:
+  name: binding-one
+  namespace: my-policies
+placementRef:
+  apiGroup: apps.open-cluster-management.io
+  kind: PlacementRule
+  name: placement-one
+subjects:
+  - apiGroup: policy.open-cluster-management.io
+    kind: Policy
+    name: one
+---
+apiVersion: policy.open-cluster-management.io/v1
+kind: PlacementBinding
+metadata:
+  name: binding-two
+  namespace: my-policies
+placementRef:
+  apiGroup: apps.open-cluster-management.io
+  kind: PlacementRule
+  name: placement-two
+subjects:
+  - apiGroup: policy.open-cluster-management.io
+    kind: Policy
+    name: two