From ea3abc3107b6a3f1d698078d9fecc95e115ad662 Mon Sep 17 00:00:00 2001 From: Pranjal Date: Thu, 27 Aug 2026 11:15:51 +0530 Subject: [PATCH] Reject velero backup delete when BSL is read-only (#10353) The CLI now checks backup storage location access mode before submitting a delete request. Also reject backups with no storage location set and cache BSL lookups when deleting multiple backups. Fixes #4203 Signed-off-by: PranjalManhgaye --- changelogs/unreleased/10353-PranjalManhgaye | 1 + pkg/cmd/cli/backup/delete.go | 22 +++++++++++++++ pkg/cmd/cli/backup/delete_test.go | 31 +++++++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 changelogs/unreleased/10353-PranjalManhgaye diff --git a/changelogs/unreleased/10353-PranjalManhgaye b/changelogs/unreleased/10353-PranjalManhgaye new file mode 100644 index 000000000..46e5d5eed --- /dev/null +++ b/changelogs/unreleased/10353-PranjalManhgaye @@ -0,0 +1 @@ +Reject velero backup delete when backup storage location is read-only diff --git a/pkg/cmd/cli/backup/delete.go b/pkg/cmd/cli/backup/delete.go index ba5a4954b..f76e4fcf2 100644 --- a/pkg/cmd/cli/backup/delete.go +++ b/pkg/cmd/cli/backup/delete.go @@ -121,7 +121,29 @@ func Run(o *cli.DeleteOptions) error { } // create a backup deletion request for each + bslCache := make(map[string]*velerov1api.BackupStorageLocation) for _, b := range backups { + storageLocationName := b.Spec.StorageLocation + if storageLocationName == "" { + errs = append(errs, errors.Errorf("cannot delete backup %q because it does not have a backup storage location set", b.Name)) + continue + } + + location, ok := bslCache[storageLocationName] + if !ok { + location = &velerov1api.BackupStorageLocation{} + if err := o.Client.Get(context.TODO(), controllerclient.ObjectKey{Namespace: o.Namespace, Name: storageLocationName}, location); err != nil { + errs = append(errs, errors.Wrapf(err, "error getting backup storage location %q for backup %q", storageLocationName, b.Name)) + continue + } + bslCache[storageLocationName] = location + } + + if location.Spec.AccessMode == velerov1api.BackupStorageLocationAccessModeReadOnly { + errs = append(errs, errors.Errorf("cannot delete backup %q because backup storage location %q is currently in read-only mode", b.Name, location.Name)) + continue + } + deleteRequest := builder.ForDeleteBackupRequest(o.Namespace, "").BackupName(b.Name). ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, label.GetValidName(b.Name), velerov1api.BackupUIDLabel, string(b.UID)), builder.WithGenerateName(b.Name+"-")).Result() diff --git a/pkg/cmd/cli/backup/delete_test.go b/pkg/cmd/cli/backup/delete_test.go index 3278e8153..1e745e265 100644 --- a/pkg/cmd/cli/backup/delete_test.go +++ b/pkg/cmd/cli/backup/delete_test.go @@ -26,6 +26,7 @@ import ( "github.com/stretchr/testify/require" controllerclient "sigs.k8s.io/controller-runtime/pkg/client" + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/builder" factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" "github.com/vmware-tanzu/velero/pkg/cmd/cli" @@ -82,3 +83,33 @@ func TestDeleteCommand(t *testing.T) { require.Contains(t, stdout, fmt.Sprintf("backups.velero.io \"%s\" not found.", backup2)) } } + +func TestDeleteCommandReadOnlyBSL(t *testing.T) { + const ( + backupName = "backup-readonly" + bslName = "readonly-bsl" + ) + + client := velerotest.NewFakeControllerRuntimeClient(t) + require.NoError(t, client.Create(t.Context(), builder.ForBackupStorageLocation(cmdtest.VeleroNameSpace, bslName). + AccessMode(velerov1api.BackupStorageLocationAccessModeReadOnly). + Phase(velerov1api.BackupStorageLocationPhaseAvailable). + Result(), &controllerclient.CreateOptions{})) + require.NoError(t, client.Create(t.Context(), builder.ForBackup(cmdtest.VeleroNameSpace, backupName). + StorageLocation(bslName). + Result(), &controllerclient.CreateOptions{})) + + o := cli.NewDeleteOptions("backup") + o.Client = client + o.Namespace = cmdtest.VeleroNameSpace + o.Confirm = true + o.Names = []string{backupName} + + err := Run(o) + require.Error(t, err) + require.Contains(t, err.Error(), fmt.Sprintf("cannot delete backup %q because backup storage location %q is currently in read-only mode", backupName, bslName)) + + deleteRequestList := new(velerov1api.DeleteBackupRequestList) + require.NoError(t, client.List(t.Context(), deleteRequestList, &controllerclient.ListOptions{Namespace: cmdtest.VeleroNameSpace})) + require.Empty(t, deleteRequestList.Items) +}