mirrors/velero
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/velero.git synced 2026-07-28 19:12:43 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

address review comments

Browse Source 
improve wording on validation errors for empty resourceFilters

Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>
This commit is contained in:
Adam Zhang
2026-06-04 13:24:27 +08:00
parent eb0659f06d
commit ca0506daa8
2 changed files with 155 additions and 110 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/resourcepolicies/resource_policies.go
+11 -2
View File
@@ -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)
internal/resourcepolicies/resource_policies_test.go
+144 -108
View File
@@ -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)
}
})
}
}