From da223b953966425f5177089a05db18ec5f7bab6d Mon Sep 17 00:00:00 2001
From: "Billie H. Cleek" <bhcleek@gmail.com>
Date: Sun, 28 Aug 2016 22:22:48 -0700
Subject: [PATCH 1/3] add scp support to ansible provisioner

Handle running `scp -t` and `scp -f` exec requests in the
ansible-provisioner's SSH
server to allow Ansible to use SCP so that SFTP doesn't have to be
installed on the node.

Update the BATS tests to test the ansible provisioner.
---
 provisioner/ansible/adapter.go                |  78 ++--
 provisioner/ansible/scp.go                    | 338 ++++++++++++++++++
 .../provisioner-ansible/all_options.json      |   4 +-
 .../fixtures/provisioner-ansible/minimal.json |   1 -
 .../fixtures/provisioner-ansible/playbook.yml |   2 +-
 test/fixtures/provisioner-ansible/scp.json    |  24 ++
 test/fixtures/provisioner-ansible/sftp.json   |  29 ++
 test/provisioner_ansible.bats                 |  14 +
 .../source/docs/provisioners/ansible.html.md  |   9 +-
 9 files changed, 471 insertions(+), 28 deletions(-)
 create mode 100644 provisioner/ansible/scp.go
 create mode 100644 test/fixtures/provisioner-ansible/scp.json
 create mode 100644 test/fixtures/provisioner-ansible/sftp.json

diff --git a/provisioner/ansible/adapter.go b/provisioner/ansible/adapter.go
index 8dc0f72f4..15cf886b7 100644
--- a/provisioner/ansible/adapter.go
+++ b/provisioner/ansible/adapter.go
@@ -8,11 +8,14 @@ import (
 	"io"
 	"log"
 	"net"
+	"strings"
 
 	"github.com/mitchellh/packer/packer"
 	"golang.org/x/crypto/ssh"
 )
 
+// An adapter satisfies SSH requests (from an Ansible client) by delegating SSH
+// exec and subsystem commands to a packer.Communicator.
 type adapter struct {
 	done    <-chan struct{}
 	l       net.Listener
@@ -132,29 +135,15 @@ func (c *adapter) handleSession(newChannel ssh.NewChannel) error {
 					return
 				}
 
-				cmd := &packer.RemoteCmd{
-					Stdin:   channel,
-					Stdout:  channel,
-					Stderr:  channel.Stderr(),
-					Command: string(req.Payload),
-				}
+				go func(channel ssh.Channel) {
+					exit := c.exec(string(req.Payload), channel, channel, channel.Stderr())
 
-				if err := c.comm.Start(cmd); err != nil {
-					c.ui.Error(err.Error())
-					req.Reply(false, nil)
-					close(done)
-					return
-				}
-
-				go func(cmd *packer.RemoteCmd, channel ssh.Channel) {
-					cmd.Wait()
 					exitStatus := make([]byte, 4)
-					binary.BigEndian.PutUint32(exitStatus, uint32(cmd.ExitStatus))
+					binary.BigEndian.PutUint32(exitStatus, uint32(exit))
 					channel.SendRequest("exit-status", false, exitStatus)
 					close(done)
-				}(cmd, channel)
+				}(channel)
 				req.Reply(true, nil)
-
 			case "subsystem":
 				req, err := newSubsystemRequest(req)
 				if err != nil {
@@ -190,11 +179,9 @@ func (c *adapter) handleSession(newChannel ssh.NewChannel) error {
 						cmd.Wait()
 						close(done)
 					}()
-
 				default:
 					c.ui.Error(fmt.Sprintf("unsupported subsystem requested: %s", req.Payload))
 					req.Reply(false, nil)
-
 				}
 			default:
 				c.ui.Message(fmt.Sprintf("rejecting %s request", req.Type))
@@ -211,6 +198,57 @@ func (c *adapter) Shutdown() {
 	c.l.Close()
 }
 
+func (c *adapter) exec(command string, in io.Reader, out io.Writer, err io.Writer) int {
+	var exitStatus int
+	switch {
+	case strings.HasPrefix(command, "scp ") && serveSCP(command[4:]):
+		err := c.scpExec(command[4:], in, out, err)
+		if err != nil {
+			log.Println(err)
+			exitStatus = 1
+		}
+	default:
+		exitStatus = c.remoteExec(command, in, out, err)
+	}
+	return exitStatus
+}
+
+func serveSCP(args string) bool {
+	opts, _ := scpOptions(args)
+	return bytes.IndexAny(opts, "tf") >= 0
+}
+
+func (c *adapter) scpExec(args string, in io.Reader, out io.Writer, err io.Writer) error {
+	opts, rest := scpOptions(args)
+
+	if i := bytes.IndexByte(opts, 't'); i >= 0 {
+		return scpUploadSession(opts, rest, in, out, c.comm)
+	}
+
+	if i := bytes.IndexByte(opts, 'f'); i >= 0 {
+		return scpDownloadSession(opts, rest, in, out, c.comm)
+	}
+	return errors.New("no scp mode specified")
+}
+
+func (c *adapter) remoteExec(command string, in io.Reader, out io.Writer, err io.Writer) int {
+	cmd := &packer.RemoteCmd{
+		Stdin:   in,
+		Stdout:  out,
+		Stderr:  err,
+		Command: command,
+	}
+
+	if err := c.comm.Start(cmd); err != nil {
+		c.ui.Error(err.Error())
+		return cmd.ExitStatus
+	}
+
+	cmd.Wait()
+
+	return cmd.ExitStatus
+}
+
 type envRequest struct {
 	*ssh.Request
 	Payload envRequestPayload
diff --git a/provisioner/ansible/scp.go b/provisioner/ansible/scp.go
new file mode 100644
index 000000000..21ccf1896
--- /dev/null
+++ b/provisioner/ansible/scp.go
@@ -0,0 +1,338 @@
+package ansible
+
+import (
+	"bufio"
+	"bytes"
+	"errors"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"log"
+	"os"
+	"path/filepath"
+	"strings"
+	"time"
+
+	"github.com/mitchellh/packer/packer"
+)
+
+const (
+	scpOK         = "\x00"
+	scpEmptyError = "\x02\n"
+)
+
+/*
+scp is a simple, but poorly documented, protocol. Thankfully, its source is
+freely available, and there is at least one page that describes it reasonably
+well.
+
+* https://raw.githubusercontent.com/openssh/openssh-portable/master/scp.c
+* https://opensource.apple.com/source/OpenSSH/OpenSSH-7.1/openssh/scp.c
+* https://blogs.oracle.com/janp/entry/how_the_scp_protocol_works is a great
+	resource, but has some bad information. Its first problem is that it doesn't
+	correctly describe why the producer has to read more responses than messages
+	it sends (because it has to read the 0 sent by the sink to start the
+	transfer). The second problem is that it omits that the producer needs to
+	send a 0 byte after file contents.
+*/
+
+func scpUploadSession(opts []byte, rest string, in io.Reader, out io.Writer, comm packer.Communicator) error {
+	rest = strings.TrimSpace(rest)
+	if len(rest) == 0 {
+		fmt.Fprintf(out, scpEmptyError)
+		return errors.New("no scp target specified")
+	}
+
+	d, err := ioutil.TempDir("", "packer-ansible-upload")
+	if err != nil {
+		fmt.Fprintf(out, scpEmptyError)
+		return err
+	}
+	defer os.RemoveAll(d)
+
+	state := &scpUploadState{destRoot: rest, srcRoot: d, comm: comm}
+
+	fmt.Fprintf(out, scpOK) // signal the client to start the transfer.
+	return state.Protocol(bufio.NewReader(in), out)
+}
+
+func scpDownloadSession(opts []byte, rest string, in io.Reader, out io.Writer, comm packer.Communicator) error {
+	rest = strings.TrimSpace(rest)
+	if len(rest) == 0 {
+		fmt.Fprintf(out, scpEmptyError)
+		return errors.New("no scp source specified")
+	}
+
+	d, err := ioutil.TempDir("", "packer-ansible-download")
+	if err != nil {
+		fmt.Fprintf(out, scpEmptyError)
+		return err
+	}
+	defer os.RemoveAll(d)
+
+	if bytes.Contains([]byte{'d'}, opts) {
+		// the only ansible module that supports downloading via scp is fetch,
+		// fetch only supports file downloads as of Ansible 2.1.
+		fmt.Fprintf(out, scpEmptyError)
+		return errors.New("directory downloads not supported")
+	}
+
+	f, err := os.Create(filepath.Join(d, filepath.Base(rest)))
+	if err != nil {
+		fmt.Fprintf(out, scpEmptyError)
+		return err
+	}
+	defer f.Close()
+
+	err = comm.Download(rest, f)
+	if err != nil {
+		fmt.Fprintf(out, scpEmptyError)
+		return err
+	}
+
+	state := &scpDownloadState{srcRoot: d}
+
+	return state.Protocol(bufio.NewReader(in), out)
+}
+
+func (state *scpDownloadState) FileProtocol(path string, info os.FileInfo, in *bufio.Reader, out io.Writer) error {
+	size := info.Size()
+	perms := fmt.Sprintf("C%04o", info.Mode().Perm())
+	fmt.Fprintln(out, perms, size, info.Name())
+	err := scpResponse(in)
+	if err != nil {
+		return err
+	}
+
+	f, err := os.Open(path)
+	if err != nil {
+		return err
+	}
+	defer f.Close()
+
+	io.CopyN(out, f, size)
+	fmt.Fprintf(out, scpOK)
+
+	return scpResponse(in)
+}
+
+type scpUploadState struct {
+	comm     packer.Communicator
+	destRoot string // destRoot is the directory on the target
+	srcRoot  string // srcRoot is the directory on the host
+	mtime    time.Time
+	atime    time.Time
+	dir      string // dir is a path relative to the roots
+}
+
+func (scp scpUploadState) DestPath() string {
+	return filepath.Join(scp.destRoot, scp.dir)
+}
+
+func (scp scpUploadState) SrcPath() string {
+	return filepath.Join(scp.srcRoot, scp.dir)
+}
+
+func (state *scpUploadState) Protocol(in *bufio.Reader, out io.Writer) error {
+	for {
+		b, err := in.ReadByte()
+		if err != nil {
+			return err
+		}
+		switch b {
+		case 'T':
+			err := state.TimeProtocol(in, out)
+			if err != nil {
+				return err
+			}
+		case 'C':
+			return state.FileProtocol(in, out)
+		case 'E':
+			state.dir = filepath.Dir(state.dir)
+			fmt.Fprintf(out, scpOK)
+			return nil
+		case 'D':
+			return state.DirProtocol(in, out)
+		default:
+			fmt.Fprintf(out, scpEmptyError)
+			return fmt.Errorf("unexpected message: %c", b)
+		}
+	}
+}
+
+func (state *scpUploadState) FileProtocol(in *bufio.Reader, out io.Writer) error {
+	defer func() {
+		state.mtime = time.Time{}
+	}()
+
+	var mode os.FileMode
+	var size int64
+	var name string
+	_, err := fmt.Fscanf(in, "%04o %d %s\n", &mode, &size, &name)
+	if err != nil {
+		fmt.Fprintf(out, scpEmptyError)
+		return fmt.Errorf("invalid file message: %v", err)
+	}
+	fmt.Fprintf(out, scpOK)
+
+	var fi os.FileInfo = fileInfo{name: name, size: size, mode: mode, mtime: state.mtime}
+
+	err = state.comm.Upload(filepath.Join(state.DestPath(), fi.Name()), io.LimitReader(in, fi.Size()), &fi)
+	if err != nil {
+		fmt.Fprintf(out, scpEmptyError)
+		return err
+	}
+
+	err = scpResponse(in)
+	if err != nil {
+		return err
+	}
+
+	fmt.Fprintf(out, scpOK)
+	return nil
+}
+
+func (state *scpUploadState) TimeProtocol(in *bufio.Reader, out io.Writer) error {
+	var m, a int64
+	if _, err := fmt.Fscanf(in, "%d 0 %d 0\n", &m, &a); err != nil {
+		fmt.Fprintf(out, scpEmptyError)
+		return err
+	}
+	fmt.Fprintf(out, scpOK)
+
+	state.atime = time.Unix(a, 0)
+	state.mtime = time.Unix(m, 0)
+	return nil
+}
+
+func (state *scpUploadState) DirProtocol(in *bufio.Reader, out io.Writer) error {
+	var mode os.FileMode
+	var length uint
+	var name string
+
+	if _, err := fmt.Fscanf(in, "%04o %d %s\n", &mode, &length, &name); err != nil {
+		fmt.Fprintf(out, scpEmptyError)
+		return fmt.Errorf("invalid directory message: %v", err)
+	}
+	fmt.Fprintf(out, scpOK)
+
+	path := filepath.Join(state.dir, name)
+	if err := os.Mkdir(path, mode); err != nil {
+		return err
+	}
+	state.dir = path
+
+	if state.atime.IsZero() {
+		state.atime = time.Now()
+	}
+	if state.mtime.IsZero() {
+		state.mtime = time.Now()
+	}
+
+	if err := os.Chtimes(path, state.atime, state.mtime); err != nil {
+		return err
+	}
+
+	if err := state.comm.UploadDir(filepath.Dir(state.DestPath()), state.SrcPath(), nil); err != nil {
+		return err
+	}
+
+	state.mtime = time.Time{}
+	state.atime = time.Time{}
+	return state.Protocol(in, out)
+}
+
+type scpDownloadState struct {
+	srcRoot string // srcRoot is the directory on the host
+}
+
+func (state *scpDownloadState) Protocol(in *bufio.Reader, out io.Writer) error {
+	r := bufio.NewReader(in)
+	// read the byte sent by the other side to start the transfer
+	scpResponse(r)
+
+	return filepath.Walk(state.srcRoot, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		if path == state.srcRoot {
+			return nil
+		}
+
+		if info.IsDir() {
+			// no need to get fancy; srcRoot should only contain one file, because
+			// Ansible only allows fetching a single file.
+			return errors.New("unexpected directory")
+		}
+
+		return state.FileProtocol(path, info, r, out)
+	})
+}
+
+func scpOptions(s string) (opts []byte, rest string) {
+	end := 0
+	opt := false
+
+Loop:
+	for i := 0; i < len(s); i++ {
+		b := s[i]
+		switch {
+		case b == ' ':
+			opt = false
+			end++
+		case b == '-':
+			opt = true
+			end++
+		case opt:
+			opts = append(opts, b)
+			end++
+		default:
+			break Loop
+		}
+	}
+
+	rest = s[end:]
+	return
+}
+
+func scpResponse(r *bufio.Reader) error {
+	code, err := r.ReadByte()
+	if err != nil {
+		return err
+	}
+
+	if code != 0 {
+		message, err := r.ReadString('\n')
+		if err != nil {
+			return fmt.Errorf("Error reading error message: %s", err)
+		}
+
+		// 1 is a warning. Anything higher (really just 2) is an error.
+		if code > 1 {
+			return errors.New(string(message))
+		}
+
+		log.Println("WARNING:", err)
+	}
+	return nil
+}
+
+type fileInfo struct {
+	name  string
+	size  int64
+	mode  os.FileMode
+	mtime time.Time
+}
+
+func (fi fileInfo) Name() string      { return fi.name }
+func (fi fileInfo) Size() int64       { return fi.size }
+func (fi fileInfo) Mode() os.FileMode { return fi.mode }
+func (fi fileInfo) ModTime() time.Time {
+	if fi.mtime.IsZero() {
+		return time.Now()
+	}
+	return fi.mtime
+}
+func (fi fileInfo) IsDir() bool      { return fi.mode.IsDir() }
+func (fi fileInfo) Sys() interface{} { return nil }
diff --git a/test/fixtures/provisioner-ansible/all_options.json b/test/fixtures/provisioner-ansible/all_options.json
index 71b887ea6..6d74d65c8 100644
--- a/test/fixtures/provisioner-ansible/all_options.json
+++ b/test/fixtures/provisioner-ansible/all_options.json
@@ -3,7 +3,7 @@
     "provisioners": [
       {
         "type": "shell-local",
-        "command": "echo 'TODO(bc): write the public key to $HOME/.ssh/known_hosts and stop using ANSIBLE_HOST_KEY_CHECKING=False'"
+        "command": "echo 'TODO(bhcleek): write the public key to $HOME/.ssh/known_hosts and stop using ANSIBLE_HOST_KEY_CHECKING=False'"
       }, {
         "type": "shell",
         "inline": [
@@ -22,7 +22,7 @@
         "groups": ["PACKER_TEST"],
         "empty_groups": ["PACKER_EMPTY_GROUP"],
         "host_alias": "packer-test",
-        "user": "packer", 
+        "user": "packer",
         "local_port": 2222,
         "ssh_host_key_file": "ansible-server.key",
         "ssh_authorized_key_file": "ansible-test-id.pub"
diff --git a/test/fixtures/provisioner-ansible/minimal.json b/test/fixtures/provisioner-ansible/minimal.json
index 141aebb6d..0536228d9 100644
--- a/test/fixtures/provisioner-ansible/minimal.json
+++ b/test/fixtures/provisioner-ansible/minimal.json
@@ -11,7 +11,6 @@
         "type": "googlecompute",
         "account_file": "{{user `account_file`}}",
         "project_id": "{{user `project_id`}}",
-
         "image_name": "packerbats-minimal-{{timestamp}}",
         "source_image": "debian-7-wheezy-v20141108",
         "zone": "us-central1-a"
diff --git a/test/fixtures/provisioner-ansible/playbook.yml b/test/fixtures/provisioner-ansible/playbook.yml
index f0e20bb2a..fbe9cc6ac 100644
--- a/test/fixtures/provisioner-ansible/playbook.yml
+++ b/test/fixtures/provisioner-ansible/playbook.yml
@@ -6,7 +6,7 @@
     - raw: date
     - command: echo "the command module"
     - command: mkdir /tmp/remote-dir
-      args: 
+      args:
         creates: /tmp/remote-dir
     - copy: src=dir/file.txt dest=/tmp/remote-dir/file.txt
     - fetch: src=/tmp/remote-dir/file.txt dest=fetched-dir validate=yes fail_on_missing=yes
diff --git a/test/fixtures/provisioner-ansible/scp.json b/test/fixtures/provisioner-ansible/scp.json
new file mode 100644
index 000000000..21d72a3c4
--- /dev/null
+++ b/test/fixtures/provisioner-ansible/scp.json
@@ -0,0 +1,24 @@
+{
+    "variables": {},
+    "provisioners": [
+      {
+        "type":  "ansible",
+        "playbook_file": "./playbook.yml",
+        "extra_arguments": [
+          "-vvvv"
+        ],
+        "ansible_env_vars": ["ANSIBLE_SCP_IF_SSH=True"],
+        "sftp_command": "/usr/bin/false"
+      }
+		],
+    "builders": [
+      {
+        "type": "googlecompute",
+        "account_file": "{{user `account_file`}}",
+        "project_id": "{{user `project_id`}}",
+        "image_name": "packerbats-scp-{{timestamp}}",
+        "source_image": "debian-7-wheezy-v20141108",
+        "zone": "us-central1-a"
+      }
+    ]
+}
diff --git a/test/fixtures/provisioner-ansible/sftp.json b/test/fixtures/provisioner-ansible/sftp.json
new file mode 100644
index 000000000..4b2c73b34
--- /dev/null
+++ b/test/fixtures/provisioner-ansible/sftp.json
@@ -0,0 +1,29 @@
+{
+    "variables": {},
+    "provisioners": [
+      {
+        "type": "shell",
+        "inline": [
+          "apt-get update",
+          "apt-get -y install python openssh-sftp-server",
+          "ls -l /usr/lib",
+          "#/usr/lib/sftp-server -?"
+        ]
+      }, {
+        "type":  "ansible",
+        "playbook_file": "./playbook.yml",
+        "sftp_command": "/usr/lib/sftp-server -e -l INFO"
+      }
+		],
+    "builders": [
+      {
+        "type": "googlecompute",
+        "account_file": "{{user `account_file`}}",
+        "project_id": "{{user `project_id`}}",
+
+        "image_name": "packerbats-sftp-{{timestamp}}",
+        "source_image": "debian-7-wheezy-v20141108",
+        "zone": "us-central1-a"
+      }
+    ]
+}
diff --git a/test/provisioner_ansible.bats b/test/provisioner_ansible.bats
index 78acdaa2a..c25fc8012 100755
--- a/test/provisioner_ansible.bats
+++ b/test/provisioner_ansible.bats
@@ -56,3 +56,17 @@ teardown() {
     [ "$status" -eq 0 ]
     [ "$(gc_has_image "packerbats-alloptions")" -eq 1 ]
 }
+
+@test "ansible provisioner: build scp.json" {
+    cd $FIXTURE_ROOT
+    run packer build ${USER_VARS} $FIXTURE_ROOT/scp.json
+    [ "$status" -eq 0 ]
+    [ "$(gc_has_image "packerbats-scp")" -eq 1 ]
+}
+
+@test "ansible provisioner: build sftp.json" {
+    cd $FIXTURE_ROOT
+    run packer build ${USER_VARS} $FIXTURE_ROOT/sftp.json
+    [ "$status" -eq 0 ]
+    [ "$(gc_has_image "packerbats-sftp")" -eq 1 ]
+}
diff --git a/website/source/docs/provisioners/ansible.html.md b/website/source/docs/provisioners/ansible.html.md
index 10ad71fce..7c80b273a 100644
--- a/website/source/docs/provisioners/ansible.html.md
+++ b/website/source/docs/provisioners/ansible.html.md
@@ -78,6 +78,8 @@ Optional Parameters:
 - `sftp_command` (string) - The command to run on the machine being provisioned
   by Packer to handle the SFTP protocol that Ansible will use to transfer
   files. The command should read and write on stdin and stdout, respectively.
+  SCP can be used instead of SFTP by setting `ANSIBLE_SCP_IF_SSH=True` in
+  `ansible_env_vars`.
   Defaults to `/usr/lib/sftp-server -e`.
 
 - `extra_arguments` (array of strings) - Extra arguments to pass to Ansible.
@@ -87,8 +89,9 @@ Optional Parameters:
 "extra_arguments": [ "--extra-vars", "Region={{user `Region`}} Stage={{user `Stage`}}" ]
 ```
 
-- `ansible_env_vars` (array of strings) - Environment variables to set before running Ansible.
-  If unset, defaults to `ANSIBLE_HOST_KEY_CHECKING=False`.
+- `ansible_env_vars` (array of strings) - Environment variables to set before
+  running Ansible.  If unset, defaults to `ANSIBLE_HOST_KEY_CHECKING=False`.
+  Set `ANSIBLE_SCP_IF_SSH=True` to use SCP instead of SFTP.
   Usage example:
 
 ```
@@ -100,8 +103,6 @@ Optional Parameters:
 
 ## Limitations
 
-- The `ansible` provisioner does not support SCP to transfer files.
-
 - Redhat / CentOS builds have been known to fail with the following error due to `sftp_command`, which should be set to `/usr/libexec/openssh/sftp-server -e`:
 
 ```

From e6a0e523e2ca2f0c78a33a7d8134c06597bdb94b Mon Sep 17 00:00:00 2001
From: "Billie H. Cleek" <bhcleek@gmail.com>
Date: Thu, 8 Sep 2016 23:07:29 -0700
Subject: [PATCH 2/3] refactor sftp subsystem request handling

Refactor the sftp subsystem request handling to make it more similar to
an exec request. This simplifies and improves the readability of the
code.
---
 provisioner/ansible/adapter.go | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/provisioner/ansible/adapter.go b/provisioner/ansible/adapter.go
index 15cf886b7..0eb6bcda4 100644
--- a/provisioner/ansible/adapter.go
+++ b/provisioner/ansible/adapter.go
@@ -159,26 +159,13 @@ func (c *adapter) handleSession(newChannel ssh.NewChannel) error {
 					if len(sftpCmd) == 0 {
 						sftpCmd = "/usr/lib/sftp-server -e"
 					}
-					cmd := &packer.RemoteCmd{
-						Stdin:   channel,
-						Stdout:  channel,
-						Stderr:  channel.Stderr(),
-						Command: sftpCmd,
-					}
 
 					c.ui.Say("starting sftp subsystem")
-					if err := c.comm.Start(cmd); err != nil {
-						c.ui.Error(err.Error())
-						req.Reply(false, nil)
-						close(done)
-						return
-					}
-
-					req.Reply(true, nil)
 					go func() {
-						cmd.Wait()
+						_ = c.remoteExec(sftpCmd, channel, channel, channel.Stderr())
 						close(done)
 					}()
+					req.Reply(true, nil)
 				default:
 					c.ui.Error(fmt.Sprintf("unsupported subsystem requested: %s", req.Payload))
 					req.Reply(false, nil)

From f760ab2fd85afcd3ab63d2c53f12c606b8fcd683 Mon Sep 17 00:00:00 2001
From: "Billie H. Cleek" <bhcleek@gmail.com>
Date: Sun, 11 Sep 2016 23:29:24 -0700
Subject: [PATCH 3/3] Make SCP the default for provisioner/ansible

Add a new option, `use_sftp` to the ansible provisioner. It's default
value is false; ansible provisioner will use SCP by default.

Refactor to consistently set all configure options for ansible
provisioner in the Prepare step.

Remove incorrect information about `ANSIBLE_HOST_KEY_CHECKING=False`
being set when `ansible_env_vars` is not set in the packer template.

Update BATS tests for the ansible provisioner to actually check that the
fetched directory contains the contents expected. This revealed a
problem with the all_options template that required adding a host to the
hosts list in the test playbook.
---
 provisioner/ansible/provisioner.go              | 17 +++++++++--------
 .../provisioner-ansible/all_options.json        |  1 +
 test/fixtures/provisioner-ansible/playbook.yml  |  2 +-
 test/fixtures/provisioner-ansible/scp.json      |  3 +--
 test/fixtures/provisioner-ansible/sftp.json     |  5 +++--
 test/provisioner_ansible.bats                   |  5 +++++
 .../source/docs/provisioners/ansible.html.md    |  9 +++++----
 7 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/provisioner/ansible/provisioner.go b/provisioner/ansible/provisioner.go
index 74e1acc53..97623dda7 100644
--- a/provisioner/ansible/provisioner.go
+++ b/provisioner/ansible/provisioner.go
@@ -52,6 +52,7 @@ type Config struct {
 	SSHHostKeyFile       string   `mapstructure:"ssh_host_key_file"`
 	SSHAuthorizedKeyFile string   `mapstructure:"ssh_authorized_key_file"`
 	SFTPCmd              string   `mapstructure:"sftp_command"`
+	UseSFTP              bool     `mapstructure:"use_sftp"`
 	inventoryFile        string
 }
 
@@ -106,6 +107,12 @@ func (p *Provisioner) Prepare(raws ...interface{}) error {
 			log.Println(p.config.SSHHostKeyFile, "does not exist")
 			errs = packer.MultiErrorAppend(errs, err)
 		}
+	} else {
+		p.config.AnsibleEnvVars = append(p.config.AnsibleEnvVars, "ANSIBLE_HOST_KEY_CHECKING=False")
+	}
+
+	if !p.config.UseSFTP {
+		p.config.AnsibleEnvVars = append(p.config.AnsibleEnvVars, "ANSIBLE_SCP_IF_SSH=True")
 	}
 
 	if len(p.config.LocalPort) > 0 {
@@ -277,7 +284,7 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error {
 		}()
 	}
 
-	if err := p.executeAnsible(ui, comm, k.privKeyFile, !hostSigner.generated); err != nil {
+	if err := p.executeAnsible(ui, comm, k.privKeyFile); err != nil {
 		return fmt.Errorf("Error executing Ansible: %s", err)
 	}
 
@@ -294,7 +301,7 @@ func (p *Provisioner) Cancel() {
 	os.Exit(0)
 }
 
-func (p *Provisioner) executeAnsible(ui packer.Ui, comm packer.Communicator, privKeyFile string, checkHostKey bool) error {
+func (p *Provisioner) executeAnsible(ui packer.Ui, comm packer.Communicator, privKeyFile string) error {
 	playbook, _ := filepath.Abs(p.config.PlaybookFile)
 	inventory := p.config.inventoryFile
 	var envvars []string
@@ -315,10 +322,6 @@ func (p *Provisioner) executeAnsible(ui packer.Ui, comm packer.Communicator, pri
 		cmd.Env = append(cmd.Env, envvars...)
 	}
 
-	if !checkHostKey {
-		cmd.Env = append(cmd.Env, "ANSIBLE_HOST_KEY_CHECKING=False")
-	}
-
 	stdout, err := cmd.StdoutPipe()
 	if err != nil {
 		return err
@@ -435,7 +438,6 @@ func newUserKey(pubKeyFile string) (*userKey, error) {
 
 type signer struct {
 	ssh.Signer
-	generated bool
 }
 
 func newSigner(privKeyFile string) (*signer, error) {
@@ -464,7 +466,6 @@ func newSigner(privKeyFile string) (*signer, error) {
 	if err != nil {
 		return nil, errors.New("Failed to extract private key from generated key pair")
 	}
-	signer.generated = true
 
 	return signer, nil
 }
diff --git a/test/fixtures/provisioner-ansible/all_options.json b/test/fixtures/provisioner-ansible/all_options.json
index 6d74d65c8..4f7e16255 100644
--- a/test/fixtures/provisioner-ansible/all_options.json
+++ b/test/fixtures/provisioner-ansible/all_options.json
@@ -18,6 +18,7 @@
           "-vvvv", "--private-key", "ansible-test-id"
         ],
         "sftp_command": "/usr/lib/sftp-server -e -l INFO",
+        "use_sftp": true,
         "ansible_env_vars": ["PACKER_ANSIBLE_TEST=1", "ANSIBLE_HOST_KEY_CHECKING=False"],
         "groups": ["PACKER_TEST"],
         "empty_groups": ["PACKER_EMPTY_GROUP"],
diff --git a/test/fixtures/provisioner-ansible/playbook.yml b/test/fixtures/provisioner-ansible/playbook.yml
index fbe9cc6ac..b352387c0 100644
--- a/test/fixtures/provisioner-ansible/playbook.yml
+++ b/test/fixtures/provisioner-ansible/playbook.yml
@@ -1,5 +1,5 @@
 ---
-- hosts: default
+- hosts: default:packer-test
   gather_facts: no
   tasks:
     - raw: touch /root/ansible-raw-test
diff --git a/test/fixtures/provisioner-ansible/scp.json b/test/fixtures/provisioner-ansible/scp.json
index 21d72a3c4..b94078f2a 100644
--- a/test/fixtures/provisioner-ansible/scp.json
+++ b/test/fixtures/provisioner-ansible/scp.json
@@ -7,10 +7,9 @@
         "extra_arguments": [
           "-vvvv"
         ],
-        "ansible_env_vars": ["ANSIBLE_SCP_IF_SSH=True"],
         "sftp_command": "/usr/bin/false"
       }
-		],
+    ],
     "builders": [
       {
         "type": "googlecompute",
diff --git a/test/fixtures/provisioner-ansible/sftp.json b/test/fixtures/provisioner-ansible/sftp.json
index 4b2c73b34..bc2e5d731 100644
--- a/test/fixtures/provisioner-ansible/sftp.json
+++ b/test/fixtures/provisioner-ansible/sftp.json
@@ -12,9 +12,10 @@
       }, {
         "type":  "ansible",
         "playbook_file": "./playbook.yml",
-        "sftp_command": "/usr/lib/sftp-server -e -l INFO"
+        "sftp_command": "/usr/lib/sftp-server -e -l INFO",
+        "use_sftp": true
       }
-		],
+    ],
     "builders": [
       {
         "type": "googlecompute",
diff --git a/test/provisioner_ansible.bats b/test/provisioner_ansible.bats
index c25fc8012..537435ae6 100755
--- a/test/provisioner_ansible.bats
+++ b/test/provisioner_ansible.bats
@@ -48,6 +48,7 @@ teardown() {
     run packer build ${USER_VARS} $FIXTURE_ROOT/minimal.json
     [ "$status" -eq 0 ]
     [ "$(gc_has_image "packerbats-minimal")" -eq 1 ]
+    diff -r dir fetched-dir/default/tmp/remote-dir > /dev/null
 }
 
 @test "ansible provisioner: build all_options.json" {
@@ -55,6 +56,7 @@ teardown() {
     run packer build ${USER_VARS} $FIXTURE_ROOT/all_options.json
     [ "$status" -eq 0 ]
     [ "$(gc_has_image "packerbats-alloptions")" -eq 1 ]
+    diff -r dir fetched-dir/packer-test/tmp/remote-dir > /dev/null
 }
 
 @test "ansible provisioner: build scp.json" {
@@ -62,6 +64,7 @@ teardown() {
     run packer build ${USER_VARS} $FIXTURE_ROOT/scp.json
     [ "$status" -eq 0 ]
     [ "$(gc_has_image "packerbats-scp")" -eq 1 ]
+    diff -r dir fetched-dir/default/tmp/remote-dir > /dev/null
 }
 
 @test "ansible provisioner: build sftp.json" {
@@ -69,4 +72,6 @@ teardown() {
     run packer build ${USER_VARS} $FIXTURE_ROOT/sftp.json
     [ "$status" -eq 0 ]
     [ "$(gc_has_image "packerbats-sftp")" -eq 1 ]
+    diff -r dir fetched-dir/default/tmp/remote-dir > /dev/null
 }
+
diff --git a/website/source/docs/provisioners/ansible.html.md b/website/source/docs/provisioners/ansible.html.md
index 7c80b273a..2656130e9 100644
--- a/website/source/docs/provisioners/ansible.html.md
+++ b/website/source/docs/provisioners/ansible.html.md
@@ -78,10 +78,12 @@ Optional Parameters:
 - `sftp_command` (string) - The command to run on the machine being provisioned
   by Packer to handle the SFTP protocol that Ansible will use to transfer
   files. The command should read and write on stdin and stdout, respectively.
-  SCP can be used instead of SFTP by setting `ANSIBLE_SCP_IF_SSH=True` in
-  `ansible_env_vars`.
   Defaults to `/usr/lib/sftp-server -e`.
 
+- `use_sftp` (boolean) - Whether to use SFTP. When false,
+  `ANSIBLE_SCP_IF_SSH=True` will be automatically added to `ansible_env_vars`.
+  Defaults to false.
+
 - `extra_arguments` (array of strings) - Extra arguments to pass to Ansible.
   Usage example:
 
@@ -90,8 +92,7 @@ Optional Parameters:
 ```
 
 - `ansible_env_vars` (array of strings) - Environment variables to set before
-  running Ansible.  If unset, defaults to `ANSIBLE_HOST_KEY_CHECKING=False`.
-  Set `ANSIBLE_SCP_IF_SSH=True` to use SCP instead of SFTP.
+  running Ansible.
   Usage example:
 
 ```