Refactored quite a few things after review...

Also renamed the provisioner to just `chef` as it’s out intention to
end up with one provisioner for all types of `chef` clients.

builtin/bins/provisioner-chef/main.go

@@ -1,7 +1,7 @@
 package main
 
 import (
-	"github.com/hashicorp/terraform/builtin/provisioners/chef-client"
+	"github.com/hashicorp/terraform/builtin/provisioners/chef"
 	"github.com/hashicorp/terraform/plugin"
 	"github.com/hashicorp/terraform/terraform"
 )
@@ -9,7 +9,7 @@ import (
 func main() {
 	plugin.Serve(&plugin.ServeOpts{
 		ProvisionerFunc: func() terraform.ResourceProvisioner {
-			return new(chefclient.ResourceProvisioner)
+			return new(chef.ResourceProvisioner)
 		},
 	})
 }

builtin/provisioners/chef/resource_provisioner.go

@@ -1,4 +1,4 @@
-package chefclient
+package chef
 
 import (
 	"bytes"
@@ -22,9 +22,12 @@ import (
 )
 
 const (
+	clienrb        = "client.rb"
+	defaultChefEnv = "_default"
 	firstBoot      = "first-boot.json"
 	logfileDir     = "logfiles"
 	linuxConfDir   = "/etc/chef"
+	validationKey  = "validation.pem"
 	windowsConfDir = "C:/chef"
 )
 
@@ -52,7 +55,7 @@ ENV['HTTPS_PROXY'] = "{{ .HTTPSProxy }}"
 
 // Provisioner represents a specificly configured chef provisioner
 type Provisioner struct {
-	Attributes           interface{} `mapstructure:"-"`
+	Attributes           interface{} `mapstructure:"attributes"`
 	Environment          string      `mapstructure:"environment"`
 	LogToFile            bool        `mapstructure:"log_to_file"`
 	HTTPProxy            string      `mapstructure:"http_proxy"`
@@ -169,6 +172,7 @@ func (r *ResourceProvisioner) decodeConfig(c *terraform.ResourceConfig) (*Provis
 	p := new(Provisioner)
 
 	decConf := &mapstructure.DecoderConfig{
+		ErrorUnused:      true,
 		WeaklyTypedInput: true,
 		Result:           p,
 	}
@@ -182,7 +186,7 @@ func (r *ResourceProvisioner) decodeConfig(c *terraform.ResourceConfig) (*Provis
 	}
 
 	if p.Environment == "" {
-		p.Environment = "_default"
+		p.Environment = defaultChefEnv
 	}
 
 	if attrs, ok := c.Raw["attributes"]; ok {
@@ -241,7 +245,7 @@ func (p *Provisioner) runChefClientFunc(
 		cmd := fmt.Sprintf("chef-client -j %q -E %q", fb, p.Environment)
 
 		if p.LogToFile {
-			if err := os.MkdirAll(logfileDir, 0777); err != nil {
+			if err := os.MkdirAll(logfileDir, 0755); err != nil {
 				return fmt.Errorf("Error creating logfile directory %s: %v", logfileDir, err)
 			}
 
@@ -290,16 +294,16 @@ func (p *Provisioner) deployConfigFiles(
 	o terraform.UIOutput,
 	comm communicator.Communicator,
 	confDir string) error {
-	// Open the validation .pem file
+	// Open the validation  key file
 	f, err := os.Open(p.ValidationKeyPath)
 	if err != nil {
 		return err
 	}
 	defer f.Close()
 
-	// Copy the validation .pem to the new instance
-	if err := comm.Upload(path.Join(confDir, "validation.pem"), f); err != nil {
-		return fmt.Errorf("Uploading validation.pem failed: %v", err)
+	// Copy the validation key to the new instance
+	if err := comm.Upload(path.Join(confDir, validationKey), f); err != nil {
+		return fmt.Errorf("Uploading %s failed: %v", validationKey, err)
 	}
 
 	// Make strings.Join available for use within the template
@@ -307,18 +311,18 @@ func (p *Provisioner) deployConfigFiles(
 		"join": strings.Join,
 	}
 
-	// Create a new template and parse the client.rb into it
-	t := template.Must(template.New("client.rb").Funcs(funcMap).Parse(clientConf))
+	// Create a new template and parse the client config into it
+	t := template.Must(template.New(clienrb).Funcs(funcMap).Parse(clientConf))
 
 	var buf bytes.Buffer
 	err = t.Execute(&buf, p)
 	if err != nil {
-		return fmt.Errorf("Error executing client.rb template: %s", err)
+		return fmt.Errorf("Error executing %s template: %s", clienrb, err)
 	}
 
-	// Copy the client.rb to the new instance
-	if err := comm.Upload(path.Join(confDir, "client.rb"), &buf); err != nil {
-		return fmt.Errorf("Uploading client.rb failed: %v", err)
+	// Copy the client config to the new instance
+	if err := comm.Upload(path.Join(confDir, clienrb), &buf); err != nil {
+		return fmt.Errorf("Uploading %s failed: %v", clienrb, err)
 	}
 
 	// Create a map with first boot settings
@@ -327,6 +331,13 @@ func (p *Provisioner) deployConfigFiles(
 		fb = p.Attributes.(map[string]interface{})
 	}
 
+	// Check if the run_list was also in the attributes and if so log a warning
+	// that it will be overwritten with the value of the run_list argument.
+	if _, found := fb["run_list"]; found {
+		log.Printf("[WARNING] Found a 'run_list' specified in the configured attributes! " +
+			"This value will be overwritten by the value of the `run_list` argument!")
+	}
+
 	// Add the initial runlist to the first boot settings
 	fb["run_list"] = p.RunList
 
@@ -338,7 +349,7 @@ func (p *Provisioner) deployConfigFiles(
 
 	// Copy the first-boot.json to the new instance
 	if err := comm.Upload(path.Join(confDir, firstBoot), bytes.NewReader(d)); err != nil {
-		return fmt.Errorf("Uploading first-boot.json failed: %v", err)
+		return fmt.Errorf("Uploading %s failed: %v", firstBoot, err)
 	}
 
 	return nil
@@ -369,6 +380,7 @@ func (p *Provisioner) runCommand(
 		Stderr:  errW,
 	}
 
+	log.Printf("[DEBUG] Executing remote command: %q", cmd.Command)
 	if err := comm.Start(cmd); err != nil {
 		return fmt.Errorf("Error executing command %q: %v", cmd.Command, err)
 	}

builtin/provisioners/chef/resource_provisioner_test.go

@@ -1,4 +1,4 @@
-package chefclient
+package chef
 
 import (
 	"testing"

builtin/provisioners/chef/ssh_provisioner.go

@@ -1,42 +1,44 @@
-package chefclient
+package chef
 
 import (
-	"bytes"
+	"fmt"
 	"strings"
 
 	"github.com/hashicorp/terraform/communicator"
 	"github.com/hashicorp/terraform/terraform"
 )
 
+const (
+	installURL = "https://www.chef.io/chef/install.sh"
+)
+
 func (p *Provisioner) sshInstallChefClient(
 	o terraform.UIOutput,
 	comm communicator.Communicator) error {
-	var installCmd bytes.Buffer
 
-	// Build up a single command based on the given config options
-	installCmd.WriteString("curl")
+	// Build up the command prefix
+	prefix := ""
 	if p.HTTPProxy != "" {
-		installCmd.WriteString(" --proxy " + p.HTTPProxy)
+		prefix += fmt.Sprintf("proxy_http='%s' ", p.HTTPProxy)
 	}
 	if p.NOProxy != nil {
-		installCmd.WriteString(" --noproxy " + strings.Join(p.NOProxy, ","))
+		prefix += fmt.Sprintf("no_proxy='%s' ", strings.Join(p.NOProxy, ","))
 	}
-	installCmd.WriteString(" -LO https://www.chef.io/chef/install.sh 2>/dev/null &&")
-	if !p.PreventSudo {
-		installCmd.WriteString(" sudo")
-	}
-	installCmd.WriteString(" bash ./install.sh")
-	if p.Version != "" {
-		installCmd.WriteString(" -v " + p.Version)
-	}
-	installCmd.WriteString(" &&")
-	if !p.PreventSudo {
-		installCmd.WriteString(" sudo")
-	}
-	installCmd.WriteString(" rm -f install.sh")
 
-	// Execute the command to install Chef Client
-	return p.runCommand(o, comm, installCmd.String())
+	// First download the install.sh script from Chef
+	err := p.runCommand(o, comm, fmt.Sprintf("%scurl -LO %s", prefix, installURL))
+	if err != nil {
+		return err
+	}
+
+	// Then execute the install.sh scrip to download and install Chef Client
+	err = p.runCommand(o, comm, fmt.Sprintf("%sbash ./install.sh -v %s", prefix, p.Version))
+	if err != nil {
+		return err
+	}
+
+	// And finally cleanup the install.sh script again
+	return p.runCommand(o, comm, fmt.Sprintf("%srm -f install.sh", prefix))
 }
 
 func (p *Provisioner) sshCreateConfigFiles(

builtin/provisioners/chef/ssh_provisioner_test.go

@@ -1,4 +1,4 @@
-package chefclient
+package chef
 
 import (
 	"testing"
@@ -22,8 +22,9 @@ func TestResourceProvider_sshInstallChefClient(t *testing.T) {
 			}),
 
 			Commands: map[string]bool{
-				"sudo curl -LO https://www.chef.io/chef/install.sh 2>/dev/null && " +
-					"sudo bash ./install.sh && sudo rm -f install.sh": true,
+				"sudo curl -LO https://www.chef.io/chef/install.sh": true,
+				"sudo bash ./install.sh -v ":                        true,
+				"sudo rm -f install.sh":                             true,
 			},
 		},
 
@@ -38,8 +39,9 @@ func TestResourceProvider_sshInstallChefClient(t *testing.T) {
 			}),
 
 			Commands: map[string]bool{
-				"curl -LO https://www.chef.io/chef/install.sh 2>/dev/null && " +
-					"bash ./install.sh && rm -f install.sh": true,
+				"curl -LO https://www.chef.io/chef/install.sh": true,
+				"bash ./install.sh -v ":                        true,
+				"rm -f install.sh":                             true,
 			},
 		},
 
@@ -55,8 +57,9 @@ func TestResourceProvider_sshInstallChefClient(t *testing.T) {
 			}),
 
 			Commands: map[string]bool{
-				"curl --proxy http://proxy.local -LO https://www.chef.io/chef/install.sh 2>/dev/null && " +
-					"bash ./install.sh && rm -f install.sh": true,
+				"proxy_http='http://proxy.local' curl -LO https://www.chef.io/chef/install.sh": true,
+				"proxy_http='http://proxy.local' bash ./install.sh -v ":                        true,
+				"proxy_http='http://proxy.local' rm -f install.sh":                             true,
 			},
 		},
 
@@ -73,8 +76,11 @@ func TestResourceProvider_sshInstallChefClient(t *testing.T) {
 			}),
 
 			Commands: map[string]bool{
-				"curl --proxy http://proxy.local --noproxy http://local.local,http://local.org -LO " +
-					"https://www.chef.io/chef/install.sh 2>/dev/null && bash ./install.sh && " +
+				"proxy_http='http://proxy.local' no_proxy='http://local.local,http://local.org' " +
+					"curl -LO https://www.chef.io/chef/install.sh": true,
+				"proxy_http='http://proxy.local' no_proxy='http://local.local,http://local.org' " +
+					"bash ./install.sh -v ": true,
+				"proxy_http='http://proxy.local' no_proxy='http://local.local,http://local.org' " +
 					"rm -f install.sh": true,
 			},
 		},
@@ -91,8 +97,9 @@ func TestResourceProvider_sshInstallChefClient(t *testing.T) {
 			}),
 
 			Commands: map[string]bool{
-				"curl -LO https://www.chef.io/chef/install.sh 2>/dev/null && " +
-					"bash ./install.sh -v 11.18.6 && rm -f install.sh": true,
+				"curl -LO https://www.chef.io/chef/install.sh": true,
+				"bash ./install.sh -v 11.18.6":                 true,
+				"rm -f install.sh":                             true,
 			},
 		},
 	}

builtin/provisioners/chef/winrm_provisioner.go

@@ -1,4 +1,4 @@
-package chefclient
+package chef
 
 import (
 	"fmt"

builtin/provisioners/chef/winrm_provisioner_test.go

@@ -1,4 +1,4 @@
-package chefclient
+package chef
 
 import (
 	"fmt"

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

@@ -1,16 +1,22 @@
 ---
 layout: "docs"
-page_title: "Provisioner: chef-client"
-sidebar_current: "docs-provisioners-chef-client"
+page_title: "Provisioner: chef"
+sidebar_current: "docs-provisioners-chef"
 description: |-
-  The `chef-client` provisioner invokes a Chef Client run on a remote resource after first installing and configuring Chef Client on the remote resource. The `chef-client` provisioner supports both `ssh` and `winrm` type connections.
+  The `chef` provisioner invokes a Chef Client run on a remote resource after first installing and configuring Chef Client on the remote resource. The `chef` provisioner supports both `ssh` and `winrm` type connections.
 ---
 
-# chef Provisioner
+# Chef Provisioner
 
-The `chef-client` provisioner invokes a Chef Client run on a remote resource after first
-installing and configuring Chef Client on the remote resource. The `chef-client` provisioner
-supports both `ssh` and `winrm` type [connections](/docs/provisioners/connection.html).
+The `chef` provisioner invokes a Chef Client run on a remote resource after first installing
+and configuring Chef Client on the remote resource. The `chef` provisioner supports both `ssh`
+and `winrm` type [connections](/docs/provisioners/connection.html).
+
+## Requirements
+
+In order for the `chef` provisioner to work properly, you need either `cURL` (when using
+a `ssh` type connection) or `PowerShell 2.0` (when using a `winrm` type connection) to be
+available on the target machine.
 
 ## Example usage
 
@@ -18,7 +24,7 @@ supports both `ssh` and `winrm` type [connections](/docs/provisioners/connection
 # Start a initial chef run on a resource
 resource "aws_instance" "web" {
     ...
-    provisioner "chef-client"  {
+    provisioner "chef"  {
         attributes {
             "key" = "value"
             "app" {
@@ -73,7 +79,7 @@ The following arguments are supported:
   the organization. See the example.
 
 * `skip_install (boolean)` - (Optional) Skip the installation of Chef Client on the remote
-  machine. This assumes Chef Client is already installed when you run the `chef-client`
+  machine. This assumes Chef Client is already installed when you run the `chef`
   provisioner.
 
 * `ssl_verify_mode (string)` - (Optional) Use to set the verify mode for Chef Client HTTPS

website/source/layouts/docs.erb

@@ -178,9 +178,9 @@
 				<li<%= sidebar_current("docs-provisioners") %>>
 				<a href="/docs/provisioners/index.html">Provisioners</a>
 				<ul class="nav">
-					<li<%= sidebar_current("docs-provisioners-chef-client") %>>
-+					<a href="/docs/provisioners/chef-client.html">chef-client</a>
-+					</li>
+					<li<%= sidebar_current("docs-provisioners-chef") %>>
+					<a href="/docs/provisioners/chef.html">chef</a>
+					</li>
 
 					<li<%= sidebar_current("docs-provisioners-connection") %>>
 					<a href="/docs/provisioners/connection.html">connection</a>