Count policyset placement and validate config (#35)

Policyset placement was not counted in config validation which was causing
placement to be generated by default

Signed-off-by: Yu Cao <[email protected]>

Author: Yu Cao

internal/plugin.go

@@ -587,6 +587,76 @@ func (p *Plugin) assertValidConfig() error {
 		}
 	}
 
+	for i := range p.PolicySets {
+		seen := map[string]bool{}
+		plcset := &p.PolicySets[i]
+
+		if plcset.Name == "" {
+			return fmt.Errorf(
+				"each policySet must have a name set, but did not find a name at policySet array index %d", i,
+			)
+		}
+
+		if seen[plcset.Name] {
+			return fmt.Errorf(
+				"each policySet must have a unique name set, but found a duplicate name: %s", plcset.Name,
+			)
+		}
+		seen[plcset.Name] = true
+
+		// Validate policy Placement settings
+		if plcset.Placement.PlacementRulePath != "" && plcset.Placement.PlacementPath != "" {
+			return fmt.Errorf(
+				"policySet %s must provide only one of placementRulePath or placementPath", plcset.Name,
+			)
+		}
+
+		if len(plcset.Placement.ClusterSelectors) > 0 && len(plcset.Placement.LabelSelector) > 0 {
+			return fmt.Errorf(
+				"policySet %s must provide only one of placement.labelSelector or placement.clusterselectors",
+				plcset.Name,
+			)
+		}
+		if (len(plcset.Placement.ClusterSelectors) != 0 || len(plcset.Placement.LabelSelector) != 0) &&
+			(plcset.Placement.PlacementRulePath != "" || plcset.Placement.PlacementPath != "") {
+			return fmt.Errorf(
+				"policySet %s may not specify a placement selector and placement path together", plcset.Name,
+			)
+		}
+		if plcset.Placement.PlacementRulePath != "" {
+			_, err := os.Stat(plcset.Placement.PlacementRulePath)
+			if err != nil {
+				return fmt.Errorf(
+					"could not read the placement rule path %s",
+					plcset.Placement.PlacementRulePath,
+				)
+			}
+		}
+		if plcset.Placement.PlacementPath != "" {
+			_, err := os.Stat(plcset.Placement.PlacementPath)
+			if err != nil {
+				return fmt.Errorf(
+					"could not read the placement path %s",
+					plcset.Placement.PlacementPath,
+				)
+			}
+		}
+
+		foundPl := false
+		if len(plcset.Placement.LabelSelector) != 0 || plcset.Placement.PlacementPath != "" {
+			plCount.plc++
+			foundPl = true
+		}
+		if len(plcset.Placement.ClusterSelectors) != 0 || plcset.Placement.PlacementRulePath != "" {
+			plCount.plr++
+			if foundPl {
+				return fmt.Errorf(
+					"policySet %s may not use both Placement and PlacementRule kinds", plcset.Name,
+				)
+			}
+		}
+	}
+
 	// Validate only one type of placement kind is in use
 	if plCount.plc != 0 && plCount.plr != 0 {
 		return fmt.Errorf(

internal/plugin_test.go

@@ -1347,6 +1347,10 @@ func TestGeneratePolicySetsWithPlacement(t *testing.T) {
 
 	p.applyDefaults(map[string]interface{}{})
 
+	if err := p.assertValidConfig(); err != nil {
+		t.Fatal(err.Error())
+	}
+
 	expected := `
 ---
 apiVersion: policy.open-cluster-management.io/v1
@@ -1389,25 +1393,26 @@ spec:
     policies:
         - policy-app-config
 ---
-apiVersion: cluster.open-cluster-management.io/v1alpha1
-kind: Placement
+apiVersion: apps.open-cluster-management.io/v1
+kind: PlacementRule
 metadata:
     name: my-placement-rule
     namespace: my-policies
 spec:
-    predicates:
-        - requiredClusterSelector:
-            labelSelector:
-                matchExpressions: []
+    clusterConditions:
+        - status: "True"
+          type: ManagedClusterConditionAvailable
+    clusterSelector:
+        matchExpressions: []
 ---
 apiVersion: policy.open-cluster-management.io/v1
 kind: PlacementBinding
 metadata:
     name: my-placement-binding
     namespace: my-policies
 placementRef:
-    apiGroup: cluster.open-cluster-management.io
-    kind: Placement
+    apiGroup: apps.open-cluster-management.io
+    kind: PlacementRule
     name: my-placement-rule
 subjects:
     - apiGroup: policy.open-cluster-management.io
@@ -1458,6 +1463,9 @@ func TestGeneratePolicySetsWithPolicyPlacement(t *testing.T) {
 	}
 
 	p.applyDefaults(map[string]interface{}{})
+	if err := p.assertValidConfig(); err != nil {
+		t.Fatal(err.Error())
+	}
 	p.Policies[0].GeneratePlacementWhenInSet = true
 
 	expected := `
@@ -1502,40 +1510,42 @@ spec:
     policies:
         - policy-app-config
 ---
-apiVersion: cluster.open-cluster-management.io/v1alpha1
-kind: Placement
+apiVersion: apps.open-cluster-management.io/v1
+kind: PlacementRule
 metadata:
     name: my-placement
     namespace: my-policies
 spec:
-    predicates:
-        - requiredClusterSelector:
-            labelSelector:
-                matchExpressions: []
+    clusterConditions:
+        - status: "True"
+          type: ManagedClusterConditionAvailable
+    clusterSelector:
+        matchExpressions: []
 ---
-apiVersion: cluster.open-cluster-management.io/v1alpha1
-kind: Placement
+apiVersion: apps.open-cluster-management.io/v1
+kind: PlacementRule
 metadata:
     name: policyset-placement
     namespace: my-policies
 spec:
-    predicates:
-        - requiredClusterSelector:
-            labelSelector:
-                matchExpressions:
-                    - key: my
-                      operator: In
-                      values:
-                        - app
+    clusterConditions:
+        - status: "True"
+          type: ManagedClusterConditionAvailable
+    clusterSelector:
+        matchExpressions:
+            - key: my
+              operator: In
+              values:
+                - app
 ---
 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
+    apiGroup: apps.open-cluster-management.io
+    kind: PlacementRule
     name: my-placement
 subjects:
     - apiGroup: policy.open-cluster-management.io
@@ -1548,8 +1558,8 @@ metadata:
     name: my-placement-binding
     namespace: my-policies
 placementRef:
-    apiGroup: cluster.open-cluster-management.io
-    kind: Placement
+    apiGroup: apps.open-cluster-management.io
+    kind: PlacementRule
     name: policyset-placement
 subjects:
     - apiGroup: policy.open-cluster-management.io