diff --git a/changelogs/unreleased/9847-adam-jian-zhang b/changelogs/unreleased/9847-adam-jian-zhang new file mode 100644 index 000000000..82de39ca0 --- /dev/null +++ b/changelogs/unreleased/9847-adam-jian-zhang @@ -0,0 +1 @@ +Fix issue #9813, add validations for ClusterScopedFilterPolicy diff --git a/internal/resourcepolicies/resource_policies.go b/internal/resourcepolicies/resource_policies.go index 232633484..1eec8c8e2 100644 --- a/internal/resourcepolicies/resource_policies.go +++ b/internal/resourcepolicies/resource_policies.go @@ -261,6 +261,10 @@ func (p *Policies) Validate() error { } } + if err := p.validateClusterScopedFilterPolicy(); err != nil { + return errors.WithStack(err) + } + if err := p.validateNamespacedFilterPolicies(); err != nil { return errors.WithStack(err) } @@ -414,3 +418,44 @@ func (p *Policies) validateNamespacedFilterPolicies() error { return nil } + +func (p *Policies) validateClusterScopedFilterPolicy() error { + if p.clusterScopedFilterPolicy == nil { + return nil + } + + if len(p.clusterScopedFilterPolicy.ResourceFilters) == 0 { + return fmt.Errorf("clusterScopedFilterPolicy: resourceFilters cannot be empty; remove the policy block entirely if it is not needed") + } + + 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) + } + } + } + + return nil +} diff --git a/internal/resourcepolicies/resource_policies_test.go b/internal/resourcepolicies/resource_policies_test.go index 898c6d1ca..e5736a0e8 100644 --- a/internal/resourcepolicies/resource_policies_test.go +++ b/internal/resourcepolicies/resource_policies_test.go @@ -1536,3 +1536,147 @@ namespacedFilterPolicies: assert.Equal(t, []string{"team-*", "another-pattern"}, policy2.Namespaces) assert.Equal(t, []string{"Deployment", "Service"}, policy2.ResourceFilters[0].Kinds) } + +func TestClusterScopedFilterPolicies(t *testing.T) { + testCases := []struct { + name string + yamlData string + wantErr bool + errMsg string + }{ + { + 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: "resourceFilters cannot be empty; remove the policy block entirely if it is not needed", + }, + { + 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", + }, + { + name: "invalid - bad glob in names", + yamlData: `version: v1 +clusterScopedFilterPolicy: + resourceFilters: + - kinds: ["ClusterRole"] + names: ["[invalid"]`, + wantErr: true, + errMsg: "invalid glob pattern", + }, + { + name: "invalid - bad glob in excludedNames", + yamlData: `version: v1 +clusterScopedFilterPolicy: + resourceFilters: + - kinds: ["ClusterRole"] + excludedNames: ["[bad"]`, + wantErr: true, + errMsg: "invalid glob pattern", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + resPolicies, err := unmarshalResourcePolicies(&tc.yamlData) + require.NoError(t, err) + + policies := &Policies{} + err = policies.BuildPolicy(resPolicies) + require.NoError(t, err) + + err = policies.Validate() + if tc.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.errMsg) + } else { + require.NoError(t, err) + } + }) + } +}