diff --git a/internal/resourcepolicies/resource_policies.go b/internal/resourcepolicies/resource_policies.go index c0a697f22..1eec8c8e2 100644 --- a/internal/resourcepolicies/resource_policies.go +++ b/internal/resourcepolicies/resource_policies.go @@ -261,11 +261,14 @@ func (p *Policies) Validate() error { } } - if err := p.validateNamespacedFilterPolicies(); err != nil { if err := p.validateClusterScopedFilterPolicy(); err != nil { return errors.WithStack(err) } + if err := p.validateNamespacedFilterPolicies(); err != nil { + return errors.WithStack(err) + } + return nil } @@ -410,13 +413,19 @@ func (p *Policies) validateNamespacedFilterPolicies() error { return fmt.Errorf( "namespacedFilterPolicies: duplicate namespace pattern '%s' found in policies %v", pattern, policyIndices) + } + } + + return nil +} + 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") + return fmt.Errorf("clusterScopedFilterPolicy: resourceFilters cannot be empty; remove the policy block entirely if it is not needed") } seenKinds := make(map[string]int) diff --git a/internal/resourcepolicies/resource_policies_test.go b/internal/resourcepolicies/resource_policies_test.go index 8cd8955a9..e5736a0e8 100644 --- a/internal/resourcepolicies/resource_policies_test.go +++ b/internal/resourcepolicies/resource_policies_test.go @@ -1244,7 +1244,6 @@ func TestPVCPhaseMatch(t *testing.T) { } func TestNamespacedFilterPolicies(t *testing.T) { -func TestClusterScopedFilterPolicies(t *testing.T) { testCases := []struct { name string yamlData string @@ -1304,49 +1303,6 @@ 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", @@ -1464,49 +1420,6 @@ 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", }, @@ -1517,11 +1430,6 @@ 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", @@ -1538,14 +1446,6 @@ 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", }, } @@ -1557,11 +1457,6 @@ clusterScopedFilterPolicy: 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 { @@ -1575,9 +1470,6 @@ clusterScopedFilterPolicy: // 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) } }) } @@ -1644,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) + } + }) + } +}