fix(dataprotection): preserve backup namespace for cross-namespace restores

apis/dataprotection/v1alpha1/restore_types.go

@@ -105,6 +105,7 @@ type BackupRef struct {
 	Name string `json:"name"`
 
 	// Specifies the backup namespace.
+	// Cross-namespace VolumeSnapshot restores are NOT supported.
 	//
 	// +kubebuilder:validation:Required
 	Namespace string `json:"namespace"`

apis/operations/v1alpha1/opsrequest_types.go

@@ -513,8 +513,9 @@ type FromBackup struct {
 	// +kubebuilder:validation:Required
 	Name string `json:"name"`
 
-	// Specifies the namespace of the Backup namespace.
+	// Specifies the namespace of the Backup.
 	// If not specified, the namespace of the OpsRequest will be used.
+	// Cross-namespace VolumeSnapshot restores are NOT supported.
 	// +optional
 	Namespace string `json:"namespace,omitempty"`
 

config/crd/bases/dataprotection.kubeblocks.io_restores.yaml

@@ -83,7 +83,9 @@ spec:
                     description: Specifies the backup name.
                     type: string
                   namespace:
-                    description: Specifies the backup namespace.
+                    description: |-
+                      Specifies the backup namespace.
+                      Cross-namespace VolumeSnapshot restores are NOT supported.
                     type: string
                   sourceTargetName:
                     description: Specifies the source target for restoration, identified

config/crd/bases/operations.kubeblocks.io_opsrequests.yaml

@@ -619,8 +619,9 @@ spec:
                               type: string
                             namespace:
                               description: |-
-                                Specifies the namespace of the Backup namespace.
+                                Specifies the namespace of the Backup.
                                 If not specified, the namespace of the OpsRequest will be used.
+                                Cross-namespace VolumeSnapshot restores are NOT supported.
                               type: string
                             restoreEnv:
                               description: |-

deploy/helm/crds/dataprotection.kubeblocks.io_restores.yaml

@@ -83,7 +83,9 @@ spec:
                     description: Specifies the backup name.
                     type: string
                   namespace:
-                    description: Specifies the backup namespace.
+                    description: |-
+                      Specifies the backup namespace.
+                      Cross-namespace VolumeSnapshot restores are NOT supported.
                     type: string
                   sourceTargetName:
                     description: Specifies the source target for restoration, identified

deploy/helm/crds/operations.kubeblocks.io_opsrequests.yaml

@@ -619,8 +619,9 @@ spec:
                               type: string
                             namespace:
                               description: |-
-                                Specifies the namespace of the Backup namespace.
+                                Specifies the namespace of the Backup.
                                 If not specified, the namespace of the OpsRequest will be used.
+                                Cross-namespace VolumeSnapshot restores are NOT supported.
                               type: string
                             restoreEnv:
                               description: |-

docs/developer_docs/api-reference/dataprotection.md

@@ -2858,7 +2858,8 @@ string
 </em>
 </td>
 <td>
-<p>Specifies the backup namespace.</p>
+<p>Specifies the backup namespace.
+Cross-namespace VolumeSnapshot restores are NOT supported.</p>
 </td>
 </tr>
 <tr>

docs/developer_docs/api-reference/operations.md

@@ -1089,8 +1089,9 @@ string
 </td>
 <td>
 <em>(Optional)</em>
-<p>Specifies the namespace of the Backup namespace.
-If not specified, the namespace of the OpsRequest will be used.</p>
+<p>Specifies the namespace of the Backup.
+If not specified, the namespace of the OpsRequest will be used.
+Cross-namespace VolumeSnapshot restores are NOT supported.</p>
 </td>
 </tr>
 <tr>

pkg/controller/plan/restore.go

@@ -150,7 +150,7 @@ func (r *RestoreManager) resolvePrepareDataBackup(backupObj *dpv1alpha1.Backup)
 		Spec: dpv1alpha1.RestoreSpec{
 			Backup: dpv1alpha1.BackupRef{
 				Name:      backupObj.Name,
-				Namespace: r.namespace,
+				Namespace: backupObj.Namespace,
 			},
 			RestoreTime: r.RestoreTime,
 		},
@@ -160,7 +160,7 @@ func (r *RestoreManager) resolvePrepareDataBackup(backupObj *dpv1alpha1.Backup)
 		Recorder: record.NewFakeRecorder(16),
 	}
 	restoreMGR := dprestore.NewRestoreManager(restore, reqCtx.Recorder, r.Scheme, r.Client)
-	backupSet, err := restoreMGR.GetBackupActionSetByNamespaced(reqCtx, r.Client, backupObj.Name, r.namespace)
+	backupSet, err := restoreMGR.GetBackupActionSetByNamespaced(reqCtx, r.Client, backupObj.Name, backupObj.Namespace)
 	if err != nil {
 		return nil, err
 	}
@@ -291,7 +291,7 @@ func (r *RestoreManager) buildPrepareDataRestore(comp *component.SynthesizedComp
 		Spec: dpv1alpha1.RestoreSpec{
 			Backup: dpv1alpha1.BackupRef{
 				Name:             backupObj.Name,
-				Namespace:        r.namespace,
+				Namespace:        backupObj.Namespace,
 				SourceTargetName: sourceTargetName,
 			},
 			RestoreTime: r.RestoreTime,
@@ -353,7 +353,7 @@ func (r *RestoreManager) DoPostReady(comp *component.SynthesizedComponent,
 		Spec: dpv1alpha1.RestoreSpec{
 			Backup: dpv1alpha1.BackupRef{
 				Name:             backupObj.Name,
-				Namespace:        r.namespace,
+				Namespace:        backupObj.Namespace,
 				SourceTargetName: sourceTargetName,
 			},
 			RestoreTime: r.RestoreTime,

pkg/controller/plan/restore_test.go

@@ -20,8 +20,10 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 package plan
 
 import (
+	"context"
 	"fmt"
 	"strings"
+	"testing"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -44,6 +46,32 @@ import (
 	testdp "github.com/apecloud/kubeblocks/pkg/testutil/dataprotection"
 )
 
+func TestBuildPrepareDataRestorePreservesBackupNamespace(t *testing.T) {
+	manager := NewRestoreManager(context.Background(), nil, &appsv1.Cluster{
+		ObjectMeta: metav1.ObjectMeta{Name: "target", Namespace: "target", UID: "12345678"},
+	}, nil, nil, 1, 0)
+	comp := &component.SynthesizedComponent{
+		Name: "mysql",
+		VolumeClaimTemplates: []corev1.PersistentVolumeClaimTemplate{{
+			ObjectMeta: metav1.ObjectMeta{Name: "data"},
+		}},
+	}
+	backup := &dpv1alpha1.Backup{
+		ObjectMeta: metav1.ObjectMeta{Name: "backup", Namespace: "source"},
+		Status: dpv1alpha1.BackupStatus{BackupMethod: &dpv1alpha1.BackupMethod{
+			TargetVolumes: &dpv1alpha1.TargetVolumeInfo{Volumes: []string{"data"}},
+		}},
+	}
+
+	restore, err := manager.BuildPrepareDataRestore(comp, backup, nil)
+	if err != nil {
+		t.Fatalf("BuildPrepareDataRestore() error = %v", err)
+	}
+	if restore.Spec.Backup.Namespace != backup.Namespace {
+		t.Fatalf("backup namespace = %q, want %q", restore.Spec.Backup.Namespace, backup.Namespace)
+	}
+}
+
 var _ = Describe("Restore", func() {
 	const backupName = "test-backup-job"
 	const sourceCluster = "source-cluster"

pkg/dataprotection/restore/manager.go

@@ -99,7 +99,7 @@ func (r *RestoreManager) GetBackupActionSetByNamespaced(reqCtx intctrlutil.Reque
 	if backupMethod == nil {
 		return nil, intctrlutil.NewFatalError(fmt.Sprintf(`status.backupMethod of backup "%s" is empty`, backupName))
 	}
-	useVolumeSnapshot := backupMethod.SnapshotVolumes != nil && *backupMethod.SnapshotVolumes
+	useVolumeSnapshot := boolptr.IsSetToTrue(backupMethod.SnapshotVolumes)
 	actionSet, err := utils.GetActionSetByName(reqCtx, cli, backup.Status.BackupMethod.ActionSetName)
 	if err != nil {
 		return nil, err
@@ -256,15 +256,19 @@ func (r *RestoreManager) getBaseBackupActionSetForContinuous(reqCtx intctrlutil.
 		return notFoundLatestBackup()
 	}
 	// 3. get the action set
-	var actionSetName string
-	if latestBackup.Status.BackupMethod != nil {
-		actionSetName = latestBackup.Status.BackupMethod.ActionSetName
+	backupMethod := latestBackup.Status.BackupMethod
+	if backupMethod == nil {
+		return nil, intctrlutil.NewFatalError(fmt.Sprintf(`status.backupMethod of backup "%s" is empty`, latestBackup.Name))
 	}
-	actionSet, err := utils.GetActionSetByName(reqCtx, cli, actionSetName)
+	actionSet, err := utils.GetActionSetByName(reqCtx, cli, backupMethod.ActionSetName)
 	if err != nil {
 		return nil, err
 	}
-	return &BackupActionSet{Backup: latestBackup, ActionSet: actionSet}, nil
+	return &BackupActionSet{
+		Backup:            latestBackup,
+		ActionSet:         actionSet,
+		UseVolumeSnapshot: boolptr.IsSetToTrue(backupMethod.SnapshotVolumes),
+	}, nil
 }
 
 func (r *RestoreManager) listCompletedBackups(reqCtx intctrlutil.RequestCtx, cli client.Client, continuousBackup *dpv1alpha1.Backup, backupType dpv1alpha1.BackupType) ([]dpv1alpha1.Backup, error) {

pkg/dataprotection/restore/manager_test.go

@@ -500,6 +500,26 @@ var _ = Describe("RestoreManager Test", func() {
 				return continuousBackup, restore
 			}
 
+			It("returns a fatal error when the base Backup has no status.backupMethod", func() {
+				baseBackup := mockBackupForRestore(
+					&testCtx, actionSet.Name, testdp.BackupPVCName, true, false, dpv1alpha1.BackupTypeFull,
+					"", "2023-01-01T10:00:00Z", "",
+				)
+				Expect(testapps.ChangeObjStatus(&testCtx, baseBackup, func() {
+					baseBackup.Status.BackupMethod = nil
+				})).Should(Succeed())
+
+				continuousBackup, restore := createContinuousBackupAndRestore("2023-01-01T11:30:00Z")
+				reqCtx := getReqCtx()
+				restoreMGR := NewRestoreManager(restore, recorder, k8sClient.Scheme(), k8sClient)
+				backupSet, err := restoreMGR.GetBackupActionSetByNamespaced(reqCtx, k8sClient, continuousBackup.Name, testCtx.DefaultNamespace)
+				Expect(err).ShouldNot(HaveOccurred())
+
+				err = restoreMGR.BuildContinuousRestoreManager(reqCtx, k8sClient, *backupSet)
+				Expect(err).Should(MatchError(ContainSubstring("status.backupMethod")))
+				Expect(intctrlutil.IsTargetError(err, intctrlutil.ErrorTypeFatal)).Should(BeTrue())
+			})
+
 			It("respects UnifyFullAndContinuousRestore annotation", func() {
 				By("create a completed backup")
 				_ = mockBackupForRestore(&testCtx, actionSet.Name, testdp.BackupPVCName, true, false, dpv1alpha1.BackupTypeFull, "", "2023-01-01T10:00:00Z", "")

pkg/dataprotection/restore/utils.go

@@ -272,7 +272,17 @@ func ValidateAndInitRestoreMGR(reqCtx intctrlutil.RequestCtx,
 	default:
 		err = intctrlutil.NewFatalError(fmt.Sprintf("backup type of %s is empty", backupName))
 	}
-	return err
+	if err != nil {
+		return err
+	}
+	for i := range restoreMgr.PrepareDataBackupSets {
+		prepareDataBackupSet := &restoreMgr.PrepareDataBackupSets[i]
+		if prepareDataBackupSet.UseVolumeSnapshot && prepareDataBackupSet.Backup.Namespace != restoreMgr.Restore.Namespace {
+			return intctrlutil.NewFatalError(fmt.Sprintf("cross-namespace VolumeSnapshot restore from Backup %s/%s is not supported",
+				prepareDataBackupSet.Backup.Namespace, prepareDataBackupSet.Backup.Name))
+		}
+	}
+	return nil
 }
 
 func cutJobName(jobName string) string {

pkg/dataprotection/restore/utils_test.go

@@ -8,7 +8,7 @@ it under the terms of the GNU Affero General Public License as published by
 the Free Software Foundation, either version 3 of the License, or
 (at your option) any later version.
 
-This program is distributed in the hope that it will be useful
+This program is distributed in the hope that it will be useful,
 but WITHOUT ANY WARRANTY; without even the implied warranty of
 MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 GNU Affero General Public License for more details.
@@ -20,15 +20,134 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 package restore
 
 import (
+	"context"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	dpv1alpha1 "github.com/apecloud/kubeblocks/apis/dataprotection/v1alpha1"
 	"github.com/apecloud/kubeblocks/pkg/constant"
+	intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil"
+	dptypes "github.com/apecloud/kubeblocks/pkg/dataprotection/types"
 )
 
+func TestValidateAndInitRestoreMGRCrossNamespaceBackup(t *testing.T) {
+	scheme := runtime.NewScheme()
+	assert.NoError(t, dpv1alpha1.AddToScheme(scheme))
+
+	actionSet := &dpv1alpha1.ActionSet{
+		ObjectMeta: metav1.ObjectMeta{Name: "full-action"},
+		Spec: dpv1alpha1.ActionSetSpec{
+			BackupType: dpv1alpha1.BackupTypeFull,
+			Restore:    &dpv1alpha1.RestoreActionSpec{PrepareData: &dpv1alpha1.JobActionSpec{}},
+		},
+	}
+	backup := &dpv1alpha1.Backup{
+		ObjectMeta: metav1.ObjectMeta{Name: "backup", Namespace: "source"},
+		Status: dpv1alpha1.BackupStatus{
+			Phase:        dpv1alpha1.BackupPhaseCompleted,
+			BackupMethod: &dpv1alpha1.BackupMethod{Name: "full", ActionSetName: actionSet.Name},
+		},
+	}
+	restoreObj := &dpv1alpha1.Restore{
+		ObjectMeta: metav1.ObjectMeta{Name: "restore", Namespace: "target"},
+		Spec: dpv1alpha1.RestoreSpec{
+			Backup: dpv1alpha1.BackupRef{Name: backup.Name, Namespace: backup.Namespace},
+		},
+	}
+	reqCtx := intctrlutil.RequestCtx{
+		Ctx: context.Background(),
+		Req: ctrl.Request{NamespacedName: client.ObjectKeyFromObject(restoreObj)},
+	}
+
+	t.Run("allows the Backup reference by default", func(t *testing.T) {
+		cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(actionSet, backup).Build()
+		mgr := &RestoreManager{Restore: restoreObj.DeepCopy()}
+		assert.NoError(t, ValidateAndInitRestoreMGR(reqCtx, cli, mgr))
+		assert.Len(t, mgr.PrepareDataBackupSets, 1)
+	})
+
+	t.Run("rejects a cross-namespace VolumeSnapshot restore", func(t *testing.T) {
+		snapshotBackup := backup.DeepCopy()
+		snapshotBackup.Status.BackupMethod.SnapshotVolumes = new(bool)
+		*snapshotBackup.Status.BackupMethod.SnapshotVolumes = true
+		cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(actionSet, snapshotBackup).Build()
+		err := ValidateAndInitRestoreMGR(reqCtx, cli, &RestoreManager{Restore: restoreObj.DeepCopy()})
+		assert.ErrorContains(t, err, "VolumeSnapshot")
+		assert.True(t, intctrlutil.IsTargetError(err, intctrlutil.ErrorTypeFatal))
+	})
+
+	t.Run("rejects a cross-namespace VolumeSnapshot base Backup selected for PITR", func(t *testing.T) {
+		const backupPolicyName = "policy"
+		now := time.Now().UTC().Truncate(time.Second)
+		continuousStart := metav1.NewTime(now.Add(-2 * time.Hour))
+		baseBackupEnd := metav1.NewTime(now.Add(-time.Hour))
+		continuousEnd := metav1.NewTime(now)
+		snapshotVolumes := true
+
+		continuousActionSet := &dpv1alpha1.ActionSet{
+			ObjectMeta: metav1.ObjectMeta{Name: "continuous-action"},
+			Spec: dpv1alpha1.ActionSetSpec{
+				BackupType: dpv1alpha1.BackupTypeContinuous,
+				Restore:    &dpv1alpha1.RestoreActionSpec{},
+			},
+		}
+		baseBackup := &dpv1alpha1.Backup{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "base-snapshot",
+				Namespace: backup.Namespace,
+				Labels: map[string]string{
+					dptypes.BackupTypeLabelKey:   string(dpv1alpha1.BackupTypeFull),
+					dptypes.BackupPolicyLabelKey: backupPolicyName,
+				},
+			},
+			Spec: dpv1alpha1.BackupSpec{BackupPolicyName: backupPolicyName},
+			Status: dpv1alpha1.BackupStatus{
+				Phase: dpv1alpha1.BackupPhaseCompleted,
+				TimeRange: &dpv1alpha1.BackupTimeRange{
+					Start: &continuousStart,
+					End:   &baseBackupEnd,
+				},
+				BackupMethod: &dpv1alpha1.BackupMethod{
+					Name:            "snapshot",
+					ActionSetName:   actionSet.Name,
+					SnapshotVolumes: &snapshotVolumes,
+				},
+			},
+		}
+		continuousBackup := &dpv1alpha1.Backup{
+			ObjectMeta: metav1.ObjectMeta{Name: "continuous", Namespace: backup.Namespace},
+			Spec:       dpv1alpha1.BackupSpec{BackupPolicyName: backupPolicyName},
+			Status: dpv1alpha1.BackupStatus{
+				Phase: dpv1alpha1.BackupPhaseCompleted,
+				TimeRange: &dpv1alpha1.BackupTimeRange{
+					Start: &continuousStart,
+					End:   &continuousEnd,
+				},
+				BackupMethod: &dpv1alpha1.BackupMethod{
+					Name:          "continuous",
+					ActionSetName: continuousActionSet.Name,
+				},
+			},
+		}
+		pitrRestore := restoreObj.DeepCopy()
+		pitrRestore.Spec.Backup = dpv1alpha1.BackupRef{Name: continuousBackup.Name, Namespace: continuousBackup.Namespace}
+		pitrRestore.Spec.RestoreTime = now.Add(-30 * time.Minute).Format(time.RFC3339)
+		cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(actionSet, continuousActionSet, baseBackup, continuousBackup).Build()
+
+		err := ValidateAndInitRestoreMGR(reqCtx, cli, &RestoreManager{Restore: pitrRestore})
+		assert.ErrorContains(t, err, "VolumeSnapshot")
+		assert.ErrorContains(t, err, "source/base-snapshot")
+		assert.True(t, intctrlutil.IsTargetError(err, intctrlutil.ErrorTypeFatal))
+	})
+}
+
 func TestGetSourcePodNameForTargetPod(t *testing.T) {
 	target := &dpv1alpha1.BackupStatusTarget{
 		BackupTarget: dpv1alpha1.BackupTarget{