address review comments regarding fallbacks

fix fallbacks in backup filter policies,
- refactor the code to start with global policies, and apply
  override if exists, and document the behaviour inline according
  to the design
- Ensure that unlisted cluster-scoped kinds properly fall back
  to global label selectors
- unlisted namespace-scoped kinds are explicitly skipped when
  evaluating policy label selectors

add test coverage for GetNamespaceFilter glob matching and ordering

Added a unit test to verify that GetNamespaceFilter correctly matches
compiled glob patterns against namespace strings, and properly honors
first-match semantics when a namespace matches multiple patterns.

Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>
This commit is contained in:
Adam Zhang
2026-06-08 09:54:13 +08:00
parent a35aacfb50
commit f216f2497d
2 changed files with 100 additions and 21 deletions
+71
View File
@@ -30,6 +30,7 @@ import (
"testing"
"time"
"github.com/gobwas/glob"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
@@ -6048,3 +6049,73 @@ func TestBackupWithResPoliciesLogs(t *testing.T) {
err = h.backupper.Backup(h.log, backupReq, backupFile, nil, nil, nil)
require.Error(t, err)
}
func TestGetNamespaceFilter(t *testing.T) {
// Pre-compile our globs to simulate what resolveNamespacedFilterPolicies does
teamFrontendGlob, err := glob.Compile("team-frontend-*")
require.NoError(t, err)
teamGlob, err := glob.Compile("team-*")
require.NoError(t, err)
// Define our filter map
filterMap := map[string]*ResolvedNamespaceFilter{
"exact-match-ns": {CatchAllFilter: &ResolvedResourceFilter{}},
"team-frontend-*": {CatchAllFilter: &ResolvedResourceFilter{}},
"team-*": {CatchAllFilter: &ResolvedResourceFilter{}},
}
// Create request with patterns in a specific order (first-match semantics)
req := &Request{
NamespacedFilterMap: filterMap,
NamespacedFilterPatterns: []NamespacedFilterPattern{
{Pattern: "team-frontend-*", Compiled: teamFrontendGlob}, // Most specific first
{Pattern: "team-*", Compiled: teamGlob}, // Broader second
},
}
tests := []struct {
name string
namespace string
expectNil bool
expectMatched string // The pattern or exact string that should match
}{
{
name: "exact string match bypasses glob matching",
namespace: "exact-match-ns",
expectNil: false,
expectMatched: "exact-match-ns",
},
{
name: "reviewer requested: glob pattern matching",
namespace: "team-backend-prod",
expectNil: false,
expectMatched: "team-*",
},
{
name: "reviewer requested: first-match ordering",
namespace: "team-frontend-prod",
expectNil: false,
expectMatched: "team-frontend-*", // Should match this because it's first in NamespacedFilterPatterns
},
{
name: "no match returns nil",
namespace: "unrelated-ns",
expectNil: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := req.GetNamespaceFilter(tt.namespace)
if tt.expectNil {
assert.Nil(t, result)
} else {
assert.NotNil(t, result)
// Ensure the returned filter points to the correct reference in our map
assert.Same(t, filterMap[tt.expectMatched], result)
}
})
}
}
+29 -21
View File
@@ -538,46 +538,54 @@ func (r *itemCollector) listResourceByLabelsPerNamespace(
return nil, err
}
// Determine label selectors — per-namespace/per-kind or global
// 1. Start with global selectors (existing default behavior)
var orLabelSelectors []string
var labelSelector string
if r.backupRequest.Spec.OrLabelSelectors != nil {
for _, s := range r.backupRequest.Spec.OrLabelSelectors {
orLabelSelectors = append(orLabelSelectors, metav1.FormatLabelSelector(s))
}
}
if selector := r.backupRequest.Spec.LabelSelector; selector != nil {
labelSelector = metav1.FormatLabelSelector(selector)
}
// 2. Apply fine-grained filter overrides if applicable
if !resource.Namespaced && r.backupRequest.ClusterScopedFilterMap != nil {
rf := r.backupRequest.ClusterScopedFilterMap[gr.String()]
if rf != nil {
if rf := r.backupRequest.ClusterScopedFilterMap[gr.String()]; rf != nil {
// Overwrite global selectors with specific filter
orLabelSelectors = nil
labelSelector = ""
if rf.LabelSelector != nil {
labelSelector = rf.LabelSelector.String()
}
if len(rf.OrLabelSelectors) > 0 {
for _, s := range rf.OrLabelSelectors {
orLabelSelectors = append(orLabelSelectors, s.String())
}
for _, s := range rf.OrLabelSelectors {
orLabelSelectors = append(orLabelSelectors, s.String())
}
}
// ClusterScopedFilterPolicy: If rf == nil, it intentionally falls back to the global selectors initialized above
} else if nsFilter := r.backupRequest.GetNamespaceFilter(namespace); nsFilter != nil {
rf := nsFilter.ResourceFilterMap[gr.String()]
if rf == nil {
rf = nsFilter.CatchAllFilter
}
if rf != nil {
// Overwrite global selectors with specific filter
orLabelSelectors = nil
labelSelector = ""
if rf.LabelSelector != nil {
labelSelector = rf.LabelSelector.String()
}
if len(rf.OrLabelSelectors) > 0 {
for _, s := range rf.OrLabelSelectors {
orLabelSelectors = append(orLabelSelectors, s.String())
}
for _, s := range rf.OrLabelSelectors {
orLabelSelectors = append(orLabelSelectors, s.String())
}
}
} else {
// Use global selectors (existing behavior)
if r.backupRequest.Spec.OrLabelSelectors != nil {
for _, s := range r.backupRequest.Spec.OrLabelSelectors {
orLabelSelectors = append(orLabelSelectors, metav1.FormatLabelSelector(s))
}
}
if selector := r.backupRequest.Spec.LabelSelector; selector != nil {
labelSelector = metav1.FormatLabelSelector(selector)
} else {
// NamespacedFilterPolicies: namespacedFilterPolicies acts as an exclusive allowlist.
// If neither a kind-specific entry nor a catch-all entry exists, skip the kind.
logger.Debug("Skipping resource kind for namespace as it is not present in the namespace filter policy")
return nil, nil
}
}