fix: dataprotection uses secret create by system account by default (…

…#1804)

apis/dataprotection/v1alpha1/backuppolicy_types.go

@@ -80,9 +80,11 @@ type TargetCluster struct {
 	// +kubebuilder:pruning:PreserveUnknownFields
 	LabelsSelector *metav1.LabelSelector `json:"labelsSelector"`
 
-	// target db cluster access secret
-	// +kubebuilder:validation:Required
-	Secret BackupPolicySecret `json:"secret"`
+	// Secret is used to connect to the target database cluster.
+	// If not set, secret will be inherited from backup policy template.
+	// if still not set, the controller will check if any system account for dataprotection has been created.
+	// +optional
+	Secret *BackupPolicySecret `json:"secret,omitempty"`
 }
 
 // BackupPolicySecret defined for the target database secret that backup tool can connect.

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

@@ -1684,7 +1684,10 @@ spec:
                     type: object
                     x-kubernetes-preserve-unknown-fields: true
                   secret:
-                    description: target db cluster access secret
+                    description: Secret is used to connect to the target database
+                      cluster. If not set, secret will be inherited from backup policy
+                      template. if still not set, the controller will check if any
+                      system account for dataprotection has been created.
                     properties:
                       name:
                         description: the secret name
@@ -1703,7 +1706,6 @@ spec:
                     type: object
                 required:
                 - labelsSelector
-                - secret
                 type: object
               ttl:
                 description: TTL is a time.Duration-parseable string describing how

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

@@ -106,7 +106,10 @@ spec:
                     type: object
                     x-kubernetes-preserve-unknown-fields: true
                   secret:
-                    description: target db cluster access secret
+                    description: Secret is used to connect to the target database
+                      cluster. If not set, secret will be inherited from backup policy
+                      template. if still not set, the controller will check if any
+                      system account for dataprotection has been created.
                     properties:
                       name:
                         description: the secret name
@@ -125,7 +128,6 @@ spec:
                     type: object
                 required:
                 - labelsSelector
-                - secret
                 type: object
               targetVolumeMounts:
                 description: array of restore volume mounts .

controllers/apps/const.go

@@ -31,7 +31,6 @@ const (
 	clusterVersionLabelKey     = "clusterversion.kubeblocks.io/name"
 	statefulSetPodNameLabelKey = "statefulset.kubernetes.io/pod-name"
 	csRoleChangedAnnotKey      = "cs.kubeblocks.io/event-handled"
-	clusterAccountLabelKey     = "account.kubeblocks.io/name"
 
 	// annotations keys
 	lifecycleAnnotationKey = "cluster.kubeblocks.io/lifecycle"

controllers/apps/systemaccount_controller.go

@@ -387,7 +387,7 @@ func (r *jobCompletitionPredicate) Delete(e event.DeleteEvent) bool {
 	}
 
 	ml := job.ObjectMeta.Labels
-	accountName, ok := ml[clusterAccountLabelKey]
+	accountName, ok := ml[constant.ClusterAccountLabelKey]
 	if !ok {
 		return false
 	}

controllers/apps/systemaccount_controller_test.go

@@ -31,6 +31,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1"
+	"github.com/apecloud/kubeblocks/internal/constant"
 	"github.com/apecloud/kubeblocks/internal/controller/component"
 	intctrlutil "github.com/apecloud/kubeblocks/internal/generics"
 	testapps "github.com/apecloud/kubeblocks/internal/testutil/apps"
@@ -354,7 +355,7 @@ var _ = Describe("SystemAccount Controller", func() {
 					jobs := &batchv1.JobList{}
 					g.Expect(k8sClient.List(ctx, jobs, client.InNamespace(cluster.Namespace), ml)).To(Succeed())
 					for _, job := range jobs.Items {
-						_, ok := job.Labels[clusterAccountLabelKey]
+						_, ok := job.Labels[constant.ClusterAccountLabelKey]
 						g.Expect(ok).To(BeTrue())
 						g.Expect(len(job.ObjectMeta.OwnerReferences)).To(BeEquivalentTo(1))
 						g.Expect(checkOwnerReferenceToObj(job.OwnerReferences[0], cluster)).To(BeTrue())

controllers/apps/systemaccount_util.go

@@ -217,13 +217,13 @@ func renderSecretWithPwd(key componentUniqueKey, username, passwd string) *corev
 	secretData[accountPasswdForSecret] = []byte(passwd)
 
 	ml := getLabelsForSecretsAndJobs(key)
-	ml[clusterAccountLabelKey] = username
+	ml[constant.ClusterAccountLabelKey] = username
 	return renderSecret(key, username, ml, secretData)
 }
 
 func renderSecretByCopy(key componentUniqueKey, username string, fromSecret *corev1.Secret) *corev1.Secret {
 	ml := getLabelsForSecretsAndJobs(key)
-	ml[clusterAccountLabelKey] = username
+	ml[constant.ClusterAccountLabelKey] = username
 	return renderSecret(key, username, ml, fromSecret.Data)
 }
 
@@ -266,13 +266,13 @@ func getAccountFacts(secrets *corev1.SecretList, jobs *batchv1.JobList) (detecte
 	detectedFacts = appsv1alpha1.KBAccountInvalid
 	// parse account name from secret's label
 	for _, secret := range secrets.Items {
-		if accountName, exists := secret.ObjectMeta.Labels[clusterAccountLabelKey]; exists {
+		if accountName, exists := secret.ObjectMeta.Labels[constant.ClusterAccountLabelKey]; exists {
 			updateFacts(appsv1alpha1.AccountName(accountName), &detectedFacts)
 		}
 	}
 	// parse account name from job's label
 	for _, job := range jobs.Items {
-		if accountName, exists := job.ObjectMeta.Labels[clusterAccountLabelKey]; exists {
+		if accountName, exists := job.ObjectMeta.Labels[constant.ClusterAccountLabelKey]; exists {
 			updateFacts(appsv1alpha1.AccountName(accountName), &detectedFacts)
 		}
 	}
@@ -355,7 +355,7 @@ func calibrateJobMetaAndSpec(job *batchv1.Job, cluster *appsv1alpha1.Cluster, co
 	debugModeOn := getDebugMode(cluster.Annotations[debugClusterAnnotationKey])
 	// add label
 	ml := getLabelsForSecretsAndJobs(compKey)
-	ml[clusterAccountLabelKey] = (string)(account)
+	ml[constant.ClusterAccountLabelKey] = (string)(account)
 	job.ObjectMeta.Labels = ml
 
 	// if debug mode is on, jobs will retain after execution.

controllers/dataprotection/backuppolicy_controller.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"embed"
 	"encoding/json"
+	"fmt"
 	"sort"
 	"time"
 
@@ -37,6 +38,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
+	appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1"
 	dataprotectionv1alpha1 "github.com/apecloud/kubeblocks/apis/dataprotection/v1alpha1"
 	"github.com/apecloud/kubeblocks/internal/constant"
 	intctrlutil "github.com/apecloud/kubeblocks/internal/controllerutil"
@@ -156,10 +158,19 @@ func (r *BackupPolicyReconciler) doInProgressPhaseAction(
 		backupPolicy.Labels[k] = v
 	}
 
+	if backupPolicy.Spec.Target.Secret == nil {
+		backupPolicy.Spec.Target.Secret = &dataprotectionv1alpha1.BackupPolicySecret{}
+	}
+
 	// merge backup policy template spec
 	if err := r.mergeBackupPolicyTemplate(reqCtx, backupPolicy); err != nil {
 		return intctrlutil.CheckedRequeueWithError(err, reqCtx.Log, "")
 	}
+
+	if err := r.fillSecretName(reqCtx, backupPolicy, true); err != nil {
+		return intctrlutil.CheckedRequeueWithError(err, reqCtx.Log, "")
+	}
+	// fill remaining fields
 	r.fillDefaultValueIfRequired(backupPolicy)
 
 	if err := r.Client.Patch(reqCtx.Ctx, backupPolicy, patch); err != nil {
@@ -225,6 +236,13 @@ func (r *BackupPolicyReconciler) mergeBackupPolicyTemplate(
 	if backupPolicy.Spec.BackupToolName == "" {
 		backupPolicy.Spec.BackupToolName = template.Spec.BackupToolName
 	}
+
+	// if template.Spec.CredentialKeyword is nil, use system account; else use root conn secret
+	useSysAcct := template.Spec.CredentialKeyword == nil
+	if err := r.fillSecretName(reqCtx, backupPolicy, useSysAcct); err != nil {
+		return err
+	}
+
 	if template.Spec.CredentialKeyword != nil {
 		if backupPolicy.Spec.Target.Secret.UserKeyword == "" {
 			backupPolicy.Spec.Target.Secret.UserKeyword = template.Spec.CredentialKeyword.UserKeyword
@@ -258,6 +276,43 @@ func (r *BackupPolicyReconciler) fillDefaultValueIfRequired(backupPolicy *datapr
 	}
 }
 
+// fillSecretName fills secret name if it is empty.
+// If BackupPolicy.Sect.Target.Secret is not nil, use secret specified in BackupPolicy.
+// Otherwise, lookup BackupPolicyTemplate and check if username and password are specified.
+// If so, use root connection secret; otherwise, try system account before root connection.
+func (r *BackupPolicyReconciler) fillSecretName(reqCtx intctrlutil.RequestCtx, backupPolicy *dataprotectionv1alpha1.BackupPolicy, useSysAccount bool) error {
+	if len(backupPolicy.Spec.Target.Secret.Name) > 0 {
+		return nil
+	}
+	// get cluster name from labels
+	instanceName := backupPolicy.Spec.Target.LabelsSelector.MatchLabels[constant.AppInstanceLabelKey]
+	if len(instanceName) == 0 {
+		return fmt.Errorf("failed to get instance name from labels: %v", backupPolicy.Spec.Target.LabelsSelector.MatchLabels)
+	}
+	var labels map[string]string
+	if useSysAccount {
+		labels = map[string]string{
+			constant.AppInstanceLabelKey:    instanceName,
+			constant.ClusterAccountLabelKey: (string)(appsv1alpha1.DataprotectionAccount),
+		}
+	} else {
+		labels = map[string]string{
+			constant.AppInstanceLabelKey:  instanceName,
+			constant.AppManagedByLabelKey: constant.AppName,
+		}
+	}
+
+	secrets := corev1.SecretList{}
+	if err := r.Client.List(reqCtx.Ctx, &secrets, client.MatchingLabels(labels)); err != nil {
+		return err
+	}
+	if len(secrets.Items) > 0 {
+		backupPolicy.Spec.Target.Secret.Name = secrets.Items[0].GetName()
+		return nil
+	}
+	return fmt.Errorf("no secret found for backup policy %s", backupPolicy.GetName())
+}
+
 func (r *BackupPolicyReconciler) buildCronJob(backupPolicy *dataprotectionv1alpha1.BackupPolicy) (*batchv1.CronJob, error) {
 	tplFile := "cronjob.cue"
 	cueFS, _ := debme.FS(cueTemplates, "cue")