builder/virtualbox: use new communicator abstraction

builder/virtualbox/common/ssh.go

@@ -17,13 +17,13 @@ func SSHAddress(state multistep.StateBag) (string, error) {
 func SSHConfigFunc(config SSHConfig) func(multistep.StateBag) (*gossh.ClientConfig, error) {
 	return func(state multistep.StateBag) (*gossh.ClientConfig, error) {
 		auth := []gossh.AuthMethod{
-			gossh.Password(config.SSHPassword),
+			gossh.Password(config.Comm.SSHPassword),
 			gossh.KeyboardInteractive(
-				ssh.PasswordKeyboardInteractive(config.SSHPassword)),
+				ssh.PasswordKeyboardInteractive(config.Comm.SSHPassword)),
 		}
 
 		if config.SSHKeyPath != "" {
-			signer, err := commonssh.FileSigner(config.SSHKeyPath)
+			signer, err := commonssh.FileSigner(config.Comm.SSHPrivateKey)
 			if err != nil {
 				return nil, err
 			}
@@ -32,7 +32,7 @@ func SSHConfigFunc(config SSHConfig) func(multistep.StateBag) (*gossh.ClientConf
 		}
 
 		return &gossh.ClientConfig{
-			User: config.SSHUser,
+			User: config.Comm.SSHUsername,
 			Auth: auth,
 		}, nil
 	}

builder/virtualbox/common/ssh_config.go

@@ -2,25 +2,23 @@ package common
 
 import (
 	"errors"
-	"fmt"
-	"os"
 	"time"
 
-	commonssh "github.com/mitchellh/packer/common/ssh"
+	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/template/interpolate"
 )
 
 type SSHConfig struct {
-	SSHHostPortMin    uint   `mapstructure:"ssh_host_port_min"`
+	Comm communicator.Config `mapstructure:",squash"`
-	SSHHostPortMax    uint   `mapstructure:"ssh_host_port_max"`
-	SSHKeyPath        string `mapstructure:"ssh_key_path"`
-	SSHPassword       string `mapstructure:"ssh_password"`
-	SSHPort           uint   `mapstructure:"ssh_port"`
-	SSHUser           string `mapstructure:"ssh_username"`
-	RawSSHWaitTimeout string `mapstructure:"ssh_wait_timeout"`
-	SSHSkipNatMapping bool   `mapstructure:"ssh_skip_nat_mapping"`
 
-	SSHWaitTimeout time.Duration
+	SSHHostPortMin    uint `mapstructure:"ssh_host_port_min"`
+	SSHHostPortMax    uint `mapstructure:"ssh_host_port_max"`
+	SSHSkipNatMapping bool `mapstructure:"ssh_skip_nat_mapping"`
+
+	// These are deprecated, but we keep them around for BC
+	// TODO(@mitchellh): remove
+	SSHKeyPath     string        `mapstructure:"ssh_key_path"`
+	SSHWaitTimeout time.Duration `mapstructure:"ssh_wait_timeout"`
 }
 
 func (c *SSHConfig) Prepare(ctx *interpolate.Context) []error {
@@ -32,37 +30,19 @@ func (c *SSHConfig) Prepare(ctx *interpolate.Context) []error {
 		c.SSHHostPortMax = 4444
 	}
 
-	if c.SSHPort == 0 {
+	// TODO: backwards compatibility, write fixer instead
-		c.SSHPort = 22
-	}
-
-	if c.RawSSHWaitTimeout == "" {
-		c.RawSSHWaitTimeout = "20m"
-	}
-
-	var errs []error
 	if c.SSHKeyPath != "" {
-		if _, err := os.Stat(c.SSHKeyPath); err != nil {
+		c.Comm.SSHPrivateKey = c.SSHKeyPath
-			errs = append(errs, fmt.Errorf("ssh_key_path is invalid: %s", err))
+	}
-		} else if _, err := commonssh.FileSigner(c.SSHKeyPath); err != nil {
+	if c.SSHWaitTimeout != 0 {
-			errs = append(errs, fmt.Errorf("ssh_key_path is invalid: %s", err))
+		c.Comm.SSHTimeout = c.SSHWaitTimeout
-		}
 	}
 
+	errs := c.Comm.Prepare(ctx)
 	if c.SSHHostPortMin > c.SSHHostPortMax {
 		errs = append(errs,
 			errors.New("ssh_host_port_min must be less than ssh_host_port_max"))
 	}
 
-	if c.SSHUser == "" {
-		errs = append(errs, errors.New("An ssh_username must be specified."))
-	}
-
-	var err error
-	c.SSHWaitTimeout, err = time.ParseDuration(c.RawSSHWaitTimeout)
-	if err != nil {
-		errs = append(errs, fmt.Errorf("Failed parsing ssh_wait_timeout: %s", err))
-	}
-
 	return errs
 }

builder/virtualbox/common/ssh_config_test.go

@@ -4,11 +4,15 @@ import (
 	"io/ioutil"
 	"os"
 	"testing"
+
+	"github.com/mitchellh/packer/helper/communicator"
 )
 
 func testSSHConfig() *SSHConfig {
 	return &SSHConfig{
-		SSHUser: "foo",
+		Comm: communicator.Config{
+			SSHUsername: "foo",
+		},
 	}
 }
 
@@ -27,8 +31,8 @@ func TestSSHConfigPrepare(t *testing.T) {
 		t.Errorf("bad max ssh host port: %d", c.SSHHostPortMax)
 	}
 
-	if c.SSHPort != 22 {
+	if c.Comm.SSHPort != 22 {
-		t.Errorf("bad ssh port: %d", c.SSHPort)
+		t.Errorf("bad ssh port: %d", c.Comm.SSHPort)
 	}
 }
 
@@ -109,46 +113,14 @@ func TestSSHConfigPrepare_SSHUser(t *testing.T) {
 	var errs []error
 
 	c = testSSHConfig()
-	c.SSHUser = ""
+	c.Comm.SSHUsername = ""
 	errs = c.Prepare(testConfigTemplate(t))
 	if len(errs) == 0 {
 		t.Fatalf("should have error")
 	}
 
 	c = testSSHConfig()
-	c.SSHUser = "exists"
+	c.Comm.SSHUsername = "exists"
-	errs = c.Prepare(testConfigTemplate(t))
-	if len(errs) > 0 {
-		t.Fatalf("should not have error: %#v", errs)
-	}
-}
-
-func TestSSHConfigPrepare_SSHWaitTimeout(t *testing.T) {
-	var c *SSHConfig
-	var errs []error
-
-	// Defaults
-	c = testSSHConfig()
-	c.RawSSHWaitTimeout = ""
-	errs = c.Prepare(testConfigTemplate(t))
-	if len(errs) > 0 {
-		t.Fatalf("should not have error: %#v", errs)
-	}
-	if c.RawSSHWaitTimeout != "20m" {
-		t.Fatalf("bad value: %s", c.RawSSHWaitTimeout)
-	}
-
-	// Test with a bad value
-	c = testSSHConfig()
-	c.RawSSHWaitTimeout = "this is not good"
-	errs = c.Prepare(testConfigTemplate(t))
-	if len(errs) == 0 {
-		t.Fatal("should have error")
-	}
-
-	// Test with a good one
-	c = testSSHConfig()
-	c.RawSSHWaitTimeout = "5s"
 	errs = c.Prepare(testConfigTemplate(t))
 	if len(errs) > 0 {
 		t.Fatalf("should not have error: %#v", errs)

builder/virtualbox/iso/builder.go

@@ -11,6 +11,7 @@ import (
 	"github.com/mitchellh/multistep"
 	vboxcommon "github.com/mitchellh/packer/builder/virtualbox/common"
 	"github.com/mitchellh/packer/common"
+	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/helper/config"
 	"github.com/mitchellh/packer/packer"
 	"github.com/mitchellh/packer/template/interpolate"
@@ -253,7 +254,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		},
 		new(vboxcommon.StepAttachFloppy),
 		&vboxcommon.StepForwardSSH{
-			GuestPort:      b.config.SSHPort,
+			GuestPort:      uint(b.config.SSHConfig.Comm.SSHPort),
 			HostPortMin:    b.config.SSHHostPortMin,
 			HostPortMax:    b.config.SSHHostPortMax,
 			SkipNatMapping: b.config.SSHSkipNatMapping,
@@ -271,10 +272,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			VMName:      b.config.VMName,
 			Ctx:         b.config.ctx,
 		},
-		&common.StepConnectSSH{
+		&communicator.StepConnect{
-			SSHAddress:     vboxcommon.SSHAddress,
+			Config:     &b.config.SSHConfig.Comm,
-			SSHConfig:      vboxcommon.SSHConfigFunc(b.config.SSHConfig),
+			SSHAddress: vboxcommon.SSHAddress,
-			SSHWaitTimeout: b.config.SSHWaitTimeout,
+			SSHConfig:  vboxcommon.SSHConfigFunc(b.config.SSHConfig),
 		},
 		&vboxcommon.StepUploadVersion{
 			Path: b.config.VBoxVersionFile,

builder/virtualbox/ovf/builder.go

@@ -10,6 +10,7 @@ import (
 	"github.com/mitchellh/multistep"
 	vboxcommon "github.com/mitchellh/packer/builder/virtualbox/common"
 	"github.com/mitchellh/packer/common"
+	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/packer"
 )
 
@@ -82,7 +83,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		},
 		new(vboxcommon.StepAttachFloppy),
 		&vboxcommon.StepForwardSSH{
-			GuestPort:      b.config.SSHPort,
+			GuestPort:      uint(b.config.SSHConfig.Comm.SSHPort),
 			HostPortMin:    b.config.SSHHostPortMin,
 			HostPortMax:    b.config.SSHHostPortMax,
 			SkipNatMapping: b.config.SSHSkipNatMapping,
@@ -100,10 +101,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			VMName:      b.config.VMName,
 			Ctx:         b.config.ctx,
 		},
-		&common.StepConnectSSH{
+		&communicator.StepConnect{
-			SSHAddress:     vboxcommon.SSHAddress,
+			Config:     &b.config.SSHConfig.Comm,
-			SSHConfig:      vboxcommon.SSHConfigFunc(b.config.SSHConfig),
+			SSHAddress: vboxcommon.SSHAddress,
-			SSHWaitTimeout: b.config.SSHWaitTimeout,
+			SSHConfig:  vboxcommon.SSHConfigFunc(b.config.SSHConfig),
 		},
 		&vboxcommon.StepUploadVersion{
 			Path: b.config.VBoxVersionFile,