Automated cherry pick of pingcap#1836: backup: add TLS to backup br (p…

…ingcap#1950) * Backup: support TLS for br component (pingcap#1836) * backup: add TLS to backup br * fix API docs Co-authored-by: 尹亮 <30903849+shuijing198799@users.noreply.github.com>
cofyc · Mar 17, 2020 · 579a2ce · 579a2ce
1 parent 7a4dc2a
commit 579a2ce
Show file tree

Hide file tree

Showing 13 changed files with 150 additions and 113 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -1502,68 +1502,57 @@ <h3 id="pingcap.com/v1alpha1.BRConfig">BRConfig
 <tbody>
 <tr>
 <td>
-<code>pd</code></br>
-<em>
-string
-</em>
-</td>
-<td>
-<p>PDAddress is the PD address of the tidb cluster</p>
-</td>
-</tr>
-<tr>
-<td>
-<code>db</code></br>
+<code>enableTLSClient</code></br>
 <em>
-string
+bool
 </em>
 </td>
 <td>
-<p>DB is the specific DB which will be backed-up or restored</p>
+<p>Whether enable TLS in TiDBCluster</p>
 </td>
 </tr>
 <tr>
 <td>
-<code>table</code></br>
+<code>cluster</code></br>
 <em>
 string
 </em>
 </td>
 <td>
-<p>Table is the specific table which will be backed-up or restored</p>
+<p>ClusterName of backup/restore cluster</p>
 </td>
 </tr>
 <tr>
 <td>
-<code>ca</code></br>
+<code>clusterNamespace</code></br>
 <em>
 string
 </em>
 </td>
 <td>
-<p>CA is the CA certificate path for TLS connection</p>
+<p>Namespace of backup/restore cluster</p>
 </td>
 </tr>
 <tr>
 <td>
-<code>cert</code></br>
+<code>db</code></br>
 <em>
 string
 </em>
 </td>
 <td>
-<p>Cert is the certificate path for TLS connection</p>
+<p>DB is the specific DB which will be backed-up or restored</p>
 </td>
 </tr>
 <tr>
 <td>
-<code>key</code></br>
+<code>table</code></br>
 <em>
 string
 </em>
 </td>
 <td>
-<p>Key is the private key path for TLS connection</p>
+<p>Table is the specific table which will be backed-up or restored</p>
 </td>
 </tr>
 <tr>

diff --git 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