From dda779de6501412ecbf33249847f98d074a0543e Mon Sep 17 00:00:00 2001 From: Subhramit Basu Date: Tue, 9 Jun 2026 01:40:32 +0530 Subject: [PATCH] Reject restores from backups not in a completed or partially failed phase (#9792) * Add phase check validations in restore controller Signed-off-by: subhramit * Adapt existing tests Signed-off-by: subhramit * Add tests Signed-off-by: subhramit * Update doc Signed-off-by: subhramit * Add changelog Signed-off-by: Subhramit Basu * Update pkg/controller/restore_controller_test.go Signed-off-by: Subhramit Basu --------- Signed-off-by: subhramit Signed-off-by: Subhramit Basu --- changelogs/unreleased/9792-subhramit | 1 + pkg/controller/restore_controller.go | 11 +++++ pkg/controller/restore_controller_test.go | 52 ++++++++++++++++++--- site/content/docs/main/restore-reference.md | 2 +- 4 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 changelogs/unreleased/9792-subhramit diff --git a/changelogs/unreleased/9792-subhramit b/changelogs/unreleased/9792-subhramit new file mode 100644 index 000000000..8868ed627 --- /dev/null +++ b/changelogs/unreleased/9792-subhramit @@ -0,0 +1 @@ +Restores from backups not in a completed or partially failed phase are now rejected. diff --git a/pkg/controller/restore_controller.go b/pkg/controller/restore_controller.go index 469951f27..285b08281 100644 --- a/pkg/controller/restore_controller.go +++ b/pkg/controller/restore_controller.go @@ -399,6 +399,17 @@ func (r *restoreReconciler) validateAndComplete(restore *api.Restore) (backupInf return backupInfo{}, nil } + // reject restores from backups that are not in a usable phase + switch info.backup.Status.Phase { + case api.BackupPhaseCompleted, api.BackupPhasePartiallyFailed: + // ok + default: + restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, + fmt.Sprintf("backup %q is in phase %q and cannot be used as a restore source", + info.backup.Name, info.backup.Status.Phase)) + return backupInfo{}, nil + } + // Fill in the ScheduleName so it's easier to consume for metrics. if restore.Spec.ScheduleName == "" { restore.Spec.ScheduleName = info.backup.GetLabels()[api.ScheduleNameLabel] diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go index b013ee64d..6f03a6074 100644 --- a/pkg/controller/restore_controller_test.go +++ b/pkg/controller/restore_controller_test.go @@ -305,7 +305,7 @@ func TestRestoreReconcile(t *testing.T) { name: "restorer throwing an error causes the restore to fail", location: defaultStorageLocation, restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).Result(), - backup: defaultBackup().StorageLocation("default").Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseCompleted).Result(), restorerError: errors.New("blarg"), expectedErr: false, expectedPhase: string(velerov1api.RestorePhaseInProgress), @@ -319,7 +319,7 @@ func TestRestoreReconcile(t *testing.T) { name: "valid restore with none existingresourcepolicy gets executed", location: defaultStorageLocation, restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).ExistingResourcePolicy("none").Result(), - backup: defaultBackup().StorageLocation("default").Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseCompleted).Result(), expectedErr: false, expectedPhase: string(velerov1api.RestorePhaseInProgress), expectedStartTime: ×tamp, @@ -330,7 +330,7 @@ func TestRestoreReconcile(t *testing.T) { name: "valid restore with update existingresourcepolicy gets executed", location: defaultStorageLocation, restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).ExistingResourcePolicy("update").Result(), - backup: defaultBackup().StorageLocation("default").Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseCompleted).Result(), expectedErr: false, expectedPhase: string(velerov1api.RestorePhaseInProgress), expectedStartTime: ×tamp, @@ -352,7 +352,7 @@ func TestRestoreReconcile(t *testing.T) { name: "valid restore gets executed", location: defaultStorageLocation, restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).Result(), - backup: defaultBackup().StorageLocation("default").Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseCompleted).Result(), expectedErr: false, expectedPhase: string(velerov1api.RestorePhaseInProgress), expectedStartTime: ×tamp, @@ -363,7 +363,7 @@ func TestRestoreReconcile(t *testing.T) { name: "valid restore gets executed and only includes pod volume backups from restore namespace", location: defaultStorageLocation, restore: NewRestore("foo", "bar2", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).Result(), - backup: defaultBackup().StorageLocation("default").Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseCompleted).Result(), podVolumeBackups: []*velerov1api.PodVolumeBackup{ builder.ForPodVolumeBackup("foo", "pvb-1").ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, "backup-1")).Result(), builder.ForPodVolumeBackup("other-ns", "pvb-2").ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, "backup-1")).Result(), @@ -444,7 +444,7 @@ func TestRestoreReconcile(t *testing.T) { expectedStartTime: ×tamp, expectedCompletedTime: ×tamp, backupStoreGetBackupContentsErr: errors.New("Couldn't download backup"), - backup: defaultBackup().StorageLocation("default").Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseCompleted).Result(), }, { name: "restore attached with an expected finalizer gets cleaned up successfully", @@ -473,7 +473,7 @@ func TestRestoreReconcile(t *testing.T) { name: "valid restore with empty VolumeInfos", location: defaultStorageLocation, restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).Result(), - backup: defaultBackup().StorageLocation("default").Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseCompleted).Result(), emptyVolumeInfo: true, expectedErr: false, expectedPhase: string(velerov1api.RestorePhaseInProgress), @@ -497,6 +497,44 @@ func TestRestoreReconcile(t *testing.T) { backup: defaultBackup().StorageLocation("default").Result(), expectedErr: true, }, + { + name: "restore from backup in Deleting phase fails validation", + location: defaultStorageLocation, + restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseDeleting).Result(), + expectedErr: false, + expectedPhase: string(velerov1api.RestorePhaseFailedValidation), + expectedValidationErrors: []string{`backup "backup-1" is in phase "Deleting" and cannot be used as a restore source`}, + }, + { + name: "restore from backup in InProgress phase fails validation", + location: defaultStorageLocation, + restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseInProgress).Result(), + expectedErr: false, + expectedPhase: string(velerov1api.RestorePhaseFailedValidation), + expectedValidationErrors: []string{`backup "backup-1" is in phase "InProgress" and cannot be used as a restore source`}, + }, + { + name: "restore from backup in PartiallyFailed phase succeeds", + location: defaultStorageLocation, + restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhasePartiallyFailed).Result(), + expectedErr: false, + expectedPhase: string(velerov1api.RestorePhaseInProgress), + expectedStartTime: ×tamp, + expectedCompletedTime: ×tamp, + expectedRestorerCall: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseInProgress).Result(), + }, + { + name: "restore from backup in Failed phase fails validation", + location: defaultStorageLocation, + restore: NewRestore("foo", "bar", "backup-1", "ns-1", "", velerov1api.RestorePhaseNew).Result(), + backup: defaultBackup().StorageLocation("default").Phase(velerov1api.BackupPhaseFailed).Result(), + expectedErr: false, + expectedPhase: string(velerov1api.RestorePhaseFailedValidation), + expectedValidationErrors: []string{`backup "backup-1" is in phase "Failed" and cannot be used as a restore source`}, + }, } formatFlag := logging.FormatText diff --git a/site/content/docs/main/restore-reference.md b/site/content/docs/main/restore-reference.md index b14a1dac9..cf9d34b51 100644 --- a/site/content/docs/main/restore-reference.md +++ b/site/content/docs/main/restore-reference.md @@ -27,7 +27,7 @@ The following is an overview of Velero's restore process that starts after you r 1. The Velero client makes a call to the Kubernetes API server to create a [`Restore`](api-types/restore.md) object. -1. The `RestoreController` notices the new Restore object and performs validation. +1. The `RestoreController` notices the new Restore object and performs validation. This includes verifying that the referenced backup is in a usable phase. Only backups in `Completed` or `PartiallyFailed` phase are accepted as restore sources. 1. The `RestoreController` fetches basic information about the backup being restored, like the [BackupStorageLocation](locations.md) (BSL). It also fetches a tarball of the cluster resources in the backup, any volumes that will be restored using File System Backup, and any volume snapshots to be restored.