Skip to content

Commit e1f6be8

Browse files
dhaiducekdhaiducek
committed
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 cea8468 commit e1f6be8

File tree

3 files changed

+175
-18
lines changed

3 files changed

+175
-18
lines changed

‎internal/ordering_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,6 +667,30 @@ 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+
extraDependencies:
680+
- kind: IamPolicy
681+
name: manifestextra
682+
policies:
683+
- name: one
684+
manifests:
685+
- path: {{printf "%v/%v" .Dir "configpolicy.yaml"}}
686+
- path: {{printf "%v/%v" .Dir "configmap.yaml"}}
687+
- name: two
688+
manifests:
689+
- path: {{printf "%v/%v" .Dir "configmap.yaml"}}
690+
`,
691+
wantFile: "testdata/ordering/manifest-extradeps-configpolicy-defaults.yaml",
692+
wantErr: "",
693+
},
670694
"extraDependencies defaults can be overwritten": {
671695
tmpDir: tmpDir,
672696
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"
@@ -671,6 +672,8 @@ func (p *Plugin) applyDefaults(unmarshaledConfig map[string]interface{}) {
671672
for j := range policy.Manifests {
672673
manifest := &policy.Manifests[j]
673674

675+
// Only use the policy's ConfigurationPolicyOptions values when they're not explicitly set in
676+
// the manifest.
674677
if manifest.ComplianceType == "" {
675678
manifest.ComplianceType = policy.ComplianceType
676679
}
@@ -679,13 +682,6 @@ func (p *Plugin) applyDefaults(unmarshaledConfig map[string]interface{}) {
679682
manifest.MetadataComplianceType = policy.MetadataComplianceType
680683
}
681684

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

10071003
evalInterval := manifest.EvaluationInterval
10081004

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

1017-
if evalInterval.Compliant != "" || evalInterval.NonCompliant != "" {
1012+
if !reflect.DeepEqual(evalInterval, policy.EvaluationInterval) {
10181013
return fmt.Errorf(errorMsgFmt, "evaluationInterval")
10191014
}
10201015

1021-
selector := manifest.NamespaceSelector
1022-
if selector.Exclude != nil || selector.Include != nil ||
1023-
selector.MatchLabels != nil || selector.MatchExpressions != nil {
1016+
if !reflect.DeepEqual(manifest.NamespaceSelector, policy.NamespaceSelector) {
10241017
return fmt.Errorf(errorMsgFmt, "namespaceSelector")
10251018
}
10261019

1027-
if manifest.PruneObjectBehavior != "" {
1020+
if manifest.PruneObjectBehavior != policy.PruneObjectBehavior {
10281021
return fmt.Errorf(errorMsgFmt, "pruneObjectBehavior")
10291022
}
10301023

1031-
if manifest.RemediationAction != "" {
1024+
if manifest.RemediationAction != policy.RemediationAction {
10321025
return fmt.Errorf(errorMsgFmt, "remediationAction")
10331026
}
10341027

1035-
if manifest.Severity != "" {
1028+
if manifest.Severity != policy.Severity {
10361029
return fmt.Errorf(errorMsgFmt, "severity")
10371030
}
10381031

1039-
if len(manifest.ExtraDependencies) != 0 {
1032+
if !reflect.DeepEqual(manifest.ExtraDependencies, policy.ExtraDependencies) {
10401033
return fmt.Errorf(errorMsgFmt, "extraDependencies")
10411034
}
10421035

1043-
if manifest.IgnorePending {
1036+
if manifest.IgnorePending != policy.IgnorePending {
10441037
return fmt.Errorf(errorMsgFmt, "ignorePending")
10451038
}
10461039
}

‎internal/testdata/ordering/manifest-extradeps-configpolicy-defaults.yaml‎

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)