Allow override across different placement types

This change will allow placements to override placementrule defaults and vice versa. ref: https://issues.redhat.com/browse/ACM-10152 Signed-off-by: Jason Zhang <jaszhang@redhat.com>
open-cluster-management-io · Apr 10, 2024 · a097bd7 · a097bd7
1 parent 1a25c98
commit a097bd7
Show file tree

Hide file tree

Showing 3 changed files with 211 additions and 23 deletions.
diff --git a/internal/plugin.go b/internal/plugin.go
@@ -834,28 +834,32 @@ func applyDefaultPlacementFields(placement *types.PlacementConfig, defaultPlacem
 		len(defaultPlacement.ClusterSelector) != 0 ||
 		defaultPlacement.PlacementRulePath != "" ||
 		defaultPlacement.PlacementRuleName != ""
+	policyPlcUnset := len(placement.LabelSelector) == 0 &&
+		placement.PlacementPath == "" &&
+		placement.PlacementName == ""
+	policyPlrUnset := len(placement.ClusterSelectors) == 0 &&
+		len(placement.ClusterSelector) == 0 &&
+		placement.PlacementRulePath == "" &&
+		placement.PlacementRuleName == ""
 
 	// If both cluster label selectors and placement path/name aren't set, then use the defaults with a
 	// priority on placement path followed by placement name.
-	if len(placement.LabelSelector) == 0 &&
-		placement.PlacementPath == "" &&
-		placement.PlacementName == "" &&
-		plcDefaultSet {
-		if defaultPlacement.PlacementPath != "" {
+	if policyPlcUnset && plcDefaultSet {
+		if !policyPlrUnset {
+			return
+		} else if defaultPlacement.PlacementPath != "" {
 			placement.PlacementPath = defaultPlacement.PlacementPath
 		} else if defaultPlacement.PlacementName != "" {
 			placement.PlacementName = defaultPlacement.PlacementName
 		} else if len(defaultPlacement.LabelSelector) > 0 {
 			placement.LabelSelector = defaultPlacement.LabelSelector
 		}
-	} else if len(placement.ClusterSelectors) == 0 &&
+	} else if policyPlrUnset && plrDefaultSet {
 		// Else if both cluster selectors and placement rule path/name aren't set, then use the defaults with a
 		// priority on placement rule path followed by placement rule name.
-		len(placement.ClusterSelector) == 0 &&
-		placement.PlacementRulePath == "" &&
-		placement.PlacementRuleName == "" &&
-		plrDefaultSet {
-		if defaultPlacement.PlacementRulePath != "" {
+		if !policyPlcUnset {
+			return
+		} else if defaultPlacement.PlacementRulePath != "" {
 			placement.PlacementRulePath = defaultPlacement.PlacementRulePath
 		} else if defaultPlacement.PlacementRuleName != "" {
 			placement.PlacementRuleName = defaultPlacement.PlacementRuleName

diff --git a/internal/plugin_config_test.go b/internal/plugin_config_test.go
@@ -788,13 +788,9 @@ policies:
 	p := Plugin{}
 
 	err = p.Config([]byte(config), tmpDir)
-	if err == nil {
-		t.Fatal("Expected an error but did not get one")
+	if err != nil {
+		t.Fatal(err.Error())
 	}
-
-	expected := "policy policy-app-config must specify only one of " +
-		"placement selector, placement path, or placement name"
-	assertEqual(t, err.Error(), expected)
 }
 
 func TestConfigMultipleDefaultAndPolicyPlacementNames(t *testing.T) {
@@ -824,13 +820,9 @@ policies:
 	p := Plugin{}
 
 	err := p.Config([]byte(config), tmpDir)
-	if err == nil {
-		t.Fatal("Expected an error but did not get one")
+	if err != nil {
+		t.Fatal(err.Error())
 	}
-
-	expected := "policy policy-app-config must specify only one of " +
-		"placement selector, placement path, or placement name"
-	assertEqual(t, err.Error(), expected)
 }
 
 func TestConfigPlacementInvalidMixture(t *testing.T) {

diff --git a/internal/plugin_test.go b/internal/plugin_test.go
@@ -574,6 +574,198 @@ subjects:
 	assertEqual(t, string(output), expected)
 }
 
+func TestGeneratePolicyOverrideDefaultPlacement(t *testing.T) {
+	t.Parallel()
+	tmpDir := t.TempDir()
+	createConfigMap(t, tmpDir, "configmap.yaml")
+
+	p := Plugin{}
+	var err error
+
+	p.baseDirectory, err = filepath.EvalSymlinks(tmpDir)
+	if err != nil {
+		t.Fatal(err.Error())
+	}
+
+	p.PolicyDefaults.Placement.PlacementName = "my-placement"
+	p.PolicyDefaults.Namespace = "my-policies"
+	PolicyConf := types.PolicyConfig{
+		Name: "policy-app-config",
+		Manifests: []types.Manifest{
+			{
+				Path: path.Join(tmpDir, "configmap.yaml"),
+			},
+		},
+		PolicyOptions: types.PolicyOptions{
+			Placement: types.PlacementConfig{
+				PlacementRuleName: "my-placement-rule",
+			},
+		},
+	}
+	p.Policies = append(p.Policies, PolicyConf)
+	p.applyDefaults(map[string]interface{}{})
+
+	assertEqual(t, p.Policies[0].ConsolidateManifests, true)
+
+	if err := p.assertValidConfig(); err != nil {
+		t.Fatal(err.Error())
+	}
+
+	expected := `
+---
+apiVersion: policy.open-cluster-management.io/v1
+kind: Policy
+metadata:
+    annotations:
+        policy.open-cluster-management.io/categories: CM Configuration Management
+        policy.open-cluster-management.io/controls: CM-2 Baseline Configuration
+        policy.open-cluster-management.io/description: ""
+        policy.open-cluster-management.io/standards: NIST SP 800-53
+    name: policy-app-config
+    namespace: my-policies
+spec:
+    disabled: false
+    policy-templates:
+        - objectDefinition:
+            apiVersion: policy.open-cluster-management.io/v1
+            kind: ConfigurationPolicy
+            metadata:
+                name: policy-app-config
+            spec:
+                object-templates:
+                    - complianceType: musthave
+                      objectDefinition:
+                        apiVersion: v1
+                        data:
+                            game.properties: enemies=potato
+                        kind: ConfigMap
+                        metadata:
+                            name: my-configmap
+                remediationAction: inform
+                severity: low
+    remediationAction: inform
+---
+apiVersion: policy.open-cluster-management.io/v1
+kind: PlacementBinding
+metadata:
+    name: binding-policy-app-config
+    namespace: my-policies
+placementRef:
+    apiGroup: apps.open-cluster-management.io
+    kind: PlacementRule
+    name: my-placement-rule
+subjects:
+    - apiGroup: policy.open-cluster-management.io
+      kind: Policy
+      name: policy-app-config
+`
+
+	expected = strings.TrimPrefix(expected, "\n")
+
+	output, err := p.Generate()
+	if err != nil {
+		t.Fatal(err.Error())
+	}
+
+	assertEqual(t, string(output), expected)
+}
+
+func TestGeneratePolicyOverrideDefaultPlacementRule(t *testing.T) {
+	t.Parallel()
+	tmpDir := t.TempDir()
+	createConfigMap(t, tmpDir, "configmap.yaml")
+
+	p := Plugin{}
+	var err error
+
+	p.baseDirectory, err = filepath.EvalSymlinks(tmpDir)
+	if err != nil {
+		t.Fatal(err.Error())
+	}
+
+	p.PolicyDefaults.Placement.PlacementRuleName = "my-placement-rule"
+	p.PolicyDefaults.Namespace = "my-policies"
+	PolicyConf := types.PolicyConfig{
+		Name: "policy-app-config",
+		Manifests: []types.Manifest{
+			{
+				Path: path.Join(tmpDir, "configmap.yaml"),
+			},
+		},
+		PolicyOptions: types.PolicyOptions{
+			Placement: types.PlacementConfig{
+				PlacementName: "my-placement",
+			},
+		},
+	}
+	p.Policies = append(p.Policies, PolicyConf)
+	p.applyDefaults(map[string]interface{}{})
+
+	assertEqual(t, p.Policies[0].ConsolidateManifests, true)
+
+	if err := p.assertValidConfig(); err != nil {
+		t.Fatal(err.Error())
+	}
+
+	expected := `
+---
+apiVersion: policy.open-cluster-management.io/v1
+kind: Policy
+metadata:
+    annotations:
+        policy.open-cluster-management.io/categories: CM Configuration Management
+        policy.open-cluster-management.io/controls: CM-2 Baseline Configuration
+        policy.open-cluster-management.io/description: ""
+        policy.open-cluster-management.io/standards: NIST SP 800-53
+    name: policy-app-config
+    namespace: my-policies
+spec:
+    disabled: false
+    policy-templates:
+        - objectDefinition:
+            apiVersion: policy.open-cluster-management.io/v1
+            kind: ConfigurationPolicy
+            metadata:
+                name: policy-app-config
+            spec:
+                object-templates:
+                    - complianceType: musthave
+                      objectDefinition:
+                        apiVersion: v1
+                        data:
+                            game.properties: enemies=potato
+                        kind: ConfigMap
+                        metadata:
+                            name: my-configmap
+                remediationAction: inform
+                severity: low
+    remediationAction: inform
+---
+apiVersion: policy.open-cluster-management.io/v1
+kind: PlacementBinding
+metadata:
+    name: binding-policy-app-config
+    namespace: my-policies
+placementRef:
+    apiGroup: cluster.open-cluster-management.io
+    kind: Placement
+    name: my-placement
+subjects:
+    - apiGroup: policy.open-cluster-management.io
+      kind: Policy
+      name: policy-app-config
+`
+
+	expected = strings.TrimPrefix(expected, "\n")
+
+	output, err := p.Generate()
+	if err != nil {
+		t.Fatal(err.Error())
+	}
+
+	assertEqual(t, string(output), expected)
+}
+
 func TestGeneratePolicyExistingPlacementName(t *testing.T) {
 	t.Parallel()
 	tmpDir := t.TempDir()