Skip to content

Commit 98488b5

Browse files
dhaiducekopenshift-merge-bot[bot]
authored andcommitted
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]>
1 parent 8ff7064 commit 98488b5

File tree

3 files changed

+178
-18
lines changed

3 files changed

+178
-18
lines changed

internal/ordering_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,6 +667,33 @@ policies:
667667
wantFile: "testdata/ordering/manifest-extradeps-configpolicy.yaml",
668668
wantErr: "",
669669
},
670+
"manifest extraDependencies are handled with ConfigurationPolicy manifests when defaults are set": {
671+
tmpDir: tmpDir,
672+
generator: `
673+
apiVersion: policy.open-cluster-management.io/v1
674+
kind: PolicyGenerator
675+
metadata:
676+
name: test
677+
policyDefaults:
678+
namespace: my-policies
679+
placement:
680+
clusterSelector:
681+
matchExpressions: []
682+
extraDependencies:
683+
- kind: IamPolicy
684+
name: manifestextra
685+
policies:
686+
- name: one
687+
manifests:
688+
- path: {{printf "%v/%v" .Dir "configpolicy.yaml"}}
689+
- path: {{printf "%v/%v" .Dir "configmap.yaml"}}
690+
- name: two
691+
manifests:
692+
- path: {{printf "%v/%v" .Dir "configmap.yaml"}}
693+
`,
694+
wantFile: "testdata/ordering/manifest-extradeps-configpolicy-defaults.yaml",
695+
wantErr: "",
696+
},
670697
"extraDependencies defaults can be overwritten": {
671698
tmpDir: tmpDir,
672699
generator: `

internal/plugin.go

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"fmt"
99
"os"
1010
"path/filepath"
11+
"reflect"
1112
"sort"
1213
"strings"
1314
"time"
@@ -673,6 +674,8 @@ func (p *Plugin) applyDefaults(unmarshaledConfig map[string]interface{}) {
673674
for j := range policy.Manifests {
674675
manifest := &policy.Manifests[j]
675676

677+
// Only use the policy's ConfigurationPolicyOptions values when they're not explicitly set in
678+
// the manifest.
676679
if manifest.ComplianceType == "" {
677680
manifest.ComplianceType = policy.ComplianceType
678681
}
@@ -681,13 +684,6 @@ func (p *Plugin) applyDefaults(unmarshaledConfig map[string]interface{}) {
681684
manifest.MetadataComplianceType = policy.MetadataComplianceType
682685
}
683686

684-
// If the manifests are consolidated to a single ConfigurationPolicy object, don't set
685-
// ConfigurationPolicy options per manifest.
686-
if policy.ConsolidateManifests {
687-
continue
688-
}
689-
690-
// Only use the policy's ConfigurationPolicyOptions values when they're not explicitly set in the manifest.
691687
if manifest.EvaluationInterval.Compliant == "" {
692688
set := isEvaluationIntervalSetManifest(unmarshaledConfig, i, j, "compliant")
693689
if !set {
@@ -1008,41 +1004,38 @@ func (p *Plugin) assertValidConfig() error {
10081004

10091005
evalInterval := manifest.EvaluationInterval
10101006

1011-
// Verify that consolidated manifests don't specify fields
1012-
// that can't be overridden at the objectTemplate level
1007+
// Verify that consolidated manifests fields match that of the policy configuration.
10131008
if policy.ConsolidateManifests {
10141009
errorMsgFmt := fmt.Sprintf(
10151010
"the policy %s has the %%s value set on manifest[%d] but consolidateManifests is true",
10161011
policy.Name, j,
10171012
)
10181013

1019-
if evalInterval.Compliant != "" || evalInterval.NonCompliant != "" {
1014+
if !reflect.DeepEqual(evalInterval, policy.EvaluationInterval) {
10201015
return fmt.Errorf(errorMsgFmt, "evaluationInterval")
10211016
}
10221017

1023-
selector := manifest.NamespaceSelector
1024-
if selector.Exclude != nil || selector.Include != nil ||
1025-
selector.MatchLabels != nil || selector.MatchExpressions != nil {
1018+
if !reflect.DeepEqual(manifest.NamespaceSelector, policy.NamespaceSelector) {
10261019
return fmt.Errorf(errorMsgFmt, "namespaceSelector")
10271020
}
10281021

1029-
if manifest.PruneObjectBehavior != "" {
1022+
if manifest.PruneObjectBehavior != policy.PruneObjectBehavior {
10301023
return fmt.Errorf(errorMsgFmt, "pruneObjectBehavior")
10311024
}
10321025

1033-
if manifest.RemediationAction != "" {
1026+
if manifest.RemediationAction != policy.RemediationAction {
10341027
return fmt.Errorf(errorMsgFmt, "remediationAction")
10351028
}
10361029

1037-
if manifest.Severity != "" {
1030+
if manifest.Severity != policy.Severity {
10381031
return fmt.Errorf(errorMsgFmt, "severity")
10391032
}
10401033

1041-
if len(manifest.ExtraDependencies) != 0 {
1034+
if !reflect.DeepEqual(manifest.ExtraDependencies, policy.ExtraDependencies) {
10421035
return fmt.Errorf(errorMsgFmt, "extraDependencies")
10431036
}
10441037

1045-
if manifest.IgnorePending {
1038+
if manifest.IgnorePending != policy.IgnorePending {
10461039
return fmt.Errorf(errorMsgFmt, "ignorePending")
10471040
}
10481041
}
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
---
2+
apiVersion: policy.open-cluster-management.io/v1
3+
kind: Policy
4+
metadata:
5+
annotations:
6+
policy.open-cluster-management.io/categories: CM Configuration Management
7+
policy.open-cluster-management.io/controls: CM-2 Baseline Configuration
8+
policy.open-cluster-management.io/standards: NIST SP 800-53
9+
name: one
10+
namespace: my-policies
11+
spec:
12+
disabled: false
13+
policy-templates:
14+
- extraDependencies:
15+
- apiVersion: policy.open-cluster-management.io/v1
16+
compliance: Compliant
17+
kind: IamPolicy
18+
name: manifestextra
19+
objectDefinition:
20+
apiVersion: policy.open-cluster-management.io/v1
21+
kind: ConfigurationPolicy
22+
metadata:
23+
name: configpolicy-my-configmap
24+
spec:
25+
object-templates:
26+
- complianceType: musthave
27+
objectDefinition:
28+
apiVersion: v1
29+
data:
30+
game.properties: enemies=potato
31+
kind: ConfigMap
32+
metadata:
33+
name: my-configmap
34+
remediationAction: inform
35+
severity: low
36+
- extraDependencies:
37+
- apiVersion: policy.open-cluster-management.io/v1
38+
compliance: Compliant
39+
kind: IamPolicy
40+
name: manifestextra
41+
objectDefinition:
42+
apiVersion: policy.open-cluster-management.io/v1
43+
kind: ConfigurationPolicy
44+
metadata:
45+
name: one
46+
spec:
47+
object-templates:
48+
- complianceType: musthave
49+
objectDefinition:
50+
apiVersion: v1
51+
data:
52+
game.properties: enemies=potato
53+
kind: ConfigMap
54+
metadata:
55+
name: my-configmap
56+
remediationAction: inform
57+
severity: low
58+
remediationAction: inform
59+
---
60+
apiVersion: policy.open-cluster-management.io/v1
61+
kind: Policy
62+
metadata:
63+
annotations:
64+
policy.open-cluster-management.io/categories: CM Configuration Management
65+
policy.open-cluster-management.io/controls: CM-2 Baseline Configuration
66+
policy.open-cluster-management.io/standards: NIST SP 800-53
67+
name: two
68+
namespace: my-policies
69+
spec:
70+
disabled: false
71+
policy-templates:
72+
- extraDependencies:
73+
- apiVersion: policy.open-cluster-management.io/v1
74+
compliance: Compliant
75+
kind: IamPolicy
76+
name: manifestextra
77+
objectDefinition:
78+
apiVersion: policy.open-cluster-management.io/v1
79+
kind: ConfigurationPolicy
80+
metadata:
81+
name: two
82+
spec:
83+
object-templates:
84+
- complianceType: musthave
85+
objectDefinition:
86+
apiVersion: v1
87+
data:
88+
game.properties: enemies=potato
89+
kind: ConfigMap
90+
metadata:
91+
name: my-configmap
92+
remediationAction: inform
93+
severity: low
94+
remediationAction: inform
95+
---
96+
apiVersion: apps.open-cluster-management.io/v1
97+
kind: PlacementRule
98+
metadata:
99+
name: placement-one
100+
namespace: my-policies
101+
spec:
102+
clusterSelector:
103+
matchExpressions: []
104+
---
105+
apiVersion: apps.open-cluster-management.io/v1
106+
kind: PlacementRule
107+
metadata:
108+
name: placement-two
109+
namespace: my-policies
110+
spec:
111+
clusterSelector:
112+
matchExpressions: []
113+
---
114+
apiVersion: policy.open-cluster-management.io/v1
115+
kind: PlacementBinding
116+
metadata:
117+
name: binding-one
118+
namespace: my-policies
119+
placementRef:
120+
apiGroup: apps.open-cluster-management.io
121+
kind: PlacementRule
122+
name: placement-one
123+
subjects:
124+
- apiGroup: policy.open-cluster-management.io
125+
kind: Policy
126+
name: one
127+
---
128+
apiVersion: policy.open-cluster-management.io/v1
129+
kind: PlacementBinding
130+
metadata:
131+
name: binding-two
132+
namespace: my-policies
133+
placementRef:
134+
apiGroup: apps.open-cluster-management.io
135+
kind: PlacementRule
136+
name: placement-two
137+
subjects:
138+
- apiGroup: policy.open-cluster-management.io
139+
kind: Policy
140+
name: two

0 commit comments

Comments
 (0)