From 16f513e05ad5618e346f953e2a7797d755dbaac2 Mon Sep 17 00:00:00 2001 From: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com> Date: Fri, 26 Jan 2024 14:36:20 -0500 Subject: [PATCH] Add `recordDiff` to policy generator ref: https://issues.redhat.com/browse/ACM-9088 Signed-off-by: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com> --- docs/policygenerator-reference.yaml | 7 ++ internal/plugin.go | 12 ++- internal/plugin_test.go | 4 + internal/types/types.go | 1 + internal/utils.go | 5 + internal/utils_test.go | 139 ++++++++++++++++------------ 6 files changed, 108 insertions(+), 60 deletions(-) diff --git a/docs/policygenerator-reference.yaml b/docs/policygenerator-reference.yaml index b78f0b3..259ff25 100644 --- a/docs/policygenerator-reference.yaml +++ b/docs/policygenerator-reference.yaml @@ -160,6 +160,9 @@ policyDefaults: # the responsibility of the administrator to ensure the placement rule exists. Use of this setting will prevent a # placement rule from being generated, but the placement binding will still be created. placementRuleName: "" + # Optional. Whether (and where) to record the diff between the policy and objects on the cluster. Defaults to an empty + # string, which is equivalent to "None". + recordDiff: "" # Optional. The remediation action ("inform" or "enforce") for each configuration policy. This defaults to "inform". remediationAction: "inform" # Optional. The severity of the policy violation. This defaults to "low". @@ -238,6 +241,8 @@ policies: # Optional. (See policyDefaults.remediationAction for description.) # Cannot be specified when policyDefaults.consolidateManifests is set to true. remediationAction: "" + # Optional. (See policyDefaults.recordDiff for description.) + recordDiff: "" # Optional. (See policyDefaults.severity for description.) # Cannot be specified when policyDefaults.consolidateManifests is set to true. severity: "low" @@ -308,6 +313,8 @@ policies: placement: {} # Optional. (See policyDefaults.remediationAction for description.) remediationAction: "" + # Optional. (See policyDefaults.recordDiff for description.) + recordDiff: "" # Optional. (See policyDefaults.severity for description.) severity: "low" # Optional. (See policyDefaults.standards for description.) diff --git a/internal/plugin.go b/internal/plugin.go index 27bed5f..944485a 100644 --- a/internal/plugin.go +++ b/internal/plugin.go @@ -563,11 +563,15 @@ func (p *Plugin) applyDefaults(unmarshaledConfig map[string]interface{}) { policy.Description = p.PolicyDefaults.Description } + if policy.RecordDiff == "" { + policy.RecordDiff = p.PolicyDefaults.RecordDiff + } + if policy.ComplianceType == "" { policy.ComplianceType = p.PolicyDefaults.ComplianceType } - if policy.MetadataComplianceType == "" && p.PolicyDefaults.MetadataComplianceType != "" { + if policy.MetadataComplianceType == "" { policy.MetadataComplianceType = p.PolicyDefaults.MetadataComplianceType } @@ -693,7 +697,7 @@ func (p *Plugin) applyDefaults(unmarshaledConfig map[string]interface{}) { manifest.ComplianceType = policy.ComplianceType } - if manifest.MetadataComplianceType == "" && policy.MetadataComplianceType != "" { + if manifest.MetadataComplianceType == "" { manifest.MetadataComplianceType = policy.MetadataComplianceType } @@ -729,6 +733,10 @@ func (p *Plugin) applyDefaults(unmarshaledConfig map[string]interface{}) { manifest.Severity = policy.Severity } + if manifest.RecordDiff == "" { + manifest.RecordDiff = policy.RecordDiff + } + if isManifestFieldSet(unmarshaledConfig, i, j, "extraDependencies") { applyDefaultDependencyFields(manifest.ExtraDependencies, p.PolicyDefaults.Namespace) } else { diff --git a/internal/plugin_test.go b/internal/plugin_test.go index 358bc36..696b28b 100644 --- a/internal/plugin_test.go +++ b/internal/plugin_test.go @@ -42,6 +42,7 @@ func TestGenerate(t *testing.T) { p.PolicyDefaults.Placement.Name = "my-placement" p.PolicyDefaults.Namespace = "my-policies" p.PolicyDefaults.MetadataComplianceType = "musthave" + p.PolicyDefaults.RecordDiff = "Log" p.PolicyDefaults.PruneObjectBehavior = "DeleteAll" patch := map[string]interface{}{ "metadata": map[string]interface{}{ @@ -68,6 +69,7 @@ func TestGenerate(t *testing.T) { { ConfigurationPolicyOptions: types.ConfigurationPolicyOptions{ MetadataComplianceType: "mustonlyhave", + RecordDiff: "None", }, Path: path.Join(tmpDir, "configmap.yaml"), }, @@ -117,6 +119,7 @@ spec: labels: chandler: bing name: my-configmap + recordDiff: Log pruneObjectBehavior: None remediationAction: inform severity: low @@ -151,6 +154,7 @@ spec: kind: ConfigMap metadata: name: my-configmap + recordDiff: None pruneObjectBehavior: DeleteAll remediationAction: inform severity: low diff --git a/internal/types/types.go b/internal/types/types.go index 1d4f608..a910fcf 100644 --- a/internal/types/types.go +++ b/internal/types/types.go @@ -43,6 +43,7 @@ type ConfigurationPolicyOptions struct { EvaluationInterval EvaluationInterval `json:"evaluationInterval,omitempty" yaml:"evaluationInterval,omitempty"` NamespaceSelector NamespaceSelector `json:"namespaceSelector,omitempty" yaml:"namespaceSelector,omitempty"` PruneObjectBehavior string `json:"pruneObjectBehavior,omitempty" yaml:"pruneObjectBehavior,omitempty"` + RecordDiff string `json:"recordDiff,omitempty" yaml:"recordDiff,omitempty"` } type Manifest struct { diff --git a/internal/utils.go b/internal/utils.go index da272f7..5561818 100644 --- a/internal/utils.go +++ b/internal/utils.go @@ -171,6 +171,7 @@ func getPolicyTemplates(policyConf *types.PolicyConfig) ([]map[string]interface{ for i, manifestGroup := range manifestGroups { complianceType := policyConf.Manifests[i].ComplianceType metadataComplianceType := policyConf.Manifests[i].MetadataComplianceType + recordDiff := policyConf.Manifests[i].RecordDiff ignorePending := policyConf.Manifests[i].IgnorePending extraDeps := policyConf.Manifests[i].ExtraDependencies @@ -218,6 +219,10 @@ func getPolicyTemplates(policyConf *types.PolicyConfig) ([]map[string]interface{ objTemplate["metadataComplianceType"] = metadataComplianceType } + if recordDiff != "" { + objTemplate["recordDiff"] = recordDiff + } + if policyConf.ConsolidateManifests { // put all objTemplate with manifest into single consolidated objectTemplates objectTemplates = append(objectTemplates, objTemplate) diff --git a/internal/utils_test.go b/internal/utils_test.go index 72c0365..9e40130 100644 --- a/internal/utils_test.go +++ b/internal/utils_test.go @@ -175,6 +175,7 @@ data: ConfigurationPolicyOptions: types.ConfigurationPolicyOptions{ ComplianceType: "musthave", MetadataComplianceType: "mustonlyhave", + RecordDiff: "Log", }, Path: manifestPath, }, @@ -186,6 +187,7 @@ data: ConfigurationPolicyOptions: types.ConfigurationPolicyOptions{ ComplianceType: "mustnothave", MetadataComplianceType: "musthave", + RecordDiff: "None", }, Path: manifestPath, }, @@ -202,25 +204,29 @@ data: } // Test both passing in individual files and a flat directory - tests := []struct { + tests := map[string]struct { ExpectedComplianceType string ExpectedMetadataComplianceType string + ExpectedRecordDiff string Manifests []types.Manifest }{ - { + "musthave compType/mustonlyhave metaCompType/Log recDiff": { ExpectedComplianceType: "musthave", ExpectedMetadataComplianceType: "mustonlyhave", + ExpectedRecordDiff: "Log", Manifests: manifestFiles, }, - { + "mustnothave compType/musthave metaCompType/None recDiff": { ExpectedComplianceType: "mustnothave", ExpectedMetadataComplianceType: "musthave", + ExpectedRecordDiff: "None", Manifests: manifestFilesMustNotHave, }, // The applyDefaults method would normally fill in ComplianceType on each manifest entry. - { + "musthave compType/empty metaCompType/empty recDiff": { ExpectedComplianceType: "musthave", ExpectedMetadataComplianceType: "", + ExpectedRecordDiff: "", Manifests: []types.Manifest{{ ConfigurationPolicyOptions: types.ConfigurationPolicyOptions{ComplianceType: "musthave"}, Path: tmpDir, @@ -230,74 +236,91 @@ data: // test ConsolidateManifests = true (default case) // policyTemplates will have only one policyTemplate // and two objTemplate under this policyTemplate - for _, test := range tests { - policyConf := types.PolicyConfig{ - PolicyOptions: types.PolicyOptions{ - ConsolidateManifests: true, - }, - ConfigurationPolicyOptions: types.ConfigurationPolicyOptions{ - ComplianceType: "musthave", - RemediationAction: "inform", - Severity: "low", - }, - Manifests: test.Manifests, - Name: "policy-app-config", - } + for testName, test := range tests { + test := test - policyTemplates, err := getPolicyTemplates(&policyConf) - if err != nil { - t.Fatalf("Failed to get the policy templates: %v", err) - } + t.Run(testName, func(t *testing.T) { + t.Parallel() + policyConf := types.PolicyConfig{ + PolicyOptions: types.PolicyOptions{ + ConsolidateManifests: true, + }, + ConfigurationPolicyOptions: types.ConfigurationPolicyOptions{ + ComplianceType: "musthave", + RemediationAction: "inform", + Severity: "low", + }, + Manifests: test.Manifests, + Name: "policy-app-config", + } - assertEqual(t, len(policyTemplates), 1) + policyTemplates, err := getPolicyTemplates(&policyConf) + if err != nil { + t.Fatalf("Failed to get the policy templates: %v", err) + } - policyTemplate := policyTemplates[0] - objdef := policyTemplate["objectDefinition"].(map[string]interface{}) + assertEqual(t, len(policyTemplates), 1) - assertEqual(t, objdef["metadata"].(map[string]interface{})["name"].(string), "policy-app-config") + policyTemplate := policyTemplates[0] + objdef := policyTemplate["objectDefinition"].(map[string]interface{}) - spec, ok := objdef["spec"].(map[string]interface{}) - if !ok { - t.Fatal("The spec field is an invalid format") - } + assertEqual(t, objdef["metadata"].(map[string]interface{})["name"].(string), "policy-app-config") - assertEqual(t, spec["remediationAction"], "inform") - assertEqual(t, spec["severity"], "low") + spec, ok := objdef["spec"].(map[string]interface{}) + if !ok { + t.Fatal("The spec field is an invalid format") + } - objTemplates, ok := spec["object-templates"].([]map[string]interface{}) - if !ok { - t.Fatal("The object-templates field is an invalid format") - } + assertEqual(t, spec["remediationAction"], "inform") + assertEqual(t, spec["severity"], "low") - assertEqual(t, len(objTemplates), 2) - assertEqual(t, objTemplates[0]["complianceType"], test.ExpectedComplianceType) + objTemplates, ok := spec["object-templates"].([]map[string]interface{}) + if !ok { + t.Fatal("The object-templates field is an invalid format") + } - if test.ExpectedMetadataComplianceType != "" { - assertEqual(t, objTemplates[0]["metadataComplianceType"], test.ExpectedMetadataComplianceType) - } else { - assertEqual(t, objTemplates[0]["metadataComplianceType"], nil) - } + assertEqual(t, len(objTemplates), 2) + assertEqual(t, objTemplates[0]["complianceType"], test.ExpectedComplianceType) - kind1, ok := objTemplates[0]["objectDefinition"].(map[string]interface{})["kind"] - if !ok { - t.Fatal("The objectDefinition field is an invalid format") - } + if test.ExpectedMetadataComplianceType != "" { + assertEqual(t, objTemplates[0]["metadataComplianceType"], test.ExpectedMetadataComplianceType) + } else { + assertEqual(t, objTemplates[0]["metadataComplianceType"], nil) + } - assertEqual(t, kind1, "ConfigMap") - assertEqual(t, objTemplates[1]["complianceType"], test.ExpectedComplianceType) + if test.ExpectedRecordDiff != "" { + assertEqual(t, objTemplates[0]["recordDiff"], test.ExpectedRecordDiff) + } else { + assertEqual(t, objTemplates[0]["recordDiff"], nil) + } - if test.ExpectedMetadataComplianceType != "" { - assertEqual(t, objTemplates[0]["metadataComplianceType"], test.ExpectedMetadataComplianceType) - } else { - assertEqual(t, objTemplates[0]["metadataComplianceType"], nil) - } + kind1, ok := objTemplates[0]["objectDefinition"].(map[string]interface{})["kind"] + if !ok { + t.Fatal("The objectDefinition field is an invalid format") + } - kind2, ok := objTemplates[1]["objectDefinition"].(map[string]interface{})["kind"] - if !ok { - t.Fatal("The objectDefinition field is an invalid format") - } + assertEqual(t, kind1, "ConfigMap") + assertEqual(t, objTemplates[1]["complianceType"], test.ExpectedComplianceType) - assertEqual(t, kind2, "ConfigMap") + if test.ExpectedMetadataComplianceType != "" { + assertEqual(t, objTemplates[1]["metadataComplianceType"], test.ExpectedMetadataComplianceType) + } else { + assertEqual(t, objTemplates[1]["metadataComplianceType"], nil) + } + + if test.ExpectedRecordDiff != "" { + assertEqual(t, objTemplates[1]["recordDiff"], test.ExpectedRecordDiff) + } else { + assertEqual(t, objTemplates[1]["recordDiff"], nil) + } + + kind2, ok := objTemplates[1]["objectDefinition"].(map[string]interface{})["kind"] + if !ok { + t.Fatal("The objectDefinition field is an invalid format") + } + + assertEqual(t, kind2, "ConfigMap") + }) } }