Skip to content

Commit c36d294

Browse files
dhaiducekdhaiducek
committed
Provide consistent overrides in manifests
This adds missing keys and validation to overrides in `manifests` ref: stolostron/backlog#25982 Signed-off-by: Dale Haiducek <[email protected]>
1 parent 2d7a140 commit c36d294

File tree

5 files changed

+215
-32
lines changed

5 files changed

+215
-32
lines changed

‎docs/policygenerator-reference.yaml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,21 @@ policies:
144144
complianceType: "musthave"
145145
# Optional. (See policyDefaults.metadataComplianceType for description.)
146146
metadataComplianceType: ""
147+
# Optional. (See policyDefaults.namespaceSelector for description.)
148+
# Cannot be specified when policyDefaults.consolidateManifests is set to true.
149+
namespaceSelector: {}
147150
# Optional. (See policyDefaults.evaluationInterval for description.)
151+
# Cannot be specified when policyDefaults.consolidateManifests is set to true.
148152
evaluationInterval: {}
149153
# Optional. (See policyDefaults.pruneObjectBehavior for description.)
154+
# Cannot be specified when policyDefaults.consolidateManifests is set to true.
150155
pruneObjectBehavior: ""
156+
# Optional. (See policyDefaults.remediationAction for description.)
157+
# Cannot be specified when policyDefaults.consolidateManifests is set to true.
158+
remediationAction: ""
159+
# Optional. (See policyDefaults.severity for description.)
160+
# Cannot be specified when policyDefaults.consolidateManifests is set to true.
161+
severity: "low"
151162
# (Note: a path to a directory containing a Kustomize manifest is a supported alternative.) Optional. A
152163
# Kustomize patch to apply to the manifest(s) at the path. If there are multiple manifests, the patch requires
153164
# the apiVersion, kind, metadata.name, and metadata.namespace (if applicable) fields to be set so Kustomize can

‎internal/plugin.go‎

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -573,12 +573,12 @@ func (p *Plugin) applyDefaults(unmarshaledConfig map[string]interface{}) {
573573
}
574574

575575
// If the manifests are consolidated to a single ConfigurationPolicy object, don't set
576-
// the evaluation interval per manifest.
576+
// ConfigurationPolicy options per manifest.
577577
if policy.ConsolidateManifests {
578578
continue
579579
}
580580

581-
// Only use the policy's evaluationInterval value when it's not explicitly set in the manifest.
581+
// Only use the policy's ConfigurationPolicyOptions values when they're not explicitly set in the manifest.
582582
if manifest.EvaluationInterval.Compliant == "" {
583583
set := isEvaluationIntervalSetManifest(unmarshaledConfig, i, j, "compliant")
584584
if !set {
@@ -593,9 +593,23 @@ func (p *Plugin) applyDefaults(unmarshaledConfig map[string]interface{}) {
593593
}
594594
}
595595

596+
selector := manifest.NamespaceSelector
597+
if selector.Exclude != nil || selector.Include != nil ||
598+
selector.MatchLabels != nil || selector.MatchExpressions != nil {
599+
manifest.NamespaceSelector = policy.NamespaceSelector
600+
}
601+
602+
if manifest.RemediationAction == "" && policy.RemediationAction != "" {
603+
manifest.RemediationAction = policy.RemediationAction
604+
}
605+
596606
if manifest.PruneObjectBehavior == "" && policy.PruneObjectBehavior != "" {
597607
manifest.PruneObjectBehavior = policy.PruneObjectBehavior
598608
}
609+
610+
if manifest.Severity == "" && manifest.Severity != "" {
611+
manifest.Severity = policy.Severity
612+
}
599613
}
600614

601615
for _, plcsetInPlc := range policy.PolicySets {
@@ -800,25 +814,37 @@ func (p *Plugin) assertValidConfig() error {
800814
return err
801815
}
802816

817+
evalInterval := manifest.EvaluationInterval
818+
803819
// Verify that consolidated manifests don't specify fields
804820
// that can't be overridden at the objectTemplate level
805-
if policy.ConsolidateManifests && manifest.PruneObjectBehavior != "" {
806-
return fmt.Errorf(
807-
"the policy %s has the pruneObjectBehavior value set on manifest[%d] but "+
808-
"consolidateManifests is true",
809-
policy.Name,
810-
j,
821+
if policy.ConsolidateManifests {
822+
errorMsgFmt := fmt.Sprintf(
823+
"the policy %s has the %%s value set on manifest[%d] but consolidateManifests is true",
824+
policy.Name, j,
811825
)
812-
}
813826

814-
evalInterval := &manifest.EvaluationInterval
815-
if policy.ConsolidateManifests && (evalInterval.Compliant != "" || evalInterval.NonCompliant != "") {
816-
return fmt.Errorf(
817-
"the policy %s has the evaluationInterval value set on manifest[%d] but "+
818-
"consolidateManifests is true",
819-
policy.Name,
820-
j,
821-
)
827+
if evalInterval.Compliant != "" || evalInterval.NonCompliant != "" {
828+
return fmt.Errorf(errorMsgFmt, "evaluationInterval")
829+
}
830+
831+
selector := manifest.NamespaceSelector
832+
if selector.Exclude != nil || selector.Include != nil ||
833+
selector.MatchLabels != nil || selector.MatchExpressions != nil {
834+
return fmt.Errorf(errorMsgFmt, "namespaceSelector")
835+
}
836+
837+
if manifest.PruneObjectBehavior != "" {
838+
return fmt.Errorf(errorMsgFmt, "pruneObjectBehavior")
839+
}
840+
841+
if manifest.RemediationAction != "" {
842+
return fmt.Errorf(errorMsgFmt, "remediationAction")
843+
}
844+
845+
if manifest.Severity != "" {
846+
return fmt.Errorf(errorMsgFmt, "severity")
847+
}
822848
}
823849

824850
if evalInterval.Compliant != "" && evalInterval.Compliant != "never" {

‎internal/plugin_config_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,6 +1030,30 @@ func TestConfigInvalidManifestKey(t *testing.T) {
10301030
`the policy policy-app has the pruneObjectBehavior value set` +
10311031
` on manifest[0] but consolidateManifests is true`,
10321032
},
1033+
"namespaceSelector specified in manifest": {
1034+
"namespaceSelector",
1035+
"",
1036+
"",
1037+
`{"include": ["test"]}`,
1038+
`the policy policy-app has the namespaceSelector value set` +
1039+
` on manifest[0] but consolidateManifests is true`,
1040+
},
1041+
"remediationAction specified in manifest": {
1042+
"remediationAction",
1043+
"",
1044+
"",
1045+
"enforce",
1046+
`the policy policy-app has the remediationAction value set` +
1047+
` on manifest[0] but consolidateManifests is true`,
1048+
},
1049+
"severity specified in manifest": {
1050+
"severity",
1051+
"",
1052+
"",
1053+
"critical",
1054+
`the policy policy-app has the severity value set` +
1055+
` on manifest[0] but consolidateManifests is true`,
1056+
},
10331057
}
10341058

10351059
for testName, test := range tests {

‎internal/plugin_test.go‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
package internal
33

44
import (
5+
"encoding/json"
56
"fmt"
67
"io/ioutil"
78
"path"
@@ -10,6 +11,7 @@ import (
1011
"strings"
1112
"testing"
1213

14+
"gopkg.in/yaml.v3"
1315
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1416
"open-cluster-management.io/ocm-kustomize-generator-plugins/internal/types"
1517
)
@@ -185,6 +187,108 @@ subjects:
185187
assertEqual(t, string(output), expected)
186188
}
187189

190+
func TestConfigManifestKeyOverride(t *testing.T) {
191+
t.Parallel()
192+
tmpDir := t.TempDir()
193+
createConfigMap(t, tmpDir, "configmap.yaml")
194+
195+
tests := map[string]struct {
196+
// Individual values can't be used for compliant/noncompliant since an empty string means
197+
// to not inherit from the policy defaults.
198+
keyName string
199+
defaultKey string
200+
policyKey string
201+
manifestKey string
202+
}{
203+
"pruneObjectBehavior specified in manifest": {
204+
"pruneObjectBehavior",
205+
"None",
206+
"DeleteIfCreated",
207+
`"DeleteAll"`,
208+
},
209+
"namespaceSelector specified in manifest": {
210+
"namespaceSelector",
211+
`{"matchLabels":{"name":"test"}}`,
212+
`{"exclude":["test"]}`,
213+
`{"include":["test"]}`,
214+
},
215+
"remediationAction specified in manifest": {
216+
"remediationAction",
217+
"inform",
218+
"inform",
219+
`"enforce"`,
220+
},
221+
"severity specified in manifest": {
222+
"severity",
223+
"low",
224+
"medium",
225+
`"critical"`,
226+
},
227+
}
228+
229+
for testName, test := range tests {
230+
test := test
231+
232+
t.Run(
233+
testName,
234+
func(t *testing.T) {
235+
t.Parallel()
236+
config := fmt.Sprintf(`
237+
apiVersion: policy.open-cluster-management.io/v1
238+
kind: PolicyGenerator
239+
metadata:
240+
name: policy-generator-name
241+
policyDefaults:
242+
namespace: my-policies
243+
consolidateManifests: false
244+
%s: %s
245+
policies:
246+
- name: policy-app
247+
%s: %s
248+
manifests:
249+
- path: %s
250+
%s: %s
251+
`,
252+
test.keyName, test.defaultKey,
253+
test.keyName, test.policyKey,
254+
path.Join(tmpDir, "configmap.yaml"),
255+
test.keyName, test.manifestKey,
256+
)
257+
258+
p := Plugin{}
259+
err := p.Config([]byte(config), tmpDir)
260+
if err != nil {
261+
t.Fatal("Unexpected error", err)
262+
}
263+
264+
assertEqual(t, p.Policies[0].ConsolidateManifests, false)
265+
266+
output, err := p.Generate()
267+
if err != nil {
268+
t.Fatal("Failed to generate policies from PolicyGenerator manifest", err)
269+
}
270+
271+
var policyObj map[string]interface{}
272+
err = yaml.Unmarshal(output, &policyObj)
273+
if err != nil {
274+
t.Fatal("Failed to unmarshal object", err)
275+
}
276+
277+
policyTemplate := policyObj["spec"].(map[string]interface{})["policy-templates"].([]interface{})[0]
278+
objectDef := policyTemplate.(map[string]interface{})["objectDefinition"].(map[string]interface{})
279+
configSpec := objectDef["spec"].(map[string]interface{})
280+
281+
jsonConfig, err := json.Marshal(configSpec[test.keyName])
282+
if err != nil {
283+
t.Fatal("Failed to marshal policy to JSON", err)
284+
}
285+
286+
assertEqual(t, string(jsonConfig), test.manifestKey)
287+
},
288+
)
289+
}
290+
}
291+
188292
func TestGeneratePolicyExisingPlacementRuleName(t *testing.T) {
189293
t.Parallel()
190294
tmpDir := t.TempDir()

‎internal/utils.go‎

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,8 @@ func getPolicyTemplates(policyConf *types.PolicyConfig) ([]map[string]map[string
208208
policyConf,
209209
len(policyTemplates)+1,
210210
&[]map[string]interface{}{objTemplate},
211-
&policyConf.Manifests[i].EvaluationInterval,
212-
policyConf.Manifests[i].PruneObjectBehavior,
211+
policyConf.Manifests[i].ConfigurationPolicyOptions,
213212
)
214-
setNamespaceSelector(policyConf, policyTemplate)
215213
policyTemplates = append(policyTemplates, *policyTemplate)
216214
}
217215
}
@@ -230,10 +228,8 @@ func getPolicyTemplates(policyConf *types.PolicyConfig) ([]map[string]map[string
230228
policyConf,
231229
1,
232230
&objectTemplates,
233-
&policyConf.EvaluationInterval,
234-
policyConf.PruneObjectBehavior,
231+
policyConf.ConfigurationPolicyOptions,
235232
)
236-
setNamespaceSelector(policyConf, policyTemplate)
237233
policyTemplates = append(policyTemplates, *policyTemplate)
238234
}
239235

@@ -266,13 +262,16 @@ func isPolicyTypeManifest(manifest map[string]interface{}) (bool, error) {
266262
}
267263

268264
// setNamespaceSelector sets the namespace selector, if set, on the input policy template.
269-
func setNamespaceSelector(policyConf *types.PolicyConfig, policyTemplate *map[string]map[string]interface{}) {
265+
func setNamespaceSelector(
266+
policyConf types.ConfigurationPolicyOptions,
267+
policyTemplate map[string]map[string]interface{},
268+
) {
270269
selector := policyConf.NamespaceSelector
271270
if selector.Exclude != nil ||
272271
selector.Include != nil ||
273272
selector.MatchLabels != nil ||
274273
selector.MatchExpressions != nil {
275-
spec := (*policyTemplate)["objectDefinition"]["spec"].(map[string]interface{})
274+
spec := policyTemplate["objectDefinition"]["spec"].(map[string]interface{})
276275
spec["namespaceSelector"] = selector
277276
}
278277
}
@@ -306,8 +305,7 @@ func buildPolicyTemplate(
306305
policyConf *types.PolicyConfig,
307306
policyNum int,
308307
objectTemplates *[]map[string]interface{},
309-
evaluationInterval *types.EvaluationInterval,
310-
pruneObjectBehavior string,
308+
configPolicyOptionsOverrides types.ConfigurationPolicyOptions,
311309
) *map[string]map[string]interface{} {
312310
var name string
313311
if policyNum > 1 {
@@ -331,16 +329,18 @@ func buildPolicyTemplate(
331329
},
332330
}
333331

332+
// Set NamespaceSelector with policy configuration
333+
setNamespaceSelector(policyConf.ConfigurationPolicyOptions, policyTemplate)
334+
334335
if len(policyConf.ConfigurationPolicyAnnotations) > 0 {
335336
metadata := policyTemplate["objectDefinition"]["metadata"].(map[string]interface{})
336337
metadata["annotations"] = policyConf.ConfigurationPolicyAnnotations
337338
}
338339

339-
if pruneObjectBehavior != "" {
340-
configSpec := policyTemplate["objectDefinition"]["spec"].(map[string]interface{})
341-
configSpec["pruneObjectBehavior"] = policyConf.PruneObjectBehavior
342-
}
340+
configSpec := policyTemplate["objectDefinition"]["spec"].(map[string]interface{})
343341

342+
// Set EvaluationInterval with manifest overrides
343+
evaluationInterval := configPolicyOptionsOverrides.EvaluationInterval
344344
if evaluationInterval.Compliant != "" || evaluationInterval.NonCompliant != "" {
345345
evalInterval := map[string]interface{}{}
346346

@@ -352,7 +352,25 @@ func buildPolicyTemplate(
352352
evalInterval["noncompliant"] = evaluationInterval.NonCompliant
353353
}
354354

355-
policyTemplate["objectDefinition"]["spec"].(map[string]interface{})["evaluationInterval"] = evalInterval
355+
configSpec["evaluationInterval"] = evalInterval
356+
}
357+
358+
// Set NamespaceSelector with manifest overrides
359+
setNamespaceSelector(configPolicyOptionsOverrides, policyTemplate)
360+
361+
// Set PruneObjectBehavior with manifest overrides
362+
if configPolicyOptionsOverrides.PruneObjectBehavior != "" {
363+
configSpec["pruneObjectBehavior"] = configPolicyOptionsOverrides.PruneObjectBehavior
364+
}
365+
366+
// Set RemediationAction with manifest overrides
367+
if configPolicyOptionsOverrides.RemediationAction != "" {
368+
configSpec["remediationAction"] = configPolicyOptionsOverrides.RemediationAction
369+
}
370+
371+
// Set Severity with manifest overrides
372+
if configPolicyOptionsOverrides.Severity != "" {
373+
configSpec["severity"] = configPolicyOptionsOverrides.Severity
356374
}
357375

358376
return &policyTemplate

0 commit comments

Comments
 (0)