Merge branch 'master' into aws-filters

.travis.yml

@@ -6,8 +6,7 @@ sudo: false
 language: go
 
 go:
-  - 1.9.x
-  - 1.10.x
+  - 1.11.x
   - master
 
 install:

CHANGELOG.md

@@ -2,23 +2,29 @@
 
 ### IMPROVEMENTS:
 
+* builder/alicloud: Support source image coming from marketplace [GH-6588]
 * builder/amazon-chroot: New feature `root_volume_tags` to tag the created
     volumes. [GH-6504]
 * builder/azure: Implement clean_image_name template engine. [GH-6558]
-* builder/digitalocean: Add support for tagging to instances [GH-6546]
 * builder/cloudstack: Add option to use a fixed port via public_port. [GH-6532]
+* builder/digitalocean: Add support for tagging to instances [GH-6546]
+* builder/googlecompute: Add new `min_cpu_platform` feature [GH-6607]
 * builder/lxc: Allow unplivileged LXC containers. [GH-6279]
 * builder/oci: Add `metadata` feature to Packer config. [GH-6498]
+* builder/openstack: Add support for getting config from clouds-public.yaml.
+    [GH-6595]
 * builder/openstack: Add support for ports. [GH-6570]
-* builder/openstack: Add support for getting config from clouds-public.yaml. [GH-6595]
+* builder/openstack: Add support for source_image_filter. [GH-6490]
+* builder/openstack: Migrate floating IP usage to Network v2 API from Compute
+    API. [GH-6373]
 * builder/openstack: Support Block Storage volumes as boot volume. [GH-6596]
-* builder/openstack: Migrate floating IP usage to Network v2 API from Compute API. [GH-6373]
-| builder/openstack: Add support for source_image_filter. [GH-6490]
 * builder/qemu: add ssh agent support. [GH-6541]
 * builder/qemu: New `use_backing_file` feature [GH-6249]
 * builder/vmware-iso: Try to use ISO files uploaded to the datastore when
     building remotely instead of uploading them freshly every time [GH-5165]
 * command/validate: Warn users if config needs fixing. [GH-6423]
+* core: Add progress-bar to download step. [GH-5851]
+* core: Deduplicate ui and log lines that stream to terminal [GH-6611]
 * post-processor/vagrant: Support for Docker images. [GH-6494]
 * postprocessor/vagrant: Add support for Azure. [GH-6576]
 * provisioner/ansible: Enable {{.WinRMPassword}} template engine. [GH-6450]
@@ -34,6 +40,8 @@
 * core: Better error handling in downloader when connection error occurs.
     [GH-6557]
 * core: Fix broken pathing checks in checksum files. [GH-6525]
+* provisioner/windows-restart: Provisioner now works when used in conjuction
+    with SSH communicator [GH-6606]
 
 ### BACKWARDS INCOMPATIBILITIES:
 * builder/amazon: "owners" field on source_ami_filter is now required for

Makefile

@@ -11,7 +11,7 @@ GOPATH=$(shell go env GOPATH)
 # gofmt
 UNFORMATTED_FILES=$(shell find . -not -path "./vendor/*" -name "*.go" | xargs gofmt -s -l)
 
-EXECUTABLE_FILES=$(shell find . -type f -perm +111 | egrep -v '^\./(vendor/|\.git|bin/|scripts/|pkg/)' | egrep -v '.*(\.sh|\.bats)' | egrep -v './provisioner/ansible/test-fixtures/exit1')
+EXECUTABLE_FILES=$(shell find . -type f -perm +111 | egrep -v '^\./(website/vendor|vendor/|\.git|bin/|scripts/|pkg/)' | egrep -v '.*(\.sh|\.bats|\.git)' | egrep -v './provisioner/ansible/test-fixtures/exit1')
 
 # Get the git commit
 GIT_DIRTY=$(shell test -n "`git status --porcelain`" && echo "+CHANGES" || true)
@@ -50,7 +50,6 @@ deps:
 	@go get -u github.com/mna/pigeon
 	@go get github.com/kardianos/govendor
 	@go get golang.org/x/tools/cmd/goimports
-	@govendor sync
 
 dev: deps ## Build and install a development build
 	@grep 'const VersionPrerelease = ""' version/version.go > /dev/null ; if [ $$? -eq 0 ]; then \

appveyor.yml

@@ -16,8 +16,55 @@ environment:
 clone_folder: c:\gopath\src\github.com\hashicorp\packer
 
 install:
+  - ps: |
+        # Installs golang on Windows.
+        #
+        # # Run script:
+        # .\install-go.ps1
+        #
+        # # Download and run script:
+        # $env:GOVERSION = '1.5.3'
+        # iex ((new-object net.webclient).DownloadString('SCRIPT_URL_HERE'))
+
+        $version = '1.11'
+
+        $downloadDir = $env:TEMP
+        $packageName = 'golang'
+        $url32 = 'https://storage.googleapis.com/golang/go' + $version + '.windows-386.zip'
+        $url64 = 'https://storage.googleapis.com/golang/go' + $version + '.windows-amd64.zip'
+        $goroot = "C:\go$version"
+
+        # Determine type of system
+        if ($ENV:PROCESSOR_ARCHITECTURE -eq "AMD64") {
+          $url = $url64
+        } else {
+          $url = $url32
+        }
+
+        if (Test-Path "$goroot\bin\go") {
+          Write-Host "Go is installed to $goroot"
+          exit
+        }
+
+        echo "Downloading $url"
+        $zip = "$downloadDir\golang-$version.zip"
+        if (!(Test-Path "$zip")) {
+          $downloader = new-object System.Net.WebClient
+          $downloader.DownloadFile($url, $zip)
+        }
+
+        echo "Extracting $zip to $goroot"
+        if (Test-Path "$downloadDir\go") {
+          rm -Force -Recurse -Path "$downloadDir\go"
+        }
+        Add-Type -AssemblyName System.IO.Compression.FileSystem
+        [System.IO.Compression.ZipFile]::ExtractToDirectory("$zip", $downloadDir)
+        mv "$downloadDir\go" $goroot
   - set GO15VENDOREXPERIMENT=1
+  - set GOROOT=C:\go1.11
+  - set Path=%GOROOT%\bin;%PATH%
   - echo %Path%
+  - echo %GOROOT%
   - go version
   - go env
   - go get github.com/mitchellh/gox

builder/alicloud/ecs/builder.go

@@ -67,7 +67,8 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		return nil, errs
 	}
 
-	log.Println(common.ScrubConfig(b.config, b.config.AlicloudAccessKey, b.config.AlicloudSecretKey))
+	packer.LogSecretFilter.Set(b.config.AlicloudAccessKey, b.config.AlicloudSecretKey)
+	log.Println(b.config)
 	return nil, nil
 }
 
@@ -95,13 +96,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			SourceECSImageId: b.config.AlicloudSourceImage,
 		},
 		&stepConfigAlicloudKeyPair{
-			Debug:                b.config.PackerDebug,
-			KeyPairName:          b.config.SSHKeyPairName,
-			PrivateKeyFile:       b.config.Comm.SSHPrivateKey,
-			TemporaryKeyPairName: b.config.TemporaryKeyPairName,
-			SSHAgentAuth:         b.config.Comm.SSHAgentAuth,
-			DebugKeyPath:         fmt.Sprintf("ecs_%s.pem", b.config.PackerBuildName),
-			RegionId:             b.config.AlicloudRegion,
+			Debug:        b.config.PackerDebug,
+			Comm:         &b.config.Comm,
+			DebugKeyPath: fmt.Sprintf("ecs_%s.pem", b.config.PackerBuildName),
+			RegionId:     b.config.AlicloudRegion,
 		},
 	}
 	if b.chooseNetworkType() == VpcNet {
@@ -156,10 +154,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			Host: SSHHost(
 				client,
 				b.config.SSHPrivateIp),
-			SSHConfig: SSHConfig(
-				b.config.RunConfig.Comm.SSHAgentAuth,
-				b.config.RunConfig.Comm.SSHUsername,
-				b.config.RunConfig.Comm.SSHPassword),
+			SSHConfig: b.config.RunConfig.Comm.SSHConfigFunc(),
 		},
 		&common.StepProvision{},
 		&stepStopAlicloudInstance{
@@ -232,7 +227,7 @@ func (b *Builder) isVpcSpecified() bool {
 
 func (b *Builder) isUserDataNeeded() bool {
 	// Public key setup requires userdata
-	if b.config.RunConfig.Comm.SSHPrivateKey != "" {
+	if b.config.RunConfig.Comm.SSHPrivateKeyFile != "" {
 		return true
 	}
 
@@ -240,5 +235,5 @@ func (b *Builder) isUserDataNeeded() bool {
 }
 
 func (b *Builder) isKeyPairNeeded() bool {
-	return b.config.SSHKeyPairName != "" || b.config.TemporaryKeyPairName != ""
+	return b.config.Comm.SSHKeyPairName != "" || b.config.Comm.SSHTemporaryKeyPairName != ""
 }

builder/alicloud/ecs/run_config.go

@@ -31,19 +31,17 @@ type RunConfig struct {
 	InstanceName             string `mapstructure:"instance_name"`
 	InternetChargeType       string `mapstructure:"internet_charge_type"`
 	InternetMaxBandwidthOut  int    `mapstructure:"internet_max_bandwidth_out"`
-	TemporaryKeyPairName     string `mapstructure:"temporary_key_pair_name"`
 
 	// Communicator settings
-	Comm           communicator.Config `mapstructure:",squash"`
-	SSHKeyPairName string              `mapstructure:"ssh_keypair_name"`
-	SSHPrivateIp   bool                `mapstructure:"ssh_private_ip"`
+	Comm         communicator.Config `mapstructure:",squash"`
+	SSHPrivateIp bool                `mapstructure:"ssh_private_ip"`
 }
 
 func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
-	if c.SSHKeyPairName == "" && c.TemporaryKeyPairName == "" &&
-		c.Comm.SSHPrivateKey == "" && c.Comm.SSHPassword == "" && c.Comm.WinRMPassword == "" {
+	if c.Comm.SSHKeyPairName == "" && c.Comm.SSHTemporaryKeyPairName == "" &&
+		c.Comm.SSHPrivateKeyFile == "" && c.Comm.SSHPassword == "" && c.Comm.WinRMPassword == "" {
 
-		c.TemporaryKeyPairName = fmt.Sprintf("packer_%s", uuid.TimeOrderedUUID())
+		c.Comm.SSHTemporaryKeyPairName = fmt.Sprintf("packer_%s", uuid.TimeOrderedUUID())
 	}
 
 	// Validation

builder/alicloud/ecs/run_config_test.go

@@ -105,21 +105,21 @@ func TestRunConfigPrepare_UserDataFile(t *testing.T) {
 
 func TestRunConfigPrepare_TemporaryKeyPairName(t *testing.T) {
 	c := testConfig()
-	c.TemporaryKeyPairName = ""
+	c.Comm.SSHTemporaryKeyPairName = ""
 	if err := c.Prepare(nil); len(err) != 0 {
 		t.Fatalf("err: %s", err)
 	}
 
-	if c.TemporaryKeyPairName == "" {
+	if c.Comm.SSHTemporaryKeyPairName == "" {
 		t.Fatal("keypair name is empty")
 	}
 
-	c.TemporaryKeyPairName = "ssh-key-123"
+	c.Comm.SSHTemporaryKeyPairName = "ssh-key-123"
 	if err := c.Prepare(nil); len(err) != 0 {
 		t.Fatalf("err: %s", err)
 	}
 
-	if c.TemporaryKeyPairName != "ssh-key-123" {
+	if c.Comm.SSHTemporaryKeyPairName != "ssh-key-123" {
 		t.Fatal("keypair name does not match")
 	}
 }

builder/alicloud/ecs/ssh_helper.go

@@ -1,15 +1,9 @@
 package ecs
 
 import (
-	"fmt"
-	"net"
-	"os"
 	"time"
 
-	packerssh "github.com/hashicorp/packer/communicator/ssh"
 	"github.com/hashicorp/packer/helper/multistep"
-	"golang.org/x/crypto/ssh"
-	"golang.org/x/crypto/ssh/agent"
 )
 
 var (
@@ -27,57 +21,3 @@ func SSHHost(e alicloudSSHHelper, private bool) func(multistep.StateBag) (string
 		return ipAddress, nil
 	}
 }
-
-// SSHConfig returns a function that can be used for the SSH communicator
-// config for connecting to the instance created over SSH using the private key
-// or password.
-func SSHConfig(useAgent bool, username, password string) func(multistep.StateBag) (*ssh.ClientConfig, error) {
-	return func(state multistep.StateBag) (*ssh.ClientConfig, error) {
-		if useAgent {
-			authSock := os.Getenv("SSH_AUTH_SOCK")
-			if authSock == "" {
-				return nil, fmt.Errorf("SSH_AUTH_SOCK is not set")
-			}
-
-			sshAgent, err := net.Dial("unix", authSock)
-			if err != nil {
-				return nil, fmt.Errorf("Cannot connect to SSH Agent socket %q: %s", authSock, err)
-			}
-
-			return &ssh.ClientConfig{
-				User: username,
-				Auth: []ssh.AuthMethod{
-					ssh.PublicKeysCallback(agent.NewClient(sshAgent).Signers),
-				},
-				HostKeyCallback: ssh.InsecureIgnoreHostKey(),
-			}, nil
-		}
-
-		privateKey, hasKey := state.GetOk("privateKey")
-		if hasKey {
-
-			signer, err := ssh.ParsePrivateKey([]byte(privateKey.(string)))
-			if err != nil {
-				return nil, fmt.Errorf("Error setting up SSH config: %s", err)
-			}
-			return &ssh.ClientConfig{
-				User: username,
-				Auth: []ssh.AuthMethod{
-					ssh.PublicKeys(signer),
-				},
-				HostKeyCallback: ssh.InsecureIgnoreHostKey(),
-			}, nil
-
-		} else {
-			return &ssh.ClientConfig{
-				User: username,
-				Auth: []ssh.AuthMethod{
-					ssh.Password(password),
-					ssh.KeyboardInteractive(
-						packerssh.PasswordKeyboardInteractive(password)),
-				},
-				HostKeyCallback: ssh.InsecureIgnoreHostKey(),
-			}, nil
-		}
-	}
-}

builder/alicloud/ecs/step_attach_keypair.go

@@ -16,15 +16,15 @@ type stepAttachKeyPair struct {
 }
 
 func (s *stepAttachKeyPair) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
-	keyPairName := state.Get("keyPair").(string)
-	if keyPairName == "" {
-		return multistep.ActionContinue
-	}
 	ui := state.Get("ui").(packer.Ui)
 	client := state.Get("client").(*ecs.Client)
 	config := state.Get("config").(Config)
 	instance := state.Get("instance").(*ecs.InstanceAttributesType)
 	timeoutPoint := time.Now().Add(120 * time.Second)
+	keyPairName := config.Comm.SSHKeyPairName
+	if keyPairName == "" {
+		return multistep.ActionContinue
+	}
 	for {
 		err := client.AttachKeyPair(&ecs.AttachKeyPairArgs{RegionId: common.Region(config.AlicloudRegion),
 			KeyPairName: keyPairName, InstanceIds: "[\"" + instance.InstanceId + "\"]"})
@@ -51,14 +51,14 @@ func (s *stepAttachKeyPair) Run(_ context.Context, state multistep.StateBag) mul
 }
 
 func (s *stepAttachKeyPair) Cleanup(state multistep.StateBag) {
-	keyPairName := state.Get("keyPair").(string)
-	if keyPairName == "" {
-		return
-	}
 	client := state.Get("client").(*ecs.Client)
 	config := state.Get("config").(Config)
 	ui := state.Get("ui").(packer.Ui)
 	instance := state.Get("instance").(*ecs.InstanceAttributesType)
+	keyPairName := config.Comm.SSHKeyPairName
+	if keyPairName == "" {
+		return
+	}
 
 	err := client.DetachKeyPair(&ecs.DetachKeyPairArgs{RegionId: common.Region(config.AlicloudRegion),
 		KeyPairName: keyPairName, InstanceIds: "[\"" + instance.InstanceId + "\"]"})

builder/alicloud/ecs/step_config_key_pair.go

@@ -9,18 +9,16 @@ import (
 
 	"github.com/denverdino/aliyungo/common"
 	"github.com/denverdino/aliyungo/ecs"
+	"github.com/hashicorp/packer/helper/communicator"
 	"github.com/hashicorp/packer/helper/multistep"
 	"github.com/hashicorp/packer/packer"
 )
 
 type stepConfigAlicloudKeyPair struct {
-	Debug                bool
-	SSHAgentAuth         bool
-	DebugKeyPath         string
-	TemporaryKeyPairName string
-	KeyPairName          string
-	PrivateKeyFile       string
-	RegionId             string
+	Debug        bool
+	Comm         *communicator.Config
+	DebugKeyPath string
+	RegionId     string
 
 	keyName string
 }
@@ -28,43 +26,41 @@ type stepConfigAlicloudKeyPair struct {
 func (s *stepConfigAlicloudKeyPair) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
 	ui := state.Get("ui").(packer.Ui)
 
-	if s.PrivateKeyFile != "" {
+	if s.Comm.SSHPrivateKeyFile != "" {
 		ui.Say("Using existing SSH private key")
-		privateKeyBytes, err := ioutil.ReadFile(s.PrivateKeyFile)
+		privateKeyBytes, err := ioutil.ReadFile(s.Comm.SSHPrivateKeyFile)
 		if err != nil {
 			state.Put("error", fmt.Errorf(
 				"Error loading configured private key file: %s", err))
 			return multistep.ActionHalt
 		}
 
-		state.Put("keyPair", s.KeyPairName)
-		state.Put("privateKey", string(privateKeyBytes))
+		s.Comm.SSHPrivateKey = privateKeyBytes
 
 		return multistep.ActionContinue
 	}
 
-	if s.SSHAgentAuth && s.KeyPairName == "" {
+	if s.Comm.SSHAgentAuth && s.Comm.SSHKeyPairName == "" {
 		ui.Say("Using SSH Agent with key pair in source image")
 		return multistep.ActionContinue
 	}
 
-	if s.SSHAgentAuth && s.KeyPairName != "" {
-		ui.Say(fmt.Sprintf("Using SSH Agent for existing key pair %s", s.KeyPairName))
-		state.Put("keyPair", s.KeyPairName)
+	if s.Comm.SSHAgentAuth && s.Comm.SSHKeyPairName != "" {
+		ui.Say(fmt.Sprintf("Using SSH Agent for existing key pair %s", s.Comm.SSHKeyPairName))
 		return multistep.ActionContinue
 	}
 
-	if s.TemporaryKeyPairName == "" {
+	if s.Comm.SSHTemporaryKeyPairName == "" {
 		ui.Say("Not using temporary keypair")
-		state.Put("keyPair", "")
+		s.Comm.SSHKeyPairName = ""
 		return multistep.ActionContinue
 	}
 
 	client := state.Get("client").(*ecs.Client)
 
-	ui.Say(fmt.Sprintf("Creating temporary keypair: %s", s.TemporaryKeyPairName))
+	ui.Say(fmt.Sprintf("Creating temporary keypair: %s", s.Comm.SSHTemporaryKeyPairName))
 	keyResp, err := client.CreateKeyPair(&ecs.CreateKeyPairArgs{
-		KeyPairName: s.TemporaryKeyPairName,
+		KeyPairName: s.Comm.SSHTemporaryKeyPairName,
 		RegionId:    common.Region(s.RegionId),
 	})
 	if err != nil {
@@ -73,11 +69,11 @@ func (s *stepConfigAlicloudKeyPair) Run(_ context.Context, state multistep.State
 	}
 
 	// Set the keyname so we know to delete it later
-	s.keyName = s.TemporaryKeyPairName
+	s.keyName = s.Comm.SSHTemporaryKeyPairName
 
 	// Set some state data for use in future steps
-	state.Put("keyPair", s.keyName)
-	state.Put("privateKey", keyResp.PrivateKeyBody)
+	s.Comm.SSHKeyPairName = s.keyName
+	s.Comm.SSHPrivateKey = []byte(keyResp.PrivateKeyBody)
 
 	// If we're in debug mode, output the private key to the working
 	// directory.
@@ -112,7 +108,7 @@ func (s *stepConfigAlicloudKeyPair) Cleanup(state multistep.StateBag) {
 	// If no key name is set, then we never created it, so just return
 	// If we used an SSH private key file, do not go about deleting
 	// keypairs
-	if s.PrivateKeyFile != "" || (s.KeyPairName == "" && s.keyName == "") {
+	if s.Comm.SSHPrivateKeyFile != "" || (s.Comm.SSHKeyPairName == "" && s.keyName == "") {
 		return
 	}
 

builder/amazon/chroot/builder.go

@@ -42,6 +42,7 @@ type Config struct {
 	PreMountCommands  []string                   `mapstructure:"pre_mount_commands"`
 	RootDeviceName    string                     `mapstructure:"root_device_name"`
 	RootVolumeSize    int64                      `mapstructure:"root_volume_size"`
+	RootVolumeType    string                     `mapstructure:"root_volume_type"`
 	SourceAmi         string                     `mapstructure:"source_ami"`
 	SourceAmiFilter   awscommon.AmiFilterOptions `mapstructure:"source_ami_filter"`
 	RootVolumeTags    awscommon.TagMap           `mapstructure:"root_volume_tags"`
@@ -176,7 +177,8 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		return warns, errs
 	}
 
-	log.Println(common.ScrubConfig(b.config, b.config.AccessKey, b.config.SecretKey, b.config.Token))
+	packer.LogSecretFilter.Set(b.config.AccessKey, b.config.SecretKey, b.config.Token)
+	log.Println(b.config)
 	return warns, nil
 }
 
@@ -222,6 +224,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 				EnableAMISriovNetSupport: b.config.AMISriovNetSupport,
 				EnableAMIENASupport:      b.config.AMIENASupport,
 				AmiFilters:               b.config.SourceAmiFilter,
+				AMIVirtType:              b.config.AMIVirtType,
 			},
 			&StepCheckRootDevice{},
 		)
@@ -231,6 +234,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		&StepFlock{},
 		&StepPrepareDevice{},
 		&StepCreateVolume{
+			RootVolumeType: b.config.RootVolumeType,
 			RootVolumeSize: b.config.RootVolumeSize,
 			RootVolumeTags: b.config.RootVolumeTags,
 			Ctx:            b.config.ctx,

builder/amazon/chroot/communicator.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"syscall"
 
@@ -23,8 +24,10 @@ type Communicator struct {
 }
 
 func (c *Communicator) Start(cmd *packer.RemoteCmd) error {
+	// need extra escapes for the command since we're wrapping it in quotes
+	cmd.Command = strconv.Quote(cmd.Command)
 	command, err := c.CmdWrapper(
-		fmt.Sprintf("chroot %s /bin/sh -c \"%s\"", c.Chroot, cmd.Command))
+		fmt.Sprintf("chroot %s /bin/sh -c %s", c.Chroot, cmd.Command))
 	if err != nil {
 		return err
 	}

builder/amazon/chroot/step_create_volume.go

@@ -2,6 +2,7 @@ package chroot
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log"
 
@@ -21,6 +22,7 @@ import (
 type StepCreateVolume struct {
 	volumeId       string
 	RootVolumeSize int64
+	RootVolumeType string
 	RootVolumeTags awscommon.TagMap
 	Ctx            interpolate.Context
 }
@@ -53,11 +55,21 @@ func (s *StepCreateVolume) Run(ctx context.Context, state multistep.StateBag) mu
 
 	var createVolume *ec2.CreateVolumeInput
 	if config.FromScratch {
+		rootVolumeType := ec2.VolumeTypeGp2
+		if s.RootVolumeType == "io1" {
+			err := errors.New("Cannot use io1 volume when building from scratch")
+			state.Put("error", err)
+			ui.Error(err.Error())
+			return multistep.ActionHalt
+		} else if s.RootVolumeType != "" {
+			rootVolumeType = s.RootVolumeType
+		}
 		createVolume = &ec2.CreateVolumeInput{
 			AvailabilityZone: instance.Placement.AvailabilityZone,
 			Size:             aws.Int64(s.RootVolumeSize),
-			VolumeType:       aws.String(ec2.VolumeTypeGp2),
+			VolumeType:       aws.String(rootVolumeType),
 		}
+
 	} else {
 		// Determine the root device snapshot
 		image := state.Get("source_image").(*ec2.Image)
@@ -70,26 +82,13 @@ func (s *StepCreateVolume) Run(ctx context.Context, state multistep.StateBag) mu
 			}
 		}
 
-		if rootDevice == nil {
-			err := fmt.Errorf("Couldn't find root device!")
+		ui.Say("Creating the root volume...")
+		createVolume, err = s.buildCreateVolumeInput(*instance.Placement.AvailabilityZone, rootDevice)
+		if err != nil {
 			state.Put("error", err)
 			ui.Error(err.Error())
 			return multistep.ActionHalt
 		}
-
-		ui.Say("Creating the root volume...")
-		vs := *rootDevice.Ebs.VolumeSize
-		if s.RootVolumeSize > *rootDevice.Ebs.VolumeSize {
-			vs = s.RootVolumeSize
-		}
-
-		createVolume = &ec2.CreateVolumeInput{
-			AvailabilityZone: instance.Placement.AvailabilityZone,
-			Size:             aws.Int64(vs),
-			SnapshotId:       rootDevice.Ebs.SnapshotId,
-			VolumeType:       rootDevice.Ebs.VolumeType,
-			Iops:             rootDevice.Ebs.Iops,
-		}
 	}
 
 	if len(tagSpecs) > 0 {
@@ -137,3 +136,33 @@ func (s *StepCreateVolume) Cleanup(state multistep.StateBag) {
 		ui.Error(fmt.Sprintf("Error deleting EBS volume: %s", err))
 	}
 }
+
+func (s *StepCreateVolume) buildCreateVolumeInput(az string, rootDevice *ec2.BlockDeviceMapping) (*ec2.CreateVolumeInput, error) {
+	if rootDevice == nil {
+		return nil, fmt.Errorf("Couldn't find root device!")
+	}
+	createVolumeInput := &ec2.CreateVolumeInput{
+		AvailabilityZone: aws.String(az),
+		Size:             rootDevice.Ebs.VolumeSize,
+		SnapshotId:       rootDevice.Ebs.SnapshotId,
+		VolumeType:       rootDevice.Ebs.VolumeType,
+		Iops:             rootDevice.Ebs.Iops,
+	}
+	if s.RootVolumeSize > *rootDevice.Ebs.VolumeSize {
+		createVolumeInput.Size = aws.Int64(s.RootVolumeSize)
+	}
+
+	if s.RootVolumeType == "" || s.RootVolumeType == *rootDevice.Ebs.VolumeType {
+		return createVolumeInput, nil
+	}
+
+	if s.RootVolumeType == "io1" {
+		return nil, fmt.Errorf("Root volume type cannot be io1, because existing root volume type was %s", *rootDevice.Ebs.VolumeType)
+	}
+
+	createVolumeInput.VolumeType = aws.String(s.RootVolumeType)
+	// non io1 cannot set iops
+	createVolumeInput.Iops = nil
+
+	return createVolumeInput, nil
+}

builder/amazon/chroot/step_create_volume_test.go

@@ -0,0 +1,73 @@
+package chroot
+
+import (
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func buildTestRootDevice() *ec2.BlockDeviceMapping {
+	return &ec2.BlockDeviceMapping{
+		Ebs: &ec2.EbsBlockDevice{
+			VolumeSize: aws.Int64(10),
+			SnapshotId: aws.String("snap-1234"),
+			VolumeType: aws.String("gp2"),
+		},
+	}
+}
+
+func TestCreateVolume_Default(t *testing.T) {
+	stepCreateVolume := new(StepCreateVolume)
+	_, err := stepCreateVolume.buildCreateVolumeInput("test-az", buildTestRootDevice())
+	assert.NoError(t, err)
+}
+
+func TestCreateVolume_Shrink(t *testing.T) {
+	stepCreateVolume := StepCreateVolume{RootVolumeSize: 1}
+	testRootDevice := buildTestRootDevice()
+	ret, err := stepCreateVolume.buildCreateVolumeInput("test-az", testRootDevice)
+	assert.NoError(t, err)
+	// Ensure that the new value is equal to the size of the old root device
+	assert.Equal(t, *ret.Size, *testRootDevice.Ebs.VolumeSize)
+}
+
+func TestCreateVolume_Expand(t *testing.T) {
+	stepCreateVolume := StepCreateVolume{RootVolumeSize: 25}
+	testRootDevice := buildTestRootDevice()
+	ret, err := stepCreateVolume.buildCreateVolumeInput("test-az", testRootDevice)
+	assert.NoError(t, err)
+	// Ensure that the new value is equal to the size of the value passed in
+	assert.Equal(t, *ret.Size, stepCreateVolume.RootVolumeSize)
+}
+
+func TestCreateVolume_io1_to_io1(t *testing.T) {
+	stepCreateVolume := StepCreateVolume{RootVolumeType: "io1"}
+	testRootDevice := buildTestRootDevice()
+	testRootDevice.Ebs.VolumeType = aws.String("io1")
+	testRootDevice.Ebs.Iops = aws.Int64(1000)
+	ret, err := stepCreateVolume.buildCreateVolumeInput("test-az", testRootDevice)
+	assert.NoError(t, err)
+	assert.Equal(t, *ret.VolumeType, stepCreateVolume.RootVolumeType)
+	assert.Equal(t, *ret.Iops, *testRootDevice.Ebs.Iops)
+}
+
+func TestCreateVolume_io1_to_gp2(t *testing.T) {
+	stepCreateVolume := StepCreateVolume{RootVolumeType: "gp2"}
+	testRootDevice := buildTestRootDevice()
+	testRootDevice.Ebs.VolumeType = aws.String("io1")
+	testRootDevice.Ebs.Iops = aws.Int64(1000)
+
+	ret, err := stepCreateVolume.buildCreateVolumeInput("test-az", testRootDevice)
+	assert.NoError(t, err)
+	assert.Equal(t, *ret.VolumeType, stepCreateVolume.RootVolumeType)
+	assert.Nil(t, ret.Iops)
+}
+
+func TestCreateVolume_gp2_to_io1(t *testing.T) {
+	stepCreateVolume := StepCreateVolume{RootVolumeType: "io1"}
+	testRootDevice := buildTestRootDevice()
+
+	_, err := stepCreateVolume.buildCreateVolumeInput("test-az", testRootDevice)
+	assert.Error(t, err)
+}

builder/amazon/common/access_config.go

@@ -43,6 +43,9 @@ func (c *AccessConfig) Session() (*session.Session, error) {
 		config.WithCredentials(staticCreds)
 	}
 
+	// default is 3, and when it was causing failures for users being throttled
+	config = config.WithMaxRetries(20)
+
 	if c.RawRegion != "" {
 		config = config.WithRegion(c.RawRegion)
 	} else if region := c.metadataRegion(); region != "" {

builder/amazon/common/run_config.go

@@ -60,14 +60,15 @@ func (d *SecurityGroupFilterOptions) Empty() bool {
 type RunConfig struct {
 	AssociatePublicIpAddress          bool                       `mapstructure:"associate_public_ip_address"`
 	AvailabilityZone                  string                     `mapstructure:"availability_zone"`
+	BlockDurationMinutes              int64                      `mapstructure:"block_duration_minutes"`
 	DisableStopInstance               bool                       `mapstructure:"disable_stop_instance"`
 	EbsOptimized                      bool                       `mapstructure:"ebs_optimized"`
 	EnableT2Unlimited                 bool                       `mapstructure:"enable_t2_unlimited"`
 	IamInstanceProfile                string                     `mapstructure:"iam_instance_profile"`
 	InstanceInitiatedShutdownBehavior string                     `mapstructure:"shutdown_behavior"`
 	InstanceType                      string                     `mapstructure:"instance_type"`
-	RunTags                           map[string]string          `mapstructure:"run_tags"`
 	SecurityGroupFilter               SecurityGroupFilterOptions `mapstructure:"security_group_filter"`
+	RunTags                           map[string]string          `mapstructure:"run_tags"`
 	SecurityGroupId                   string                     `mapstructure:"security_group_id"`
 	SecurityGroupIds                  []string                   `mapstructure:"security_group_ids"`
 	SourceAmi                         string                     `mapstructure:"source_ami"`
@@ -86,9 +87,7 @@ type RunConfig struct {
 	WindowsPasswordTimeout            time.Duration              `mapstructure:"windows_password_timeout"`
 
 	// Communicator settings
-	Comm           communicator.Config `mapstructure:",squash"`
-	SSHKeyPairName string              `mapstructure:"ssh_keypair_name"`
-	SSHInterface   string              `mapstructure:"ssh_interface"`
+	Comm communicator.Config `mapstructure:",squash"`
 }
 
 func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
@@ -96,10 +95,10 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
 	// ssh_private_key_file, then create a temporary one, but only if the
 	// temporary_key_pair_name has not been provided and we are not using
 	// ssh_password.
-	if c.SSHKeyPairName == "" && c.TemporaryKeyPairName == "" &&
-		c.Comm.SSHPrivateKey == "" && c.Comm.SSHPassword == "" {
+	if c.Comm.SSHKeyPairName == "" && c.Comm.SSHTemporaryKeyPairName == "" &&
+		c.Comm.SSHPrivateKeyFile == "" && c.Comm.SSHPassword == "" {
 
-		c.TemporaryKeyPairName = fmt.Sprintf("packer_%s", uuid.TimeOrderedUUID())
+		c.Comm.SSHTemporaryKeyPairName = fmt.Sprintf("packer_%s", uuid.TimeOrderedUUID())
 	}
 
 	if c.WindowsPasswordTimeout == 0 {
@@ -114,18 +113,18 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
 	errs := c.Comm.Prepare(ctx)
 
 	// Validating ssh_interface
-	if c.SSHInterface != "public_ip" &&
-		c.SSHInterface != "private_ip" &&
-		c.SSHInterface != "public_dns" &&
-		c.SSHInterface != "private_dns" &&
-		c.SSHInterface != "" {
-		errs = append(errs, fmt.Errorf("Unknown interface type: %s", c.SSHInterface))
+	if c.Comm.SSHInterface != "public_ip" &&
+		c.Comm.SSHInterface != "private_ip" &&
+		c.Comm.SSHInterface != "public_dns" &&
+		c.Comm.SSHInterface != "private_dns" &&
+		c.Comm.SSHInterface != "" {
+		errs = append(errs, fmt.Errorf("Unknown interface type: %s", c.Comm.SSHInterface))
 	}
 
-	if c.SSHKeyPairName != "" {
-		if c.Comm.Type == "winrm" && c.Comm.WinRMPassword == "" && c.Comm.SSHPrivateKey == "" {
+	if c.Comm.SSHKeyPairName != "" {
+		if c.Comm.Type == "winrm" && c.Comm.WinRMPassword == "" && c.Comm.SSHPrivateKeyFile == "" {
 			errs = append(errs, fmt.Errorf("ssh_private_key_file must be provided to retrieve the winrm password when using ssh_keypair_name."))
-		} else if c.Comm.SSHPrivateKey == "" && !c.Comm.SSHAgentAuth {
+		} else if c.Comm.SSHPrivateKeyFile == "" && !c.Comm.SSHAgentAuth {
 			errs = append(errs, fmt.Errorf("ssh_private_key_file must be provided or ssh_agent_auth enabled when ssh_keypair_name is specified."))
 		}
 	}
@@ -142,6 +141,11 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
 		errs = append(errs, fmt.Errorf("An instance_type must be specified"))
 	}
 
+	if c.BlockDurationMinutes%60 != 0 {
+		errs = append(errs, fmt.Errorf(
+			"block_duration_minutes must be multiple of 60"))
+	}
+
 	if c.SpotPrice == "auto" {
 		if c.SpotPriceAutoProduct == "" {
 			errs = append(errs, fmt.Errorf(

builder/amazon/common/run_config_test.go

@@ -206,27 +206,27 @@ func TestRunConfigPrepare_UserDataFile(t *testing.T) {
 
 func TestRunConfigPrepare_TemporaryKeyPairName(t *testing.T) {
 	c := testConfig()
-	c.TemporaryKeyPairName = ""
+	c.Comm.SSHTemporaryKeyPairName = ""
 	if err := c.Prepare(nil); len(err) != 0 {
 		t.Fatalf("err: %s", err)
 	}
 
-	if c.TemporaryKeyPairName == "" {
+	if c.Comm.SSHTemporaryKeyPairName == "" {
 		t.Fatal("keypair name is empty")
 	}
 
 	// Match prefix and UUID, e.g. "packer_5790d491-a0b8-c84c-c9d2-2aea55086550".
 	r := regexp.MustCompile(`\Apacker_(?:(?i)[a-f\d]{8}(?:-[a-f\d]{4}){3}-[a-f\d]{12}?)\z`)
-	if !r.MatchString(c.TemporaryKeyPairName) {
+	if !r.MatchString(c.Comm.SSHTemporaryKeyPairName) {
 		t.Fatal("keypair name is not valid")
 	}
 
-	c.TemporaryKeyPairName = "ssh-key-123"
+	c.Comm.SSHTemporaryKeyPairName = "ssh-key-123"
 	if err := c.Prepare(nil); len(err) != 0 {
 		t.Fatalf("err: %s", err)
 	}
 
-	if c.TemporaryKeyPairName != "ssh-key-123" {
+	if c.Comm.SSHTemporaryKeyPairName != "ssh-key-123" {
 		t.Fatal("keypair name does not match")
 	}
 }

builder/amazon/common/ssh.go

@@ -3,15 +3,10 @@ package common
 import (
 	"errors"
 	"fmt"
-	"net"
-	"os"
 	"time"
 
 	"github.com/aws/aws-sdk-go/service/ec2"
-	packerssh "github.com/hashicorp/packer/communicator/ssh"
 	"github.com/hashicorp/packer/helper/multistep"
-	"golang.org/x/crypto/ssh"
-	"golang.org/x/crypto/ssh/agent"
 )
 
 type ec2Describer interface {
@@ -85,56 +80,3 @@ func SSHHost(e ec2Describer, sshInterface string) func(multistep.StateBag) (stri
 		return "", errors.New("couldn't determine address for instance")
 	}
 }
-
-// SSHConfig returns a function that can be used for the SSH communicator
-// config for connecting to the instance created over SSH using the private key
-// or password.
-func SSHConfig(useAgent bool, username, password string) func(multistep.StateBag) (*ssh.ClientConfig, error) {
-	return func(state multistep.StateBag) (*ssh.ClientConfig, error) {
-		if useAgent {
-			authSock := os.Getenv("SSH_AUTH_SOCK")
-			if authSock == "" {
-				return nil, fmt.Errorf("SSH_AUTH_SOCK is not set")
-			}
-
-			sshAgent, err := net.Dial("unix", authSock)
-			if err != nil {
-				return nil, fmt.Errorf("Cannot connect to SSH Agent socket %q: %s", authSock, err)
-			}
-
-			return &ssh.ClientConfig{
-				User: username,
-				Auth: []ssh.AuthMethod{
-					ssh.PublicKeysCallback(agent.NewClient(sshAgent).Signers),
-				},
-				HostKeyCallback: ssh.InsecureIgnoreHostKey(),
-			}, nil
-		}
-
-		privateKey, hasKey := state.GetOk("privateKey")
-		if hasKey {
-
-			signer, err := ssh.ParsePrivateKey([]byte(privateKey.(string)))
-			if err != nil {
-				return nil, fmt.Errorf("Error setting up SSH config: %s", err)
-			}
-			return &ssh.ClientConfig{
-				User: username,
-				Auth: []ssh.AuthMethod{
-					ssh.PublicKeys(signer),
-				},
-				HostKeyCallback: ssh.InsecureIgnoreHostKey(),
-			}, nil
-
-		} else {
-			return &ssh.ClientConfig{
-				User:            username,
-				HostKeyCallback: ssh.InsecureIgnoreHostKey(),
-				Auth: []ssh.AuthMethod{
-					ssh.Password(password),
-					ssh.KeyboardInteractive(
-						packerssh.PasswordKeyboardInteractive(password)),
-				}}, nil
-		}
-	}
-}

builder/amazon/common/state.go

@@ -40,10 +40,16 @@ func WaitUntilAMIAvailable(ctx aws.Context, conn *ec2.EC2, imageId string) error
 		ImageIds: []*string{&imageId},
 	}
 
+	waitOpts := getWaiterOptions()
+	if len(waitOpts) == 0 {
+		// Bump this default to 30 minutes because the aws default
+		// of ten minutes doesn't work for some of our long-running copies.
+		waitOpts = append(waitOpts, request.WithWaiterMaxAttempts(120))
+	}
 	err := conn.WaitUntilImageAvailableWithContext(
 		ctx,
 		&imageInput,
-		getWaiterOptions()...)
+		waitOpts...)
 	return err
 }
 
@@ -163,6 +169,8 @@ func WaitForVolumeToBeAttached(c *ec2.EC2, ctx aws.Context, input *ec2.DescribeV
 			return req, nil
 		},
 	}
+	w.ApplyOptions(opts...)
+
 	return w.WaitWithContext(ctx)
 }
 
@@ -192,6 +200,8 @@ func WaitForVolumeToBeDetached(c *ec2.EC2, ctx aws.Context, input *ec2.DescribeV
 			return req, nil
 		},
 	}
+	w.ApplyOptions(opts...)
+
 	return w.WaitWithContext(ctx)
 }
 
@@ -221,6 +231,8 @@ func WaitForImageToBeImported(c *ec2.EC2, ctx aws.Context, input *ec2.DescribeIm
 			return req, nil
 		},
 	}
+	w.ApplyOptions(opts...)
+
 	return w.WaitWithContext(ctx)
 }
 
@@ -275,11 +287,11 @@ func getOverride(varInfo envInfo) envInfo {
 	return varInfo
 }
 func getEnvOverrides() overridableWaitVars {
-	// Load env vars from environment, and use them to override defaults
+	// Load env vars from environment.
 	envValues := overridableWaitVars{
-		envInfo{"AWS_POLL_DELAY_SECONDS", 2, false},
+		envInfo{"AWS_POLL_DELAY_SECONDS", 0, false},
 		envInfo{"AWS_MAX_ATTEMPTS", 0, false},
-		envInfo{"AWS_TIMEOUT_SECONDS", 300, false},
+		envInfo{"AWS_TIMEOUT_SECONDS", 0, false},
 	}
 
 	envValues.awsMaxAttempts = getOverride(envValues.awsMaxAttempts)

builder/amazon/common/step_get_password.go

@@ -12,7 +12,6 @@ import (
 	"time"
 
 	"github.com/aws/aws-sdk-go/service/ec2"
-	commonhelper "github.com/hashicorp/packer/helper/common"
 	"github.com/hashicorp/packer/helper/communicator"
 	"github.com/hashicorp/packer/helper/multistep"
 	"github.com/hashicorp/packer/packer"
@@ -95,19 +94,19 @@ WaitLoop:
 			"Password (since debug is enabled): %s", s.Comm.WinRMPassword))
 	}
 	// store so that we can access this later during provisioning
-	commonhelper.SetSharedState("winrm_password", s.Comm.WinRMPassword, s.BuildName)
+	state.Put("winrm_password", s.Comm.WinRMPassword)
+	packer.LogSecretFilter.Set(s.Comm.WinRMPassword)
 
 	return multistep.ActionContinue
 }
 
 func (s *StepGetPassword) Cleanup(multistep.StateBag) {
-	commonhelper.RemoveSharedStateFile("winrm_password", s.BuildName)
 }
 
 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)
+	privateKey := s.Comm.SSHPrivateKey
 
 	for {
 		select {

builder/amazon/common/step_key_pair.go

@@ -8,17 +8,15 @@ import (
 	"runtime"
 
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/hashicorp/packer/helper/communicator"
 	"github.com/hashicorp/packer/helper/multistep"
 	"github.com/hashicorp/packer/packer"
 )
 
 type StepKeyPair struct {
-	Debug                bool
-	SSHAgentAuth         bool
-	DebugKeyPath         string
-	TemporaryKeyPairName string
-	KeyPairName          string
-	PrivateKeyFile       string
+	Debug        bool
+	Comm         *communicator.Config
+	DebugKeyPath string
 
 	doCleanup bool
 }
@@ -26,43 +24,41 @@ type StepKeyPair struct {
 func (s *StepKeyPair) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
 	ui := state.Get("ui").(packer.Ui)
 
-	if s.PrivateKeyFile != "" {
+	if s.Comm.SSHPrivateKeyFile != "" {
 		ui.Say("Using existing SSH private key")
-		privateKeyBytes, err := ioutil.ReadFile(s.PrivateKeyFile)
+		privateKeyBytes, err := ioutil.ReadFile(s.Comm.SSHPrivateKeyFile)
 		if err != nil {
 			state.Put("error", fmt.Errorf(
 				"Error loading configured private key file: %s", err))
 			return multistep.ActionHalt
 		}
 
-		state.Put("keyPair", s.KeyPairName)
-		state.Put("privateKey", string(privateKeyBytes))
+		s.Comm.SSHPrivateKey = privateKeyBytes
 
 		return multistep.ActionContinue
 	}
 
-	if s.SSHAgentAuth && s.KeyPairName == "" {
+	if s.Comm.SSHAgentAuth && s.Comm.SSHKeyPairName == "" {
 		ui.Say("Using SSH Agent with key pair in Source AMI")
 		return multistep.ActionContinue
 	}
 
-	if s.SSHAgentAuth && s.KeyPairName != "" {
-		ui.Say(fmt.Sprintf("Using SSH Agent for existing key pair %s", s.KeyPairName))
-		state.Put("keyPair", s.KeyPairName)
+	if s.Comm.SSHAgentAuth && s.Comm.SSHKeyPairName != "" {
+		ui.Say(fmt.Sprintf("Using SSH Agent for existing key pair %s", s.Comm.SSHKeyPairName))
 		return multistep.ActionContinue
 	}
 
-	if s.TemporaryKeyPairName == "" {
+	if s.Comm.SSHTemporaryKeyPairName == "" {
 		ui.Say("Not using temporary keypair")
-		state.Put("keyPair", "")
+		s.Comm.SSHKeyPairName = ""
 		return multistep.ActionContinue
 	}
 
 	ec2conn := state.Get("ec2").(*ec2.EC2)
 
-	ui.Say(fmt.Sprintf("Creating temporary keypair: %s", s.TemporaryKeyPairName))
+	ui.Say(fmt.Sprintf("Creating temporary keypair: %s", s.Comm.SSHTemporaryKeyPairName))
 	keyResp, err := ec2conn.CreateKeyPair(&ec2.CreateKeyPairInput{
-		KeyName: &s.TemporaryKeyPairName})
+		KeyName: &s.Comm.SSHTemporaryKeyPairName})
 	if err != nil {
 		state.Put("error", fmt.Errorf("Error creating temporary keypair: %s", err))
 		return multistep.ActionHalt
@@ -70,9 +66,9 @@ func (s *StepKeyPair) Run(_ context.Context, state multistep.StateBag) multistep
 
 	s.doCleanup = true
 
-	// Set some state data for use in future steps
-	state.Put("keyPair", s.TemporaryKeyPairName)
-	state.Put("privateKey", *keyResp.KeyMaterial)
+	// Set some data for use in future steps
+	s.Comm.SSHKeyPairName = s.Comm.SSHTemporaryKeyPairName
+	s.Comm.SSHPrivateKey = []byte(*keyResp.KeyMaterial)
 
 	// If we're in debug mode, output the private key to the working
 	// directory.
@@ -113,10 +109,10 @@ func (s *StepKeyPair) Cleanup(state multistep.StateBag) {
 
 	// Remove the keypair
 	ui.Say("Deleting temporary keypair...")
-	_, err := ec2conn.DeleteKeyPair(&ec2.DeleteKeyPairInput{KeyName: &s.TemporaryKeyPairName})
+	_, err := ec2conn.DeleteKeyPair(&ec2.DeleteKeyPairInput{KeyName: &s.Comm.SSHTemporaryKeyPairName})
 	if err != nil {
 		ui.Error(fmt.Sprintf(
-			"Error cleaning up keypair. Please delete the key manually: %s", s.TemporaryKeyPairName))
+			"Error cleaning up keypair. Please delete the key manually: %s", s.Comm.SSHTemporaryKeyPairName))
 	}
 
 	// Also remove the physical key if we're debugging.

builder/amazon/common/step_run_source_instance.go

@@ -12,6 +12,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/ec2"
 
 	retry "github.com/hashicorp/packer/common"
+	"github.com/hashicorp/packer/helper/communicator"
 	"github.com/hashicorp/packer/helper/multistep"
 	"github.com/hashicorp/packer/packer"
 	"github.com/hashicorp/packer/template/interpolate"
@@ -20,6 +21,7 @@ import (
 type StepRunSourceInstance struct {
 	AssociatePublicIpAddress          bool
 	BlockDevices                      BlockDevices
+	Comm                              *communicator.Config
 	Ctx                               interpolate.Context
 	Debug                             bool
 	EbsOptimized                      bool
@@ -40,10 +42,7 @@ type StepRunSourceInstance struct {
 
 func (s *StepRunSourceInstance) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
 	ec2conn := state.Get("ec2").(*ec2.EC2)
-	var keyName string
-	if name, ok := state.GetOk("keyPair"); ok {
-		keyName = name.(string)
-	}
+
 	securityGroupIds := aws.StringSlice(state.Get("securityGroupIds").([]string))
 	ui := state.Get("ui").(packer.Ui)
 
@@ -149,8 +148,8 @@ func (s *StepRunSourceInstance) Run(ctx context.Context, state multistep.StateBa
 		volTags.Report(ui)
 	}
 
-	if keyName != "" {
-		runOpts.KeyName = &keyName
+	if s.Comm.SSHKeyPairName != "" {
+		runOpts.KeyName = &s.Comm.SSHKeyPairName
 	}
 
 	subnetId := state.Get("subnet_id").(string)

builder/amazon/common/step_run_spot_instance.go

@@ -14,6 +14,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/ec2"
 
 	retry "github.com/hashicorp/packer/common"
+	"github.com/hashicorp/packer/helper/communicator"
 	"github.com/hashicorp/packer/helper/multistep"
 	"github.com/hashicorp/packer/packer"
 	"github.com/hashicorp/packer/template/interpolate"
@@ -22,7 +23,9 @@ import (
 type StepRunSpotInstance struct {
 	AssociatePublicIpAddress          bool
 	BlockDevices                      BlockDevices
+	BlockDurationMinutes              int64
 	Debug                             bool
+	Comm                              *communicator.Config
 	EbsOptimized                      bool
 	ExpectedRootDevice                string
 	IamInstanceProfile                string
@@ -44,10 +47,6 @@ type StepRunSpotInstance struct {
 
 func (s *StepRunSpotInstance) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
 	ec2conn := state.Get("ec2").(*ec2.EC2)
-	var keyName string
-	if name, ok := state.GetOk("keyPair"); ok {
-		keyName = name.(string)
-	}
 	securityGroupIds := aws.StringSlice(state.Get("securityGroupIds").([]string))
 	ui := state.Get("ui").(packer.Ui)
 
@@ -189,13 +188,14 @@ func (s *StepRunSpotInstance) Run(ctx context.Context, state multistep.StateBag)
 		runOpts.SecurityGroupIds = securityGroupIds
 	}
 
-	if keyName != "" {
-		runOpts.KeyName = &keyName
+	if s.Comm.SSHKeyPairName != "" {
+		runOpts.KeyName = &s.Comm.SSHKeyPairName
 	}
 
 	runSpotResp, err := ec2conn.RequestSpotInstances(&ec2.RequestSpotInstancesInput{
-		SpotPrice:           &spotPrice,
-		LaunchSpecification: runOpts,
+		BlockDurationMinutes: &s.BlockDurationMinutes,
+		LaunchSpecification:  runOpts,
+		SpotPrice:            &spotPrice,
 	})
 	if err != nil {
 		err := fmt.Errorf("Error launching source spot instance: %s", err)

builder/amazon/common/step_source_ami_info.go

@@ -21,6 +21,7 @@ type StepSourceAMIInfo struct {
 	SourceAmi                string
 	EnableAMISriovNetSupport bool
 	EnableAMIENASupport      bool
+	AMIVirtType              string
 	AmiFilters               AmiFilterOptions
 }
 
@@ -93,11 +94,13 @@ func (s *StepSourceAMIInfo) Run(_ context.Context, state multistep.StateBag) mul
 
 	// Enhanced Networking can only be enabled on HVM AMIs.
 	// See http://goo.gl/icuXh5
-	if (s.EnableAMIENASupport || s.EnableAMISriovNetSupport) && *image.VirtualizationType != "hvm" {
-		err := fmt.Errorf("Cannot enable enhanced networking, source AMI '%s' is not HVM", s.SourceAmi)
-		state.Put("error", err)
-		ui.Error(err.Error())
-		return multistep.ActionHalt
+	if s.EnableAMIENASupport || s.EnableAMISriovNetSupport {
+		err = s.canEnableEnhancedNetworking(image)
+		if err != nil {
+			state.Put("error", err)
+			ui.Error(err.Error())
+			return multistep.ActionHalt
+		}
 	}
 
 	state.Put("source_image", image)
@@ -105,3 +108,16 @@ func (s *StepSourceAMIInfo) Run(_ context.Context, state multistep.StateBag) mul
 }
 
 func (s *StepSourceAMIInfo) Cleanup(multistep.StateBag) {}
+
+func (s *StepSourceAMIInfo) canEnableEnhancedNetworking(image *ec2.Image) error {
+	if s.AMIVirtType == "hvm" {
+		return nil
+	}
+	if s.AMIVirtType != "" {
+		return fmt.Errorf("Cannot enable enhanced networking, AMIVirtType '%s' is not HVM", s.AMIVirtType)
+	}
+	if *image.VirtualizationType != "hvm" {
+		return fmt.Errorf("Cannot enable enhanced networking, source AMI '%s' is not HVM", s.SourceAmi)
+	}
+	return nil
+}

builder/amazon/common/step_source_ami_info_test.go

@@ -0,0 +1,42 @@
+package common
+
+import (
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestStepSourceAmiInfo_PVImage(t *testing.T) {
+	err := new(StepSourceAMIInfo).canEnableEnhancedNetworking(&ec2.Image{
+		VirtualizationType: aws.String("paravirtual"),
+	})
+	assert.Error(t, err)
+}
+
+func TestStepSourceAmiInfo_HVMImage(t *testing.T) {
+	err := new(StepSourceAMIInfo).canEnableEnhancedNetworking(&ec2.Image{
+		VirtualizationType: aws.String("hvm"),
+	})
+	assert.NoError(t, err)
+}
+
+func TestStepSourceAmiInfo_PVImageWithAMIVirtPV(t *testing.T) {
+	stepSourceAMIInfo := StepSourceAMIInfo{
+		AMIVirtType: "paravirtual",
+	}
+	err := stepSourceAMIInfo.canEnableEnhancedNetworking(&ec2.Image{
+		VirtualizationType: aws.String("paravirtual"),
+	})
+	assert.Error(t, err)
+}
+
+func TestStepSourceAmiInfo_PVImageWithAMIVirtHVM(t *testing.T) {
+	stepSourceAMIInfo := StepSourceAMIInfo{
+		AMIVirtType: "hvm",
+	}
+	err := stepSourceAMIInfo.canEnableEnhancedNetworking(&ec2.Image{
+		VirtualizationType: aws.String("paravirtual"),
+	})
+	assert.NoError(t, err)
+}

builder/amazon/ebs/builder.go

@@ -81,7 +81,8 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		return nil, errs
 	}
 
-	log.Println(common.ScrubConfig(b.config, b.config.AccessKey, b.config.SecretKey, b.config.Token))
+	packer.LogSecretFilter.Set(b.config.AccessKey, b.config.SecretKey, b.config.Token)
+	log.Println(b.config)
 	return nil, nil
 }
 
@@ -107,6 +108,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		instanceStep = &awscommon.StepRunSpotInstance{
 			AssociatePublicIpAddress:          b.config.AssociatePublicIpAddress,
 			BlockDevices:                      b.config.BlockDevices,
+			BlockDurationMinutes:              b.config.BlockDurationMinutes,
 			Ctx:                               b.config.ctx,
 			Debug:                             b.config.PackerDebug,
 			EbsOptimized:                      b.config.EbsOptimized,
@@ -127,6 +129,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		instanceStep = &awscommon.StepRunSourceInstance{
 			AssociatePublicIpAddress:          b.config.AssociatePublicIpAddress,
 			BlockDevices:                      b.config.BlockDevices,
+			Comm:                              &b.config.RunConfig.Comm,
 			Ctx:                               b.config.ctx,
 			Debug:                             b.config.PackerDebug,
 			EbsOptimized:                      b.config.EbsOptimized,
@@ -155,6 +158,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			EnableAMISriovNetSupport: b.config.AMISriovNetSupport,
 			EnableAMIENASupport:      b.config.AMIENASupport,
 			AmiFilters:               b.config.SourceAmiFilter,
+			AMIVirtType:              b.config.AMIVirtType,
 		},
 		&awscommon.StepNetworkInfo{
 			VpcId:               b.config.VpcId,
@@ -166,12 +170,9 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			AvailabilityZone:    b.config.AvailabilityZone,
 		},
 		&awscommon.StepKeyPair{
-			Debug:                b.config.PackerDebug,
-			SSHAgentAuth:         b.config.Comm.SSHAgentAuth,
-			DebugKeyPath:         fmt.Sprintf("ec2_%s.pem", b.config.PackerBuildName),
-			KeyPairName:          b.config.SSHKeyPairName,
-			TemporaryKeyPairName: b.config.TemporaryKeyPairName,
-			PrivateKeyFile:       b.config.RunConfig.Comm.SSHPrivateKey,
+			Debug:        b.config.PackerDebug,
+			Comm:         &b.config.RunConfig.Comm,
+			DebugKeyPath: fmt.Sprintf("ec2_%s.pem", b.config.PackerBuildName),
 		},
 		&awscommon.StepSecurityGroup{
 			SecurityGroupFilter:   b.config.SecurityGroupFilter,
@@ -193,11 +194,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			Config: &b.config.RunConfig.Comm,
 			Host: awscommon.SSHHost(
 				ec2conn,
-				b.config.SSHInterface),
-			SSHConfig: awscommon.SSHConfig(
-				b.config.RunConfig.Comm.SSHAgentAuth,
-				b.config.RunConfig.Comm.SSHUsername,
-				b.config.RunConfig.Comm.SSHPassword),
+				b.config.Comm.SSHInterface),
+			SSHConfig: b.config.RunConfig.Comm.SSHConfigFunc(),
 		},
 		&common.StepProvision{},
 		&awscommon.StepStopEBSBackedInstance{
@@ -248,7 +246,6 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 	// Run!
 	b.runner = common.NewRunner(steps, b.config.PackerConfig, ui)
 	b.runner.Run(state)
-
 	// If there was an error, return that
 	if rawErr, ok := state.GetOk("error"); ok {
 		return nil, rawErr.(error)

builder/amazon/ebs/step_create_ami.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/aws/aws-sdk-go/service/ec2"
 	awscommon "github.com/hashicorp/packer/builder/amazon/common"
+	"github.com/hashicorp/packer/common/random"
 	"github.com/hashicorp/packer/helper/multistep"
 	"github.com/hashicorp/packer/packer"
 )
@@ -22,10 +23,17 @@ func (s *stepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi
 	ui := state.Get("ui").(packer.Ui)
 
 	// Create the image
-	ui.Say(fmt.Sprintf("Creating the AMI: %s", config.AMIName))
+	amiName := config.AMIName
+	if config.AMIEncryptBootVolume {
+		// to avoid having a temporary unencrypted
+		// image named config.AMIName
+		amiName = random.AlphaNum(7)
+	}
+
+	ui.Say(fmt.Sprintf("Creating unencrypted AMI %s from instance %s", amiName, *instance.InstanceId))
 	createOpts := &ec2.CreateImageInput{
 		InstanceId:          instance.InstanceId,
-		Name:                &config.AMIName,
+		Name:                &amiName,
 		BlockDeviceMappings: config.BlockDevices.BuildAMIDevices(),
 	}
 

builder/amazon/ebssurrogate/builder.go

@@ -96,7 +96,8 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		return nil, errs
 	}
 
-	log.Println(common.ScrubConfig(b.config, b.config.AccessKey, b.config.SecretKey, b.config.Token))
+	packer.LogSecretFilter.Set(b.config.AccessKey, b.config.SecretKey, b.config.Token)
+	log.Println(b.config)
 	return nil, nil
 }
 
@@ -121,6 +122,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		instanceStep = &awscommon.StepRunSpotInstance{
 			AssociatePublicIpAddress:          b.config.AssociatePublicIpAddress,
 			BlockDevices:                      b.config.BlockDevices,
+			BlockDurationMinutes:              b.config.BlockDurationMinutes,
 			Ctx:                               b.config.ctx,
 			Debug:                             b.config.PackerDebug,
 			EbsOptimized:                      b.config.EbsOptimized,
@@ -141,6 +143,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		instanceStep = &awscommon.StepRunSourceInstance{
 			AssociatePublicIpAddress:          b.config.AssociatePublicIpAddress,
 			BlockDevices:                      b.config.BlockDevices,
+			Comm:                              &b.config.RunConfig.Comm,
 			Ctx:                               b.config.ctx,
 			Debug:                             b.config.PackerDebug,
 			EbsOptimized:                      b.config.EbsOptimized,
@@ -172,6 +175,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			EnableAMISriovNetSupport: b.config.AMISriovNetSupport,
 			EnableAMIENASupport:      b.config.AMIENASupport,
 			AmiFilters:               b.config.SourceAmiFilter,
+			AMIVirtType:              b.config.AMIVirtType,
 		},
 		&awscommon.StepNetworkInfo{
 			VpcId:               b.config.VpcId,
@@ -183,12 +187,9 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			AvailabilityZone:    b.config.AvailabilityZone,
 		},
 		&awscommon.StepKeyPair{
-			Debug:                b.config.PackerDebug,
-			SSHAgentAuth:         b.config.Comm.SSHAgentAuth,
-			DebugKeyPath:         fmt.Sprintf("ec2_%s.pem", b.config.PackerBuildName),
-			KeyPairName:          b.config.SSHKeyPairName,
-			TemporaryKeyPairName: b.config.TemporaryKeyPairName,
-			PrivateKeyFile:       b.config.RunConfig.Comm.SSHPrivateKey,
+			Debug:        b.config.PackerDebug,
+			Comm:         &b.config.RunConfig.Comm,
+			DebugKeyPath: fmt.Sprintf("ec2_%s.pem", b.config.PackerBuildName),
 		},
 		&awscommon.StepSecurityGroup{
 			SecurityGroupFilter:   b.config.SecurityGroupFilter,
@@ -210,11 +211,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			Config: &b.config.RunConfig.Comm,
 			Host: awscommon.SSHHost(
 				ec2conn,
-				b.config.SSHInterface),
-			SSHConfig: awscommon.SSHConfig(
-				b.config.RunConfig.Comm.SSHAgentAuth,
-				b.config.RunConfig.Comm.SSHUsername,
-				b.config.RunConfig.Comm.SSHPassword),
+				b.config.Comm.SSHInterface),
+			SSHConfig: b.config.RunConfig.Comm.SSHConfigFunc(),
 		},
 		&common.StepProvision{},
 		&awscommon.StepStopEBSBackedInstance{

builder/amazon/ebsvolume/builder.go

@@ -81,7 +81,8 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		return nil, errs
 	}
 
-	log.Println(common.ScrubConfig(b.config, b.config.AccessKey, b.config.SecretKey, b.config.Token))
+	packer.LogSecretFilter.Set(b.config.AccessKey, b.config.SecretKey, b.config.Token)
+	log.Println(b.config)
 	return nil, nil
 }
 
@@ -105,6 +106,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		instanceStep = &awscommon.StepRunSpotInstance{
 			AssociatePublicIpAddress:          b.config.AssociatePublicIpAddress,
 			BlockDevices:                      b.config.launchBlockDevices,
+			BlockDurationMinutes:              b.config.BlockDurationMinutes,
 			Ctx:                               b.config.ctx,
 			Debug:                             b.config.PackerDebug,
 			EbsOptimized:                      b.config.EbsOptimized,
@@ -124,6 +126,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		instanceStep = &awscommon.StepRunSourceInstance{
 			AssociatePublicIpAddress:          b.config.AssociatePublicIpAddress,
 			BlockDevices:                      b.config.launchBlockDevices,
+			Comm:                              &b.config.RunConfig.Comm,
 			Ctx:                               b.config.ctx,
 			Debug:                             b.config.PackerDebug,
 			EbsOptimized:                      b.config.EbsOptimized,
@@ -158,12 +161,9 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			AvailabilityZone:    b.config.AvailabilityZone,
 		},
 		&awscommon.StepKeyPair{
-			Debug:                b.config.PackerDebug,
-			SSHAgentAuth:         b.config.Comm.SSHAgentAuth,
-			DebugKeyPath:         fmt.Sprintf("ec2_%s.pem", b.config.PackerBuildName),
-			KeyPairName:          b.config.SSHKeyPairName,
-			TemporaryKeyPairName: b.config.TemporaryKeyPairName,
-			PrivateKeyFile:       b.config.RunConfig.Comm.SSHPrivateKey,
+			Debug:        b.config.PackerDebug,
+			Comm:         &b.config.RunConfig.Comm,
+			DebugKeyPath: fmt.Sprintf("ec2_%s.pem", b.config.PackerBuildName),
 		},
 		&awscommon.StepSecurityGroup{
 			SecurityGroupFilter:   b.config.SecurityGroupFilter,
@@ -186,11 +186,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			Config: &b.config.RunConfig.Comm,
 			Host: awscommon.SSHHost(
 				ec2conn,
-				b.config.SSHInterface),
-			SSHConfig: awscommon.SSHConfig(
-				b.config.RunConfig.Comm.SSHAgentAuth,
-				b.config.RunConfig.Comm.SSHUsername,
-				b.config.RunConfig.Comm.SSHPassword),
+				b.config.Comm.SSHInterface),
+			SSHConfig: b.config.RunConfig.Comm.SSHConfigFunc(),
 		},
 		&common.StepProvision{},
 		&awscommon.StepStopEBSBackedInstance{

builder/amazon/instance/builder.go

@@ -166,8 +166,8 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 	if errs != nil && len(errs.Errors) > 0 {
 		return nil, errs
 	}
-
-	log.Println(common.ScrubConfig(b.config, b.config.AccessKey, b.config.SecretKey, b.config.Token))
+	packer.LogSecretFilter.Set(b.config.AccessKey, b.config.SecretKey, b.config.Token)
+	log.Println(b.config)
 	return nil, nil
 }
 
@@ -192,6 +192,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		instanceStep = &awscommon.StepRunSpotInstance{
 			AssociatePublicIpAddress: b.config.AssociatePublicIpAddress,
 			BlockDevices:             b.config.BlockDevices,
+			BlockDurationMinutes:     b.config.BlockDurationMinutes,
 			Ctx:                      b.config.ctx,
 			Debug:                    b.config.PackerDebug,
 			EbsOptimized:             b.config.EbsOptimized,
@@ -209,6 +210,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		instanceStep = &awscommon.StepRunSourceInstance{
 			AssociatePublicIpAddress: b.config.AssociatePublicIpAddress,
 			BlockDevices:             b.config.BlockDevices,
+			Comm:                     &b.config.RunConfig.Comm,
 			Ctx:                      b.config.ctx,
 			Debug:                    b.config.PackerDebug,
 			EbsOptimized:             b.config.EbsOptimized,
@@ -234,6 +236,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			EnableAMISriovNetSupport: b.config.AMISriovNetSupport,
 			EnableAMIENASupport:      b.config.AMIENASupport,
 			AmiFilters:               b.config.SourceAmiFilter,
+			AMIVirtType:              b.config.AMIVirtType,
 		},
 		&awscommon.StepNetworkInfo{
 			VpcId:               b.config.VpcId,
@@ -245,12 +248,9 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			AvailabilityZone:    b.config.AvailabilityZone,
 		},
 		&awscommon.StepKeyPair{
-			Debug:                b.config.PackerDebug,
-			SSHAgentAuth:         b.config.Comm.SSHAgentAuth,
-			DebugKeyPath:         fmt.Sprintf("ec2_%s.pem", b.config.PackerBuildName),
-			KeyPairName:          b.config.SSHKeyPairName,
-			PrivateKeyFile:       b.config.RunConfig.Comm.SSHPrivateKey,
-			TemporaryKeyPairName: b.config.TemporaryKeyPairName,
+			Debug:        b.config.PackerDebug,
+			Comm:         &b.config.RunConfig.Comm,
+			DebugKeyPath: fmt.Sprintf("ec2_%s.pem", b.config.PackerBuildName),
 		},
 		&awscommon.StepSecurityGroup{
 			CommConfig:            &b.config.RunConfig.Comm,
@@ -269,11 +269,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			Config: &b.config.RunConfig.Comm,
 			Host: awscommon.SSHHost(
 				ec2conn,
-				b.config.SSHInterface),
-			SSHConfig: awscommon.SSHConfig(
-				b.config.RunConfig.Comm.SSHAgentAuth,
-				b.config.RunConfig.Comm.SSHUsername,
-				b.config.RunConfig.Comm.SSHPassword),
+				b.config.Comm.SSHInterface),
+			SSHConfig: b.config.RunConfig.Comm.SSHConfigFunc(),
 		},
 		&common.StepProvision{},
 		&StepUploadX509Cert{},

builder/azure/arm/builder.go

@@ -174,7 +174,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			&communicator.StepConnectSSH{
 				Config:    &b.config.Comm,
 				Host:      lin.SSHHost,
-				SSHConfig: lin.SSHConfig(b.config.UserName),
+				SSHConfig: b.config.Comm.SSHConfigFunc(),
 			},
 			&packerCommon.StepProvision{},
 			NewStepGetOSDisk(azureClient, ui),
@@ -229,7 +229,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		ui.Message(fmt.Sprintf("temp admin user: '%s'", b.config.UserName))
 		ui.Message(fmt.Sprintf("temp admin password: '%s'", b.config.Password))
 
-		if b.config.sshPrivateKey != "" {
+		if len(b.config.Comm.SSHPrivateKey) != 0 {
 			debugKeyPath := fmt.Sprintf("%s-%s.pem", b.config.PackerBuildName, b.config.tmpComputeName)
 			ui.Message(fmt.Sprintf("temp ssh key: %s", debugKeyPath))
 
@@ -282,7 +282,7 @@ func (b *Builder) writeSSHPrivateKey(ui packer.Ui, debugKeyPath string) {
 	defer f.Close()
 
 	// Write the key out
-	if _, err := f.Write([]byte(b.config.sshPrivateKey)); err != nil {
+	if _, err := f.Write(b.config.Comm.SSHPrivateKey); err != nil {
 		ui.Say(fmt.Sprintf("Error saving debug key: %s", err))
 		return
 	}
@@ -333,7 +333,6 @@ func (b *Builder) getBlobAccount(ctx context.Context, client *AzureClient, resou
 
 func (b *Builder) configureStateBag(stateBag multistep.StateBag) {
 	stateBag.Put(constants.AuthorizedKey, b.config.sshAuthorizedKey)
-	stateBag.Put(constants.PrivateKey, b.config.sshPrivateKey)
 
 	stateBag.Put(constants.ArmTags, b.config.AzureTags)
 	stateBag.Put(constants.ArmComputeName, b.config.tmpComputeName)

builder/azure/arm/builder_test.go

@@ -15,7 +15,6 @@ func TestStateBagShouldBePopulatedExpectedValues(t *testing.T) {
 
 	var expectedStateBagKeys = []string{
 		constants.AuthorizedKey,
-		constants.PrivateKey,
 
 		constants.ArmTags,
 		constants.ArmComputeName,

builder/azure/arm/config.go

@@ -22,7 +22,6 @@ import (
 	"github.com/hashicorp/packer/builder/azure/common/constants"
 	"github.com/hashicorp/packer/builder/azure/pkcs12"
 	"github.com/hashicorp/packer/common"
-	commonhelper "github.com/hashicorp/packer/helper/common"
 	"github.com/hashicorp/packer/helper/communicator"
 	"github.com/hashicorp/packer/helper/config"
 	"github.com/hashicorp/packer/packer"
@@ -144,7 +143,6 @@ type Config struct {
 
 	// Authentication with the VM via SSH
 	sshAuthorizedKey string
-	sshPrivateKey    string
 
 	// Authentication with the VM via WinRM
 	winrmCertificate string
@@ -315,8 +313,8 @@ func setSshValues(c *Config) error {
 		c.Comm.SSHTimeout = 20 * time.Minute
 	}
 
-	if c.Comm.SSHPrivateKey != "" {
-		privateKeyBytes, err := ioutil.ReadFile(c.Comm.SSHPrivateKey)
+	if c.Comm.SSHPrivateKeyFile != "" {
+		privateKeyBytes, err := ioutil.ReadFile(c.Comm.SSHPrivateKeyFile)
 		if err != nil {
 			return err
 		}
@@ -330,7 +328,7 @@ func setSshValues(c *Config) error {
 			publicKey.Type(),
 			base64.StdEncoding.EncodeToString(publicKey.Marshal()),
 			time.Now().Format(time.RFC3339))
-		c.sshPrivateKey = string(privateKeyBytes)
+		c.Comm.SSHPrivateKey = privateKeyBytes
 
 	} else {
 		sshKeyPair, err := NewOpenSshKeyPair()
@@ -339,7 +337,7 @@ func setSshValues(c *Config) error {
 		}
 
 		c.sshAuthorizedKey = sshKeyPair.AuthorizedKey()
-		c.sshPrivateKey = sshKeyPair.PrivateKey()
+		c.Comm.SSHPrivateKey = sshKeyPair.PrivateKey()
 	}
 
 	return nil
@@ -361,9 +359,7 @@ func setRuntimeValues(c *Config) {
 	var tempName = NewTempName()
 
 	c.tmpAdminPassword = tempName.AdminPassword
-	// store so that we can access this later during provisioning
-	commonhelper.SetSharedState("winrm_password", c.tmpAdminPassword, c.PackerConfig.PackerBuildName)
-
+	packer.LogSecretFilter.Set(c.tmpAdminPassword)
 	c.tmpCertificatePassword = tempName.CertificatePassword
 	if c.TempComputeName == "" {
 		c.tmpComputeName = tempName.ComputeName
@@ -475,7 +471,7 @@ func assertTagProperties(c *Config, errs *packer.MultiError) {
 			errs = packer.MultiErrorAppend(errs, fmt.Errorf("the tag name %q exceeds (%d) the 512 character limit", k, len(k)))
 		}
 		if len(*v) > 256 {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("the tag name %q exceeds (%d) the 256 character limit", v, len(*v)))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("the tag name %q exceeds (%d) the 256 character limit", *v, len(*v)))
 		}
 	}
 }

builder/azure/arm/config_test.go

@@ -586,12 +586,12 @@ func TestConfigShouldAcceptTags(t *testing.T) {
 
 	value := c.AzureTags["tag01"]
 	if *value != "value01" {
-		t.Errorf("expected AzureTags[\"tag01\"] to have value \"value01\", but got %q", value)
+		t.Errorf("expected AzureTags[\"tag01\"] to have value \"value01\", but got %q", *value)
 	}
 
 	value = c.AzureTags["tag02"]
 	if *value != "value02" {
-		t.Errorf("expected AzureTags[\"tag02\"] to have value \"value02\", but got %q", value)
+		t.Errorf("expected AzureTags[\"tag02\"] to have value \"value02\", but got %q", *value)
 	}
 }
 
@@ -799,7 +799,7 @@ func TestConfigShouldRejectCustomAndPlatformManagedImageBuild(t *testing.T) {
 
 func TestConfigShouldRejectCustomAndImageUrlForManagedImageBuild(t *testing.T) {
 	config := map[string]interface{}{
-		"image_url":                                "ignore",
+		"image_url": "ignore",
 		"custom_managed_image_resource_group_name": "ignore",
 		"custom_managed_image_name":                "ignore",
 		"location":                                 "ignore",

builder/azure/arm/openssh_key_pair.go

@@ -49,11 +49,11 @@ func (s *OpenSshKeyPair) AuthorizedKey() string {
 		time.Now().Format(time.RFC3339))
 }
 
-func (s *OpenSshKeyPair) PrivateKey() string {
-	privateKey := string(pem.EncodeToMemory(&pem.Block{
+func (s *OpenSshKeyPair) PrivateKey() []byte {
+	privateKey := pem.EncodeToMemory(&pem.Block{
 		Type:  "RSA PRIVATE KEY",
 		Bytes: x509.MarshalPKCS1PrivateKey(s.privateKey),
-	}))
+	})
 
 	return privateKey
 }

builder/azure/arm/resource_resolver.go

@@ -24,7 +24,7 @@ type resourceResolver struct {
 
 func newResourceResolver(client *AzureClient) *resourceResolver {
 	return &resourceResolver{
-		client: client,
+		client:                          client,
 		findVirtualNetworkResourceGroup: findVirtualNetworkResourceGroup,
 		findVirtualNetworkSubnet:        findVirtualNetworkSubnet,
 	}

builder/azure/arm/step_delete_resource_group.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/hashicorp/packer/builder/azure/common/constants"
+	retry "github.com/hashicorp/packer/common"
 	"github.com/hashicorp/packer/helper/multistep"
 	"github.com/hashicorp/packer/packer"
 )
@@ -93,11 +94,19 @@ func (s *StepDeleteResourceGroup) deleteDeploymentResources(ctx context.Context,
 			resourceType,
 			resourceName))
 
-		err := deleteResource(ctx, s.client,
-			resourceType,
-			resourceName,
-			resourceGroupName)
-		s.reportIfError(err, resourceName)
+		err := retry.Retry(10, 600, 10, func(attempt uint) (bool, error) {
+			err := deleteResource(ctx, s.client,
+				resourceType,
+				resourceName,
+				resourceGroupName)
+			if err != nil {
+				s.reportIfError(err, resourceName)
+				return false, nil
+			}
+
+			return true, nil
+		})
+
 		if err = deploymentOperations.Next(); err != nil {
 			return err
 		}

builder/azure/arm/step_save_winrm_password.go

@@ -3,8 +3,8 @@ package arm
 import (
 	"context"
 
-	commonhelper "github.com/hashicorp/packer/helper/common"
 	"github.com/hashicorp/packer/helper/multistep"
+	"github.com/hashicorp/packer/packer"
 )
 
 type StepSaveWinRMPassword struct {
@@ -14,10 +14,10 @@ type StepSaveWinRMPassword struct {
 
 func (s *StepSaveWinRMPassword) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
 	// store so that we can access this later during provisioning
-	commonhelper.SetSharedState("winrm_password", s.Password, s.BuildName)
+	state.Put("winrm_password", s.Password)
+	packer.LogSecretFilter.Set(s.Password)
 	return multistep.ActionContinue
 }
 
 func (s *StepSaveWinRMPassword) Cleanup(multistep.StateBag) {
-	commonhelper.RemoveSharedStateFile("winrm_password", s.BuildName)
 }

builder/azure/arm/template_factory_test.go

@@ -261,10 +261,10 @@ growpart:
 // Ensure the VM template is correct when building from a custom managed image.
 func TestVirtualMachineDeployment08(t *testing.T) {
 	config := map[string]interface{}{
-		"location":                                 "ignore",
-		"subscription_id":                          "ignore",
-		"os_type":                                  constants.Target_Linux,
-		"communicator":                             "none",
+		"location":        "ignore",
+		"subscription_id": "ignore",
+		"os_type":         constants.Target_Linux,
+		"communicator":    "none",
 		"custom_managed_image_resource_group_name": "CustomManagedImageResourceGroupName",
 		"custom_managed_image_name":                "CustomManagedImageName",
 		"managed_image_name":                       "ManagedImageName",

builder/azure/arm/tempname.go

@@ -4,17 +4,7 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/hashicorp/packer/builder/azure/common"
-)
-
-const (
-	TempNameAlphabet = "0123456789bcdfghjklmnpqrstvwxyz"
-
-	numbers   = "0123456789"
-	lowerCase = "abcdefghijklmnopqrstuvwxyz"
-	upperCase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
-
-	TempPasswordAlphabet = numbers + lowerCase + upperCase
+	"github.com/hashicorp/packer/common/random"
 )
 
 type TempName struct {
@@ -34,7 +24,7 @@ type TempName struct {
 func NewTempName() *TempName {
 	tempName := &TempName{}
 
-	suffix := common.RandomString(TempNameAlphabet, 10)
+	suffix := random.AlphaNumLower(10)
 	tempName.ComputeName = fmt.Sprintf("pkrvm%s", suffix)
 	tempName.DeploymentName = fmt.Sprintf("pkrdp%s", suffix)
 	tempName.KeyVaultName = fmt.Sprintf("pkrkv%s", suffix)
@@ -46,7 +36,7 @@ func NewTempName() *TempName {
 	tempName.ResourceGroupName = fmt.Sprintf("packer-Resource-Group-%s", suffix)
 
 	tempName.AdminPassword = generatePassword()
-	tempName.CertificatePassword = common.RandomString(TempPasswordAlphabet, 32)
+	tempName.CertificatePassword = random.AlphaNum(32)
 
 	return tempName
 }
@@ -60,16 +50,16 @@ func NewTempName() *TempName {
 func generatePassword() string {
 	var s string
 	for i := 0; i < 100; i++ {
-		s := common.RandomString(TempPasswordAlphabet, 32)
-		if !strings.ContainsAny(s, numbers) {
+		s := random.AlphaNum(32)
+		if !strings.ContainsAny(s, random.PossibleNumbers) {
 			continue
 		}
 
-		if !strings.ContainsAny(s, lowerCase) {
+		if !strings.ContainsAny(s, random.PossibleLowerCase) {
 			continue
 		}
 
-		if !strings.ContainsAny(s, upperCase) {
+		if !strings.ContainsAny(s, random.PossibleUpperCase) {
 			continue
 		}
 

builder/azure/arm/tempname_test.go

@@ -3,6 +3,8 @@ package arm
 import (
 	"strings"
 	"testing"
+
+	"github.com/hashicorp/packer/common/random"
 )
 
 func TestTempNameShouldCreatePrefixedRandomNames(t *testing.T) {
@@ -44,14 +46,14 @@ func TestTempNameShouldCreatePrefixedRandomNames(t *testing.T) {
 func TestTempAdminPassword(t *testing.T) {
 	tempName := NewTempName()
 
-	if !strings.ContainsAny(tempName.AdminPassword, numbers) {
-		t.Errorf("Expected AdminPassword to contain at least one of '%s'!", numbers)
+	if !strings.ContainsAny(tempName.AdminPassword, random.PossibleNumbers) {
+		t.Errorf("Expected AdminPassword to contain at least one of '%s'!", random.PossibleNumbers)
 	}
-	if !strings.ContainsAny(tempName.AdminPassword, lowerCase) {
-		t.Errorf("Expected AdminPassword to contain at least one of '%s'!", lowerCase)
+	if !strings.ContainsAny(tempName.AdminPassword, random.PossibleLowerCase) {
+		t.Errorf("Expected AdminPassword to contain at least one of '%s'!", random.PossibleLowerCase)
 	}
-	if !strings.ContainsAny(tempName.AdminPassword, upperCase) {
-		t.Errorf("Expected AdminPassword to contain at least one of '%s'!", upperCase)
+	if !strings.ContainsAny(tempName.AdminPassword, random.PossibleUpperCase) {
+		t.Errorf("Expected AdminPassword to contain at least one of '%s'!", random.PossibleUpperCase)
 	}
 }
 

builder/azure/common/constants/stateBag.go

@@ -5,7 +5,6 @@ const (
 	AuthorizedKey string = "authorizedKey"
 	Certificate   string = "certificate"
 	Error         string = "error"
-	PrivateKey    string = "privateKey"
 	SSHHost       string = "sshHost"
 	Thumbprint    string = "thumbprint"
 	Ui            string = "ui"

builder/azure/common/lin/ssh.go

@@ -1,36 +1,11 @@
 package lin
 
 import (
-	"fmt"
-
 	"github.com/hashicorp/packer/builder/azure/common/constants"
 	"github.com/hashicorp/packer/helper/multistep"
-	"golang.org/x/crypto/ssh"
 )
 
 func SSHHost(state multistep.StateBag) (string, error) {
 	host := state.Get(constants.SSHHost).(string)
 	return host, nil
 }
-
-// SSHConfig returns a function that can be used for the SSH communicator
-// config for connecting to the instance created over SSH using the generated
-// private key.
-func SSHConfig(username string) func(multistep.StateBag) (*ssh.ClientConfig, error) {
-	return func(state multistep.StateBag) (*ssh.ClientConfig, error) {
-		privateKey := state.Get(constants.PrivateKey).(string)
-
-		signer, err := ssh.ParsePrivateKey([]byte(privateKey))
-		if err != nil {
-			return nil, fmt.Errorf("Error setting up SSH config: %s", err)
-		}
-
-		return &ssh.ClientConfig{
-			User: username,
-			Auth: []ssh.AuthMethod{
-				ssh.PublicKeys(signer),
-			},
-			HostKeyCallback: ssh.InsecureIgnoreHostKey(),
-		}, nil
-	}
-}

builder/azure/common/lin/step_create_cert.go

@@ -1,112 +0,0 @@
-package lin
-
-import (
-	"context"
-	"crypto/rand"
-	"crypto/rsa"
-	"crypto/sha1"
-	"crypto/x509"
-	"crypto/x509/pkix"
-	"encoding/pem"
-	"fmt"
-	"log"
-	"math/big"
-	"time"
-
-	"github.com/hashicorp/packer/builder/azure/common/constants"
-
-	"github.com/hashicorp/packer/helper/multistep"
-	"github.com/hashicorp/packer/packer"
-)
-
-type StepCreateCert struct {
-	TmpServiceName string
-}
-
-func (s *StepCreateCert) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
-	ui := state.Get("ui").(packer.Ui)
-
-	ui.Say("Creating temporary certificate...")
-
-	err := s.createCert(state)
-	if err != nil {
-		err = fmt.Errorf("Error creating temporary certificate: %s", err)
-		state.Put("error", err)
-		ui.Error(err.Error())
-		return multistep.ActionHalt
-	}
-
-	return multistep.ActionContinue
-}
-
-func (s *StepCreateCert) Cleanup(state multistep.StateBag) {}
-
-func (s *StepCreateCert) createCert(state multistep.StateBag) error {
-
-	log.Println("createCert: Generating RSA key pair...")
-
-	priv, err := rsa.GenerateKey(rand.Reader, 2048)
-	if err != nil {
-		err = fmt.Errorf("Failed to Generate Private Key: %s", err)
-		return err
-	}
-
-	// ASN.1 DER encoded form
-	privkey := string(pem.EncodeToMemory(&pem.Block{
-		Type:  "RSA PRIVATE KEY",
-		Bytes: x509.MarshalPKCS1PrivateKey(priv),
-	}))
-
-	// Set the private key in the state bag for later
-	state.Put(constants.PrivateKey, privkey)
-	log.Printf("createCert: Private key:\n%s", privkey)
-
-	log.Println("createCert: Creating certificate...")
-
-	host := fmt.Sprintf("%s.cloudapp.net", s.TmpServiceName)
-	notBefore := time.Now()
-	notAfter := notBefore.Add(365 * 24 * time.Hour)
-
-	serialNumber, err := rand.Int(rand.Reader, new(big.Int).Lsh(big.NewInt(1), 128))
-	if err != nil {
-		err = fmt.Errorf("Failed to Generate Serial Number: %v", err)
-		return err
-	}
-
-	template := x509.Certificate{
-		SerialNumber: serialNumber,
-		Issuer: pkix.Name{
-			CommonName: host,
-		},
-		Subject: pkix.Name{
-			CommonName: host,
-		},
-		NotBefore: notBefore,
-		NotAfter:  notAfter,
-
-		KeyUsage:              x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
-		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
-		BasicConstraintsValid: true,
-	}
-
-	derBytes, err := x509.CreateCertificate(rand.Reader, &template, &template, &priv.PublicKey, priv)
-	if err != nil {
-		err = fmt.Errorf("Failed to Create Certificate: %s", err)
-		return err
-	}
-
-	cert := string(pem.EncodeToMemory(&pem.Block{
-		Type:  "RSA PRIVATE KEY",
-		Bytes: derBytes,
-	}))
-	state.Put(constants.Certificate, cert)
-	log.Printf("createCert: Certificate:\n%s", cert)
-
-	h := sha1.New()
-	h.Write(derBytes)
-	thumbprint := fmt.Sprintf("%X", h.Sum(nil))
-	state.Put(constants.Thumbprint, thumbprint)
-	log.Printf("createCert: Thumbprint:\n%s", thumbprint)
-
-	return nil
-}

builder/azure/common/lin/step_generalize_os.go

@@ -1,56 +0,0 @@
-package lin
-
-import (
-	"bytes"
-	"context"
-	"fmt"
-	"log"
-
-	"github.com/hashicorp/packer/helper/multistep"
-	"github.com/hashicorp/packer/packer"
-)
-
-type StepGeneralizeOS struct {
-	Command string
-}
-
-func (s *StepGeneralizeOS) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
-	ui := state.Get("ui").(packer.Ui)
-	comm := state.Get("communicator").(packer.Communicator)
-
-	ui.Say("Executing OS generalization...")
-
-	var stdout, stderr bytes.Buffer
-	cmd := &packer.RemoteCmd{
-		Command: s.Command,
-		Stdout:  &stdout,
-		Stderr:  &stderr,
-	}
-
-	if err := comm.Start(cmd); err != nil {
-		err = fmt.Errorf("Failed executing OS generalization command: %s", err)
-		state.Put("error", err)
-		ui.Error(err.Error())
-		return multistep.ActionHalt
-	}
-
-	// Wait for the command to run
-	cmd.Wait()
-
-	// If the command failed to run, notify the user in some way.
-	if cmd.ExitStatus != 0 {
-		state.Put("error", fmt.Errorf(
-			"OS generalization has non-zero exit status.\n\nStdout: %s\n\nStderr: %s",
-			stdout.String(), stderr.String()))
-		return multistep.ActionHalt
-	}
-
-	log.Printf("OS generalization stdout: %s", stdout.String())
-	log.Printf("OS generalization stderr: %s", stderr.String())
-
-	return multistep.ActionContinue
-}
-
-func (s *StepGeneralizeOS) Cleanup(state multistep.StateBag) {
-	// do nothing
-}

builder/azure/common/randomstring.go

@@ -1,45 +0,0 @@
-package common
-
-import (
-	"math/rand"
-	"os"
-	"time"
-)
-
-var pwSymbols = []string{
-	"abcdefghijklmnopqrstuvwxyz",
-	"ABCDEFGHIJKLMNOPQRSTUVWXYZ",
-	"0123456789",
-}
-
-var rnd = rand.New(rand.NewSource(time.Now().UnixNano() + int64(os.Getpid())))
-
-func RandomString(chooseFrom string, length int) (randomString string) {
-	cflen := len(chooseFrom)
-	for i := 0; i < length; i++ {
-		randomString += string(chooseFrom[rnd.Intn(cflen)])
-	}
-	return
-}
-
-func RandomPassword() (password string) {
-	pwlen := 15
-	batchsize := pwlen / len(pwSymbols)
-	pw := make([]byte, 0, pwlen)
-	// choose character set
-	for c := 0; len(pw) < pwlen; c++ {
-		s := RandomString(pwSymbols[c%len(pwSymbols)], rnd.Intn(batchsize-1)+1)
-		pw = append(pw, []byte(s)...)
-	}
-	// truncate
-	pw = pw[:pwlen]
-
-	// permute
-	for c := 0; c < pwlen-1; c++ {
-		i := rnd.Intn(pwlen-c) + c
-		x := pw[c]
-		pw[c] = pw[i]
-		pw[i] = x
-	}
-	return string(pw)
-}

builder/azure/common/randomstring_test.go

@@ -1,15 +0,0 @@
-package common
-
-import (
-	"testing"
-)
-
-func TestRandomPassword_generates_15char_passwords(t *testing.T) {
-	for i := 0; i < 100; i++ {
-		pw := RandomPassword()
-		t.Logf("pw: %v", pw)
-		if len(pw) != 15 {
-			t.Fatalf("len(pw)!=15, but %v: %v (%v)", len(pw), pw, i)
-		}
-	}
-}

builder/azure/common/template/template_parameters_test.go

@@ -14,8 +14,8 @@ func TestTemplateParametersShouldHaveExpectedKeys(t *testing.T) {
 		DnsNameForPublicIP:         &TemplateParameter{Value: "sentinel"},
 		OSDiskName:                 &TemplateParameter{Value: "sentinel"},
 		StorageAccountBlobEndpoint: &TemplateParameter{Value: "sentinel"},
-		VMName: &TemplateParameter{Value: "sentinel"},
-		VMSize: &TemplateParameter{Value: "sentinel"},
+		VMName:                     &TemplateParameter{Value: "sentinel"},
+		VMSize:                     &TemplateParameter{Value: "sentinel"},
 	}
 
 	bs, err := json.Marshal(params)
@@ -55,8 +55,8 @@ func TestParameterValuesShouldBeSet(t *testing.T) {
 		DnsNameForPublicIP:         &TemplateParameter{Value: "dnsnameforpublicip00"},
 		OSDiskName:                 &TemplateParameter{Value: "osdiskname00"},
 		StorageAccountBlobEndpoint: &TemplateParameter{Value: "storageaccountblobendpoint00"},
-		VMName: &TemplateParameter{Value: "vmname00"},
-		VMSize: &TemplateParameter{Value: "vmsize00"},
+		VMName:                     &TemplateParameter{Value: "vmname00"},
+		VMSize:                     &TemplateParameter{Value: "vmsize00"},
 	}
 
 	bs, err := json.Marshal(params)

builder/cloudstack/builder.go

@@ -64,12 +64,9 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			HTTPPortMax: b.config.HTTPPortMax,
 		},
 		&stepKeypair{
-			Debug:                b.config.PackerDebug,
-			DebugKeyPath:         fmt.Sprintf("cs_%s.pem", b.config.PackerBuildName),
-			KeyPair:              b.config.Keypair,
-			PrivateKeyFile:       b.config.Comm.SSHPrivateKey,
-			SSHAgentAuth:         b.config.Comm.SSHAgentAuth,
-			TemporaryKeyPairName: b.config.TemporaryKeypairName,
+			Debug:        b.config.PackerDebug,
+			Comm:         &b.config.Comm,
+			DebugKeyPath: fmt.Sprintf("cs_%s.pem", b.config.PackerBuildName),
 		},
 		&stepCreateSecurityGroup{},
 		&stepCreateInstance{
@@ -78,12 +75,9 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		},
 		&stepSetupNetworking{},
 		&communicator.StepConnect{
-			Config: &b.config.Comm,
-			Host:   commHost,
-			SSHConfig: sshConfig(
-				b.config.Comm.SSHAgentAuth,
-				b.config.Comm.SSHUsername,
-				b.config.Comm.SSHPassword),
+			Config:    &b.config.Comm,
+			Host:      commHost,
+			SSHConfig: b.config.Comm.SSHConfigFunc(),
 			SSHPort:   commPort,
 			WinRMPort: commPort,
 		},

builder/cloudstack/config.go

@@ -34,7 +34,6 @@ type Config struct {
 	Expunge                bool     `mapstructure:"expunge"`
 	Hypervisor             string   `mapstructure:"hypervisor"`
 	InstanceName           string   `mapstructure:"instance_name"`
-	Keypair                string   `mapstructure:"keypair"`
 	Network                string   `mapstructure:"network"`
 	Project                string   `mapstructure:"project"`
 	PublicIPAddress        string   `mapstructure:"public_ip_address"`
@@ -126,9 +125,9 @@ func NewConfig(raws ...interface{}) (*Config, error) {
 	// If we are not given an explicit keypair, ssh_password or ssh_private_key_file,
 	// then create a temporary one, but only if the temporary_keypair_name has not
 	// been provided.
-	if c.Keypair == "" && c.TemporaryKeypairName == "" &&
-		c.Comm.SSHPrivateKey == "" && c.Comm.SSHPassword == "" {
-		c.TemporaryKeypairName = fmt.Sprintf("packer_%s", uuid.TimeOrderedUUID())
+	if c.Comm.SSHKeyPairName == "" && c.Comm.SSHTemporaryKeyPairName == "" &&
+		c.Comm.SSHPrivateKeyFile == "" && c.Comm.SSHPassword == "" {
+		c.Comm.SSHTemporaryKeyPairName = fmt.Sprintf("packer_%s", uuid.TimeOrderedUUID())
 	}
 
 	// Process required parameters.

builder/cloudstack/ssh.go

@@ -2,13 +2,8 @@ package cloudstack
 
 import (
 	"fmt"
-	"net"
-	"os"
 
-	packerssh "github.com/hashicorp/packer/communicator/ssh"
 	"github.com/hashicorp/packer/helper/multistep"
-	"golang.org/x/crypto/ssh"
-	"golang.org/x/crypto/ssh/agent"
 )
 
 func commHost(state multistep.StateBag) (string, error) {
@@ -28,55 +23,3 @@ func commPort(state multistep.StateBag) (int, error) {
 
 	return commPort, nil
 }
-
-func sshConfig(useAgent bool, username, password string) func(state multistep.StateBag) (*ssh.ClientConfig, error) {
-	return func(state multistep.StateBag) (*ssh.ClientConfig, error) {
-		if useAgent {
-			authSock := os.Getenv("SSH_AUTH_SOCK")
-			if authSock == "" {
-				return nil, fmt.Errorf("SSH_AUTH_SOCK is not set")
-			}
-
-			sshAgent, err := net.Dial("unix", authSock)
-			if err != nil {
-				return nil, fmt.Errorf("Cannot connect to SSH Agent socket %q: %s", authSock, err)
-			}
-
-			return &ssh.ClientConfig{
-				User: username,
-				Auth: []ssh.AuthMethod{
-					ssh.PublicKeysCallback(agent.NewClient(sshAgent).Signers),
-				},
-				HostKeyCallback: ssh.InsecureIgnoreHostKey(),
-			}, nil
-		}
-
-		privateKey, hasKey := state.GetOk("privateKey")
-
-		if hasKey {
-			signer, err := ssh.ParsePrivateKey([]byte(privateKey.(string)))
-			if err != nil {
-				return nil, fmt.Errorf("Error setting up SSH config: %s", err)
-			}
-
-			return &ssh.ClientConfig{
-				User: username,
-				Auth: []ssh.AuthMethod{
-					ssh.PublicKeys(signer),
-				},
-				HostKeyCallback: ssh.InsecureIgnoreHostKey(),
-			}, nil
-
-		} else {
-
-			return &ssh.ClientConfig{
-				User:            username,
-				HostKeyCallback: ssh.InsecureIgnoreHostKey(),
-				Auth: []ssh.AuthMethod{
-					ssh.Password(password),
-					ssh.KeyboardInteractive(
-						packerssh.PasswordKeyboardInteractive(password)),
-				}}, nil
-		}
-	}
-}

builder/cloudstack/step_create_instance.go

@@ -46,10 +46,9 @@ func (s *stepCreateInstance) Run(_ context.Context, state multistep.StateBag) mu
 	p.SetName(config.InstanceName)
 	p.SetDisplayname("Created by Packer")
 
-	if keypair, ok := state.GetOk("keypair"); ok {
-		kp := keypair.(string)
-		ui.Message(fmt.Sprintf("Using keypair: %s", kp))
-		p.SetKeypair(kp)
+	if len(config.Comm.SSHKeyPairName) != 0 {
+		ui.Message(fmt.Sprintf("Using keypair: %s", config.Comm.SSHKeyPairName))
+		p.SetKeypair(config.Comm.SSHKeyPairName)
 	}
 
 	if securitygroups, ok := state.GetOk("security_groups"); ok {

builder/cloudstack/step_keypair.go

@@ -7,59 +7,55 @@ import (
 	"os"
 	"runtime"
 
+	"github.com/hashicorp/packer/helper/communicator"
 	"github.com/hashicorp/packer/helper/multistep"
 	"github.com/hashicorp/packer/packer"
 	"github.com/xanzy/go-cloudstack/cloudstack"
 )
 
 type stepKeypair struct {
-	Debug                bool
-	DebugKeyPath         string
-	KeyPair              string
-	PrivateKeyFile       string
-	SSHAgentAuth         bool
-	TemporaryKeyPairName string
+	Debug        bool
+	Comm         *communicator.Config
+	DebugKeyPath string
 }
 
 func (s *stepKeypair) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
 	ui := state.Get("ui").(packer.Ui)
 
-	if s.PrivateKeyFile != "" {
-		privateKeyBytes, err := ioutil.ReadFile(s.PrivateKeyFile)
+	if s.Comm.SSHPrivateKeyFile != "" {
+		privateKeyBytes, err := ioutil.ReadFile(s.Comm.SSHPrivateKeyFile)
 		if err != nil {
 			state.Put("error", fmt.Errorf(
 				"Error loading configured private key file: %s", err))
 			return multistep.ActionHalt
 		}
 
-		state.Put("keypair", s.KeyPair)
-		state.Put("privateKey", string(privateKeyBytes))
+		s.Comm.SSHPrivateKey = privateKeyBytes
 
 		return multistep.ActionContinue
 	}
 
-	if s.SSHAgentAuth && s.KeyPair == "" {
+	if s.Comm.SSHAgentAuth && s.Comm.SSHKeyPairName == "" {
 		ui.Say("Using SSH Agent with keypair in Source image")
 		return multistep.ActionContinue
 	}
 
-	if s.SSHAgentAuth && s.KeyPair != "" {
-		ui.Say(fmt.Sprintf("Using SSH Agent for existing keypair %s", s.KeyPair))
-		state.Put("keypair", s.KeyPair)
+	if s.Comm.SSHAgentAuth && s.Comm.SSHKeyPairName != "" {
+		ui.Say(fmt.Sprintf("Using SSH Agent for existing keypair %s", s.Comm.SSHKeyPairName))
 		return multistep.ActionContinue
 	}
 
-	if s.TemporaryKeyPairName == "" {
+	if s.Comm.SSHTemporaryKeyPairName == "" {
 		ui.Say("Not using a keypair")
-		state.Put("keypair", "")
+		s.Comm.SSHKeyPairName = ""
 		return multistep.ActionContinue
 	}
 
 	client := state.Get("client").(*cloudstack.CloudStackClient)
 
-	ui.Say(fmt.Sprintf("Creating temporary keypair: %s ...", s.TemporaryKeyPairName))
+	ui.Say(fmt.Sprintf("Creating temporary keypair: %s ...", s.Comm.SSHTemporaryKeyPairName))
 
-	p := client.SSH.NewCreateSSHKeyPairParams(s.TemporaryKeyPairName)
+	p := client.SSH.NewCreateSSHKeyPairParams(s.Comm.SSHTemporaryKeyPairName)
 
 	cfg := state.Get("config").(*Config)
 	if cfg.Project != "" {
@@ -81,7 +77,7 @@ func (s *stepKeypair) Run(_ context.Context, state multistep.StateBag) multistep
 		return multistep.ActionHalt
 	}
 
-	ui.Say(fmt.Sprintf("Created temporary keypair: %s", s.TemporaryKeyPairName))
+	ui.Say(fmt.Sprintf("Created temporary keypair: %s", s.Comm.SSHTemporaryKeyPairName))
 
 	// If we're in debug mode, output the private key to the working directory.
 	if s.Debug {
@@ -112,15 +108,15 @@ func (s *stepKeypair) Run(_ context.Context, state multistep.StateBag) multistep
 		}
 	}
 
-	// Set some state data for use in future steps
-	state.Put("keypair", s.TemporaryKeyPairName)
-	state.Put("privateKey", keypair.Privatekey)
+	// Set some data for use in future steps
+	s.Comm.SSHKeyPairName = s.Comm.SSHTemporaryKeyPairName
+	s.Comm.SSHPrivateKey = []byte(keypair.Privatekey)
 
 	return multistep.ActionContinue
 }
 
 func (s *stepKeypair) Cleanup(state multistep.StateBag) {
-	if s.TemporaryKeyPairName == "" {
+	if s.Comm.SSHTemporaryKeyPairName == "" {
 		return
 	}
 
@@ -128,17 +124,17 @@ func (s *stepKeypair) Cleanup(state multistep.StateBag) {
 	client := state.Get("client").(*cloudstack.CloudStackClient)
 	cfg := state.Get("config").(*Config)
 
-	p := client.SSH.NewDeleteSSHKeyPairParams(s.TemporaryKeyPairName)
+	p := client.SSH.NewDeleteSSHKeyPairParams(s.Comm.SSHTemporaryKeyPairName)
 	if cfg.Project != "" {
 		p.SetProjectid(cfg.Project)
 	}
 
-	ui.Say(fmt.Sprintf("Deleting temporary keypair: %s ...", s.TemporaryKeyPairName))
+	ui.Say(fmt.Sprintf("Deleting temporary keypair: %s ...", s.Comm.SSHTemporaryKeyPairName))
 
 	_, err := client.SSH.DeleteSSHKeyPair(p)
 	if err != nil {
 		ui.Error(err.Error())
 		ui.Error(fmt.Sprintf(
-			"Error cleaning up keypair. Please delete the key manually: %s", s.TemporaryKeyPairName))
+			"Error cleaning up keypair. Please delete the key manually: %s", s.Comm.SSHTemporaryKeyPairName))
 	}
 }

builder/digitalocean/builder.go

@@ -87,7 +87,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		&communicator.StepConnect{
 			Config:    &b.config.Comm,
 			Host:      commHost,
-			SSHConfig: sshConfig,
+			SSHConfig: b.config.Comm.SSHConfigFunc(),
 		},
 		new(common.StepProvision),
 		new(stepShutdown),

builder/digitalocean/config.go

@@ -138,6 +138,6 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) {
 		return nil, nil, errs
 	}
 
-	common.ScrubConfig(c, c.APIToken)
+	packer.LogSecretFilter.Set(c.APIToken)
 	return c, nil, nil
 }

builder/digitalocean/ssh.go

@@ -1,10 +1,6 @@
 package digitalocean
 
 import (
-	"fmt"
-
-	"golang.org/x/crypto/ssh"
-
 	"github.com/hashicorp/packer/helper/multistep"
 )
 
@@ -12,21 +8,3 @@ func commHost(state multistep.StateBag) (string, error) {
 	ipAddress := state.Get("droplet_ip").(string)
 	return ipAddress, nil
 }
-
-func sshConfig(state multistep.StateBag) (*ssh.ClientConfig, error) {
-	config := state.Get("config").(Config)
-	privateKey := state.Get("privateKey").(string)
-
-	signer, err := ssh.ParsePrivateKey([]byte(privateKey))
-	if err != nil {
-		return nil, fmt.Errorf("Error setting up SSH config: %s", err)
-	}
-
-	return &ssh.ClientConfig{
-		User: config.Comm.SSHUsername,
-		Auth: []ssh.AuthMethod{
-			ssh.PublicKeys(signer),
-		},
-		HostKeyCallback: ssh.InsecureIgnoreHostKey(),
-	}, nil
-}

builder/digitalocean/step_create_ssh_key.go

@@ -28,6 +28,7 @@ type stepCreateSSHKey struct {
 func (s *stepCreateSSHKey) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
 	client := state.Get("client").(*godo.Client)
 	ui := state.Get("ui").(packer.Ui)
+	c := state.Get("config").(*Config)
 
 	ui.Say("Creating temporary ssh key for droplet...")
 
@@ -41,8 +42,8 @@ func (s *stepCreateSSHKey) Run(_ context.Context, state multistep.StateBag) mult
 		Bytes:   priv_der,
 	}
 
-	// Set the private key in the statebag for later
-	state.Put("privateKey", string(pem.EncodeToMemory(&priv_blk)))
+	// Set the private key in the config for later
+	c.Comm.SSHPrivateKey = pem.EncodeToMemory(&priv_blk)
 
 	// Marshal the public key into SSH compatible format
 	// TODO properly handle the public key error

builder/docker/builder.go

@@ -48,7 +48,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		&communicator.StepConnect{
 			Config:    &b.config.Comm,
 			Host:      commHost,
-			SSHConfig: sshConfig(&b.config.Comm),
+			SSHConfig: b.config.Comm.SSHConfigFunc(),
 			CustomConnect: map[string]multistep.Step{
 				"docker": &StepConnectDocker{},
 			},

builder/docker/comm.go

@@ -1,13 +1,7 @@
 package docker
 
 import (
-	"fmt"
-	"io/ioutil"
-
-	"github.com/hashicorp/packer/communicator/ssh"
-	"github.com/hashicorp/packer/helper/communicator"
 	"github.com/hashicorp/packer/helper/multistep"
-	gossh "golang.org/x/crypto/ssh"
 )
 
 func commHost(state multistep.StateBag) (string, error) {
@@ -15,40 +9,3 @@ func commHost(state multistep.StateBag) (string, error) {
 	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),
-				},
-				HostKeyCallback: gossh.InsecureIgnoreHostKey(),
-			}, nil
-		} else {
-			// password based auth
-			return &gossh.ClientConfig{
-				User: comm.SSHUsername,
-				Auth: []gossh.AuthMethod{
-					gossh.Password(comm.SSHPassword),
-					gossh.KeyboardInteractive(
-						ssh.PasswordKeyboardInteractive(comm.SSHPassword)),
-				},
-				HostKeyCallback: gossh.InsecureIgnoreHostKey(),
-			}, nil
-		}
-	}
-}

builder/docker/communicator_test.go

@@ -68,8 +68,8 @@ func TestUploadDownload(t *testing.T) {
 	hooks[packer.HookProvision] = []packer.Hook{
 		&packer.ProvisionHook{
 			Provisioners: []*packer.HookedProvisioner{
-				{upload, nil, ""},
-				{download, nil, ""},
+				{Provisioner: upload, Config: nil, TypeName: ""},
+				{Provisioner: download, Config: nil, TypeName: ""},
 			},
 		},
 	}
@@ -157,9 +157,9 @@ func TestLargeDownload(t *testing.T) {
 	hooks[packer.HookProvision] = []packer.Hook{
 		&packer.ProvisionHook{
 			Provisioners: []*packer.HookedProvisioner{
-				{shell, nil, ""},
-				{downloadCupcake, nil, ""},
-				{downloadBigcake, nil, ""},
+				{Provisioner: shell, Config: nil, TypeName: ""},
+				{Provisioner: downloadCupcake, Config: nil, TypeName: ""},
+				{Provisioner: downloadBigcake, Config: nil, TypeName: ""},
 			},
 		},
 	}
@@ -266,10 +266,10 @@ func TestFixUploadOwner(t *testing.T) {
 	hooks[packer.HookProvision] = []packer.Hook{
 		&packer.ProvisionHook{
 			Provisioners: []*packer.HookedProvisioner{
-				{fileProvisioner, nil, ""},
-				{dirProvisioner, nil, ""},
-				{shellProvisioner, nil, ""},
-				{verifyProvisioner, nil, ""},
+				{Provisioner: fileProvisioner, Config: nil, TypeName: ""},
+				{Provisioner: dirProvisioner, Config: nil, TypeName: ""},
+				{Provisioner: shellProvisioner, Config: nil, TypeName: ""},
+				{Provisioner: verifyProvisioner, Config: nil, TypeName: ""},
 			},
 		},
 	}

builder/googlecompute/builder.go

@@ -51,9 +51,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 	steps := []multistep.Step{
 		new(StepCheckExistingImage),
 		&StepCreateSSHKey{
-			Debug:          b.config.PackerDebug,
-			DebugKeyPath:   fmt.Sprintf("gce_%s.pem", b.config.PackerBuildName),
-			PrivateKeyFile: b.config.Comm.SSHPrivateKey,
+			Debug:        b.config.PackerDebug,
+			DebugKeyPath: fmt.Sprintf("gce_%s.pem", b.config.PackerBuildName),
 		},
 		&StepCreateInstance{
 			Debug: b.config.PackerDebug,
@@ -68,7 +67,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		&communicator.StepConnect{
 			Config:      &b.config.Comm,
 			Host:        commHost,
-			SSHConfig:   sshConfig,
+			SSHConfig:   b.config.Comm.SSHConfigFunc(),
 			WinRMConfig: winrmConfig,
 		},
 		new(common.StepProvision),

builder/googlecompute/driver_gce.go

@@ -168,7 +168,27 @@ func (d *driverGCE) DeleteDisk(zone, name string) (<-chan error, error) {
 }
 
 func (d *driverGCE) GetImage(name string, fromFamily bool) (*Image, error) {
-	projects := []string{d.projectId, "centos-cloud", "coreos-cloud", "cos-cloud", "debian-cloud", "google-containers", "opensuse-cloud", "rhel-cloud", "suse-cloud", "ubuntu-os-cloud", "windows-cloud", "gce-nvme", "windows-sql-cloud", "rhel-sap-cloud"}
+	projects := []string{
+		d.projectId,
+		// Public projects, drawn from
+		// https://cloud.google.com/compute/docs/images
+		"centos-cloud",
+		"cos-cloud",
+		"coreos-cloud",
+		"debian-cloud",
+		"rhel-cloud",
+		"rhel-sap-cloud",
+		"suse-cloud",
+		"suse-sap-cloud",
+		"ubuntu-os-cloud",
+		"windows-cloud",
+		"windows-sql-cloud",
+		"gce-uefi-images",
+		"gce-nvme",
+		// misc
+		"google-containers",
+		"opensuse-cloud",
+	}
 	var errs error
 	for _, project := range projects {
 		image, err := d.GetImageFromProject(project, name, fromFamily)

builder/googlecompute/ssh.go

@@ -1,32 +1,10 @@
 package googlecompute
 
 import (
-	"fmt"
-
 	"github.com/hashicorp/packer/helper/multistep"
-	"golang.org/x/crypto/ssh"
 )
 
 func commHost(state multistep.StateBag) (string, error) {
 	ipAddress := state.Get("instance_ip").(string)
 	return ipAddress, nil
 }
-
-// sshConfig returns the ssh configuration.
-func sshConfig(state multistep.StateBag) (*ssh.ClientConfig, error) {
-	config := state.Get("config").(*Config)
-	privateKey := state.Get("ssh_private_key").(string)
-
-	signer, err := ssh.ParsePrivateKey([]byte(privateKey))
-	if err != nil {
-		return nil, fmt.Errorf("Error setting up SSH config: %s", err)
-	}
-
-	return &ssh.ClientConfig{
-		User: config.Comm.SSHUsername,
-		Auth: []ssh.AuthMethod{
-			ssh.PublicKeys(signer),
-		},
-		HostKeyCallback: ssh.InsecureIgnoreHostKey(),
-	}, nil
-}

builder/googlecompute/step_create_instance.go

@@ -76,7 +76,7 @@ func getImage(c *Config, d Driver) (*Image, error) {
 func (s *StepCreateInstance) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
 	c := state.Get("config").(*Config)
 	d := state.Get("driver").(Driver)
-	sshPublicKey := state.Get("ssh_public_key").(string)
+
 	ui := state.Get("ui").(packer.Ui)
 
 	sourceImage, err := getImage(c, d)
@@ -98,7 +98,7 @@ func (s *StepCreateInstance) Run(_ context.Context, state multistep.StateBag) mu
 
 	var errCh <-chan error
 	var metadata map[string]string
-	metadata, err = c.createInstanceMetadata(sourceImage, sshPublicKey)
+	metadata, err = c.createInstanceMetadata(sourceImage, string(c.Comm.SSHPublicKey))
 	errCh, err = d.RunInstance(&InstanceConfig{
 		AcceleratorType:              c.AcceleratorType,
 		AcceleratorCount:             c.AcceleratorCount,

builder/googlecompute/step_create_ssh_key.go

@@ -17,28 +17,27 @@ import (
 
 // StepCreateSSHKey represents a Packer build step that generates SSH key pairs.
 type StepCreateSSHKey struct {
-	Debug          bool
-	DebugKeyPath   string
-	PrivateKeyFile string
+	Debug        bool
+	DebugKeyPath string
 }
 
 // Run executes the Packer build step that generates SSH key pairs.
-// The key pairs are added to the multistep state as "ssh_private_key" and
-// "ssh_public_key".
+// The key pairs are added to the ssh config
 func (s *StepCreateSSHKey) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
 	ui := state.Get("ui").(packer.Ui)
+	config := state.Get("config").(*Config)
 
-	if s.PrivateKeyFile != "" {
+	if config.Comm.SSHPrivateKeyFile != "" {
 		ui.Say("Using existing SSH private key")
-		privateKeyBytes, err := ioutil.ReadFile(s.PrivateKeyFile)
+		privateKeyBytes, err := ioutil.ReadFile(config.Comm.SSHPrivateKeyFile)
 		if err != nil {
 			state.Put("error", fmt.Errorf(
 				"Error loading configured private key file: %s", err))
 			return multistep.ActionHalt
 		}
 
-		state.Put("ssh_private_key", string(privateKeyBytes))
-		state.Put("ssh_public_key", "")
+		config.Comm.SSHPrivateKey = privateKeyBytes
+		config.Comm.SSHPublicKey = nil
 
 		return multistep.ActionContinue
 	}
@@ -65,8 +64,8 @@ func (s *StepCreateSSHKey) Run(_ context.Context, state multistep.StateBag) mult
 		ui.Error(err.Error())
 		return multistep.ActionHalt
 	}
-	state.Put("ssh_private_key", string(pem.EncodeToMemory(&priv_blk)))
-	state.Put("ssh_public_key", string(ssh.MarshalAuthorizedKey(pub)))
+	config.Comm.SSHPrivateKey = pem.EncodeToMemory(&priv_blk)
+	config.Comm.SSHPublicKey = ssh.MarshalAuthorizedKey(pub)
 
 	if s.Debug {
 		ui.Message(fmt.Sprintf("Saving key for debug purposes: %s", s.DebugKeyPath))

builder/googlecompute/step_create_ssh_key_test.go

@@ -17,7 +17,8 @@ func TestStepCreateSSHKey_impl(t *testing.T) {
 func TestStepCreateSSHKey_privateKey(t *testing.T) {
 	state := testState(t)
 	step := new(StepCreateSSHKey)
-	step.PrivateKeyFile = "test-fixtures/fake-key"
+	cfg := state.Get("config").(*Config)
+	cfg.Comm.SSHPrivateKeyFile = "test-fixtures/fake-key"
 	defer step.Cleanup(state)
 
 	// run the step
@@ -26,7 +27,7 @@ func TestStepCreateSSHKey_privateKey(t *testing.T) {
 	}
 
 	// Verify that we have a public/private key
-	if _, ok := state.GetOk("ssh_private_key"); !ok {
+	if len(cfg.Comm.SSHPrivateKey) == 0 {
 		t.Fatal("should have key")
 	}
 }
@@ -34,6 +35,7 @@ func TestStepCreateSSHKey_privateKey(t *testing.T) {
 func TestStepCreateSSHKey(t *testing.T) {
 	state := testState(t)
 	step := new(StepCreateSSHKey)
+	cfg := state.Get("config").(*Config)
 	defer step.Cleanup(state)
 
 	// run the step
@@ -42,10 +44,10 @@ func TestStepCreateSSHKey(t *testing.T) {
 	}
 
 	// Verify that we have a public/private key
-	if _, ok := state.GetOk("ssh_private_key"); !ok {
+	if len(cfg.Comm.SSHPrivateKey) == 0 {
 		t.Fatal("should have key")
 	}
-	if _, ok := state.GetOk("ssh_public_key"); !ok {
+	if len(cfg.Comm.SSHPublicKey) == 0 {
 		t.Fatal("should have key")
 	}
 }
@@ -60,6 +62,7 @@ func TestStepCreateSSHKey_debug(t *testing.T) {
 
 	state := testState(t)
 	step := new(StepCreateSSHKey)
+	cfg := state.Get("config").(*Config)
 	step.Debug = true
 	step.DebugKeyPath = tf.Name()
 
@@ -71,10 +74,10 @@ func TestStepCreateSSHKey_debug(t *testing.T) {
 	}
 
 	// Verify that we have a public/private key
-	if _, ok := state.GetOk("ssh_private_key"); !ok {
+	if len(cfg.Comm.SSHPrivateKey) == 0 {
 		t.Fatal("should have key")
 	}
-	if _, ok := state.GetOk("ssh_public_key"); !ok {
+	if len(cfg.Comm.SSHPublicKey) == 0 {
 		t.Fatal("should have key")
 	}
 	if _, err := os.Stat(tf.Name()); err != nil {

builder/googlecompute/step_create_windows_password.go

@@ -13,7 +13,6 @@ import (
 	"os"
 	"time"
 
-	commonhelper "github.com/hashicorp/packer/helper/common"
 	"github.com/hashicorp/packer/helper/multistep"
 	"github.com/hashicorp/packer/packer"
 )
@@ -33,6 +32,7 @@ func (s *StepCreateWindowsPassword) Run(_ context.Context, state multistep.State
 
 	if c.Comm.WinRMPassword != "" {
 		state.Put("winrm_password", c.Comm.WinRMPassword)
+		packer.LogSecretFilter.Set(c.Comm.WinRMPassword)
 		return multistep.ActionContinue
 	}
 
@@ -113,7 +113,7 @@ func (s *StepCreateWindowsPassword) Run(_ context.Context, state multistep.State
 	}
 
 	state.Put("winrm_password", data.password)
-	commonhelper.SetSharedState("winrm_password", data.password, c.PackerConfig.PackerBuildName)
+	packer.LogSecretFilter.Set(data.password)
 
 	return multistep.ActionContinue
 }

builder/hyperv/common/driver.go

@@ -70,7 +70,7 @@ type Driver interface {
 
 	DeleteVirtualSwitch(string) error
 
-	CreateVirtualMachine(string, string, string, string, int64, int64, int64, string, uint, bool, bool) error
+	CreateVirtualMachine(string, string, string, int64, int64, int64, string, uint, bool, bool) error
 
 	AddVirtualMachineHardDrive(string, string, string, int64, int64, string) error
 
@@ -94,9 +94,11 @@ type Driver interface {
 
 	ExportVirtualMachine(string, string) error
 
-	CompactDisks(string, string) error
+	PreserveLegacyExportBehaviour(string, string) error
 
-	CopyExportedVirtualMachine(string, string, string, string) error
+	MoveCreatedVHDsToOutputDir(string, string) error
+
+	CompactDisks(string) (string, error)
 
 	RestartVirtualMachine(string) error
 

builder/hyperv/common/driver_mock.go

@@ -124,7 +124,6 @@ type DriverMock struct {
 	CreateVirtualMachine_VmName           string
 	CreateVirtualMachine_Path             string
 	CreateVirtualMachine_HarddrivePath    string
-	CreateVirtualMachine_VhdPath          string
 	CreateVirtualMachine_Ram              int64
 	CreateVirtualMachine_DiskSize         int64
 	CreateVirtualMachine_DiskBlockSize    int64
@@ -135,7 +134,7 @@ type DriverMock struct {
 	CreateVirtualMachine_Err              error
 
 	CloneVirtualMachine_Called                bool
-	CloneVirtualMachine_CloneFromVmxcPath     string
+	CloneVirtualMachine_CloneFromVmcxPath     string
 	CloneVirtualMachine_CloneFromVmName       string
 	CloneVirtualMachine_CloneFromSnapshotName string
 	CloneVirtualMachine_CloneAllSnapshots     bool
@@ -186,17 +185,20 @@ type DriverMock struct {
 	ExportVirtualMachine_Path   string
 	ExportVirtualMachine_Err    error
 
-	CompactDisks_Called  bool
-	CompactDisks_ExpPath string
-	CompactDisks_VhdDir  string
-	CompactDisks_Err     error
+	PreserveLegacyExportBehaviour_Called  bool
+	PreserveLegacyExportBehaviour_SrcPath string
+	PreserveLegacyExportBehaviour_DstPath string
+	PreserveLegacyExportBehaviour_Err     error
 
-	CopyExportedVirtualMachine_Called     bool
-	CopyExportedVirtualMachine_ExpPath    string
-	CopyExportedVirtualMachine_OutputPath string
-	CopyExportedVirtualMachine_VhdDir     string
-	CopyExportedVirtualMachine_VmDir      string
-	CopyExportedVirtualMachine_Err        error
+	MoveCreatedVHDsToOutputDir_Called  bool
+	MoveCreatedVHDsToOutputDir_SrcPath string
+	MoveCreatedVHDsToOutputDir_DstPath string
+	MoveCreatedVHDsToOutputDir_Err     error
+
+	CompactDisks_Called bool
+	CompactDisks_Path   string
+	CompactDisks_Result string
+	CompactDisks_Err    error
 
 	RestartVirtualMachine_Called bool
 	RestartVirtualMachine_VmName string
@@ -393,7 +395,8 @@ func (d *DriverMock) CreateVirtualSwitch(switchName string, switchType string) (
 	return d.CreateVirtualSwitch_Return, d.CreateVirtualSwitch_Err
 }
 
-func (d *DriverMock) AddVirtualMachineHardDrive(vmName string, vhdFile string, vhdName string, vhdSizeBytes int64, vhdDiskBlockSize int64, controllerType string) error {
+func (d *DriverMock) AddVirtualMachineHardDrive(vmName string, vhdFile string, vhdName string,
+	vhdSizeBytes int64, vhdDiskBlockSize int64, controllerType string) error {
 	d.AddVirtualMachineHardDrive_Called = true
 	d.AddVirtualMachineHardDrive_VmName = vmName
 	d.AddVirtualMachineHardDrive_VhdFile = vhdFile
@@ -404,12 +407,13 @@ func (d *DriverMock) AddVirtualMachineHardDrive(vmName string, vhdFile string, v
 	return d.AddVirtualMachineHardDrive_Err
 }
 
-func (d *DriverMock) CreateVirtualMachine(vmName string, path string, harddrivePath string, vhdPath string, ram int64, diskSize int64, diskBlockSize int64, switchName string, generation uint, diffDisks bool, fixedVHD bool) error {
+func (d *DriverMock) CreateVirtualMachine(vmName string, path string, harddrivePath string,
+	ram int64, diskSize int64, diskBlockSize int64, switchName string, generation uint,
+	diffDisks bool, fixedVHD bool) error {
 	d.CreateVirtualMachine_Called = true
 	d.CreateVirtualMachine_VmName = vmName
 	d.CreateVirtualMachine_Path = path
 	d.CreateVirtualMachine_HarddrivePath = harddrivePath
-	d.CreateVirtualMachine_VhdPath = vhdPath
 	d.CreateVirtualMachine_Ram = ram
 	d.CreateVirtualMachine_DiskSize = diskSize
 	d.CreateVirtualMachine_DiskBlockSize = diskBlockSize
@@ -419,9 +423,11 @@ func (d *DriverMock) CreateVirtualMachine(vmName string, path string, harddriveP
 	return d.CreateVirtualMachine_Err
 }
 
-func (d *DriverMock) CloneVirtualMachine(cloneFromVmxcPath string, cloneFromVmName string, cloneFromSnapshotName string, cloneAllSnapshots bool, vmName string, path string, harddrivePath string, ram int64, switchName string) error {
+func (d *DriverMock) CloneVirtualMachine(cloneFromVmcxPath string, cloneFromVmName string,
+	cloneFromSnapshotName string, cloneAllSnapshots bool, vmName string, path string,
+	harddrivePath string, ram int64, switchName string) error {
 	d.CloneVirtualMachine_Called = true
-	d.CloneVirtualMachine_CloneFromVmxcPath = cloneFromVmxcPath
+	d.CloneVirtualMachine_CloneFromVmcxPath = cloneFromVmcxPath
 	d.CloneVirtualMachine_CloneFromVmName = cloneFromVmName
 	d.CloneVirtualMachine_CloneFromSnapshotName = cloneFromSnapshotName
 	d.CloneVirtualMachine_CloneAllSnapshots = cloneAllSnapshots
@@ -489,20 +495,25 @@ func (d *DriverMock) ExportVirtualMachine(vmName string, path string) error {
 	return d.ExportVirtualMachine_Err
 }
 
-func (d *DriverMock) CompactDisks(expPath string, vhdDir string) error {
-	d.CompactDisks_Called = true
-	d.CompactDisks_ExpPath = expPath
-	d.CompactDisks_VhdDir = vhdDir
-	return d.CompactDisks_Err
+func (d *DriverMock) PreserveLegacyExportBehaviour(srcPath string, dstPath string) error {
+	d.PreserveLegacyExportBehaviour_Called = true
+	d.PreserveLegacyExportBehaviour_SrcPath = srcPath
+	d.PreserveLegacyExportBehaviour_DstPath = dstPath
+	return d.PreserveLegacyExportBehaviour_Err
 }
 
-func (d *DriverMock) CopyExportedVirtualMachine(expPath string, outputPath string, vhdDir string, vmDir string) error {
-	d.CopyExportedVirtualMachine_Called = true
-	d.CopyExportedVirtualMachine_ExpPath = expPath
-	d.CopyExportedVirtualMachine_OutputPath = outputPath
-	d.CopyExportedVirtualMachine_VhdDir = vhdDir
-	d.CopyExportedVirtualMachine_VmDir = vmDir
-	return d.CopyExportedVirtualMachine_Err
+func (d *DriverMock) MoveCreatedVHDsToOutputDir(srcPath string, dstPath string) error {
+	d.MoveCreatedVHDsToOutputDir_Called = true
+	d.MoveCreatedVHDsToOutputDir_SrcPath = srcPath
+	d.MoveCreatedVHDsToOutputDir_DstPath = dstPath
+	return d.MoveCreatedVHDsToOutputDir_Err
+}
+
+func (d *DriverMock) CompactDisks(path string) (result string, err error) {
+	d.CompactDisks_Called = true
+	d.CompactDisks_Path = path
+	d.CompactDisks_Result = "Mock compact result msg: mockdisk.vhdx. Disk size reduced by 20%"
+	return d.CompactDisks_Result, d.CompactDisks_Err
 }
 
 func (d *DriverMock) RestartVirtualMachine(vmName string) error {
@@ -519,7 +530,8 @@ func (d *DriverMock) CreateDvdDrive(vmName string, isoPath string, generation ui
 	return d.CreateDvdDrive_ControllerNumber, d.CreateDvdDrive_ControllerLocation, d.CreateDvdDrive_Err
 }
 
-func (d *DriverMock) MountDvdDrive(vmName string, path string, controllerNumber uint, controllerLocation uint) error {
+func (d *DriverMock) MountDvdDrive(vmName string, path string, controllerNumber uint,
+	controllerLocation uint) error {
 	d.MountDvdDrive_Called = true
 	d.MountDvdDrive_VmName = vmName
 	d.MountDvdDrive_Path = path
@@ -528,7 +540,8 @@ func (d *DriverMock) MountDvdDrive(vmName string, path string, controllerNumber
 	return d.MountDvdDrive_Err
 }
 
-func (d *DriverMock) SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation uint, generation uint) error {
+func (d *DriverMock) SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation uint,
+	generation uint) error {
 	d.SetBootDvdDrive_Called = true
 	d.SetBootDvdDrive_VmName = vmName
 	d.SetBootDvdDrive_ControllerNumber = controllerNumber

builder/hyperv/common/driver_ps_4.go

@@ -175,16 +175,24 @@ func (d *HypervPS4Driver) CreateVirtualSwitch(switchName string, switchType stri
 	return hyperv.CreateVirtualSwitch(switchName, switchType)
 }
 
-func (d *HypervPS4Driver) AddVirtualMachineHardDrive(vmName string, vhdFile string, vhdName string, vhdSizeBytes int64, diskBlockSize int64, controllerType string) error {
-	return hyperv.AddVirtualMachineHardDiskDrive(vmName, vhdFile, vhdName, vhdSizeBytes, diskBlockSize, controllerType)
+func (d *HypervPS4Driver) AddVirtualMachineHardDrive(vmName string, vhdFile string, vhdName string,
+	vhdSizeBytes int64, diskBlockSize int64, controllerType string) error {
+	return hyperv.AddVirtualMachineHardDiskDrive(vmName, vhdFile, vhdName, vhdSizeBytes,
+		diskBlockSize, controllerType)
 }
 
-func (d *HypervPS4Driver) CreateVirtualMachine(vmName string, path string, harddrivePath string, vhdPath string, ram int64, diskSize int64, diskBlockSize int64, switchName string, generation uint, diffDisks bool, fixedVHD bool) error {
-	return hyperv.CreateVirtualMachine(vmName, path, harddrivePath, vhdPath, ram, diskSize, diskBlockSize, switchName, generation, diffDisks, fixedVHD)
+func (d *HypervPS4Driver) CreateVirtualMachine(vmName string, path string, harddrivePath string, ram int64,
+	diskSize int64, diskBlockSize int64, switchName string, generation uint, diffDisks bool,
+	fixedVHD bool) error {
+	return hyperv.CreateVirtualMachine(vmName, path, harddrivePath, ram, diskSize, diskBlockSize, switchName,
+		generation, diffDisks, fixedVHD)
 }
 
-func (d *HypervPS4Driver) CloneVirtualMachine(cloneFromVmxcPath string, cloneFromVmName string, cloneFromSnapshotName string, cloneAllSnapshots bool, vmName string, path string, harddrivePath string, ram int64, switchName string) error {
-	return hyperv.CloneVirtualMachine(cloneFromVmxcPath, cloneFromVmName, cloneFromSnapshotName, cloneAllSnapshots, vmName, path, harddrivePath, ram, switchName)
+func (d *HypervPS4Driver) CloneVirtualMachine(cloneFromVmcxPath string, cloneFromVmName string,
+	cloneFromSnapshotName string, cloneAllSnapshots bool, vmName string, path string, harddrivePath string,
+	ram int64, switchName string) error {
+	return hyperv.CloneVirtualMachine(cloneFromVmcxPath, cloneFromVmName, cloneFromSnapshotName,
+		cloneAllSnapshots, vmName, path, harddrivePath, ram, switchName)
 }
 
 func (d *HypervPS4Driver) DeleteVirtualMachine(vmName string) error {
@@ -211,7 +219,8 @@ func (d *HypervPS4Driver) SetVirtualMachineVirtualizationExtensions(vmName strin
 	return hyperv.SetVirtualMachineVirtualizationExtensions(vmName, enable)
 }
 
-func (d *HypervPS4Driver) EnableVirtualMachineIntegrationService(vmName string, integrationServiceName string) error {
+func (d *HypervPS4Driver) EnableVirtualMachineIntegrationService(vmName string,
+	integrationServiceName string) error {
 	return hyperv.EnableVirtualMachineIntegrationService(vmName, integrationServiceName)
 }
 
@@ -219,12 +228,16 @@ func (d *HypervPS4Driver) ExportVirtualMachine(vmName string, path string) error
 	return hyperv.ExportVirtualMachine(vmName, path)
 }
 
-func (d *HypervPS4Driver) CompactDisks(expPath string, vhdDir string) error {
-	return hyperv.CompactDisks(expPath, vhdDir)
+func (d *HypervPS4Driver) PreserveLegacyExportBehaviour(srcPath string, dstPath string) error {
+	return hyperv.PreserveLegacyExportBehaviour(srcPath, dstPath)
 }
 
-func (d *HypervPS4Driver) CopyExportedVirtualMachine(expPath string, outputPath string, vhdDir string, vmDir string) error {
-	return hyperv.CopyExportedVirtualMachine(expPath, outputPath, vhdDir, vmDir)
+func (d *HypervPS4Driver) MoveCreatedVHDsToOutputDir(srcPath string, dstPath string) error {
+	return hyperv.MoveCreatedVHDsToOutputDir(srcPath, dstPath)
+}
+
+func (d *HypervPS4Driver) CompactDisks(path string) (result string, err error) {
+	return hyperv.CompactDisks(path)
 }
 
 func (d *HypervPS4Driver) RestartVirtualMachine(vmName string) error {
@@ -235,11 +248,13 @@ func (d *HypervPS4Driver) CreateDvdDrive(vmName string, isoPath string, generati
 	return hyperv.CreateDvdDrive(vmName, isoPath, generation)
 }
 
-func (d *HypervPS4Driver) MountDvdDrive(vmName string, path string, controllerNumber uint, controllerLocation uint) error {
+func (d *HypervPS4Driver) MountDvdDrive(vmName string, path string, controllerNumber uint,
+	controllerLocation uint) error {
 	return hyperv.MountDvdDrive(vmName, path, controllerNumber, controllerLocation)
 }
 
-func (d *HypervPS4Driver) SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation uint, generation uint) error {
+func (d *HypervPS4Driver) SetBootDvdDrive(vmName string, controllerNumber uint, controllerLocation uint,
+	generation uint) error {
 	return hyperv.SetBootDvdDrive(vmName, controllerNumber, controllerLocation, generation)
 }
 

builder/hyperv/common/ssh.go

@@ -1,10 +1,7 @@
 package common
 
 import (
-	commonssh "github.com/hashicorp/packer/common/ssh"
-	"github.com/hashicorp/packer/communicator/ssh"
 	"github.com/hashicorp/packer/helper/multistep"
-	gossh "golang.org/x/crypto/ssh"
 )
 
 func CommHost(state multistep.StateBag) (string, error) {
@@ -23,28 +20,3 @@ func CommHost(state multistep.StateBag) (string, error) {
 
 	return ip, 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.Comm.SSHPassword),
-			gossh.KeyboardInteractive(
-				ssh.PasswordKeyboardInteractive(config.Comm.SSHPassword)),
-		}
-
-		if config.Comm.SSHPrivateKey != "" {
-			signer, err := commonssh.FileSigner(config.Comm.SSHPrivateKey)
-			if err != nil {
-				return nil, err
-			}
-
-			auth = append(auth, gossh.PublicKeys(signer))
-		}
-
-		return &gossh.ClientConfig{
-			User:            config.Comm.SSHUsername,
-			Auth:            auth,
-			HostKeyCallback: gossh.InsecureIgnoreHostKey(),
-		}, nil
-	}
-}

builder/hyperv/common/step_clone_vm.go

@@ -16,7 +16,7 @@ import (
 // Produces:
 //   VMName string - The name of the VM
 type StepCloneVM struct {
-	CloneFromVMXCPath              string
+	CloneFromVMCXPath              string
 	CloneFromVMName                string
 	CloneFromSnapshotName          string
 	CloneAllSnapshots              bool
@@ -37,7 +37,7 @@ func (s *StepCloneVM) Run(_ context.Context, state multistep.StateBag) multistep
 	ui := state.Get("ui").(packer.Ui)
 	ui.Say("Cloning virtual machine...")
 
-	path := state.Get("packerTempDir").(string)
+	path := state.Get("build_dir").(string)
 
 	// Determine if we even have an existing virtual harddrive to attach
 	harddrivePath := ""
@@ -55,7 +55,8 @@ func (s *StepCloneVM) Run(_ context.Context, state multistep.StateBag) multistep
 	// convert the MB to bytes
 	ramSize := int64(s.RamSize * 1024 * 1024)
 
-	err := driver.CloneVirtualMachine(s.CloneFromVMXCPath, s.CloneFromVMName, s.CloneFromSnapshotName, s.CloneAllSnapshots, s.VMName, path, harddrivePath, ramSize, s.SwitchName)
+	err := driver.CloneVirtualMachine(s.CloneFromVMCXPath, s.CloneFromVMName, s.CloneFromSnapshotName,
+		s.CloneAllSnapshots, s.VMName, path, harddrivePath, ramSize, s.SwitchName)
 	if err != nil {
 		err := fmt.Errorf("Error cloning virtual machine: %s", err)
 		state.Put("error", err)

builder/hyperv/common/step_collate_artifacts.go

@@ -0,0 +1,69 @@
+package common
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/hashicorp/packer/helper/multistep"
+	"github.com/hashicorp/packer/packer"
+)
+
+type StepCollateArtifacts struct {
+	OutputDir  string
+	SkipExport bool
+}
+
+// Runs the step required to collate all build artifacts under the
+// specified output directory
+func (s *StepCollateArtifacts) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
+	driver := state.Get("driver").(Driver)
+	ui := state.Get("ui").(packer.Ui)
+
+	ui.Say("Collating build artifacts...")
+
+	if s.SkipExport {
+		// Get the path to the main build directory from the statebag
+		var buildDir string
+		if v, ok := state.GetOk("build_dir"); ok {
+			buildDir = v.(string)
+		}
+		// If the user has chosen to skip a full export of the VM the only
+		// artifacts that they are interested in will be the VHDs. The
+		// called function searches for all disks under the given source
+		// directory and moves them to a 'Virtual Hard Disks' folder under
+		// the destination directory
+		err := driver.MoveCreatedVHDsToOutputDir(buildDir, s.OutputDir)
+		if err != nil {
+			err = fmt.Errorf("Error moving VHDs from build dir to output dir: %s", err)
+			state.Put("error", err)
+			ui.Error(err.Error())
+			return multistep.ActionHalt
+		}
+	} else {
+		// Get the full path to the export directory from the statebag
+		var exportPath string
+		if v, ok := state.GetOk("export_path"); ok {
+			exportPath = v.(string)
+		}
+		// The export process exports the VM into a folder named 'vm name'
+		// under the output directory. However, to maintain backwards
+		// compatibility, we now need to shuffle around the exported folders
+		// so the 'Snapshots', 'Virtual Hard Disks' and 'Virtual Machines'
+		// directories appear *directly* under <output directory>.
+		// The empty '<output directory>/<vm name>' directory is removed
+		// when complete.
+		// The 'Snapshots' folder will not be moved into the output
+		// directory if it is empty.
+		err := driver.PreserveLegacyExportBehaviour(exportPath, s.OutputDir)
+		if err != nil {
+			// No need to halt here; Just warn the user instead
+			err = fmt.Errorf("WARNING: Error restoring legacy export dir structure: %s", err)
+			ui.Error(err.Error())
+		}
+	}
+
+	return multistep.ActionContinue
+}
+
+// Cleanup does nothing
+func (s *StepCollateArtifacts) Cleanup(state multistep.StateBag) {}

builder/hyperv/common/step_collate_artifacts_test.go

@@ -0,0 +1,92 @@
+package common
+
+import (
+	"context"
+	"path/filepath"
+	"testing"
+
+	"github.com/hashicorp/packer/helper/multistep"
+)
+
+func TestStepCollateArtifacts_impl(t *testing.T) {
+	var _ multistep.Step = new(StepCollateArtifacts)
+}
+
+func TestStepCollateArtifacts_exportedArtifacts(t *testing.T) {
+	state := testState(t)
+	step := new(StepCollateArtifacts)
+
+	step.OutputDir = "foopath"
+	vmName := "foo"
+
+	// Uses export path from the state bag
+	exportPath := filepath.Join(step.OutputDir, vmName)
+	state.Put("export_path", exportPath)
+
+	driver := state.Get("driver").(*DriverMock)
+
+	// Test the run
+	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
+		t.Fatalf("Bad action: %v", action)
+	}
+	if _, ok := state.GetOk("error"); ok {
+		t.Fatal("Should NOT have error")
+	}
+
+	// Test the driver
+	if !driver.PreserveLegacyExportBehaviour_Called {
+		t.Fatal("Should have called PreserveLegacyExportBehaviour")
+	}
+	if driver.PreserveLegacyExportBehaviour_SrcPath != exportPath {
+		t.Fatalf("Should call with correct srcPath. Got: %s Wanted: %s",
+			driver.PreserveLegacyExportBehaviour_SrcPath, exportPath)
+	}
+	if driver.PreserveLegacyExportBehaviour_DstPath != step.OutputDir {
+		t.Fatalf("Should call with correct dstPath. Got: %s Wanted: %s",
+			driver.PreserveLegacyExportBehaviour_DstPath, step.OutputDir)
+	}
+
+	// Should only be called when skip_export is true
+	if driver.MoveCreatedVHDsToOutputDir_Called {
+		t.Fatal("Should NOT have called MoveCreatedVHDsToOutputDir")
+	}
+}
+
+func TestStepCollateArtifacts_skipExportArtifacts(t *testing.T) {
+	state := testState(t)
+	step := new(StepCollateArtifacts)
+
+	// Needs the path to the main output directory and build directory
+	step.OutputDir = "foopath"
+	buildDir := "fooBuildPath"
+	state.Put("build_dir", buildDir)
+	// Export has been skipped
+	step.SkipExport = true
+
+	driver := state.Get("driver").(*DriverMock)
+
+	// Test the run
+	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
+		t.Fatalf("Bad action: %v", action)
+	}
+	if _, ok := state.GetOk("error"); ok {
+		t.Fatal("Should NOT have error")
+	}
+
+	// Test the driver
+	if !driver.MoveCreatedVHDsToOutputDir_Called {
+		t.Fatal("Should have called MoveCreatedVHDsToOutputDir")
+	}
+	if driver.MoveCreatedVHDsToOutputDir_SrcPath != buildDir {
+		t.Fatalf("Should call with correct srcPath. Got: %s Wanted: %s",
+			driver.MoveCreatedVHDsToOutputDir_SrcPath, buildDir)
+	}
+	if driver.MoveCreatedVHDsToOutputDir_DstPath != step.OutputDir {
+		t.Fatalf("Should call with correct dstPath. Got: %s Wanted: %s",
+			driver.MoveCreatedVHDsToOutputDir_DstPath, step.OutputDir)
+	}
+
+	if driver.PreserveLegacyExportBehaviour_Called {
+		t.Fatal("Should NOT have called PreserveLegacyExportBehaviour")
+	}
+}

builder/hyperv/common/step_compact_disk.go

@@ -0,0 +1,50 @@
+package common
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/hashicorp/packer/helper/multistep"
+	"github.com/hashicorp/packer/packer"
+)
+
+type StepCompactDisk struct {
+	SkipCompaction bool
+}
+
+// Run runs a compaction/optimisation process on attached VHD/VHDX disks
+func (s *StepCompactDisk) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
+	driver := state.Get("driver").(Driver)
+	ui := state.Get("ui").(packer.Ui)
+
+	if s.SkipCompaction {
+		ui.Say("Skipping disk compaction...")
+		return multistep.ActionContinue
+	}
+
+	// Get the dir used to store the VMs files during the build process
+	var buildDir string
+	if v, ok := state.GetOk("build_dir"); ok {
+		buildDir = v.(string)
+	}
+
+	ui.Say("Compacting disks...")
+	// CompactDisks searches for all VHD/VHDX files under the supplied
+	// path and runs the compacting process on each of them. If no disks
+	// are found under the supplied path this is treated as a 'soft' error
+	// and a warning message is printed. All other errors halt the build.
+	result, err := driver.CompactDisks(buildDir)
+	if err != nil {
+		err := fmt.Errorf("Error compacting disks: %s", err)
+		state.Put("error", err)
+		ui.Error(err.Error())
+		return multistep.ActionHalt
+	}
+	// Report disk compaction results/warn if no disks were found
+	ui.Message(result)
+
+	return multistep.ActionContinue
+}
+
+// Cleanup does nothing
+func (s *StepCompactDisk) Cleanup(state multistep.StateBag) {}

builder/hyperv/common/step_compact_disk_test.go

@@ -0,0 +1,63 @@
+package common
+
+import (
+	"context"
+	"testing"
+
+	"github.com/hashicorp/packer/helper/multistep"
+)
+
+func TestStepCompactDisk_impl(t *testing.T) {
+	var _ multistep.Step = new(StepCompactDisk)
+}
+
+func TestStepCompactDisk(t *testing.T) {
+	state := testState(t)
+	step := new(StepCompactDisk)
+
+	// Set up the path to the build directory
+	buildDir := "foopath"
+	state.Put("build_dir", buildDir)
+
+	driver := state.Get("driver").(*DriverMock)
+
+	// Test the run
+	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
+		t.Fatalf("Bad action: %v", action)
+	}
+	if _, ok := state.GetOk("error"); ok {
+		t.Fatal("Should NOT have error")
+	}
+
+	// Test the driver
+	if !driver.CompactDisks_Called {
+		t.Fatal("Should have called CompactDisks")
+	}
+	if driver.CompactDisks_Path != buildDir {
+		t.Fatalf("Should call with correct path. Got: %s Wanted: %s", driver.CompactDisks_Path, buildDir)
+	}
+}
+
+func TestStepCompactDisk_skip(t *testing.T) {
+	state := testState(t)
+	step := new(StepCompactDisk)
+	step.SkipCompaction = true
+
+	// Set up the path to the build directory
+	state.Put("build_dir", "foopath")
+
+	driver := state.Get("driver").(*DriverMock)
+
+	// Test the run
+	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
+		t.Fatalf("Bad action: %v", action)
+	}
+	if _, ok := state.GetOk("error"); ok {
+		t.Fatalf("Should NOT have error")
+	}
+
+	// Test the driver
+	if driver.CompactDisks_Called {
+		t.Fatal("Should NOT have called CompactDisks")
+	}
+}

builder/hyperv/common/step_create_build_dir.go

@@ -0,0 +1,66 @@
+package common
+
+import (
+	"context"
+	"fmt"
+	"io/ioutil"
+	"log"
+	"os"
+
+	"github.com/hashicorp/packer/helper/multistep"
+	"github.com/hashicorp/packer/packer"
+)
+
+type StepCreateBuildDir struct {
+	// User supplied directory under which we create the main build
+	// directory. The build directory is used to house the VM files and
+	// folders during the build. If unspecified the default temp directory
+	// for the OS is used
+	TempPath string
+	// The full path to the build directory. This is the concatenation of
+	// TempPath plus a directory uniquely named for the build
+	buildDir string
+}
+
+// Creates the main directory used to house the VMs files and folders
+// during the build
+func (s *StepCreateBuildDir) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
+	ui := state.Get("ui").(packer.Ui)
+
+	ui.Say("Creating build directory...")
+
+	if s.TempPath == "" {
+		s.TempPath = os.TempDir()
+	}
+
+	var err error
+	s.buildDir, err = ioutil.TempDir(s.TempPath, "packerhv")
+	if err != nil {
+		err = fmt.Errorf("Error creating build directory: %s", err)
+		state.Put("error", err)
+		ui.Error(err.Error())
+		return multistep.ActionHalt
+	}
+
+	log.Printf("Created build directory: %s", s.buildDir)
+
+	// Record the build directory location for later steps
+	state.Put("build_dir", s.buildDir)
+
+	return multistep.ActionContinue
+}
+
+// Cleanup removes the build directory
+func (s *StepCreateBuildDir) Cleanup(state multistep.StateBag) {
+	if s.buildDir == "" {
+		return
+	}
+
+	ui := state.Get("ui").(packer.Ui)
+	ui.Say("Deleting build directory...")
+
+	err := os.RemoveAll(s.buildDir)
+	if err != nil {
+		ui.Error(fmt.Sprintf("Error deleting build directory: %s", err))
+	}
+}

builder/hyperv/common/step_create_build_dir_test.go

@@ -0,0 +1,113 @@
+package common
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"regexp"
+	"testing"
+
+	"github.com/hashicorp/packer/helper/multistep"
+)
+
+func TestStepCreateBuildDir_imp(t *testing.T) {
+	var _ multistep.Step = new(StepCreateBuildDir)
+}
+
+func TestStepCreateBuildDir_Defaults(t *testing.T) {
+	state := testState(t)
+	step := new(StepCreateBuildDir)
+
+	// Default is for the user not to supply value for TempPath. When
+	// nothing is set the step should use the OS temp directory as the root
+	// for the build directory
+	step.TempPath = ""
+
+	// Test the run
+	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
+		t.Fatalf("Bad action: %v", action)
+	}
+	if _, ok := state.GetOk("error"); ok {
+		t.Fatal("Should NOT have error")
+	}
+
+	if v, ok := state.GetOk("build_dir"); !ok {
+		t.Fatal("Should store path to build directory in statebag as 'build_dir'")
+	} else {
+		// On windows convert everything to forward slash separated paths
+		// This prevents the regexp interpreting backslashes as escape sequences
+		stateBuildDir := filepath.ToSlash(v.(string))
+		expectedBuildDirRe := regexp.MustCompile(
+			filepath.ToSlash(filepath.Join(os.TempDir(), "packerhv") + `[[:digit:]]{9}$`))
+		match := expectedBuildDirRe.MatchString(stateBuildDir)
+		if !match {
+			t.Fatalf("Got path that doesn't match expected format in 'build_dir': %s", stateBuildDir)
+		}
+	}
+
+	// Test Cleanup
+	step.Cleanup(state)
+	if _, err := os.Stat(step.buildDir); err == nil {
+		t.Fatalf("Build directory should NOT exist after Cleanup: %s", step.buildDir)
+	}
+}
+
+func TestStepCreateBuildDir_UserDefinedTempPath(t *testing.T) {
+	state := testState(t)
+	step := new(StepCreateBuildDir)
+
+	// Create a directory we'll use as the user supplied temp_path
+	step.TempPath = genTestDirPath("userTempDir")
+	err := os.Mkdir(step.TempPath, 0755) // The directory must exist
+	if err != nil {
+		t.Fatal("Error creating test directory")
+	}
+	defer os.RemoveAll(step.TempPath)
+
+	// Test the run
+	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
+		t.Fatalf("Bad action: %v", action)
+	}
+	if _, ok := state.GetOk("error"); ok {
+		t.Fatal("Should NOT have error")
+	}
+
+	if v, ok := state.GetOk("build_dir"); !ok {
+		t.Fatal("Should store path to build directory in statebag as 'build_dir'")
+	} else {
+		// On windows convert everything to forward slash separated paths
+		// This prevents the regexp interpreting backslashes as escape sequences
+		stateBuildDir := filepath.ToSlash(v.(string))
+		expectedBuildDirRe := regexp.MustCompile(
+			filepath.ToSlash(filepath.Join(step.TempPath, "packerhv") + `[[:digit:]]{9}$`))
+		match := expectedBuildDirRe.MatchString(stateBuildDir)
+		if !match {
+			t.Fatalf("Got path that doesn't match expected format in 'build_dir': %s", stateBuildDir)
+		}
+	}
+
+	// Test Cleanup
+	step.Cleanup(state)
+	if _, err := os.Stat(step.buildDir); err == nil {
+		t.Fatalf("Build directory should NOT exist after Cleanup: %s", step.buildDir)
+	}
+	if _, err := os.Stat(step.TempPath); err != nil {
+		t.Fatal("User supplied root for build directory should NOT be deleted by Cleanup")
+	}
+}
+
+func TestStepCreateBuildDir_BadTempPath(t *testing.T) {
+	state := testState(t)
+	step := new(StepCreateBuildDir)
+
+	// Bad
+	step.TempPath = genTestDirPath("iDontExist")
+
+	// Test the run
+	if action := step.Run(context.Background(), state); action != multistep.ActionHalt {
+		t.Fatalf("Bad action: %v", action)
+	}
+	if _, ok := state.GetOk("error"); !ok {
+		t.Fatal("Should have error due to bad path")
+	}
+}

builder/hyperv/common/step_create_external_switch.go

@@ -9,7 +9,7 @@ import (
 	"github.com/hashicorp/packer/packer"
 )
 
-// This step creates switch for VM.
+// This step creates an external switch for the VM.
 //
 // Produces:
 //   SwitchName string - The name of the Switch
@@ -18,6 +18,9 @@ type StepCreateExternalSwitch struct {
 	oldSwitchName string
 }
 
+// Run runs the step required to create an external switch. Depending on
+// the connectivity of the host machine, the external switch will allow the
+// build VM to connect to the outside world.
 func (s *StepCreateExternalSwitch) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
 	driver := state.Get("driver").(Driver)
 	ui := state.Get("ui").(packer.Ui)
@@ -30,10 +33,12 @@ func (s *StepCreateExternalSwitch) Run(_ context.Context, state multistep.StateB
 
 	packerExternalSwitchName := "paes_" + uuid.TimeOrderedUUID()
 
+	// CreateExternalVirtualSwitch checks for an existing external switch,
+	// creating one if required, and connects the VM to it
 	err = driver.CreateExternalVirtualSwitch(vmName, packerExternalSwitchName)
 	if err != nil {
-		err := fmt.Errorf("Error creating switch: %s", err)
-		state.Put(errorMsg, err)
+		err := fmt.Errorf(errorMsg, err)
+		state.Put("error", err)
 		ui.Error(err.Error())
 		s.SwitchName = ""
 		return multistep.ActionHalt

builder/hyperv/common/step_create_tempdir.go

@@ -1,85 +0,0 @@
-package common
-
-import (
-	"context"
-	"fmt"
-	"io/ioutil"
-	"os"
-
-	"github.com/hashicorp/packer/helper/multistep"
-	"github.com/hashicorp/packer/packer"
-)
-
-type StepCreateTempDir struct {
-	// The user-supplied root directories into which we create subdirectories.
-	TempPath    string
-	VhdTempPath string
-	// The subdirectories with the randomly generated name.
-	dirPath    string
-	vhdDirPath string
-}
-
-func (s *StepCreateTempDir) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
-	ui := state.Get("ui").(packer.Ui)
-
-	ui.Say("Creating temporary directory...")
-
-	if s.TempPath == "" {
-		s.TempPath = os.TempDir()
-	}
-
-	packerTempDir, err := ioutil.TempDir(s.TempPath, "packerhv")
-	if err != nil {
-		err := fmt.Errorf("Error creating temporary directory: %s", err)
-		state.Put("error", err)
-		ui.Error(err.Error())
-		return multistep.ActionHalt
-	}
-
-	s.dirPath = packerTempDir
-	state.Put("packerTempDir", packerTempDir)
-
-	if s.VhdTempPath == "" {
-		// Fall back to regular temp dir if no separate VHD temp dir set.
-		state.Put("packerVhdTempDir", packerTempDir)
-	} else {
-		packerVhdTempDir, err := ioutil.TempDir(s.VhdTempPath, "packerhv-vhd")
-		if err != nil {
-			err := fmt.Errorf("Error creating temporary VHD directory: %s", err)
-			state.Put("error", err)
-			ui.Error(err.Error())
-			return multistep.ActionHalt
-		}
-
-		s.vhdDirPath = packerVhdTempDir
-		state.Put("packerVhdTempDir", packerVhdTempDir)
-	}
-
-	//	ui.Say("packerTempDir = '" + packerTempDir + "'")
-
-	return multistep.ActionContinue
-}
-
-func (s *StepCreateTempDir) Cleanup(state multistep.StateBag) {
-	ui := state.Get("ui").(packer.Ui)
-
-	if s.dirPath != "" {
-		ui.Say("Deleting temporary directory...")
-
-		err := os.RemoveAll(s.dirPath)
-
-		if err != nil {
-			ui.Error(fmt.Sprintf("Error deleting temporary directory: %s", err))
-		}
-	}
-
-	if s.vhdDirPath != "" && s.dirPath != s.vhdDirPath {
-		ui.Say("Deleting temporary VHD directory...")
-
-		err := os.RemoveAll(s.vhdDirPath)
-
-		if err != nil {
-			ui.Error(fmt.Sprintf("Error deleting temporary VHD directory: %s", err))
-		}
-	}
-}

builder/hyperv/common/step_create_vm.go

@@ -32,8 +32,6 @@ type StepCreateVM struct {
 	AdditionalDiskSize             []uint
 	DifferencingDisk               bool
 	MacAddress                     string
-	SkipExport                     bool
-	OutputDir                      string
 	FixedVHD                       bool
 }
 
@@ -42,7 +40,10 @@ func (s *StepCreateVM) Run(_ context.Context, state multistep.StateBag) multiste
 	ui := state.Get("ui").(packer.Ui)
 	ui.Say("Creating virtual machine...")
 
-	path := state.Get("packerTempDir").(string)
+	var path string
+	if v, ok := state.GetOk("build_dir"); ok {
+		path = v.(string)
+	}
 
 	// Determine if we even have an existing virtual harddrive to attach
 	harddrivePath := ""
@@ -57,19 +58,13 @@ func (s *StepCreateVM) Run(_ context.Context, state multistep.StateBag) multiste
 		log.Println("No existing virtual harddrive, not attaching.")
 	}
 
-	vhdPath := state.Get("packerVhdTempDir").(string)
-
-	// inline vhd path if export is skipped
-	if s.SkipExport {
-		vhdPath = filepath.Join(s.OutputDir, "Virtual Hard Disks")
-	}
-
 	// convert the MB to bytes
 	ramSize := int64(s.RamSize * 1024 * 1024)
 	diskSize := int64(s.DiskSize * 1024 * 1024)
 	diskBlockSize := int64(s.DiskBlockSize * 1024 * 1024)
 
-	err := driver.CreateVirtualMachine(s.VMName, path, harddrivePath, vhdPath, ramSize, diskSize, diskBlockSize, s.SwitchName, s.Generation, s.DifferencingDisk, s.FixedVHD)
+	err := driver.CreateVirtualMachine(s.VMName, path, harddrivePath, ramSize, diskSize, diskBlockSize,
+		s.SwitchName, s.Generation, s.DifferencingDisk, s.FixedVHD)
 	if err != nil {
 		err := fmt.Errorf("Error creating virtual machine: %s", err)
 		state.Put("error", err)
@@ -128,7 +123,7 @@ func (s *StepCreateVM) Run(_ context.Context, state multistep.StateBag) multiste
 		for index, size := range s.AdditionalDiskSize {
 			diskSize := int64(size * 1024 * 1024)
 			diskFile := fmt.Sprintf("%s-%d.vhdx", s.VMName, index)
-			err = driver.AddVirtualMachineHardDrive(s.VMName, vhdPath, diskFile, diskSize, diskBlockSize, "SCSI")
+			err = driver.AddVirtualMachineHardDrive(s.VMName, path, diskFile, diskSize, diskBlockSize, "SCSI")
 			if err != nil {
 				err := fmt.Errorf("Error creating and attaching additional disk drive: %s", err)
 				state.Put("error", err)

builder/hyperv/common/step_export_vm.go

@@ -3,84 +3,49 @@ package common
 import (
 	"context"
 	"fmt"
-	"io/ioutil"
 	"path/filepath"
 
 	"github.com/hashicorp/packer/helper/multistep"
 	"github.com/hashicorp/packer/packer"
 )
 
-const (
-	vhdDir string = "Virtual Hard Disks"
-	vmDir  string = "Virtual Machines"
-)
-
 type StepExportVm struct {
-	OutputDir      string
-	SkipCompaction bool
-	SkipExport     bool
+	OutputDir  string
+	SkipExport bool
 }
 
 func (s *StepExportVm) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
 	driver := state.Get("driver").(Driver)
 	ui := state.Get("ui").(packer.Ui)
-	var err error
-	var errorMsg string
 
-	vmName := state.Get("vmName").(string)
-	tmpPath := state.Get("packerTempDir").(string)
-	outputPath := s.OutputDir
-	expPath := s.OutputDir
+	if s.SkipExport {
+		ui.Say("Skipping export of virtual machine...")
+		return multistep.ActionContinue
+	}
 
-	// create temp path to export vm
-	errorMsg = "Error creating temp export path: %s"
-	vmExportPath, err := ioutil.TempDir(tmpPath, "export")
+	ui.Say("Exporting virtual machine...")
+
+	// The VM name is needed for the export command
+	var vmName string
+	if v, ok := state.GetOk("vmName"); ok {
+		vmName = v.(string)
+	}
+
+	// The export process exports the VM to a folder named 'vmName' under
+	// the output directory. This contains the usual 'Snapshots', 'Virtual
+	// Hard Disks' and 'Virtual Machines' directories.
+	err := driver.ExportVirtualMachine(vmName, s.OutputDir)
 	if err != nil {
-		err := fmt.Errorf(errorMsg, err)
+		err = fmt.Errorf("Error exporting vm: %s", err)
 		state.Put("error", err)
 		ui.Error(err.Error())
 		return multistep.ActionHalt
 	}
-	if !s.SkipExport {
-		ui.Say("Exporting vm...")
 
-		err = driver.ExportVirtualMachine(vmName, vmExportPath)
-		if err != nil {
-			errorMsg = "Error exporting vm: %s"
-			err := fmt.Errorf(errorMsg, err)
-			state.Put("error", err)
-			ui.Error(err.Error())
-			return multistep.ActionHalt
-		}
-		// copy to output dir
-		expPath = filepath.Join(vmExportPath, vmName)
-	}
+	// Store the path to the export directory for later steps
+	exportPath := filepath.Join(s.OutputDir, vmName)
+	state.Put("export_path", exportPath)
 
-	if s.SkipCompaction {
-		ui.Say("Skipping disk compaction...")
-	} else {
-		ui.Say("Compacting disks...")
-		err = driver.CompactDisks(expPath, vhdDir)
-		if err != nil {
-			errorMsg = "Error compacting disks: %s"
-			err := fmt.Errorf(errorMsg, err)
-			state.Put("error", err)
-			ui.Error(err.Error())
-			return multistep.ActionHalt
-		}
-	}
-
-	if !s.SkipExport {
-		ui.Say("Copying to output dir...")
-		err = driver.CopyExportedVirtualMachine(expPath, outputPath, vhdDir, vmDir)
-		if err != nil {
-			errorMsg = "Error exporting vm: %s"
-			err := fmt.Errorf(errorMsg, err)
-			state.Put("error", err)
-			ui.Error(err.Error())
-			return multistep.ActionHalt
-		}
-	}
 	return multistep.ActionContinue
 }
 

builder/hyperv/common/step_export_vm_test.go

@@ -0,0 +1,87 @@
+package common
+
+import (
+	"context"
+	"path/filepath"
+	"testing"
+
+	"github.com/hashicorp/packer/helper/multistep"
+)
+
+func TestStepExportVm_impl(t *testing.T) {
+	var _ multistep.Step = new(StepExportVm)
+}
+
+func TestStepExportVm(t *testing.T) {
+	state := testState(t)
+	step := new(StepExportVm)
+
+	// ExportVirtualMachine needs the VM name and a path to export to
+	vmName := "foo"
+	state.Put("vmName", vmName)
+	outputDir := "foopath"
+	step.OutputDir = outputDir
+
+	driver := state.Get("driver").(*DriverMock)
+
+	// Test the run
+	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
+		t.Fatalf("Bad action: %v", action)
+	}
+	if _, ok := state.GetOk("error"); ok {
+		t.Fatal("Should NOT have error")
+	}
+
+	// Test the driver
+	if !driver.ExportVirtualMachine_Called {
+		t.Fatal("Should have called ExportVirtualMachine")
+	}
+	if driver.ExportVirtualMachine_Path != outputDir {
+		t.Fatalf("Should call with correct path. Got: %s Wanted: %s",
+			driver.ExportVirtualMachine_Path, outputDir)
+	}
+	if driver.ExportVirtualMachine_VmName != vmName {
+		t.Fatalf("Should call with correct vm name. Got: %s Wanted: %s",
+			driver.ExportVirtualMachine_VmName, vmName)
+	}
+
+	// Test we stored the export path in the statebag and it is correct
+	expectedPath := filepath.Join(outputDir, vmName)
+	if exportPath, ok := state.GetOk("export_path"); !ok {
+		t.Fatal("Should set export_path")
+	} else if exportPath != expectedPath {
+		t.Fatalf("Bad path stored for export_path. Got: %#v Wanted: %#v", exportPath, expectedPath)
+	}
+}
+
+func TestStepExportVm_skip(t *testing.T) {
+	state := testState(t)
+	step := new(StepExportVm)
+	step.SkipExport = true
+
+	// ExportVirtualMachine needs the VM name and a path to export to
+	vmName := "foo"
+	state.Put("vmName", vmName)
+	outputDir := "foopath"
+	step.OutputDir = outputDir
+
+	driver := state.Get("driver").(*DriverMock)
+
+	// Test the run
+	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
+		t.Fatalf("Bad action: %v", action)
+	}
+	if _, ok := state.GetOk("error"); ok {
+		t.Fatalf("Should NOT have error")
+	}
+
+	// Test the driver
+	if driver.ExportVirtualMachine_Called {
+		t.Fatal("Should NOT have called ExportVirtualMachine")
+	}
+
+	// Should not store the export path in the statebag
+	if _, ok := state.GetOk("export_path"); ok {
+		t.Fatal("Should NOT have stored export_path in the statebag")
+	}
+}

builder/hyperv/common/step_mount_guest_additions.go

@@ -56,7 +56,8 @@ func (s *StepMountGuestAdditions) Run(_ context.Context, state multistep.StateBa
 		return multistep.ActionHalt
 	}
 
-	log.Println(fmt.Sprintf("ISO %s mounted on DVD controller %v, location %v", s.GuestAdditionsPath, controllerNumber, controllerLocation))
+	log.Println(fmt.Sprintf("ISO %s mounted on DVD controller %v, location %v", s.GuestAdditionsPath,
+		controllerNumber, controllerLocation))
 
 	return multistep.ActionContinue
 }

builder/hyperv/common/step_mount_secondary_dvd_images.go

@@ -58,7 +58,8 @@ func (s *StepMountSecondaryDvdImages) Run(_ context.Context, state multistep.Sta
 			return multistep.ActionHalt
 		}
 
-		log.Println(fmt.Sprintf("ISO %s mounted on DVD controller %v, location %v", isoPath, controllerNumber, controllerLocation))
+		log.Println(fmt.Sprintf("ISO %s mounted on DVD controller %v, location %v", isoPath, controllerNumber,
+			controllerLocation))
 	}
 
 	return multistep.ActionContinue

builder/hyperv/common/step_output_dir_test.go

@@ -0,0 +1,146 @@
+package common
+
+import (
+	"context"
+	"os"
+	"testing"
+
+	"github.com/hashicorp/packer/helper/multistep"
+)
+
+func TestStepOutputDir_imp(t *testing.T) {
+	var _ multistep.Step = new(StepOutputDir)
+}
+
+func TestStepOutputDir_Default(t *testing.T) {
+	state := testState(t)
+	step := new(StepOutputDir)
+
+	step.Path = genTestDirPath("packerHypervOutput")
+
+	// Test the run
+	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
+		t.Fatalf("Bad action: %v", action)
+	}
+	if _, ok := state.GetOk("error"); ok {
+		t.Fatal("Should NOT have error")
+	}
+
+	// The directory should have been created
+	if _, err := os.Stat(step.Path); err != nil {
+		t.Fatal("Should have created output directory")
+	}
+
+	// Remove the directory created due to test
+	err := os.RemoveAll(step.Path)
+	if err != nil {
+		t.Fatalf("Error encountered removing directory created by test: %s", err)
+	}
+}
+
+func TestStepOutputDir_DirectoryAlreadyExistsNoForce(t *testing.T) {
+	state := testState(t)
+	step := new(StepOutputDir)
+
+	step.Path = genTestDirPath("packerHypervOutput")
+
+	// Create the directory so that we can test
+	err := os.Mkdir(step.Path, 0755)
+	if err != nil {
+		t.Fatal("Test failed to create directory for test of Cancel and Cleanup")
+	}
+	defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong
+
+	step.Force = false // Default
+	// Test the run
+	if action := step.Run(context.Background(), state); action != multistep.ActionHalt {
+		t.Fatalf("Should halt when directory exists and 'Force' is false. Bad action: %v", action)
+	}
+	if _, ok := state.GetOk("error"); !ok {
+		t.Fatal("Should error when directory exists and 'Force' is false")
+	}
+}
+
+func TestStepOutputDir_DirectoryAlreadyExistsForce(t *testing.T) {
+	state := testState(t)
+	step := new(StepOutputDir)
+
+	step.Path = genTestDirPath("packerHypervOutput")
+
+	// Create the directory so that we can test
+	err := os.Mkdir(step.Path, 0755)
+	if err != nil {
+		t.Fatal("Test failed to create directory for test of Cancel and Cleanup")
+	}
+	defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong
+
+	step.Force = true // User specified that existing directory and contents should be discarded
+	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
+		t.Fatalf("Should complete when directory exists and 'Force' is true. Bad action: %v", action)
+	}
+	if _, ok := state.GetOk("error"); ok {
+		t.Fatalf("Should NOT error when directory exists and 'Force' is true: %s", err)
+	}
+}
+
+func TestStepOutputDir_CleanupBuildCancelled(t *testing.T) {
+	state := testState(t)
+	step := new(StepOutputDir)
+
+	step.Path = genTestDirPath("packerHypervOutput")
+
+	// Create the directory so that we can test the cleanup
+	err := os.Mkdir(step.Path, 0755)
+	if err != nil {
+		t.Fatal("Test failed to create directory for test of Cancel and Cleanup")
+	}
+	defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong
+
+	// 'Cancel' the build
+	state.Put(multistep.StateCancelled, true)
+
+	// Ensure the directory isn't removed if the cleanup flag is false
+	step.cleanup = false
+	step.Cleanup(state)
+	if _, err := os.Stat(step.Path); err != nil {
+		t.Fatal("Output dir should NOT be removed if on 'Cancel' if cleanup flag is unset/false")
+	}
+
+	// Ensure the directory is removed if the cleanup flag is true
+	step.cleanup = true
+	step.Cleanup(state)
+	if _, err := os.Stat(step.Path); err == nil {
+		t.Fatalf("Output directory should NOT exist after 'Cancel' and Cleanup: %s", step.Path)
+	}
+}
+
+func TestStepOutputDir_CleanupBuildHalted(t *testing.T) {
+	state := testState(t)
+	step := new(StepOutputDir)
+
+	step.Path = genTestDirPath("packerHypervOutput")
+
+	// Create the directory so that we can test the cleanup
+	err := os.Mkdir(step.Path, 0755)
+	if err != nil {
+		t.Fatal("Test failed to create directory for test of Cancel and Cleanup")
+	}
+	defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong
+
+	// 'Halt' the build and test the directory is removed
+	state.Put(multistep.StateHalted, true)
+
+	// Ensure the directory isn't removed if the cleanup flag is false
+	step.cleanup = false
+	step.Cleanup(state)
+	if _, err := os.Stat(step.Path); err != nil {
+		t.Fatal("Output dir should NOT be removed if on 'Halt' if cleanup flag is unset/false")
+	}
+
+	// Ensure the directory is removed if the cleanup flag is true
+	step.cleanup = true
+	step.Cleanup(state)
+	if _, err := os.Stat(step.Path); err == nil {
+		t.Fatalf("Output directory should NOT exist after 'Halt' and Cleanup: %s", step.Path)
+	}
+}

builder/hyperv/common/step_shutdown.go

@@ -17,10 +17,9 @@ import (
 //
 // Uses:
 //   communicator packer.Communicator
-//   dir OutputDir
-//   driver Driver
-//   ui     packer.Ui
-//   vmx_path string
+//   driver       Driver
+//   ui           packer.Ui
+//   vmName       string
 //
 // Produces:
 //   <nothing>

builder/hyperv/common/step_sleep.go

@@ -18,7 +18,8 @@ func (s *StepSleep) Run(_ context.Context, state multistep.StateBag) multistep.S
 	ui := state.Get("ui").(packer.Ui)
 
 	if len(s.ActionName) > 0 {
-		ui.Say(s.ActionName + "! Waiting for " + fmt.Sprintf("%v", uint(s.Minutes)) + " minutes to let the action to complete...")
+		ui.Say(s.ActionName + "! Waiting for " + fmt.Sprintf("%v", uint(s.Minutes)) +
+			" minutes to let the action to complete...")
 	}
 	time.Sleep(time.Minute * s.Minutes)
 

builder/hyperv/common/step_test.go

@@ -0,0 +1,28 @@
+package common
+
+import (
+	"bytes"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/hashicorp/packer/common/uuid"
+	"github.com/hashicorp/packer/helper/multistep"
+	"github.com/hashicorp/packer/packer"
+)
+
+func testState(t *testing.T) multistep.StateBag {
+	state := new(multistep.BasicStateBag)
+	state.Put("driver", new(DriverMock))
+	state.Put("ui", &packer.BasicUi{
+		Reader: new(bytes.Buffer),
+		Writer: new(bytes.Buffer),
+	})
+	return state
+}
+
+// Generates an absolute path to a directory under OS temp with a name
+// beginning with prefix and a UUID appended to the end
+func genTestDirPath(prefix string) string {
+	return filepath.Join(os.TempDir(), prefix+"-"+uuid.TimeOrderedUUID())
+}

builder/hyperv/common/step_type_boot_command.go

@@ -21,10 +21,11 @@ type bootCommandTemplateData struct {
 
 // This step "types" the boot command into the VM via the Hyper-V virtual keyboard
 type StepTypeBootCommand struct {
-	BootCommand string
-	BootWait    time.Duration
-	SwitchName  string
-	Ctx         interpolate.Context
+	BootCommand   string
+	BootWait      time.Duration
+	SwitchName    string
+	Ctx           interpolate.Context
+	GroupInterval time.Duration
 }
 
 func (s *StepTypeBootCommand) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
@@ -66,7 +67,7 @@ func (s *StepTypeBootCommand) Run(ctx context.Context, state multistep.StateBag)
 		scanCodesToSendString := strings.Join(codes, " ")
 		return driver.TypeScanCodes(vmName, scanCodesToSendString)
 	}
-	d := bootcommand.NewPCXTDriver(sendCodes, -1)
+	d := bootcommand.NewPCXTDriver(sendCodes, -1, s.GroupInterval)
 
 	ui.Say("Typing the boot command...")
 	command, err := interpolate.Render(s.BootCommand, &s.Ctx)

builder/hyperv/common/step_unmount_dvddrive.go

@@ -27,7 +27,8 @@ func (s *StepUnmountDvdDrive) Run(_ context.Context, state multistep.StateBag) m
 	dvdController := dvdControllerState.(DvdControllerProperties)
 
 	if dvdController.Existing {
-		ui.Say(fmt.Sprintf("Unmounting os dvd drives controller %d location %d ...", dvdController.ControllerNumber, dvdController.ControllerLocation))
+		ui.Say(fmt.Sprintf("Unmounting os dvd drives controller %d location %d ...",
+			dvdController.ControllerNumber, dvdController.ControllerLocation))
 		err := driver.UnmountDvdDrive(vmName, dvdController.ControllerNumber, dvdController.ControllerLocation)
 		if err != nil {
 			err := fmt.Errorf("Error unmounting os dvd drive: %s", err)
@@ -36,7 +37,8 @@ func (s *StepUnmountDvdDrive) Run(_ context.Context, state multistep.StateBag) m
 			return multistep.ActionHalt
 		}
 	} else {
-		ui.Say(fmt.Sprintf("Delete os dvd drives controller %d location %d ...", dvdController.ControllerNumber, dvdController.ControllerLocation))
+		ui.Say(fmt.Sprintf("Delete os dvd drives controller %d location %d ...",
+			dvdController.ControllerNumber, dvdController.ControllerLocation))
 		err := driver.DeleteDvdDrive(vmName, dvdController.ControllerNumber, dvdController.ControllerLocation)
 		if err != nil {
 			err := fmt.Errorf("Error deleting os dvd drive: %s", err)

builder/hyperv/common/step_unmount_guest_additions.go

@@ -27,7 +27,8 @@ func (s *StepUnmountGuestAdditions) Run(_ context.Context, state multistep.State
 	dvdController := dvdControllerState.(DvdControllerProperties)
 
 	if dvdController.Existing {
-		ui.Say(fmt.Sprintf("Unmounting Integration Services dvd drives controller %d location %d ...", dvdController.ControllerNumber, dvdController.ControllerLocation))
+		ui.Say(fmt.Sprintf("Unmounting Integration Services dvd drives controller %d location %d ...",
+			dvdController.ControllerNumber, dvdController.ControllerLocation))
 		err := driver.UnmountDvdDrive(vmName, dvdController.ControllerNumber, dvdController.ControllerLocation)
 		if err != nil {
 			err := fmt.Errorf("Error unmounting Integration Services dvd drive: %s", err)
@@ -36,7 +37,8 @@ func (s *StepUnmountGuestAdditions) Run(_ context.Context, state multistep.State
 			return multistep.ActionHalt
 		}
 	} else {
-		ui.Say(fmt.Sprintf("Delete Integration Services dvd drives controller %d location %d ...", dvdController.ControllerNumber, dvdController.ControllerLocation))
+		ui.Say(fmt.Sprintf("Delete Integration Services dvd drives controller %d location %d ...",
+			dvdController.ControllerNumber, dvdController.ControllerLocation))
 		err := driver.DeleteDvdDrive(vmName, dvdController.ControllerNumber, dvdController.ControllerLocation)
 		if err != nil {
 			err := fmt.Errorf("Error deleting Integration Services dvd drive: %s", err)

builder/hyperv/common/step_unmount_secondary_dvd_images.go

@@ -28,7 +28,8 @@ func (s *StepUnmountSecondaryDvdImages) Run(_ context.Context, state multistep.S
 
 	for _, dvdController := range dvdControllers {
 		if dvdController.Existing {
-			ui.Say(fmt.Sprintf("Unmounting secondary dvd drives controller %d location %d ...", dvdController.ControllerNumber, dvdController.ControllerLocation))
+			ui.Say(fmt.Sprintf("Unmounting secondary dvd drives controller %d location %d ...",
+				dvdController.ControllerNumber, dvdController.ControllerLocation))
 			err := driver.UnmountDvdDrive(vmName, dvdController.ControllerNumber, dvdController.ControllerLocation)
 			if err != nil {
 				err := fmt.Errorf("Error unmounting secondary dvd drive: %s", err)
@@ -37,7 +38,8 @@ func (s *StepUnmountSecondaryDvdImages) Run(_ context.Context, state multistep.S
 				return multistep.ActionHalt
 			}
 		} else {
-			ui.Say(fmt.Sprintf("Delete secondary dvd drives controller %d location %d ...", dvdController.ControllerNumber, dvdController.ControllerLocation))
+			ui.Say(fmt.Sprintf("Delete secondary dvd drives controller %d location %d ...",
+				dvdController.ControllerNumber, dvdController.ControllerLocation))
 			err := driver.DeleteDvdDrive(vmName, dvdController.ControllerNumber, dvdController.ControllerLocation)
 			if err != nil {
 				err := fmt.Errorf("Error deleting secondary dvd drive: %s", err)

builder/hyperv/iso/builder.go

@@ -96,11 +96,6 @@ type Config struct {
 	EnableVirtualizationExtensions bool   `mapstructure:"enable_virtualization_extensions"`
 	TempPath                       string `mapstructure:"temp_path"`
 
-	// A separate path can be used for storing the VM's disk image. The purpose is to enable
-	// reading and writing to take place on different physical disks (read from VHD temp path
-	// write to regular temp path while exporting the VM) to eliminate a single-disk bottleneck.
-	VhdTempPath string `mapstructure:"vhd_temp_path"`
-
 	Communicator string `mapstructure:"communicator"`
 
 	AdditionalDiskSize []uint `mapstructure:"disk_additional_size"`
@@ -150,7 +145,9 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 	errs = packer.MultiErrorAppend(errs, b.config.SSHConfig.Prepare(&b.config.ctx)...)
 	errs = packer.MultiErrorAppend(errs, b.config.ShutdownConfig.Prepare(&b.config.ctx)...)
 
-	if len(b.config.ISOConfig.ISOUrls) < 1 || (strings.ToLower(filepath.Ext(b.config.ISOConfig.ISOUrls[0])) != ".vhd" && strings.ToLower(filepath.Ext(b.config.ISOConfig.ISOUrls[0])) != ".vhdx") {
+	if len(b.config.ISOConfig.ISOUrls) < 1 ||
+		(strings.ToLower(filepath.Ext(b.config.ISOConfig.ISOUrls[0])) != ".vhd" &&
+			strings.ToLower(filepath.Ext(b.config.ISOConfig.ISOUrls[0])) != ".vhdx") {
 		//We only create a new hard drive if an existing one to copy from does not exist
 		err = b.checkDiskSize()
 		if err != nil {
@@ -254,25 +251,35 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 
 	if b.config.Generation < 2 && numberOfIsos > 2 {
 		if b.config.GuestAdditionsMode == "attach" {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are only 2 ide controllers available, so we can't support guest additions and these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", ")))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are only 2 ide controllers available, "+
+				"so we can't support guest additions and these secondary dvds: %s",
+				strings.Join(b.config.SecondaryDvdImages, ", ")))
 		} else {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are only 2 ide controllers available, so we can't support these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", ")))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are only 2 ide controllers available, "+
+				"so we can't support these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", ")))
 		}
 	} else if b.config.Generation > 1 && len(b.config.SecondaryDvdImages) > 16 {
 		if b.config.GuestAdditionsMode == "attach" {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are not enough drive letters available for scsi (limited to 16), so we can't support guest additions and these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", ")))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are not enough drive letters available "+
+				"for scsi (limited to 16), so we can't support guest additions and these secondary dvds: %s",
+				strings.Join(b.config.SecondaryDvdImages, ", ")))
 		} else {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are not enough drive letters available for scsi (limited to 16), so we can't support these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", ")))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are not enough drive letters available "+
+				"for scsi (limited to 16), so we can't support these secondary dvds: %s",
+				strings.Join(b.config.SecondaryDvdImages, ", ")))
 		}
 	}
 
 	if b.config.EnableVirtualizationExtensions {
 		hasVirtualMachineVirtualizationExtensions, err := powershell.HasVirtualMachineVirtualizationExtensions()
 		if err != nil {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting virtual machine virtualization extensions support: %s", err))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting virtual machine virtualization "+
+				"extensions support: %s", err))
 		} else {
 			if !hasVirtualMachineVirtualizationExtensions {
-				errs = packer.MultiErrorAppend(errs, fmt.Errorf("This version of Hyper-V does not support virtual machine virtualization extension. Please use Windows 10 or Windows Server 2016 or newer."))
+				errs = packer.MultiErrorAppend(errs, fmt.Errorf("This version of Hyper-V does not support "+
+					"virtual machine virtualization extension. Please use Windows 10 or Windows Server "+
+					"2016 or newer."))
 			}
 		}
 	}
@@ -307,24 +314,29 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 
 	if b.config.EnableVirtualizationExtensions {
 		if b.config.EnableDynamicMemory {
-			warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, dynamic memory should not be allowed.")
+			warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, " +
+				"dynamic memory should not be allowed.")
 			warnings = appendWarnings(warnings, warning)
 		}
 
 		if !b.config.EnableMacSpoofing {
-			warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, mac spoofing should be allowed.")
+			warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, " +
+				"mac spoofing should be allowed.")
 			warnings = appendWarnings(warnings, warning)
 		}
 
 		if b.config.RamSize < MinNestedVirtualizationRamSize {
-			warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, there should be 4GB or more memory set for the vm, otherwise Hyper-V may fail to start any nested VMs.")
+			warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, " +
+				"there should be 4GB or more memory set for the vm, otherwise Hyper-V may fail to start " +
+				"any nested VMs.")
 			warnings = appendWarnings(warnings, warning)
 		}
 	}
 
 	if b.config.SwitchVlanId != "" {
 		if b.config.SwitchVlanId != b.config.VlanId {
-			warning = fmt.Sprintf("Switch network adaptor vlan should match virtual machine network adaptor vlan. The switch will not be able to see traffic from the VM.")
+			warning = fmt.Sprintf("Switch network adaptor vlan should match virtual machine network adaptor " +
+				"vlan. The switch will not be able to see traffic from the VM.")
 			warnings = appendWarnings(warnings, warning)
 		}
 	}
@@ -355,9 +367,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 	state.Put("ui", ui)
 
 	steps := []multistep.Step{
-		&hypervcommon.StepCreateTempDir{
-			TempPath:    b.config.TempPath,
-			VhdTempPath: b.config.VhdTempPath,
+		&hypervcommon.StepCreateBuildDir{
+			TempPath: b.config.TempPath,
 		},
 		&hypervcommon.StepOutputDir{
 			Force: b.config.PackerForce,
@@ -399,8 +410,6 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			EnableVirtualizationExtensions: b.config.EnableVirtualizationExtensions,
 			AdditionalDiskSize:             b.config.AdditionalDiskSize,
 			DifferencingDisk:               b.config.DifferencingDisk,
-			SkipExport:                     b.config.SkipExport,
-			OutputDir:                      b.config.OutputDir,
 			MacAddress:                     b.config.MacAddress,
 			FixedVHD:                       b.config.FixedVHD,
 		},
@@ -434,17 +443,18 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		},
 
 		&hypervcommon.StepTypeBootCommand{
-			BootCommand: b.config.FlatBootCommand(),
-			BootWait:    b.config.BootWait,
-			SwitchName:  b.config.SwitchName,
-			Ctx:         b.config.ctx,
+			BootCommand:   b.config.FlatBootCommand(),
+			BootWait:      b.config.BootWait,
+			SwitchName:    b.config.SwitchName,
+			Ctx:           b.config.ctx,
+			GroupInterval: b.config.BootConfig.BootGroupInterval,
 		},
 
 		// configure the communicator ssh, winrm
 		&communicator.StepConnect{
 			Config:    &b.config.SSHConfig.Comm,
 			Host:      hypervcommon.CommHost,
-			SSHConfig: hypervcommon.SSHConfigFunc(&b.config.SSHConfig),
+			SSHConfig: b.config.SSHConfig.Comm.SSHConfigFunc(),
 		},
 
 		// provision requires communicator to be setup
@@ -466,10 +476,16 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		&hypervcommon.StepUnmountFloppyDrive{
 			Generation: b.config.Generation,
 		},
-		&hypervcommon.StepExportVm{
-			OutputDir:      b.config.OutputDir,
+		&hypervcommon.StepCompactDisk{
 			SkipCompaction: b.config.SkipCompaction,
-			SkipExport:     b.config.SkipExport,
+		},
+		&hypervcommon.StepExportVm{
+			OutputDir:  b.config.OutputDir,
+			SkipExport: b.config.SkipExport,
+		},
+		&hypervcommon.StepCollateArtifacts{
+			OutputDir:  b.config.OutputDir,
+			SkipExport: b.config.SkipExport,
 		},
 
 		// the clean up actions for each step will be executed reverse order
@@ -526,11 +542,14 @@ func (b *Builder) checkDiskSize() error {
 	log.Println(fmt.Sprintf("%s: %v", "DiskSize", b.config.DiskSize))
 
 	if b.config.DiskSize < MinDiskSize {
-		return fmt.Errorf("disk_size: Virtual machine requires disk space >= %v GB, but defined: %v", MinDiskSize, b.config.DiskSize/1024)
+		return fmt.Errorf("disk_size: Virtual machine requires disk space >= %v GB, but defined: %v",
+			MinDiskSize, b.config.DiskSize/1024)
 	} else if b.config.DiskSize > MaxDiskSize && !b.config.FixedVHD {
-		return fmt.Errorf("disk_size: Virtual machine requires disk space <= %v GB, but defined: %v", MaxDiskSize, b.config.DiskSize/1024)
+		return fmt.Errorf("disk_size: Virtual machine requires disk space <= %v GB, but defined: %v",
+			MaxDiskSize, b.config.DiskSize/1024)
 	} else if b.config.DiskSize > MaxVHDSize && b.config.FixedVHD {
-		return fmt.Errorf("disk_size: Virtual machine requires disk space <= %v GB, but defined: %v", MaxVHDSize/1024, b.config.DiskSize/1024)
+		return fmt.Errorf("disk_size: Virtual machine requires disk space <= %v GB, but defined: %v",
+			MaxVHDSize/1024, b.config.DiskSize/1024)
 	}
 
 	return nil
@@ -544,9 +563,11 @@ func (b *Builder) checkDiskBlockSize() error {
 	log.Println(fmt.Sprintf("%s: %v", "DiskBlockSize", b.config.DiskBlockSize))
 
 	if b.config.DiskBlockSize < MinDiskBlockSize {
-		return fmt.Errorf("disk_block_size: Virtual machine requires disk block size >= %v MB, but defined: %v", MinDiskBlockSize, b.config.DiskBlockSize)
+		return fmt.Errorf("disk_block_size: Virtual machine requires disk block size >= %v MB, but defined: %v",
+			MinDiskBlockSize, b.config.DiskBlockSize)
 	} else if b.config.DiskBlockSize > MaxDiskBlockSize {
-		return fmt.Errorf("disk_block_size: Virtual machine requires disk block size <= %v MB, but defined: %v", MaxDiskBlockSize, b.config.DiskBlockSize)
+		return fmt.Errorf("disk_block_size: Virtual machine requires disk block size <= %v MB, but defined: %v",
+			MaxDiskBlockSize, b.config.DiskBlockSize)
 	}
 
 	return nil
@@ -560,9 +581,11 @@ func (b *Builder) checkRamSize() error {
 	log.Println(fmt.Sprintf("%s: %v", "RamSize", b.config.RamSize))
 
 	if b.config.RamSize < MinRamSize {
-		return fmt.Errorf("ram_size: Virtual machine requires memory size >= %v MB, but defined: %v", MinRamSize, b.config.RamSize)
+		return fmt.Errorf("ram_size: Virtual machine requires memory size >= %v MB, but defined: %v",
+			MinRamSize, b.config.RamSize)
 	} else if b.config.RamSize > MaxRamSize {
-		return fmt.Errorf("ram_size: Virtual machine requires memory size <= %v MB, but defined: %v", MaxRamSize, b.config.RamSize)
+		return fmt.Errorf("ram_size: Virtual machine requires memory size <= %v MB, but defined: %v",
+			MaxRamSize, b.config.RamSize)
 	}
 
 	return nil

builder/hyperv/iso/builder_test.go

@@ -104,7 +104,8 @@ func TestBuilderPrepare_DiskBlockSize(t *testing.T) {
 		t.Fatalf("bad err: %s", err)
 	}
 	if b.config.DiskBlockSize != expected_default_block_size {
-		t.Fatalf("bad default block size with empty config: %d. Expected %d", b.config.DiskBlockSize, expected_default_block_size)
+		t.Fatalf("bad default block size with empty config: %d. Expected %d", b.config.DiskBlockSize,
+			expected_default_block_size)
 	}
 
 	test_sizes := []uint{0, 1, 32, 256, 512, 1 * 1024, 32 * 1024}
@@ -117,7 +118,8 @@ func TestBuilderPrepare_DiskBlockSize(t *testing.T) {
 				t.Fatalf("bad, should have no warns: %#v", warns)
 			}
 			if err == nil {
-				t.Fatalf("bad, should have error but didn't. disk_block_size=%d outside expected valid range [%d,%d]", test_size, expected_min_block_size, expected_max_block_size)
+				t.Fatalf("bad, should have error. disk_block_size=%d outside expected valid range [%d,%d]",
+					test_size, expected_min_block_size, expected_max_block_size)
 			}
 		} else {
 			if len(warns) > 0 {
@@ -128,11 +130,13 @@ func TestBuilderPrepare_DiskBlockSize(t *testing.T) {
 			}
 			if test_size == 0 {
 				if b.config.DiskBlockSize != expected_default_block_size {
-					t.Fatalf("bad default block size with 0 value config: %d. Expected: %d", b.config.DiskBlockSize, expected_default_block_size)
+					t.Fatalf("bad default block size with 0 value config: %d. Expected: %d",
+						b.config.DiskBlockSize, expected_default_block_size)
 				}
 			} else {
 				if b.config.DiskBlockSize != test_size {
-					t.Fatalf("bad block size with 0 value config: %d. Expected: %d", b.config.DiskBlockSize, expected_default_block_size)
+					t.Fatalf("bad block size with 0 value config: %d. Expected: %d", b.config.DiskBlockSize,
+						expected_default_block_size)
 				}
 			}
 		}
@@ -147,7 +151,8 @@ func TestBuilderPrepare_FixedVHDFormat(t *testing.T) {
 	config["skip_compaction"] = true
 	config["differencing_disk"] = false
 
-	//use_fixed_vhd_format should work with generation = 1, skip_compaction = true, and differencing_disk = false
+	// use_fixed_vhd_format should work with generation = 1, skip_compaction
+	// = true, and differencing_disk = false
 	warns, err := b.Prepare(config)
 	if len(warns) > 0 {
 		t.Fatalf("bad: %#v", warns)

builder/hyperv/vmcx/builder.go

@@ -62,7 +62,7 @@ type Config struct {
 	GuestAdditionsPath string `mapstructure:"guest_additions_path"`
 
 	// This is the path to a directory containing an exported virtual machine.
-	CloneFromVMXCPath string `mapstructure:"clone_from_vmxc_path"`
+	CloneFromVMCXPath string `mapstructure:"clone_from_vmcx_path"`
 
 	// This is the name of the virtual machine to clone from.
 	CloneFromVMName string `mapstructure:"clone_from_vm_name"`
@@ -91,6 +91,7 @@ type Config struct {
 	EnableSecureBoot               bool   `mapstructure:"enable_secure_boot"`
 	SecureBootTemplate             string `mapstructure:"secure_boot_template"`
 	EnableVirtualizationExtensions bool   `mapstructure:"enable_virtualization_extensions"`
+	TempPath                       string `mapstructure:"temp_path"`
 
 	Communicator string `mapstructure:"communicator"`
 
@@ -157,36 +158,45 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 	b.config.Generation = 1
 
 	if b.config.CloneFromVMName == "" {
-		if b.config.CloneFromVMXCPath == "" {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("The clone_from_vm_name must be specified if clone_from_vmxc_path is not specified."))
+		if b.config.CloneFromVMCXPath == "" {
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("The clone_from_vm_name must be specified if "+
+				"clone_from_vmcx_path is not specified."))
 		}
 	} else {
 		virtualMachineExists, err := powershell.DoesVirtualMachineExist(b.config.CloneFromVMName)
 		if err != nil {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting if virtual machine to clone from exists: %s", err))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting if virtual machine to clone "+
+				"from exists: %s", err))
 		} else {
 			if !virtualMachineExists {
-				errs = packer.MultiErrorAppend(errs, fmt.Errorf("Virtual machine '%s' to clone from does not exist.", b.config.CloneFromVMName))
+				errs = packer.MultiErrorAppend(errs, fmt.Errorf("Virtual machine '%s' to clone from does not "+
+					"exist.", b.config.CloneFromVMName))
 			} else {
 				b.config.Generation, err = powershell.GetVirtualMachineGeneration(b.config.CloneFromVMName)
 				if err != nil {
-					errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting virtual machine to clone from generation: %s", err))
+					errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting virtual machine to clone "+
+						"from generation: %s", err))
 				}
 
 				if b.config.CloneFromSnapshotName != "" {
-					virtualMachineSnapshotExists, err := powershell.DoesVirtualMachineSnapshotExist(b.config.CloneFromVMName, b.config.CloneFromSnapshotName)
+					virtualMachineSnapshotExists, err := powershell.DoesVirtualMachineSnapshotExist(
+						b.config.CloneFromVMName, b.config.CloneFromSnapshotName)
 					if err != nil {
-						errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting if virtual machine snapshot to clone from exists: %s", err))
+						errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting if virtual machine "+
+							"snapshot to clone from exists: %s", err))
 					} else {
 						if !virtualMachineSnapshotExists {
-							errs = packer.MultiErrorAppend(errs, fmt.Errorf("Virtual machine snapshot '%s' on virtual machine '%s' to clone from does not exist.", b.config.CloneFromSnapshotName, b.config.CloneFromVMName))
+							errs = packer.MultiErrorAppend(errs, fmt.Errorf("Virtual machine snapshot '%s' on "+
+								"virtual machine '%s' to clone from does not exist.",
+								b.config.CloneFromSnapshotName, b.config.CloneFromVMName))
 						}
 					}
 				}
 
 				virtualMachineOn, err := powershell.IsVirtualMachineOn(b.config.CloneFromVMName)
 				if err != nil {
-					errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting if virtual machine to clone is running: %s", err))
+					errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting if virtual machine to "+
+						"clone is running: %s", err))
 				} else {
 					if virtualMachineOn {
 						warning := fmt.Sprintf("Cloning from a virtual machine that is running.")
@@ -197,15 +207,16 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		}
 	}
 
-	if b.config.CloneFromVMXCPath == "" {
+	if b.config.CloneFromVMCXPath == "" {
 		if b.config.CloneFromVMName == "" {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("The clone_from_vmxc_path be specified if clone_from_vm_name must is not specified."))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("The clone_from_vmcx_path be specified if "+
+				"clone_from_vm_name must is not specified."))
 		}
 	} else {
-		if _, err := os.Stat(b.config.CloneFromVMXCPath); os.IsNotExist(err) {
+		if _, err := os.Stat(b.config.CloneFromVMCXPath); os.IsNotExist(err) {
 			if err != nil {
 				errs = packer.MultiErrorAppend(
-					errs, fmt.Errorf("CloneFromVMXCPath does not exist: %s", err))
+					errs, fmt.Errorf("CloneFromVMCXPath does not exist: %s", err))
 			}
 		}
 	}
@@ -276,25 +287,36 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 
 	if b.config.Generation < 2 && numberOfIsos > 2 {
 		if b.config.GuestAdditionsMode == "attach" {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are only 2 ide controllers available, so we can't support guest additions and these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", ")))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are only 2 ide controllers available, so "+
+				"we can't support guest additions and these secondary dvds: %s",
+				strings.Join(b.config.SecondaryDvdImages, ", ")))
 		} else {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are only 2 ide controllers available, so we can't support these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", ")))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are only 2 ide controllers available, so "+
+				"we can't support these secondary dvds: %s",
+				strings.Join(b.config.SecondaryDvdImages, ", ")))
 		}
 	} else if b.config.Generation > 1 && len(b.config.SecondaryDvdImages) > 16 {
 		if b.config.GuestAdditionsMode == "attach" {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are not enough drive letters available for scsi (limited to 16), so we can't support guest additions and these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", ")))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are not enough drive letters available for "+
+				"scsi (limited to 16), so we can't support guest additions and these secondary dvds: %s",
+				strings.Join(b.config.SecondaryDvdImages, ", ")))
 		} else {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are not enough drive letters available for scsi (limited to 16), so we can't support these secondary dvds: %s", strings.Join(b.config.SecondaryDvdImages, ", ")))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("There are not enough drive letters available for "+
+				"scsi (limited to 16), so we can't support these secondary dvds: %s",
+				strings.Join(b.config.SecondaryDvdImages, ", ")))
 		}
 	}
 
 	if b.config.EnableVirtualizationExtensions {
 		hasVirtualMachineVirtualizationExtensions, err := powershell.HasVirtualMachineVirtualizationExtensions()
 		if err != nil {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting virtual machine virtualization extensions support: %s", err))
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("Failed detecting virtual machine virtualization "+
+				"extensions support: %s", err))
 		} else {
 			if !hasVirtualMachineVirtualizationExtensions {
-				errs = packer.MultiErrorAppend(errs, fmt.Errorf("This version of Hyper-V does not support virtual machine virtualization extension. Please use Windows 10 or Windows Server 2016 or newer."))
+				errs = packer.MultiErrorAppend(errs, fmt.Errorf("This version of Hyper-V does not support "+
+					"virtual machine virtualization extension. Please use Windows 10 or Windows Server 2016 "+
+					"or newer."))
 			}
 		}
 	}
@@ -314,24 +336,29 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 
 	if b.config.EnableVirtualizationExtensions {
 		if b.config.EnableDynamicMemory {
-			warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, dynamic memory should not be allowed.")
+			warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, " +
+				"dynamic memory should not be allowed.")
 			warnings = appendWarnings(warnings, warning)
 		}
 
 		if !b.config.EnableMacSpoofing {
-			warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, mac spoofing should be allowed.")
+			warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, " +
+				"mac spoofing should be allowed.")
 			warnings = appendWarnings(warnings, warning)
 		}
 
 		if b.config.RamSize < MinNestedVirtualizationRamSize {
-			warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, there should be 4GB or more memory set for the vm, otherwise Hyper-V may fail to start any nested VMs.")
+			warning = fmt.Sprintf("For nested virtualization, when virtualization extension is enabled, " +
+				"there should be 4GB or more memory set for the vm, otherwise Hyper-V may fail to start " +
+				"any nested VMs.")
 			warnings = appendWarnings(warnings, warning)
 		}
 	}
 
 	if b.config.SwitchVlanId != "" {
 		if b.config.SwitchVlanId != b.config.VlanId {
-			warning = fmt.Sprintf("Switch network adaptor vlan should match virtual machine network adaptor vlan. The switch will not be able to see traffic from the VM.")
+			warning = fmt.Sprintf("Switch network adaptor vlan should match virtual machine network adaptor " +
+				"vlan. The switch will not be able to see traffic from the VM.")
 			warnings = appendWarnings(warnings, warning)
 		}
 	}
@@ -362,7 +389,9 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 	state.Put("ui", ui)
 
 	steps := []multistep.Step{
-		&hypervcommon.StepCreateTempDir{},
+		&hypervcommon.StepCreateBuildDir{
+			TempPath: b.config.TempPath,
+		},
 		&hypervcommon.StepOutputDir{
 			Force: b.config.PackerForce,
 			Path:  b.config.OutputDir,
@@ -397,7 +426,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			SwitchName: b.config.SwitchName,
 		},
 		&hypervcommon.StepCloneVM{
-			CloneFromVMXCPath:              b.config.CloneFromVMXCPath,
+			CloneFromVMCXPath:              b.config.CloneFromVMCXPath,
 			CloneFromVMName:                b.config.CloneFromVMName,
 			CloneFromSnapshotName:          b.config.CloneFromSnapshotName,
 			CloneAllSnapshots:              b.config.CloneAllSnapshots,
@@ -443,17 +472,18 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		},
 
 		&hypervcommon.StepTypeBootCommand{
-			BootCommand: b.config.FlatBootCommand(),
-			BootWait:    b.config.BootWait,
-			SwitchName:  b.config.SwitchName,
-			Ctx:         b.config.ctx,
+			BootCommand:   b.config.FlatBootCommand(),
+			BootWait:      b.config.BootWait,
+			SwitchName:    b.config.SwitchName,
+			Ctx:           b.config.ctx,
+			GroupInterval: b.config.BootConfig.BootGroupInterval,
 		},
 
 		// configure the communicator ssh, winrm
 		&communicator.StepConnect{
 			Config:    &b.config.SSHConfig.Comm,
 			Host:      hypervcommon.CommHost,
-			SSHConfig: hypervcommon.SSHConfigFunc(&b.config.SSHConfig),
+			SSHConfig: b.config.SSHConfig.Comm.SSHConfigFunc(),
 		},
 
 		// provision requires communicator to be setup
@@ -475,10 +505,16 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		&hypervcommon.StepUnmountFloppyDrive{
 			Generation: b.config.Generation,
 		},
-		&hypervcommon.StepExportVm{
-			OutputDir:      b.config.OutputDir,
+		&hypervcommon.StepCompactDisk{
 			SkipCompaction: b.config.SkipCompaction,
-			SkipExport:     b.config.SkipExport,
+		},
+		&hypervcommon.StepExportVm{
+			OutputDir:  b.config.OutputDir,
+			SkipExport: b.config.SkipExport,
+		},
+		&hypervcommon.StepCollateArtifacts{
+			OutputDir:  b.config.OutputDir,
+			SkipExport: b.config.SkipExport,
 		},
 
 		// the clean up actions for each step will be executed reverse order
@@ -535,9 +571,11 @@ func (b *Builder) checkRamSize() error {
 	log.Println(fmt.Sprintf("%s: %v", "RamSize", b.config.RamSize))
 
 	if b.config.RamSize < MinRamSize {
-		return fmt.Errorf("ram_size: Virtual machine requires memory size >= %v MB, but defined: %v", MinRamSize, b.config.RamSize)
+		return fmt.Errorf("ram_size: Virtual machine requires memory size >= %v MB, but defined: %v",
+			MinRamSize, b.config.RamSize)
 	} else if b.config.RamSize > MaxRamSize {
-		return fmt.Errorf("ram_size: Virtual machine requires memory size <= %v MB, but defined: %v", MaxRamSize, b.config.RamSize)
+		return fmt.Errorf("ram_size: Virtual machine requires memory size <= %v MB, but defined: %v",
+			MaxRamSize, b.config.RamSize)
 	}
 
 	return nil

builder/hyperv/vmcx/builder_test.go

@@ -23,7 +23,7 @@ func testConfig() map[string]interface{} {
 		"ssh_username":            "foo",
 		"ram_size":                64,
 		"guest_additions_mode":    "none",
-		"clone_from_vmxc_path":    "generated",
+		"clone_from_vmcx_path":    "generated",
 		packer.BuildNameConfigKey: "foo",
 	}
 }
@@ -40,13 +40,13 @@ func TestBuilderPrepare_Defaults(t *testing.T) {
 	var b Builder
 	config := testConfig()
 
-	//Create vmxc folder
+	//Create vmcx folder
 	td, err := ioutil.TempDir("", "packer")
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
 	defer os.RemoveAll(td)
-	config["clone_from_vmxc_path"] = td
+	config["clone_from_vmcx_path"] = td
 
 	warns, err := b.Prepare(config)
 	if len(warns) > 0 {
@@ -65,13 +65,13 @@ func TestBuilderPrepare_InvalidKey(t *testing.T) {
 	var b Builder
 	config := testConfig()
 
-	//Create vmxc folder
+	//Create vmcx folder
 	td, err := ioutil.TempDir("", "packer")
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
 	defer os.RemoveAll(td)
-	config["clone_from_vmxc_path"] = td
+	config["clone_from_vmcx_path"] = td
 
 	// Add a random key
 	config["i_should_not_be_valid"] = true
@@ -87,7 +87,7 @@ func TestBuilderPrepare_InvalidKey(t *testing.T) {
 func TestBuilderPrepare_CloneFromExistingMachineOrImportFromExportedMachineSettingsRequired(t *testing.T) {
 	var b Builder
 	config := testConfig()
-	delete(config, "clone_from_vmxc_path")
+	delete(config, "clone_from_vmcx_path")
 
 	warns, err := b.Prepare(config)
 	if len(warns) > 0 {
@@ -102,7 +102,7 @@ func TestBuilderPrepare_ExportedMachinePathDoesNotExist(t *testing.T) {
 	var b Builder
 	config := testConfig()
 
-	//Create vmxc folder
+	//Create vmcx folder
 	td, err := ioutil.TempDir("", "packer")
 	if err != nil {
 		t.Fatalf("err: %s", err)
@@ -111,7 +111,7 @@ func TestBuilderPrepare_ExportedMachinePathDoesNotExist(t *testing.T) {
 	//Delete the folder immediately
 	os.RemoveAll(td)
 
-	config["clone_from_vmxc_path"] = td
+	config["clone_from_vmcx_path"] = td
 
 	warns, err := b.Prepare(config)
 	if len(warns) > 0 {
@@ -126,7 +126,7 @@ func TestBuilderPrepare_ExportedMachinePathExists(t *testing.T) {
 	var b Builder
 	config := testConfig()
 
-	//Create vmxc folder
+	//Create vmcx folder
 	td, err := ioutil.TempDir("", "packer")
 	if err != nil {
 		t.Fatalf("err: %s", err)
@@ -135,7 +135,7 @@ func TestBuilderPrepare_ExportedMachinePathExists(t *testing.T) {
 	//Only delete afterwards
 	defer os.RemoveAll(td)
 
-	config["clone_from_vmxc_path"] = td
+	config["clone_from_vmcx_path"] = td
 
 	warns, err := b.Prepare(config)
 	if len(warns) > 0 {
@@ -146,10 +146,10 @@ func TestBuilderPrepare_ExportedMachinePathExists(t *testing.T) {
 	}
 }
 
-func disabled_TestBuilderPrepare_CloneFromVmSettingUsedSoNoCloneFromVmxcPathRequired(t *testing.T) {
+func disabled_TestBuilderPrepare_CloneFromVmSettingUsedSoNoCloneFromVmcxPathRequired(t *testing.T) {
 	var b Builder
 	config := testConfig()
-	delete(config, "clone_from_vmxc_path")
+	delete(config, "clone_from_vmcx_path")
 
 	config["clone_from_vm_name"] = "test_machine_name_that_does_not_exist"
 
@@ -162,7 +162,8 @@ func disabled_TestBuilderPrepare_CloneFromVmSettingUsedSoNoCloneFromVmxcPathRequ
 		t.Fatal("should have error")
 	} else {
 		errorMessage := err.Error()
-		if errorMessage != "1 error(s) occurred:\n\n* Virtual machine 'test_machine_name_that_does_not_exist' to clone from does not exist." {
+		if errorMessage != "1 error(s) occurred:\n\n* Virtual machine 'test_machine_name_that_does_not_exist' "+
+			"to clone from does not exist." {
 			t.Fatalf("should not have error: %s", err)
 		}
 	}
@@ -172,13 +173,13 @@ func TestBuilderPrepare_ISOChecksum(t *testing.T) {
 	var b Builder
 	config := testConfig()
 
-	//Create vmxc folder
+	//Create vmcx folder
 	td, err := ioutil.TempDir("", "packer")
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
 	defer os.RemoveAll(td)
-	config["clone_from_vmxc_path"] = td
+	config["clone_from_vmcx_path"] = td
 
 	// Test bad
 	config["iso_checksum"] = ""
@@ -210,13 +211,13 @@ func TestBuilderPrepare_ISOChecksumType(t *testing.T) {
 	var b Builder
 	config := testConfig()
 
-	//Create vmxc folder
+	//Create vmcx folder
 	td, err := ioutil.TempDir("", "packer")
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
 	defer os.RemoveAll(td)
-	config["clone_from_vmxc_path"] = td
+	config["clone_from_vmcx_path"] = td
 
 	// Test bad
 	config["iso_checksum_type"] = ""
@@ -274,13 +275,13 @@ func TestBuilderPrepare_ISOUrl(t *testing.T) {
 	var b Builder
 	config := testConfig()
 
-	//Create vmxc folder
+	//Create vmcx folder
 	td, err := ioutil.TempDir("", "packer")
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
 	defer os.RemoveAll(td)
-	config["clone_from_vmxc_path"] = td
+	config["clone_from_vmcx_path"] = td
 
 	delete(config, "iso_url")
 	delete(config, "iso_urls")
@@ -353,13 +354,13 @@ func TestBuilderPrepare_FloppyFiles(t *testing.T) {
 	var b Builder
 	config := testConfig()
 
-	//Create vmxc folder
+	//Create vmcx folder
 	td, err := ioutil.TempDir("", "packer")
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
 	defer os.RemoveAll(td)
-	config["clone_from_vmxc_path"] = td
+	config["clone_from_vmcx_path"] = td
 
 	delete(config, "floppy_files")
 	warns, err := b.Prepare(config)
@@ -395,13 +396,13 @@ func TestBuilderPrepare_InvalidFloppies(t *testing.T) {
 	var b Builder
 	config := testConfig()
 
-	//Create vmxc folder
+	//Create vmcx folder
 	td, err := ioutil.TempDir("", "packer")
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
 	defer os.RemoveAll(td)
-	config["clone_from_vmxc_path"] = td
+	config["clone_from_vmcx_path"] = td
 
 	config["floppy_files"] = []string{"nonexistent.bat", "nonexistent.ps1"}
 	b = Builder{}
@@ -420,13 +421,13 @@ func TestBuilderPrepare_CommConfig(t *testing.T) {
 	{
 		config := testConfig()
 
-		//Create vmxc folder
+		//Create vmcx folder
 		td, err := ioutil.TempDir("", "packer")
 		if err != nil {
 			t.Fatalf("err: %s", err)
 		}
 		defer os.RemoveAll(td)
-		config["clone_from_vmxc_path"] = td
+		config["clone_from_vmcx_path"] = td
 
 		config["communicator"] = "winrm"
 		config["winrm_username"] = "username"
@@ -457,13 +458,13 @@ func TestBuilderPrepare_CommConfig(t *testing.T) {
 	{
 		config := testConfig()
 
-		//Create vmxc folder
+		//Create vmcx folder
 		td, err := ioutil.TempDir("", "packer")
 		if err != nil {
 			t.Fatalf("err: %s", err)
 		}
 		defer os.RemoveAll(td)
-		config["clone_from_vmxc_path"] = td
+		config["clone_from_vmcx_path"] = td
 
 		config["communicator"] = "ssh"
 		config["ssh_username"] = "username"
@@ -495,13 +496,13 @@ func TestUserVariablesInBootCommand(t *testing.T) {
 	var b Builder
 	config := testConfig()
 
-	//Create vmxc folder
+	//Create vmcx folder
 	td, err := ioutil.TempDir("", "packer")
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
 	defer os.RemoveAll(td)
-	config["clone_from_vmxc_path"] = td
+	config["clone_from_vmcx_path"] = td
 
 	config[packer.UserVariablesConfigKey] = map[string]string{"test-variable": "test"}
 	config["boot_command"] = []string{"blah {{user `test-variable`}} blah"}

builder/ncloud/builder.go

@@ -51,7 +51,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 				Host: func(stateBag multistep.StateBag) (string, error) {
 					return stateBag.Get("PublicIP").(string), nil
 				},
-				SSHConfig: SSHConfig(b.config.Comm.SSHUsername),
+				SSHConfig: b.config.Comm.SSHConfigFunc(),
 			},
 			&common.StepProvision{},
 			NewStepStopServerInstance(conn, ui),

builder/ncloud/config_test.go

@@ -7,15 +7,15 @@ import (
 
 func testConfig() map[string]interface{} {
 	return map[string]interface{}{
-		"access_key":                "access_key",
-		"secret_key":                "secret_key",
-		"server_image_product_code": "SPSW0WINNT000016",
-		"server_product_code":       "SPSVRSSD00000011",
-		"server_image_name":         "packer-test {{timestamp}}",
-		"server_image_description":  "server description",
-		"block_storage_size":        100,
-		"user_data":                 "#!/bin/sh\nyum install -y httpd\ntouch /var/www/html/index.html\nchkconfig --level 2345 httpd on",
-		"region":                    "Korea",
+		"access_key":                            "access_key",
+		"secret_key":                            "secret_key",
+		"server_image_product_code":             "SPSW0WINNT000016",
+		"server_product_code":                   "SPSVRSSD00000011",
+		"server_image_name":                     "packer-test {{timestamp}}",
+		"server_image_description":              "server description",
+		"block_storage_size":                    100,
+		"user_data":                             "#!/bin/sh\nyum install -y httpd\ntouch /var/www/html/index.html\nchkconfig --level 2345 httpd on",
+		"region":                                "Korea",
 		"access_control_group_configuration_no": "33",
 		"communicator":                          "ssh",
 		"ssh_username":                          "root",
@@ -24,15 +24,15 @@ func testConfig() map[string]interface{} {
 
 func testConfigForMemberServerImage() map[string]interface{} {
 	return map[string]interface{}{
-		"access_key":               "access_key",
-		"secret_key":               "secret_key",
-		"server_product_code":      "SPSVRSSD00000011",
-		"member_server_image_no":   "2440",
-		"server_image_name":        "packer-test {{timestamp}}",
-		"server_image_description": "server description",
-		"block_storage_size":       100,
-		"user_data":                "#!/bin/sh\nyum install -y httpd\ntouch /var/www/html/index.html\nchkconfig --level 2345 httpd on",
-		"region":                   "Korea",
+		"access_key":                            "access_key",
+		"secret_key":                            "secret_key",
+		"server_product_code":                   "SPSVRSSD00000011",
+		"member_server_image_no":                "2440",
+		"server_image_name":                     "packer-test {{timestamp}}",
+		"server_image_description":              "server description",
+		"block_storage_size":                    100,
+		"user_data":                             "#!/bin/sh\nyum install -y httpd\ntouch /var/www/html/index.html\nchkconfig --level 2345 httpd on",
+		"region":                                "Korea",
 		"access_control_group_configuration_no": "33",
 		"communicator":                          "ssh",
 		"ssh_username":                          "root",

builder/ncloud/ssh.go

@@ -1,25 +0,0 @@
-package ncloud
-
-import (
-	packerssh "github.com/hashicorp/packer/communicator/ssh"
-	"github.com/hashicorp/packer/helper/multistep"
-	"golang.org/x/crypto/ssh"
-)
-
-// SSHConfig returns a function that can be used for the SSH communicator
-// config for connecting to the specified host via SSH
-func SSHConfig(username string) func(multistep.StateBag) (*ssh.ClientConfig, error) {
-	return func(state multistep.StateBag) (*ssh.ClientConfig, error) {
-		password := state.Get("Password").(string)
-
-		return &ssh.ClientConfig{
-			User: username,
-			Auth: []ssh.AuthMethod{
-				ssh.Password(password),
-				ssh.KeyboardInteractive(
-					packerssh.PasswordKeyboardInteractive(password)),
-			},
-			HostKeyCallback: ssh.InsecureIgnoreHostKey(),
-		}, nil
-	}
-}