From c17d6a0a049f721543dd61862ef7335ed05de9f5 Mon Sep 17 00:00:00 2001 From: Adam Zhang Date: Fri, 10 Apr 2026 15:40:03 +0800 Subject: [PATCH] Fix DataUpload list scope in CSI PVC backup plugin The `getDataUpload` function in the CSI PVC backup plugin was previously making a cluster-scoped list query to retrieve DataUpload CRs. In environments with strict minimum-privilege RBAC, this would fail with forbidden errors. This explicitly passes the backup namespace into the `ListOptions` when calling `crClient.List`, correctly scoping the queries to the backup's namespace. Unit tests have also been updated to ensure cross-namespace queries are rejected appropriately. Signed-off-by: Adam Zhang --- changelogs/unreleased/9708-adam-jian-zhang | 1 + pkg/backup/actions/csi/pvc_action.go | 6 +- pkg/backup/actions/csi/pvc_action_test.go | 75 ++++++++++++++++++---- 3 files changed, 69 insertions(+), 13 deletions(-) create mode 100644 changelogs/unreleased/9708-adam-jian-zhang diff --git a/changelogs/unreleased/9708-adam-jian-zhang b/changelogs/unreleased/9708-adam-jian-zhang new file mode 100644 index 000000000..59ae81e9a --- /dev/null +++ b/changelogs/unreleased/9708-adam-jian-zhang @@ -0,0 +1 @@ +Fix issue #9703, fix CSI PVC Backup Plugin list options to only list in installed namespace diff --git a/pkg/backup/actions/csi/pvc_action.go b/pkg/backup/actions/csi/pvc_action.go index dbd60892d..7f5fd2afa 100644 --- a/pkg/backup/actions/csi/pvc_action.go +++ b/pkg/backup/actions/csi/pvc_action.go @@ -467,7 +467,7 @@ func (p *pvcBackupItemAction) Progress( return progress, biav2.InvalidOperationIDError(operationID) } - dataUpload, err := getDataUpload(context.Background(), p.crClient, operationID) + dataUpload, err := getDataUpload(context.Background(), p.crClient, backup.Namespace, operationID) if err != nil { p.log.Errorf( "fail to get DataUpload for backup %s/%s by operation ID %s: %s", @@ -512,7 +512,7 @@ func (p *pvcBackupItemAction) Cancel(operationID string, backup *velerov1api.Bac return biav2.InvalidOperationIDError(operationID) } - dataUpload, err := getDataUpload(context.Background(), p.crClient, operationID) + dataUpload, err := getDataUpload(context.Background(), p.crClient, backup.Namespace, operationID) if err != nil { p.log.Errorf( "fail to get DataUpload for backup %s/%s: %s", @@ -605,10 +605,12 @@ func createDataUpload( func getDataUpload( ctx context.Context, crClient crclient.Client, + namespace string, operationID string, ) (*velerov2alpha1.DataUpload, error) { dataUploadList := new(velerov2alpha1.DataUploadList) err := crClient.List(ctx, dataUploadList, &crclient.ListOptions{ + Namespace: namespace, LabelSelector: labels.SelectorFromSet( map[string]string{velerov1api.AsyncOperationIDLabel: operationID}, ), diff --git a/pkg/backup/actions/csi/pvc_action_test.go b/pkg/backup/actions/csi/pvc_action_test.go index 33116d5c8..9ffe20be5 100644 --- a/pkg/backup/actions/csi/pvc_action_test.go +++ b/pkg/backup/actions/csi/pvc_action_test.go @@ -307,6 +307,28 @@ func TestProgress(t *testing.T) { operationID: "testing", expectedErr: "not found DataUpload for operationID testing", }, + { + name: "DataUpload in different namespace is not found", + backup: builder.ForBackup("velero", "test").Result(), + dataUpload: &velerov2alpha1.DataUpload{ + TypeMeta: metav1.TypeMeta{ + Kind: "DataUpload", + APIVersion: "v2alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "other-namespace", + Name: "testing", + Labels: map[string]string{ + velerov1api.AsyncOperationIDLabel: "testing", + }, + }, + Status: velerov2alpha1.DataUploadStatus{ + Phase: velerov2alpha1.DataUploadPhaseFailed, + }, + }, + operationID: "testing", + expectedErr: "not found DataUpload for operationID testing", + }, { name: "DataUpload is found", backup: builder.ForBackup("velero", "test").Result(), @@ -375,15 +397,15 @@ func TestCancel(t *testing.T) { tests := []struct { name string backup *velerov1api.Backup - dataUpload velerov2alpha1.DataUpload + dataUpload *velerov2alpha1.DataUpload operationID string - expectedErr error + expectedErr string expectedDataUpload velerov2alpha1.DataUpload }{ { name: "Cancel DataUpload", backup: builder.ForBackup("velero", "test").Result(), - dataUpload: velerov2alpha1.DataUpload{ + dataUpload: &velerov2alpha1.DataUpload{ TypeMeta: metav1.TypeMeta{ Kind: "DataUpload", APIVersion: velerov2alpha1.SchemeGroupVersion.String(), @@ -414,6 +436,31 @@ func TestCancel(t *testing.T) { }, }, }, + { + name: "DataUpload cannot be found", + backup: builder.ForBackup("velero", "test").Result(), + operationID: "testing", + expectedErr: "not found DataUpload for operationID testing", + }, + { + name: "DataUpload in different namespace is not found", + backup: builder.ForBackup("velero", "test").Result(), + dataUpload: &velerov2alpha1.DataUpload{ + TypeMeta: metav1.TypeMeta{ + Kind: "DataUpload", + APIVersion: velerov2alpha1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "other-namespace", + Name: "testing", + Labels: map[string]string{ + velerov1api.AsyncOperationIDLabel: "testing", + }, + }, + }, + operationID: "testing", + expectedErr: "not found DataUpload for operationID testing", + }, } for _, tc := range tests { @@ -426,17 +473,23 @@ func TestCancel(t *testing.T) { crClient: crClient, } - err := crClient.Create(t.Context(), &tc.dataUpload) - require.NoError(t, err) + if tc.dataUpload != nil { + err := crClient.Create(t.Context(), tc.dataUpload) + require.NoError(t, err) + } - err = pvcBIA.Cancel(tc.operationID, tc.backup) - require.NoError(t, err) + err := pvcBIA.Cancel(tc.operationID, tc.backup) + if tc.expectedErr != "" { + require.EqualError(t, err, tc.expectedErr) + } else { + require.NoError(t, err) - du := new(velerov2alpha1.DataUpload) - err = crClient.Get(t.Context(), crclient.ObjectKey{Namespace: tc.dataUpload.Namespace, Name: tc.dataUpload.Name}, du) - require.NoError(t, err) + du := new(velerov2alpha1.DataUpload) + err = crClient.Get(t.Context(), crclient.ObjectKey{Namespace: tc.dataUpload.Namespace, Name: tc.dataUpload.Name}, du) + require.NoError(t, err) - require.True(t, cmp.Equal(tc.expectedDataUpload, *du, cmpopts.IgnoreFields(velerov2alpha1.DataUpload{}, "ResourceVersion"))) + require.True(t, cmp.Equal(tc.expectedDataUpload, *du, cmpopts.IgnoreFields(velerov2alpha1.DataUpload{}, "ResourceVersion"))) + } }) } }