cluster: check, deploy support use ssh-agent instead of identity-file (…

…#1416)
pingcap · Jun 11, 2021 · ffa74b5 · ffa74b5
1 parent befddd5
commit ffa74b5
Show file tree

Hide file tree

Showing 8 changed files with 57 additions and 69 deletions.
diff --git a/components/cluster/command/check.go b/components/cluster/command/check.go
@@ -16,7 +16,6 @@ package command
 import (
 	"path"
 
-	"github.com/pingcap/tiup/pkg/cluster/executor"
 	"github.com/pingcap/tiup/pkg/cluster/manager"
 	operator "github.com/pingcap/tiup/pkg/cluster/operation"
 	"github.com/pingcap/tiup/pkg/utils"
@@ -44,10 +43,6 @@ conflict checks with other clusters`,
 			if opt.ExistCluster {
 				clusterReport.ID = scrubClusterName(args[0])
 			}
-			// natvie ssh has it's own logic to find the default identity_file
-			if gOpt.SSHType == executor.SSHTypeSystem && !utils.IsFlagSetByUser(cmd.Flags(), "identity_file") {
-				opt.IdentityFile = ""
-			}
 			return cm.CheckCluster(args[0], opt, gOpt)
 		},
 	}

diff --git a/components/cluster/command/deploy.go b/components/cluster/command/deploy.go
@@ -19,7 +19,6 @@ import (
 	"path"
 
 	"github.com/pingcap/tiup/pkg/cliutil"
-	"github.com/pingcap/tiup/pkg/cluster/executor"
 	"github.com/pingcap/tiup/pkg/cluster/manager"
 	operator "github.com/pingcap/tiup/pkg/cluster/operation"
 	"github.com/pingcap/tiup/pkg/cluster/spec"
@@ -53,11 +52,6 @@ func newDeploy() *cobra.Command {
 				return nil
 			}
 
-			// natvie ssh has it's own logic to find the default identity_file
-			if gOpt.SSHType == executor.SSHTypeSystem && !utils.IsFlagSetByUser(cmd.Flags(), "identity_file") {
-				opt.IdentityFile = ""
-			}
-
 			clusterName := args[0]
 			version, err := utils.FmtVer(args[1])
 			if err != nil {

diff --git a/components/cluster/command/scale_out.go b/components/cluster/command/scale_out.go
@@ -17,7 +17,6 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/pingcap/tiup/pkg/cluster/executor"
 	"github.com/pingcap/tiup/pkg/cluster/manager"
 	"github.com/pingcap/tiup/pkg/cluster/spec"
 	"github.com/pingcap/tiup/pkg/cluster/task"
@@ -38,11 +37,6 @@ func newScaleOutCmd() *cobra.Command {
 				return cmd.Help()
 			}
 
-			// natvie ssh has it's own logic to find the default identity_file
-			if gOpt.SSHType == executor.SSHTypeSystem && !utils.IsFlagSetByUser(cmd.Flags(), "identity_file") {
-				opt.IdentityFile = ""
-			}
-
 			clusterName := args[0]
 			clusterReport.ID = scrubClusterName(clusterName)
 			teleCommand = append(teleCommand, scrubClusterName(clusterName))

diff --git a/components/dm/command/deploy.go b/components/dm/command/deploy.go
@@ -19,7 +19,6 @@ import (
 
 	"github.com/pingcap/errors"
 	"github.com/pingcap/tiup/pkg/cliutil"
-	"github.com/pingcap/tiup/pkg/cluster/executor"
 	"github.com/pingcap/tiup/pkg/cluster/manager"
 	operator "github.com/pingcap/tiup/pkg/cluster/operation"
 	"github.com/pingcap/tiup/pkg/cluster/spec"
@@ -47,11 +46,6 @@ func newDeployCmd() *cobra.Command {
 				return nil
 			}
 
-			// natvie ssh has it's own logic to find the default identity_file
-			if gOpt.SSHType == executor.SSHTypeSystem && !utils.IsFlagSetByUser(cmd.Flags(), "identity_file") {
-				opt.IdentityFile = ""
-			}
-
 			clusterName := args[0]
 			version, err := utils.FmtVer(args[1])
 			if err != nil {

diff --git a/components/dm/command/scale_out.go b/components/dm/command/scale_out.go
@@ -16,7 +16,6 @@ package command
 import (
 	"path/filepath"
 
-	"github.com/pingcap/tiup/pkg/cluster/executor"
 	"github.com/pingcap/tiup/pkg/cluster/manager"
 	"github.com/pingcap/tiup/pkg/cluster/spec"
 	"github.com/pingcap/tiup/pkg/cluster/task"
@@ -37,11 +36,6 @@ func newScaleOutCmd() *cobra.Command {
 				return cmd.Help()
 			}
 
-			// natvie ssh has it's own logic to find the default identity_file
-			if gOpt.SSHType == executor.SSHTypeSystem && !utils.IsFlagSetByUser(cmd.Flags(), "identity_file") {
-				opt.IdentityFile = ""
-			}
-
 			clusterName := args[0]
 			topoFile := args[1]
 

diff --git a/pkg/cliutil/ssh.go b/pkg/cliutil/ssh.go
@@ -18,12 +18,13 @@ import (
 
 	"github.com/ScaleFT/sshkeys"
 	"github.com/pingcap/tiup/pkg/errutil"
+	"github.com/pingcap/tiup/pkg/utils"
 	"golang.org/x/crypto/ssh"
 )
 
 var (
-	// ErrIdentityFileReadFiled is ErrIdentityFileReadFiled
-	ErrIdentityFileReadFiled = errNS.NewType("id_read_failed", errutil.ErrTraitPreCheck)
+	// ErrIdentityFileReadFailed is ErrIdentityFileReadFailed
+	ErrIdentityFileReadFailed = errNS.NewType("id_read_failed", errutil.ErrTraitPreCheck)
 )
 
 // SSHConnectionProps is SSHConnectionProps
@@ -41,50 +42,65 @@ func ReadIdentityFileOrPassword(identityFilePath string, usePass bool) (*SSHConn
 		return &SSHConnectionProps{
 			Password: password,
 		}, nil
-	} else if identityFilePath == "" {
-		return &SSHConnectionProps{}, nil
 	}
 
 	// Identity file is specified, check identity file
-	buf, err := os.ReadFile(identityFilePath)
-	if err != nil {
-		return nil, ErrIdentityFileReadFiled.
-			Wrap(err, "Failed to read SSH identity file '%s'", identityFilePath).
-			WithProperty(SuggestionFromTemplate(`
+	if len(identityFilePath) > 0 && utils.IsExist(identityFilePath) {
+		buf, err := os.ReadFile(identityFilePath)
+		if err != nil {
+			return nil, ErrIdentityFileReadFailed.
+				Wrap(err, "Failed to read SSH identity file '%s'", identityFilePath).
+				WithProperty(SuggestionFromTemplate(`
 Please check whether your SSH identity file {{ColorKeyword}}{{.File}}{{ColorReset}} exists and have access permission.
 `, map[string]string{
-				"File": identityFilePath,
-			}))
-	}
+					"File": identityFilePath,
+				}))
+		}
 
-	// Try to decode as not encrypted
-	_, err = ssh.ParsePrivateKey(buf)
-	if err == nil {
-		return &SSHConnectionProps{
-			IdentityFile: identityFilePath,
-		}, nil
-	}
+		// Try to decode as not encrypted
+		_, err = ssh.ParsePrivateKey(buf)
+		if err == nil {
+			return &SSHConnectionProps{
+				IdentityFile: identityFilePath,
+			}, nil
+		}
 
-	// Other kind of error.. e.g. not a valid SSH key
-	if _, ok := err.(*ssh.PassphraseMissingError); !ok {
-		return nil, ErrIdentityFileReadFiled.
-			Wrap(err, "Failed to read SSH identity file '%s'", identityFilePath).
-			WithProperty(SuggestionFromTemplate(`
+		// Other kind of error.. e.g. not a valid SSH key
+		if _, ok := err.(*ssh.PassphraseMissingError); !ok {
+			return nil, ErrIdentityFileReadFailed.
+				Wrap(err, "Failed to read SSH identity file '%s'", identityFilePath).
+				WithProperty(SuggestionFromTemplate(`
 Looks like your SSH private key {{ColorKeyword}}{{.File}}{{ColorReset}} is invalid.
 `, map[string]string{
-				"File": identityFilePath,
-			}))
+					"File": identityFilePath,
+				}))
+		}
+
+		// SSH key is passphrase protected
+		passphrase := PromptForPassword("The SSH identity key is encrypted. Input its passphrase: ")
+		if _, err := sshkeys.ParseEncryptedPrivateKey(buf, []byte(passphrase)); err != nil {
+			return nil, ErrIdentityFileReadFailed.
+				Wrap(err, "Failed to decrypt SSH identity file '%s'", identityFilePath)
+		}
+
+		return &SSHConnectionProps{
+			IdentityFile:           identityFilePath,
+			IdentityFilePassphrase: passphrase,
+		}, nil
 	}
 
-	// SSH key is passphrase protected
-	passphrase := PromptForPassword("The SSH identity key is encrypted. Input its passphrase: ")
-	if _, err := sshkeys.ParseEncryptedPrivateKey(buf, []byte(passphrase)); err != nil {
-		return nil, ErrIdentityFileReadFiled.
-			Wrap(err, "Failed to decrypt SSH identity file '%s'", identityFilePath)
+	// No password, nor identity file were specified, check ssh-agent via the env SSH_AUTH_SOCK
+	sshAuthSock := os.Getenv("SSH_AUTH_SOCK")
+	if len(sshAuthSock) == 0 {
+		return nil, ErrIdentityFileReadFailed.New("none of ssh password, identity file, SSH_AUTH_SOCK specified")
+	}
+	stat, err := os.Stat(sshAuthSock)
+	if err != nil {
+		return nil, ErrIdentityFileReadFailed.Wrap(err, "Failed to stat SSH_AUTH_SOCK file: '%s'", sshAuthSock)
+	}
+	if stat.Mode()&os.ModeSocket == 0 {
+		return nil, ErrIdentityFileReadFailed.New("The SSH_AUTH_SOCK file: '%s' is not a valid unix socket file", sshAuthSock)
 	}
 
-	return &SSHConnectionProps{
-		IdentityFile:           identityFilePath,
-		IdentityFilePassphrase: passphrase,
-	}, nil
+	return &SSHConnectionProps{}, nil
 }
diff --git a/pkg/cluster/manager/check.go b/pkg/cluster/manager/check.go
@@ -89,11 +89,6 @@ func (m *Manager) CheckCluster(clusterOrTopoName string, opt CheckOptions, gOpt
 		}
 	}
 
-	// natvie ssh has it's own logic to find the default identity_file
-	// if gOpt.SSHType == executor.SSHTypeSystem && !utils.IsFlagSetByUser(cmd.Flags(), "identity_file") {
-	// 	opt.identityFile = ""
-	// }
-
 	var sshConnProps *cliutil.SSHConnectionProps = &cliutil.SSHConnectionProps{}
 	if gOpt.SSHType != executor.SSHTypeNone {
 		var err error

diff --git a/tests/tiup-cluster/script/cmd_subtest.sh b/tests/tiup-cluster/script/cmd_subtest.sh
@@ -21,7 +21,13 @@ function cmd_subtest() {
         client="--ssh=system"
     fi
 
+    # identify SSH via ssh-agent
+    eval $(ssh-agent) &> /dev/null
+    ssh-add /root/.ssh/id_rsa &> /dev/null
+
+    mv /root/.ssh/id_rsa{,.bak}
     tiup-cluster $client check $topo -i ~/.ssh/id_rsa --enable-mem --enable-cpu --apply
+    mv /root/.ssh/id_rsa{.bak,}
 
     check_result=`tiup-cluster $client --yes check $topo -i ~/.ssh/id_rsa`