Merge remote-tracking branch 'origin/main' into bugfix/10755-cluster-restore-finalizer

Author: leon-ape

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"`
 

apis/workloads/v1/instance_types.go

@@ -153,7 +153,7 @@ type InstanceStatus2 struct {
 	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 
 	// Represents the latest available observations of an instance's current state.
-	// Known .status.conditions.type are: "InstanceFailure", "InstanceReady", "InstanceAvailable"
+	// Known .status.conditions.type are: "InstanceFailure", "InstanceReady", "InstanceAvailable", "Restore"
 	//
 	// +optional
 	Conditions []metav1.Condition `json:"conditions,omitempty"`

apis/workloads/v1/instanceset_types.go

@@ -738,7 +738,7 @@ const (
 	// InstanceFailure is added in an instance set when at least one of its instances(pods) is in a `Failed` phase.
 	InstanceFailure ConditionType = "InstanceFailure"
 
-	// InstanceRestore indicates whether the initial data restore for this InstanceSet has completed.
+	// InstanceRestore indicates whether the initial data restore for this Instance or InstanceSet has completed.
 	InstanceRestore ConditionType = "Restore"
 
 	// InstanceUpdateRestricted represents a ConditionType that indicates updates to an InstanceSet are blocked(when the

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: |-

config/crd/bases/workloads.kubeblocks.io_instances.yaml

@@ -10267,7 +10267,7 @@ spec:
               conditions:
                 description: |-
                   Represents the latest available observations of an instance's current state.
-                  Known .status.conditions.type are: "InstanceFailure", "InstanceReady", "InstanceAvailable"
+                  Known .status.conditions.type are: "InstanceFailure", "InstanceReady", "InstanceAvailable", "Restore"
                 items:
                   description: Condition contains details for one aspect of the current
                     state of this API Resource.

controllers/dataprotection/volumepopulator_controller.go

@@ -1000,13 +1000,13 @@ func (r *VolumePopulatorReconciler) validateRestoreAndBuildMGR(reqCtx intctrluti
 			}
 		}
 	}
-	if err = r.restoreSystemAccountSecrets(reqCtx, pvc, backupNamespace); err != nil {
-		return nil, err
-	}
 	restoreMgr := dprestore.NewRestoreManager(restore, r.Recorder, r.Scheme, r.Client)
 	if err = dprestore.ValidateAndInitRestoreMGR(reqCtx, r.Client, restoreMgr); err != nil {
 		return nil, err
 	}
+	if err = r.restoreSystemAccountSecrets(reqCtx, pvc, backupNamespace); err != nil {
+		return nil, err
+	}
 	if decision.mode == pvcRestoreModeProvisionOnly {
 		restoreMgr.PrepareDataBackupSets = nil
 	}

controllers/dataprotection/volumepopulator_controller_test.go

@@ -774,6 +774,21 @@ var _ = Describe("Volume Populator Controller test", func() {
 					g.Expect(restoreCondition).ShouldNot(BeNil())
 					g.Expect(restoreCondition.Status).Should(Equal(corev1.ConditionUnknown))
 				})).Should(Succeed())
+
+				populateKey := types.NamespacedName{
+					Namespace: testCtx.DefaultNamespace,
+					Name:      getPopulatePVCName(pvc.UID),
+				}
+				Eventually(testapps.CheckObjExists(&testCtx, populateKey,
+					&corev1.PersistentVolumeClaim{}, true)).Should(Succeed())
+				Eventually(testapps.CheckObjExists(&testCtx, populateKey,
+					&dpv1alpha1.Restore{}, true)).Should(Succeed())
+
+				By("clean resources created by the case")
+				cleanEnv()
+				testapps.DeleteObject(&testCtx, secretKey, &corev1.Secret{})
+				Eventually(testapps.CheckObjExists(&testCtx, secretKey,
+					&corev1.Secret{}, false)).Should(Succeed())
 			})
 
 			It("test VolumePopulator when it fails", func() {
@@ -2735,6 +2750,151 @@ func TestRestoreSystemAccountSecretsUsesShardingSecretName(t *testing.T) {
 		systemAccountSecretName(systemAccountSecretScopeComponent, "cluster", "mysql", "admin"))
 }
 
+func TestValidateRestoreBeforeRestoringSystemAccountSecrets(t *testing.T) {
+	scheme := runtime.NewScheme()
+	require.NoError(t, corev1.AddToScheme(scheme))
+	require.NoError(t, kbappsv1.AddToScheme(scheme))
+	require.NoError(t, dpv1alpha1.AddToScheme(scheme))
+	require.NoError(t, workloadsv1.AddToScheme(scheme))
+
+	const (
+		sourceNamespace = "source"
+		targetNamespace = "target"
+		clusterName     = "cluster"
+		componentName   = "mysql"
+		accountName     = "admin"
+	)
+	actionSet := &dpv1alpha1.ActionSet{
+		ObjectMeta: metav1.ObjectMeta{Name: "snapshot-action"},
+		Spec: dpv1alpha1.ActionSetSpec{
+			BackupType: dpv1alpha1.BackupTypeFull,
+			Restore: &dpv1alpha1.RestoreActionSpec{
+				PrepareData: &dpv1alpha1.JobActionSpec{},
+			},
+		},
+	}
+	encryptor := intctrlutil.NewEncryptor(viper.GetString(constant.CfgKeyDPEncryptionKey))
+	encryptedPassword, err := encryptor.Encrypt([]byte("restored-password"))
+	require.NoError(t, err)
+	accounts, err := json.Marshal(map[string]map[string]string{
+		componentName: {accountName: encryptedPassword},
+	})
+	require.NoError(t, err)
+	backup := &dpv1alpha1.Backup{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: sourceNamespace,
+			Name:      "backup",
+			Annotations: map[string]string{
+				constant.EncryptedSystemAccountsAnnotationKey: string(accounts),
+			},
+		},
+		Status: dpv1alpha1.BackupStatus{
+			Phase: dpv1alpha1.BackupPhaseCompleted,
+			BackupMethod: &dpv1alpha1.BackupMethod{
+				Name:            "snapshot",
+				ActionSetName:   actionSet.Name,
+				SnapshotVolumes: ptr.To(true),
+				TargetVolumes:   &dpv1alpha1.TargetVolumeInfo{Volumes: []string{"data"}},
+			},
+		},
+	}
+	cluster := &kbappsv1.Cluster{
+		ObjectMeta: metav1.ObjectMeta{Namespace: targetNamespace, Name: clusterName, UID: types.UID("cluster-uid")},
+		Spec: kbappsv1.ClusterSpec{
+			Restore: &kbappsv1.ClusterRestore{
+				Source: kbappsv1.ClusterRestoreSource{
+					APIGroup:  dptypes.DataprotectionAPIGroup,
+					Kind:      dptypes.BackupKind,
+					Name:      backup.Name,
+					Namespace: backup.Namespace,
+				},
+			},
+		},
+	}
+	component := &kbappsv1.Component{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: targetNamespace,
+			Name:      constant.GenerateClusterComponentName(clusterName, componentName),
+			UID:       types.UID("component-uid"),
+		},
+	}
+	instanceSet := &workloadsv1.InstanceSet{
+		TypeMeta: metav1.TypeMeta{APIVersion: workloadsv1.GroupVersion.String(), Kind: workloadsv1.InstanceSetKind},
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: targetNamespace,
+			Name:      "cluster-mysql",
+			UID:       types.UID("instanceset-uid"),
+			Labels: map[string]string{
+				constant.AppInstanceLabelKey:    clusterName,
+				constant.KBAppComponentLabelKey: componentName,
+			},
+		},
+		Spec: workloadsv1.InstanceSetSpec{
+			Replicas: ptr.To[int32](1),
+			VolumeClaimTemplates: []corev1.PersistentVolumeClaim{{
+				ObjectMeta: metav1.ObjectMeta{Name: "data"},
+			}},
+		},
+	}
+	apiGroup := dptypes.DataprotectionAPIGroup
+	pvc := &corev1.PersistentVolumeClaim{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: targetNamespace,
+			Name:      "data-cluster-mysql-0",
+			Labels: map[string]string{
+				constant.AppInstanceLabelKey:             clusterName,
+				constant.KBAppComponentLabelKey:          componentName,
+				constant.KBAppPodNameLabelKey:            "cluster-mysql-0",
+				constant.VolumeClaimTemplateNameLabelKey: "data",
+			},
+			Annotations: map[string]string{
+				constant.RestoreSourceNamespaceAnnotationKey: sourceNamespace,
+				constant.RestoreVolumeTemplateAnnotationKey:  "data",
+			},
+			OwnerReferences: []metav1.OwnerReference{{
+				APIVersion: workloadsv1.GroupVersion.String(),
+				Kind:       workloadsv1.InstanceSetKind,
+				Name:       instanceSet.Name,
+				UID:        instanceSet.UID,
+				Controller: ptr.To(true),
+			}},
+		},
+		Spec: corev1.PersistentVolumeClaimSpec{
+			DataSourceRef: &corev1.TypedObjectReference{
+				APIGroup: &apiGroup,
+				Kind:     dptypes.BackupKind,
+				Name:     backup.Name,
+			},
+		},
+	}
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: targetNamespace,
+			Name:      constant.GenerateAccountSecretName(clusterName, componentName, accountName),
+		},
+		Data: map[string][]byte{
+			constant.AccountNameForSecret:   []byte(accountName),
+			constant.AccountPasswdForSecret: []byte("existing-password"),
+		},
+	}
+	reconciler := &VolumePopulatorReconciler{
+		Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(
+			actionSet, backup, cluster, component, instanceSet, secret,
+		).Build(),
+		Scheme: scheme,
+	}
+
+	_, err = reconciler.validateRestoreAndBuildMGR(intctrlutil.RequestCtx{Ctx: context.Background()}, pvc)
+	require.ErrorContains(t, err, "cross-namespace VolumeSnapshot")
+	require.True(t, intctrlutil.IsTargetError(err, intctrlutil.ErrorTypeFatal))
+
+	got := &corev1.Secret{}
+	require.NoError(t, reconciler.Client.Get(context.Background(), client.ObjectKeyFromObject(secret), got))
+	require.Equal(t, []byte("existing-password"), got.Data[constant.AccountPasswdForSecret])
+	require.NotContains(t, got.Annotations, constant.SystemAccountProvisionedAnnotationKey)
+	require.Empty(t, got.OwnerReferences)
+}
+
 func TestRestoreSystemAccountSecretsRestoresComponentAndShardingSecrets(t *testing.T) {
 	scheme := runtime.NewScheme()
 	require.NoError(t, corev1.AddToScheme(scheme))

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