Skip to content

Commit b63a522

Browse files
Joeavaikathclaude
Joeavaikath
and
claude
committed
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 <jvaikath@redhat.com> * Changelog Signed-off-by: Joseph <jvaikath@redhat.com> * 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) <noreply@anthropic.com> Signed-off-by: Joseph <jvaikath@redhat.com> * 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) <noreply@anthropic.com> Signed-off-by: Joseph <jvaikath@redhat.com> * 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) <noreply@anthropic.com> Signed-off-by: Joseph <jvaikath@redhat.com> * Add checks around empty includenamespaces Signed-off-by: Joseph <jvaikath@redhat.com> * gofmt Signed-off-by: Joseph <jvaikath@redhat.com> --------- Signed-off-by: Joseph <jvaikath@redhat.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4c91959 commit b63a522

5 files changed

Lines changed: 70 additions & 5 deletions

File tree

‎changelogs/unreleased/9684-Joeavaikath‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix wildcard expansion when includes is empty and excludes has wildcards

‎pkg/controller/backup_controller.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,13 @@ func (b *backupReconciler) prepareBackupRequest(ctx context.Context, backup *vel
570570
}
571571
}
572572

573+
// Empty IncludedNamespaces means "include all namespaces". Normalize
574+
// to ["*"] so that downstream wildcard expansion does not collapse
575+
// an empty-includes + wildcard-excludes combination into "back up nothing".
576+
if len(request.Spec.IncludedNamespaces) == 0 {
577+
request.Spec.IncludedNamespaces = []string{"*"}
578+
}
579+
573580
// validate the included/excluded namespaces
574581
for _, err := range collections.ValidateNamespaceIncludesExcludes(request.Spec.IncludedNamespaces, request.Spec.ExcludedNamespaces) {
575582
request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("Invalid included/excluded namespace lists: %v", err))

‎pkg/controller/backup_controller_test.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,34 @@ func TestBackupLocationLabel(t *testing.T) {
320320
}
321321
}
322322

323+
func TestPrepareBackupRequest_EmptyIncludedNamespacesNormalizedToWildcard(t *testing.T) {
324+
formatFlag := logging.FormatText
325+
logger := logging.DefaultLogger(logrus.DebugLevel, formatFlag)
326+
327+
apiServer := velerotest.NewAPIServer(t)
328+
discoveryHelper, err := discovery.NewHelper(apiServer.DiscoveryClient, logger)
329+
require.NoError(t, err)
330+
331+
backupLocation := builder.ForBackupStorageLocation("velero", "loc-1").Result()
332+
fakeClient := velerotest.NewFakeControllerRuntimeClient(t, backupLocation)
333+
334+
c := &backupReconciler{
335+
discoveryHelper: discoveryHelper,
336+
kbClient: fakeClient,
337+
defaultBackupLocation: backupLocation.Name,
338+
clock: &clock.RealClock{},
339+
formatFlag: formatFlag,
340+
}
341+
342+
backup := defaultBackup().Result()
343+
backup.Spec.IncludedNamespaces = nil
344+
345+
res := c.prepareBackupRequest(ctx, backup, logger)
346+
defer res.WorkerPool.Stop()
347+
348+
assert.Equal(t, []string{"*"}, res.Spec.IncludedNamespaces)
349+
}
350+
323351
func Test_prepareBackupRequest_BackupStorageLocation(t *testing.T) {
324352
var (
325353
defaultBackupTTL = metav1.Duration{Duration: 24 * 30 * time.Hour}
@@ -709,6 +737,7 @@ func TestProcessBackupCompletions(t *testing.T) {
709737
},
710738
Spec: velerov1api.BackupSpec{
711739
StorageLocation: defaultBackupLocation.Name,
740+
IncludedNamespaces: []string{"*"},
712741
DefaultVolumesToFsBackup: boolptr.True(),
713742
SnapshotMoveData: boolptr.False(),
714743
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -749,6 +778,7 @@ func TestProcessBackupCompletions(t *testing.T) {
749778
},
750779
Spec: velerov1api.BackupSpec{
751780
StorageLocation: "alt-loc",
781+
IncludedNamespaces: []string{"*"},
752782
DefaultVolumesToFsBackup: boolptr.False(),
753783
SnapshotMoveData: boolptr.False(),
754784
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -793,6 +823,7 @@ func TestProcessBackupCompletions(t *testing.T) {
793823
},
794824
Spec: velerov1api.BackupSpec{
795825
StorageLocation: "read-write",
826+
IncludedNamespaces: []string{"*"},
796827
DefaultVolumesToFsBackup: boolptr.True(),
797828
SnapshotMoveData: boolptr.False(),
798829
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -834,6 +865,7 @@ func TestProcessBackupCompletions(t *testing.T) {
834865
Spec: velerov1api.BackupSpec{
835866
TTL: metav1.Duration{Duration: 10 * time.Minute},
836867
StorageLocation: defaultBackupLocation.Name,
868+
IncludedNamespaces: []string{"*"},
837869
DefaultVolumesToFsBackup: boolptr.False(),
838870
SnapshotMoveData: boolptr.False(),
839871
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -875,6 +907,7 @@ func TestProcessBackupCompletions(t *testing.T) {
875907
},
876908
Spec: velerov1api.BackupSpec{
877909
StorageLocation: defaultBackupLocation.Name,
910+
IncludedNamespaces: []string{"*"},
878911
DefaultVolumesToFsBackup: boolptr.True(),
879912
SnapshotMoveData: boolptr.False(),
880913
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -917,6 +950,7 @@ func TestProcessBackupCompletions(t *testing.T) {
917950
},
918951
Spec: velerov1api.BackupSpec{
919952
StorageLocation: defaultBackupLocation.Name,
953+
IncludedNamespaces: []string{"*"},
920954
DefaultVolumesToFsBackup: boolptr.False(),
921955
SnapshotMoveData: boolptr.False(),
922956
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -959,6 +993,7 @@ func TestProcessBackupCompletions(t *testing.T) {
959993
},
960994
Spec: velerov1api.BackupSpec{
961995
StorageLocation: defaultBackupLocation.Name,
996+
IncludedNamespaces: []string{"*"},
962997
DefaultVolumesToFsBackup: boolptr.True(),
963998
SnapshotMoveData: boolptr.False(),
964999
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1001,6 +1036,7 @@ func TestProcessBackupCompletions(t *testing.T) {
10011036
},
10021037
Spec: velerov1api.BackupSpec{
10031038
StorageLocation: defaultBackupLocation.Name,
1039+
IncludedNamespaces: []string{"*"},
10041040
DefaultVolumesToFsBackup: boolptr.True(),
10051041
SnapshotMoveData: boolptr.False(),
10061042
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1043,6 +1079,7 @@ func TestProcessBackupCompletions(t *testing.T) {
10431079
},
10441080
Spec: velerov1api.BackupSpec{
10451081
StorageLocation: defaultBackupLocation.Name,
1082+
IncludedNamespaces: []string{"*"},
10461083
DefaultVolumesToFsBackup: boolptr.False(),
10471084
SnapshotMoveData: boolptr.False(),
10481085
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1086,6 +1123,7 @@ func TestProcessBackupCompletions(t *testing.T) {
10861123
},
10871124
Spec: velerov1api.BackupSpec{
10881125
StorageLocation: defaultBackupLocation.Name,
1126+
IncludedNamespaces: []string{"*"},
10891127
DefaultVolumesToFsBackup: boolptr.True(),
10901128
SnapshotMoveData: boolptr.False(),
10911129
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1129,6 +1167,7 @@ func TestProcessBackupCompletions(t *testing.T) {
11291167
},
11301168
Spec: velerov1api.BackupSpec{
11311169
StorageLocation: defaultBackupLocation.Name,
1170+
IncludedNamespaces: []string{"*"},
11321171
DefaultVolumesToFsBackup: boolptr.True(),
11331172
SnapshotMoveData: boolptr.False(),
11341173
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1172,6 +1211,7 @@ func TestProcessBackupCompletions(t *testing.T) {
11721211
},
11731212
Spec: velerov1api.BackupSpec{
11741213
StorageLocation: defaultBackupLocation.Name,
1214+
IncludedNamespaces: []string{"*"},
11751215
DefaultVolumesToFsBackup: boolptr.False(),
11761216
SnapshotMoveData: boolptr.True(),
11771217
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1216,6 +1256,7 @@ func TestProcessBackupCompletions(t *testing.T) {
12161256
},
12171257
Spec: velerov1api.BackupSpec{
12181258
StorageLocation: defaultBackupLocation.Name,
1259+
IncludedNamespaces: []string{"*"},
12191260
DefaultVolumesToFsBackup: boolptr.False(),
12201261
SnapshotMoveData: boolptr.False(),
12211262
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1260,6 +1301,7 @@ func TestProcessBackupCompletions(t *testing.T) {
12601301
},
12611302
Spec: velerov1api.BackupSpec{
12621303
StorageLocation: defaultBackupLocation.Name,
1304+
IncludedNamespaces: []string{"*"},
12631305
DefaultVolumesToFsBackup: boolptr.False(),
12641306
SnapshotMoveData: boolptr.False(),
12651307
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1304,6 +1346,7 @@ func TestProcessBackupCompletions(t *testing.T) {
13041346
},
13051347
Spec: velerov1api.BackupSpec{
13061348
StorageLocation: defaultBackupLocation.Name,
1349+
IncludedNamespaces: []string{"*"},
13071350
DefaultVolumesToFsBackup: boolptr.False(),
13081351
SnapshotMoveData: boolptr.True(),
13091352
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1349,6 +1392,7 @@ func TestProcessBackupCompletions(t *testing.T) {
13491392
},
13501393
Spec: velerov1api.BackupSpec{
13511394
StorageLocation: defaultBackupLocation.Name,
1395+
IncludedNamespaces: []string{"*"},
13521396
DefaultVolumesToFsBackup: boolptr.False(),
13531397
SnapshotMoveData: boolptr.False(),
13541398
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1393,6 +1437,7 @@ func TestProcessBackupCompletions(t *testing.T) {
13931437
},
13941438
Spec: velerov1api.BackupSpec{
13951439
StorageLocation: defaultBackupLocation.Name,
1440+
IncludedNamespaces: []string{"*"},
13961441
DefaultVolumesToFsBackup: boolptr.False(),
13971442
SnapshotMoveData: boolptr.True(),
13981443
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1441,6 +1486,7 @@ func TestProcessBackupCompletions(t *testing.T) {
14411486
},
14421487
Spec: velerov1api.BackupSpec{
14431488
StorageLocation: defaultBackupLocation.Name,
1489+
IncludedNamespaces: []string{"*"},
14441490
DefaultVolumesToFsBackup: boolptr.False(),
14451491
SnapshotMoveData: boolptr.True(),
14461492
IncludedClusterScopedResources: []string{"storageclasses"},
@@ -1491,6 +1537,7 @@ func TestProcessBackupCompletions(t *testing.T) {
14911537
},
14921538
Spec: velerov1api.BackupSpec{
14931539
StorageLocation: defaultBackupLocation.Name,
1540+
IncludedNamespaces: []string{"*"},
14941541
DefaultVolumesToFsBackup: boolptr.False(),
14951542
SnapshotMoveData: boolptr.True(),
14961543
IncludedClusterScopedResources: []string{"storageclasses"},

‎pkg/util/wildcard/expand.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ func ShouldExpandWildcards(includes []string, excludes []string, fromBackup bool
1919
return false
2020
}
2121

22+
// Empty includes is equivalent to * (match all) - don't expand
23+
if len(includes) == 0 {
24+
return false
25+
}
26+
2227
wildcardFound := false
2328
for _, include := range includes {
2429
if containsWildcardPattern(include) {

‎pkg/util/wildcard/expand_test.go‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,16 @@ func TestShouldExpandWildcards(t *testing.T) {
8484
expected: true, // ** is a wildcard pattern (but will error during validation)
8585
},
8686
{
87-
name: "empty slices",
88-
includes: []string{},
89-
excludes: []string{},
90-
fromBackup: true,
91-
expected: false,
87+
name: "empty slices",
88+
includes: []string{},
89+
excludes: []string{},
90+
expected: false,
91+
},
92+
{
93+
name: "empty includes with wildcard excludes - should not expand",
94+
includes: []string{},
95+
excludes: []string{"ns*"},
96+
expected: false,
9297
},
9398
{
9499
name: "complex wildcard patterns",

0 commit comments

Comments
 (0)