From 8f81ac101c974ebbc4f1a982a2164028212ded23 Mon Sep 17 00:00:00 2001 From: Joseph Antony Vaikath Date: Wed, 22 Apr 2026 16:24:22 -0400 Subject: [PATCH] Fix wildcard expansion when includes is empty and excludes has wildcards (#9684) * Fix wildcard expansion when includes is empty and excludes has wildcards When a Backup CR is applied via kubectl with empty includedNamespaces and a wildcard in excludedNamespaces, ShouldExpandWildcards triggers expansion. The empty includes expands to nil, but wildcardExpanded is set to true, causing ShouldInclude to return false for all namespaces. Populate expanded includes with all active namespaces when the original includes was empty (meaning "include all") so that the wildcardExpanded check does not falsely reject everything. Signed-off-by: Joseph * Changelog Signed-off-by: Joseph * Normalize empty includes to * instead of active namespaces list This ensures consistent behavior between CLI and kubectl-apply paths for Namespace CR inclusion when excludes contain wildcards. Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Joseph * Move empty includes normalization to backup controller Instead of normalizing empty IncludedNamespaces to ["*"] in the collections layer's ExpandIncludesExcludes, do it earlier in prepareBackupRequest. This ensures the spec is correct before any downstream processing. Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Joseph * Update TestProcessBackupCompletions for wildcard normalization Add IncludedNamespaces: []string{"*"} to all expected BackupSpec structs, reflecting the new prepareBackupRequest normalization. Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Joseph * Add checks around empty includenamespaces Signed-off-by: Joseph * gofmt Signed-off-by: Joseph --------- Signed-off-by: Joseph Co-authored-by: Claude Opus 4.6 (1M context) Signed-off-by: Xun Jiang --- changelogs/unreleased/9743-Joeavaikath | 1 + pkg/controller/backup_controller.go | 7 +++ pkg/controller/backup_controller_test.go | 66 +++++++++++++++++------- pkg/util/wildcard/expand.go | 5 ++ pkg/util/wildcard/expand_test.go | 6 +++ 5 files changed, 66 insertions(+), 19 deletions(-) create mode 100644 changelogs/unreleased/9743-Joeavaikath diff --git a/changelogs/unreleased/9743-Joeavaikath b/changelogs/unreleased/9743-Joeavaikath new file mode 100644 index 000000000..d5f5d6e76 --- /dev/null +++ b/changelogs/unreleased/9743-Joeavaikath @@ -0,0 +1 @@ +Fix wildcard expansion when includes is empty and excludes has wildcards diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go index 335becdb8..490c79aed 100644 --- a/pkg/controller/backup_controller.go +++ b/pkg/controller/backup_controller.go @@ -570,6 +570,13 @@ func (b *backupReconciler) prepareBackupRequest(ctx context.Context, backup *vel } } + // Empty IncludedNamespaces means "include all namespaces". Normalize + // to ["*"] so that downstream wildcard expansion does not collapse + // an empty-includes + wildcard-excludes combination into "back up nothing". + if len(request.Spec.IncludedNamespaces) == 0 { + request.Spec.IncludedNamespaces = []string{"*"} + } + // validate the included/excluded namespaces for _, err := range collections.ValidateNamespaceIncludesExcludes(request.Spec.IncludedNamespaces, request.Spec.ExcludedNamespaces) { request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("Invalid included/excluded namespace lists: %v", err)) diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index 030ef9cdd..c65f1d15d 100644 --- a/pkg/controller/backup_controller_test.go +++ b/pkg/controller/backup_controller_test.go @@ -320,6 +320,34 @@ func TestBackupLocationLabel(t *testing.T) { } } +func TestPrepareBackupRequest_EmptyIncludedNamespacesNormalizedToWildcard(t *testing.T) { + formatFlag := logging.FormatText + logger := logging.DefaultLogger(logrus.DebugLevel, formatFlag) + + apiServer := velerotest.NewAPIServer(t) + discoveryHelper, err := discovery.NewHelper(apiServer.DiscoveryClient, logger) + require.NoError(t, err) + + backupLocation := builder.ForBackupStorageLocation("velero", "loc-1").Result() + fakeClient := velerotest.NewFakeControllerRuntimeClient(t, backupLocation) + + c := &backupReconciler{ + discoveryHelper: discoveryHelper, + kbClient: fakeClient, + defaultBackupLocation: backupLocation.Name, + clock: &clock.RealClock{}, + formatFlag: formatFlag, + } + + backup := defaultBackup().Result() + backup.Spec.IncludedNamespaces = nil + + res := c.prepareBackupRequest(ctx, backup, logger) + defer res.WorkerPool.Stop() + + assert.Equal(t, []string{"*"}, res.Spec.IncludedNamespaces) +} + func Test_prepareBackupRequest_BackupStorageLocation(t *testing.T) { var ( defaultBackupTTL = metav1.Duration{Duration: 24 * 30 * time.Hour} @@ -709,11 +737,11 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -749,11 +777,11 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: "alt-loc", + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -793,11 +821,11 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: "read-write", + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -834,11 +862,11 @@ func TestProcessBackupCompletions(t *testing.T) { Spec: velerov1api.BackupSpec{ TTL: metav1.Duration{Duration: 10 * time.Minute}, StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -875,11 +903,11 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -917,11 +945,11 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -959,11 +987,11 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1001,11 +1029,11 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1043,11 +1071,11 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1086,11 +1114,11 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFailed, @@ -1129,11 +1157,11 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFailed, @@ -1172,11 +1200,11 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.True(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1216,11 +1244,11 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1260,11 +1288,11 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1304,11 +1332,11 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.True(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1349,11 +1377,11 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1393,11 +1421,11 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.True(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1441,13 +1469,13 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.True(), IncludedClusterScopedResources: []string{"storageclasses"}, ExcludedClusterScopedResources: append([]string{"clusterroles"}, autoExcludeClusterScopedResources...), IncludedNamespaceScopedResources: []string{"pods"}, ExcludedNamespaceScopedResources: append([]string{"secrets"}, autoExcludeNamespaceScopedResources...), - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1491,13 +1519,13 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + IncludedNamespaces: []string{"*"}, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.True(), IncludedClusterScopedResources: []string{"storageclasses"}, ExcludedClusterScopedResources: append([]string{"clusterroles"}, autoExcludeClusterScopedResources...), IncludedNamespaceScopedResources: []string{"pods"}, ExcludedNamespaceScopedResources: append([]string{"secrets"}, autoExcludeNamespaceScopedResources...), - IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, diff --git a/pkg/util/wildcard/expand.go b/pkg/util/wildcard/expand.go index ec572f359..2d35e5dd5 100644 --- a/pkg/util/wildcard/expand.go +++ b/pkg/util/wildcard/expand.go @@ -19,6 +19,11 @@ func ShouldExpandWildcards(includes []string, excludes []string, fromBackup bool return false } + // Empty includes is equivalent to * (match all) - don't expand + if len(includes) == 0 { + return false + } + wildcardFound := false for _, include := range includes { if containsWildcardPattern(include) { diff --git a/pkg/util/wildcard/expand_test.go b/pkg/util/wildcard/expand_test.go index 4d1d68761..9851106da 100644 --- a/pkg/util/wildcard/expand_test.go +++ b/pkg/util/wildcard/expand_test.go @@ -90,6 +90,12 @@ func TestShouldExpandWildcards(t *testing.T) { fromBackup: true, expected: false, }, + { + name: "empty includes with wildcard excludes - should not expand", + includes: []string{}, + excludes: []string{"ns*"}, + expected: false, + }, { name: "complex wildcard patterns", includes: []string{"*-prod"},