From 579a2ce0dda0ec4125addd098767c9c12d3732d0 Mon Sep 17 00:00:00 2001 From: Yecheng Fu Date: Tue, 17 Mar 2020 14:08:43 +0800 Subject: [PATCH] Automated cherry pick of #1836: backup: add TLS to backup br (#1950) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Backup: support TLS for br component (#1836) * backup: add TLS to backup br * fix API docs Co-authored-by: 尹亮 <30903849+shuijing198799@users.noreply.github.com> --- cmd/backup-manager/app/backup/backup.go | 25 +++++-- cmd/backup-manager/app/constants/constants.go | 6 ++ cmd/backup-manager/app/restore/restore.go | 14 ++++ cmd/backup-manager/app/util/util.go | 14 +--- docs/api-references/docs.html | 33 +++------ manifests/crd.yaml | 69 ++++++++----------- .../pingcap/v1alpha1/openapi_generated.go | 31 ++++----- pkg/apis/pingcap/v1alpha1/types.go | 14 ++-- pkg/backup/backup/backup_manager.go | 17 ++++- .../backupschedule/backup_schedule_manager.go | 9 ++- pkg/backup/constants/constants.go | 6 ++ pkg/backup/restore/restore_manager.go | 17 ++++- pkg/backup/util/util.go | 8 +-- 13 files changed, 150 insertions(+), 113 deletions(-) diff --git a/cmd/backup-manager/app/backup/backup.go b/cmd/backup-manager/app/backup/backup.go index 18cc163a97..5e9ab4210d 100644 --- a/cmd/backup-manager/app/backup/backup.go +++ b/cmd/backup-manager/app/backup/backup.go @@ -18,12 +18,14 @@ import ( "fmt" "io" "os/exec" + "path" "github.com/gogo/protobuf/proto" kvbackup "github.com/pingcap/kvproto/pkg/backup" "github.com/pingcap/tidb-operator/cmd/backup-manager/app/constants" "github.com/pingcap/tidb-operator/cmd/backup-manager/app/util" "github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1" + corev1 "k8s.io/api/core/v1" "k8s.io/klog" ) @@ -33,10 +35,21 @@ type Options struct { } func (bo *Options) backupData(backup *v1alpha1.Backup) (string, error) { - args, path, err := constructOptions(backup) + clusterNamespace := backup.Spec.BR.ClusterNamespace + if backup.Spec.BR.ClusterNamespace == "" { + clusterNamespace = backup.Namespace + } + args, remotePath, err := constructOptions(backup) if err != nil { return "", err } + args = append(args, fmt.Sprintf("--pd=%s-pd.%s:2379", backup.Spec.BR.Cluster, clusterNamespace)) + if backup.Spec.BR.EnableTLSClient { + args = append(args, fmt.Sprintf("--ca=%s", constants.ServiceAccountCAPath)) + args = append(args, fmt.Sprintf("--cert=%s", path.Join(constants.BRCertPath, corev1.TLSCertKey))) + args = append(args, fmt.Sprintf("--key=%s", path.Join(constants.BRCertPath, corev1.TLSPrivateKeyKey))) + } + var btype string if backup.Spec.Type == "" { btype = string(v1alpha1.BackupTypeFull) @@ -51,10 +64,10 @@ func (bo *Options) backupData(backup *v1alpha1.Backup) (string, error) { klog.Infof("Running br command with args: %v", fullArgs) output, err := exec.Command("br", fullArgs...).CombinedOutput() if err != nil { - return path, fmt.Errorf("cluster %s, execute br command %v failed, output: %s, err: %v", bo, fullArgs, string(output), err) + return remotePath, fmt.Errorf("cluster %s, execute br command %v failed, output: %s, err: %v", bo, fullArgs, string(output), err) } klog.Infof("Backup data for cluster %s successfully, output: %s", bo, string(output)) - return path, nil + return remotePath, nil } // getCommitTs get backup position from `EndVersion` in BR backup meta @@ -88,9 +101,9 @@ func getCommitTs(backup *v1alpha1.Backup) (uint64, error) { // constructOptions constructs options for BR and also return the remote path func constructOptions(backup *v1alpha1.Backup) ([]string, string, error) { - args, path, err := util.ConstructBRGlobalOptionsForBackup(backup) + args, remotePath, err := util.ConstructBRGlobalOptionsForBackup(backup) if err != nil { - return args, path, err + return args, remotePath, err } config := backup.Spec.BR if config.Concurrency != nil { @@ -105,7 +118,7 @@ func constructOptions(backup *v1alpha1.Backup) ([]string, string, error) { if config.Checksum != nil { args = append(args, fmt.Sprintf("--checksum=%t", *config.Checksum)) } - return args, path, nil + return args, remotePath, nil } // getBackupSize get the backup data size from remote diff --git a/cmd/backup-manager/app/constants/constants.go b/cmd/backup-manager/app/constants/constants.go index 24b5ad7d30..c471181138 100644 --- a/cmd/backup-manager/app/constants/constants.go +++ b/cmd/backup-manager/app/constants/constants.go @@ -56,4 +56,10 @@ const ( // MetaFile is the file name for meta data of backup with BR MetaFile = "backupmeta" + + // BR certificate storage path + BRCertPath = "/var/lib/br-tls" + + // ServiceAccountCAPath is where is CABundle of serviceaccount locates + ServiceAccountCAPath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" ) diff --git a/cmd/backup-manager/app/restore/restore.go b/cmd/backup-manager/app/restore/restore.go index 347bebcb43..90d0667ee0 100644 --- a/cmd/backup-manager/app/restore/restore.go +++ b/cmd/backup-manager/app/restore/restore.go @@ -16,9 +16,12 @@ package restore import ( "fmt" "os/exec" + "path" + "github.com/pingcap/tidb-operator/cmd/backup-manager/app/constants" "github.com/pingcap/tidb-operator/cmd/backup-manager/app/util" "github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1" + corev1 "k8s.io/api/core/v1" "k8s.io/klog" ) @@ -27,10 +30,21 @@ type Options struct { } func (ro *Options) restoreData(restore *v1alpha1.Restore) error { + clusterNamespace := restore.Spec.BR.ClusterNamespace + if restore.Spec.BR.ClusterNamespace == "" { + clusterNamespace = restore.Namespace + } args, err := constructBROptions(restore) if err != nil { return err } + args = append(args, fmt.Sprintf("--pd=%s-pd.%s:2379", restore.Spec.BR.Cluster, clusterNamespace)) + if restore.Spec.BR.EnableTLSClient { + args = append(args, fmt.Sprintf("--ca=%s", constants.ServiceAccountCAPath)) + args = append(args, fmt.Sprintf("--cert=%s", path.Join(constants.BRCertPath, corev1.TLSCertKey))) + args = append(args, fmt.Sprintf("--key=%s", path.Join(constants.BRCertPath, corev1.TLSPrivateKeyKey))) + } + var restoreType string if restore.Spec.Type == "" { restoreType = string(v1alpha1.BackupTypeFull) diff --git a/cmd/backup-manager/app/util/util.go b/cmd/backup-manager/app/util/util.go index 093b09e8b3..ca5723c1d4 100644 --- a/cmd/backup-manager/app/util/util.go +++ b/cmd/backup-manager/app/util/util.go @@ -112,7 +112,7 @@ func ConstructBRGlobalOptionsForBackup(backup *v1alpha1.Backup) ([]string, strin return nil, "", fmt.Errorf("no config for br in backup %s/%s", backup.Namespace, backup.Name) } args = append(args, constructBRGlobalOptions(config)...) - storageArgs, path, err := getRemoteStorage(backup.Spec.StorageProvider) + storageArgs, remotePath, err := getRemoteStorage(backup.Spec.StorageProvider) if err != nil { return nil, "", err } @@ -123,7 +123,7 @@ func ConstructBRGlobalOptionsForBackup(backup *v1alpha1.Backup) ([]string, strin if backup.Spec.Type == v1alpha1.BackupTypeTable && config.Table != "" { args = append(args, fmt.Sprintf("--table=%s", config.Table)) } - return args, path, nil + return args, remotePath, nil } // ConstructBRGlobalOptionsForRestore constructs BR global options for restore. @@ -151,16 +151,6 @@ func ConstructBRGlobalOptionsForRestore(restore *v1alpha1.Restore) ([]string, er // constructBRGlobalOptions constructs BR basic global options. func constructBRGlobalOptions(config *v1alpha1.BRConfig) []string { var args []string - args = append(args, fmt.Sprintf("--pd=%s", config.PDAddress)) - if config.CA != "" { - args = append(args, fmt.Sprintf("--ca=%s", config.CA)) - } - if config.Cert != "" { - args = append(args, fmt.Sprintf("--cert=%s", config.Cert)) - } - if config.Key != "" { - args = append(args, fmt.Sprintf("--key=%s", config.Key)) - } if config.LogLevel != "" { args = append(args, fmt.Sprintf("--log-level=%s", config.LogLevel)) } diff --git a/docs/api-references/docs.html b/docs/api-references/docs.html index cd98e9637d..fba2417167 100644 --- a/docs/api-references/docs.html +++ b/docs/api-references/docs.html @@ -1502,68 +1502,57 @@

BRConfig -pd
- -string - - - -

PDAddress is the PD address of the tidb cluster

- - - - -db
+enableTLSClient
-string +bool -

DB is the specific DB which will be backed-up or restored

+

Whether enable TLS in TiDBCluster

-table
+cluster
string -

Table is the specific table which will be backed-up or restored

+

ClusterName of backup/restore cluster

-ca
+clusterNamespace
string -

CA is the CA certificate path for TLS connection

+

Namespace of backup/restore cluster

-cert
+db
string -

Cert is the certificate path for TLS connection

+

DB is the specific DB which will be backed-up or restored

-key
+table
string -

Key is the private key path for TLS connection

+

Table is the specific table which will be backed-up or restored

diff --git a/manifests/crd.yaml b/manifests/crd.yaml index ed4d7cc79d..fdbd93e8bc 100644 --- a/manifests/crd.yaml +++ b/manifests/crd.yaml @@ -6746,15 +6746,15 @@ spec: br: description: BRConfig contains config for BR properties: - ca: - description: CA is the CA certificate path for TLS connection - type: string - cert: - description: Cert is the certificate path for TLS connection - type: string checksum: description: Checksum specifies whether to run checksum after backup type: boolean + cluster: + description: ClusterName of backup/restore cluster + type: string + clusterNamespace: + description: Namespace of backup/restore cluster + type: string concurrency: description: Concurrency is the size of thread pool on each node that execute the backup task @@ -6763,18 +6763,15 @@ spec: db: description: DB is the specific DB which will be backed-up or restored type: string - key: - description: Key is the private key path for TLS connection - type: string + enableTLSClient: + description: Whether enable TLS in TiDBCluster + type: boolean logLevel: description: LogLevel is the log level type: string onLine: description: OnLine specifies whether online during restore type: boolean - pd: - description: PDAddress is the PD address of the tidb cluster - type: string rateLimit: description: RateLimit is the rate limit of the backup task, MB/s per node @@ -6797,7 +6794,7 @@ spec: e.g. 1m, 1h type: string required: - - pd + - cluster type: object from: description: TiDBAccessConfig defines the configuration for access tidb @@ -7588,15 +7585,15 @@ spec: br: description: BRConfig contains config for BR properties: - ca: - description: CA is the CA certificate path for TLS connection - type: string - cert: - description: Cert is the certificate path for TLS connection - type: string checksum: description: Checksum specifies whether to run checksum after backup type: boolean + cluster: + description: ClusterName of backup/restore cluster + type: string + clusterNamespace: + description: Namespace of backup/restore cluster + type: string concurrency: description: Concurrency is the size of thread pool on each node that execute the backup task @@ -7605,18 +7602,15 @@ spec: db: description: DB is the specific DB which will be backed-up or restored type: string - key: - description: Key is the private key path for TLS connection - type: string + enableTLSClient: + description: Whether enable TLS in TiDBCluster + type: boolean logLevel: description: LogLevel is the log level type: string onLine: description: OnLine specifies whether online during restore type: boolean - pd: - description: PDAddress is the PD address of the tidb cluster - type: string rateLimit: description: RateLimit is the rate limit of the backup task, MB/s per node @@ -7639,7 +7633,7 @@ spec: e.g. 1m, 1h type: string required: - - pd + - cluster type: object gcs: description: GcsStorageProvider represents the google cloud storage @@ -8471,16 +8465,16 @@ spec: br: description: BRConfig contains config for BR properties: - ca: - description: CA is the CA certificate path for TLS connection - type: string - cert: - description: Cert is the certificate path for TLS connection - type: string checksum: description: Checksum specifies whether to run checksum after backup type: boolean + cluster: + description: ClusterName of backup/restore cluster + type: string + clusterNamespace: + description: Namespace of backup/restore cluster + type: string concurrency: description: Concurrency is the size of thread pool on each node that execute the backup task @@ -8490,18 +8484,15 @@ spec: description: DB is the specific DB which will be backed-up or restored type: string - key: - description: Key is the private key path for TLS connection - type: string + enableTLSClient: + description: Whether enable TLS in TiDBCluster + type: boolean logLevel: description: LogLevel is the log level type: string onLine: description: OnLine specifies whether online during restore type: boolean - pd: - description: PDAddress is the PD address of the tidb cluster - type: string rateLimit: description: RateLimit is the rate limit of the backup task, MB/s per node @@ -8524,7 +8515,7 @@ spec: e.g. 1m, 1h type: string required: - - pd + - cluster type: object from: description: TiDBAccessConfig defines the configuration for access diff --git a/pkg/apis/pingcap/v1alpha1/openapi_generated.go b/pkg/apis/pingcap/v1alpha1/openapi_generated.go index 44bc243165..3e901c7b39 100644 --- a/pkg/apis/pingcap/v1alpha1/openapi_generated.go +++ b/pkg/apis/pingcap/v1alpha1/openapi_generated.go @@ -375,44 +375,37 @@ func schema_pkg_apis_pingcap_v1alpha1_BRConfig(ref common.ReferenceCallback) com Description: "BRConfig contains config for BR", Type: []string{"object"}, Properties: map[string]spec.Schema{ - "pd": { - SchemaProps: spec.SchemaProps{ - Description: "PDAddress is the PD address of the tidb cluster", - Type: []string{"string"}, - Format: "", - }, - }, - "db": { + "enableTLSClient": { SchemaProps: spec.SchemaProps{ - Description: "DB is the specific DB which will be backed-up or restored", - Type: []string{"string"}, + Description: "Whether enable TLS in TiDBCluster", + Type: []string{"boolean"}, Format: "", }, }, - "table": { + "cluster": { SchemaProps: spec.SchemaProps{ - Description: "Table is the specific table which will be backed-up or restored", + Description: "ClusterName of backup/restore cluster", Type: []string{"string"}, Format: "", }, }, - "ca": { + "clusterNamespace": { SchemaProps: spec.SchemaProps{ - Description: "CA is the CA certificate path for TLS connection", + Description: "Namespace of backup/restore cluster", Type: []string{"string"}, Format: "", }, }, - "cert": { + "db": { SchemaProps: spec.SchemaProps{ - Description: "Cert is the certificate path for TLS connection", + Description: "DB is the specific DB which will be backed-up or restored", Type: []string{"string"}, Format: "", }, }, - "key": { + "table": { SchemaProps: spec.SchemaProps{ - Description: "Key is the private key path for TLS connection", + Description: "Table is the specific table which will be backed-up or restored", Type: []string{"string"}, Format: "", }, @@ -474,7 +467,7 @@ func schema_pkg_apis_pingcap_v1alpha1_BRConfig(ref common.ReferenceCallback) com }, }, }, - Required: []string{"pd"}, + Required: []string{"cluster"}, }, }, } diff --git a/pkg/apis/pingcap/v1alpha1/types.go b/pkg/apis/pingcap/v1alpha1/types.go index 6c6465dc24..25eb8551df 100644 --- a/pkg/apis/pingcap/v1alpha1/types.go +++ b/pkg/apis/pingcap/v1alpha1/types.go @@ -809,18 +809,16 @@ type BackupSpec struct { // +k8s:openapi-gen=true // BRConfig contains config for BR type BRConfig struct { - // PDAddress is the PD address of the tidb cluster - PDAddress string `json:"pd"` + // Whether enable TLS in TiDBCluster + EnableTLSClient bool `json:"enableTLSClient,omitempty"` + // ClusterName of backup/restore cluster + Cluster string `json:"cluster"` + // Namespace of backup/restore cluster + ClusterNamespace string `json:"clusterNamespace,omitempty"` // DB is the specific DB which will be backed-up or restored DB string `json:"db,omitempty"` // Table is the specific table which will be backed-up or restored Table string `json:"table,omitempty"` - // CA is the CA certificate path for TLS connection - CA string `json:"ca,omitempty"` - // Cert is the certificate path for TLS connection - Cert string `json:"cert,omitempty"` - // Key is the private key path for TLS connection - Key string `json:"key,omitempty"` // LogLevel is the log level LogLevel string `json:"logLevel,omitempty"` // StatusAddr is the HTTP listening address for the status report service. Set to empty string to disable diff --git a/pkg/backup/backup/backup_manager.go b/pkg/backup/backup/backup_manager.go index 3922099e3b..5b9a9b36ed 100644 --- a/pkg/backup/backup/backup_manager.go +++ b/pkg/backup/backup/backup_manager.go @@ -194,7 +194,6 @@ func (bm *backupManager) makeExportJob(backup *v1alpha1.Backup) (*batchv1.Job, s } backupLabel := label.NewBackup().Instance(backup.GetInstanceName()).BackupJob().Backup(name) - // TODO: need add ResourceRequirement for backup job podSpec := &corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ @@ -271,6 +270,20 @@ func (bm *backupManager) makeBackupJob(backup *v1alpha1.Backup) (*batchv1.Job, s } backupLabel := label.NewBackup().Instance(backup.GetInstanceName()).BackupJob().Backup(name) + volumeMounts := []corev1.VolumeMount{} + volumes := []corev1.Volume{} + if backup.Spec.BR.EnableTLSClient { + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: "br-tls", ReadOnly: true, MountPath: constants.BRCertPath, + }) + volumes = append(volumes, corev1.Volume{ + Name: "br-tls", VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: fmt.Sprintf("%s-client", controller.PDMemberName(backup.Spec.BR.Cluster)), + }, + }, + }) + } podSpec := &corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ @@ -284,12 +297,14 @@ func (bm *backupManager) makeBackupJob(backup *v1alpha1.Backup) (*batchv1.Job, s Image: controller.TidbBackupManagerImage, Args: args, ImagePullPolicy: corev1.PullAlways, + VolumeMounts: volumeMounts, Env: envVars, }, }, RestartPolicy: corev1.RestartPolicyNever, Affinity: backup.Spec.Affinity, Tolerations: backup.Spec.Tolerations, + Volumes: volumes, }, } diff --git a/pkg/backup/backupschedule/backup_schedule_manager.go b/pkg/backup/backupschedule/backup_schedule_manager.go index e2fe7e92c9..b27ae240c6 100644 --- a/pkg/backup/backupschedule/backup_schedule_manager.go +++ b/pkg/backup/backupschedule/backup_schedule_manager.go @@ -215,9 +215,16 @@ func (bm *backupScheduleManager) createBackup(bs *v1alpha1.BackupSchedule, times } } } else { + var pdAddress, clusterNamespace string + clusterNamespace = backupSpec.BR.ClusterNamespace + if backupSpec.BR.ClusterNamespace == "" { + clusterNamespace = ns + } + pdAddress = fmt.Sprintf("%s-pd.%s:2379", backupSpec.BR.Cluster, clusterNamespace) + if backupSpec.S3 != nil { backupSpec.S3.Prefix = path.Join(backupSpec.S3.Prefix, - strings.ReplaceAll(backupSpec.BR.PDAddress, ":", "-")+"-"+timestamp.UTC().Format(constants.TimeFormat)) + strings.ReplaceAll(pdAddress, ":", "-")+"-"+timestamp.UTC().Format(constants.TimeFormat)) } } diff --git a/pkg/backup/constants/constants.go b/pkg/backup/constants/constants.go index 82ad258a12..ba7d8d706d 100644 --- a/pkg/backup/constants/constants.go +++ b/pkg/backup/constants/constants.go @@ -49,4 +49,10 @@ const ( // BackupManagerEnvVarPrefix represents the environment variable used for tidb-backup-manager must include this prefix BackupManagerEnvVarPrefix = "BACKUP_MANAGER" + + // BR certificate storage path + BRCertPath = "/var/lib/br-tls" + + // ServiceAccountCAPath is where is CABundle of serviceaccount locates + ServiceAccountCAPath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" ) diff --git a/pkg/backup/restore/restore_manager.go b/pkg/backup/restore/restore_manager.go index 1dbd11b94f..03303b16da 100644 --- a/pkg/backup/restore/restore_manager.go +++ b/pkg/backup/restore/restore_manager.go @@ -253,7 +253,20 @@ func (rm *restoreManager) makeRestoreJob(restore *v1alpha1.Restore) (*batchv1.Jo } restoreLabel := label.NewBackup().Instance(restore.GetInstanceName()).RestoreJob().Restore(name) - + volumeMounts := []corev1.VolumeMount{} + volumes := []corev1.Volume{} + if restore.Spec.BR.EnableTLSClient { + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: "br-tls", ReadOnly: true, MountPath: constants.BRCertPath, + }) + volumes = append(volumes, corev1.Volume{ + Name: "br-tls", VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: fmt.Sprintf("%s-client", controller.PDMemberName(restore.Spec.BR.Cluster)), + }, + }, + }) + } // TODO: need add ResourceRequirement for restore job podSpec := &corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ @@ -267,9 +280,11 @@ func (rm *restoreManager) makeRestoreJob(restore *v1alpha1.Restore) (*batchv1.Jo Image: controller.TidbBackupManagerImage, Args: args, ImagePullPolicy: corev1.PullAlways, + VolumeMounts: volumeMounts, Env: envVars, }, }, + Volumes: volumes, RestartPolicy: corev1.RestartPolicyNever, }, } diff --git a/pkg/backup/util/util.go b/pkg/backup/util/util.go index d62307b7bb..86317f53e9 100644 --- a/pkg/backup/util/util.go +++ b/pkg/backup/util/util.go @@ -299,8 +299,8 @@ func ValidateBackup(backup *v1alpha1.Backup) error { return fmt.Errorf("missing StorageSize config in spec of %s/%s", ns, name) } } else { - if backup.Spec.BR.PDAddress == "" { - return fmt.Errorf("pd address should be configured for BR in spec of %s/%s", ns, name) + if backup.Spec.BR.Cluster == "" { + return fmt.Errorf("cluster should be configured for BR in spec of %s/%s", ns, name) } if backup.Spec.Type != "" && backup.Spec.Type != v1alpha1.BackupTypeFull && @@ -351,8 +351,8 @@ func ValidateRestore(restore *v1alpha1.Restore) error { return fmt.Errorf("missing StorageSize config in spec of %s/%s", ns, name) } } else { - if restore.Spec.BR.PDAddress == "" { - return fmt.Errorf("pd address should be configured for BR in spec of %s/%s", ns, name) + if restore.Spec.BR.Cluster == "" { + return fmt.Errorf("cluster should be configured for BR in spec of %s/%s", ns, name) } if restore.Spec.Type != "" && restore.Spec.Type != v1alpha1.BackupTypeFull &&