Add validations for ClusterScopedFilterPolicy

Added validations for ClusterScopedFilterPolicy to
report errors for various invalid scenarios.

Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>

changelogs/unreleased/9847-adam-jian-zhang

@@ -0,0 +1 @@
+Fix issue #9813, add validations for ClusterScopedFilterPolicy

internal/resourcepolicies/resource_policies.go

@@ -262,6 +262,7 @@ func (p *Policies) Validate() error {
 	}
 
 	if err := p.validateNamespacedFilterPolicies(); err != nil {
+	if err := p.validateClusterScopedFilterPolicy(); err != nil {
 		return errors.WithStack(err)
 	}
 
@@ -409,6 +410,41 @@ func (p *Policies) validateNamespacedFilterPolicies() error {
 			return fmt.Errorf(
 				"namespacedFilterPolicies: duplicate namespace pattern '%s' found in policies %v",
 				pattern, policyIndices)
+func (p *Policies) validateClusterScopedFilterPolicy() error {
+	if p.clusterScopedFilterPolicy == nil {
+		return nil
+	}
+
+	if len(p.clusterScopedFilterPolicy.ResourceFilters) == 0 {
+		return fmt.Errorf("clusterScopedFilterPolicy: at least one resourceFilter must be specified")
+	}
+
+	seenKinds := make(map[string]int)
+	for j, rf := range p.clusterScopedFilterPolicy.ResourceFilters {
+		if rf.IsCatchAll() {
+			return fmt.Errorf("clusterScopedFilterPolicy.resourceFilters[%d]: kinds must be specified (catch-all is not supported)", j)
+		}
+
+		for _, kind := range rf.Kinds {
+			if prevJ, ok := seenKinds[kind]; ok {
+				return fmt.Errorf("clusterScopedFilterPolicy: kind %q appears in both resourceFilters[%d] and resourceFilters[%d]", kind, prevJ, j)
+			}
+			seenKinds[kind] = j
+		}
+
+		if len(rf.LabelSelector) > 0 && len(rf.OrLabelSelectors) > 0 {
+			return fmt.Errorf("clusterScopedFilterPolicy.resourceFilters[%d]: labelSelector and orLabelSelectors cannot co-exist", j)
+		}
+
+		for k, pattern := range rf.Names {
+			if _, err := glob.Compile(pattern); err != nil {
+				return fmt.Errorf("clusterScopedFilterPolicy.resourceFilters[%d].names[%d]: invalid glob pattern %q: %v", j, k, pattern, err)
+			}
+		}
+		for k, pattern := range rf.ExcludedNames {
+			if _, err := glob.Compile(pattern); err != nil {
+				return fmt.Errorf("clusterScopedFilterPolicy.resourceFilters[%d].excludedNames[%d]: invalid glob pattern %q: %v", j, k, pattern, err)
+			}
 		}
 	}
 

internal/resourcepolicies/resource_policies_test.go

@@ -1244,6 +1244,7 @@ func TestPVCPhaseMatch(t *testing.T) {
 }
 
 func TestNamespacedFilterPolicies(t *testing.T) {
+func TestClusterScopedFilterPolicies(t *testing.T) {
 	testCases := []struct {
 		name     string
 		yamlData string
@@ -1303,6 +1304,49 @@ namespacedFilterPolicies:
 			yamlData: `version: v1
 namespacedFilterPolicies:
 - namespaces: ["test"]
+			name: "valid - single kind with names",
+			yamlData: `version: v1
+clusterScopedFilterPolicy:
+  resourceFilters:
+  - kinds: ["ClusterRole"]
+    names: ["my-app-*"]`,
+			wantErr: false,
+		},
+		{
+			name: "valid - multi-kind with labelSelector",
+			yamlData: `version: v1
+clusterScopedFilterPolicy:
+  resourceFilters:
+  - kinds: ["ClusterRole", "ClusterRoleBinding"]
+    labelSelector:
+      app: my-app`,
+			wantErr: false,
+		},
+		{
+			name: "valid - orLabelSelectors",
+			yamlData: `version: v1
+clusterScopedFilterPolicy:
+  resourceFilters:
+  - kinds: ["CustomResourceDefinition"]
+    orLabelSelectors:
+    - app: my-app
+    - app: other-app`,
+			wantErr: false,
+		},
+		{
+			name: "valid - excludedNames",
+			yamlData: `version: v1
+clusterScopedFilterPolicy:
+  resourceFilters:
+  - kinds: ["ClusterRole"]
+    names: ["my-*"]
+    excludedNames: ["my-debug-*"]`,
+			wantErr: false,
+		},
+		{
+			name: "invalid - empty resourceFilters",
+			yamlData: `version: v1
+clusterScopedFilterPolicy:
   resourceFilters: []`,
 			wantErr: true,
 			errMsg:  "at least one resourceFilter must be specified",
@@ -1420,6 +1464,49 @@ namespacedFilterPolicies:
       app: web
     orLabelSelectors:
     - env: prod`,
+			name: "invalid - empty kinds in clusterScopedFilterPolicy",
+			yamlData: `version: v1
+clusterScopedFilterPolicy:
+  resourceFilters:
+  - kinds: []
+    names: ["my-app-*"]`,
+			wantErr: true,
+			errMsg:  "kinds must be specified",
+		},
+		{
+			name: "invalid - asterisk kinds (explicit catch-all) in clusterScopedFilterPolicy",
+			yamlData: `version: v1
+clusterScopedFilterPolicy:
+  resourceFilters:
+  - kinds: ["*"]
+    labelSelector:
+      app: my-app`,
+			wantErr: true,
+			errMsg:  "kinds must be specified",
+		},
+		{
+			name: "invalid - duplicate kinds across entries",
+			yamlData: `version: v1
+clusterScopedFilterPolicy:
+  resourceFilters:
+  - kinds: ["ClusterRole"]
+    names: ["my-app-*"]
+  - kinds: ["ClusterRole"]
+    labelSelector:
+      app: other`,
+			wantErr: true,
+			errMsg:  `kind "ClusterRole" appears in both`,
+		},
+		{
+			name: "invalid - labelSelector and orLabelSelectors co-exist",
+			yamlData: `version: v1
+clusterScopedFilterPolicy:
+  resourceFilters:
+  - kinds: ["ClusterRole"]
+    labelSelector:
+      app: my-app
+    orLabelSelectors:
+    - app: other`,
 			wantErr: true,
 			errMsg:  "labelSelector and orLabelSelectors cannot co-exist",
 		},
@@ -1430,6 +1517,11 @@ namespacedFilterPolicies:
 - namespaces: ["test"]
   resourceFilters:
   - kinds: ["Pod"]
+			name: "invalid - bad glob in names",
+			yamlData: `version: v1
+clusterScopedFilterPolicy:
+  resourceFilters:
+  - kinds: ["ClusterRole"]
     names: ["[invalid"]`,
 			wantErr: true,
 			errMsg:  "invalid glob pattern",
@@ -1446,6 +1538,14 @@ namespacedFilterPolicies:
   - kinds: ["ConfigMap"]`,
 			wantErr: true,
 			errMsg:  "duplicate namespace pattern",
+			name: "invalid - bad glob in excludedNames",
+			yamlData: `version: v1
+clusterScopedFilterPolicy:
+  resourceFilters:
+  - kinds: ["ClusterRole"]
+    excludedNames: ["[bad"]`,
+			wantErr: true,
+			errMsg:  "invalid glob pattern",
 		},
 	}
 
@@ -1457,6 +1557,11 @@ namespacedFilterPolicies:
 			policies := &Policies{}
 			err = policies.BuildPolicy(resPolicies)
 			require.NoError(t, err) // BuildPolicy should always succeed for our test cases
+			require.NoError(t, err)
+
+			policies := &Policies{}
+			err = policies.BuildPolicy(resPolicies)
+			require.NoError(t, err)
 
 			err = policies.Validate()
 			if tc.wantErr {
@@ -1470,6 +1575,9 @@ namespacedFilterPolicies:
 				// Verify that we can retrieve the policies
 				nfPolicies := policies.GetNamespacedFilterPolicies()
 				assert.GreaterOrEqual(t, len(nfPolicies), 1) // Valid test cases have at least 1 policy
+				assert.Contains(t, err.Error(), tc.errMsg)
+			} else {
+				require.NoError(t, err)
 			}
 		})
 	}