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..fd697c5ac6 100644 --- a/pkg/controller/backup_controller.go +++ b/pkg/controller/backup_controller.go @@ -575,6 +575,45 @@ 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) + // Only append if not already present (e.g. from the exclude-from-backup label logic). + alreadyExcluded := false + for _, ns := range request.Spec.ExcludedNamespaces { + if ns == veleroNs { + alreadyExcluded = true + break + } + } + if !alreadyExcluded { + 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..26e1e1dd99 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...),