Merge pull request #24080 from fabiomatavelli/b-fix-remote-exec

ssh: return error if host is empty

communicator/communicator_test.go

@@ -16,6 +16,7 @@ func TestCommunicator_new(t *testing.T) {
 		Ephemeral: terraform.EphemeralState{
 			ConnInfo: map[string]string{
 				"type": "telnet",
+				"host": "127.0.0.1",
 			},
 		},
 	}

communicator/ssh/communicator_test.go

@@ -149,6 +149,25 @@ func TestNew_Invalid(t *testing.T) {
 	}
 }
 
+func TestNew_InvalidHost(t *testing.T) {
+	r := &terraform.InstanceState{
+		Ephemeral: terraform.EphemeralState{
+			ConnInfo: map[string]string{
+				"type":     "ssh",
+				"user":     "user",
+				"password": "i-am-invalid",
+				"port":     "22",
+				"timeout":  "30s",
+			},
+		},
+	}
+
+	_, err := New(r)
+	if err == nil {
+		t.Fatal("should have had an error creating communicator")
+	}
+}
+
 func TestStart(t *testing.T) {
 	address := newMockLineServer(t, nil, testClientPublicKey)
 	parts := strings.Split(address, ":")
@@ -691,6 +710,7 @@ func TestScriptPath(t *testing.T) {
 			Ephemeral: terraform.EphemeralState{
 				ConnInfo: map[string]string{
 					"type":        "ssh",
+					"host":        "127.0.0.1",
 					"script_path": tc.Input,
 				},
 			},
@@ -715,7 +735,14 @@ func TestScriptPath_randSeed(t *testing.T) {
 	// Pre GH-4186 fix, this value was the deterministic start the pseudorandom
 	// chain of unseeded math/rand values for Int31().
 	staticSeedPath := "/tmp/terraform_1298498081.sh"
-	c, err := New(&terraform.InstanceState{})
+	c, err := New(&terraform.InstanceState{
+		Ephemeral: terraform.EphemeralState{
+			ConnInfo: map[string]string{
+				"type": "ssh",
+				"host": "127.0.0.1",
+			},
+		},
+	})
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}

communicator/ssh/provisioner.go

@@ -16,7 +16,7 @@ import (
 	"github.com/hashicorp/terraform/communicator/shared"
 	"github.com/hashicorp/terraform/terraform"
 	"github.com/mitchellh/mapstructure"
-	"github.com/xanzy/ssh-agent"
+	sshagent "github.com/xanzy/ssh-agent"
 	"golang.org/x/crypto/ssh"
 	"golang.org/x/crypto/ssh/agent"
 	"golang.org/x/crypto/ssh/knownhosts"
@@ -93,6 +93,12 @@ func parseConnectionInfo(s *terraform.InstanceState) (*connectionInfo, error) {
 		connInfo.User = DefaultUser
 	}
 
+	// Check if host is empty.
+	// Otherwise return error.
+	if connInfo.Host == "" {
+		return nil, fmt.Errorf("host for provisioner cannot be empty")
+	}
+
 	// Format the host if needed.
 	// Needed for IPv6 support.
 	connInfo.Host = shared.IpFormat(connInfo.Host)

communicator/ssh/provisioner_test.go

@@ -131,3 +131,24 @@ func TestProvisioner_connInfoHostname(t *testing.T) {
 		t.Fatalf("bad %v", conf)
 	}
 }
+
+func TestProvisioner_connInfoEmptyHostname(t *testing.T) {
+	r := &terraform.InstanceState{
+		Ephemeral: terraform.EphemeralState{
+			ConnInfo: map[string]string{
+				"type":        "ssh",
+				"user":        "root",
+				"password":    "supersecret",
+				"private_key": "someprivatekeycontents",
+				"host":        "",
+				"port":        "22",
+				"timeout":     "30s",
+			},
+		},
+	}
+
+	_, err := parseConnectionInfo(r)
+	if err == nil {
+		t.Fatalf("bad: should not allow empty host")
+	}
+}