Skip to content

Auto-exclude Velero namespace from backups instead of hard error #9615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
achinmishra wants to merge 3 commits into
base: main
Choose a base branch
from
+160 −0
Open

Auto-exclude Velero namespace from backups instead of hard error #9615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5568ccf
Auto-exclude Velero namespace from backups instead of hard error
achinmishra Apr 14, 2026
9bcf1c5
Auto-exclude Velero namespace from backups instead of hard error
achinmishra Apr 14, 2026
5965bcf
Merge branch 'main' into achinmishra/velero-ns-backup-validation
achinmishra Apr 14, 2026
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/9615-achinmishra
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Auto-exclude the Velero namespace from backups with a warning instead of failing validation
39 changes: 39 additions & 0 deletions pkg/controller/backup_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Copy Markdown
Contributor

@Lyndon-Li Lyndon-Li Apr 21, 2026

Choose a reason for hiding this comment

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

I see two problems for making the code here:

  1. This code is called by every reconcile, it would be a performance bottleneck for large scale envs.
  2. At present, we have the filter things in the collection stage, even if we want to auto modify the filter result, we should do it in the same collection stage

nsFilter := collections.NewIncludesExcludes().
Comment thread
Lyndon-Li marked this conversation as resolved.
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
Copy link
Copy Markdown
Collaborator

@shubham-pampattiwar shubham-pampattiwar Apr 23, 2026

Choose a reason for hiding this comment

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

@achinmishra minor note: after #9684, empty IncludedNamespaces gets normalized to ["*"] before this code runs, so hadExplicitIncludes is always true. The logic still works correctly because the wildcard string stays in the filtered list, but the variable name is misleading. Consider renaming to something like hadIncludesBeforeFiltering, or adding a brief comment explaining the interaction with the normalization above

// 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")
Comment thread
achinmishra marked this conversation as resolved.
Expand Down
120 changes: 120 additions & 0 deletions pkg/controller/backup_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Comment thread
achinmishra marked this conversation as resolved.
backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-1").Phase(velerov1api.BackupPhaseReadyToStart).IncludedNamespaces("[vV]elero").Result(),
Copy link
Copy Markdown
Collaborator

@shubham-pampattiwar shubham-pampattiwar Apr 23, 2026

Choose a reason for hiding this comment

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

@achinmishra the character class test case ([vV]elero) sets expectValidationError: true, but that error comes from the existing namespace name validation, not from the auto-exclude logic. It might be clearer to either add a comment explaining why the validation error is expected, or split it into two separate test cases so each behavior is tested independently.

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"},
},
}
Copy link
Copy Markdown
Collaborator

@shubham-pampattiwar shubham-pampattiwar Apr 23, 2026

Choose a reason for hiding this comment

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

@achinmishra could you add a test case where Velero is installed in a custom namespace (e.g. builder.ForBackup("openshift-adp", "backup-1"))? This would verify the auto-exclude works correctly for non-default installations like OADP.


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
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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...),
Expand Down Expand Up @@ -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...),
Expand Down
Loading