only re-set path name if it needs to be unquoted; otherwise unquote r… (#8826)

builder/vagrant/driver_mock.go

@@ -0,0 +1,88 @@
+package vagrant
+
+// Create a mock driver so that we can test Vagrant builder steps
+type MockVagrantDriver struct {
+	InitCalled      bool
+	AddCalled       bool
+	UpCalled        bool
+	HaltCalled      bool
+	SuspendCalled   bool
+	SSHConfigCalled bool
+	DestroyCalled   bool
+	PackageCalled   bool
+	VerifyCalled    bool
+	VersionCalled   bool
+
+	ReturnError     error
+	ReturnSSHConfig *VagrantSSHConfig
+	GlobalID        string
+}
+
+func (d *MockVagrantDriver) Init([]string) error {
+	d.InitCalled = true
+	return d.ReturnError
+}
+
+func (d *MockVagrantDriver) Add([]string) error {
+	d.AddCalled = true
+	return d.ReturnError
+}
+
+func (d *MockVagrantDriver) Up([]string) (string, string, error) {
+	d.UpCalled = true
+	return "", "", nil
+}
+
+func (d *MockVagrantDriver) Halt(string) error {
+	d.HaltCalled = true
+	return d.ReturnError
+}
+
+func (d *MockVagrantDriver) Suspend(string) error {
+	d.SuspendCalled = true
+	return d.ReturnError
+}
+
+func (d *MockVagrantDriver) SSHConfig(gid string) (*VagrantSSHConfig, error) {
+	d.SSHConfigCalled = true
+	// track the input value
+	d.GlobalID = gid
+
+	if d.ReturnSSHConfig != nil {
+		return d.ReturnSSHConfig, nil
+	}
+
+	sshConfig := VagrantSSHConfig{
+		Hostname:               "127.0.0.1",
+		User:                   "vagrant",
+		Port:                   "2222",
+		UserKnownHostsFile:     "/dev/null",
+		StrictHostKeyChecking:  false,
+		PasswordAuthentication: false,
+		IdentityFile:           "\"/path with spaces/insecure_private_key\"",
+		IdentitiesOnly:         true,
+		LogLevel:               "FATAL"}
+	return &sshConfig, d.ReturnError
+}
+
+func (d *MockVagrantDriver) Destroy(string) error {
+	d.DestroyCalled = true
+	return d.ReturnError
+}
+
+func (d *MockVagrantDriver) Package([]string) error {
+	d.PackageCalled = true
+	return d.ReturnError
+}
+
+func (d *MockVagrantDriver) Verify() error {
+	d.VerifyCalled = true
+	return d.ReturnError
+}
+
+func (d *MockVagrantDriver) Version() (string, error) {
+	d.VersionCalled = true
+	return "", d.ReturnError
+}
+
+// End of mock definition

builder/vagrant/step_ssh_config.go

@@ -56,9 +56,9 @@ func (s *StepSSHConfig) Run(ctx context.Context, state multistep.StateBag) multi
 	}
 	log.Printf("identity file is %s", sshConfig.IdentityFile)
 	log.Printf("Removing quotes from identity file")
-	sshConfig.IdentityFile, err = strconv.Unquote(sshConfig.IdentityFile)
+	unquoted, err := strconv.Unquote(sshConfig.IdentityFile)
-	if err != nil {
+	if err == nil {
-		log.Printf("Error unquoting identity file: %s", err)
+		sshConfig.IdentityFile = unquoted
 	}
 	config.Comm.SSHPrivateKeyFile = sshConfig.IdentityFile
 	config.Comm.SSHUsername = sshConfig.User

builder/vagrant/step_ssh_config_test.go

@@ -0,0 +1,98 @@
+package vagrant
+
+import (
+	"context"
+	"testing"
+
+	"github.com/hashicorp/packer/helper/multistep"
+)
+
+func TestStepSSHConfig_Impl(t *testing.T) {
+	var raw interface{}
+	raw = new(StepSSHConfig)
+	if _, ok := raw.(multistep.Step); !ok {
+		t.Fatalf("initialize should be a step")
+	}
+}
+
+func TestPrepStepSSHConfig_GlobalID(t *testing.T) {
+	driver := &MockVagrantDriver{}
+	config := &Config{}
+	state := new(multistep.BasicStateBag)
+	state.Put("driver", driver)
+	state.Put("config", config)
+
+	step := StepSSHConfig{
+		GlobalID: "adsfadf",
+	}
+	_ = step.Run(context.Background(), state)
+	if driver.GlobalID != "adsfadf" {
+		t.Fatalf("Should have called SSHConfig with GlobalID asdfasdf")
+	}
+}
+
+func TestPrepStepSSHConfig_NoGlobalID(t *testing.T) {
+	driver := &MockVagrantDriver{}
+	config := &Config{}
+	state := new(multistep.BasicStateBag)
+	state.Put("driver", driver)
+	state.Put("config", config)
+
+	step := StepSSHConfig{}
+	_ = step.Run(context.Background(), state)
+	if driver.GlobalID != "source" {
+		t.Fatalf("Should have called SSHConfig with GlobalID source")
+	}
+}
+
+func TestPrepStepSSHConfig_SpacesInPath(t *testing.T) {
+	driver := &MockVagrantDriver{}
+	driver.ReturnSSHConfig = &VagrantSSHConfig{
+		Hostname:               "127.0.0.1",
+		User:                   "vagrant",
+		Port:                   "2222",
+		UserKnownHostsFile:     "/dev/null",
+		StrictHostKeyChecking:  false,
+		PasswordAuthentication: false,
+		IdentityFile:           "\"/path with spaces/insecure_private_key\"",
+		IdentitiesOnly:         true,
+		LogLevel:               "FATAL"}
+
+	config := &Config{}
+	state := new(multistep.BasicStateBag)
+	state.Put("driver", driver)
+	state.Put("config", config)
+
+	step := StepSSHConfig{}
+	_ = step.Run(context.Background(), state)
+	expected := "/path with spaces/insecure_private_key"
+	if config.Comm.SSHPrivateKeyFile != expected {
+		t.Fatalf("Bad config private key. Recieved: %s; expected: %s.", config.Comm.SSHPrivateKeyFile, expected)
+	}
+}
+
+func TestPrepStepSSHConfig_NoSpacesInPath(t *testing.T) {
+	driver := &MockVagrantDriver{}
+	driver.ReturnSSHConfig = &VagrantSSHConfig{
+		Hostname:               "127.0.0.1",
+		User:                   "vagrant",
+		Port:                   "2222",
+		UserKnownHostsFile:     "/dev/null",
+		StrictHostKeyChecking:  false,
+		PasswordAuthentication: false,
+		IdentityFile:           "/path/without/spaces/insecure_private_key",
+		IdentitiesOnly:         true,
+		LogLevel:               "FATAL"}
+
+	config := &Config{}
+	state := new(multistep.BasicStateBag)
+	state.Put("driver", driver)
+	state.Put("config", config)
+
+	step := StepSSHConfig{}
+	_ = step.Run(context.Background(), state)
+	expected := "/path/without/spaces/insecure_private_key"
+	if config.Comm.SSHPrivateKeyFile != expected {
+		t.Fatalf("Bad config private key. Recieved: %s; expected: %s.", config.Comm.SSHPrivateKeyFile, expected)
+	}
+}