diff --git a/communicator/ssh/communicator.go b/communicator/ssh/communicator.go index a6a7f9cf9..380ba4951 100644 --- a/communicator/ssh/communicator.go +++ b/communicator/ssh/communicator.go @@ -46,8 +46,8 @@ type Config struct { // Pty, if true, will request a pty from the remote end. Pty bool - // DisableAgent, if true, will not forward the SSH agent. - DisableAgent bool + // DisableAgentForwarding, if true, will not forward the SSH agent. + DisableAgentForwarding bool // HandshakeTimeout limits the amount of time we'll wait to handshake before // saying the connection failed. @@ -327,7 +327,7 @@ func (c *comm) connectToAgent() { return } - if c.config.DisableAgent { + if c.config.DisableAgentForwarding { log.Printf("[INFO] SSH agent forwarding is disabled.") return } diff --git a/fix/fixer.go b/fix/fixer.go index 3c285c4be..2e55f4d69 100644 --- a/fix/fixer.go +++ b/fix/fixer.go @@ -29,6 +29,7 @@ func init() { "parallels-headless": new(FixerParallelsHeadless), "parallels-deprecations": new(FixerParallelsDeprecations), "sshkeypath": new(FixerSSHKeyPath), + "sshdisableagent": new(FixerSSHDisableAgent), "manifest-filename": new(FixerManifestFilename), "amazon-shutdown_behavior": new(FixerAmazonShutdownBehavior), } @@ -43,6 +44,7 @@ func init() { "parallels-headless", "parallels-deprecations", "sshkeypath", + "sshdisableagent", "manifest-filename", "amazon-shutdown_behavior", } diff --git a/fix/fixer_sshdisableagent.go b/fix/fixer_sshdisableagent.go new file mode 100644 index 000000000..b399af52a --- /dev/null +++ b/fix/fixer_sshdisableagent.go @@ -0,0 +1,50 @@ +package fix + +import ( + "github.com/mitchellh/mapstructure" +) + +// FixerSSHDisableAgent changes the "ssh_disable_agent" of a template +// to "ssh_disable_agent_forwarding". +type FixerSSHDisableAgent struct{} + +func (FixerSSHDisableAgent) 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 { + sshDisableAgentRaw, ok := builder["ssh_disable_agent"] + if !ok { + continue + } + + sshDisableAgent, ok := sshDisableAgentRaw.(bool) + if !ok { + continue + } + + // only assign to ssh_disable_agent_forwarding if it doesn't + // already exist; otherwise we'll just ignore ssh_disable_agent + _, sshDisableAgentIncluded := builder["ssh_disable_agent_forwarding"] + if !sshDisableAgentIncluded { + builder["ssh_disable_agent_forwarding"] = sshDisableAgent + } + + delete(builder, "ssh_disable_agent") + } + + input["builders"] = tpl.Builders + return input, nil +} + +func (FixerSSHDisableAgent) Synopsis() string { + return `Updates builders using "ssh_disable_agent" to use "ssh_disable_agent_forwarding"` +} diff --git a/fix/fixer_sshdisableagent_test.go b/fix/fixer_sshdisableagent_test.go new file mode 100644 index 000000000..c38f9c581 --- /dev/null +++ b/fix/fixer_sshdisableagent_test.go @@ -0,0 +1,83 @@ +package fix + +import ( + "reflect" + "testing" +) + +func TestFixerSSHDisableAgent_Impl(t *testing.T) { + var _ Fixer = new(FixerSSHDisableAgent) +} + +func TestFixerSSHDisableAgent_Fix(t *testing.T) { + cases := []struct { + Input map[string]interface{} + Expected map[string]interface{} + }{ + // No disable_agent field + { + Input: map[string]interface{}{ + "type": "virtualbox", + }, + + Expected: map[string]interface{}{ + "type": "virtualbox", + }, + }, + + // disable_agent_forwarding without disable_agent + { + Input: map[string]interface{}{ + "ssh_disable_agent_forwarding": true, + }, + + Expected: map[string]interface{}{ + "ssh_disable_agent_forwarding": true, + }, + }, + + // disable_agent without disable_agent_forwarding + { + Input: map[string]interface{}{ + "ssh_disable_agent": true, + }, + + Expected: map[string]interface{}{ + "ssh_disable_agent_forwarding": true, + }, + }, + + // disable_agent and disable_agent_forwarding + { + Input: map[string]interface{}{ + "ssh_disable_agent": true, + "ssh_disable_agent_forwarding": false, + }, + + Expected: map[string]interface{}{ + "ssh_disable_agent_forwarding": false, + }, + }, + } + + for _, tc := range cases { + var f FixerSSHDisableAgent + + 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) + } + } +} diff --git a/helper/communicator/config.go b/helper/communicator/config.go index a39262abb..243ca224d 100644 --- a/helper/communicator/config.go +++ b/helper/communicator/config.go @@ -16,23 +16,23 @@ type Config struct { Type string `mapstructure:"communicator"` // SSH - SSHHost string `mapstructure:"ssh_host"` - SSHPort int `mapstructure:"ssh_port"` - SSHUsername string `mapstructure:"ssh_username"` - SSHPassword string `mapstructure:"ssh_password"` - SSHPrivateKey string `mapstructure:"ssh_private_key_file"` - SSHPty bool `mapstructure:"ssh_pty"` - SSHTimeout time.Duration `mapstructure:"ssh_timeout"` - SSHAgentAuth bool `mapstructure:"ssh_agent_auth"` - SSHDisableAgent bool `mapstructure:"ssh_disable_agent"` - SSHHandshakeAttempts int `mapstructure:"ssh_handshake_attempts"` - SSHBastionHost string `mapstructure:"ssh_bastion_host"` - SSHBastionPort int `mapstructure:"ssh_bastion_port"` - SSHBastionAgentAuth bool `mapstructure:"ssh_bastion_agent_auth"` - SSHBastionUsername string `mapstructure:"ssh_bastion_username"` - SSHBastionPassword string `mapstructure:"ssh_bastion_password"` - SSHBastionPrivateKey string `mapstructure:"ssh_bastion_private_key_file"` - SSHFileTransferMethod string `mapstructure:"ssh_file_transfer_method"` + SSHHost string `mapstructure:"ssh_host"` + SSHPort int `mapstructure:"ssh_port"` + SSHUsername string `mapstructure:"ssh_username"` + SSHPassword string `mapstructure:"ssh_password"` + SSHPrivateKey string `mapstructure:"ssh_private_key_file"` + SSHPty bool `mapstructure:"ssh_pty"` + SSHTimeout time.Duration `mapstructure:"ssh_timeout"` + SSHAgentAuth bool `mapstructure:"ssh_agent_auth"` + SSHDisableAgentForwarding bool `mapstructure:"ssh_disable_agent_forwarding"` + SSHHandshakeAttempts int `mapstructure:"ssh_handshake_attempts"` + SSHBastionHost string `mapstructure:"ssh_bastion_host"` + SSHBastionPort int `mapstructure:"ssh_bastion_port"` + SSHBastionAgentAuth bool `mapstructure:"ssh_bastion_agent_auth"` + SSHBastionUsername string `mapstructure:"ssh_bastion_username"` + SSHBastionPassword string `mapstructure:"ssh_bastion_password"` + SSHBastionPrivateKey string `mapstructure:"ssh_bastion_private_key_file"` + SSHFileTransferMethod string `mapstructure:"ssh_file_transfer_method"` // WinRM WinRMUser string `mapstructure:"winrm_username"` diff --git a/helper/communicator/step_connect_ssh.go b/helper/communicator/step_connect_ssh.go index 2daee1be6..669d52410 100644 --- a/helper/communicator/step_connect_ssh.go +++ b/helper/communicator/step_connect_ssh.go @@ -160,11 +160,11 @@ func (s *StepConnectSSH) waitForSSH(state multistep.StateBag, cancel <-chan stru // Then we attempt to connect via SSH config := &ssh.Config{ - Connection: connFunc, - SSHConfig: sshConfig, - Pty: s.Config.SSHPty, - DisableAgent: s.Config.SSHDisableAgent, - UseSftp: s.Config.SSHFileTransferMethod == "sftp", + Connection: connFunc, + SSHConfig: sshConfig, + Pty: s.Config.SSHPty, + DisableAgentForwarding: s.Config.SSHDisableAgentForwarding, + UseSftp: s.Config.SSHFileTransferMethod == "sftp", } log.Println("[INFO] Attempting SSH connection...") diff --git a/website/source/docs/templates/communicator.html.md b/website/source/docs/templates/communicator.html.md index 07d6229ed..07304f7d6 100644 --- a/website/source/docs/templates/communicator.html.md +++ b/website/source/docs/templates/communicator.html.md @@ -76,8 +76,8 @@ The SSH communicator has the following options: - `ssh_bastion_username` (string) - The username to connect to the bastion host. -- `ssh_disable_agent` (boolean) - If true, SSH agent forwarding will be - disabled. Defaults to false. +- `ssh_disable_agent_forwarding` (boolean) - If true, SSH agent forwarding + will be disabled. Defaults to false. - `ssh_file_transfer_method` (`scp` or `sftp`) - How to transfer files, Secure copy (default) or SSH File Transfer Protocol.