Skip to content

Commit 3883f69

Browse files
Joeavaikathclaude
authored and
priyansh17
committed
Fix wildcard expansion when includes is empty and excludes has wildcards (velero-io#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 7d2e2ba commit 3883f69

5 files changed

Lines changed: 66 additions & 0 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,
@@ -748,6 +777,7 @@ func TestProcessBackupCompletions(t *testing.T) {
748777
},
749778
Spec: velerov1api.BackupSpec{
750779
StorageLocation: "alt-loc",
780+
IncludedNamespaces: []string{"*"},
751781
DefaultVolumesToFsBackup: boolptr.False(),
752782
SnapshotMoveData: boolptr.False(),
753783
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -791,6 +821,7 @@ func TestProcessBackupCompletions(t *testing.T) {
791821
},
792822
Spec: velerov1api.BackupSpec{
793823
StorageLocation: "read-write",
824+
IncludedNamespaces: []string{"*"},
794825
DefaultVolumesToFsBackup: boolptr.True(),
795826
SnapshotMoveData: boolptr.False(),
796827
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -831,6 +862,7 @@ func TestProcessBackupCompletions(t *testing.T) {
831862
Spec: velerov1api.BackupSpec{
832863
TTL: metav1.Duration{Duration: 10 * time.Minute},
833864
StorageLocation: defaultBackupLocation.Name,
865+
IncludedNamespaces: []string{"*"},
834866
DefaultVolumesToFsBackup: boolptr.False(),
835867
SnapshotMoveData: boolptr.False(),
836868
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -871,6 +903,7 @@ func TestProcessBackupCompletions(t *testing.T) {
871903
},
872904
Spec: velerov1api.BackupSpec{
873905
StorageLocation: defaultBackupLocation.Name,
906+
IncludedNamespaces: []string{"*"},
874907
DefaultVolumesToFsBackup: boolptr.True(),
875908
SnapshotMoveData: boolptr.False(),
876909
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -912,6 +945,7 @@ func TestProcessBackupCompletions(t *testing.T) {
912945
},
913946
Spec: velerov1api.BackupSpec{
914947
StorageLocation: defaultBackupLocation.Name,
948+
IncludedNamespaces: []string{"*"},
915949
DefaultVolumesToFsBackup: boolptr.False(),
916950
SnapshotMoveData: boolptr.False(),
917951
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -953,6 +987,7 @@ func TestProcessBackupCompletions(t *testing.T) {
953987
},
954988
Spec: velerov1api.BackupSpec{
955989
StorageLocation: defaultBackupLocation.Name,
990+
IncludedNamespaces: []string{"*"},
956991
DefaultVolumesToFsBackup: boolptr.True(),
957992
SnapshotMoveData: boolptr.False(),
958993
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -994,6 +1029,7 @@ func TestProcessBackupCompletions(t *testing.T) {
9941029
},
9951030
Spec: velerov1api.BackupSpec{
9961031
StorageLocation: defaultBackupLocation.Name,
1032+
IncludedNamespaces: []string{"*"},
9971033
DefaultVolumesToFsBackup: boolptr.True(),
9981034
SnapshotMoveData: boolptr.False(),
9991035
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1035,6 +1071,7 @@ func TestProcessBackupCompletions(t *testing.T) {
10351071
},
10361072
Spec: velerov1api.BackupSpec{
10371073
StorageLocation: defaultBackupLocation.Name,
1074+
IncludedNamespaces: []string{"*"},
10381075
DefaultVolumesToFsBackup: boolptr.False(),
10391076
SnapshotMoveData: boolptr.False(),
10401077
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1077,6 +1114,7 @@ func TestProcessBackupCompletions(t *testing.T) {
10771114
},
10781115
Spec: velerov1api.BackupSpec{
10791116
StorageLocation: defaultBackupLocation.Name,
1117+
IncludedNamespaces: []string{"*"},
10801118
DefaultVolumesToFsBackup: boolptr.True(),
10811119
SnapshotMoveData: boolptr.False(),
10821120
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1119,6 +1157,7 @@ func TestProcessBackupCompletions(t *testing.T) {
11191157
},
11201158
Spec: velerov1api.BackupSpec{
11211159
StorageLocation: defaultBackupLocation.Name,
1160+
IncludedNamespaces: []string{"*"},
11221161
DefaultVolumesToFsBackup: boolptr.True(),
11231162
SnapshotMoveData: boolptr.False(),
11241163
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1161,6 +1200,7 @@ func TestProcessBackupCompletions(t *testing.T) {
11611200
},
11621201
Spec: velerov1api.BackupSpec{
11631202
StorageLocation: defaultBackupLocation.Name,
1203+
IncludedNamespaces: []string{"*"},
11641204
DefaultVolumesToFsBackup: boolptr.False(),
11651205
SnapshotMoveData: boolptr.True(),
11661206
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1204,6 +1244,7 @@ func TestProcessBackupCompletions(t *testing.T) {
12041244
},
12051245
Spec: velerov1api.BackupSpec{
12061246
StorageLocation: defaultBackupLocation.Name,
1247+
IncludedNamespaces: []string{"*"},
12071248
DefaultVolumesToFsBackup: boolptr.False(),
12081249
SnapshotMoveData: boolptr.False(),
12091250
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1247,6 +1288,7 @@ func TestProcessBackupCompletions(t *testing.T) {
12471288
},
12481289
Spec: velerov1api.BackupSpec{
12491290
StorageLocation: defaultBackupLocation.Name,
1291+
IncludedNamespaces: []string{"*"},
12501292
DefaultVolumesToFsBackup: boolptr.False(),
12511293
SnapshotMoveData: boolptr.False(),
12521294
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1290,6 +1332,7 @@ func TestProcessBackupCompletions(t *testing.T) {
12901332
},
12911333
Spec: velerov1api.BackupSpec{
12921334
StorageLocation: defaultBackupLocation.Name,
1335+
IncludedNamespaces: []string{"*"},
12931336
DefaultVolumesToFsBackup: boolptr.False(),
12941337
SnapshotMoveData: boolptr.True(),
12951338
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1334,6 +1377,7 @@ func TestProcessBackupCompletions(t *testing.T) {
13341377
},
13351378
Spec: velerov1api.BackupSpec{
13361379
StorageLocation: defaultBackupLocation.Name,
1380+
IncludedNamespaces: []string{"*"},
13371381
DefaultVolumesToFsBackup: boolptr.False(),
13381382
SnapshotMoveData: boolptr.False(),
13391383
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1377,6 +1421,7 @@ func TestProcessBackupCompletions(t *testing.T) {
13771421
},
13781422
Spec: velerov1api.BackupSpec{
13791423
StorageLocation: defaultBackupLocation.Name,
1424+
IncludedNamespaces: []string{"*"},
13801425
DefaultVolumesToFsBackup: boolptr.False(),
13811426
SnapshotMoveData: boolptr.True(),
13821427
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
@@ -1424,6 +1469,7 @@ func TestProcessBackupCompletions(t *testing.T) {
14241469
},
14251470
Spec: velerov1api.BackupSpec{
14261471
StorageLocation: defaultBackupLocation.Name,
1472+
IncludedNamespaces: []string{"*"},
14271473
DefaultVolumesToFsBackup: boolptr.False(),
14281474
SnapshotMoveData: boolptr.True(),
14291475
IncludedClusterScopedResources: []string{"storageclasses"},
@@ -1473,6 +1519,7 @@ func TestProcessBackupCompletions(t *testing.T) {
14731519
},
14741520
Spec: velerov1api.BackupSpec{
14751521
StorageLocation: defaultBackupLocation.Name,
1522+
IncludedNamespaces: []string{"*"},
14761523
DefaultVolumesToFsBackup: boolptr.False(),
14771524
SnapshotMoveData: boolptr.True(),
14781525
IncludedClusterScopedResources: []string{"storageclasses"},

‎pkg/util/wildcard/expand.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ import (
99
)
1010

1111
func ShouldExpandWildcards(includes []string, excludes []string) bool {
12+
// Empty includes is equivalent to * (match all) - don't expand
13+
if len(includes) == 0 {
14+
return false
15+
}
16+
1217
wildcardFound := false
1318
for _, include := range includes {
1419
// Special case: "*" alone means "match all" - don't expand

‎pkg/util/wildcard/expand_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,12 @@ func TestShouldExpandWildcards(t *testing.T) {
6868
excludes: []string{},
6969
expected: false,
7070
},
71+
{
72+
name: "empty includes with wildcard excludes - should not expand",
73+
includes: []string{},
74+
excludes: []string{"ns*"},
75+
expected: false,
76+
},
7177
{
7278
name: "complex wildcard patterns",
7379
includes: []string{"*-prod"},

0 commit comments

Comments
 (0)