From 0cbe4d8efb9284f714f69b46dd9263bb3ee621b6 Mon Sep 17 00:00:00 2001
From: Alisdair McDiarmid <alisdair@users.noreply.github.com>
Date: Mon, 10 May 2021 11:46:07 -0400
Subject: [PATCH] communicator/ssh: Fix crash with SSH bastion port

The connection block schema defines the bastion_port argument as a
number, but we were incorrectly trying to convert it from a string. This
commit fixes that by attempting to convert the cty.Number to the int
result type, returning the error on failure.

An alternative approach would be to change the bastion_port argument in
the schema to be a string, matching the port argument. I'm less sure
about the secondary effects of that change, though.
---
 communicator/ssh/provisioner.go      |  5 ++---
 communicator/ssh/provisioner_test.go | 26 +++++++++++++++++++++++++-
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/communicator/ssh/provisioner.go b/communicator/ssh/provisioner.go
index a3fa80c42..72a492f39 100644
--- a/communicator/ssh/provisioner.go
+++ b/communicator/ssh/provisioner.go
@@ -17,6 +17,7 @@ import (
 	"github.com/hashicorp/terraform/communicator/shared"
 	sshagent "github.com/xanzy/ssh-agent"
 	"github.com/zclconf/go-cty/cty"
+	"github.com/zclconf/go-cty/cty/gocty"
 	"golang.org/x/crypto/ssh"
 	"golang.org/x/crypto/ssh/agent"
 	"golang.org/x/crypto/ssh/knownhosts"
@@ -127,11 +128,9 @@ func decodeConnInfo(v cty.Value) (*connectionInfo, error) {
 		case "bastion_host_key":
 			connInfo.BastionHostKey = v.AsString()
 		case "bastion_port":
-			p, err := strconv.Atoi(v.AsString())
-			if err != nil {
+			if err := gocty.FromCtyValue(v, &connInfo.BastionPort); err != nil {
 				return nil, err
 			}
-			connInfo.BastionPort = p
 		case "agent_identity":
 			connInfo.AgentIdentity = v.AsString()
 		}
diff --git a/communicator/ssh/provisioner_test.go b/communicator/ssh/provisioner_test.go
index ace46f49d..98b3cf7e6 100644
--- a/communicator/ssh/provisioner_test.go
+++ b/communicator/ssh/provisioner_test.go
@@ -17,6 +17,7 @@ func TestProvisioner_connInfo(t *testing.T) {
 		"port":         cty.StringVal("22"),
 		"timeout":      cty.StringVal("30s"),
 		"bastion_host": cty.StringVal("127.0.1.1"),
+		"bastion_port": cty.NumberIntVal(20022),
 	})
 
 	conf, err := parseConnectionInfo(v)
@@ -54,7 +55,7 @@ func TestProvisioner_connInfo(t *testing.T) {
 	if conf.BastionHost != "127.0.1.1" {
 		t.Fatalf("bad: %v", conf)
 	}
-	if conf.BastionPort != 22 {
+	if conf.BastionPort != 20022 {
 		t.Fatalf("bad: %v", conf)
 	}
 	if conf.BastionUser != "root" {
@@ -135,3 +136,26 @@ func TestProvisioner_connInfoEmptyHostname(t *testing.T) {
 		t.Fatalf("bad: should not allow empty host")
 	}
 }
+
+func TestProvisioner_stringBastionPort(t *testing.T) {
+	v := cty.ObjectVal(map[string]cty.Value{
+		"type":         cty.StringVal("ssh"),
+		"user":         cty.StringVal("root"),
+		"password":     cty.StringVal("supersecret"),
+		"private_key":  cty.StringVal("someprivatekeycontents"),
+		"host":         cty.StringVal("example.com"),
+		"port":         cty.StringVal("22"),
+		"timeout":      cty.StringVal("30s"),
+		"bastion_host": cty.StringVal("example.com"),
+		"bastion_port": cty.StringVal("12345"),
+	})
+
+	conf, err := parseConnectionInfo(v)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	if conf.BastionPort != 12345 {
+		t.Fatalf("bad %v", conf)
+	}
+}