Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/unreleased/9684-Joeavaikath
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix wildcard expansion when includes is empty and excludes has wildcards
7 changes: 7 additions & 0 deletions pkg/controller/backup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Comment thread
Joeavaikath marked this conversation as resolved.
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))
Expand Down
47 changes: 47 additions & 0 deletions pkg/controller/backup_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,34 @@ func TestBackupLocationLabel(t *testing.T) {
}
}

func TestPrepareBackupRequest_EmptyIncludedNamespacesNormalizedToWildcard(t *testing.T) {
Comment thread
Joeavaikath marked this conversation as resolved.
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}
Expand Down Expand Up @@ -709,6 +737,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.True(),
SnapshotMoveData: boolptr.False(),
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
Expand Down Expand Up @@ -748,6 +777,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: "alt-loc",
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.False(),
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
Expand Down Expand Up @@ -791,6 +821,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: "read-write",
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.True(),
SnapshotMoveData: boolptr.False(),
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
Expand Down Expand Up @@ -831,6 +862,7 @@ 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,
Expand Down Expand Up @@ -871,6 +903,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.True(),
SnapshotMoveData: boolptr.False(),
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
Expand Down Expand Up @@ -912,6 +945,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.False(),
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
Expand Down Expand Up @@ -953,6 +987,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.True(),
SnapshotMoveData: boolptr.False(),
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
Expand Down Expand Up @@ -994,6 +1029,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.True(),
SnapshotMoveData: boolptr.False(),
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
Expand Down Expand Up @@ -1035,6 +1071,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.False(),
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
Expand Down Expand Up @@ -1077,6 +1114,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.True(),
SnapshotMoveData: boolptr.False(),
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
Expand Down Expand Up @@ -1119,6 +1157,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.True(),
SnapshotMoveData: boolptr.False(),
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
Expand Down Expand Up @@ -1161,6 +1200,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.True(),
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
Expand Down Expand Up @@ -1204,6 +1244,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.False(),
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
Expand Down Expand Up @@ -1247,6 +1288,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.False(),
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
Expand Down Expand Up @@ -1290,6 +1332,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.True(),
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
Expand Down Expand Up @@ -1334,6 +1377,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.False(),
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
Expand Down Expand Up @@ -1377,6 +1421,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.True(),
ExcludedClusterScopedResources: autoExcludeClusterScopedResources,
Expand Down Expand Up @@ -1424,6 +1469,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.True(),
IncludedClusterScopedResources: []string{"storageclasses"},
Expand Down Expand Up @@ -1473,6 +1519,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
IncludedNamespaces: []string{"*"},
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.True(),
IncludedClusterScopedResources: []string{"storageclasses"},
Expand Down
5 changes: 5 additions & 0 deletions pkg/util/wildcard/expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ import (
)

func ShouldExpandWildcards(includes []string, excludes []string) bool {
// Empty includes is equivalent to * (match all) - don't expand
if len(includes) == 0 {
return false
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix is reasonable for the main branch now, but release-1.18 expands asterisk.
We should consider that and make modifications accordingly for release-1.18.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will backport to release-1.18 branch

}

wildcardFound := false
for _, include := range includes {
// Special case: "*" alone means "match all" - don't expand
Expand Down
6 changes: 6 additions & 0 deletions pkg/util/wildcard/expand_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ func TestShouldExpandWildcards(t *testing.T) {
excludes: []string{},
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"},
Expand Down
Loading