Merge pull request #253 from roots/key-generate-better-error-handling

Fix ssh-keyscan error handling in `key generate` command

Author: swalkinshaw

cmd/key_generate.go

@@ -17,6 +17,7 @@ import (
 	"github.com/mitchellh/cli"
 	"github.com/mitchellh/go-homedir"
 	"github.com/posener/complete"
+	"github.com/roots/trellis-cli/command"
 	"github.com/roots/trellis-cli/trellis"
 )
 
@@ -82,7 +83,7 @@ func (c *KeyGenerateCommand) Run(args []string) int {
 			return 1
 		}
 
-		_, err = exec.Command("gh", "auth", "status").Output()
+		_, err = command.Cmd("gh", []string{"auth", "status"}).Output()
 		if err != nil {
 			c.UI.Error("Error: GitHub CLI is not authenticated.")
 			c.UI.Error("Run `gh auth login` first.")
@@ -128,7 +129,7 @@ func (c *KeyGenerateCommand) Run(args []string) int {
 	}
 
 	keygenArgs := []string{"-t", "ed25519", "-C", deployKeyName, "-f", keyPath, "-P", ""}
-	sshKeygen := exec.Command("ssh-keygen", keygenArgs...)
+	sshKeygen := command.Cmd("ssh-keygen", keygenArgs)
 	sshKeygen.Stdout = io.Discard
 	sshKeygen.Stderr = os.Stderr
 	err := sshKeygen.Run()
@@ -197,14 +198,15 @@ func (c *KeyGenerateCommand) Run(args []string) int {
 		return 1
 	}
 
-	keyscanOutput, err := exec.Command("ssh-keyscan", "-t", "ed25519", "-H", strings.Join(hosts, " ")).Output()
-	if err != nil {
-		c.UI.Error("Error: could not set SSH known hosts. ssh-keyscan command failed.")
-		c.UI.Error(err.Error())
+	knownHosts := keyscanHosts(hosts)
+
+	if len(knownHosts) == 0 {
+		c.UI.Error("Error: could not set SSH known hosts.")
+		c.UI.Error(fmt.Sprintf("ssh-keyscan command failed for all hosts: %s", hosts))
 		return 1
 	}
 
-	err = githubCLI("secret", "set", sshKnownHostsSecret, "--body", string(keyscanOutput))
+	err = githubCLI("secret", "set", sshKnownHostsSecret, "--body", strings.Join(knownHosts, "\n"))
 	if err != nil {
 		c.UI.Error("Error: could not set GitHub secret")
 		c.UI.Error(err.Error())
@@ -308,15 +310,16 @@ func (c *KeyGenerateCommand) AutocompleteFlags() complete.Flags {
 }
 
 func githubCLI(args ...string) error {
-	ghCmd := exec.Command("gh", args...)
+	ghCmd := command.Cmd("gh", args)
 	ghCmd.Stdout = io.Discard
 	ghCmd.Stderr = os.Stderr
 
 	return ghCmd.Run()
 }
 
 func getAnsibleHosts() (hosts []string, err error) {
-	hostsOutput, err := exec.Command("ansible", "all", "--list-hosts").Output()
+	args := []string{"all", "--list-hosts", "--limit", "!development"}
+	hostsOutput, err := command.Cmd("ansible", args).Output()
 
 	if err != nil {
 		return nil, err
@@ -352,8 +355,26 @@ func parseAnsibleHosts(output string) (hosts []string) {
 			continue
 		}
 
+		// remove default placeholder since it will cause  an error
+		// this isn't ideal, but it will do
+		if host == "your_server_hostname" {
+			continue
+		}
+
 		hosts = append(hosts, host)
 	}
 
 	return hosts
 }
+
+func keyscanHosts(hosts []string) (knownHosts []string) {
+	for _, host := range hosts {
+		output, err := command.Cmd("ssh-keyscan", []string{"-t", "ed25519", "-H", "-T", "1", host}).Output()
+
+		if err == nil {
+			knownHosts = append(knownHosts, string(output))
+		}
+	}
+
+	return knownHosts
+}

cmd/key_generate_test.go

@@ -4,12 +4,14 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"reflect"
 	"strings"
 	"testing"
 
 	"github.com/mitchellh/cli"
+	"github.com/roots/trellis-cli/command"
 	"github.com/roots/trellis-cli/trellis"
 )
 
@@ -149,6 +151,40 @@ func TestKeyGenerateExistingPublicKey(t *testing.T) {
 	}
 }
 
+func TestKeyGenerateKeyscan(t *testing.T) {
+	defer trellis.LoadFixtureProject(t)()
+	trellis := trellis.NewTrellis()
+
+	tmpDir, _ := ioutil.TempDir("", "key_generate_test")
+	defer os.RemoveAll(tmpDir)
+
+	// fake gh binary to satisfy ok.LookPath
+	ghPath := filepath.Join(tmpDir, "gh")
+	os.OpenFile(ghPath, os.O_CREATE, 0555)
+	path := os.Getenv("PATH")
+	os.Setenv("PATH", fmt.Sprintf("PATH=%s:%s", path, tmpDir))
+
+	mockExecCommand := func(command string, args []string) *exec.Cmd {
+		cs := []string{"-test.run=TestKeyGenerateHelperProcess", "--", command}
+		cs = append(cs, args...)
+		cmd := exec.Command(os.Args[0], cs...)
+		cmd.Env = []string{"GO_WANT_HELPER_PROCESS=1", fmt.Sprintf("GO_TEST_HELPER_TMP_PATH=%s", tmpDir)}
+		return cmd
+	}
+
+	command.Mock(mockExecCommand)
+	defer command.Restore()
+
+	ui := cli.NewMockUi()
+	keyGenerateCommand := NewKeyGenerateCommand(ui, trellis)
+
+	code := keyGenerateCommand.Run([]string{"--path", tmpDir})
+
+	if code != 0 {
+		t.Errorf("expected code %d to be %d", code, 0)
+	}
+}
+
 func TestParseAnsibleHosts(t *testing.T) {
 	output := `
   hosts (3):
@@ -162,10 +198,49 @@ func TestParseAnsibleHosts(t *testing.T) {
 	expectedHosts := []string{
 		"192.168.56.5",
 		"192.168.56.10",
-		"your_server_hostname",
 	}
 
 	if !reflect.DeepEqual(hosts, expectedHosts) {
 		t.Errorf("expected hosts %q to equal %q", hosts, expectedHosts)
 	}
 }
+
+func TestKeyGenerateHelperProcess(t *testing.T) {
+	if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" {
+		return
+	}
+
+	switch os.Args[3] {
+	case "ansible":
+		hosts := `
+good_host
+bad_host
+your_server_hostname
+`
+		fmt.Fprintf(os.Stdout, hosts)
+		os.Exit(0)
+	case "ssh-keyscan":
+		switch os.Args[len(os.Args)-1] {
+		case "good_host":
+			// return fake hash for a good host
+			host := "|1|5XBUprxMy6abCgLQkQ0= ssh-ed25519 AAAAC3NzaC1lZYqEOf"
+			fmt.Fprintf(os.Stdout, host)
+			os.Exit(0)
+		case "bad_host":
+			// simulate error for a bad host
+			os.Exit(1)
+		}
+	case "ssh-keygen":
+		tmpDir := os.Getenv("GO_TEST_HELPER_TMP_PATH")
+		path := filepath.Join(tmpDir, "trellis_example_com_ed25519.pub")
+		os.OpenFile(path, os.O_CREATE, 0644)
+		path = filepath.Join(tmpDir, "trellis_example_com_ed25519")
+		os.OpenFile(path, os.O_CREATE, 0644)
+		os.Exit(0)
+	case "gh":
+		// make all gh commands succeed. No output needed
+		os.Exit(0)
+	default:
+		t.Fatalf("unexpected command %s", os.Args[3])
+	}
+}