From 5568ccf130a39dd254d8f03227d8910d0e46b0a4 Mon Sep 17 00:00:00 2001 From: Achin Mishra Date: Tue, 14 Apr 2026 12:36:18 -0700 Subject: [PATCH] Auto-exclude Velero namespace from backups instead of hard error Velero does not support backing up its own namespace. Previously this would fail during execution with no clear signal at request time. This change auto-excludes the Velero namespace during backup preparation: - When included via wildcard or empty includes (all namespaces): adds the Velero namespace to ExcludedNamespaces with a warning log. - When explicitly listed alongside other namespaces: removes it from IncludedNamespaces, adds to ExcludedNamespaces, and logs a warning. - When it is the only included namespace: returns a validation error to prevent a silent empty backup. Signed-off-by: Achin Mishra --- changelogs/unreleased/9615-achinmishra | 1 + pkg/controller/backup_controller.go | 29 ++++++ pkg/controller/backup_controller_test.go | 120 +++++++++++++++++++++++ 3 files changed, 150 insertions(+) create mode 100644 changelogs/unreleased/9615-achinmishra diff --git a/changelogs/unreleased/9615-achinmishra b/changelogs/unreleased/9615-achinmishra new file mode 100644 index 0000000000..0ec5271eca --- /dev/null +++ b/changelogs/unreleased/9615-achinmishra @@ -0,0 +1 @@ +Auto-exclude the Velero namespace from backups with a warning instead of failing validation diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go index 496308a6e0..7451012783 100644 --- a/pkg/controller/backup_controller.go +++ b/pkg/controller/backup_controller.go @@ -575,6 +575,35 @@ func (b *backupReconciler) prepareBackupRequest(ctx context.Context, backup *vel request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("Invalid included/excluded namespace lists: %v", err)) } + // Auto-exclude the Velero namespace from the backup. + // Velero does not support backing up its own namespace. + veleroNs := request.Backup.Namespace + nsFilter := collections.NewIncludesExcludes(). + Includes(request.Spec.IncludedNamespaces...). + Excludes(request.Spec.ExcludedNamespaces...) + if nsFilter.ShouldInclude(veleroNs) { + // Remove the Velero namespace from IncludedNamespaces if explicitly listed, + // to avoid a contradictory state where it appears in both includes and excludes. + filtered := make([]string, 0, len(request.Spec.IncludedNamespaces)) + for _, ns := range request.Spec.IncludedNamespaces { + if ns != veleroNs { + filtered = append(filtered, ns) + } + } + hadExplicitIncludes := len(request.Spec.IncludedNamespaces) > 0 + request.Spec.IncludedNamespaces = filtered + + // If the Velero namespace was the only included namespace, the backup would + // be empty. Return a validation error instead of silently proceeding. + if hadExplicitIncludes && len(request.Spec.IncludedNamespaces) == 0 { + request.Status.ValidationErrors = append(request.Status.ValidationErrors, + fmt.Sprintf("Velero namespace %q is the only included namespace, but Velero does not support backing up its own namespace", veleroNs)) + } else { + logger.Warnf("Velero namespace %q is being excluded from this backup because Velero does not support backing up its own namespace.", veleroNs) + request.Spec.ExcludedNamespaces = append(request.Spec.ExcludedNamespaces, veleroNs) + } + } + // validate that only one exists orLabelSelector or just labelSelector (singular) if request.Spec.OrLabelSelectors != nil && request.Spec.LabelSelector != nil { request.Status.ValidationErrors = append(request.Status.ValidationErrors, "encountered labelSelector as well as orLabelSelectors in backup spec, only one can be specified") diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index 3864989002..bade2b28eb 100644 --- a/pkg/controller/backup_controller_test.go +++ b/pkg/controller/backup_controller_test.go @@ -270,6 +270,107 @@ func TestProcessBackupValidationFailures(t *testing.T) { } } +func TestProcessBackupVeleroNamespaceValidation(t *testing.T) { + defaultBackupLocation := builder.ForBackupStorageLocation("velero", "loc-1").Phase(velerov1api.BackupStorageLocationPhaseAvailable).Result() + + tests := []struct { + name string + backup *velerov1api.Backup + expectedIncludedNamespaces []string + expectedExcludedNamespaces []string + expectValidationError bool + }{ + { + name: "velero namespace explicitly as only IncludedNamespace returns validation error", + backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-1").Phase(velerov1api.BackupPhaseReadyToStart).IncludedNamespaces("velero").Result(), + expectedExcludedNamespaces: nil, + expectValidationError: true, + }, + { + name: "all namespaces (empty includes) auto-excludes velero namespace", + backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-1").Phase(velerov1api.BackupPhaseReadyToStart).Result(), + expectedExcludedNamespaces: []string{"velero"}, + }, + { + name: "velero namespace already excluded stays excluded", + backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-1").Phase(velerov1api.BackupPhaseReadyToStart).ExcludedNamespaces("velero").Result(), + expectedExcludedNamespaces: []string{"velero"}, + }, + { + name: "glob pattern matching velero namespace auto-excludes it", + backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-1").Phase(velerov1api.BackupPhaseReadyToStart).IncludedNamespaces("vel*").Result(), + expectedExcludedNamespaces: []string{"velero"}, + }, + { + name: "glob pattern in excludes matching velero namespace keeps it excluded", + backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-1").Phase(velerov1api.BackupPhaseReadyToStart).ExcludedNamespaces("vel*").Result(), + expectedExcludedNamespaces: []string{"vel*"}, + }, + { + name: "glob pattern not matching velero namespace does not auto-exclude", + backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-1").Phase(velerov1api.BackupPhaseReadyToStart).IncludedNamespaces("prod-*").Result(), + expectedExcludedNamespaces: nil, + }, + { + name: "single char wildcard matching velero namespace auto-excludes it", + backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-1").Phase(velerov1api.BackupPhaseReadyToStart).IncludedNamespaces("veler?").Result(), + expectedExcludedNamespaces: []string{"velero"}, + }, + { + name: "character class wildcard matching velero namespace auto-excludes it", + backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-1").Phase(velerov1api.BackupPhaseReadyToStart).IncludedNamespaces("[vV]elero").Result(), + expectedExcludedNamespaces: []string{"velero"}, + expectValidationError: true, + }, + { + name: "velero plus other namespaces in includes removes velero and backs up the others", + backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-1").Phase(velerov1api.BackupPhaseReadyToStart).IncludedNamespaces("velero", "default", "kube-system").Result(), + expectedIncludedNamespaces: []string{"default", "kube-system"}, + expectedExcludedNamespaces: []string{"velero"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(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) + + fakeClient := velerotest.NewFakeControllerRuntimeClient(t, defaultBackupLocation) + + c := &backupReconciler{ + logger: logger, + discoveryHelper: discoveryHelper, + kbClient: fakeClient, + defaultBackupLocation: defaultBackupLocation.Name, + clock: &clock.RealClock{}, + formatFlag: formatFlag, + metrics: metrics.NewServerMetrics(), + backupTracker: NewBackupTracker(), + } + + require.NotNil(t, test.backup) + + res := c.prepareBackupRequest(ctx, test.backup, logger) + defer res.WorkerPool.Stop() + require.NotNil(t, res) + + if test.expectValidationError { + assert.NotEmpty(t, res.Status.ValidationErrors) + } else { + assert.Empty(t, res.Status.ValidationErrors) + } + assert.Equal(t, test.expectedExcludedNamespaces, res.Spec.ExcludedNamespaces) + if test.expectedIncludedNamespaces != nil { + assert.Equal(t, test.expectedIncludedNamespaces, res.Spec.IncludedNamespaces) + } + }) + } +} + func TestBackupLocationLabel(t *testing.T) { tests := []struct { name string @@ -711,6 +812,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -750,6 +852,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: "alt-loc", DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -793,6 +896,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: "read-write", DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -833,6 +937,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -873,6 +978,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -914,6 +1020,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -955,6 +1062,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -996,6 +1104,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -1037,6 +1146,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -1079,6 +1189,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -1121,6 +1232,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.True(), SnapshotMoveData: boolptr.False(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -1163,6 +1275,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.True(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -1206,6 +1319,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -1249,6 +1363,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -1292,6 +1407,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.True(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -1336,6 +1452,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.False(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -1379,6 +1496,7 @@ func TestProcessBackupCompletions(t *testing.T) { StorageLocation: defaultBackupLocation.Name, DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.True(), + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, }, @@ -1427,6 +1545,7 @@ func TestProcessBackupCompletions(t *testing.T) { DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.True(), IncludedClusterScopedResources: []string{"storageclasses"}, + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: append([]string{"clusterroles"}, autoExcludeClusterScopedResources...), IncludedNamespaceScopedResources: []string{"pods"}, ExcludedNamespaceScopedResources: append([]string{"secrets"}, autoExcludeNamespaceScopedResources...), @@ -1476,6 +1595,7 @@ func TestProcessBackupCompletions(t *testing.T) { DefaultVolumesToFsBackup: boolptr.False(), SnapshotMoveData: boolptr.True(), IncludedClusterScopedResources: []string{"storageclasses"}, + ExcludedNamespaces: []string{velerov1api.DefaultNamespace}, ExcludedClusterScopedResources: append([]string{"clusterroles"}, autoExcludeClusterScopedResources...), IncludedNamespaceScopedResources: []string{"pods"}, ExcludedNamespaceScopedResources: append([]string{"secrets"}, autoExcludeNamespaceScopedResources...),