Merge pull request velero-io#9708 from adam-jian-zhang/fix-csi-pvc-backup-plugin-scoping-1.18

Fix DataUpload list scope in CSI PVC backup plugin

Author: blackpiglet

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

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},
 		),

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")))
+			}
 		})
 	}
 }