cleanup ansible provisioner key generation

* Clearly separate host signer and user key generation into separate
  functions and data structures.

* Remove inaccurate comment about needing to specify both files if
  either one is specified.

* Rename parameters for clarity according to their meaning to the
  callee.

* Style the code with gofmt.

provisioner/ansible/provisioner.go

@@ -78,8 +78,7 @@ func (p *Provisioner) Prepare(raws ...interface{}) error {
 		errs = packer.MultiErrorAppend(errs, err)
 	}
 
-	// Check that the authorized key file exists ( this should really be called the public key )
+	// Check that the authorized key file exists
-	// Check for either file ( if you specify either file you must specify both files )
 	if len(p.config.SSHAuthorizedKeyFile) > 0 {
 		err = validateFileConfig(p.config.SSHAuthorizedKeyFile, "ssh_authorized_key_file", true)
 		if err != nil {
@@ -109,107 +108,18 @@ func (p *Provisioner) Prepare(raws ...interface{}) error {
 	return nil
 }
 
-type Keys struct {
-	//! This is the public key that we will allow to authenticate
-	public ssh.PublicKey
-
-	//! This is the name of the file of the private key that coorlates
-	//the the public key
-	filename string
-
-	//! This is the servers private key ( Set in case the public key
-	//is autogenerated )
-	private ssh.Signer
-
-	//! This is the flag to say the server key was generated
-	generated bool
-}
-
 func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error {
 	ui.Say("Provisioning with Ansible...")
 
-	keyFactory := func(pubKeyFile string, privKeyFile string) (*Keys, error) {
+	k, err := newUserKey(p.config.SSHAuthorizedKeyFile)
-		var public ssh.PublicKey
-		var private ssh.Signer
-		var filename string = ""
-		var generated bool = false
-
-		if len(pubKeyFile) > 0 {
-			pubKeyBytes, err := ioutil.ReadFile(pubKeyFile)
-			if err != nil {
-				return nil, errors.New("Failed to read public key")
-			}
-			public, _, _, _, err = ssh.ParseAuthorizedKey(pubKeyBytes)
-			if err != nil {
-				return nil, errors.New("Failed to parse authorized key")
-			}
-		} else {
-			key, err := rsa.GenerateKey(rand.Reader, 2048)
-			if err != nil {
-				return nil, errors.New("Failed to generate key pair")
-			}
-			public, err = ssh.NewPublicKey(key.Public())
-			if err != nil {
-				return nil, errors.New("Failed to extract public key from generated key pair")
-			}
-
-			// To support Ansible calling back to us we need to write
-			// this file down
-			privateKeyDer := x509.MarshalPKCS1PrivateKey(key)
-			privateKeyBlock := pem.Block{
-				Type:    "RSA PRIVATE KEY",
-				Headers: nil,
-				Bytes:   privateKeyDer,
-			}
-			tf, err := ioutil.TempFile("", "ansible-key")
-			if err != nil {
-				return nil, errors.New("failed to create temp file for generated key")
-			}
-			_, err = tf.Write(pem.EncodeToMemory(&privateKeyBlock))
-			if err != nil {
-				return nil, errors.New("failed to write private key to temp file")
-			}
-
-			err = tf.Close()
-			if err != nil {
-				return nil, errors.New("failed to close private key temp file")
-			}
-			filename = tf.Name()
-		}
-
-		if len(privKeyFile) > 0 {
-			privateBytes, err := ioutil.ReadFile(privKeyFile)
-			if err != nil {
-				return nil, errors.New("Failed to load private host key")
-			}
-
-			private, err = ssh.ParsePrivateKey(privateBytes)
-			if err != nil {
-				return nil, errors.New("Failed to parse private host key")
-			}
-		} else {
-			key, err := rsa.GenerateKey(rand.Reader, 2048)
-			if err != nil {
-				return nil, errors.New("Failed to generate server key pair")
-			}
-
-			private, err = ssh.NewSignerFromKey(key)
-			if err != nil {
-				return nil, errors.New("Failed to extract private key from generated key pair")
-			}
-			generated = true
-		}
-		return &Keys{public, filename, private, generated}, nil
-	}
-
-	k, err := keyFactory(p.config.SSHAuthorizedKeyFile, p.config.SSHHostKeyFile)
 	if err != nil {
 		return err
 	}
 
+	hostSigner, err := newSigner(p.config.SSHHostKeyFile)
 	// Remove the private key file
-	if len(k.filename) > 0 {
+	if len(k.privKeyFile) > 0 {
-		defer os.Remove(k.filename)
+		defer os.Remove(k.privKeyFile)
 	}
 
 	keyChecker := ssh.CertChecker{
@@ -219,7 +129,7 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error {
 				return nil, errors.New("authentication failed")
 			}
 
-			if !bytes.Equal(k.public.Marshal(), pubKey.Marshal()) {
+			if !bytes.Equal(k.Marshal(), pubKey.Marshal()) {
 				ui.Say("unauthorized key")
 				return nil, errors.New("authentication failed")
 			}
@@ -236,7 +146,7 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error {
 		//NoClientAuth:      true,
 	}
 
-	config.AddHostKey(k.private)
+	config.AddHostKey(hostSigner)
 
 	localListener, err := func() (net.Listener, error) {
 		port, err := strconv.ParseUint(p.config.LocalPort, 10, 16)
@@ -299,7 +209,7 @@ func (p *Provisioner) Provision(ui packer.Ui, comm packer.Communicator) error {
 		}()
 	}
 
-	if err := p.executeAnsible(ui, comm, k.filename, k.generated); err != nil {
+	if err := p.executeAnsible(ui, comm, k.privKeyFile, !hostSigner.generated); err != nil {
 		return fmt.Errorf("Error executing Ansible: %s", err)
 	}
 
@@ -317,20 +227,19 @@ func (p *Provisioner) Cancel() {
 	os.Exit(0)
 }
 
-func (p *Provisioner) executeAnsible(ui packer.Ui, comm packer.Communicator, authToken string, generated bool) error {
+func (p *Provisioner) executeAnsible(ui packer.Ui, comm packer.Communicator, privKeyFile string, checkHostKey bool) error {
 	playbook, _ := filepath.Abs(p.config.PlaybookFile)
 	inventory := p.config.inventoryFile
 
 	args := []string{playbook, "-i", inventory}
-	if len(authToken) > 0 {
+	if len(privKeyFile) > 0 {
-		args = append(args, "--private-key", authToken)
+		args = append(args, "--private-key", privKeyFile)
 	}
 	args = append(args, p.config.ExtraArguments...)
 
 	cmd := exec.Command(p.config.Command, args...)
 
-	// If we have autogenerated the key files turn off host key checking
+	if !checkHostKey {
-	if generated {
 		cmd.Env = os.Environ()
 		cmd.Env = append(cmd.Env, "ANSIBLE_HOST_KEY_CHECKING=False")
 	}
@@ -385,6 +294,97 @@ func validateFileConfig(name string, config string, req bool) error {
 	return nil
 }
 
+type userKey struct {
+	ssh.PublicKey
+	privKeyFile string
+}
+
+func newUserKey(pubKeyFile string) (*userKey, error) {
+	userKey := new(userKey)
+	if len(pubKeyFile) > 0 {
+		pubKeyBytes, err := ioutil.ReadFile(pubKeyFile)
+		if err != nil {
+			return nil, errors.New("Failed to read public key")
+		}
+		userKey.PublicKey, _, _, _, err = ssh.ParseAuthorizedKey(pubKeyBytes)
+		if err != nil {
+			return nil, errors.New("Failed to parse authorized key")
+		}
+
+		return userKey, nil
+	}
+
+	key, err := rsa.GenerateKey(rand.Reader, 2048)
+	if err != nil {
+		return nil, errors.New("Failed to generate key pair")
+	}
+	userKey.PublicKey, err = ssh.NewPublicKey(key.Public())
+	if err != nil {
+		return nil, errors.New("Failed to extract public key from generated key pair")
+	}
+
+	// To support Ansible calling back to us we need to write
+	// this file down
+	privateKeyDer := x509.MarshalPKCS1PrivateKey(key)
+	privateKeyBlock := pem.Block{
+		Type:    "RSA PRIVATE KEY",
+		Headers: nil,
+		Bytes:   privateKeyDer,
+	}
+	tf, err := ioutil.TempFile("", "ansible-key")
+	if err != nil {
+		return nil, errors.New("failed to create temp file for generated key")
+	}
+	_, err = tf.Write(pem.EncodeToMemory(&privateKeyBlock))
+	if err != nil {
+		return nil, errors.New("failed to write private key to temp file")
+	}
+
+	err = tf.Close()
+	if err != nil {
+		return nil, errors.New("failed to close private key temp file")
+	}
+	userKey.privKeyFile = tf.Name()
+
+	return userKey, nil
+}
+
+type signer struct {
+	ssh.Signer
+	generated bool
+}
+
+func newSigner(privKeyFile string) (*signer, error) {
+	signer := new(signer)
+
+	if len(privKeyFile) > 0 {
+		privateBytes, err := ioutil.ReadFile(privKeyFile)
+		if err != nil {
+			return nil, errors.New("Failed to load private host key")
+		}
+
+		signer.Signer, err = ssh.ParsePrivateKey(privateBytes)
+		if err != nil {
+			return nil, errors.New("Failed to parse private host key")
+		}
+
+		return signer, nil
+	}
+
+	key, err := rsa.GenerateKey(rand.Reader, 2048)
+	if err != nil {
+		return nil, errors.New("Failed to generate server key pair")
+	}
+
+	signer.Signer, err = ssh.NewSignerFromKey(key)
+	if err != nil {
+		return nil, errors.New("Failed to extract private key from generated key pair")
+	}
+	signer.generated = true
+
+	return signer, nil
+}
+
 // Ui provides concurrency-safe access to packer.Ui.
 type Ui struct {
 	sem chan int

website/source/docs/provisioners/ansible.html.markdown

@@ -47,18 +47,17 @@ Required Parameters:
 
 Optional Parameters:
 
-- `ssh_host_key_file` (string) - The SSH key that will be used to run
+- `ssh_host_key_file` (string) - The SSH key that will be used to run the SSH
-  the SSH server on the host machine to forward commands to the target
+  server on the host machine to forward commands to the target machine. Ansible
-  machine. Ansible connects to this server and will validate the
+  connects to this server and will validate the identity of the server using
-  identity of the server using the system known_hosts. The default behaviour is
+  the system known_hosts. The default behaviour is to generate and use a one
-  to generate and use a one time key, and disable
+  time key, and disable host_key_verification in ansible to allow it to connect
-  host_key_verification in ansible to allow it to connect to the
+  to the server 
-  server 
 
-- `ssh_authorized_key_file` (string) - The SSH public key of the
+- `ssh_authorized_key_file` (string) - The SSH public key of the Ansible
-  Ansible `ssh_user`. The default behaviour is to generate and use a
+  `ssh_user`. The default behaviour is to generate and use a one time key. If
-  one time key. If this file is generated the coorisponding private
+  this file is generated the coorisponding private key will be passed via the
-  key will be passed via the `--private-key` option to Ansible.
+  `--private-key` option to Ansible.
 
 - `local_port` (string) - The port on which to attempt to listen for SSH
   connections. This value is a starting point.  The provisioner will attempt
@@ -67,9 +66,9 @@ Optional Parameters:
   listen on a system-chosen port.
 
 
-- `sftp_command` (string) - The command to run on the provisioned machine to handle the
+- `sftp_command` (string) - The command to run on the provisioned machine to
-  SFTP protocol that Ansible will use to transfer files. The command should
+  handle the SFTP protocol that Ansible will use to transfer files. The command
-  read and write on stdin and stdout, respectively. Defaults to
+  should read and write on stdin and stdout, respectively. Defaults to
   `/usr/lib/sftp-server -e`.
 
 - `extra_arguments` (string) - Extra arguments to pass to Ansible