Merge branch 'master' into f-file-builder

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 BACKWARDS INCOMPATIBILITIES:
 
+  * core: SSH connection will no longer request a PTY by default. This
+      can be enabled per builder.
   * builder/digitalocean: no longer supports the v1 API which has been
       deprecated for some time. Most configurations should continue to
       work as long as you use the `api_token` field for auth.
@@ -11,12 +13,32 @@ BACKWARDS INCOMPATIBILITIES:
 
 FEATURES:
 
+  * **WinRM:** You can now connect via WinRM with almost every builder.
+      See the docs for more info. [GH-2239]
+  * **Windows AWS Support:** Windows AMIs can now be built without any
+      external plugins: Packer will start a Windows instance, get the
+      admin password, and can use WinRM (above) to connect through. [GH-2240]
+  * **Disable SSH:** Set `communicator` to "none" in any builder to disable SSH
+      connections. Note that provisioners won't work if this is done. [GH-1591]
+  * **SSH Agent Forwarding:** SSH Agent Forwarding will now be enabled
+      to allow access to remote servers such as private git repos. [GH-1066]
+  * **Docker builder supports SSH**: The Docker builder now supports containers
+      with SSH, just set `communicator` to "ssh" [GH-2244]
+  * **New config function: `build_name`**: The name of the currently running
+      build. [GH-2232]
+  * **New config function: `build_type`**: The type of the currently running
+      builder. This is useful for provisioners. [GH-2232]
   * **New config function: `template_dir`**: The directory to the template
       being built. This should be used for template-relative paths. [GH-54]
 
 IMPROVEMENTS:
 
   * core: Interrupt handling for SIGTERM signal as well. [GH-1858]
+  * builder/*: Add `ssh_handshake_attempts` to configure the number of
+      handshake attempts done before failure [GH-2237]
+  * builder/amazon: Add `force_deregister` option for automatic AMI
+      deregistration [GH-2221]
+  * builder/amazon: Now applies tags to EBS snapshots [GH-2212]
   * builder/digitalocean: Save SSH key to pwd if debug mode is on. [GH-1829]
   * builder/digitalocean: User data support [GH-2113]
   * builder/parallels: Support Parallels Desktop 11 [GH-2199]
@@ -26,10 +48,14 @@ IMPROVEMENTS:
       have prohibitive firewalls
   * builder/openstack: Flavor names can be used as well as refs
   * builder/openstack: Add `availability_zone` [GH-2016]
+  * builder/openstack: Machine will be stopped prior to imaging if the
+      cluster supports the `startstop` extension. [GH-2223]
+  * builder/openstack: Support for user data [GH-2224]
   * builder/virtualbox: Added option: `ssh_skip_nat_mapping` to skip the
       automatic port forward for SSH and to use the guest port directly. [GH-1078]
   * builder/virtualbox: Added SCSI support
   * builder/vmware: Support for additional disks [GH-1382]
+  * command/fix: After fixing, the template is validated [GH-2228]
   * command/push: Add `-name` flag for specifying name from CLI [GH-2042]
   * command/push: Push configuration in templates supports variables [GH-1861]
   * post-processor/docker-save: Can be chained [GH-2179]
@@ -39,6 +65,7 @@ IMPROVEMENTS:
 BUG FIXES:
 
   * core: Fix potential panic for post-processor plugin exits [GH-2098]
+  * core: `PACKER_CONFIG` may point to a non-existent file [GH-2226]
   * builder/amazon: Allow spaces in AMI names when using `clean_ami_name` [GH-2182]
   * builder/amazon: Remove deprecated ec2-upload-bundle paramger [GH-1931]
   * builder/amazon: Use IAM Profile to upload bundle if provided [GH-1985]
@@ -66,13 +93,18 @@ BUG FIXES:
   * builder/docker: Fix crash that could occur at certain timed ctrl-c [GH-1838]
   * builder/docker: validate that `export_path` is not a directory [GH-2105]
   * builder/google: `ssh_timeout` is respected [GH-1781]
+  * builder/openstack: `ssh_interface` can be used to specify the interface
+      to retrieve the SSH IP from. [GH-2220]
   * builder/qemu: Add `disk_discard` option [GH-2120]
+  * builder/qemu: Use proper SSH port, not hardcoded to 22. [GH-2236]
   * builder/virtualbox: Bind HTTP server to IPv4, which is more compatible with
       OS installers. [GH-1709]
   * builder/virtualbox: Remove the floppy controller in addition to the
       floppy disk. [GH-1879]
   * builder/virtualbox: Fixed regression where downloading ISO without a
       ".iso" extension didn't work. [GH-1839]
+  * builder/virtualbox: Output dir is verified at runtime, not template
+      validation time. [GH-2233]
   * builder/vmware: Add 100ms delay between keystrokes to avoid subtle
       timing issues in most cases. [GH-1663]
   * builder/vmware: Bind HTTP server to IPv4, which is more compatible with
@@ -80,6 +112,10 @@ BUG FIXES:
   * builder/vmware: Case-insensitive match of MAC address to find IP [GH-1989]
   * builder/vmware: More robust IP parsing from ifconfig output [GH-1999]
   * builder/vmware: Nested output directories for ESXi work [GH-2174]
+  * builder/vmware: Output dir is verified at runtime, not template
+      validation time. [GH-2233]
+  * command/fix: For the `virtualbox` to `virtualbox-iso` builder rename,
+      provisioner overrides are now also fixed [GH-2231]
   * command/validate: don't crash for invalid builds [GH-2139]
   * post-processor/atlas: Find common archive prefix for Windows [GH-1874]
   * post-processor/atlas: Fix index out of range panic [GH-1959]
@@ -89,6 +125,7 @@ BUG FIXES:
   * provisioner/salt-masterless: Add `--retcode-passthrough` to salt-call
   * provisioner/shell: chmod executable script to 0755, not 0777 [GH-1708]
   * provisioner/shell: inline commands failing will fail the provisioner [GH-2069]
+  * provisioner/shell: single quotes in env vars are escaped [GH-2229]
 
 ## 0.7.5 (December 9, 2014)
 

builder/amazon/chroot/builder.go

@@ -147,6 +147,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 
 	// Build the steps
 	steps := []multistep.Step{
+		&awscommon.StepPreValidate{
+			DestAmiName:     b.config.AMIName,
+			ForceDeregister: b.config.AMIForceDeregister,
+		},
 		&StepInstanceInfo{},
 		&awscommon.StepSourceAMIInfo{
 			SourceAmi:          b.config.SourceAmi,
@@ -164,6 +168,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		&StepChrootProvision{},
 		&StepEarlyCleanup{},
 		&StepSnapshot{},
+		&awscommon.StepDeregisterAMI{
+			ForceDeregister: b.config.AMIForceDeregister,
+			AMIName:         b.config.AMIName,
+		},
 		&StepRegisterAMI{},
 		&awscommon.StepAMIRegionCopy{
 			AccessConfig: &b.config.AccessConfig,

builder/amazon/common/ami_config.go

@@ -17,6 +17,7 @@ type AMIConfig struct {
 	AMIRegions            []string          `mapstructure:"ami_regions"`
 	AMITags               map[string]string `mapstructure:"tags"`
 	AMIEnhancedNetworking bool              `mapstructure:"enhanced_networking"`
+	AMIForceDeregister    bool              `mapstructure:"force_deregister"`
 }
 
 func (c *AMIConfig) Prepare(ctx *interpolate.Context) []error {

builder/amazon/common/run_config.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/mitchellh/packer/common/uuid"
+	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/template/interpolate"
 )
 
@@ -21,40 +22,32 @@ type RunConfig struct {
 	SourceAmi                string            `mapstructure:"source_ami"`
 	SpotPrice                string            `mapstructure:"spot_price"`
 	SpotPriceAutoProduct     string            `mapstructure:"spot_price_auto_product"`
-	RawSSHTimeout            string            `mapstructure:"ssh_timeout"`
-	SSHUsername              string            `mapstructure:"ssh_username"`
-	SSHPrivateKeyFile        string            `mapstructure:"ssh_private_key_file"`
-	SSHPrivateIp             bool              `mapstructure:"ssh_private_ip"`
-	SSHPort                  int               `mapstructure:"ssh_port"`
 	SecurityGroupId          string            `mapstructure:"security_group_id"`
 	SecurityGroupIds         []string          `mapstructure:"security_group_ids"`
 	SubnetId                 string            `mapstructure:"subnet_id"`
 	TemporaryKeyPairName     string            `mapstructure:"temporary_key_pair_name"`
 	UserData                 string            `mapstructure:"user_data"`
 	UserDataFile             string            `mapstructure:"user_data_file"`
+	WindowsPasswordTimeout   time.Duration     `mapstructure:"windows_password_timeout"`
 	VpcId                    string            `mapstructure:"vpc_id"`
 
-	// Unexported fields that are calculated from others
-	sshTimeout time.Duration
+	// Communicator settings
+	Comm         communicator.Config `mapstructure:",squash"`
+	SSHPrivateIp bool                `mapstructure:"ssh_private_ip"`
 }
 
 func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
-	// Defaults
-	if c.SSHPort == 0 {
-		c.SSHPort = 22
-	}
-
-	if c.RawSSHTimeout == "" {
-		c.RawSSHTimeout = "5m"
-	}
-
 	if c.TemporaryKeyPairName == "" {
 		c.TemporaryKeyPairName = fmt.Sprintf(
 			"packer %s", uuid.TimeOrderedUUID())
 	}
 
+	if c.WindowsPasswordTimeout == 0 {
+		c.WindowsPasswordTimeout = 10 * time.Minute
+	}
+
 	// Validation
-	var errs []error
+	errs := c.Comm.Prepare(ctx)
 	if c.SourceAmi == "" {
 		errs = append(errs, errors.New("A source_ami must be specified"))
 	}
@@ -70,10 +63,6 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
 		}
 	}
 
-	if c.SSHUsername == "" {
-		errs = append(errs, errors.New("An ssh_username must be specified"))
-	}
-
 	if c.UserData != "" && c.UserDataFile != "" {
 		errs = append(errs, fmt.Errorf("Only one of user_data or user_data_file can be specified."))
 	} else if c.UserDataFile != "" {
@@ -91,15 +80,5 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
 		}
 	}
 
-	var err error
-	c.sshTimeout, err = time.ParseDuration(c.RawSSHTimeout)
-	if err != nil {
-		errs = append(errs, fmt.Errorf("Failed parsing ssh_timeout: %s", err))
-	}
-
 	return errs
 }
-
-func (c *RunConfig) SSHTimeout() time.Duration {
-	return c.sshTimeout
-}

builder/amazon/common/run_config_test.go

@@ -4,6 +4,8 @@ import (
 	"io/ioutil"
 	"os"
 	"testing"
+
+	"github.com/mitchellh/packer/helper/communicator"
 )
 
 func init() {
@@ -19,7 +21,10 @@ func testConfig() *RunConfig {
 	return &RunConfig{
 		SourceAmi:    "abcd",
 		InstanceType: "m1.small",
-		SSHUsername:  "root",
+
+		Comm: communicator.Config{
+			SSHUsername: "foo",
+		},
 	}
 }
 
@@ -62,41 +67,28 @@ func TestRunConfigPrepare_SpotAuto(t *testing.T) {
 
 func TestRunConfigPrepare_SSHPort(t *testing.T) {
 	c := testConfig()
-	c.SSHPort = 0
-	if err := c.Prepare(nil); len(err) != 0 {
-		t.Fatalf("err: %s", err)
-	}
-
-	if c.SSHPort != 22 {
-		t.Fatalf("invalid value: %d", c.SSHPort)
-	}
-
-	c.SSHPort = 44
+	c.Comm.SSHPort = 0
 	if err := c.Prepare(nil); len(err) != 0 {
 		t.Fatalf("err: %s", err)
 	}
 
-	if c.SSHPort != 44 {
-		t.Fatalf("invalid value: %d", c.SSHPort)
+	if c.Comm.SSHPort != 22 {
+		t.Fatalf("invalid value: %d", c.Comm.SSHPort)
 	}
-}
 
-func TestRunConfigPrepare_SSHTimeout(t *testing.T) {
-	c := testConfig()
-	c.RawSSHTimeout = ""
+	c.Comm.SSHPort = 44
 	if err := c.Prepare(nil); len(err) != 0 {
 		t.Fatalf("err: %s", err)
 	}
 
-	c.RawSSHTimeout = "bad"
-	if err := c.Prepare(nil); len(err) != 1 {
-		t.Fatalf("err: %s", err)
+	if c.Comm.SSHPort != 44 {
+		t.Fatalf("invalid value: %d", c.Comm.SSHPort)
 	}
 }
 
 func TestRunConfigPrepare_SSHUsername(t *testing.T) {
 	c := testConfig()
-	c.SSHUsername = ""
+	c.Comm.SSHUsername = ""
 	if err := c.Prepare(nil); len(err) != 1 {
 		t.Fatalf("err: %s", err)
 	}

builder/amazon/common/ssh.go

@@ -10,9 +10,9 @@ import (
 	"golang.org/x/crypto/ssh"
 )
 
-// SSHAddress returns a function that can be given to the SSH communicator
+// SSHHost returns a function that can be given to the SSH communicator
 // for determining the SSH address based on the instance DNS name.
-func SSHAddress(e *ec2.EC2, port int, private bool) func(multistep.StateBag) (string, error) {
+func SSHHost(e *ec2.EC2, private bool) func(multistep.StateBag) (string, error) {
 	return func(state multistep.StateBag) (string, error) {
 		for j := 0; j < 2; j++ {
 			var host string
@@ -28,7 +28,7 @@ func SSHAddress(e *ec2.EC2, port int, private bool) func(multistep.StateBag) (st
 			}
 
 			if host != "" {
-				return fmt.Sprintf("%s:%d", host, port), nil
+				return host, nil
 			}
 
 			r, err := e.DescribeInstances(&ec2.DescribeInstancesInput{

builder/amazon/common/step_create_tags.go

@@ -25,19 +25,56 @@ func (s *StepCreateTags) Run(state multistep.StateBag) multistep.StepAction {
 			var ec2Tags []*ec2.Tag
 			for key, value := range s.Tags {
 				ui.Message(fmt.Sprintf("Adding tag: \"%s\": \"%s\"", key, value))
-				ec2Tags = append(ec2Tags, &ec2.Tag{Key: &key, Value: &value})
+				ec2Tags = append(ec2Tags, &ec2.Tag{
+					Key:   aws.String(key),
+					Value: aws.String(value),
+				})
+			}
+
+			// Declare list of resources to tag
+			resourceIds := []*string{&ami}
+
+			// Retrieve image list for given AMI
+			imageResp, err := ec2conn.DescribeImages(&ec2.DescribeImagesInput{
+				ImageIDs: resourceIds,
+			})
+
+			if err != nil {
+				err := fmt.Errorf("Error retrieving details for AMI (%s): %s", ami, err)
+				state.Put("error", err)
+				ui.Error(err.Error())
+				return multistep.ActionHalt
+			}
+
+			if len(imageResp.Images) == 0 {
+				err := fmt.Errorf("Error retrieving details for AMI (%s), no images found", ami)
+				state.Put("error", err)
+				ui.Error(err.Error())
+				return multistep.ActionHalt
+			}
+
+			image := imageResp.Images[0]
+
+			// Add only those with a Snapshot ID, i.e. not Ephemeral
+			for _, device := range image.BlockDeviceMappings {
+				if device.EBS != nil && device.EBS.SnapshotID != nil {
+					ui.Say(fmt.Sprintf("Tagging snapshot: %s", *device.EBS.SnapshotID))
+					resourceIds = append(resourceIds, device.EBS.SnapshotID)
+				}
 			}
 
 			regionconn := ec2.New(&aws.Config{
 				Credentials: ec2conn.Config.Credentials,
 				Region:      region,
 			})
-			_, err := regionconn.CreateTags(&ec2.CreateTagsInput{
-				Resources: []*string{&ami},
+
+			_, err = regionconn.CreateTags(&ec2.CreateTagsInput{
+				Resources: resourceIds,
 				Tags:      ec2Tags,
 			})
+
 			if err != nil {
-				err := fmt.Errorf("Error adding tags to AMI (%s): %s", ami, err)
+				err := fmt.Errorf("Error adding tags to Resources (%#v): %s", resourceIds, err)
 				state.Put("error", err)
 				ui.Error(err.Error())
 				return multistep.ActionHalt

builder/amazon/common/step_deregister_ami.go

+package common
+
+import (
+	"fmt"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/packer"
+)
+
+type StepDeregisterAMI struct {
+	ForceDeregister bool
+	AMIName         string
+}
+
+func (s *StepDeregisterAMI) Run(state multistep.StateBag) multistep.StepAction {
+	ec2conn := state.Get("ec2").(*ec2.EC2)
+	ui := state.Get("ui").(packer.Ui)
+
+	// check for force deregister
+	if s.ForceDeregister {
+		resp, err := ec2conn.DescribeImages(&ec2.DescribeImagesInput{
+			Filters: []*ec2.Filter{&ec2.Filter{
+				Name:   aws.String("name"),
+				Values: []*string{aws.String(s.AMIName)},
+			}}})
+
+		if err != nil {
+			err := fmt.Errorf("Error creating AMI: %s", err)
+			state.Put("error", err)
+			ui.Error(err.Error())
+			return multistep.ActionHalt
+		}
+
+		// deregister image(s) by that name
+		for _, i := range resp.Images {
+			_, err := ec2conn.DeregisterImage(&ec2.DeregisterImageInput{
+				ImageID: i.ImageID,
+			})
+
+			if err != nil {
+				err := fmt.Errorf("Error deregistering existing AMI: %s", err)
+				state.Put("error", err)
+				ui.Error(err.Error())
+				return multistep.ActionHalt
+			}
+			ui.Say(fmt.Sprintf("Deregistered AMI %s, id: %s", s.AMIName, *i.ImageID))
+		}
+	}
+
+	return multistep.ActionContinue
+}
+
+func (s *StepDeregisterAMI) Cleanup(state multistep.StateBag) {
+}

builder/amazon/common/step_get_password.go

+package common
+
+import (
+	"crypto/rsa"
+	"crypto/x509"
+	"encoding/base64"
+	"encoding/pem"
+	"errors"
+	"fmt"
+	"log"
+	"time"
+
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/helper/communicator"
+	"github.com/mitchellh/packer/packer"
+)
+
+// StepGetPassword reads the password from a Windows server and sets it
+// on the WinRM config.
+type StepGetPassword struct {
+	Comm    *communicator.Config
+	Timeout time.Duration
+}
+
+func (s *StepGetPassword) Run(state multistep.StateBag) multistep.StepAction {
+	ui := state.Get("ui").(packer.Ui)
+	image := state.Get("source_image").(*ec2.Image)
+
+	// Skip if we're not Windows...
+	if image.Platform == nil || *image.Platform != "windows" {
+		log.Printf("[INFO] Not Windows, skipping get password...")
+		return multistep.ActionContinue
+	}
+
+	// If we already have a password, skip it
+	if s.Comm.WinRMPassword != "" {
+		ui.Say("Skipping waiting for password since WinRM password set...")
+		return multistep.ActionContinue
+	}
+
+	// Get the password
+	var password string
+	var err error
+	cancel := make(chan struct{})
+	waitDone := make(chan bool, 1)
+	go func() {
+		ui.Say("Waiting for auto-generated password for instance...")
+		ui.Message(
+			"It is normal for this process to take up to 15 minutes,\n" +
+				"but it usually takes around 5. Please wait.")
+		password, err = s.waitForPassword(state, cancel)
+		waitDone <- true
+	}()
+
+	timeout := time.After(s.Timeout)
+WaitLoop:
+	for {
+		// Wait for either SSH to become available, a timeout to occur,
+		// or an interrupt to come through.
+		select {
+		case <-waitDone:
+			if err != nil {
+				ui.Error(fmt.Sprintf("Error waiting for password: %s", err))
+				state.Put("error", err)
+				return multistep.ActionHalt
+			}
+
+			ui.Message(fmt.Sprintf(" \nPassword retrieved!"))
+			s.Comm.WinRMPassword = password
+			break WaitLoop
+		case <-timeout:
+			err := fmt.Errorf("Timeout waiting for password.")
+			state.Put("error", err)
+			ui.Error(err.Error())
+			close(cancel)
+			return multistep.ActionHalt
+		case <-time.After(1 * time.Second):
+			if _, ok := state.GetOk(multistep.StateCancelled); ok {
+				// The step sequence was cancelled, so cancel waiting for password
+				// and just start the halting process.
+				close(cancel)
+				log.Println("[WARN] Interrupt detected, quitting waiting for password.")
+				return multistep.ActionHalt
+			}
+		}
+	}
+	return multistep.ActionContinue
+}
+
+func (s *StepGetPassword) Cleanup(multistep.StateBag) {}
+
+func (s *StepGetPassword) waitForPassword(state multistep.StateBag, cancel <-chan struct{}) (string, error) {
+	ec2conn := state.Get("ec2").(*ec2.EC2)
+	instance := state.Get("instance").(*ec2.Instance)
+	privateKey := state.Get("privateKey").(string)
+
+	for {
+		select {
+		case <-cancel:
+			log.Println("[INFO] Retrieve password wait cancelled. Exiting loop.")
+			return "", errors.New("Retrieve password wait cancelled")
+		case <-time.After(5 * time.Second):
+		}
+
+		resp, err := ec2conn.GetPasswordData(&ec2.GetPasswordDataInput{
+			InstanceID: instance.InstanceID,
+		})
+		if err != nil {
+			err := fmt.Errorf("Error retrieving auto-generated instance password: %s", err)
+			return "", err
+		}
+
+		if resp.PasswordData != nil && *resp.PasswordData != "" {
+			decryptedPassword, err := decryptPasswordDataWithPrivateKey(
+				*resp.PasswordData, []byte(privateKey))
+			if err != nil {
+				err := fmt.Errorf("Error decrypting auto-generated instance password: %s", err)
+				return "", err
+			}
+
+			return decryptedPassword, nil
+		}
+
+		log.Printf("[DEBUG] Password is blank, will retry...")
+	}
+}
+
+func decryptPasswordDataWithPrivateKey(passwordData string, pemBytes []byte) (string, error) {
+	encryptedPasswd, err := base64.StdEncoding.DecodeString(passwordData)
+	if err != nil {
+		return "", err
+	}
+
+	block, _ := pem.Decode(pemBytes)
+	var asn1Bytes []byte
+	if _, ok := block.Headers["DEK-Info"]; ok {
+		return "", errors.New("encrypted private key isn't yet supported")
+		/*
+			asn1Bytes, err = x509.DecryptPEMBlock(block, password)
+			if err != nil {
+				return "", err
+			}
+		*/
+	} else {
+		asn1Bytes = block.Bytes
+	}
+
+	key, err := x509.ParsePKCS1PrivateKey(asn1Bytes)
+	if err != nil {
+		return "", err
+	}
+
+	out, err := rsa.DecryptPKCS1v15(nil, key, encryptedPasswd)
+	if err != nil {
+		return "", err
+	}
+
+	return string(out), nil
+}

builder/amazon/common/step_pre_validate.go

@@ -14,11 +14,17 @@ import (
 //
 type StepPreValidate struct {
 	DestAmiName     string
+	ForceDeregister bool
 }
 
 func (s *StepPreValidate) Run(state multistep.StateBag) multistep.StepAction {
-	ec2conn := state.Get("ec2").(*ec2.EC2)
 	ui := state.Get("ui").(packer.Ui)
+	if s.ForceDeregister {
+		ui.Say("Force Deregister flag found, skipping prevalidating AMI Name")
+		return multistep.ActionContinue
+	}
+
+	ec2conn := state.Get("ec2").(*ec2.EC2)
 
 	ui.Say("Prevalidating AMI Name...")
 	resp, err := ec2conn.DescribeImages(&ec2.DescribeImagesInput{

builder/amazon/common/step_run_source_instance.go

 package common
 
 import (
+	"encoding/base64"
 	"fmt"
 	"io/ioutil"
 	"log"
@@ -53,7 +54,14 @@ func (s *StepRunSourceInstance) Run(state multistep.StateBag) multistep.StepActi
 			return multistep.ActionHalt
 		}
 
+		// Test if it is encoded already, and if not, encode it
+		if _, err := base64.StdEncoding.DecodeString(string(contents)); err != nil {
+			log.Printf("[DEBUG] base64 encoding user data...")
+			contents = []byte(base64.StdEncoding.EncodeToString(contents))
+		}
+
 		userData = string(contents)
+
 	}
 
 	ui.Say("Launching a source AWS instance...")
@@ -174,11 +182,15 @@ func (s *StepRunSourceInstance) Run(state multistep.StateBag) multistep.StepActi
 				ImageID:            &s.SourceAMI,
 				InstanceType:       &s.InstanceType,
 				UserData:           &userData,
-				SecurityGroupIDs:   securityGroupIds,
 				IAMInstanceProfile: &ec2.IAMInstanceProfileSpecification{Name: &s.IamInstanceProfile},
-				SubnetID:           &s.SubnetId,
 				NetworkInterfaces: []*ec2.InstanceNetworkInterfaceSpecification{
-					&ec2.InstanceNetworkInterfaceSpecification{AssociatePublicIPAddress: &s.AssociatePublicIpAddress},
+					&ec2.InstanceNetworkInterfaceSpecification{
+						DeviceIndex:              aws.Long(0),
+						AssociatePublicIPAddress: &s.AssociatePublicIpAddress,
+						SubnetID:                 &s.SubnetId,
+						Groups:                   securityGroupIds,
+						DeleteOnTermination:      aws.Boolean(true),
+					},
 				},
 				Placement: &ec2.SpotPlacement{
 					AvailabilityZone: &availabilityZone,

builder/amazon/common/step_security_group.go

@@ -9,12 +9,13 @@ import (
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/common/uuid"
+	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/packer"
 )
 
 type StepSecurityGroup struct {
+	CommConfig       *communicator.Config
 	SecurityGroupIds []string
-	SSHPort          int
 	VpcId            string
 
 	createdGroupId string
@@ -30,8 +31,9 @@ func (s *StepSecurityGroup) Run(state multistep.StateBag) multistep.StepAction {
 		return multistep.ActionContinue
 	}
 
-	if s.SSHPort == 0 {
-		panic("SSHPort must be set to a non-zero value.")
+	port := s.CommConfig.Port()
+	if port == 0 {
+		panic("port must be set to a non-zero value.")
 	}
 
 	// Create the group
@@ -57,15 +59,17 @@ func (s *StepSecurityGroup) Run(state multistep.StateBag) multistep.StepAction {
 	req := &ec2.AuthorizeSecurityGroupIngressInput{
 		GroupID:    groupResp.GroupID,
 		IPProtocol: aws.String("tcp"),
-		FromPort:   aws.Long(int64(s.SSHPort)),
-		ToPort:     aws.Long(int64(s.SSHPort)),
+		FromPort:   aws.Long(int64(port)),
+		ToPort:     aws.Long(int64(port)),
 		CIDRIP:     aws.String("0.0.0.0/0"),
 	}
 
 	// We loop and retry this a few times because sometimes the security
 	// group isn't available immediately because AWS resources are eventaully
 	// consistent.
-	ui.Say("Authorizing SSH access on the temporary security group...")
+	ui.Say(fmt.Sprintf(
+		"Authorizing access to port %d the temporary security group...",
+		port))
 	for i := 0; i < 5; i++ {
 		_, err = ec2conn.AuthorizeSecurityGroupIngress(req)
 		if err == nil {

builder/amazon/ebs/builder.go

@@ -13,6 +13,7 @@ import (
 	"github.com/mitchellh/multistep"
 	awscommon "github.com/mitchellh/packer/builder/amazon/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"
@@ -80,6 +81,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 	steps := []multistep.Step{
 		&awscommon.StepPreValidate{
 			DestAmiName:     b.config.AMIName,
+			ForceDeregister: b.config.AMIForceDeregister,
 		},
 		&awscommon.StepSourceAMIInfo{
 			SourceAmi:          b.config.SourceAmi,
@@ -89,11 +91,11 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			Debug:          b.config.PackerDebug,
 			DebugKeyPath:   fmt.Sprintf("ec2_%s.pem", b.config.PackerBuildName),
 			KeyPairName:    b.config.TemporaryKeyPairName,
-			PrivateKeyFile: b.config.SSHPrivateKeyFile,
+			PrivateKeyFile: b.config.RunConfig.Comm.SSHPrivateKey,
 		},
 		&awscommon.StepSecurityGroup{
 			SecurityGroupIds: b.config.SecurityGroupIds,
-			SSHPort:          b.config.SSHPort,
+			CommConfig:       &b.config.RunConfig.Comm,
 			VpcId:            b.config.VpcId,
 		},
 		&awscommon.StepRunSourceInstance{
@@ -112,16 +114,26 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			BlockDevices:             b.config.BlockDevices,
 			Tags:                     b.config.RunTags,
 		},
-		&common.StepConnectSSH{
-			SSHAddress: awscommon.SSHAddress(
-				ec2conn, b.config.SSHPort, b.config.SSHPrivateIp),
-			SSHConfig:      awscommon.SSHConfig(b.config.SSHUsername),
-			SSHWaitTimeout: b.config.SSHTimeout(),
+		&awscommon.StepGetPassword{
+			Comm:    &b.config.RunConfig.Comm,
+			Timeout: b.config.WindowsPasswordTimeout,
+		},
+		&communicator.StepConnect{
+			Config: &b.config.RunConfig.Comm,
+			Host: awscommon.SSHHost(
+				ec2conn,
+				b.config.SSHPrivateIp),
+			SSHConfig: awscommon.SSHConfig(
+				b.config.RunConfig.Comm.SSHUsername),
 		},
 		&common.StepProvision{},
 		&stepStopInstance{SpotPrice: b.config.SpotPrice},
 		// TODO(mitchellh): verify works with spots
 		&stepModifyInstance{},
+		&awscommon.StepDeregisterAMI{
+			ForceDeregister: b.config.AMIForceDeregister,
+			AMIName:         b.config.AMIName,
+		},
 		&stepCreateAMI{},
 		&awscommon.StepAMIRegionCopy{
 			AccessConfig: &b.config.AccessConfig,

builder/amazon/ebs/builder_acc_test.go

@@ -28,6 +28,22 @@ func TestBuilderAcc_regionCopy(t *testing.T) {
 	})
 }
 
+func TestBuilderAcc_forceDeregister(t *testing.T) {
+	// Build the same AMI name twice, with force_deregister on the second run
+	builderT.Test(t, builderT.TestCase{
+		PreCheck:             func() { testAccPreCheck(t) },
+		Builder:              &Builder{},
+		Template:             buildForceDeregisterConfig("false", "dereg"),
+		SkipArtifactTeardown: true,
+	})
+
+	builderT.Test(t, builderT.TestCase{
+		PreCheck: func() { testAccPreCheck(t) },
+		Builder:  &Builder{},
+		Template: buildForceDeregisterConfig("true", "dereg"),
+	})
+}
+
 func checkRegionCopy(regions []string) builderT.TestCheckFunc {
 	return func(artifacts []packer.Artifact) error {
 		if len(artifacts) > 1 {
@@ -107,3 +123,21 @@ const testBuilderAccRegionCopy = `
 	}]
 }
 `
+
+const testBuilderAccForceDeregister = `
+{
+	"builders": [{
+		"type": "test",
+		"region": "us-east-1",
+		"instance_type": "m3.medium",
+		"source_ami": "ami-76b2a71e",
+		"ssh_username": "ubuntu",
+		"force_deregister": "%s",
+		"ami_name": "packer-test-%s"
+	}]
+}
+`
+
+func buildForceDeregisterConfig(name, flag string) string {
+	return fmt.Sprintf(testBuilderAccForceDeregister, name, flag)
+}

builder/amazon/ebs/tags_acc_test.go

+package ebs
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/mitchellh/packer/builder/amazon/common"
+	builderT "github.com/mitchellh/packer/helper/builder/testing"
+	"github.com/mitchellh/packer/packer"
+)
+
+func TestBuilderTagsAcc_basic(t *testing.T) {
+	builderT.Test(t, builderT.TestCase{
+		PreCheck: func() { testAccPreCheck(t) },
+		Builder:  &Builder{},
+		Template: testBuilderTagsAccBasic,
+		Check:    checkTags(),
+	})
+}
+
+func checkTags() builderT.TestCheckFunc {
+	return func(artifacts []packer.Artifact) error {
+		if len(artifacts) > 1 {
+			return fmt.Errorf("more than 1 artifact")
+		}
+
+		tags := make(map[string]string)
+		tags["OS_Version"] = "Ubuntu"
+		tags["Release"] = "Latest"
+
+		// Get the actual *Artifact pointer so we can access the AMIs directly
+		artifactRaw := artifacts[0]
+		artifact, ok := artifactRaw.(*common.Artifact)
+		if !ok {
+			return fmt.Errorf("unknown artifact: %#v", artifactRaw)
+		}
+
+		// describe the image, get block devices with a snapshot
+		ec2conn, _ := testEC2Conn()
+		imageResp, err := ec2conn.DescribeImages(&ec2.DescribeImagesInput{
+			ImageIDs: []*string{aws.String(artifact.Amis["us-east-1"])},
+		})
+
+		if err != nil {
+			return fmt.Errorf("Error retrieving details for AMI Artifcat (%#v) in Tags Test: %s", artifact, err)
+		}
+
+		if len(imageResp.Images) == 0 {
+			return fmt.Errorf("No images found for AMI Artifcat (%#v) in Tags Test: %s", artifact, err)
+		}
+
+		image := imageResp.Images[0]
+
+		// Check only those with a Snapshot ID, i.e. not Ephemeral
+		var snapshots []*string
+		for _, device := range image.BlockDeviceMappings {
+			if device.EBS != nil && device.EBS.SnapshotID != nil {
+				snapshots = append(snapshots, device.EBS.SnapshotID)
+			}
+		}
+
+		// grab matching snapshot info
+		resp, err := ec2conn.DescribeSnapshots(&ec2.DescribeSnapshotsInput{
+			SnapshotIDs: snapshots,
+		})
+
+		if err != nil {
+			return fmt.Errorf("Error retreiving Snapshots for AMI Artifcat (%#v) in Tags Test: %s", artifact, err)
+		}
+
+		if len(resp.Snapshots) == 0 {
+			return fmt.Errorf("No Snapshots found for AMI Artifcat (%#v) in Tags Test", artifact)
+		}
+
+		// grab the snapshots, check the tags
+		for _, s := range resp.Snapshots {
+			expected := len(tags)
+			for _, t := range s.Tags {
+				for key, value := range tags {
+					if key == *t.Key && value == *t.Value {
+						expected--
+					}
+				}
+			}
+
+			if expected > 0 {
+				return fmt.Errorf("Not all tags found")
+			}
+		}
+
+		return nil
+	}
+}
+
+const testBuilderTagsAccBasic = `
+{
+  "builders": [
+    {
+      "type": "test",
+      "region": "us-east-1",
+      "source_ami": "ami-9eaa1cf6",
+      "instance_type": "t2.micro",
+      "ssh_username": "ubuntu",
+      "ami_name": "packer-tags-testing-{{timestamp}}",
+      "tags": {
+        "OS_Version": "Ubuntu",
+        "Release": "Latest"
+      }
+    }
+  ]
+}
+`

builder/amazon/instance/builder.go

@@ -13,6 +13,7 @@ import (
 	"github.com/mitchellh/multistep"
 	awscommon "github.com/mitchellh/packer/builder/amazon/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"
@@ -167,6 +168,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 
 	// Build the steps
 	steps := []multistep.Step{
+		&awscommon.StepPreValidate{
+			DestAmiName:     b.config.AMIName,
+			ForceDeregister: b.config.AMIForceDeregister,
+		},
 		&awscommon.StepSourceAMIInfo{
 			SourceAmi:          b.config.SourceAmi,
 			EnhancedNetworking: b.config.AMIEnhancedNetworking,
@@ -175,11 +180,11 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			Debug:          b.config.PackerDebug,
 			DebugKeyPath:   fmt.Sprintf("ec2_%s.pem", b.config.PackerBuildName),
 			KeyPairName:    b.config.TemporaryKeyPairName,
-			PrivateKeyFile: b.config.SSHPrivateKeyFile,
+			PrivateKeyFile: b.config.RunConfig.Comm.SSHPrivateKey,
 		},
 		&awscommon.StepSecurityGroup{
+			CommConfig:       &b.config.RunConfig.Comm,
 			SecurityGroupIds: b.config.SecurityGroupIds,
-			SSHPort:          b.config.SSHPort,
 			VpcId:            b.config.VpcId,
 		},
 		&awscommon.StepRunSourceInstance{
@@ -197,11 +202,17 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			BlockDevices:             b.config.BlockDevices,
 			Tags:                     b.config.RunTags,
 		},
-		&common.StepConnectSSH{
-			SSHAddress: awscommon.SSHAddress(
-				ec2conn, b.config.SSHPort, b.config.SSHPrivateIp),
-			SSHConfig:      awscommon.SSHConfig(b.config.SSHUsername),
-			SSHWaitTimeout: b.config.SSHTimeout(),
+		&awscommon.StepGetPassword{
+			Comm:    &b.config.RunConfig.Comm,
+			Timeout: b.config.WindowsPasswordTimeout,
+		},
+		&communicator.StepConnect{
+			Config: &b.config.RunConfig.Comm,
+			Host: awscommon.SSHHost(
+				ec2conn,
+				b.config.SSHPrivateIp),
+			SSHConfig: awscommon.SSHConfig(
+				b.config.RunConfig.Comm.SSHUsername),
 		},
 		&common.StepProvision{},
 		&StepUploadX509Cert{},
@@ -211,6 +222,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		&StepUploadBundle{
 			Debug: b.config.PackerDebug,
 		},
+		&awscommon.StepDeregisterAMI{
+			ForceDeregister: b.config.AMIForceDeregister,
+			AMIName:         b.config.AMIName,
+		},
 		&StepRegisterAMI{},
 		&awscommon.StepAMIRegionCopy{
 			AccessConfig: &b.config.AccessConfig,

builder/digitalocean/builder.go

@@ -6,11 +6,11 @@ package digitalocean
 import (
 	"fmt"
 	"log"
-	"time"
 
 	"github.com/digitalocean/godo"
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/common"
+	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/packer"
 	"golang.org/x/oauth2"
 )
@@ -53,10 +53,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		},
 		new(stepCreateDroplet),
 		new(stepDropletInfo),
-		&common.StepConnectSSH{
-			SSHAddress:     sshAddress,
+		&communicator.StepConnect{
+			Config:    &b.config.Comm,
+			Host:      commHost,
 			SSHConfig: sshConfig,
-			SSHWaitTimeout: 5 * time.Minute,
 		},
 		new(common.StepProvision),
 		new(stepShutdown),

builder/digitalocean/builder_test.go

@@ -3,6 +3,7 @@ package digitalocean
 import (
 	"strconv"
 	"testing"
+	"time"
 
 	"github.com/mitchellh/packer/packer"
 )
@@ -163,8 +164,8 @@ func TestBuilderPrepare_SSHUsername(t *testing.T) {
 		t.Fatalf("should not have error: %s", err)
 	}
 
-	if b.config.SSHUsername != "root" {
-		t.Errorf("invalid: %s", b.config.SSHUsername)
+	if b.config.Comm.SSHUsername != "root" {
+		t.Errorf("invalid: %s", b.config.Comm.SSHUsername)
 	}
 
 	// Test set
@@ -178,52 +179,11 @@ func TestBuilderPrepare_SSHUsername(t *testing.T) {
 		t.Fatalf("should not have error: %s", err)
 	}
 
-	if b.config.SSHUsername != "foo" {
-		t.Errorf("invalid: %s", b.config.SSHUsername)
+	if b.config.Comm.SSHUsername != "foo" {
+		t.Errorf("invalid: %s", b.config.Comm.SSHUsername)
 	}
 }
 
-func TestBuilderPrepare_SSHTimeout(t *testing.T) {
-	var b Builder
-	config := testConfig()
-
-	// Test default
-	warnings, err := b.Prepare(config)
-	if len(warnings) > 0 {
-		t.Fatalf("bad: %#v", warnings)
-	}
-	if err != nil {
-		t.Fatalf("should not have error: %s", err)
-	}
-
-	if b.config.RawSSHTimeout != "1m" {
-		t.Errorf("invalid: %s", b.config.RawSSHTimeout)
-	}
-
-	// Test set
-	config["ssh_timeout"] = "30s"
-	b = Builder{}
-	warnings, err = b.Prepare(config)
-	if len(warnings) > 0 {
-		t.Fatalf("bad: %#v", warnings)
-	}
-	if err != nil {
-		t.Fatalf("should not have error: %s", err)
-	}
-
-	// Test bad
-	config["ssh_timeout"] = "tubes"
-	b = Builder{}
-	warnings, err = b.Prepare(config)
-	if len(warnings) > 0 {
-		t.Fatalf("bad: %#v", warnings)
-	}
-	if err == nil {
-		t.Fatal("should have error")
-	}
-
-}
-
 func TestBuilderPrepare_StateTimeout(t *testing.T) {
 	var b Builder
 	config := testConfig()
@@ -237,8 +197,8 @@ func TestBuilderPrepare_StateTimeout(t *testing.T) {
 		t.Fatalf("should not have error: %s", err)
 	}
 
-	if b.config.RawStateTimeout != "6m" {
-		t.Errorf("invalid: %s", b.config.RawStateTimeout)
+	if b.config.StateTimeout != 6*time.Minute {
+		t.Errorf("invalid: %s", b.config.StateTimeout)
 	}
 
 	// Test set

builder/digitalocean/config.go

@@ -9,6 +9,7 @@ import (
 	"github.com/mitchellh/mapstructure"
 	"github.com/mitchellh/packer/common"
 	"github.com/mitchellh/packer/common/uuid"
+	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/helper/config"
 	"github.com/mitchellh/packer/packer"
 	"github.com/mitchellh/packer/template/interpolate"
@@ -16,6 +17,7 @@ import (
 
 type Config struct {
 	common.PackerConfig `mapstructure:",squash"`
+	Comm                communicator.Config `mapstructure:",squash"`
 
 	APIToken string `mapstructure:"api_token"`
 
@@ -25,18 +27,9 @@ type Config struct {
 
 	PrivateNetworking bool          `mapstructure:"private_networking"`
 	SnapshotName      string        `mapstructure:"snapshot_name"`
+	StateTimeout      time.Duration `mapstructure:"state_timeout"`
 	DropletName       string        `mapstructure:"droplet_name"`
 	UserData          string        `mapstructure:"user_data"`
-	SSHUsername       string `mapstructure:"ssh_username"`
-	SSHPort           uint   `mapstructure:"ssh_port"`
-
-	RawSSHTimeout   string `mapstructure:"ssh_timeout"`
-	RawStateTimeout string `mapstructure:"state_timeout"`
-
-	// These are unexported since they're set by other fields
-	// being set.
-	sshTimeout   time.Duration
-	stateTimeout time.Duration
 
 	ctx *interpolate.Context
 }
@@ -79,29 +72,22 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) {
 		c.DropletName = fmt.Sprintf("packer-%s", uuid.TimeOrderedUUID())
 	}
 
-	if c.SSHUsername == "" {
+	if c.Comm.SSHUsername == "" {
 		// Default to "root". You can override this if your
 		// SourceImage has a different user account then the DO default
-		c.SSHUsername = "root"
-	}
-
-	if c.SSHPort == 0 {
-		// Default to port 22 per DO default
-		c.SSHPort = 22
+		c.Comm.SSHUsername = "root"
 	}
 
-	if c.RawSSHTimeout == "" {
-		// Default to 1 minute timeouts
-		c.RawSSHTimeout = "1m"
-	}
-
-	if c.RawStateTimeout == "" {
+	if c.StateTimeout == 0 {
 		// Default to 6 minute timeouts waiting for
 		// desired state. i.e waiting for droplet to become active
-		c.RawStateTimeout = "6m"
+		c.StateTimeout = 6 * time.Minute
 	}
 
 	var errs *packer.MultiError
+	if es := c.Comm.Prepare(c.ctx); len(es) > 0 {
+		errs = packer.MultiErrorAppend(errs, es...)
+	}
 	if c.APIToken == "" {
 		// Required configurations that will display errors if not set
 		errs = packer.MultiErrorAppend(
@@ -123,20 +109,6 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) {
 			errs, errors.New("image is required"))
 	}
 
-	sshTimeout, err := time.ParseDuration(c.RawSSHTimeout)
-	if err != nil {
-		errs = packer.MultiErrorAppend(
-			errs, fmt.Errorf("Failed parsing ssh_timeout: %s", err))
-	}
-	c.sshTimeout = sshTimeout
-
-	stateTimeout, err := time.ParseDuration(c.RawStateTimeout)
-	if err != nil {
-		errs = packer.MultiErrorAppend(
-			errs, fmt.Errorf("Failed parsing state_timeout: %s", err))
-	}
-	c.stateTimeout = stateTimeout
-
 	if errs != nil && len(errs.Errors) > 0 {
 		return nil, nil, errs
 	}

builder/digitalocean/ssh.go

@@ -2,14 +2,14 @@ package digitalocean
 
 import (
 	"fmt"
-	"github.com/mitchellh/multistep"
 	"golang.org/x/crypto/ssh"
+
+	"github.com/mitchellh/multistep"
 )
 
-func sshAddress(state multistep.StateBag) (string, error) {
-	config := state.Get("config").(Config)
+func commHost(state multistep.StateBag) (string, error) {
 	ipAddress := state.Get("droplet_ip").(string)
-	return fmt.Sprintf("%s:%d", ipAddress, config.SSHPort), nil
+	return ipAddress, nil
 }
 
 func sshConfig(state multistep.StateBag) (*ssh.ClientConfig, error) {
@@ -22,7 +22,7 @@ func sshConfig(state multistep.StateBag) (*ssh.ClientConfig, error) {
 	}
 
 	return &ssh.ClientConfig{
-		User: config.SSHUsername,
+		User: config.Comm.SSHUsername,
 		Auth: []ssh.AuthMethod{
 			ssh.PublicKeys(signer),
 		},

builder/digitalocean/step_droplet_info.go

@@ -18,7 +18,7 @@ func (s *stepDropletInfo) Run(state multistep.StateBag) multistep.StepAction {
 
 	ui.Say("Waiting for droplet to become active...")
 
-	err := waitForDropletState("active", dropletId, client, c.stateTimeout)
+	err := waitForDropletState("active", dropletId, client, c.StateTimeout)
 	if err != nil {
 		err := fmt.Errorf("Error waiting for droplet to become active: %s", err)
 		state.Put("error", err)

builder/digitalocean/step_power_off.go

@@ -42,7 +42,7 @@ func (s *stepPowerOff) Run(state multistep.StateBag) multistep.StepAction {
 	}
 
 	log.Println("Waiting for poweroff event to complete...")
-	err = waitForDropletState("off", dropletId, client, c.stateTimeout)
+	err = waitForDropletState("off", dropletId, client, c.StateTimeout)
 	if err != nil {
 		state.Put("error", err)
 		ui.Error(err.Error())

builder/digitalocean/step_snapshot.go

@@ -41,7 +41,7 @@ func (s *stepSnapshot) Run(state multistep.StateBag) multistep.StepAction {
 
 	// With the pending state over, verify that we're in the active state
 	ui.Say("Waiting for snapshot to complete...")
-	err = waitForDropletState("active", dropletId, client, c.stateTimeout)
+	err = waitForDropletState("active", dropletId, client, c.StateTimeout)
 	if err != nil {
 		err := fmt.Errorf("Error waiting for snapshot to complete: %s", err)
 		state.Put("error", err)

builder/docker/builder.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/common"
+	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/packer"
 )
 
@@ -42,7 +43,15 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		&StepTempDir{},
 		&StepPull{},
 		&StepRun{},
-		&StepProvision{},
+		&communicator.StepConnect{
+			Config:    &b.config.Comm,
+			Host:      commHost,
+			SSHConfig: sshConfig(&b.config.Comm),
+			CustomConnect: map[string]multistep.Step{
+				"docker": &StepConnectDocker{},
+			},
+		},
+		&common.StepProvision{},
 	}
 
 	if b.config.Commit {

builder/docker/comm.go

+package docker
+
+import (
+	"fmt"
+	"io/ioutil"
+
+	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/communicator/ssh"
+	"github.com/mitchellh/packer/helper/communicator"
+	gossh "golang.org/x/crypto/ssh"
+)
+
+func commHost(state multistep.StateBag) (string, error) {
+	containerId := state.Get("container_id").(string)
+	driver := state.Get("driver").(Driver)
+	return driver.IPAddress(containerId)
+}
+
+func sshConfig(comm *communicator.Config) func(state multistep.StateBag) (*gossh.ClientConfig, error) {
+	return func(state multistep.StateBag) (*gossh.ClientConfig, error) {
+		if comm.SSHPrivateKey != "" {
+			// key based auth
+			bytes, err := ioutil.ReadFile(comm.SSHPrivateKey)
+			if err != nil {
+				return nil, fmt.Errorf("Error setting up SSH config: %s", err)
+			}
+			privateKey := string(bytes)
+
+			signer, err := gossh.ParsePrivateKey([]byte(privateKey))
+			if err != nil {
+				return nil, fmt.Errorf("Error setting up SSH config: %s", err)
+			}
+
+			return &gossh.ClientConfig{
+				User: comm.SSHUsername,
+				Auth: []gossh.AuthMethod{
+					gossh.PublicKeys(signer),
+				},
+			}, nil
+		} else {
+			// password based auth
+			return &gossh.ClientConfig{
+				User: comm.SSHUsername,
+				Auth: []gossh.AuthMethod{
+					gossh.Password(comm.SSHPassword),
+					gossh.KeyboardInteractive(
+						ssh.PasswordKeyboardInteractive(comm.SSHPassword)),
+				},
+			}, nil
+		}
+	}
+}

builder/docker/config.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/mitchellh/mapstructure"
 	"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"
@@ -13,6 +14,7 @@ import (
 
 type Config struct {
 	common.PackerConfig `mapstructure:",squash"`
+	Comm                communicator.Config `mapstructure:",squash"`
 
 	Commit     bool
 	ExportPath string `mapstructure:"export_path"`
@@ -69,7 +71,15 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) {
 		c.Pull = true
 	}
 
+	// Default to the normal Docker type
+	if c.Comm.Type == "" {
+		c.Comm.Type = "docker"
+	}
+
 	var errs *packer.MultiError
+	if es := c.Comm.Prepare(&c.ctx); len(es) > 0 {
+		errs = packer.MultiErrorAppend(errs, es...)
+	}
 	if c.Image == "" {
 		errs = packer.MultiErrorAppend(errs,
 			fmt.Errorf("image must be specified"))

builder/docker/driver.go

@@ -22,6 +22,10 @@ type Driver interface {
 	// Import imports a container from a tar file
 	Import(path, repo string) (string, error)
 
+	// IPAddress returns the address of the container that can be used
+	// for external access.
+	IPAddress(id string) (string, error)
+
 	// Login. This will lock the driver from performing another Login
 	// until Logout is called. Therefore, any users MUST call Logout.
 	Login(repo, email, username, password string) error

builder/docker/driver_docker.go

@@ -116,6 +116,23 @@ func (d *DockerDriver) Import(path string, repo string) (string, error) {
 	return strings.TrimSpace(stdout.String()), nil
 }
 
+func (d *DockerDriver) IPAddress(id string) (string, error) {
+	var stderr, stdout bytes.Buffer
+	cmd := exec.Command(
+		"docker",
+		"inspect",
+		"--format",
+		"{{ .NetworkSettings.IPAddress }}",
+		id)
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+	if err := cmd.Run(); err != nil {
+		return "", fmt.Errorf("Error: %s\n\nStderr: %s", err, stderr.String())
+	}
+
+	return strings.TrimSpace(stdout.String()), nil
+}
+
 func (d *DockerDriver) Login(repo, email, user, pass string) error {
 	d.l.Lock()
 

builder/docker/driver_mock.go

@@ -23,6 +23,11 @@ type MockDriver struct {
 	ImportId     string
 	ImportErr    error
 
+	IPAddressCalled bool
+	IPAddressID     string
+	IPAddressResult string
+	IPAddressErr    error
+
 	LoginCalled   bool
 	LoginEmail    string
 	LoginUsername string
@@ -104,6 +109,12 @@ func (d *MockDriver) Import(path, repo string) (string, error) {
 	return d.ImportId, d.ImportErr
 }
 
+func (d *MockDriver) IPAddress(id string) (string, error) {
+	d.IPAddressCalled = true
+	d.IPAddressID = id
+	return d.IPAddressResult, d.IPAddressErr
+}
+
 func (d *MockDriver) Login(r, e, u, p string) error {
 	d.LoginCalled = true
 	d.LoginRepo = r

builder/docker/step_provision.go → builder/docker/step_connect_docker.go

@@ -2,12 +2,11 @@ package docker
 
 import (
 	"github.com/mitchellh/multistep"
-	"github.com/mitchellh/packer/common"
 )
 
-type StepProvision struct{}
+type StepConnectDocker struct{}
 
-func (s *StepProvision) Run(state multistep.StateBag) multistep.StepAction {
+func (s *StepConnectDocker) Run(state multistep.StateBag) multistep.StepAction {
 	containerId := state.Get("container_id").(string)
 	driver := state.Get("driver").(Driver)
 	tempDir := state.Get("temp_dir").(string)
@@ -28,8 +27,8 @@ func (s *StepProvision) Run(state multistep.StateBag) multistep.StepAction {
 		Version:      version,
 	}
 
-	prov := common.StepProvision{Comm: comm}
-	return prov.Run(state)
+	state.Put("communicator", comm)
+	return multistep.ActionContinue
 }
 
-func (s *StepProvision) Cleanup(state multistep.StateBag) {}
+func (s *StepConnectDocker) Cleanup(state multistep.StateBag) {}

builder/googlecompute/builder.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/common"
+	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/packer"
 )
 
@@ -60,10 +61,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		&StepInstanceInfo{
 			Debug: b.config.PackerDebug,
 		},
-		&common.StepConnectSSH{
-			SSHAddress:     sshAddress,
+		&communicator.StepConnect{
+			Config:    &b.config.Comm,
+			Host:      commHost,
 			SSHConfig: sshConfig,
-			SSHWaitTimeout: b.config.sshTimeout,
 		},
 		new(common.StepProvision),
 		new(StepTeardownInstance),

builder/googlecompute/config.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/mitchellh/packer/common"
 	"github.com/mitchellh/packer/common/uuid"
+	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/helper/config"
 	"github.com/mitchellh/packer/packer"
 	"github.com/mitchellh/packer/template/interpolate"
@@ -17,6 +18,7 @@ import (
 // state of the config object.
 type Config struct {
 	common.PackerConfig `mapstructure:",squash"`
+	Comm                communicator.Config `mapstructure:",squash"`
 
 	AccountFile string `mapstructure:"account_file"`
 	ProjectId   string `mapstructure:"project_id"`
@@ -31,16 +33,12 @@ type Config struct {
 	Network              string            `mapstructure:"network"`
 	SourceImage          string            `mapstructure:"source_image"`
 	SourceImageProjectId string            `mapstructure:"source_image_project_id"`
-	SSHUsername          string            `mapstructure:"ssh_username"`
-	SSHPort              uint              `mapstructure:"ssh_port"`
-	RawSSHTimeout        string            `mapstructure:"ssh_timeout"`
 	RawStateTimeout      string            `mapstructure:"state_timeout"`
 	Tags                 []string          `mapstructure:"tags"`
 	Zone                 string            `mapstructure:"zone"`
 
 	account         accountFile
 	privateKeyBytes []byte
-	sshTimeout      time.Duration
 	stateTimeout    time.Duration
 	ctx             *interpolate.Context
 }
@@ -88,20 +86,12 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) {
 		c.MachineType = "n1-standard-1"
 	}
 
-	if c.RawSSHTimeout == "" {
-		c.RawSSHTimeout = "5m"
-	}
-
 	if c.RawStateTimeout == "" {
 		c.RawStateTimeout = "5m"
 	}
 
-	if c.SSHUsername == "" {
-		c.SSHUsername = "root"
-	}
-
-	if c.SSHPort == 0 {
-		c.SSHPort = 22
+	if c.Comm.SSHUsername == "" {
+		c.Comm.SSHUsername = "root"
 	}
 
 	var errs *packer.MultiError
@@ -122,14 +112,6 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) {
 			errs, errors.New("a zone must be specified"))
 	}
 
-	// Process timeout settings.
-	sshTimeout, err := time.ParseDuration(c.RawSSHTimeout)
-	if err != nil {
-		errs = packer.MultiErrorAppend(
-			errs, fmt.Errorf("Failed parsing ssh_timeout: %s", err))
-	}
-	c.sshTimeout = sshTimeout
-
 	stateTimeout, err := time.ParseDuration(c.RawStateTimeout)
 	if err != nil {
 		errs = packer.MultiErrorAppend(

builder/googlecompute/ssh.go

@@ -2,15 +2,14 @@ package googlecompute
 
 import (
 	"fmt"
+
 	"github.com/mitchellh/multistep"
 	"golang.org/x/crypto/ssh"
 )
 
-// sshAddress returns the ssh address.
-func sshAddress(state multistep.StateBag) (string, error) {
-	config := state.Get("config").(*Config)
+func commHost(state multistep.StateBag) (string, error) {
 	ipAddress := state.Get("instance_ip").(string)
-	return fmt.Sprintf("%s:%d", ipAddress, config.SSHPort), nil
+	return ipAddress, nil
 }
 
 // sshConfig returns the ssh configuration.
@@ -24,7 +23,7 @@ func sshConfig(state multistep.StateBag) (*ssh.ClientConfig, error) {
 	}
 
 	return &ssh.ClientConfig{
-		User: config.SSHUsername,
+		User: config.Comm.SSHUsername,
 		Auth: []ssh.AuthMethod{
 			ssh.PublicKeys(signer),
 		},

builder/googlecompute/step_create_instance.go

@@ -32,7 +32,7 @@ func (config *Config) getInstanceMetadata(sshPublicKey string) map[string]string
 
 	// Merge any existing ssh keys with our public key
 	sshMetaKey := "sshKeys"
-	sshKeys := fmt.Sprintf("%s:%s", config.SSHUsername, sshPublicKey)
+	sshKeys := fmt.Sprintf("%s:%s", config.Comm.SSHUsername, sshPublicKey)
 	if confSshKeys, exists := instanceMetadata[sshMetaKey]; exists {
 		sshKeys = fmt.Sprintf("%s\n%s", sshKeys, confSshKeys)
 	}

builder/null/builder.go

 package null
 
 import (
+	"log"
+
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/common"
+	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/packer"
-	"log"
-	"time"
 )
 
 const BuilderId = "fnoeding.null"
@@ -27,10 +28,13 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 
 func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) {
 	steps := []multistep.Step{
-		&common.StepConnectSSH{
-			SSHAddress:     SSHAddress(b.config.Host, b.config.Port),
-			SSHConfig:      SSHConfig(b.config.SSHUsername, b.config.SSHPassword, b.config.SSHPrivateKeyFile),
-			SSHWaitTimeout: 1 * time.Minute,
+		&communicator.StepConnect{
+			Config: &b.config.CommConfig,
+			Host:   CommHost(b.config.CommConfig.SSHHost),
+			SSHConfig: SSHConfig(
+				b.config.CommConfig.SSHUsername,
+				b.config.CommConfig.SSHPassword,
+				b.config.CommConfig.SSHPrivateKey),
 		},
 		&common.StepProvision{},
 	}

builder/null/config.go

@@ -2,7 +2,9 @@ package null
 
 import (
 	"fmt"
+
 	"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"
@@ -11,49 +13,40 @@ import (
 type Config struct {
 	common.PackerConfig `mapstructure:",squash"`
 
-	Host              string `mapstructure:"host"`
-	Port              int    `mapstructure:"port"`
-	SSHUsername       string `mapstructure:"ssh_username"`
-	SSHPassword       string `mapstructure:"ssh_password"`
-	SSHPrivateKeyFile string `mapstructure:"ssh_private_key_file"`
+	CommConfig communicator.Config `mapstructure:",squash"`
 }
 
 func NewConfig(raws ...interface{}) (*Config, []string, error) {
-	c := new(Config)
+	var c Config
 
-	err := config.Decode(c, &config.DecodeOpts{
+	err := config.Decode(&c, &config.DecodeOpts{
 		Interpolate:       true,
-		InterpolateFilter: &interpolate.RenderFilter{
-			Exclude: []string{
-				"run_command",
-			},
-		},
+		InterpolateFilter: &interpolate.RenderFilter{},
 	}, raws...)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	if c.Port == 0 {
-		c.Port = 22
-	}
-
 	var errs *packer.MultiError
-	if c.Host == "" {
+	if es := c.CommConfig.Prepare(nil); len(es) > 0 {
+		errs = packer.MultiErrorAppend(errs, es...)
+	}
+	if c.CommConfig.SSHHost == "" {
 		errs = packer.MultiErrorAppend(errs,
 			fmt.Errorf("host must be specified"))
 	}
 
-	if c.SSHUsername == "" {
+	if c.CommConfig.SSHUsername == "" {
 		errs = packer.MultiErrorAppend(errs,
 			fmt.Errorf("ssh_username must be specified"))
 	}
 
-	if c.SSHPassword == "" && c.SSHPrivateKeyFile == "" {
+	if c.CommConfig.SSHPassword == "" && c.CommConfig.SSHPrivateKey == "" {
 		errs = packer.MultiErrorAppend(errs,
 			fmt.Errorf("one of ssh_password and ssh_private_key_file must be specified"))
 	}
 
-	if c.SSHPassword != "" && c.SSHPrivateKeyFile != "" {
+	if c.CommConfig.SSHPassword != "" && c.CommConfig.SSHPrivateKey != "" {
 		errs = packer.MultiErrorAppend(errs,
 			fmt.Errorf("only one of ssh_password and ssh_private_key_file must be specified"))
 	}
@@ -62,5 +55,5 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) {
 		return nil, nil, errs
 	}
 
-	return c, nil, nil
+	return &c, nil, nil
 }

builder/null/config_test.go

 package null
 
 import (
+	"os"
 	"testing"
+
+	"github.com/mitchellh/packer/helper/communicator"
 )
 
 func testConfig() map[string]interface{} {
 	return map[string]interface{}{
-		"host":         "foo",
+		"ssh_host":     "foo",
 		"ssh_username": "bar",
 		"ssh_password": "baz",
 	}
@@ -48,8 +51,8 @@ func TestConfigPrepare_port(t *testing.T) {
 	// default port should be 22
 	delete(raw, "port")
 	c, warns, errs := NewConfig(raw)
-	if c.Port != 22 {
-		t.Fatalf("bad: port should default to 22, not %d", c.Port)
+	if c.CommConfig.SSHPort != 22 {
+		t.Fatalf("bad: port should default to 22, not %d", c.CommConfig.SSHPort)
 	}
 	testConfigOk(t, warns, errs)
 }
@@ -58,12 +61,12 @@ func TestConfigPrepare_host(t *testing.T) {
 	raw := testConfig()
 
 	// No host
-	delete(raw, "host")
+	delete(raw, "ssh_host")
 	_, warns, errs := NewConfig(raw)
 	testConfigErr(t, warns, errs)
 
 	// Good host
-	raw["host"] = "good"
+	raw["ssh_host"] = "good"
 	_, warns, errs = NewConfig(raw)
 	testConfigOk(t, warns, errs)
 }
@@ -97,7 +100,9 @@ func TestConfigPrepare_sshCredential(t *testing.T) {
 	testConfigOk(t, warns, errs)
 
 	// only ssh_private_key_file
-	raw["ssh_private_key_file"] = "good"
+	testFile := communicator.TestPEM(t)
+	defer os.Remove(testFile)
+	raw["ssh_private_key_file"] = testFile
 	delete(raw, "ssh_password")
 	_, warns, errs = NewConfig(raw)
 	testConfigOk(t, warns, errs)

builder/null/ssh.go

@@ -8,11 +8,9 @@ import (
 	"io/ioutil"
 )
 
-// SSHAddress returns a function that can be given to the SSH communicator
-// for determining the SSH address
-func SSHAddress(host string, port int) func(multistep.StateBag) (string, error) {
+func CommHost(host string) func(multistep.StateBag) (string, error) {
 	return func(state multistep.StateBag) (string, error) {
-		return fmt.Sprintf("%s:%d", host, port), nil
+		return host, nil
 	}
 }
 

builder/openstack/builder.go

@@ -5,10 +5,11 @@ package openstack
 
 import (
 	"fmt"
-	"github.com/mitchellh/multistep"
-	"github.com/mitchellh/packer/common"
 	"log"
 
+	"github.com/mitchellh/multistep"
+	"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"
@@ -19,6 +20,7 @@ const BuilderId = "mitchellh.openstack"
 
 type Config struct {
 	common.PackerConfig `mapstructure:",squash"`
+
 	AccessConfig `mapstructure:",squash"`
 	ImageConfig  `mapstructure:",squash"`
 	RunConfig    `mapstructure:",squash"`
@@ -67,6 +69,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 
 	// Build the steps
 	steps := []multistep.Step{
+		&StepLoadExtensions{},
 		&StepLoadFlavor{
 			Flavor: b.config.Flavor,
 		},
@@ -80,6 +83,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			SecurityGroups:   b.config.SecurityGroups,
 			Networks:         b.config.Networks,
 			AvailabilityZone: b.config.AvailabilityZone,
+			UserData:         b.config.UserData,
+			UserDataFile:     b.config.UserDataFile,
 		},
 		&StepWaitForRackConnect{
 			Wait: b.config.RackconnectWait,
@@ -88,12 +93,15 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			FloatingIpPool: b.config.FloatingIpPool,
 			FloatingIp:     b.config.FloatingIp,
 		},
-		&common.StepConnectSSH{
-			SSHAddress:     SSHAddress(computeClient, b.config.SSHInterface, b.config.SSHPort),
-			SSHConfig:      SSHConfig(b.config.SSHUsername),
-			SSHWaitTimeout: b.config.SSHTimeout(),
+		&communicator.StepConnect{
+			Config: &b.config.RunConfig.Comm,
+			Host: CommHost(
+				computeClient,
+				b.config.SSHInterface),
+			SSHConfig: SSHConfig(b.config.RunConfig.Comm.SSHUsername),
 		},
 		&common.StepProvision{},
+		&StepStopServer{},
 		&stepCreateImage{},
 	}
 

builder/openstack/run_config.go

@@ -2,48 +2,37 @@ package openstack
 
 import (
 	"errors"
-	"fmt"
-	"time"
 
+	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/template/interpolate"
 )
 
 // RunConfig contains configuration for running an instance from a source
 // image and details on how to access that launched image.
 type RunConfig struct {
+	Comm         communicator.Config `mapstructure:",squash"`
+	SSHInterface string              `mapstructure:"ssh_interface"`
+
 	SourceImage      string   `mapstructure:"source_image"`
 	Flavor           string   `mapstructure:"flavor"`
-	RawSSHTimeout    string   `mapstructure:"ssh_timeout"`
-	SSHUsername      string   `mapstructure:"ssh_username"`
-	SSHPort          int      `mapstructure:"ssh_port"`
-	SSHInterface     string   `mapstructure:"ssh_interface"`
 	AvailabilityZone string   `mapstructure:"availability_zone"`
 	RackconnectWait  bool     `mapstructure:"rackconnect_wait"`
 	FloatingIpPool   string   `mapstructure:"floating_ip_pool"`
 	FloatingIp       string   `mapstructure:"floating_ip"`
 	SecurityGroups   []string `mapstructure:"security_groups"`
 	Networks         []string `mapstructure:"networks"`
+	UserData         string   `mapstructure:"user_data"`
+	UserDataFile     string   `mapstructure:"user_data_file"`
 
 	// Not really used, but here for BC
 	OpenstackProvider string `mapstructure:"openstack_provider"`
 	UseFloatingIp     bool   `mapstructure:"use_floating_ip"`
-
-	// Unexported fields that are calculated from others
-	sshTimeout time.Duration
 }
 
 func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
 	// Defaults
-	if c.SSHUsername == "" {
-		c.SSHUsername = "root"
-	}
-
-	if c.SSHPort == 0 {
-		c.SSHPort = 22
-	}
-
-	if c.RawSSHTimeout == "" {
-		c.RawSSHTimeout = "5m"
+	if c.Comm.SSHUsername == "" {
+		c.Comm.SSHUsername = "root"
 	}
 
 	if c.UseFloatingIp && c.FloatingIpPool == "" {
@@ -51,8 +40,7 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
 	}
 
 	// Validation
-	var err error
-	errs := make([]error, 0)
+	errs := c.Comm.Prepare(ctx)
 	if c.SourceImage == "" {
 		errs = append(errs, errors.New("A source_image must be specified"))
 	}
@@ -61,18 +49,5 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
 		errs = append(errs, errors.New("A flavor must be specified"))
 	}
 
-	if c.SSHUsername == "" {
-		errs = append(errs, errors.New("An ssh_username must be specified"))
-	}
-
-	c.sshTimeout, err = time.ParseDuration(c.RawSSHTimeout)
-	if err != nil {
-		errs = append(errs, fmt.Errorf("Failed parsing ssh_timeout: %s", err))
-	}
-
 	return errs
 }
-
-func (c *RunConfig) SSHTimeout() time.Duration {
-	return c.sshTimeout
-}

builder/openstack/run_config_test.go

@@ -3,6 +3,8 @@ package openstack
 import (
 	"os"
 	"testing"
+
+	"github.com/mitchellh/packer/helper/communicator"
 )
 
 func init() {
@@ -17,7 +19,10 @@ func testRunConfig() *RunConfig {
 	return &RunConfig{
 		SourceImage: "abcd",
 		Flavor:      "m1.small",
-		SSHUsername: "root",
+
+		Comm: communicator.Config{
+			SSHUsername: "foo",
+		},
 	}
 }
 
@@ -47,41 +52,28 @@ func TestRunConfigPrepare_SourceImage(t *testing.T) {
 
 func TestRunConfigPrepare_SSHPort(t *testing.T) {
 	c := testRunConfig()
-	c.SSHPort = 0
-	if err := c.Prepare(nil); len(err) != 0 {
-		t.Fatalf("err: %s", err)
-	}
-
-	if c.SSHPort != 22 {
-		t.Fatalf("invalid value: %d", c.SSHPort)
-	}
-
-	c.SSHPort = 44
+	c.Comm.SSHPort = 0
 	if err := c.Prepare(nil); len(err) != 0 {
 		t.Fatalf("err: %s", err)
 	}
 
-	if c.SSHPort != 44 {
-		t.Fatalf("invalid value: %d", c.SSHPort)
+	if c.Comm.SSHPort != 22 {
+		t.Fatalf("invalid value: %d", c.Comm.SSHPort)
 	}
-}
 
-func TestRunConfigPrepare_SSHTimeout(t *testing.T) {
-	c := testRunConfig()
-	c.RawSSHTimeout = ""
+	c.Comm.SSHPort = 44
 	if err := c.Prepare(nil); len(err) != 0 {
 		t.Fatalf("err: %s", err)
 	}
 
-	c.RawSSHTimeout = "bad"
-	if err := c.Prepare(nil); len(err) != 1 {
-		t.Fatalf("err: %s", err)
+	if c.Comm.SSHPort != 44 {
+		t.Fatalf("invalid value: %d", c.Comm.SSHPort)
 	}
 }
 
 func TestRunConfigPrepare_SSHUsername(t *testing.T) {
 	c := testRunConfig()
-	c.SSHUsername = ""
+	c.Comm.SSHUsername = ""
 	if err := c.Prepare(nil); len(err) != 0 {
 		t.Fatalf("err: %s", err)
 	}

builder/openstack/server.go

@@ -28,7 +28,7 @@ type StateChangeConf struct {
 	Pending   []string
 	Refresh   StateRefreshFunc
 	StepState multistep.StateBag
-	Target    string
+	Target    []string
 }
 
 // ServerStateRefreshFunc returns a StateRefreshFunc that is used to watch
@@ -65,9 +65,11 @@ func WaitForState(conf *StateChangeConf) (i interface{}, err error) {
 			return
 		}
 
-		if currentState == conf.Target {
+		for _, t := range conf.Target {
+			if currentState == t {
 				return
 			}
+		}
 
 		if conf.StepState != nil {
 			if _, ok := conf.StepState.GetOk(multistep.StateCancelled); ok {

builder/openstack/ssh.go

@@ -13,49 +13,33 @@ import (
 	"golang.org/x/crypto/ssh"
 )
 
-// SSHAddress returns a function that can be given to the SSH communicator
-// for determining the SSH address based on the server AccessIPv4 setting..
-func SSHAddress(
+// CommHost looks up the host for the communicator.
+func CommHost(
 	client *gophercloud.ServiceClient,
-	sshinterface string, port int) func(multistep.StateBag) (string, error) {
+	sshinterface string) func(multistep.StateBag) (string, error) {
 	return func(state multistep.StateBag) (string, error) {
 		s := state.Get("server").(*servers.Server)
 
+		// If we have a specific interface, try that
+		if sshinterface != "" {
+			if addr := sshAddrFromPool(s, sshinterface); addr != "" {
+				return addr, nil
+			}
+		}
+
 		// If we have a floating IP, use that
 		ip := state.Get("access_ip").(*floatingip.FloatingIP)
 		if ip != nil && ip.IP != "" {
-			return fmt.Sprintf("%s:%d", ip.IP, port), nil
+			return ip.IP, nil
 		}
 
 		if s.AccessIPv4 != "" {
-			return fmt.Sprintf("%s:%d", s.AccessIPv4, port), nil
+			return s.AccessIPv4, nil
 		}
 
-		// Get all the addresses associated with this server. This
-		// was taken directly from Terraform.
-		for _, networkAddresses := range s.Addresses {
-			elements, ok := networkAddresses.([]interface{})
-			if !ok {
-				log.Printf(
-					"[ERROR] Unknown return type for address field: %#v",
-					networkAddresses)
-				continue
-			}
-
-			for _, element := range elements {
-				var addr string
-				address := element.(map[string]interface{})
-				if address["OS-EXT-IPS:type"] == "floating" {
-					addr = address["addr"].(string)
-				} else {
-					if address["version"].(float64) == 4 {
-						addr = address["addr"].(string)
-					}
-				}
-				if addr != "" {
-					return fmt.Sprintf("%s:%d", addr, port), nil
-				}
-			}
+		// Try to get it from the requested interface
+		if addr := sshAddrFromPool(s, sshinterface); addr != "" {
+			return addr, nil
 		}
 
 		s, err := servers.Get(client, s.ID).Extract()
@@ -90,3 +74,42 @@ func SSHConfig(username string) func(multistep.StateBag) (*ssh.ClientConfig, err
 		}, nil
 	}
 }
+
+func sshAddrFromPool(s *servers.Server, desired string) string {
+	// Get all the addresses associated with this server. This
+	// was taken directly from Terraform.
+	for pool, networkAddresses := range s.Addresses {
+		// If we have an SSH interface specified, skip it if no match
+		if desired != "" && pool != desired {
+			log.Printf(
+				"[INFO] Skipping pool %s, doesn't match requested %s",
+				pool, desired)
+			continue
+		}
+
+		elements, ok := networkAddresses.([]interface{})
+		if !ok {
+			log.Printf(
+				"[ERROR] Unknown return type for address field: %#v",
+				networkAddresses)
+			continue
+		}
+
+		for _, element := range elements {
+			var addr string
+			address := element.(map[string]interface{})
+			if address["OS-EXT-IPS:type"] == "floating" {
+				addr = address["addr"].(string)
+			} else {
+				if address["version"].(float64) == 4 {
+					addr = address["addr"].(string)
+				}
+			}
+			if addr != "" {
+				return addr
+			}
+		}
+	}
+
+	return ""
+}

builder/openstack/step_load_extensions.go

+package openstack
+
+import (
+	"fmt"
+	"log"
+
+	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/packer"
+	"github.com/rackspace/gophercloud/openstack/compute/v2/extensions"
+	"github.com/rackspace/gophercloud/pagination"
+)
+
+// StepLoadExtensions gets the FlavorRef from a Flavor. It first assumes
+// that the Flavor is a ref and verifies it. Otherwise, it tries to find
+// the flavor by name.
+type StepLoadExtensions struct{}
+
+func (s *StepLoadExtensions) Run(state multistep.StateBag) multistep.StepAction {
+	config := state.Get("config").(Config)
+	ui := state.Get("ui").(packer.Ui)
+
+	// We need the v2 compute client
+	client, err := config.computeV2Client()
+	if err != nil {
+		err = fmt.Errorf("Error initializing compute client: %s", err)
+		state.Put("error", err)
+		return multistep.ActionHalt
+	}
+
+	ui.Say("Discovering enabled extensions...")
+	result := make(map[string]struct{}, 15)
+	pager := extensions.List(client)
+	err = pager.EachPage(func(p pagination.Page) (bool, error) {
+		// Extract the extensions from this page
+		exts, err := extensions.ExtractExtensions(p)
+		if err != nil {
+			return false, err
+		}
+
+		for _, ext := range exts {
+			log.Printf("[DEBUG] Discovered extension: %s", ext.Alias)
+			result[ext.Alias] = struct{}{}
+		}
+
+		return true, nil
+	})
+	if err != nil {
+		err = fmt.Errorf("Error loading extensions: %s", err)
+		state.Put("error", err)
+		return multistep.ActionHalt
+	}
+
+	state.Put("extensions", result)
+	return multistep.ActionContinue
+}
+
+func (s *StepLoadExtensions) Cleanup(state multistep.StateBag) {
+}

builder/openstack/step_run_source_server.go

@@ -2,6 +2,7 @@ package openstack
 
 import (
 	"fmt"
+	"io/ioutil"
 	"log"
 
 	"github.com/mitchellh/multistep"
@@ -16,6 +17,8 @@ type StepRunSourceServer struct {
 	SecurityGroups   []string
 	Networks         []string
 	AvailabilityZone string
+	UserData         string
+	UserDataFile     string
 
 	server *servers.Server
 }
@@ -39,6 +42,16 @@ func (s *StepRunSourceServer) Run(state multistep.StateBag) multistep.StepAction
 		networks[i].UUID = networkUuid
 	}
 
+	userData := []byte(s.UserData)
+	if s.UserDataFile != "" {
+		userData, err = ioutil.ReadFile(s.UserDataFile)
+		if err != nil {
+			err = fmt.Errorf("Error reading user data file: %s", err)
+			state.Put("error", err)
+			return multistep.ActionHalt
+		}
+	}
+
 	ui.Say("Launching server...")
 	s.server, err = servers.Create(computeClient, keypairs.CreateOptsExt{
 		CreateOptsBuilder: servers.CreateOpts{
@@ -48,6 +61,7 @@ func (s *StepRunSourceServer) Run(state multistep.StateBag) multistep.StepAction
 			SecurityGroups:   s.SecurityGroups,
 			Networks:         networks,
 			AvailabilityZone: s.AvailabilityZone,
+			UserData:         userData,
 		},
 
 		KeyName: keyName,
@@ -65,7 +79,7 @@ func (s *StepRunSourceServer) Run(state multistep.StateBag) multistep.StepAction
 	ui.Say("Waiting for server to become ready...")
 	stateChange := StateChangeConf{
 		Pending:   []string{"BUILD"},
-		Target:    "ACTIVE",
+		Target:    []string{"ACTIVE"},
 		Refresh:   ServerStateRefreshFunc(computeClient, s.server),
 		StepState: state,
 	}
@@ -105,9 +119,9 @@ func (s *StepRunSourceServer) Cleanup(state multistep.StateBag) {
 	}
 
 	stateChange := StateChangeConf{
-		Pending: []string{"ACTIVE", "BUILD", "REBUILD", "SUSPENDED"},
+		Pending: []string{"ACTIVE", "BUILD", "REBUILD", "SUSPENDED", "SHUTOFF", "STOPPED"},
 		Refresh: ServerStateRefreshFunc(computeClient, s.server),
-		Target:  "DELETED",
+		Target:  []string{"DELETED"},
 	}
 
 	WaitForState(&stateChange)

builder/openstack/step_stop_server.go

+package openstack
+
+import (
+	"fmt"
+
+	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/packer"
+	"github.com/rackspace/gophercloud/openstack/compute/v2/extensions/startstop"
+	"github.com/rackspace/gophercloud/openstack/compute/v2/servers"
+)
+
+type StepStopServer struct{}
+
+func (s *StepStopServer) Run(state multistep.StateBag) multistep.StepAction {
+	ui := state.Get("ui").(packer.Ui)
+	config := state.Get("config").(Config)
+	extensions := state.Get("extensions").(map[string]struct{})
+	server := state.Get("server").(*servers.Server)
+
+	// Verify we have the extension
+	if _, ok := extensions["os-server-start-stop"]; !ok {
+		ui.Say("OpenStack cluster doesn't support stop, skipping...")
+		return multistep.ActionContinue
+	}
+
+	// We need the v2 compute client
+	client, err := config.computeV2Client()
+	if err != nil {
+		err = fmt.Errorf("Error initializing compute client: %s", err)
+		state.Put("error", err)
+		return multistep.ActionHalt
+	}
+
+	ui.Say("Stopping server...")
+	if err := startstop.Stop(client, server.ID).ExtractErr(); err != nil {
+		err = fmt.Errorf("Error stopping server: %s", err)
+		state.Put("error", err)
+		return multistep.ActionHalt
+	}
+
+	ui.Message("Waiting for server to stop...")
+	stateChange := StateChangeConf{
+		Pending:   []string{"ACTIVE"},
+		Target:    []string{"SHUTOFF", "STOPPED"},
+		Refresh:   ServerStateRefreshFunc(client, server),
+		StepState: state,
+	}
+	if _, err := WaitForState(&stateChange); err != nil {
+		err := fmt.Errorf("Error waiting for server (%s) to stop: %s", server.ID, err)
+		state.Put("error", err)
+		ui.Error(err.Error())
+		return multistep.ActionHalt
+	}
+
+	return multistep.ActionContinue
+}
+
+func (s *StepStopServer) Cleanup(state multistep.StateBag) {}

builder/parallels/common/ssh.go

 package common
 
 import (
-	"fmt"
-
 	"github.com/mitchellh/multistep"
 	commonssh "github.com/mitchellh/packer/common/ssh"
 	packerssh "github.com/mitchellh/packer/communicator/ssh"
 	"golang.org/x/crypto/ssh"
 )
 
-func SSHAddress(state multistep.StateBag) (string, error) {
+func CommHost(state multistep.StateBag) (string, error) {
 	vmName := state.Get("vmName").(string)
 	driver := state.Get("driver").(Driver)
 
@@ -23,19 +21,19 @@ func SSHAddress(state multistep.StateBag) (string, error) {
 		return "", err
 	}
 
-	return fmt.Sprintf("%s:22", ip), nil
+	return ip, nil
 }
 
 func SSHConfigFunc(config SSHConfig) func(multistep.StateBag) (*ssh.ClientConfig, error) {
 	return func(state multistep.StateBag) (*ssh.ClientConfig, error) {
 		auth := []ssh.AuthMethod{
-			ssh.Password(config.SSHPassword),
+			ssh.Password(config.Comm.SSHPassword),
 			ssh.KeyboardInteractive(
-				packerssh.PasswordKeyboardInteractive(config.SSHPassword)),
+				packerssh.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
 			}
@@ -44,7 +42,7 @@ func SSHConfigFunc(config SSHConfig) func(multistep.StateBag) (*ssh.ClientConfig
 		}
 
 		return &ssh.ClientConfig{
-			User: config.SSHUser,
+			User: config.Comm.SSHUsername,
 			Auth: auth,
 		}, nil
 	}

builder/parallels/common/ssh_config.go

 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 {
-	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"`
+	Comm communicator.Config `mapstructure:",squash"`
 
-	SSHWaitTimeout time.Duration
+	// 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 {
-	if c.SSHPort == 0 {
-		c.SSHPort = 22
-	}
-
-	if c.RawSSHWaitTimeout == "" {
-		c.RawSSHWaitTimeout = "20m"
-	}
-
-	var errs []error
+	// TODO: backwards compatibility, write fixer instead
 	if c.SSHKeyPath != "" {
-		if _, err := os.Stat(c.SSHKeyPath); err != nil {
-			errs = append(errs, fmt.Errorf("ssh_key_path is invalid: %s", err))
-		} else if _, err := commonssh.FileSigner(c.SSHKeyPath); err != nil {
-			errs = append(errs, fmt.Errorf("ssh_key_path is invalid: %s", err))
-		}
+		c.Comm.SSHPrivateKey = c.SSHKeyPath
 	}
-
-	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))
+	if c.SSHWaitTimeout != 0 {
+		c.Comm.SSHTimeout = c.SSHWaitTimeout
 	}
 
-	return errs
+	return c.Comm.Prepare(ctx)
 }

builder/parallels/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",
+		},
 	}
 }
 
@@ -19,8 +23,8 @@ func TestSSHConfigPrepare(t *testing.T) {
 		t.Fatalf("err: %#v", errs)
 	}
 
-	if c.SSHPort != 22 {
-		t.Errorf("bad ssh port: %d", c.SSHPort)
+	if c.Comm.SSHPort != 22 {
+		t.Errorf("bad ssh port: %d", c.Comm.SSHPort)
 	}
 }
 
@@ -78,46 +82,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"
-	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"
+	c.Comm.SSHUsername = "exists"
 	errs = c.Prepare(testConfigTemplate(t))
 	if len(errs) > 0 {
 		t.Fatalf("should not have error: %#v", errs)

builder/parallels/iso/builder.go

@@ -9,6 +9,7 @@ import (
 	"github.com/mitchellh/multistep"
 	parallelscommon "github.com/mitchellh/packer/builder/parallels/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"
@@ -245,10 +246,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{
-			SSHAddress:     parallelscommon.SSHAddress,
+		&communicator.StepConnect{
+			Config:    &b.config.SSHConfig.Comm,
+			Host:      parallelscommon.CommHost,
 			SSHConfig: parallelscommon.SSHConfigFunc(b.config.SSHConfig),
-			SSHWaitTimeout: b.config.SSHWaitTimeout,
 		},
 		&parallelscommon.StepUploadVersion{
 			Path: b.config.PrlctlVersionFile,

builder/parallels/pvm/builder.go

@@ -3,11 +3,13 @@ package pvm
 import (
 	"errors"
 	"fmt"
+	"log"
+
 	"github.com/mitchellh/multistep"
 	parallelscommon "github.com/mitchellh/packer/builder/parallels/common"
 	"github.com/mitchellh/packer/common"
+	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/packer"
-	"log"
 )
 
 // Builder implements packer.Builder and builds the actual Parallels
@@ -80,10 +82,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{
-			SSHAddress:     parallelscommon.SSHAddress,
+		&communicator.StepConnect{
+			Config:    &b.config.SSHConfig.Comm,
+			Host:      parallelscommon.CommHost,
 			SSHConfig: parallelscommon.SSHConfigFunc(b.config.SSHConfig),
-			SSHWaitTimeout: b.config.SSHWaitTimeout,
 		},
 		&parallelscommon.StepUploadVersion{
 			Path: b.config.PrlctlVersionFile,

builder/qemu/builder.go

@@ -12,7 +12,7 @@ import (
 
 	"github.com/mitchellh/multistep"
 	"github.com/mitchellh/packer/common"
-	commonssh "github.com/mitchellh/packer/common/ssh"
+	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/helper/config"
 	"github.com/mitchellh/packer/packer"
 	"github.com/mitchellh/packer/template/interpolate"
@@ -78,6 +78,7 @@ type Builder struct {
 
 type Config struct {
 	common.PackerConfig `mapstructure:",squash"`
+	Comm                communicator.Config `mapstructure:",squash"`
 
 	Accelerator     string     `mapstructure:"accelerator"`
 	BootCommand     []string   `mapstructure:"boot_command"`
@@ -103,25 +104,24 @@ type Config struct {
 	ShutdownCommand string     `mapstructure:"shutdown_command"`
 	SSHHostPortMin  uint       `mapstructure:"ssh_host_port_min"`
 	SSHHostPortMax  uint       `mapstructure:"ssh_host_port_max"`
-	SSHPassword     string     `mapstructure:"ssh_password"`
-	SSHPort         uint       `mapstructure:"ssh_port"`
-	SSHUser         string     `mapstructure:"ssh_username"`
-	SSHKeyPath      string     `mapstructure:"ssh_key_path"`
 	VNCPortMin      uint       `mapstructure:"vnc_port_min"`
 	VNCPortMax      uint       `mapstructure:"vnc_port_max"`
 	VMName          string     `mapstructure:"vm_name"`
 
+	// 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"`
+
 	// TODO(mitchellh): deprecate
 	RunOnce bool `mapstructure:"run_once"`
 
 	RawBootWait        string `mapstructure:"boot_wait"`
 	RawSingleISOUrl    string `mapstructure:"iso_url"`
 	RawShutdownTimeout string `mapstructure:"shutdown_timeout"`
-	RawSSHWaitTimeout  string `mapstructure:"ssh_wait_timeout"`
 
 	bootWait        time.Duration ``
 	shutdownTimeout time.Duration ``
-	sshWaitTimeout  time.Duration ``
 	ctx             interpolate.Context
 }
 
@@ -139,9 +139,6 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		return nil, err
 	}
 
-	var errs *packer.MultiError
-	warnings := make([]string, 0)
-
 	if b.config.DiskSize == 0 {
 		b.config.DiskSize = 40000
 	}
@@ -190,10 +187,6 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		b.config.SSHHostPortMax = 4444
 	}
 
-	if b.config.SSHPort == 0 {
-		b.config.SSHPort = 22
-	}
-
 	if b.config.VNCPortMin == 0 {
 		b.config.VNCPortMin = 5900
 	}
@@ -222,6 +215,21 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		b.config.DiskInterface = "virtio"
 	}
 
+	// TODO: backwards compatibility, write fixer instead
+	if b.config.SSHKeyPath != "" {
+		b.config.Comm.SSHPrivateKey = b.config.SSHKeyPath
+	}
+	if b.config.SSHWaitTimeout != 0 {
+		b.config.Comm.SSHTimeout = b.config.SSHWaitTimeout
+	}
+
+	var errs *packer.MultiError
+	warnings := make([]string, 0)
+
+	if es := b.config.Comm.Prepare(&b.config.ctx); len(es) > 0 {
+		errs = packer.MultiErrorAppend(errs, es...)
+	}
+
 	if !(b.config.Format == "qcow2" || b.config.Format == "raw") {
 		errs = packer.MultiErrorAppend(
 			errs, errors.New("invalid format, only 'qcow2' or 'raw' are allowed"))
@@ -314,42 +322,17 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		b.config.RawShutdownTimeout = "5m"
 	}
 
-	if b.config.RawSSHWaitTimeout == "" {
-		b.config.RawSSHWaitTimeout = "20m"
-	}
-
 	b.config.shutdownTimeout, err = time.ParseDuration(b.config.RawShutdownTimeout)
 	if err != nil {
 		errs = packer.MultiErrorAppend(
 			errs, fmt.Errorf("Failed parsing shutdown_timeout: %s", err))
 	}
 
-	if b.config.SSHKeyPath != "" {
-		if _, err := os.Stat(b.config.SSHKeyPath); err != nil {
-			errs = packer.MultiErrorAppend(
-				errs, fmt.Errorf("ssh_key_path is invalid: %s", err))
-		} else if _, err := commonssh.FileSigner(b.config.SSHKeyPath); err != nil {
-			errs = packer.MultiErrorAppend(
-				errs, fmt.Errorf("ssh_key_path is invalid: %s", err))
-		}
-	}
-
 	if b.config.SSHHostPortMin > b.config.SSHHostPortMax {
 		errs = packer.MultiErrorAppend(
 			errs, errors.New("ssh_host_port_min must be less than ssh_host_port_max"))
 	}
 
-	if b.config.SSHUser == "" {
-		errs = packer.MultiErrorAppend(
-			errs, errors.New("An ssh_username must be specified."))
-	}
-
-	b.config.sshWaitTimeout, err = time.ParseDuration(b.config.RawSSHWaitTimeout)
-	if err != nil {
-		errs = packer.MultiErrorAppend(
-			errs, fmt.Errorf("Failed parsing ssh_wait_timeout: %s", err))
-	}
-
 	if b.config.VNCPortMin > b.config.VNCPortMax {
 		errs = packer.MultiErrorAppend(
 			errs, fmt.Errorf("vnc_port_min must be less than vnc_port_max"))
@@ -409,10 +392,11 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		steprun,
 		&stepBootWait{},
 		&stepTypeBootCommand{},
-		&common.StepConnectSSH{
-			SSHAddress:     sshAddress,
+		&communicator.StepConnect{
+			Config:    &b.config.Comm,
+			Host:      commHost,
 			SSHConfig: sshConfig,
-			SSHWaitTimeout: b.config.sshWaitTimeout,
+			SSHPort:   commPort,
 		},
 		new(common.StepProvision),
 		new(stepShutdown),

builder/qemu/builder_test.go

@@ -79,8 +79,8 @@ func TestBuilderPrepare_Defaults(t *testing.T) {
 		t.Errorf("bad max ssh host port: %d", b.config.SSHHostPortMax)
 	}
 
-	if b.config.SSHPort != 22 {
-		t.Errorf("bad ssh port: %d", b.config.SSHPort)
+	if b.config.Comm.SSHPort != 22 {
+		t.Errorf("bad ssh port: %d", b.config.Comm.SSHPort)
 	}
 
 	if b.config.VMName != "packer-foo" {
@@ -595,10 +595,6 @@ func TestBuilderPrepare_SSHWaitTimeout(t *testing.T) {
 		t.Fatalf("err: %s", err)
 	}
 
-	if b.config.RawSSHWaitTimeout != "20m" {
-		t.Fatalf("bad value: %s", b.config.RawSSHWaitTimeout)
-	}
-
 	// Test with a bad value
 	config["ssh_wait_timeout"] = "this is not good"
 	b = Builder{}

builder/qemu/ssh.go

 package qemu
 
 import (
-	"fmt"
-
 	"github.com/mitchellh/multistep"
 	commonssh "github.com/mitchellh/packer/common/ssh"
 	"github.com/mitchellh/packer/communicator/ssh"
 	gossh "golang.org/x/crypto/ssh"
 )
 
-func sshAddress(state multistep.StateBag) (string, error) {
+func commHost(state multistep.StateBag) (string, error) {
+	return "127.0.0.1", nil
+}
+
+func commPort(state multistep.StateBag) (int, error) {
 	sshHostPort := state.Get("sshHostPort").(uint)
-	return fmt.Sprintf("127.0.0.1:%d", sshHostPort), nil
+	return int(sshHostPort), nil
 }
 
 func sshConfig(state multistep.StateBag) (*gossh.ClientConfig, error) {
 	config := state.Get("config").(*Config)
 
 	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)
+	if config.Comm.SSHPrivateKey != "" {
+		signer, err := commonssh.FileSigner(config.Comm.SSHPrivateKey)
 		if err != nil {
 			return nil, err
 		}
@@ -33,7 +35,7 @@ func sshConfig(state multistep.StateBag) (*gossh.ClientConfig, error) {
 	}
 
 	return &gossh.ClientConfig{
-		User: config.SSHUser,
+		User: config.Comm.SSHUsername,
 		Auth: auth,
 	}, nil
 }

builder/qemu/step_run.go

@@ -80,7 +80,8 @@ func getCommandArgs(bootDrive string, state multistep.StateBag) ([]string, error
 
 	defaultArgs["-name"] = vmName
 	defaultArgs["-machine"] = fmt.Sprintf("type=%s", config.MachineType)
-	defaultArgs["-netdev"] = fmt.Sprintf("user,id=user.0,hostfwd=tcp::%v-:22", sshHostPort)
+	defaultArgs["-netdev"] = fmt.Sprintf(
+		"user,id=user.0,hostfwd=tcp::%v-:%d", sshHostPort, config.Comm.Port())
 	defaultArgs["-device"] = fmt.Sprintf("%s,netdev=user.0", config.NetDevice)
 	defaultArgs["-drive"] = fmt.Sprintf("file=%s,if=%s,cache=%s,discard=%s", imgPath, config.DiskInterface, config.DiskCache, config.DiskDiscard)
 	if !config.DiskImage {

builder/virtualbox/common/output_config.go

@@ -2,7 +2,6 @@ package common
 
 import (
 	"fmt"
-	"os"
 
 	"github.com/mitchellh/packer/common"
 	"github.com/mitchellh/packer/template/interpolate"
@@ -17,13 +16,5 @@ func (c *OutputConfig) Prepare(ctx *interpolate.Context, pc *common.PackerConfig
 		c.OutputDir = fmt.Sprintf("output-%s", pc.PackerBuildName)
 	}
 
-	var errs []error
-	if !pc.PackerForce {
-		if _, err := os.Stat(c.OutputDir); err == nil {
-			errs = append(errs, fmt.Errorf(
-				"Output directory '%s' already exists. It must not exist.", c.OutputDir))
-		}
-	}
-
-	return errs
+	return nil
 }

builder/virtualbox/common/output_config_test.go

@@ -39,27 +39,7 @@ func TestOutputConfigPrepare_exists(t *testing.T) {
 		PackerForce:     false,
 	}
 	errs := c.Prepare(testConfigTemplate(t), pc)
-	if len(errs) == 0 {
-		t.Fatal("should have errors")
-	}
-}
-
-func TestOutputConfigPrepare_forceExists(t *testing.T) {
-	td, err := ioutil.TempDir("", "packer")
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	defer os.RemoveAll(td)
-
-	c := new(OutputConfig)
-	c.OutputDir = td
-
-	pc := &common.PackerConfig{
-		PackerBuildName: "foo",
-		PackerForce:     true,
-	}
-	errs := c.Prepare(testConfigTemplate(t), pc)
-	if len(errs) > 0 {
+	if len(errs) != 0 {
 		t.Fatal("should not have errors")
 	}
 }

builder/virtualbox/common/ssh.go

 package common
 
 import (
-	"fmt"
-
 	"github.com/mitchellh/multistep"
 	commonssh "github.com/mitchellh/packer/common/ssh"
 	"github.com/mitchellh/packer/communicator/ssh"
 	gossh "golang.org/x/crypto/ssh"
 )
 
-func SSHAddress(state multistep.StateBag) (string, error) {
+func CommHost(state multistep.StateBag) (string, error) {
+	return "127.0.0.1", nil
+}
+
+func SSHPort(state multistep.StateBag) (int, error) {
 	sshHostPort := state.Get("sshHostPort").(uint)
-	return fmt.Sprintf("127.0.0.1:%d", sshHostPort), nil
+	return int(sshHostPort), nil
 }
 
 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 +34,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 {
+	Comm communicator.Config `mapstructure:",squash"`
+
 	SSHHostPortMin    uint `mapstructure:"ssh_host_port_min"`
 	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
+	// 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 {
-		c.SSHPort = 22
-	}
-
-	if c.RawSSHWaitTimeout == "" {
-		c.RawSSHWaitTimeout = "20m"
-	}
-
-	var errs []error
+	// TODO: backwards compatibility, write fixer instead
 	if c.SSHKeyPath != "" {
-		if _, err := os.Stat(c.SSHKeyPath); err != nil {
-			errs = append(errs, fmt.Errorf("ssh_key_path is invalid: %s", err))
-		} else if _, err := commonssh.FileSigner(c.SSHKeyPath); err != nil {
-			errs = append(errs, fmt.Errorf("ssh_key_path is invalid: %s", err))
+		c.Comm.SSHPrivateKey = c.SSHKeyPath
 	}
+	if c.SSHWaitTimeout != 0 {
+		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 {
-		t.Errorf("bad ssh port: %d", c.SSHPort)
+	if c.Comm.SSHPort != 22 {
+		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"
-	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"
+	c.Comm.SSHUsername = "exists"
 	errs = c.Prepare(testConfigTemplate(t))
 	if len(errs) > 0 {
 		t.Fatalf("should not have error: %#v", errs)

builder/virtualbox/common/step_forward_ssh.go

@@ -2,11 +2,13 @@ package common
 
 import (
 	"fmt"
-	"github.com/mitchellh/multistep"
-	"github.com/mitchellh/packer/packer"
 	"log"
 	"math/rand"
 	"net"
+
+	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/helper/communicator"
+	"github.com/mitchellh/packer/packer"
 )
 
 // This step adds a NAT port forwarding definition so that SSH is available
@@ -19,7 +21,7 @@ import (
 //
 // Produces:
 type StepForwardSSH struct {
-	GuestPort      uint
+	CommConfig     *communicator.Config
 	HostPortMin    uint
 	HostPortMax    uint
 	SkipNatMapping bool
@@ -30,20 +32,21 @@ func (s *StepForwardSSH) Run(state multistep.StateBag) multistep.StepAction {
 	ui := state.Get("ui").(packer.Ui)
 	vmName := state.Get("vmName").(string)
 
-	sshHostPort := s.GuestPort
+	guestPort := s.CommConfig.Port()
+	sshHostPort := guestPort
 	if !s.SkipNatMapping {
 		log.Printf("Looking for available SSH port between %d and %d",
 			s.HostPortMin, s.HostPortMax)
-		var offset uint = 0
+		offset := 0
 
 		portRange := int(s.HostPortMax - s.HostPortMin)
 		if portRange > 0 {
 			// Have to check if > 0 to avoid a panic
-			offset = uint(rand.Intn(portRange))
+			offset = rand.Intn(portRange)
 		}
 
 		for {
-			sshHostPort = offset + s.HostPortMin
+			sshHostPort = offset + int(s.HostPortMin)
 			log.Printf("Trying port: %d", sshHostPort)
 			l, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", sshHostPort))
 			if err == nil {
@@ -57,7 +60,7 @@ func (s *StepForwardSSH) Run(state multistep.StateBag) multistep.StepAction {
 		command := []string{
 			"modifyvm", vmName,
 			"--natpf1",
-			fmt.Sprintf("packerssh,tcp,127.0.0.1,%d,,%d", sshHostPort, s.GuestPort),
+			fmt.Sprintf("packerssh,tcp,127.0.0.1,%d,,%d", sshHostPort, guestPort),
 		}
 		if err := driver.VBoxManage(command...); err != nil {
 			err := fmt.Errorf("Error creating port forwarding rule: %s", err)

builder/virtualbox/common/step_output_dir.go

@@ -22,7 +22,16 @@ type StepOutputDir struct {
 func (s *StepOutputDir) Run(state multistep.StateBag) multistep.StepAction {
 	ui := state.Get("ui").(packer.Ui)
 
-	if _, err := os.Stat(s.Path); err == nil && s.Force {
+	if _, err := os.Stat(s.Path); err == nil {
+		if !s.Force {
+			err := fmt.Errorf(
+				"Output directory exists: %s\n\n"+
+					"Use the force flag to delete it prior to building.",
+				s.Path)
+			state.Put("error", err)
+			return multistep.ActionHalt
+		}
+
 		ui.Say("Deleting previous output directory...")
 		os.RemoveAll(s.Path)
 	}

builder/virtualbox/common/step_output_dir_test.go

@@ -45,6 +45,30 @@ func TestStepOutputDir(t *testing.T) {
 	}
 }
 
+func TestStepOutputDir_exists(t *testing.T) {
+	state := testState(t)
+	step := testStepOutputDir(t)
+
+	// Make the dir
+	if err := os.MkdirAll(step.Path, 0755); err != nil {
+		t.Fatalf("bad: %s", err)
+	}
+
+	// Test the run
+	if action := step.Run(state); action != multistep.ActionHalt {
+		t.Fatalf("bad action: %#v", action)
+	}
+	if _, ok := state.GetOk("error"); !ok {
+		t.Fatal("should have error")
+	}
+
+	// Test the cleanup
+	step.Cleanup(state)
+	if _, err := os.Stat(step.Path); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+}
+
 func TestStepOutputDir_cancelled(t *testing.T) {
 	state := testState(t)
 	step := testStepOutputDir(t)

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,
+			CommConfig:     &b.config.SSHConfig.Comm,
 			HostPortMin:    b.config.SSHHostPortMin,
 			HostPortMax:    b.config.SSHHostPortMax,
 			SkipNatMapping: b.config.SSHSkipNatMapping,
@@ -271,10 +272,11 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			VMName:      b.config.VMName,
 			Ctx:         b.config.ctx,
 		},
-		&common.StepConnectSSH{
-			SSHAddress:     vboxcommon.SSHAddress,
+		&communicator.StepConnect{
+			Config:    &b.config.SSHConfig.Comm,
+			Host:      vboxcommon.CommHost,
 			SSHConfig: vboxcommon.SSHConfigFunc(b.config.SSHConfig),
-			SSHWaitTimeout: b.config.SSHWaitTimeout,
+			SSHPort:   vboxcommon.SSHPort,
 		},
 		&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,
+			CommConfig:     &b.config.SSHConfig.Comm,
 			HostPortMin:    b.config.SSHHostPortMin,
 			HostPortMax:    b.config.SSHHostPortMax,
 			SkipNatMapping: b.config.SSHSkipNatMapping,
@@ -100,10 +101,11 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			VMName:      b.config.VMName,
 			Ctx:         b.config.ctx,
 		},
-		&common.StepConnectSSH{
-			SSHAddress:     vboxcommon.SSHAddress,
+		&communicator.StepConnect{
+			Config:    &b.config.SSHConfig.Comm,
+			Host:      vboxcommon.CommHost,
 			SSHConfig: vboxcommon.SSHConfigFunc(b.config.SSHConfig),
-			SSHWaitTimeout: b.config.SSHWaitTimeout,
+			SSHPort:   vboxcommon.SSHPort,
 		},
 		&vboxcommon.StepUploadVersion{
 			Path: b.config.VBoxVersionFile,

builder/vmware/common/driver.go

@@ -29,9 +29,9 @@ type Driver interface {
 	// Checks if the VMX file at the given path is running.
 	IsRunning(string) (bool, error)
 
-	// SSHAddress returns the SSH address for the VM that is being
+	// CommHost returns the host address for the VM that is being
 	// managed by this driver.
-	SSHAddress(multistep.StateBag) (string, error)
+	CommHost(multistep.StateBag) (string, error)
 
 	// Start starts a VM specified by the path to the VMX given.
 	Start(string, bool) error

builder/vmware/common/driver_fusion5.go

@@ -69,8 +69,8 @@ func (d *Fusion5Driver) IsRunning(vmxPath string) (bool, error) {
 	return false, nil
 }
 
-func (d *Fusion5Driver) SSHAddress(state multistep.StateBag) (string, error) {
-	return SSHAddressFunc(d.SSHConfig)(state)
+func (d *Fusion5Driver) CommHost(state multistep.StateBag) (string, error) {
+	return CommHost(d.SSHConfig)(state)
 }
 
 func (d *Fusion5Driver) Start(vmxPath string, headless bool) error {

builder/vmware/common/driver_mock.go

@@ -29,10 +29,10 @@ type DriverMock struct {
 	IsRunningResult bool
 	IsRunningErr    error
 
-	SSHAddressCalled bool
-	SSHAddressState  multistep.StateBag
-	SSHAddressResult string
-	SSHAddressErr    error
+	CommHostCalled bool
+	CommHostState  multistep.StateBag
+	CommHostResult string
+	CommHostErr    error
 
 	StartCalled   bool
 	StartPath     string
@@ -92,10 +92,10 @@ func (d *DriverMock) IsRunning(path string) (bool, error) {
 	return d.IsRunningResult, d.IsRunningErr
 }
 
-func (d *DriverMock) SSHAddress(state multistep.StateBag) (string, error) {
-	d.SSHAddressCalled = true
-	d.SSHAddressState = state
-	return d.SSHAddressResult, d.SSHAddressErr
+func (d *DriverMock) CommHost(state multistep.StateBag) (string, error) {
+	d.CommHostCalled = true
+	d.CommHostState = state
+	return d.CommHostResult, d.CommHostErr
 }
 
 func (d *DriverMock) Start(path string, headless bool) error {

builder/vmware/common/driver_player5.go

@@ -97,8 +97,8 @@ func (d *Player5Driver) IsRunning(vmxPath string) (bool, error) {
 	return false, nil
 }
 
-func (d *Player5Driver) SSHAddress(state multistep.StateBag) (string, error) {
-	return SSHAddressFunc(d.SSHConfig)(state)
+func (d *Player5Driver) CommHost(state multistep.StateBag) (string, error) {
+	return CommHost(d.SSHConfig)(state)
 }
 
 func (d *Player5Driver) Start(vmxPath string, headless bool) error {

builder/vmware/common/driver_workstation9.go

@@ -70,8 +70,8 @@ func (d *Workstation9Driver) IsRunning(vmxPath string) (bool, error) {
 	return false, nil
 }
 
-func (d *Workstation9Driver) SSHAddress(state multistep.StateBag) (string, error) {
-	return SSHAddressFunc(d.SSHConfig)(state)
+func (d *Workstation9Driver) CommHost(state multistep.StateBag) (string, error) {
+	return CommHost(d.SSHConfig)(state)
 }
 
 func (d *Workstation9Driver) Start(vmxPath string, headless bool) error {

builder/vmware/common/output_config.go

@@ -2,7 +2,6 @@ package common
 
 import (
 	"fmt"
-	"os"
 
 	"github.com/mitchellh/packer/common"
 	"github.com/mitchellh/packer/template/interpolate"
@@ -17,13 +16,5 @@ func (c *OutputConfig) Prepare(ctx *interpolate.Context, pc *common.PackerConfig
 		c.OutputDir = fmt.Sprintf("output-%s", pc.PackerBuildName)
 	}
 
-	var errs []error
-	if !pc.PackerForce {
-		if _, err := os.Stat(c.OutputDir); err == nil {
-			errs = append(errs, fmt.Errorf(
-				"Output directory '%s' already exists. It must not exist.", c.OutputDir))
-		}
-	}
-
-	return errs
+	return nil
 }

builder/vmware/common/output_config_test.go

 package common
 
 import (
-	"github.com/mitchellh/packer/common"
-	"io/ioutil"
-	"os"
 	"testing"
+
+	"github.com/mitchellh/packer/common"
 )
 
 func TestOutputConfigPrepare(t *testing.T) {
@@ -23,43 +22,3 @@ func TestOutputConfigPrepare(t *testing.T) {
 		t.Fatal("should have output dir")
 	}
 }
-
-func TestOutputConfigPrepare_exists(t *testing.T) {
-	td, err := ioutil.TempDir("", "packer")
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	defer os.RemoveAll(td)
-
-	c := new(OutputConfig)
-	c.OutputDir = td
-
-	pc := &common.PackerConfig{
-		PackerBuildName: "foo",
-		PackerForce:     false,
-	}
-	errs := c.Prepare(testConfigTemplate(t), pc)
-	if len(errs) == 0 {
-		t.Fatal("should have errors")
-	}
-}
-
-func TestOutputConfigPrepare_forceExists(t *testing.T) {
-	td, err := ioutil.TempDir("", "packer")
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	defer os.RemoveAll(td)
-
-	c := new(OutputConfig)
-	c.OutputDir = td
-
-	pc := &common.PackerConfig{
-		PackerBuildName: "foo",
-		PackerForce:     true,
-	}
-	errs := c.Prepare(testConfigTemplate(t), pc)
-	if len(errs) > 0 {
-		t.Fatal("should not have errors")
-	}
-}

builder/vmware/common/ssh.go

@@ -13,13 +13,13 @@ import (
 	gossh "golang.org/x/crypto/ssh"
 )
 
-func SSHAddressFunc(config *SSHConfig) func(multistep.StateBag) (string, error) {
+func CommHost(config *SSHConfig) func(multistep.StateBag) (string, error) {
 	return func(state multistep.StateBag) (string, error) {
 		driver := state.Get("driver").(Driver)
 		vmxPath := state.Get("vmx_path").(string)
 
-		if config.SSHHost != "" {
-			return fmt.Sprintf("%s:%d", config.SSHHost, config.SSHPort), nil
+		if config.Comm.SSHHost != "" {
+			return config.Comm.SSHHost, nil
 		}
 
 		log.Println("Lookup up IP information...")
@@ -62,20 +62,20 @@ func SSHAddressFunc(config *SSHConfig) func(multistep.StateBag) (string, error)
 		}
 
 		log.Printf("Detected IP: %s", ipAddress)
-		return fmt.Sprintf("%s:%d", ipAddress, config.SSHPort), nil
+		return ipAddress, nil
 	}
 }
 
 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)
+		if config.Comm.SSHPrivateKey != "" {
+			signer, err := commonssh.FileSigner(config.Comm.SSHPrivateKey)
 			if err != nil {
 				return nil, err
 			}
@@ -84,7 +84,7 @@ func SSHConfigFunc(config *SSHConfig) func(multistep.StateBag) (*gossh.ClientCon
 		}
 
 		return &gossh.ClientConfig{
-			User: config.SSHUser,
+			User: config.Comm.SSHUsername,
 			Auth: auth,
 		}, nil
 	}

builder/vmware/common/ssh_config.go

 package common
 
 import (
-	"errors"
-	"fmt"
-	"net"
-	"os"
 	"time"
 
-	commonssh "github.com/mitchellh/packer/common/ssh"
+	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/template/interpolate"
 )
 
 type SSHConfig struct {
-	SSHUser           string `mapstructure:"ssh_username"`
+	Comm communicator.Config `mapstructure:",squash"`
+
+	// These are deprecated, but we keep them around for BC
+	// TODO(@mitchellh): remove
 	SSHKeyPath        string        `mapstructure:"ssh_key_path"`
-	SSHPassword       string `mapstructure:"ssh_password"`
-	SSHHost           string `mapstructure:"ssh_host"`
-	SSHPort           uint   `mapstructure:"ssh_port"`
 	SSHSkipRequestPty bool          `mapstructure:"ssh_skip_request_pty"`
-	RawSSHWaitTimeout string `mapstructure:"ssh_wait_timeout"`
-
-	SSHWaitTimeout time.Duration
+	SSHWaitTimeout    time.Duration `mapstructure:"ssh_wait_timeout"`
 }
 
 func (c *SSHConfig) Prepare(ctx *interpolate.Context) []error {
-	if c.SSHPort == 0 {
-		c.SSHPort = 22
-	}
-
-	if c.RawSSHWaitTimeout == "" {
-		c.RawSSHWaitTimeout = "20m"
-	}
-
-	var errs []error
+	// TODO: backwards compatibility, write fixer instead
 	if c.SSHKeyPath != "" {
-		if _, err := os.Stat(c.SSHKeyPath); err != nil {
-			errs = append(errs, fmt.Errorf("ssh_key_path is invalid: %s", err))
-		} else if _, err := commonssh.FileSigner(c.SSHKeyPath); err != nil {
-			errs = append(errs, fmt.Errorf("ssh_key_path is invalid: %s", err))
+		c.Comm.SSHPrivateKey = c.SSHKeyPath
 	}
+	if c.SSHWaitTimeout != 0 {
+		c.Comm.SSHTimeout = c.SSHWaitTimeout
 	}
-
-	if c.SSHHost != "" {
-		if ip := net.ParseIP(c.SSHHost); ip == nil {
-			if _, err := net.LookupHost(c.SSHHost); err != nil {
-				errs = append(errs, errors.New("ssh_host is an invalid IP or hostname"))
-			}
-		}
-	}
-
-	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))
+	if c.SSHSkipRequestPty {
+		c.Comm.SSHPty = false
 	}
 
-	return errs
+	return c.Comm.Prepare(ctx)
 }

builder/vmware/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",
+		},
 	}
 }
 
@@ -19,8 +23,8 @@ func TestSSHConfigPrepare(t *testing.T) {
 		t.Fatalf("err: %#v", errs)
 	}
 
-	if c.SSHPort != 22 {
-		t.Errorf("bad ssh port: %d", c.SSHPort)
+	if c.Comm.SSHPort != 22 {
+		t.Errorf("bad ssh port: %d", c.Comm.SSHPort)
 	}
 }
 
@@ -73,57 +77,6 @@ func TestSSHConfigPrepare_SSHKeyPath(t *testing.T) {
 	}
 }
 
-func TestSSHConfigPrepare_SSHUser(t *testing.T) {
-	var c *SSHConfig
-	var errs []error
-
-	c = testSSHConfig()
-	c.SSHUser = ""
-	errs = c.Prepare(testConfigTemplate(t))
-	if len(errs) == 0 {
-		t.Fatalf("should have error")
-	}
-
-	c = testSSHConfig()
-	c.SSHUser = "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)
-	}
-}
-
 const testPem = `
 -----BEGIN RSA PRIVATE KEY-----
 MIIEpQIBAAKCAQEAxd4iamvrwRJvtNDGQSIbNvvIQN8imXTRWlRY62EvKov60vqu

builder/vmware/iso/builder.go

@@ -13,6 +13,7 @@ import (
 	"github.com/mitchellh/multistep"
 	vmwcommon "github.com/mitchellh/packer/builder/vmware/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"
@@ -298,11 +299,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{
-			SSHAddress:     driver.SSHAddress,
+		&communicator.StepConnect{
+			Config:    &b.config.SSHConfig.Comm,
+			Host:      driver.CommHost,
 			SSHConfig: vmwcommon.SSHConfigFunc(&b.config.SSHConfig),
-			SSHWaitTimeout: b.config.SSHWaitTimeout,
-			NoPty:          b.config.SSHSkipRequestPty,
 		},
 		&vmwcommon.StepUploadTools{
 			RemoteType:        b.config.RemoteType,

builder/vmware/iso/builder_test.go

 package iso
 
 import (
-	"github.com/mitchellh/packer/packer"
 	"io/ioutil"
 	"os"
 	"reflect"
 	"testing"
-	"time"
+
+	"github.com/mitchellh/packer/packer"
 )
 
 func testConfig() map[string]interface{} {
@@ -138,10 +138,6 @@ func TestBuilderPrepare_Defaults(t *testing.T) {
 		t.Errorf("bad Version: %s", b.config.Version)
 	}
 
-	if b.config.SSHWaitTimeout != (20 * time.Minute) {
-		t.Errorf("bad wait timeout: %s", b.config.SSHWaitTimeout)
-	}
-
 	if b.config.VMName != "packer-foo" {
 		t.Errorf("bad vm name: %s", b.config.VMName)
 	}
@@ -353,8 +349,8 @@ func TestBuilderPrepare_OutputDir(t *testing.T) {
 	if len(warns) > 0 {
 		t.Fatalf("bad: %#v", warns)
 	}
-	if err == nil {
-		t.Fatal("should have error")
+	if err != nil {
+		t.Fatalf("err: %s", err)
 	}
 
 	// Test with a good one

builder/vmware/iso/driver_esx5.go

@@ -218,7 +218,7 @@ func (d *ESX5Driver) VNCAddress(portMin, portMax uint) (string, uint, error) {
 	return d.Host, vncPort, nil
 }
 
-func (d *ESX5Driver) SSHAddress(state multistep.StateBag) (string, error) {
+func (d *ESX5Driver) CommHost(state multistep.StateBag) (string, error) {
 	config := state.Get("config").(*Config)
 
 	if address, ok := state.GetOk("vm_address"); ok {
@@ -253,7 +253,7 @@ func (d *ESX5Driver) SSHAddress(state multistep.StateBag) (string, error) {
 		return "", errors.New("VM network port found, but no IP address")
 	}
 
-	address := fmt.Sprintf("%s:%d", record["IPAddress"], config.SSHPort)
+	address := record["IPAddress"]
 	state.Put("vm_address", address)
 	return address, nil
 }
@@ -335,7 +335,6 @@ func (d *ESX5Driver) connect() error {
 			User: d.Username,
 			Auth: auth,
 		},
-		NoPty: true,
 	}
 
 	comm, err := ssh.New(address, sshConfig)

builder/vmware/vmx/builder.go

@@ -9,6 +9,7 @@ import (
 	"github.com/mitchellh/multistep"
 	vmwcommon "github.com/mitchellh/packer/builder/vmware/common"
 	"github.com/mitchellh/packer/common"
+	"github.com/mitchellh/packer/helper/communicator"
 	"github.com/mitchellh/packer/packer"
 )
 
@@ -90,11 +91,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{
-			SSHAddress:     driver.SSHAddress,
+		&communicator.StepConnect{
+			Config:    &b.config.SSHConfig.Comm,
+			Host:      driver.CommHost,
 			SSHConfig: vmwcommon.SSHConfigFunc(&b.config.SSHConfig),
-			SSHWaitTimeout: b.config.SSHWaitTimeout,
-			NoPty:          b.config.SSHSkipRequestPty,
 		},
 		&vmwcommon.StepUploadTools{
 			RemoteType:        b.config.RemoteType,

command/fix.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 
 	"github.com/mitchellh/packer/fix"
+	"github.com/mitchellh/packer/template"
 )
 
 type FixCommand struct {
@@ -16,7 +17,9 @@ type FixCommand struct {
 }
 
 func (c *FixCommand) Run(args []string) int {
+	var flagValidate bool
 	flags := c.Meta.FlagSet("fix", FlagSetNone)
+	flags.BoolVar(&flagValidate, "validate", true, "")
 	flags.Usage = func() { c.Ui.Say(c.Help()) }
 	if err := flags.Parse(args); err != nil {
 		return 1
@@ -80,6 +83,28 @@ func (c *FixCommand) Run(args []string) int {
 	result = strings.Replace(result, `\u003c`, "<", -1)
 	result = strings.Replace(result, `\u003e`, ">", -1)
 	c.Ui.Say(result)
+
+	if flagValidate {
+		// Attemot to parse and validate the template
+		tpl, err := template.Parse(strings.NewReader(result))
+		if err != nil {
+			c.Ui.Error(fmt.Sprintf(
+				"Error! Fixed template fails to parse: %s\n\n"+
+					"This is usually caused by an error in the input template.\n"+
+					"Please fix the error and try again.",
+				err))
+			return 1
+		}
+		if err := tpl.Validate(); err != nil {
+			c.Ui.Error(fmt.Sprintf(
+				"Error! Fixed template failed to validate: %s\n\n"+
+					"This is usually caused by an error in the input template.\n"+
+					"Please fix the error and try again.",
+				err))
+			return 1
+		}
+	}
+
 	return 0
 }
 
@@ -102,6 +127,10 @@ Fixes that are run:
   pp-vagrant-override Replaces old-style provider overrides for the Vagrant
                       post-processor to new-style as of Packer 0.5.0.
   virtualbox-rename   Updates "virtualbox" builders to "virtualbox-iso"
+
+Options:
+
+  -validate=true      If true (default), validates the fixed template.
 `
 
 	return strings.TrimSpace(helpText)

command/fix_test.go

+package command
+
+import (
+	"path/filepath"
+	"testing"
+)
+
+func TestFix_noArgs(t *testing.T) {
+	c := &PushCommand{Meta: testMeta(t)}
+	code := c.Run(nil)
+	if code != 1 {
+		t.Fatalf("bad: %#v", code)
+	}
+}
+
+func TestFix_multiArgs(t *testing.T) {
+	c := &PushCommand{Meta: testMeta(t)}
+	code := c.Run([]string{"one", "two"})
+	if code != 1 {
+		t.Fatalf("bad: %#v", code)
+	}
+}
+
+func TestFix(t *testing.T) {
+	c := &FixCommand{
+		Meta: testMeta(t),
+	}
+
+	args := []string{filepath.Join(testFixture("fix"), "template.json")}
+	if code := c.Run(args); code != 0 {
+		fatalCommand(t, c.Meta)
+	}
+}
+
+func TestFix_invalidTemplate(t *testing.T) {
+	c := &FixCommand{
+		Meta: testMeta(t),
+	}
+
+	args := []string{filepath.Join(testFixture("fix-invalid"), "template.json")}
+	if code := c.Run(args); code != 1 {
+		fatalCommand(t, c.Meta)
+	}
+}
+
+func TestFix_invalidTemplateDisableValidation(t *testing.T) {
+	c := &FixCommand{
+		Meta: testMeta(t),
+	}
+
+	args := []string{
+		"-validate=false",
+		filepath.Join(testFixture("fix-invalid"), "template.json"),
+	}
+	if code := c.Run(args); code != 0 {
+		fatalCommand(t, c.Meta)
+	}
+}

command/test-fixtures/fix-invalid/template.json

+{
+    "hello": "world"
+}

command/test-fixtures/fix/template.json

+{
+    "builders": [{"type": "dummy"}],
+
+    "push": {
+        "name": "foo/bar"
+    }
+}

common/config.go

@@ -99,6 +99,14 @@ func DownloadableURL(original string) (string, error) {
 	// Make sure it is lowercased
 	url.Scheme = strings.ToLower(url.Scheme)
 
+	// This is to work around issue #5927. This can safely be removed once
+	// we distribute with a version of Go that fixes that bug.
+	//
+	// See: https://code.google.com/p/go/issues/detail?id=5927
+	if url.Path != "" && url.Path[0] != '/' {
+		url.Path = "/" + url.Path
+	}
+
 	// Verify that the scheme is something we support in our common downloader.
 	supported := []string{"file", "http", "https"}
 	found := false

common/step_connect_ssh_test.go

-package common
-
-import (
-	"github.com/mitchellh/multistep"
-	"testing"
-)
-
-func TestStepConnectSSH_Impl(t *testing.T) {
-	var raw interface{}
-	raw = new(StepConnectSSH)
-	if _, ok := raw.(multistep.Step); !ok {
-		t.Fatalf("connect ssh should be a step")
-	}
-}

communicator/ssh/communicator.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"github.com/mitchellh/packer/packer"
 	"golang.org/x/crypto/ssh"
+	"golang.org/x/crypto/ssh/agent"
 	"io"
 	"io/ioutil"
 	"log"
@@ -33,8 +34,8 @@ type Config struct {
 	// case an error occurs.
 	Connection func() (net.Conn, error)
 
-	// NoPty, if true, will not request a pty from the remote end.
-	NoPty bool
+	// Pty, if true, will request a pty from the remote end.
+	Pty bool
 }
 
 // Creates a new packer.Communicator implementation over SSH. This takes
@@ -65,7 +66,7 @@ func (c *comm) Start(cmd *packer.RemoteCmd) (err error) {
 	session.Stdout = cmd.Stdout
 	session.Stderr = cmd.Stderr
 
-	if !c.config.NoPty {
+	if c.config.Pty {
 		// Request a PTY
 		termModes := ssh.TerminalModes{
 			ssh.ECHO:          0,     // do not echo
@@ -226,10 +227,59 @@ func (c *comm) reconnect() (err error) {
 	if sshConn != nil {
 		c.client = ssh.NewClient(sshConn, sshChan, req)
 	}
+	c.connectToAgent()
 
 	return
 }
 
+func (c *comm) connectToAgent() {
+	if c.client == nil {
+		return
+	}
+
+	// open connection to the local agent
+	socketLocation := os.Getenv("SSH_AUTH_SOCK")
+	if socketLocation == "" {
+		log.Printf("[INFO] no local agent socket, will not connect agent")
+		return
+	}
+	agentConn, err := net.Dial("unix", socketLocation)
+	if err != nil {
+		log.Printf("[ERROR] could not connect to local agent socket: %s", socketLocation)
+		return
+	}
+
+	// create agent and add in auth
+	forwardingAgent := agent.NewClient(agentConn)
+	if forwardingAgent == nil {
+		log.Printf("[ERROR] Could not create agent client")
+		agentConn.Close()
+		return
+	}
+
+	// add callback for forwarding agent to SSH config
+	// XXX - might want to handle reconnects appending multiple callbacks
+	auth := ssh.PublicKeysCallback(forwardingAgent.Signers)
+	c.config.SSHConfig.Auth = append(c.config.SSHConfig.Auth, auth)
+	agent.ForwardToAgent(c.client, forwardingAgent)
+
+	// Setup a session to request agent forwarding
+	session, err := c.newSession()
+	if err != nil {
+		return
+	}
+	defer session.Close()
+
+	err = agent.RequestAgentForwarding(session)
+	if err != nil {
+		log.Printf("[ERROR] RequestAgentForwarding: %#v", err)
+		return
+	}
+
+	log.Printf("[INFO] agent forwarding enabled")
+	return
+}
+
 func (c *comm) scpSession(scpCommand string, f func(io.Writer, *bufio.Reader) error) error {
 	session, err := c.newSession()
 	if err != nil {

communicator/winrm/communicator.go

+package winrm
+
+import (
+	"fmt"
+	"io"
+	"log"
+	"os"
+
+	"github.com/masterzen/winrm/winrm"
+	"github.com/mitchellh/packer/packer"
+	"github.com/packer-community/winrmcp/winrmcp"
+
+	// This import is a bit strange, but it's needed so `make updatedeps`
+	// can see and download it
+	_ "github.com/dylanmei/winrmtest"
+)
+
+// Communicator represents the WinRM communicator
+type Communicator struct {
+	config   *Config
+	client   *winrm.Client
+	endpoint *winrm.Endpoint
+}
+
+// New creates a new communicator implementation over WinRM.
+func New(config *Config) (*Communicator, error) {
+	endpoint := &winrm.Endpoint{
+		Host: config.Host,
+		Port: config.Port,
+
+		/*
+			TODO
+			HTTPS:    connInfo.HTTPS,
+			Insecure: connInfo.Insecure,
+			CACert:   connInfo.CACert,
+		*/
+	}
+
+	// Create the client
+	params := winrm.DefaultParameters()
+	params.Timeout = formatDuration(config.Timeout)
+	client, err := winrm.NewClientWithParameters(
+		endpoint, config.Username, config.Password, params)
+	if err != nil {
+		return nil, err
+	}
+
+	// Create the shell to verify the connection
+	log.Printf("[DEBUG] connecting to remote shell using WinRM")
+	shell, err := client.CreateShell()
+	if err != nil {
+		log.Printf("[ERROR] connection error: %s", err)
+		return nil, err
+	}
+
+	if err := shell.Close(); err != nil {
+		log.Printf("[ERROR] error closing connection: %s", err)
+		return nil, err
+	}
+
+	return &Communicator{
+		config:   config,
+		client:   client,
+		endpoint: endpoint,
+	}, nil
+}
+
+// Start implementation of communicator.Communicator interface
+func (c *Communicator) Start(rc *packer.RemoteCmd) error {
+	shell, err := c.client.CreateShell()
+	if err != nil {
+		return err
+	}
+
+	log.Printf("[INFO] starting remote command: %s", rc.Command)
+	cmd, err := shell.Execute(rc.Command)
+	if err != nil {
+		return err
+	}
+
+	go runCommand(shell, cmd, rc)
+	return nil
+}
+
+func runCommand(shell *winrm.Shell, cmd *winrm.Command, rc *packer.RemoteCmd) {
+	defer shell.Close()
+
+	go io.Copy(rc.Stdout, cmd.Stdout)
+	go io.Copy(rc.Stderr, cmd.Stderr)
+
+	cmd.Wait()
+	rc.SetExited(cmd.ExitCode())
+}
+
+// Upload implementation of communicator.Communicator interface
+func (c *Communicator) Upload(path string, input io.Reader, _ *os.FileInfo) error {
+	wcp, err := c.newCopyClient()
+	if err != nil {
+		return err
+	}
+	log.Printf("Uploading file to '%s'", path)
+	return wcp.Write(path, input)
+}
+
+// UploadDir implementation of communicator.Communicator interface
+func (c *Communicator) UploadDir(dst string, src string, exclude []string) error {
+	log.Printf("Uploading dir '%s' to '%s'", src, dst)
+	wcp, err := c.newCopyClient()
+	if err != nil {
+		return err
+	}
+	return wcp.Copy(src, dst)
+}
+
+func (c *Communicator) Download(src string, dst io.Writer) error {
+	panic("download not implemented")
+}
+
+func (c *Communicator) newCopyClient() (*winrmcp.Winrmcp, error) {
+	addr := fmt.Sprintf("%s:%d", c.endpoint.Host, c.endpoint.Port)
+	return winrmcp.New(addr, &winrmcp.Config{
+		Auth: winrmcp.Auth{
+			User:     c.config.Username,
+			Password: c.config.Password,
+		},
+		OperationTimeout:      c.config.Timeout,
+		MaxOperationsPerShell: 15, // lowest common denominator
+	})
+}

communicator/winrm/communicator_test.go

+package winrm
+
+import (
+	"bytes"
+	"io"
+	"testing"
+	"time"
+
+	"github.com/dylanmei/winrmtest"
+	"github.com/mitchellh/packer/packer"
+)
+
+func newMockWinRMServer(t *testing.T) *winrmtest.Remote {
+	wrm := winrmtest.NewRemote()
+
+	wrm.CommandFunc(
+		winrmtest.MatchText("echo foo"),
+		func(out, err io.Writer) int {
+			out.Write([]byte("foo"))
+			return 0
+		})
+
+	wrm.CommandFunc(
+		winrmtest.MatchPattern(`^echo c29tZXRoaW5n >> ".*"$`),
+		func(out, err io.Writer) int {
+			return 0
+		})
+
+	wrm.CommandFunc(
+		winrmtest.MatchPattern(`^powershell.exe -EncodedCommand .*$`),
+		func(out, err io.Writer) int {
+			return 0
+		})
+
+	wrm.CommandFunc(
+		winrmtest.MatchText("powershell"),
+		func(out, err io.Writer) int {
+			return 0
+		})
+
+	return wrm
+}
+
+func TestStart(t *testing.T) {
+	wrm := newMockWinRMServer(t)
+	defer wrm.Close()
+
+	c, err := New(&Config{
+		Host:     wrm.Host,
+		Port:     wrm.Port,
+		Username: "user",
+		Password: "pass",
+		Timeout:  30 * time.Second,
+	})
+	if err != nil {
+		t.Fatalf("error creating communicator: %s", err)
+	}
+
+	var cmd packer.RemoteCmd
+	stdout := new(bytes.Buffer)
+	cmd.Command = "echo foo"
+	cmd.Stdout = stdout
+
+	err = c.Start(&cmd)
+	if err != nil {
+		t.Fatalf("error executing remote command: %s", err)
+	}
+	cmd.Wait()
+
+	if stdout.String() != "foo" {
+		t.Fatalf("bad command response: expected %q, got %q", "foo", stdout.String())
+	}
+}
+
+func TestUpload(t *testing.T) {
+	wrm := newMockWinRMServer(t)
+	defer wrm.Close()
+
+	c, err := New(&Config{
+		Host:     wrm.Host,
+		Port:     wrm.Port,
+		Username: "user",
+		Password: "pass",
+		Timeout:  30 * time.Second,
+	})
+	if err != nil {
+		t.Fatalf("error creating communicator: %s", err)
+	}
+
+	err = c.Upload("C:/Temp/terraform.cmd", bytes.NewReader([]byte("something")), nil)
+	if err != nil {
+		t.Fatalf("error uploading file: %s", err)
+	}
+}

communicator/winrm/config.go

+package winrm
+
+import (
+	"time"
+)
+
+// Config is used to configure the WinRM connection
+type Config struct {
+	Host     string
+	Port     int
+	Username string
+	Password string
+	Timeout  time.Duration
+}

communicator/winrm/time.go

+package winrm
+
+import (
+	"fmt"
+	"time"
+)
+
+// formatDuration formats the given time.Duration into an ISO8601
+// duration string.
+func formatDuration(duration time.Duration) string {
+	// We're not supporting negative durations
+	if duration.Seconds() <= 0 {
+		return "PT0S"
+	}
+
+	h := int(duration.Hours())
+	m := int(duration.Minutes()) - (h * 60)
+	s := int(duration.Seconds()) - (h*3600 + m*60)
+
+	res := "PT"
+	if h > 0 {
+		res = fmt.Sprintf("%s%dH", res, h)
+	}
+	if m > 0 {
+		res = fmt.Sprintf("%s%dM", res, m)
+	}
+	if s > 0 {
+		res = fmt.Sprintf("%s%dS", res, s)
+	}
+
+	return res
+}

communicator/winrm/time_test.go

+package winrm
+
+import (
+	"testing"
+	"time"
+)
+
+func TestFormatDuration(t *testing.T) {
+	// Test complex duration with hours, minutes, seconds
+	d := time.Duration(3701) * time.Second
+	s := formatDuration(d)
+	if s != "PT1H1M41S" {
+		t.Fatalf("bad ISO 8601 duration string: %s", s)
+	}
+
+	// Test only minutes duration
+	d = time.Duration(20) * time.Minute
+	s = formatDuration(d)
+	if s != "PT20M" {
+		t.Fatalf("bad ISO 8601 duration string for 20M: %s", s)
+	}
+
+	// Test only seconds
+	d = time.Duration(1) * time.Second
+	s = formatDuration(d)
+	if s != "PT1S" {
+		t.Fatalf("bad ISO 8601 duration string for 1S: %s", s)
+	}
+
+	// Test negative duration (unsupported)
+	d = time.Duration(-1) * time.Second
+	s = formatDuration(d)
+	if s != "PT0S" {
+		t.Fatalf("bad ISO 8601 duration string for negative: %s", s)
+	}
+}

fix/fixer_virtualbox_rename.go

@@ -8,14 +8,14 @@ import (
 type FixerVirtualBoxRename struct{}
 
 func (FixerVirtualBoxRename) Fix(input map[string]interface{}) (map[string]interface{}, error) {
-	// The type we'll decode into; we only care about builders
 	type template struct {
 		Builders     []map[string]interface{}
+		Provisioners []interface{}
 	}
 
 	// Decode the input into our structure, if we can
 	var tpl template
-	if err := mapstructure.Decode(input, &tpl); err != nil {
+	if err := mapstructure.WeakDecode(input, &tpl); err != nil {
 		return nil, err
 	}
 
@@ -37,7 +37,39 @@ func (FixerVirtualBoxRename) Fix(input map[string]interface{}) (map[string]inter
 		builder["type"] = "virtualbox-iso"
 	}
 
+	for i, raw := range tpl.Provisioners {
+		var m map[string]interface{}
+		if err := mapstructure.WeakDecode(raw, &m); err != nil {
+			// Ignore errors, could be a non-map
+			continue
+		}
+
+		raw, ok := m["override"]
+		if !ok {
+			continue
+		}
+
+		var override map[string]interface{}
+		if err := mapstructure.WeakDecode(raw, &override); err != nil {
+			return nil, err
+		}
+
+		if raw, ok := override["virtualbox"]; ok {
+			override["virtualbox-iso"] = raw
+			delete(override, "virtualbox")
+
+			// Set the change
+			m["override"] = override
+			tpl.Provisioners[i] = m
+		}
+	}
+
+	if len(tpl.Builders) > 0 {
 		input["builders"] = tpl.Builders
+	}
+	if len(tpl.Provisioners) > 0 {
+		input["provisioners"] = tpl.Provisioners
+	}
 	return input, nil
 }
 

fix/fixer_virtualbox_rename_test.go

@@ -46,3 +46,45 @@ func TestFixerVirtualBoxRename_Fix(t *testing.T) {
 		}
 	}
 }
+
+func TestFixerVirtualBoxRenameFix_provisionerOverride(t *testing.T) {
+	cases := []struct {
+		Input    map[string]interface{}
+		Expected map[string]interface{}
+	}{
+		{
+			Input: map[string]interface{}{
+				"provisioners": []interface{}{
+					map[string]interface{}{
+						"override": map[string]interface{}{
+							"virtualbox": map[string]interface{}{},
+						},
+					},
+				},
+			},
+
+			Expected: map[string]interface{}{
+				"provisioners": []interface{}{
+					map[string]interface{}{
+						"override": map[string]interface{}{
+							"virtualbox-iso": map[string]interface{}{},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		var f FixerVirtualBoxRename
+
+		output, err := f.Fix(tc.Input)
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
+
+		if !reflect.DeepEqual(output, tc.Expected) {
+			t.Fatalf("unexpected:\n\n%#v\nexpected:\n\n%#v\n", output, tc.Expected)
+		}
+	}
+}

helper/builder/testing/testing.go

@@ -41,6 +41,10 @@ type TestCase struct {
 	// in the case that the test can't guarantee all resources were
 	// properly cleaned up.
 	Teardown TestTeardownFunc
+
+	// If SkipArtifactTeardown is true, we will not attempt to destroy the
+	// artifact created in this test run.
+	SkipArtifactTeardown bool
 }
 
 // TestCheckFunc is the callback used for Check in TestStep.
@@ -163,6 +167,7 @@ func Test(t TestT, c TestCase) {
 	}
 
 TEARDOWN:
+	if !c.SkipArtifactTeardown {
 		// Delete all artifacts
 		for _, a := range artifacts {
 			if err := a.Destroy(); err != nil {
@@ -171,6 +176,7 @@ TEARDOWN:
 					a.String(), err))
 			}
 		}
+	}
 
 	// Teardown
 	if c.Teardown != nil {

helper/communicator/config.go

+package communicator
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"time"
+
+	"github.com/mitchellh/packer/template/interpolate"
+)
+
+// Config is the common configuration that communicators allow within
+// a builder.
+type Config struct {
+	Type string `mapstructure:"communicator"`
+
+	// SSH
+	SSHHost              string        `mapstructure:"ssh_host"`
+	SSHPort              int           `mapstructure:"ssh_port"`
+	SSHUsername          string        `mapstructure:"ssh_username"`
+	SSHPassword          string        `mapstructure:"ssh_password"`
+	SSHPrivateKey        string        `mapstructure:"ssh_private_key_file"`
+	SSHPty               bool          `mapstructure:"ssh_pty"`
+	SSHTimeout           time.Duration `mapstructure:"ssh_timeout"`
+	SSHHandshakeAttempts int           `mapstructure:"ssh_handshake_attempts"`
+
+	// WinRM
+	WinRMUser     string        `mapstructure:"winrm_username"`
+	WinRMPassword string        `mapstructure:"winrm_password"`
+	WinRMHost     string        `mapstructure:"winrm_host"`
+	WinRMPort     int           `mapstructure:"winrm_port"`
+	WinRMTimeout  time.Duration `mapstructure:"winrm_timeout"`
+}
+
+// Port returns the port that will be used for access based on config.
+func (c *Config) Port() int {
+	switch c.Type {
+	case "ssh":
+		return c.SSHPort
+	case "winrm":
+		return c.WinRMPort
+	default:
+		return 0
+	}
+}
+
+func (c *Config) Prepare(ctx *interpolate.Context) []error {
+	if c.Type == "" {
+		c.Type = "ssh"
+	}
+
+	var errs []error
+	switch c.Type {
+	case "ssh":
+		if es := c.prepareSSH(ctx); len(es) > 0 {
+			errs = append(errs, es...)
+		}
+	case "winrm":
+		if es := c.prepareWinRM(ctx); len(es) > 0 {
+			errs = append(errs, es...)
+		}
+	}
+
+	return errs
+}
+
+func (c *Config) prepareSSH(ctx *interpolate.Context) []error {
+	if c.SSHPort == 0 {
+		c.SSHPort = 22
+	}
+
+	if c.SSHTimeout == 0 {
+		c.SSHTimeout = 5 * time.Minute
+	}
+
+	if c.SSHHandshakeAttempts == 0 {
+		c.SSHHandshakeAttempts = 10
+	}
+
+	// Validation
+	var errs []error
+	if c.SSHUsername == "" {
+		errs = append(errs, errors.New("An ssh_username must be specified"))
+	}
+
+	if c.SSHPrivateKey != "" {
+		if _, err := os.Stat(c.SSHPrivateKey); err != nil {
+			errs = append(errs, fmt.Errorf(
+				"ssh_private_key_file is invalid: %s", err))
+		} else if _, err := SSHFileSigner(c.SSHPrivateKey); err != nil {
+			errs = append(errs, fmt.Errorf(
+				"ssh_private_key_file is invalid: %s", err))
+		}
+	}
+
+	return errs
+}
+
+func (c *Config) prepareWinRM(ctx *interpolate.Context) []error {
+	if c.WinRMPort == 0 {
+		c.WinRMPort = 5985
+	}
+
+	if c.WinRMTimeout == 0 {
+		c.WinRMTimeout = 30 * time.Minute
+	}
+
+	var errs []error
+	if c.WinRMUser == "" {
+		errs = append(errs, errors.New("winrm_username must be specified."))
+	}
+
+	return errs
+}

helper/communicator/config_test.go

+package communicator
+
+import (
+	"testing"
+
+	"github.com/mitchellh/packer/template/interpolate"
+)
+
+func testConfig() *Config {
+	return &Config{
+		SSHUsername: "root",
+	}
+}
+
+func TestConfigType(t *testing.T) {
+	c := testConfig()
+	if err := c.Prepare(testContext(t)); len(err) > 0 {
+		t.Fatalf("bad: %#v", err)
+	}
+
+	if c.Type != "ssh" {
+		t.Fatalf("bad: %#v", c)
+	}
+}
+
+func TestConfig_none(t *testing.T) {
+	c := &Config{Type: "none"}
+	if err := c.Prepare(testContext(t)); len(err) > 0 {
+		t.Fatalf("bad: %#v", err)
+	}
+}
+
+func testContext(t *testing.T) *interpolate.Context {
+	return nil
+}

helper/communicator/ssh.go

+package communicator
+
+import (
+	"encoding/pem"
+	"fmt"
+	"io/ioutil"
+	"os"
+
+	"golang.org/x/crypto/ssh"
+)
+
+// SSHFileSigner returns an ssh.Signer for a key file.
+func SSHFileSigner(path string) (ssh.Signer, error) {
+	f, err := os.Open(path)
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+
+	keyBytes, err := ioutil.ReadAll(f)
+	if err != nil {
+		return nil, err
+	}
+
+	// We parse the private key on our own first so that we can
+	// show a nicer error if the private key has a password.
+	block, _ := pem.Decode(keyBytes)
+	if block == nil {
+		return nil, fmt.Errorf(
+			"Failed to read key '%s': no key found", path)
+	}
+	if block.Headers["Proc-Type"] == "4,ENCRYPTED" {
+		return nil, fmt.Errorf(
+			"Failed to read key '%s': password protected keys are\n"+
+				"not supported. Please decrypt the key prior to use.", path)
+	}
+
+	signer, err := ssh.ParsePrivateKey(keyBytes)
+	if err != nil {
+		return nil, fmt.Errorf("Error setting up SSH config: %s", err)
+	}
+
+	return signer, nil
+}

helper/communicator/step_connect.go

+package communicator
+
+import (
+	"fmt"
+	"log"
+
+	"github.com/mitchellh/multistep"
+	gossh "golang.org/x/crypto/ssh"
+)
+
+// StepConnect is a multistep Step implementation that connects to
+// the proper communicator and stores it in the "communicator" key in the
+// state bag.
+type StepConnect struct {
+	// Config is the communicator config struct
+	Config *Config
+
+	// Host should return a host that can be connected to for communicator
+	// connections.
+	Host func(multistep.StateBag) (string, error)
+
+	// The fields below are callbacks to assist with connecting to SSH.
+	//
+	// SSHConfig should return the default configuration for
+	// connecting via SSH.
+	SSHConfig func(multistep.StateBag) (*gossh.ClientConfig, error)
+	SSHPort   func(multistep.StateBag) (int, error)
+
+	// The fields below are callbacks to assist with connecting to WinRM.
+	//
+	// WinRMConfig should return the default configuration for
+	// connecting via WinRM.
+	WinRMConfig func(multistep.StateBag) (*WinRMConfig, error)
+
+	// CustomConnect can be set to have custom connectors for specific
+	// types. These take highest precedence so you can also override
+	// existing types.
+	CustomConnect map[string]multistep.Step
+
+	substep multistep.Step
+}
+
+func (s *StepConnect) Run(state multistep.StateBag) multistep.StepAction {
+	typeMap := map[string]multistep.Step{
+		"none": nil,
+		"ssh": &StepConnectSSH{
+			Config:    s.Config,
+			Host:      s.Host,
+			SSHConfig: s.SSHConfig,
+			SSHPort:   s.SSHPort,
+		},
+		"winrm": &StepConnectWinRM{
+			Config:      s.Config,
+			Host:        s.Host,
+			WinRMConfig: s.WinRMConfig,
+		},
+	}
+	for k, v := range s.CustomConnect {
+		typeMap[k] = v
+	}
+
+	step, ok := typeMap[s.Config.Type]
+	if !ok {
+		state.Put("error", fmt.Errorf("unknown communicator type: %s", s.Config.Type))
+		return multistep.ActionHalt
+	}
+
+	if step == nil {
+		log.Printf("[INFO] communicator disabled, will not connect")
+		return multistep.ActionContinue
+	}
+
+	s.substep = step
+	return s.substep.Run(state)
+}
+
+func (s *StepConnect) Cleanup(state multistep.StateBag) {
+	if s.substep != nil {
+		s.substep.Cleanup(state)
+	}
+}

common/step_connect_ssh.go → helper/communicator/step_connect_ssh.go

-package common
+package communicator
 
 import (
 	"errors"
@@ -13,32 +13,15 @@ import (
 	gossh "golang.org/x/crypto/ssh"
 )
 
-// StepConnectSSH is a multistep Step implementation that waits for SSH
-// to become available. It gets the connection information from a single
-// configuration when creating the step.
+// StepConnectSSH is a step that only connects to SSH.
 //
-// Uses:
-//   ui packer.Ui
-//
-// Produces:
-//   communicator packer.Communicator
+// In general, you should use StepConnect.
 type StepConnectSSH struct {
-	// SSHAddress is a function that returns the TCP address to connect to
-	// for SSH. This is a function so that you can query information
-	// if necessary for this address.
-	SSHAddress func(multistep.StateBag) (string, error)
-
-	// SSHConfig is a function that returns the proper client configuration
-	// for SSH access.
+	// All the fields below are documented on StepConnect
+	Config    *Config
+	Host      func(multistep.StateBag) (string, error)
 	SSHConfig func(multistep.StateBag) (*gossh.ClientConfig, error)
-
-	// SSHWaitTimeout is the total timeout to wait for SSH to become available.
-	SSHWaitTimeout time.Duration
-
-	// NoPty, if true, will not request a Pty from the remote end.
-	NoPty bool
-
-	comm packer.Communicator
+	SSHPort   func(multistep.StateBag) (int, error)
 }
 
 func (s *StepConnectSSH) Run(state multistep.StateBag) multistep.StepAction {
@@ -55,8 +38,8 @@ func (s *StepConnectSSH) Run(state multistep.StateBag) multistep.StepAction {
 		waitDone <- true
 	}()
 
-	log.Printf("Waiting for SSH, up to timeout: %s", s.SSHWaitTimeout)
-	timeout := time.After(s.SSHWaitTimeout)
+	log.Printf("[INFO] Waiting for SSH, up to timeout: %s", s.Config.SSHTimeout)
+	timeout := time.After(s.Config.SSHTimeout)
 WaitLoop:
 	for {
 		// Wait for either SSH to become available, a timeout to occur,
@@ -70,7 +53,6 @@ WaitLoop:
 			}
 
 			ui.Say("Connected to SSH!")
-			s.comm = comm
 			state.Put("communicator", comm)
 			break WaitLoop
 		case <-timeout:
@@ -84,7 +66,7 @@ WaitLoop:
 				// The step sequence was cancelled, so cancel waiting for SSH
 				// and just start the halting process.
 				close(cancel)
-				log.Println("Interrupt detected, quitting waiting for SSH.")
+				log.Println("[WARN] Interrupt detected, quitting waiting for SSH.")
 				return multistep.ActionHalt
 			}
 		}
@@ -106,7 +88,7 @@ func (s *StepConnectSSH) waitForSSH(state multistep.StateBag, cancel <-chan stru
 		if !first {
 			select {
 			case <-cancel:
-				log.Println("SSH wait cancelled. Exiting loop.")
+				log.Println("[DEBUG] SSH wait cancelled. Exiting loop.")
 				return nil, errors.New("SSH wait cancelled")
 			case <-time.After(5 * time.Second):
 			}
@@ -114,24 +96,34 @@ func (s *StepConnectSSH) waitForSSH(state multistep.StateBag, cancel <-chan stru
 		first = false
 
 		// First we request the TCP connection information
-		address, err := s.SSHAddress(state)
+		host, err := s.Host(state)
 		if err != nil {
-			log.Printf("Error getting SSH address: %s", err)
+			log.Printf("[DEBUG] Error getting SSH address: %s", err)
 			continue
 		}
+		port := s.Config.SSHPort
+		if s.SSHPort != nil {
+			port, err = s.SSHPort(state)
+			if err != nil {
+				log.Printf("[DEBUG] Error getting SSH port: %s", err)
+				continue
+			}
+		}
 
 		// Retrieve the SSH configuration
 		sshConfig, err := s.SSHConfig(state)
 		if err != nil {
-			log.Printf("Error getting SSH config: %s", err)
+			log.Printf("[DEBUG] Error getting SSH config: %s", err)
 			continue
 		}
 
+		address := fmt.Sprintf("%s:%d", host, port)
+
 		// Attempt to connect to SSH port
 		connFunc := ssh.ConnectFunc("tcp", address)
 		nc, err := connFunc()
 		if err != nil {
-			log.Printf("TCP connection to SSH ip/port failed: %s", err)
+			log.Printf("[DEBUG] TCP connection to SSH ip/port failed: %s", err)
 			continue
 		}
 		nc.Close()
@@ -140,24 +132,27 @@ func (s *StepConnectSSH) waitForSSH(state multistep.StateBag, cancel <-chan stru
 		config := &ssh.Config{
 			Connection: connFunc,
 			SSHConfig:  sshConfig,
-			NoPty:      s.NoPty,
+			Pty:        s.Config.SSHPty,
 		}
 
-		log.Println("Attempting SSH connection...")
+		log.Println("[INFO] Attempting SSH connection...")
 		comm, err = ssh.New(address, config)
 		if err != nil {
-			log.Printf("SSH handshake err: %s", err)
+			log.Printf("[DEBUG] SSH handshake err: %s", err)
 
 			// Only count this as an attempt if we were able to attempt
 			// to authenticate. Note this is very brittle since it depends
 			// on the string of the error... but I don't see any other way.
 			if strings.Contains(err.Error(), "authenticate") {
-				log.Printf("Detected authentication error. Increasing handshake attempts.")
+				log.Printf(
+					"[DEBUG] Detected authentication error. Increasing handshake attempts.")
 				handshakeAttempts += 1
 			}
 
-			if handshakeAttempts < 10 {
-				// Try to connect via SSH a handful of times
+			if handshakeAttempts < s.Config.SSHHandshakeAttempts {
+				// Try to connect via SSH a handful of times. We sleep here
+				// so we don't get a ton of authentication errors back to back.
+				time.Sleep(2 * time.Second)
 				continue
 			}
 

helper/communicator/step_connect_test.go

+package communicator
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/mitchellh/multistep"
+	"github.com/mitchellh/packer/packer"
+)
+
+func TestStepConnect_impl(t *testing.T) {
+	var _ multistep.Step = new(StepConnect)
+}
+
+func TestStepConnect_none(t *testing.T) {
+	state := testState(t)
+
+	step := &StepConnect{
+		Config: &Config{
+			Type: "none",
+		},
+	}
+	defer step.Cleanup(state)
+
+	// run the step
+	if action := step.Run(state); action != multistep.ActionContinue {
+		t.Fatalf("bad action: %#v", action)
+	}
+}
+
+func testState(t *testing.T) multistep.StateBag {
+	state := new(multistep.BasicStateBag)
+	state.Put("hook", &packer.MockHook{})
+	state.Put("ui", &packer.BasicUi{
+		Reader: new(bytes.Buffer),
+		Writer: new(bytes.Buffer),
+	})
+	return state
+}