Merge pull request #3193 from mitchellh/f-ssh-key-path

Add fixer for SSHKeyPath

builder/parallels/common/ssh.go

@@ -32,7 +32,7 @@ func SSHConfigFunc(config SSHConfig) func(multistep.StateBag) (*ssh.ClientConfig
 				packerssh.PasswordKeyboardInteractive(config.Comm.SSHPassword)),
 		}
 
-		if config.SSHKeyPath != "" {
+		if config.Comm.SSHPrivateKey != "" {
 			signer, err := commonssh.FileSigner(config.Comm.SSHPrivateKey)
 			if err != nil {
 				return nil, err

builder/parallels/common/ssh_config.go

@@ -12,15 +12,11 @@ type SSHConfig struct {
 
 	// These are deprecated, but we keep them around for BC
 	// TODO(@mitchellh): remove
-	SSHKeyPath     string        `mapstructure:"ssh_key_path"`
 	SSHWaitTimeout time.Duration `mapstructure:"ssh_wait_timeout"`
 }
 
 func (c *SSHConfig) Prepare(ctx *interpolate.Context) []error {
 	// TODO: backwards compatibility, write fixer instead
-	if c.SSHKeyPath != "" {
-		c.Comm.SSHPrivateKey = c.SSHKeyPath
-	}
 	if c.SSHWaitTimeout != 0 {
 		c.Comm.SSHTimeout = c.SSHWaitTimeout
 	}

builder/parallels/common/ssh_config_test.go

@@ -28,19 +28,19 @@ func TestSSHConfigPrepare(t *testing.T) {
 	}
 }
 
-func TestSSHConfigPrepare_SSHKeyPath(t *testing.T) {
+func TestSSHConfigPrepare_SSHPrivateKey(t *testing.T) {
 	var c *SSHConfig
 	var errs []error
 
 	c = testSSHConfig()
-	c.SSHKeyPath = ""
+	c.Comm.SSHPrivateKey = ""
 	errs = c.Prepare(testConfigTemplate(t))
 	if len(errs) > 0 {
 		t.Fatalf("should not have error: %#v", errs)
 	}
 
 	c = testSSHConfig()
-	c.SSHKeyPath = "/i/dont/exist"
+	c.Comm.SSHPrivateKey = "/i/dont/exist"
 	errs = c.Prepare(testConfigTemplate(t))
 	if len(errs) == 0 {
 		t.Fatal("should have error")
@@ -59,7 +59,7 @@ func TestSSHConfigPrepare_SSHKeyPath(t *testing.T) {
 	}
 
 	c = testSSHConfig()
-	c.SSHKeyPath = tf.Name()
+	c.Comm.SSHPrivateKey = tf.Name()
 	errs = c.Prepare(testConfigTemplate(t))
 	if len(errs) == 0 {
 		t.Fatal("should have error")
@@ -70,7 +70,7 @@ func TestSSHConfigPrepare_SSHKeyPath(t *testing.T) {
 	tf.Truncate(0)
 	tf.Write([]byte(testPem))
 	c = testSSHConfig()
-	c.SSHKeyPath = tf.Name()
+	c.Comm.SSHPrivateKey = tf.Name()
 	errs = c.Prepare(testConfigTemplate(t))
 	if len(errs) > 0 {
 		t.Fatalf("should not have error: %#v", errs)

builder/qemu/builder.go

@@ -110,7 +110,6 @@ type Config struct {
 
 	// These are deprecated, but we keep them around for BC
 	// TODO(@mitchellh): remove
-	SSHKeyPath     string        `mapstructure:"ssh_key_path"`
 	SSHWaitTimeout time.Duration `mapstructure:"ssh_wait_timeout"`
 
 	// TODO(mitchellh): deprecate
@@ -212,9 +211,6 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 	}
 
 	// TODO: backwards compatibility, write fixer instead
-	if b.config.SSHKeyPath != "" {
-		b.config.Comm.SSHPrivateKey = b.config.SSHKeyPath
-	}
 	if b.config.SSHWaitTimeout != 0 {
 		b.config.Comm.SSHTimeout = b.config.SSHWaitTimeout
 	}

builder/qemu/builder_test.go

@@ -357,11 +357,11 @@ func TestBuilderPrepare_SSHHostPort(t *testing.T) {
 	}
 }
 
-func TestBuilderPrepare_sshKeyPath(t *testing.T) {
+func TestBuilderPrepare_SSHPrivateKey(t *testing.T) {
 	var b Builder
 	config := testConfig()
 
-	config["ssh_key_path"] = ""
+	config["ssh_private_key_file"] = ""
 	b = Builder{}
 	warns, err := b.Prepare(config)
 	if len(warns) > 0 {
@@ -371,7 +371,7 @@ func TestBuilderPrepare_sshKeyPath(t *testing.T) {
 		t.Fatalf("should not have error: %s", err)
 	}
 
-	config["ssh_key_path"] = "/i/dont/exist"
+	config["ssh_private_key_file"] = "/i/dont/exist"
 	b = Builder{}
 	warns, err = b.Prepare(config)
 	if len(warns) > 0 {
@@ -393,7 +393,7 @@ func TestBuilderPrepare_sshKeyPath(t *testing.T) {
 		t.Fatalf("err: %s", err)
 	}
 
-	config["ssh_key_path"] = tf.Name()
+	config["ssh_private_key_file"] = tf.Name()
 	b = Builder{}
 	warns, err = b.Prepare(config)
 	if len(warns) > 0 {
@@ -407,7 +407,7 @@ func TestBuilderPrepare_sshKeyPath(t *testing.T) {
 	tf.Seek(0, 0)
 	tf.Truncate(0)
 	tf.Write([]byte(testPem))
-	config["ssh_key_path"] = tf.Name()
+	config["ssh_private_key_file"] = tf.Name()
 	b = Builder{}
 	warns, err = b.Prepare(config)
 	if len(warns) > 0 {

builder/virtualbox/common/ssh.go

@@ -24,7 +24,7 @@ func SSHConfigFunc(config SSHConfig) func(multistep.StateBag) (*gossh.ClientConf
 				ssh.PasswordKeyboardInteractive(config.Comm.SSHPassword)),
 		}
 
-		if config.SSHKeyPath != "" {
+		if config.Comm.SSHPrivateKey != "" {
 			signer, err := commonssh.FileSigner(config.Comm.SSHPrivateKey)
 			if err != nil {
 				return nil, err

builder/virtualbox/common/ssh_config.go

@@ -17,7 +17,6 @@ type SSHConfig struct {
 
 	// These are deprecated, but we keep them around for BC
 	// TODO(@mitchellh): remove
-	SSHKeyPath     string        `mapstructure:"ssh_key_path"`
 	SSHWaitTimeout time.Duration `mapstructure:"ssh_wait_timeout"`
 }
 
@@ -31,9 +30,6 @@ func (c *SSHConfig) Prepare(ctx *interpolate.Context) []error {
 	}
 
 	// TODO: backwards compatibility, write fixer instead
-	if c.SSHKeyPath != "" {
-		c.Comm.SSHPrivateKey = c.SSHKeyPath
-	}
 	if c.SSHWaitTimeout != 0 {
 		c.Comm.SSHTimeout = c.SSHWaitTimeout
 	}

builder/virtualbox/common/ssh_config_test.go

@@ -59,19 +59,19 @@ func TestSSHConfigPrepare_SSHHostPort(t *testing.T) {
 	}
 }
 
-func TestSSHConfigPrepare_SSHKeyPath(t *testing.T) {
+func TestSSHConfigPrepare_SSHPrivateKey(t *testing.T) {
 	var c *SSHConfig
 	var errs []error
 
 	c = testSSHConfig()
-	c.SSHKeyPath = ""
+	c.Comm.SSHPrivateKey = ""
 	errs = c.Prepare(testConfigTemplate(t))
 	if len(errs) > 0 {
 		t.Fatalf("should not have error: %#v", errs)
 	}
 
 	c = testSSHConfig()
-	c.SSHKeyPath = "/i/dont/exist"
+	c.Comm.SSHPrivateKey = "/i/dont/exist"
 	errs = c.Prepare(testConfigTemplate(t))
 	if len(errs) == 0 {
 		t.Fatal("should have error")
@@ -90,7 +90,7 @@ func TestSSHConfigPrepare_SSHKeyPath(t *testing.T) {
 	}
 
 	c = testSSHConfig()
-	c.SSHKeyPath = tf.Name()
+	c.Comm.SSHPrivateKey = tf.Name()
 	errs = c.Prepare(testConfigTemplate(t))
 	if len(errs) == 0 {
 		t.Fatal("should have error")
@@ -101,7 +101,7 @@ func TestSSHConfigPrepare_SSHKeyPath(t *testing.T) {
 	tf.Truncate(0)
 	tf.Write([]byte(testPem))
 	c = testSSHConfig()
-	c.SSHKeyPath = tf.Name()
+	c.Comm.SSHPrivateKey = tf.Name()
 	errs = c.Prepare(testConfigTemplate(t))
 	if len(errs) > 0 {
 		t.Fatalf("should not have error: %#v", errs)

builder/vmware/common/ssh_config.go

@@ -12,16 +12,12 @@ type SSHConfig struct {
 
 	// These are deprecated, but we keep them around for BC
 	// TODO(@mitchellh): remove
-	SSHKeyPath        string        `mapstructure:"ssh_key_path"`
 	SSHSkipRequestPty bool          `mapstructure:"ssh_skip_request_pty"`
 	SSHWaitTimeout    time.Duration `mapstructure:"ssh_wait_timeout"`
 }
 
 func (c *SSHConfig) Prepare(ctx *interpolate.Context) []error {
 	// TODO: backwards compatibility, write fixer instead
-	if c.SSHKeyPath != "" {
-		c.Comm.SSHPrivateKey = c.SSHKeyPath
-	}
 	if c.SSHWaitTimeout != 0 {
 		c.Comm.SSHTimeout = c.SSHWaitTimeout
 	}

builder/vmware/common/ssh_config_test.go

@@ -28,19 +28,19 @@ func TestSSHConfigPrepare(t *testing.T) {
 	}
 }
 
-func TestSSHConfigPrepare_SSHKeyPath(t *testing.T) {
+func TestSSHConfigPrepare_SSHPrivateKey(t *testing.T) {
 	var c *SSHConfig
 	var errs []error
 
 	c = testSSHConfig()
-	c.SSHKeyPath = ""
+	c.Comm.SSHPrivateKey = ""
 	errs = c.Prepare(testConfigTemplate(t))
 	if len(errs) > 0 {
 		t.Fatalf("should not have error: %#v", errs)
 	}
 
 	c = testSSHConfig()
-	c.SSHKeyPath = "/i/dont/exist"
+	c.Comm.SSHPrivateKey = "/i/dont/exist"
 	errs = c.Prepare(testConfigTemplate(t))
 	if len(errs) == 0 {
 		t.Fatal("should have error")
@@ -59,7 +59,7 @@ func TestSSHConfigPrepare_SSHKeyPath(t *testing.T) {
 	}
 
 	c = testSSHConfig()
-	c.SSHKeyPath = tf.Name()
+	c.Comm.SSHPrivateKey = tf.Name()
 	errs = c.Prepare(testConfigTemplate(t))
 	if len(errs) == 0 {
 		t.Fatal("should have error")
@@ -70,7 +70,7 @@ func TestSSHConfigPrepare_SSHKeyPath(t *testing.T) {
 	tf.Truncate(0)
 	tf.Write([]byte(testPem))
 	c = testSSHConfig()
-	c.SSHKeyPath = tf.Name()
+	c.Comm.SSHPrivateKey = tf.Name()
 	errs = c.Prepare(testConfigTemplate(t))
 	if len(errs) > 0 {
 		t.Fatalf("should not have error: %#v", errs)

fix/fixer.go

@@ -28,6 +28,7 @@ func init() {
 		"vmware-rename":          new(FixerVMwareRename),
 		"parallels-headless":     new(FixerParallelsHeadless),
 		"parallels-deprecations": new(FixerParallelsDeprecations),
+		"sshkeypath":             new(FixerSSHKeyPath),
 	}
 
 	FixerOrder = []string{
@@ -39,5 +40,6 @@ func init() {
 		"vmware-rename",
 		"parallels-headless",
 		"parallels-deprecations",
+		"sshkeypath",
 	}
 }

fix/fixer_sshkeypath.go

@@ -0,0 +1,50 @@
+package fix
+
+import (
+	"github.com/mitchellh/mapstructure"
+)
+
+// FixerSSHKeyPath changes the "ssh_key_path" of a template
+// to "ssh_private_key_file".
+type FixerSSHKeyPath struct{}
+
+func (FixerSSHKeyPath) Fix(input map[string]interface{}) (map[string]interface{}, error) {
+	// The type we'll decode into; we only care about builders
+	type template struct {
+		Builders []map[string]interface{}
+	}
+
+	// Decode the input into our structure, if we can
+	var tpl template
+	if err := mapstructure.Decode(input, &tpl); err != nil {
+		return nil, err
+	}
+
+	for _, builder := range tpl.Builders {
+		sshKeyPathRaw, ok := builder["ssh_key_path"]
+		if !ok {
+			continue
+		}
+
+		sshKeyPath, ok := sshKeyPathRaw.(string)
+		if !ok {
+			continue
+		}
+
+		// only assign to ssh_private_key_file if it doesn't
+		// already exist; otherwise we'll just ignore ssh_key_path
+		_, sshPrivateIncluded := builder["ssh_private_key_file"]
+		if !sshPrivateIncluded {
+			builder["ssh_private_key_file"] = sshKeyPath
+		}
+
+		delete(builder, "ssh_key_path")
+	}
+
+	input["builders"] = tpl.Builders
+	return input, nil
+}
+
+func (FixerSSHKeyPath) Synopsis() string {
+	return `Updates builders using "ssh_key_path" to use "ssh_private_key_file"`
+}

fix/fixer_sshkeypath_test.go

@@ -0,0 +1,83 @@
+package fix
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestFixerSSHKeyPath_Impl(t *testing.T) {
+	var _ Fixer = new(FixerSSHKeyPath)
+}
+
+func TestFixerSSHKeyPath_Fix(t *testing.T) {
+	cases := []struct {
+		Input    map[string]interface{}
+		Expected map[string]interface{}
+	}{
+		// No key_path field
+		{
+			Input: map[string]interface{}{
+				"type": "virtualbox",
+			},
+
+			Expected: map[string]interface{}{
+				"type": "virtualbox",
+			},
+		},
+
+		// private_key_file without key_path
+		{
+			Input: map[string]interface{}{
+				"ssh_private_key_file": "id_rsa",
+			},
+
+			Expected: map[string]interface{}{
+				"ssh_private_key_file": "id_rsa",
+			},
+		},
+
+		// key_path without private_key_file
+		{
+			Input: map[string]interface{}{
+				"ssh_key_path": "id_rsa",
+			},
+
+			Expected: map[string]interface{}{
+				"ssh_private_key_file": "id_rsa",
+			},
+		},
+
+		// key_path and private_key_file
+		{
+			Input: map[string]interface{}{
+				"ssh_key_path":         "key_id_rsa",
+				"ssh_private_key_file": "private_id_rsa",
+			},
+
+			Expected: map[string]interface{}{
+				"ssh_private_key_file": "private_id_rsa",
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		var f FixerSSHKeyPath
+
+		input := map[string]interface{}{
+			"builders": []map[string]interface{}{tc.Input},
+		}
+
+		expected := map[string]interface{}{
+			"builders": []map[string]interface{}{tc.Expected},
+		}
+
+		output, err := f.Fix(input)
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
+
+		if !reflect.DeepEqual(output, expected) {
+			t.Fatalf("unexpected: %#v\nexpected: %#v\n", output, expected)
+		}
+	}
+}