Add recordDiff to policy generator

ref: https://issues.redhat.com/browse/ACM-9088 Signed-off-by: Dale Haiducek <[email protected]>
open-cluster-management-io · Jan 29, 2024 · 16f513e · 16f513e
1 parent 4bf2b47
commit 16f513e
Show file tree

Hide file tree

Showing 6 changed files with 108 additions and 60 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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")
+		})
 	}
 }