diff --git a/communicator/shared/shared.go b/communicator/shared/shared.go index 509aadd28..a8535ac60 100644 --- a/communicator/shared/shared.go +++ b/communicator/shared/shared.go @@ -36,7 +36,7 @@ var ConnectionBlockSupersetSchema = &configschema.Block{ Optional: true, }, "port": { - Type: cty.String, + Type: cty.Number, Optional: true, }, "timeout": { diff --git a/communicator/ssh/communicator_test.go b/communicator/ssh/communicator_test.go index d71044320..b553576f8 100644 --- a/communicator/ssh/communicator_test.go +++ b/communicator/ssh/communicator_test.go @@ -331,7 +331,7 @@ func TestHostKey(t *testing.T) { Password: "pass", Host: host, HostKey: pubKey, - Port: port, + Port: uint16(port), Timeout: "30s", } @@ -363,7 +363,7 @@ func TestHostKey(t *testing.T) { port, _ = strconv.Atoi(p) connInfo.HostKey = testClientPublicKey - connInfo.Port = port + connInfo.Port = uint16(port) cfg, err = prepareSSHConfig(connInfo) if err != nil { @@ -406,7 +406,7 @@ func TestHostCert(t *testing.T) { Password: "pass", Host: host, HostKey: testCAPublicKey, - Port: port, + Port: uint16(port), Timeout: "30s", } @@ -438,7 +438,7 @@ func TestHostCert(t *testing.T) { port, _ = strconv.Atoi(p) connInfo.HostKey = testClientPublicKey - connInfo.Port = port + connInfo.Port = uint16(port) cfg, err = prepareSSHConfig(connInfo) if err != nil { @@ -528,7 +528,7 @@ func TestCertificateBasedAuth(t *testing.T) { Host: host, PrivateKey: CLIENT_PEM, Certificate: CLIENT_CERT_SIGNED_BY_SERVER, - Port: port, + Port: uint16(port), Timeout: "30s", } diff --git a/communicator/ssh/provisioner.go b/communicator/ssh/provisioner.go index a3fa80c42..666f8fbea 100644 --- a/communicator/ssh/provisioner.go +++ b/communicator/ssh/provisioner.go @@ -10,13 +10,13 @@ import ( "net" "os" "path/filepath" - "strconv" "strings" "time" "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" @@ -55,7 +55,7 @@ type connectionInfo struct { Certificate string Host string HostKey string - Port int + Port uint16 Agent bool ScriptPath string TargetPlatform string @@ -68,7 +68,7 @@ type connectionInfo struct { BastionCertificate string BastionHost string BastionHostKey string - BastionPort int + BastionPort uint16 AgentIdentity string } @@ -101,11 +101,9 @@ func decodeConnInfo(v cty.Value) (*connectionInfo, error) { case "host_key": connInfo.HostKey = v.AsString() case "port": - p, err := strconv.Atoi(v.AsString()) - if err != nil { + if err := gocty.FromCtyValue(v, &connInfo.Port); err != nil { return nil, err } - connInfo.Port = p case "agent": connInfo.Agent = v.True() case "script_path": @@ -127,11 +125,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..05d8179c5 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,45 @@ 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) + } +} + +func TestProvisioner_invalidPortNumber(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.NumberIntVal(123456789), + }) + + _, err := parseConnectionInfo(v) + if err == nil { + t.Fatalf("bad: should not allow invalid port number") + } + if got, want := err.Error(), "value must be a whole number, between 0 and 65535 inclusive"; got != want { + t.Errorf("unexpected error\n got: %s\nwant: %s", got, want) + } +} diff --git a/communicator/winrm/communicator.go b/communicator/winrm/communicator.go index 4f9f28838..d877e3065 100644 --- a/communicator/winrm/communicator.go +++ b/communicator/winrm/communicator.go @@ -33,7 +33,7 @@ func New(v cty.Value) (*Communicator, error) { endpoint := &winrm.Endpoint{ Host: connInfo.Host, - Port: connInfo.Port, + Port: int(connInfo.Port), HTTPS: connInfo.HTTPS, Insecure: connInfo.Insecure, Timeout: connInfo.TimeoutVal, diff --git a/communicator/winrm/provisioner.go b/communicator/winrm/provisioner.go index 7a71fe92f..f77918ec8 100644 --- a/communicator/winrm/provisioner.go +++ b/communicator/winrm/provisioner.go @@ -4,12 +4,12 @@ import ( "fmt" "log" "path/filepath" - "strconv" "strings" "time" "github.com/hashicorp/terraform/communicator/shared" "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/gocty" ) const ( @@ -37,7 +37,7 @@ type connectionInfo struct { User string Password string Host string - Port int + Port uint16 HTTPS bool Insecure bool NTLM bool `mapstructure:"use_ntlm"` @@ -69,11 +69,9 @@ func decodeConnInfo(v cty.Value) (*connectionInfo, error) { case "host": connInfo.Host = v.AsString() case "port": - p, err := strconv.Atoi(v.AsString()) - if err != nil { + if err := gocty.FromCtyValue(v, &connInfo.Port); err != nil { return nil, err } - connInfo.Port = p case "https": connInfo.HTTPS = v.True() case "insecure": diff --git a/terraform/node_resource_validate.go b/terraform/node_resource_validate.go index 0f6640849..071f8e9b2 100644 --- a/terraform/node_resource_validate.go +++ b/terraform/node_resource_validate.go @@ -177,7 +177,7 @@ var connectionBlockSupersetSchema = &configschema.Block{ Optional: true, }, "port": { - Type: cty.String, + Type: cty.Number, Optional: true, }, "timeout": { diff --git a/terraform/node_resource_validate_test.go b/terraform/node_resource_validate_test.go index 1c89d32e3..3d63bf222 100644 --- a/terraform/node_resource_validate_test.go +++ b/terraform/node_resource_validate_test.go @@ -31,6 +31,7 @@ func TestNodeValidatableResource_ValidateProvisioner_valid(t *testing.T) { Config: configs.SynthBody("", map[string]cty.Value{ "host": cty.StringVal("localhost"), "type": cty.StringVal("ssh"), + "port": cty.NumberIntVal(10022), }), }, } @@ -104,7 +105,7 @@ func TestNodeValidatableResource_ValidateProvisioner__warning(t *testing.T) { } } -func TestNodeValidatableResource_ValidateProvisioner__conntectionInvalid(t *testing.T) { +func TestNodeValidatableResource_ValidateProvisioner__connectionInvalid(t *testing.T) { ctx := &MockEvalContext{} ctx.installSimpleEval() mp := &MockProvisioner{}