core: Use number for port in connection schema

This commit makes two changes to the provisioner connection block code:

- Change the `port` argument type from string to number, which is
  technically more correct and consistent with `bastion_port`;
- Use `uint16` as the struct member type for both ports instead of
  `int`, which gets us free range validation from the gocty package.

Includes a test of the validation message when the port number is an
invalid integer.
This commit is contained in:
Alisdair McDiarmid 2021-05-10 13:57:11 -04:00
parent 0cbe4d8efb
commit fa676bde7c
8 changed files with 35 additions and 20 deletions

View File

@ -36,7 +36,7 @@ var ConnectionBlockSupersetSchema = &configschema.Block{
Optional: true, Optional: true,
}, },
"port": { "port": {
Type: cty.String, Type: cty.Number,
Optional: true, Optional: true,
}, },
"timeout": { "timeout": {

View File

@ -331,7 +331,7 @@ func TestHostKey(t *testing.T) {
Password: "pass", Password: "pass",
Host: host, Host: host,
HostKey: pubKey, HostKey: pubKey,
Port: port, Port: uint16(port),
Timeout: "30s", Timeout: "30s",
} }
@ -363,7 +363,7 @@ func TestHostKey(t *testing.T) {
port, _ = strconv.Atoi(p) port, _ = strconv.Atoi(p)
connInfo.HostKey = testClientPublicKey connInfo.HostKey = testClientPublicKey
connInfo.Port = port connInfo.Port = uint16(port)
cfg, err = prepareSSHConfig(connInfo) cfg, err = prepareSSHConfig(connInfo)
if err != nil { if err != nil {
@ -406,7 +406,7 @@ func TestHostCert(t *testing.T) {
Password: "pass", Password: "pass",
Host: host, Host: host,
HostKey: testCAPublicKey, HostKey: testCAPublicKey,
Port: port, Port: uint16(port),
Timeout: "30s", Timeout: "30s",
} }
@ -438,7 +438,7 @@ func TestHostCert(t *testing.T) {
port, _ = strconv.Atoi(p) port, _ = strconv.Atoi(p)
connInfo.HostKey = testClientPublicKey connInfo.HostKey = testClientPublicKey
connInfo.Port = port connInfo.Port = uint16(port)
cfg, err = prepareSSHConfig(connInfo) cfg, err = prepareSSHConfig(connInfo)
if err != nil { if err != nil {
@ -528,7 +528,7 @@ func TestCertificateBasedAuth(t *testing.T) {
Host: host, Host: host,
PrivateKey: CLIENT_PEM, PrivateKey: CLIENT_PEM,
Certificate: CLIENT_CERT_SIGNED_BY_SERVER, Certificate: CLIENT_CERT_SIGNED_BY_SERVER,
Port: port, Port: uint16(port),
Timeout: "30s", Timeout: "30s",
} }

View File

@ -10,7 +10,6 @@ import (
"net" "net"
"os" "os"
"path/filepath" "path/filepath"
"strconv"
"strings" "strings"
"time" "time"
@ -56,7 +55,7 @@ type connectionInfo struct {
Certificate string Certificate string
Host string Host string
HostKey string HostKey string
Port int Port uint16
Agent bool Agent bool
ScriptPath string ScriptPath string
TargetPlatform string TargetPlatform string
@ -69,7 +68,7 @@ type connectionInfo struct {
BastionCertificate string BastionCertificate string
BastionHost string BastionHost string
BastionHostKey string BastionHostKey string
BastionPort int BastionPort uint16
AgentIdentity string AgentIdentity string
} }
@ -102,11 +101,9 @@ func decodeConnInfo(v cty.Value) (*connectionInfo, error) {
case "host_key": case "host_key":
connInfo.HostKey = v.AsString() connInfo.HostKey = v.AsString()
case "port": case "port":
p, err := strconv.Atoi(v.AsString()) if err := gocty.FromCtyValue(v, &connInfo.Port); err != nil {
if err != nil {
return nil, err return nil, err
} }
connInfo.Port = p
case "agent": case "agent":
connInfo.Agent = v.True() connInfo.Agent = v.True()
case "script_path": case "script_path":

View File

@ -159,3 +159,22 @@ func TestProvisioner_stringBastionPort(t *testing.T) {
t.Fatalf("bad %v", conf) 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)
}
}

View File

@ -33,7 +33,7 @@ func New(v cty.Value) (*Communicator, error) {
endpoint := &winrm.Endpoint{ endpoint := &winrm.Endpoint{
Host: connInfo.Host, Host: connInfo.Host,
Port: connInfo.Port, Port: int(connInfo.Port),
HTTPS: connInfo.HTTPS, HTTPS: connInfo.HTTPS,
Insecure: connInfo.Insecure, Insecure: connInfo.Insecure,
Timeout: connInfo.TimeoutVal, Timeout: connInfo.TimeoutVal,

View File

@ -4,12 +4,12 @@ import (
"fmt" "fmt"
"log" "log"
"path/filepath" "path/filepath"
"strconv"
"strings" "strings"
"time" "time"
"github.com/hashicorp/terraform/communicator/shared" "github.com/hashicorp/terraform/communicator/shared"
"github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/gocty"
) )
const ( const (
@ -37,7 +37,7 @@ type connectionInfo struct {
User string User string
Password string Password string
Host string Host string
Port int Port uint16
HTTPS bool HTTPS bool
Insecure bool Insecure bool
NTLM bool `mapstructure:"use_ntlm"` NTLM bool `mapstructure:"use_ntlm"`
@ -69,11 +69,9 @@ func decodeConnInfo(v cty.Value) (*connectionInfo, error) {
case "host": case "host":
connInfo.Host = v.AsString() connInfo.Host = v.AsString()
case "port": case "port":
p, err := strconv.Atoi(v.AsString()) if err := gocty.FromCtyValue(v, &connInfo.Port); err != nil {
if err != nil {
return nil, err return nil, err
} }
connInfo.Port = p
case "https": case "https":
connInfo.HTTPS = v.True() connInfo.HTTPS = v.True()
case "insecure": case "insecure":

View File

@ -177,7 +177,7 @@ var connectionBlockSupersetSchema = &configschema.Block{
Optional: true, Optional: true,
}, },
"port": { "port": {
Type: cty.String, Type: cty.Number,
Optional: true, Optional: true,
}, },
"timeout": { "timeout": {

View File

@ -31,6 +31,7 @@ func TestNodeValidatableResource_ValidateProvisioner_valid(t *testing.T) {
Config: configs.SynthBody("", map[string]cty.Value{ Config: configs.SynthBody("", map[string]cty.Value{
"host": cty.StringVal("localhost"), "host": cty.StringVal("localhost"),
"type": cty.StringVal("ssh"), "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 := &MockEvalContext{}
ctx.installSimpleEval() ctx.installSimpleEval()
mp := &MockProvisioner{} mp := &MockProvisioner{}