From f5449a62e090b105a826b73c49523a232ce23132 Mon Sep 17 00:00:00 2001 From: Vladislav Rassokhin Date: Tue, 29 Nov 2016 23:00:26 +0300 Subject: [PATCH 1/3] Various built-in provisioners improvements: 1. Migrate `chef` provisioner to `schema.Provisioner`: * `chef.Provisioner` structure was renamed to `ProvisionerS`and now it's decoded from `schema.ResourceData` instead of `terraform.ResourceConfig` using simple copy-paste-based solution; * Added simple schema without any validation yet. 2. Support `ValidateFunc` validate function : implemented in `file` and `chef` provisioners. --- builtin/bins/provisioner-chef/main.go | 5 +- .../provisioners/chef/linux_provisioner.go | 6 +- .../chef/linux_provisioner_test.go | 6 +- .../provisioners/chef/resource_provisioner.go | 367 +++++++++++++----- .../chef/resource_provisioner_test.go | 28 +- .../provisioners/chef/windows_provisioner.go | 4 +- .../chef/windows_provisioner_test.go | 6 +- .../provisioners/file/resource_provisioner.go | 18 +- .../file/resource_provisioner_test.go | 25 ++ .../local-exec/resource_provisioner_test.go | 11 + .../remote-exec/resource_provisioner_test.go | 10 + helper/schema/provisioner.go | 31 +- helper/schema/provisioner_test.go | 78 +++- helper/schema/testing.go | 7 + 14 files changed, 465 insertions(+), 137 deletions(-) diff --git a/builtin/bins/provisioner-chef/main.go b/builtin/bins/provisioner-chef/main.go index a12c65cf7..6e81fde49 100644 --- a/builtin/bins/provisioner-chef/main.go +++ b/builtin/bins/provisioner-chef/main.go @@ -3,13 +3,10 @@ package main import ( "github.com/hashicorp/terraform/builtin/provisioners/chef" "github.com/hashicorp/terraform/plugin" - "github.com/hashicorp/terraform/terraform" ) func main() { plugin.Serve(&plugin.ServeOpts{ - ProvisionerFunc: func() terraform.ResourceProvisioner { - return new(chef.ResourceProvisioner) - }, + ProvisionerFunc: chef.Provisioner, }) } diff --git a/builtin/provisioners/chef/linux_provisioner.go b/builtin/provisioners/chef/linux_provisioner.go index ebfe72979..a9ad806a0 100644 --- a/builtin/provisioners/chef/linux_provisioner.go +++ b/builtin/provisioners/chef/linux_provisioner.go @@ -14,7 +14,7 @@ const ( installURL = "https://www.chef.io/chef/install.sh" ) -func (p *Provisioner) linuxInstallChefClient( +func (p *ProvisionerS) linuxInstallChefClient( o terraform.UIOutput, comm communicator.Communicator) error { @@ -26,7 +26,7 @@ func (p *Provisioner) linuxInstallChefClient( if p.HTTPSProxy != "" { prefix += fmt.Sprintf("https_proxy='%s' ", p.HTTPSProxy) } - if p.NOProxy != nil { + if p.NOProxy != nil && len(p.NOProxy) > 0 { prefix += fmt.Sprintf("no_proxy='%s' ", strings.Join(p.NOProxy, ",")) } @@ -46,7 +46,7 @@ func (p *Provisioner) linuxInstallChefClient( return p.runCommand(o, comm, fmt.Sprintf("%srm -f install.sh", prefix)) } -func (p *Provisioner) linuxCreateConfigFiles( +func (p *ProvisionerS) linuxCreateConfigFiles( o terraform.UIOutput, comm communicator.Communicator) error { // Make sure the config directory exists diff --git a/builtin/provisioners/chef/linux_provisioner_test.go b/builtin/provisioners/chef/linux_provisioner_test.go index 7d2339e04..444a0f39a 100644 --- a/builtin/provisioners/chef/linux_provisioner_test.go +++ b/builtin/provisioners/chef/linux_provisioner_test.go @@ -125,14 +125,13 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { }, } - r := new(ResourceProvisioner) o := new(terraform.MockUIOutput) c := new(communicator.MockCommunicator) for k, tc := range cases { c.Commands = tc.Commands - p, err := r.decodeConfig(tc.Config) + p, err := decodeConfig(getTestResourceData(tc.Config)) if err != nil { t.Fatalf("Error: %v", err) } @@ -264,7 +263,6 @@ func TestResourceProvider_linuxCreateConfigFiles(t *testing.T) { }, } - r := new(ResourceProvisioner) o := new(terraform.MockUIOutput) c := new(communicator.MockCommunicator) @@ -272,7 +270,7 @@ func TestResourceProvider_linuxCreateConfigFiles(t *testing.T) { c.Commands = tc.Commands c.Uploads = tc.Uploads - p, err := r.decodeConfig(tc.Config) + p, err := decodeConfig(getTestResourceData(tc.Config)) if err != nil { t.Fatalf("Error: %v", err) } diff --git a/builtin/provisioners/chef/resource_provisioner.go b/builtin/provisioners/chef/resource_provisioner.go index a5482ea38..28302ecac 100644 --- a/builtin/provisioners/chef/resource_provisioner.go +++ b/builtin/provisioners/chef/resource_provisioner.go @@ -15,12 +15,13 @@ import ( "text/template" "time" + "context" "github.com/hashicorp/terraform/communicator" "github.com/hashicorp/terraform/communicator/remote" + "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/go-homedir" "github.com/mitchellh/go-linereader" - "github.com/mitchellh/mapstructure" ) const ( @@ -81,36 +82,36 @@ enable_reporting false {{ end }} ` -// Provisioner represents a Chef provisioner -type Provisioner struct { - AttributesJSON string `mapstructure:"attributes_json"` - ClientOptions []string `mapstructure:"client_options"` - DisableReporting bool `mapstructure:"disable_reporting"` - Environment string `mapstructure:"environment"` - FetchChefCertificates bool `mapstructure:"fetch_chef_certificates"` - LogToFile bool `mapstructure:"log_to_file"` - UsePolicyfile bool `mapstructure:"use_policyfile"` - PolicyGroup string `mapstructure:"policy_group"` - PolicyName string `mapstructure:"policy_name"` - HTTPProxy string `mapstructure:"http_proxy"` - HTTPSProxy string `mapstructure:"https_proxy"` - NamedRunList string `mapstructure:"named_run_list"` - NOProxy []string `mapstructure:"no_proxy"` - NodeName string `mapstructure:"node_name"` - OhaiHints []string `mapstructure:"ohai_hints"` - OSType string `mapstructure:"os_type"` - RecreateClient bool `mapstructure:"recreate_client"` - PreventSudo bool `mapstructure:"prevent_sudo"` - RunList []string `mapstructure:"run_list"` - SecretKey string `mapstructure:"secret_key"` - ServerURL string `mapstructure:"server_url"` - SkipInstall bool `mapstructure:"skip_install"` - SkipRegister bool `mapstructure:"skip_register"` - SSLVerifyMode string `mapstructure:"ssl_verify_mode"` - UserName string `mapstructure:"user_name"` - UserKey string `mapstructure:"user_key"` - VaultJSON string `mapstructure:"vault_json"` - Version string `mapstructure:"version"` +// ProvisionerS represents a Chef provisioner +type ProvisionerS struct { + AttributesJSON string + ClientOptions []string + DisableReporting bool + Environment string + FetchChefCertificates bool + LogToFile bool + UsePolicyfile bool + PolicyGroup string + PolicyName string + HTTPProxy string + HTTPSProxy string + NamedRunList string + NOProxy []string + NodeName string + OhaiHints []string + OSType string + RecreateClient bool + PreventSudo bool + RunList []string + SecretKey string + ServerURL string + SkipInstall bool + SkipRegister bool + SSLVerifyMode string + UserName string + UserKey string + VaultJSON string + Version string attributes map[string]interface{} vaults map[string][]string @@ -125,37 +126,179 @@ type Provisioner struct { useSudo bool // Deprecated Fields - ValidationClientName string `mapstructure:"validation_client_name"` - ValidationKey string `mapstructure:"validation_key"` + ValidationClientName string + ValidationKey string } -// ResourceProvisioner represents a generic chef provisioner -type ResourceProvisioner struct{} +func Provisioner() terraform.ResourceProvisioner { + return &schema.Provisioner{ + Schema: map[string]*schema.Schema{ + "attributes_json": { + Type: schema.TypeString, + Optional: true, + }, + "client_options": { + Type: schema.TypeList, + Elem: schema.Schema{ + Type: schema.TypeString, + }, + Optional: true, + }, + "disable_reporting": { + Type: schema.TypeBool, + Optional: true, + }, + "environment": { + Type: schema.TypeString, + Optional: true, + }, + "fetch_chef_certificates": { + Type: schema.TypeBool, + Optional: true, + }, + "log_to_file": { + Type: schema.TypeBool, + Optional: true, + }, + "use_policyfile": { + Type: schema.TypeBool, + Optional: true, + }, + "policy_group": { + Type: schema.TypeString, + Optional: true, + }, + "policy_name": { + Type: schema.TypeString, + Optional: true, + }, + "http_proxy": { + Type: schema.TypeString, + Optional: true, + }, + "https_proxy": { + Type: schema.TypeString, + Optional: true, + }, + "no_proxy": { + Type: schema.TypeList, + Elem: schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + Optional: true, + }, + "named_run_list": { + Type: schema.TypeString, + Optional: true, + }, + "node_name": { + Type: schema.TypeString, + Required: true, + }, + "ohai_hints": { + Type: schema.TypeList, + Elem: schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + Optional: true, + }, + "os_type": { + Type: schema.TypeString, + Optional: true, + }, + "recreate_client": { + Type: schema.TypeBool, + Optional: true, + }, + "prevent_sudo": { + Type: schema.TypeBool, + Optional: true, + }, + "run_list": { + Type: schema.TypeList, + Elem: schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + Optional: true, + }, + "secret_key": { + Type: schema.TypeString, + Optional: true, + }, + "server_url": { + Type: schema.TypeString, + Required: true, + }, + "skip_install": { + Type: schema.TypeBool, + Optional: true, + }, + "skip_register": { + Type: schema.TypeBool, + Optional: true, + }, + "ssl_verify_mode": { + Type: schema.TypeString, + Optional: true, + }, + "user_name": { + Type: schema.TypeString, + Optional: true, + }, + "user_key": { + Type: schema.TypeString, + Optional: true, + }, + "vault_json": { + Type: schema.TypeString, + Optional: true, + }, + "version": { + Type: schema.TypeString, + Optional: true, + }, -func (r *ResourceProvisioner) Stop() error { - // Noop for now. TODO in the future. - return nil + // Deprecated + "validation_client_name": { + Type: schema.TypeString, + Deprecated: "Please use user_name instead", + Optional: true, + }, + + "validation_key": { + Type: schema.TypeString, + Deprecated: "Please use user_key instead", + Optional: true, + }, + }, + ApplyFunc: Apply, + ValidateFunc: Validate, + } } +// TODO: Support context cancelling (Provisioner Stop) // Apply executes the file provisioner -func (r *ResourceProvisioner) Apply( - o terraform.UIOutput, - s *terraform.InstanceState, - c *terraform.ResourceConfig) error { +func Apply(ctx context.Context) error { + o := ctx.Value(schema.ProvOutputKey).(terraform.UIOutput) + d := ctx.Value(schema.ProvConfigDataKey).(*schema.ResourceData) // Decode the raw config for this provisioner - p, err := r.decodeConfig(c) + p, err := decodeConfig(d) if err != nil { return err } if p.OSType == "" { - switch s.Ephemeral.ConnInfo["type"] { + t := d.State().Ephemeral.ConnInfo["type"] + switch t { case "ssh", "": // The default connection type is ssh, so if the type is empty assume ssh p.OSType = "linux" case "winrm": p.OSType = "windows" default: - return fmt.Errorf("Unsupported connection type: %s", s.Ephemeral.ConnInfo["type"]) + return fmt.Errorf("Unsupported connection type: %s", t) } } @@ -169,7 +312,7 @@ func (r *ResourceProvisioner) Apply( p.generateClientKey = p.generateClientKeyFunc(linuxKnifeCmd, linuxConfDir, linuxNoOutput) p.configureVaults = p.configureVaultsFunc(linuxGemCmd, linuxKnifeCmd, linuxConfDir) p.runChefClient = p.runChefClientFunc(linuxChefCmd, linuxConfDir) - p.useSudo = !p.PreventSudo && s.Ephemeral.ConnInfo["user"] != "root" + p.useSudo = !p.PreventSudo && d.State().Ephemeral.ConnInfo["user"] != "root" case "windows": p.cleanupUserKeyCmd = fmt.Sprintf("cd %s && del /F /Q %s", windowsConfDir, p.UserName+".pem") p.createConfigFiles = p.windowsCreateConfigFiles @@ -184,7 +327,7 @@ func (r *ResourceProvisioner) Apply( } // Get a new communicator - comm, err := communicator.New(s) + comm, err := communicator.New(d.State()) if err != nil { return err } @@ -254,22 +397,16 @@ func (r *ResourceProvisioner) Apply( } // Validate checks if the required arguments are configured -func (r *ResourceProvisioner) Validate(c *terraform.ResourceConfig) (ws []string, es []error) { - p, err := r.decodeConfig(c) +func Validate(d *schema.ResourceData) (ws []string, es []error) { + p, err := decodeConfig(d) if err != nil { es = append(es, err) return ws, es } - if p.NodeName == "" { - es = append(es, errors.New("Key not found: node_name")) - } if !p.UsePolicyfile && p.RunList == nil { es = append(es, errors.New("Key not found: run_list")) } - if p.ServerURL == "" { - es = append(es, errors.New("Key not found: server_url")) - } if p.UsePolicyfile && p.PolicyName == "" { es = append(es, errors.New("Policyfile enabled but key not found: policy_name")) } @@ -284,12 +421,7 @@ func (r *ResourceProvisioner) Validate(c *terraform.ResourceConfig) (ws []string es = append(es, errors.New( "One of user_key or the deprecated validation_key must be provided")) } - if p.ValidationClientName != "" { - ws = append(ws, "validation_client_name is deprecated, please use user_name instead") - } if p.ValidationKey != "" { - ws = append(ws, "validation_key is deprecated, please use user_key instead") - if p.RecreateClient { es = append(es, errors.New( "Cannot use recreate_client=true with the deprecated validation_key, please provide a user_key")) @@ -303,38 +435,8 @@ func (r *ResourceProvisioner) Validate(c *terraform.ResourceConfig) (ws []string return ws, es } -func (r *ResourceProvisioner) decodeConfig(c *terraform.ResourceConfig) (*Provisioner, error) { - p := new(Provisioner) - - decConf := &mapstructure.DecoderConfig{ - ErrorUnused: true, - WeaklyTypedInput: true, - Result: p, - } - dec, err := mapstructure.NewDecoder(decConf) - if err != nil { - return nil, err - } - - // We need to merge both configs into a single map first. Order is - // important as we need to make sure interpolated values are used - // over raw values. This makes sure that all values are there even - // if some still need to be interpolated later on. Without this - // the validation will fail when using a variable for a required - // parameter (the node_name for example). - m := make(map[string]interface{}) - - for k, v := range c.Raw { - m[k] = v - } - - for k, v := range c.Config { - m[k] = v - } - - if err := dec.Decode(m); err != nil { - return nil, err - } +func decodeConfig(d *schema.ResourceData) (*ProvisionerS, error) { + p := decodeDataToProvisioner(d) // Make sure the supplied URL has a trailing slash p.ServerURL = strings.TrimSuffix(p.ServerURL, "/") + "/" @@ -359,17 +461,17 @@ func (r *ResourceProvisioner) decodeConfig(c *terraform.ResourceConfig) (*Provis p.UserKey = p.ValidationKey } - if attrs, ok := c.Config["attributes_json"].(string); ok && !c.IsComputed("attributes_json") { + if attrs, ok := d.GetOk("attributes_json"); ok { var m map[string]interface{} - if err := json.Unmarshal([]byte(attrs), &m); err != nil { + if err := json.Unmarshal([]byte(attrs.(string)), &m); err != nil { return nil, fmt.Errorf("Error parsing attributes_json: %v", err) } p.attributes = m } - if vaults, ok := c.Config["vault_json"].(string); ok && !c.IsComputed("vault_json") { + if vaults, ok := d.GetOk("vault_json"); ok { var m map[string]interface{} - if err := json.Unmarshal([]byte(vaults), &m); err != nil { + if err := json.Unmarshal([]byte(vaults.(string)), &m); err != nil { return nil, fmt.Errorf("Error parsing vault_json: %v", err) } @@ -395,7 +497,7 @@ func (r *ResourceProvisioner) decodeConfig(c *terraform.ResourceConfig) (*Provis return p, nil } -func (p *Provisioner) deployConfigFiles( +func (p *ProvisionerS) deployConfigFiles( o terraform.UIOutput, comm communicator.Communicator, confDir string) error { @@ -469,7 +571,7 @@ func (p *Provisioner) deployConfigFiles( return nil } -func (p *Provisioner) deployOhaiHints( +func (p *ProvisionerS) deployOhaiHints( o terraform.UIOutput, comm communicator.Communicator, hintDir string) error { @@ -490,7 +592,7 @@ func (p *Provisioner) deployOhaiHints( return nil } -func (p *Provisioner) fetchChefCertificatesFunc( +func (p *ProvisionerS) fetchChefCertificatesFunc( knifeCmd string, confDir string) func(terraform.UIOutput, communicator.Communicator) error { return func(o terraform.UIOutput, comm communicator.Communicator) error { @@ -501,7 +603,7 @@ func (p *Provisioner) fetchChefCertificatesFunc( } } -func (p *Provisioner) generateClientKeyFunc( +func (p *ProvisionerS) generateClientKeyFunc( knifeCmd string, confDir string, noOutput string) func(terraform.UIOutput, communicator.Communicator) error { @@ -562,7 +664,7 @@ func (p *Provisioner) generateClientKeyFunc( } } -func (p *Provisioner) configureVaultsFunc( +func (p *ProvisionerS) configureVaultsFunc( gemCmd string, knifeCmd string, confDir string) func(terraform.UIOutput, communicator.Communicator) error { @@ -596,7 +698,7 @@ func (p *Provisioner) configureVaultsFunc( } } -func (p *Provisioner) runChefClientFunc( +func (p *ProvisionerS) runChefClientFunc( chefCmd string, confDir string) func(terraform.UIOutput, communicator.Communicator) error { return func(o terraform.UIOutput, comm communicator.Communicator) error { @@ -634,7 +736,7 @@ func (p *Provisioner) runChefClientFunc( } // Output implementation of terraform.UIOutput interface -func (p *Provisioner) Output(output string) { +func (p *ProvisionerS) Output(output string) { logFile := path.Join(logfileDir, p.NodeName) f, err := os.OpenFile(logFile, os.O_APPEND|os.O_WRONLY, 0666) if err != nil { @@ -660,7 +762,7 @@ func (p *Provisioner) Output(output string) { } // runCommand is used to run already prepared commands -func (p *Provisioner) runCommand( +func (p *ProvisionerS) runCommand( o terraform.UIOutput, comm communicator.Communicator, command string) error { @@ -702,7 +804,7 @@ func (p *Provisioner) runCommand( return err } -func (p *Provisioner) copyOutput(o terraform.UIOutput, r io.Reader, doneCh chan<- struct{}) { +func (p *ProvisionerS) copyOutput(o terraform.UIOutput, r io.Reader, doneCh chan<- struct{}) { defer close(doneCh) lr := linereader.New(r) for line := range lr.Ch { @@ -727,3 +829,58 @@ func retryFunc(timeout time.Duration, f func() error) error { } } } + +func decodeDataToProvisioner(d *schema.ResourceData) *ProvisionerS { + return &ProvisionerS{ + AttributesJSON: d.Get("attributes_json").(string), + ClientOptions: getStringList(d.Get("client_options")), + DisableReporting: d.Get("disable_reporting").(bool), + Environment: d.Get("environment").(string), + FetchChefCertificates: d.Get("fetch_chef_certificates").(bool), + LogToFile: d.Get("log_to_file").(bool), + UsePolicyfile: d.Get("use_policyfile").(bool), + PolicyGroup: d.Get("policy_group").(string), + PolicyName: d.Get("policy_name").(string), + HTTPProxy: d.Get("http_proxy").(string), + HTTPSProxy: d.Get("https_proxy").(string), + NOProxy: getStringList(d.Get("no_proxy")), + NamedRunList: d.Get("named_run_list").(string), + NodeName: d.Get("node_name").(string), + OhaiHints: getStringList(d.Get("ohai_hints")), + OSType: d.Get("os_type").(string), + RecreateClient: d.Get("recreate_client").(bool), + PreventSudo: d.Get("prevent_sudo").(bool), + RunList: getStringList(d.Get("run_list")), + SecretKey: d.Get("secret_key").(string), + ServerURL: d.Get("server_url").(string), + SkipInstall: d.Get("skip_install").(bool), + SkipRegister: d.Get("skip_register").(bool), + SSLVerifyMode: d.Get("ssl_verify_mode").(string), + UserName: d.Get("user_name").(string), + UserKey: d.Get("user_key").(string), + VaultJSON: d.Get("vault_json").(string), + Version: d.Get("version").(string), + + // Deprecated + ValidationClientName: d.Get("validation_client_name").(string), + ValidationKey: d.Get("validation_key").(string), + } +} + +func getStringList(v interface{}) []string { + if v == nil { + return nil + } + switch l := v.(type) { + case []string: + return l + case []interface{}: + arr := make([]string, len(l)) + for i, x := range l { + arr[i] = x.(string) + } + return arr + default: + panic(fmt.Sprintf("Unsupported type: %T", v)) + } +} diff --git a/builtin/provisioners/chef/resource_provisioner_test.go b/builtin/provisioners/chef/resource_provisioner_test.go index 7c2dc3a2e..3bb728d5d 100644 --- a/builtin/provisioners/chef/resource_provisioner_test.go +++ b/builtin/provisioners/chef/resource_provisioner_test.go @@ -7,11 +7,18 @@ import ( "github.com/hashicorp/terraform/communicator" "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" ) func TestResourceProvisioner_impl(t *testing.T) { - var _ terraform.ResourceProvisioner = new(ResourceProvisioner) + var _ terraform.ResourceProvisioner = Provisioner() +} + +func TestProvisioner(t *testing.T) { + if err := Provisioner().(*schema.Provisioner).InternalValidate(); err != nil { + t.Fatalf("err: %s", err) + } } func TestResourceProvider_Validate_good(t *testing.T) { @@ -23,7 +30,7 @@ func TestResourceProvider_Validate_good(t *testing.T) { "user_name": "bob", "user_key": "USER-KEY", }) - r := new(ResourceProvisioner) + r := Provisioner() warn, errs := r.Validate(c) if len(warn) > 0 { t.Fatalf("Warnings: %v", warn) @@ -37,7 +44,7 @@ func TestResourceProvider_Validate_bad(t *testing.T) { c := testConfig(t, map[string]interface{}{ "invalid": "nope", }) - p := new(ResourceProvisioner) + p := Provisioner() warn, errs := p.Validate(c) if len(warn) > 0 { t.Fatalf("Warnings: %v", warn) @@ -59,7 +66,7 @@ func TestResourceProvider_Validate_computedValues(t *testing.T) { "user_key": "USER-KEY", "attributes_json": config.UnknownVariableValue, }) - r := new(ResourceProvisioner) + r := Provisioner() warn, errs := r.Validate(c) if len(warn) > 0 { t.Fatalf("Warnings: %v", warn) @@ -149,14 +156,13 @@ func TestResourceProvider_runChefClient(t *testing.T) { }, } - r := new(ResourceProvisioner) o := new(terraform.MockUIOutput) c := new(communicator.MockCommunicator) for k, tc := range cases { c.Commands = tc.Commands - p, err := r.decodeConfig(tc.Config) + p, err := decodeConfig(getTestResourceData(tc.Config)) if err != nil { t.Fatalf("Error: %v", err) } @@ -222,14 +228,13 @@ func TestResourceProvider_fetchChefCertificates(t *testing.T) { }, } - r := new(ResourceProvisioner) o := new(terraform.MockUIOutput) c := new(communicator.MockCommunicator) for k, tc := range cases { c.Commands = tc.Commands - p, err := r.decodeConfig(tc.Config) + p, err := decodeConfig(getTestResourceData(tc.Config)) if err != nil { t.Fatalf("Error: %v", err) } @@ -347,14 +352,13 @@ func TestResourceProvider_configureVaults(t *testing.T) { }, } - r := new(ResourceProvisioner) o := new(terraform.MockUIOutput) c := new(communicator.MockCommunicator) for k, tc := range cases { c.Commands = tc.Commands - p, err := r.decodeConfig(tc.Config) + p, err := decodeConfig(getTestResourceData(tc.Config)) if err != nil { t.Fatalf("Error: %v", err) } @@ -368,3 +372,7 @@ func TestResourceProvider_configureVaults(t *testing.T) { } } } + +func getTestResourceData(c *terraform.ResourceConfig) *schema.ResourceData { + return schema.TestResourceDataConfig(Provisioner().(*schema.Provisioner).Schema, c) +} diff --git a/builtin/provisioners/chef/windows_provisioner.go b/builtin/provisioners/chef/windows_provisioner.go index 953734021..773d1b0f2 100644 --- a/builtin/provisioners/chef/windows_provisioner.go +++ b/builtin/provisioners/chef/windows_provisioner.go @@ -46,7 +46,7 @@ Write-Host 'Installing Chef Client...' Start-Process -FilePath msiexec -ArgumentList /qn, /i, $dest -Wait ` -func (p *Provisioner) windowsInstallChefClient( +func (p *ProvisionerS) windowsInstallChefClient( o terraform.UIOutput, comm communicator.Communicator) error { script := path.Join(path.Dir(comm.ScriptPath()), "ChefClient.ps1") @@ -62,7 +62,7 @@ func (p *Provisioner) windowsInstallChefClient( return p.runCommand(o, comm, installCmd) } -func (p *Provisioner) windowsCreateConfigFiles( +func (p *ProvisionerS) windowsCreateConfigFiles( o terraform.UIOutput, comm communicator.Communicator) error { // Make sure the config directory exists diff --git a/builtin/provisioners/chef/windows_provisioner_test.go b/builtin/provisioners/chef/windows_provisioner_test.go index 659953d97..0bb0dc126 100644 --- a/builtin/provisioners/chef/windows_provisioner_test.go +++ b/builtin/provisioners/chef/windows_provisioner_test.go @@ -73,7 +73,6 @@ func TestResourceProvider_windowsInstallChefClient(t *testing.T) { }, } - r := new(ResourceProvisioner) o := new(terraform.MockUIOutput) c := new(communicator.MockCommunicator) @@ -81,7 +80,7 @@ func TestResourceProvider_windowsInstallChefClient(t *testing.T) { c.Commands = tc.Commands c.UploadScripts = tc.UploadScripts - p, err := r.decodeConfig(tc.Config) + p, err := decodeConfig(getTestResourceData(tc.Config)) if err != nil { t.Fatalf("Error: %v", err) } @@ -180,7 +179,6 @@ func TestResourceProvider_windowsCreateConfigFiles(t *testing.T) { }, } - r := new(ResourceProvisioner) o := new(terraform.MockUIOutput) c := new(communicator.MockCommunicator) @@ -188,7 +186,7 @@ func TestResourceProvider_windowsCreateConfigFiles(t *testing.T) { c.Commands = tc.Commands c.Uploads = tc.Uploads - p, err := r.decodeConfig(tc.Config) + p, err := decodeConfig(getTestResourceData(tc.Config)) if err != nil { t.Fatalf("Error: %v", err) } diff --git a/builtin/provisioners/file/resource_provisioner.go b/builtin/provisioners/file/resource_provisioner.go index 001e78af5..eb29424b4 100644 --- a/builtin/provisioners/file/resource_provisioner.go +++ b/builtin/provisioners/file/resource_provisioner.go @@ -35,7 +35,8 @@ func Provisioner() terraform.ResourceProvisioner { }, }, - ApplyFunc: applyFn, + ApplyFunc: applyFn, + ValidateFunc: Validate, } } @@ -77,6 +78,21 @@ func applyFn(ctx context.Context) error { } } +// Validate checks if the required arguments are configured +func Validate(d *schema.ResourceData) (ws []string, es []error) { + numSrc := 0 + if _, ok := d.GetOk("source"); ok == true { + numSrc++ + } + if _, ok := d.GetOk("content"); ok == true { + numSrc++ + } + if numSrc != 1 { + es = append(es, fmt.Errorf("Must provide one of 'content' or 'source'")) + } + return +} + // getSrc returns the file to use as source func getSrc(data *schema.ResourceData) (string, bool, error) { src := data.Get("source").(string) diff --git a/builtin/provisioners/file/resource_provisioner_test.go b/builtin/provisioners/file/resource_provisioner_test.go index d57dbbaa5..96afc519f 100644 --- a/builtin/provisioners/file/resource_provisioner_test.go +++ b/builtin/provisioners/file/resource_provisioner_test.go @@ -4,9 +4,20 @@ import ( "testing" "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" ) +func TestResourceProvisioner_impl(t *testing.T) { + var _ terraform.ResourceProvisioner = Provisioner() +} + +func TestProvisioner(t *testing.T) { + if err := Provisioner().(*schema.Provisioner).InternalValidate(); err != nil { + t.Fatalf("err: %s", err) + } +} + func TestResourceProvider_Validate_good_source(t *testing.T) { c := testConfig(t, map[string]interface{}{ "source": "/tmp/foo", @@ -51,6 +62,20 @@ func TestResourceProvider_Validate_bad_not_destination(t *testing.T) { } } +func TestResourceProvider_Validate_bad_no_source(t *testing.T) { + c := testConfig(t, map[string]interface{}{ + "destination": "/tmp/bar", + }) + p := Provisioner() + warn, errs := p.Validate(c) + if len(warn) > 0 { + t.Fatalf("Warnings: %v", warn) + } + if len(errs) == 0 { + t.Fatalf("Should have errors") + } +} + func TestResourceProvider_Validate_bad_to_many_src(t *testing.T) { c := testConfig(t, map[string]interface{}{ "source": "nope", diff --git a/builtin/provisioners/local-exec/resource_provisioner_test.go b/builtin/provisioners/local-exec/resource_provisioner_test.go index 2dff4a85d..85fb6abcf 100644 --- a/builtin/provisioners/local-exec/resource_provisioner_test.go +++ b/builtin/provisioners/local-exec/resource_provisioner_test.go @@ -8,9 +8,20 @@ import ( "time" "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" ) +func TestResourceProvisioner_impl(t *testing.T) { + var _ terraform.ResourceProvisioner = Provisioner() +} + +func TestProvisioner(t *testing.T) { + if err := Provisioner().(*schema.Provisioner).InternalValidate(); err != nil { + t.Fatalf("err: %s", err) + } +} + func TestResourceProvider_Apply(t *testing.T) { defer os.Remove("test_out") c := testConfig(t, map[string]interface{}{ diff --git a/builtin/provisioners/remote-exec/resource_provisioner_test.go b/builtin/provisioners/remote-exec/resource_provisioner_test.go index aa69cad61..53e943680 100644 --- a/builtin/provisioners/remote-exec/resource_provisioner_test.go +++ b/builtin/provisioners/remote-exec/resource_provisioner_test.go @@ -16,6 +16,16 @@ import ( "github.com/hashicorp/terraform/terraform" ) +func TestResourceProvisioner_impl(t *testing.T) { + var _ terraform.ResourceProvisioner = Provisioner() +} + +func TestProvisioner(t *testing.T) { + if err := Provisioner().(*schema.Provisioner).InternalValidate(); err != nil { + t.Fatalf("err: %s", err) + } +} + func TestResourceProvider_Validate_good(t *testing.T) { c := testConfig(t, map[string]interface{}{ "inline": "echo foo", diff --git a/helper/schema/provisioner.go b/helper/schema/provisioner.go index c1564a215..926df8893 100644 --- a/helper/schema/provisioner.go +++ b/helper/schema/provisioner.go @@ -41,6 +41,11 @@ type Provisioner struct { // information. ApplyFunc func(ctx context.Context) error + // ValidateFunc is the function for extended validation. This is optional. + // It is given a resource data. + // Should be provided when Scheme is not enough. + ValidateFunc func(*ResourceData) ([]string, []error) + stopCtx context.Context stopCtxCancel context.CancelFunc stopOnce sync.Once @@ -117,8 +122,30 @@ func (p *Provisioner) Stop() error { return nil } -func (p *Provisioner) Validate(c *terraform.ResourceConfig) ([]string, []error) { - return schemaMap(p.Schema).Validate(c) +func (p *Provisioner) Validate(config *terraform.ResourceConfig) ([]string, []error) { + if err := p.InternalValidate(); err != nil { + return nil, []error{fmt.Errorf( + "Internal validation of the provisioner failed! This is always a bug\n"+ + "with the provisioner itself, and not a user issue. Please report\n"+ + "this bug:\n\n%s", err)} + } + w := []string{} + e := []error{} + if p.Schema != nil { + w2, e2 := schemaMap(p.Schema).Validate(config) + w = append(w, w2...) + e = append(e, e2...) + } + if p.ValidateFunc != nil { + data := &ResourceData{ + schema: p.Schema, + config: config, + } + w2, e2 := p.ValidateFunc(data) + w = append(w, w2...) + e = append(e, e2...) + } + return w, e } // Apply implementation of terraform.ResourceProvisioner interface. diff --git a/helper/schema/provisioner_test.go b/helper/schema/provisioner_test.go index d8448acef..585586b7d 100644 --- a/helper/schema/provisioner_test.go +++ b/helper/schema/provisioner_test.go @@ -3,6 +3,7 @@ package schema import ( "context" "fmt" + "reflect" "testing" "time" @@ -14,13 +15,35 @@ func TestProvisioner_impl(t *testing.T) { var _ terraform.ResourceProvisioner = new(Provisioner) } +func noopApply(ctx context.Context) error { + return nil +} + func TestProvisionerValidate(t *testing.T) { cases := []struct { Name string P *Provisioner Config map[string]interface{} Err bool + Warns []string }{ + { + Name: "No ApplyFunc", + P: &Provisioner{}, + Config: nil, + Err: true, + }, + { + Name: "Incorrect schema", + P: &Provisioner{ + Schema: map[string]*Schema{ + "foo": {}, + }, + ApplyFunc: noopApply, + }, + Config: nil, + Err: true, + }, { "Basic required field", &Provisioner{ @@ -30,9 +53,11 @@ func TestProvisionerValidate(t *testing.T) { Type: TypeString, }, }, + ApplyFunc: noopApply, }, nil, true, + nil, }, { @@ -44,11 +69,57 @@ func TestProvisionerValidate(t *testing.T) { Type: TypeString, }, }, + ApplyFunc: noopApply, }, map[string]interface{}{ "foo": "bar", }, false, + nil, + }, + { + Name: "Warning from property validation", + P: &Provisioner{ + Schema: map[string]*Schema{ + "foo": { + Type: TypeString, + Optional: true, + ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) { + ws = append(ws, "Simple warning from property validation") + return + }, + }, + }, + ApplyFunc: noopApply, + }, + Config: map[string]interface{}{ + "foo": "", + }, + Err: false, + Warns: []string{"Simple warning from property validation"}, + }, + { + Name: "No schema", + P: &Provisioner{ + Schema: nil, + ApplyFunc: noopApply, + }, + Config: nil, + Err: false, + }, + { + Name: "Warning from provisioner ValidateFunc", + P: &Provisioner{ + Schema: nil, + ApplyFunc: noopApply, + ValidateFunc: func(*ResourceData) (ws []string, errors []error) { + ws = append(ws, "Simple warning from provisioner ValidateFunc") + return + }, + }, + Config: nil, + Err: false, + Warns: []string{"Simple warning from provisioner ValidateFunc"}, }, } @@ -59,9 +130,12 @@ func TestProvisionerValidate(t *testing.T) { t.Fatalf("err: %s", err) } - _, es := tc.P.Validate(terraform.NewResourceConfig(c)) + ws, es := tc.P.Validate(terraform.NewResourceConfig(c)) if len(es) > 0 != tc.Err { - t.Fatalf("%d: %#v", i, es) + t.Fatalf("%d: %#v %s", i, es, es) + } + if (tc.Warns != nil || len(ws) != 0) && !reflect.DeepEqual(ws, tc.Warns) { + t.Fatalf("%d: warnings mismatch, actual: %#v", i, ws) } }) } diff --git a/helper/schema/testing.go b/helper/schema/testing.go index 9765bdbc6..bbc7b0cb8 100644 --- a/helper/schema/testing.go +++ b/helper/schema/testing.go @@ -28,3 +28,10 @@ func TestResourceDataRaw( return result } + +func TestResourceDataConfig(schema map[string]*Schema, config *terraform.ResourceConfig) *ResourceData { + return &ResourceData{ + schema: schema, + config: config, + } +} From df4342bc3d309551a835d79579861e764e65e37e Mon Sep 17 00:00:00 2001 From: Vladislav Rassokhin Date: Tue, 29 Nov 2016 23:03:21 +0300 Subject: [PATCH 2/3] Regenerate plugin list since provisioners were changed in previous commits --- command/internal_plugin_list.go | 9 ++------- scripts/generate-plugins.go | 17 ++++------------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/command/internal_plugin_list.go b/command/internal_plugin_list.go index 0ac40837b..271bc8f6c 100644 --- a/command/internal_plugin_list.go +++ b/command/internal_plugin_list.go @@ -75,6 +75,7 @@ import ( vaultprovider "github.com/hashicorp/terraform/builtin/providers/vault" vcdprovider "github.com/hashicorp/terraform/builtin/providers/vcd" vsphereprovider "github.com/hashicorp/terraform/builtin/providers/vsphere" + chefprovisioner "github.com/hashicorp/terraform/builtin/provisioners/chef" fileprovisioner "github.com/hashicorp/terraform/builtin/provisioners/file" localexecprovisioner "github.com/hashicorp/terraform/builtin/provisioners/local-exec" remoteexecprovisioner "github.com/hashicorp/terraform/builtin/provisioners/remote-exec" @@ -84,9 +85,6 @@ import ( //New Provider Builds opcprovider "github.com/hashicorp/terraform-provider-opc/opc" - - // Legacy, will remove once it conforms with new structure - chefprovisioner "github.com/hashicorp/terraform/builtin/provisioners/chef" ) var InternalProviders = map[string]plugin.ProviderFunc{ @@ -162,16 +160,13 @@ var InternalProviders = map[string]plugin.ProviderFunc{ } var InternalProvisioners = map[string]plugin.ProvisionerFunc{ + "chef": chefprovisioner.Provisioner, "file": fileprovisioner.Provisioner, "local-exec": localexecprovisioner.Provisioner, "remote-exec": remoteexecprovisioner.Provisioner, } func init() { - // Legacy provisioners that don't match our heuristics for auto-finding - // built-in provisioners. - InternalProvisioners["chef"] = func() terraform.ResourceProvisioner { return new(chefprovisioner.ResourceProvisioner) } - // New Provider Layouts InternalProviders["opc"] = func() terraform.ResourceProvider { return opcprovider.Provider() } } diff --git a/scripts/generate-plugins.go b/scripts/generate-plugins.go index b4a0bc9a4..415123891 100644 --- a/scripts/generate-plugins.go +++ b/scripts/generate-plugins.go @@ -82,12 +82,11 @@ func makeProviderMap(items []plugin) string { // makeProvisionerMap creates a map of provisioners like this: // -// "file": func() terraform.ResourceProvisioner { return new(file.ResourceProvisioner) }, -// "local-exec": func() terraform.ResourceProvisioner { return new(localexec.ResourceProvisioner) }, -// "remote-exec": func() terraform.ResourceProvisioner { return new(remoteexec.ResourceProvisioner) }, +// "chef": chefprovisioner.Provisioner, +// "file": fileprovisioner.Provisioner, +// "local-exec": localexecprovisioner.Provisioner, +// "remote-exec": remoteexecprovisioner.Provisioner, // -// This is more verbose than the Provider case because there is no corresponding -// Provisioner function. func makeProvisionerMap(items []plugin) string { output := "" for _, item := range items { @@ -273,9 +272,6 @@ IMPORTS //New Provider Builds opcprovider "github.com/hashicorp/terraform-provider-opc/opc" - - // Legacy, will remove once it conforms with new structure - chefprovisioner "github.com/hashicorp/terraform/builtin/provisioners/chef" ) var InternalProviders = map[string]plugin.ProviderFunc{ @@ -287,12 +283,7 @@ PROVISIONERS } func init() { - // Legacy provisioners that don't match our heuristics for auto-finding - // built-in provisioners. - InternalProvisioners["chef"] = func() terraform.ResourceProvisioner { return new(chefprovisioner.ResourceProvisioner) } - // New Provider Layouts InternalProviders["opc"] = func() terraform.ResourceProvider { return opcprovider.Provider() } } - ` From 0e422737baa8f23919d5a7fc2c1e2772d3a32d14 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Fri, 19 May 2017 20:42:14 +0200 Subject: [PATCH 3/3] Fix and refactor the Chef provisioner The tests did pass, but that was because they only tested part of the changes. By using the `schema.TestResourceDataRaw` function the schema and config are better tested and so they pointed out a problem with the schema of the Chef provisioner. The `Elem` fields did not have a `*schema.Schema` but a `schema.Schema` and in an `Elem` schema only the `Type` field may (and must) be set. Any other fields like `Optional` are not allowed here. Next to fixing that problem I also did a little refactoring and cleaning up. Mainly making the `ProvisionerS` private (`provisioner`) and removing the deprecated fields. --- .../provisioners/chef/linux_provisioner.go | 11 +- .../chef/linux_provisioner_test.go | 53 ++- .../provisioners/chef/resource_provisioner.go | 430 +++++++----------- .../chef/resource_provisioner_test.go | 84 ++-- .../provisioners/chef/windows_provisioner.go | 8 +- .../chef/windows_provisioner_test.go | 37 +- .../provisioners/file/resource_provisioner.go | 5 +- .../file/resource_provisioner_test.go | 25 +- .../local-exec/resource_provisioner_test.go | 13 +- .../remote-exec/resource_provisioner_test.go | 37 +- helper/schema/provisioner.go | 5 +- helper/schema/testing.go | 7 - .../docs/provisioners/chef.html.markdown | 6 - 13 files changed, 310 insertions(+), 411 deletions(-) diff --git a/builtin/provisioners/chef/linux_provisioner.go b/builtin/provisioners/chef/linux_provisioner.go index a9ad806a0..6d87cfe8a 100644 --- a/builtin/provisioners/chef/linux_provisioner.go +++ b/builtin/provisioners/chef/linux_provisioner.go @@ -14,10 +14,7 @@ const ( installURL = "https://www.chef.io/chef/install.sh" ) -func (p *ProvisionerS) linuxInstallChefClient( - o terraform.UIOutput, - comm communicator.Communicator) error { - +func (p *provisioner) linuxInstallChefClient(o terraform.UIOutput, comm communicator.Communicator) error { // Build up the command prefix prefix := "" if p.HTTPProxy != "" { @@ -26,7 +23,7 @@ func (p *ProvisionerS) linuxInstallChefClient( if p.HTTPSProxy != "" { prefix += fmt.Sprintf("https_proxy='%s' ", p.HTTPSProxy) } - if p.NOProxy != nil && len(p.NOProxy) > 0 { + if len(p.NOProxy) > 0 { prefix += fmt.Sprintf("no_proxy='%s' ", strings.Join(p.NOProxy, ",")) } @@ -46,9 +43,7 @@ func (p *ProvisionerS) linuxInstallChefClient( return p.runCommand(o, comm, fmt.Sprintf("%srm -f install.sh", prefix)) } -func (p *ProvisionerS) linuxCreateConfigFiles( - o terraform.UIOutput, - comm communicator.Communicator) error { +func (p *provisioner) linuxCreateConfigFiles(o terraform.UIOutput, comm communicator.Communicator) error { // Make sure the config directory exists if err := p.runCommand(o, comm, "mkdir -p "+linuxConfDir); err != nil { return err diff --git a/builtin/provisioners/chef/linux_provisioner_test.go b/builtin/provisioners/chef/linux_provisioner_test.go index 444a0f39a..578bb69ff 100644 --- a/builtin/provisioners/chef/linux_provisioner_test.go +++ b/builtin/provisioners/chef/linux_provisioner_test.go @@ -6,22 +6,23 @@ import ( "testing" "github.com/hashicorp/terraform/communicator" + "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" ) func TestResourceProvider_linuxInstallChefClient(t *testing.T) { cases := map[string]struct { - Config *terraform.ResourceConfig + Config map[string]interface{} Commands map[string]bool }{ "Sudo": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "node_name": "nodename1", "run_list": []interface{}{"cookbook::recipe"}, "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, Commands: map[string]bool{ "sudo curl -LO https://www.chef.io/chef/install.sh": true, @@ -31,7 +32,7 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { }, "NoSudo": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "node_name": "nodename1", "prevent_sudo": true, "run_list": []interface{}{"cookbook::recipe"}, @@ -39,7 +40,7 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, Commands: map[string]bool{ "curl -LO https://www.chef.io/chef/install.sh": true, @@ -49,7 +50,7 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { }, "HTTPProxy": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "http_proxy": "http://proxy.local", "node_name": "nodename1", "prevent_sudo": true, @@ -57,7 +58,7 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, Commands: map[string]bool{ "http_proxy='http://proxy.local' curl -LO https://www.chef.io/chef/install.sh": true, @@ -67,7 +68,7 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { }, "HTTPSProxy": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "https_proxy": "https://proxy.local", "node_name": "nodename1", "prevent_sudo": true, @@ -75,7 +76,7 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, Commands: map[string]bool{ "https_proxy='https://proxy.local' curl -LO https://www.chef.io/chef/install.sh": true, @@ -85,7 +86,7 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { }, "NoProxy": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "http_proxy": "http://proxy.local", "no_proxy": []interface{}{"http://local.local", "http://local.org"}, "node_name": "nodename1", @@ -94,7 +95,7 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, Commands: map[string]bool{ "http_proxy='http://proxy.local' no_proxy='http://local.local,http://local.org' " + @@ -107,7 +108,7 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { }, "Version": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "node_name": "nodename1", "prevent_sudo": true, "run_list": []interface{}{"cookbook::recipe"}, @@ -115,7 +116,7 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { "user_name": "bob", "user_key": "USER-KEY", "version": "11.18.6", - }), + }, Commands: map[string]bool{ "curl -LO https://www.chef.io/chef/install.sh": true, @@ -131,7 +132,9 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { for k, tc := range cases { c.Commands = tc.Commands - p, err := decodeConfig(getTestResourceData(tc.Config)) + p, err := decodeConfig( + schema.TestResourceDataRaw(t, Provisioner().(*schema.Provisioner).Schema, tc.Config), + ) if err != nil { t.Fatalf("Error: %v", err) } @@ -147,12 +150,12 @@ func TestResourceProvider_linuxInstallChefClient(t *testing.T) { func TestResourceProvider_linuxCreateConfigFiles(t *testing.T) { cases := map[string]struct { - Config *terraform.ResourceConfig + Config map[string]interface{} Commands map[string]bool Uploads map[string]string }{ "Sudo": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "ohai_hints": []interface{}{"test-fixtures/ohaihint.json"}, "node_name": "nodename1", "run_list": []interface{}{"cookbook::recipe"}, @@ -160,7 +163,7 @@ func TestResourceProvider_linuxCreateConfigFiles(t *testing.T) { "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, Commands: map[string]bool{ "sudo mkdir -p " + linuxConfDir: true, @@ -187,7 +190,7 @@ func TestResourceProvider_linuxCreateConfigFiles(t *testing.T) { }, "NoSudo": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "node_name": "nodename1", "prevent_sudo": true, "run_list": []interface{}{"cookbook::recipe"}, @@ -195,7 +198,7 @@ func TestResourceProvider_linuxCreateConfigFiles(t *testing.T) { "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, Commands: map[string]bool{ "mkdir -p " + linuxConfDir: true, @@ -210,7 +213,7 @@ func TestResourceProvider_linuxCreateConfigFiles(t *testing.T) { }, "Proxy": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "http_proxy": "http://proxy.local", "https_proxy": "https://proxy.local", "no_proxy": []interface{}{"http://local.local", "https://local.local"}, @@ -222,7 +225,7 @@ func TestResourceProvider_linuxCreateConfigFiles(t *testing.T) { "ssl_verify_mode": "verify_none", "user_name": "bob", "user_key": "USER-KEY", - }), + }, Commands: map[string]bool{ "mkdir -p " + linuxConfDir: true, @@ -237,7 +240,7 @@ func TestResourceProvider_linuxCreateConfigFiles(t *testing.T) { }, "Attributes JSON": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "attributes_json": `{"key1":{"subkey1":{"subkey2a":["val1","val2","val3"],` + `"subkey2b":{"subkey3":"value3"}}},"key2":"value2"}`, "node_name": "nodename1", @@ -247,7 +250,7 @@ func TestResourceProvider_linuxCreateConfigFiles(t *testing.T) { "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, Commands: map[string]bool{ "mkdir -p " + linuxConfDir: true, @@ -270,7 +273,9 @@ func TestResourceProvider_linuxCreateConfigFiles(t *testing.T) { c.Commands = tc.Commands c.Uploads = tc.Uploads - p, err := decodeConfig(getTestResourceData(tc.Config)) + p, err := decodeConfig( + schema.TestResourceDataRaw(t, Provisioner().(*schema.Provisioner).Schema, tc.Config), + ) if err != nil { t.Fatalf("Error: %v", err) } diff --git a/builtin/provisioners/chef/resource_provisioner.go b/builtin/provisioners/chef/resource_provisioner.go index 28302ecac..7df7e93ca 100644 --- a/builtin/provisioners/chef/resource_provisioner.go +++ b/builtin/provisioners/chef/resource_provisioner.go @@ -2,6 +2,7 @@ package chef import ( "bytes" + "context" "encoding/json" "errors" "fmt" @@ -15,7 +16,6 @@ import ( "text/template" "time" - "context" "github.com/hashicorp/terraform/communicator" "github.com/hashicorp/terraform/communicator/remote" "github.com/hashicorp/terraform/helper/schema" @@ -82,9 +82,10 @@ enable_reporting false {{ end }} ` -// ProvisionerS represents a Chef provisioner -type ProvisionerS struct { - AttributesJSON string +type provisionFn func(terraform.UIOutput, communicator.Communicator) error + +type provisioner struct { + Attributes map[string]interface{} ClientOptions []string DisableReporting bool Environment string @@ -110,180 +111,153 @@ type ProvisionerS struct { SSLVerifyMode string UserName string UserKey string - VaultJSON string + Vaults map[string][]string Version string - attributes map[string]interface{} - vaults map[string][]string - cleanupUserKeyCmd string - createConfigFiles func(terraform.UIOutput, communicator.Communicator) error - installChefClient func(terraform.UIOutput, communicator.Communicator) error - fetchChefCertificates func(terraform.UIOutput, communicator.Communicator) error - generateClientKey func(terraform.UIOutput, communicator.Communicator) error - configureVaults func(terraform.UIOutput, communicator.Communicator) error - runChefClient func(terraform.UIOutput, communicator.Communicator) error + createConfigFiles provisionFn + installChefClient provisionFn + fetchChefCertificates provisionFn + generateClientKey provisionFn + configureVaults provisionFn + runChefClient provisionFn useSudo bool - - // Deprecated Fields - ValidationClientName string - ValidationKey string } +// Provisioner returns a Chef provisioner func Provisioner() terraform.ResourceProvisioner { return &schema.Provisioner{ Schema: map[string]*schema.Schema{ - "attributes_json": { - Type: schema.TypeString, - Optional: true, - }, - "client_options": { - Type: schema.TypeList, - Elem: schema.Schema{ - Type: schema.TypeString, - }, - Optional: true, - }, - "disable_reporting": { - Type: schema.TypeBool, - Optional: true, - }, - "environment": { - Type: schema.TypeString, - Optional: true, - }, - "fetch_chef_certificates": { - Type: schema.TypeBool, - Optional: true, - }, - "log_to_file": { - Type: schema.TypeBool, - Optional: true, - }, - "use_policyfile": { - Type: schema.TypeBool, - Optional: true, - }, - "policy_group": { - Type: schema.TypeString, - Optional: true, - }, - "policy_name": { - Type: schema.TypeString, - Optional: true, - }, - "http_proxy": { - Type: schema.TypeString, - Optional: true, - }, - "https_proxy": { - Type: schema.TypeString, - Optional: true, - }, - "no_proxy": { - Type: schema.TypeList, - Elem: schema.Schema{ - Type: schema.TypeString, - Optional: true, - }, - Optional: true, - }, - "named_run_list": { - Type: schema.TypeString, - Optional: true, - }, - "node_name": { + "node_name": &schema.Schema{ Type: schema.TypeString, Required: true, }, - "ohai_hints": { - Type: schema.TypeList, - Elem: schema.Schema{ - Type: schema.TypeString, - Optional: true, - }, - Optional: true, - }, - "os_type": { - Type: schema.TypeString, - Optional: true, - }, - "recreate_client": { - Type: schema.TypeBool, - Optional: true, - }, - "prevent_sudo": { - Type: schema.TypeBool, - Optional: true, - }, - "run_list": { - Type: schema.TypeList, - Elem: schema.Schema{ - Type: schema.TypeString, - Optional: true, - }, - Optional: true, - }, - "secret_key": { - Type: schema.TypeString, - Optional: true, - }, - "server_url": { + "server_url": &schema.Schema{ Type: schema.TypeString, Required: true, }, - "skip_install": { + "user_name": &schema.Schema{ + Type: schema.TypeString, + Required: true, + }, + "user_key": &schema.Schema{ + Type: schema.TypeString, + Required: true, + }, + + "attributes_json": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + "client_options": &schema.Schema{ + Type: schema.TypeList, + Elem: &schema.Schema{Type: schema.TypeString}, + Optional: true, + }, + "disable_reporting": &schema.Schema{ Type: schema.TypeBool, Optional: true, }, - "skip_register": { + "environment": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + Default: defaultEnv, + }, + "fetch_chef_certificates": &schema.Schema{ Type: schema.TypeBool, Optional: true, }, - "ssl_verify_mode": { + "log_to_file": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + }, + "use_policyfile": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + }, + "policy_group": &schema.Schema{ Type: schema.TypeString, Optional: true, }, - "user_name": { + "policy_name": &schema.Schema{ Type: schema.TypeString, Optional: true, }, - "user_key": { + "http_proxy": &schema.Schema{ Type: schema.TypeString, Optional: true, }, - "vault_json": { + "https_proxy": &schema.Schema{ Type: schema.TypeString, Optional: true, }, - "version": { + "no_proxy": &schema.Schema{ + Type: schema.TypeList, + Elem: &schema.Schema{Type: schema.TypeString}, + Optional: true, + }, + "named_run_list": &schema.Schema{ Type: schema.TypeString, Optional: true, }, - - // Deprecated - "validation_client_name": { - Type: schema.TypeString, - Deprecated: "Please use user_name instead", - Optional: true, + "ohai_hints": &schema.Schema{ + Type: schema.TypeList, + Elem: &schema.Schema{Type: schema.TypeString}, + Optional: true, }, - - "validation_key": { - Type: schema.TypeString, - Deprecated: "Please use user_key instead", - Optional: true, + "os_type": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + "recreate_client": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + }, + "prevent_sudo": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + }, + "run_list": &schema.Schema{ + Type: schema.TypeList, + Elem: &schema.Schema{Type: schema.TypeString}, + Optional: true, + }, + "secret_key": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + "skip_install": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + }, + "skip_register": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + }, + "ssl_verify_mode": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + "vault_json": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + "version": &schema.Schema{ + Type: schema.TypeString, + Optional: true, }, }, - ApplyFunc: Apply, - ValidateFunc: Validate, + + ApplyFunc: applyFn, + ValidateFunc: validateFn, } } // TODO: Support context cancelling (Provisioner Stop) -// Apply executes the file provisioner -func Apply(ctx context.Context) error { +func applyFn(ctx context.Context) error { o := ctx.Value(schema.ProvOutputKey).(terraform.UIOutput) d := ctx.Value(schema.ProvConfigDataKey).(*schema.ResourceData) + // Decode the raw config for this provisioner p, err := decodeConfig(d) if err != nil { @@ -291,8 +265,7 @@ func Apply(ctx context.Context) error { } if p.OSType == "" { - t := d.State().Ephemeral.ConnInfo["type"] - switch t { + switch t := d.State().Ephemeral.ConnInfo["type"]; t { case "ssh", "": // The default connection type is ssh, so if the type is empty assume ssh p.OSType = "linux" case "winrm": @@ -334,8 +307,7 @@ func Apply(ctx context.Context) error { // Wait and retry until we establish the connection err = retryFunc(comm.Timeout(), func() error { - err := comm.Connect(o) - return err + return comm.Connect(o) }) if err != nil { return err @@ -377,7 +349,7 @@ func Apply(ctx context.Context) error { } } - if p.VaultJSON != "" { + if p.Vaults != nil { o.Output("Configure Chef vaults...") if err := p.configureVaults(o, comm); err != nil { return err @@ -396,8 +368,7 @@ func Apply(ctx context.Context) error { return nil } -// Validate checks if the required arguments are configured -func Validate(d *schema.ResourceData) (ws []string, es []error) { +func validateFn(d *schema.ResourceData) (ws []string, es []error) { p, err := decodeConfig(d) if err != nil { es = append(es, err) @@ -413,94 +384,11 @@ func Validate(d *schema.ResourceData) (ws []string, es []error) { if p.UsePolicyfile && p.PolicyGroup == "" { es = append(es, errors.New("Policyfile enabled but key not found: policy_group")) } - if p.UserName == "" && p.ValidationClientName == "" { - es = append(es, errors.New( - "One of user_name or the deprecated validation_client_name must be provided")) - } - if p.UserKey == "" && p.ValidationKey == "" { - es = append(es, errors.New( - "One of user_key or the deprecated validation_key must be provided")) - } - if p.ValidationKey != "" { - if p.RecreateClient { - es = append(es, errors.New( - "Cannot use recreate_client=true with the deprecated validation_key, please provide a user_key")) - } - if p.VaultJSON != "" { - es = append(es, errors.New( - "Cannot configure chef vaults using the deprecated validation_key, please provide a user_key")) - } - } return ws, es } -func decodeConfig(d *schema.ResourceData) (*ProvisionerS, error) { - p := decodeDataToProvisioner(d) - - // Make sure the supplied URL has a trailing slash - p.ServerURL = strings.TrimSuffix(p.ServerURL, "/") + "/" - - if p.Environment == "" { - p.Environment = defaultEnv - } - - for i, hint := range p.OhaiHints { - hintPath, err := homedir.Expand(hint) - if err != nil { - return nil, fmt.Errorf("Error expanding the path %s: %v", hint, err) - } - p.OhaiHints[i] = hintPath - } - - if p.UserName == "" && p.ValidationClientName != "" { - p.UserName = p.ValidationClientName - } - - if p.UserKey == "" && p.ValidationKey != "" { - p.UserKey = p.ValidationKey - } - - if attrs, ok := d.GetOk("attributes_json"); ok { - var m map[string]interface{} - if err := json.Unmarshal([]byte(attrs.(string)), &m); err != nil { - return nil, fmt.Errorf("Error parsing attributes_json: %v", err) - } - p.attributes = m - } - - if vaults, ok := d.GetOk("vault_json"); ok { - var m map[string]interface{} - if err := json.Unmarshal([]byte(vaults.(string)), &m); err != nil { - return nil, fmt.Errorf("Error parsing vault_json: %v", err) - } - - v := make(map[string][]string) - for vault, items := range m { - switch items := items.(type) { - case []interface{}: - for _, item := range items { - if item, ok := item.(string); ok { - v[vault] = append(v[vault], item) - } - } - case interface{}: - if item, ok := items.(string); ok { - v[vault] = append(v[vault], item) - } - } - } - - p.vaults = v - } - - return p, nil -} - -func (p *ProvisionerS) deployConfigFiles( - o terraform.UIOutput, - comm communicator.Communicator, - confDir string) error { +func (p *provisioner) deployConfigFiles(o terraform.UIOutput, comm communicator.Communicator, confDir string) error { // Copy the user key to the new instance pk := strings.NewReader(p.UserKey) if err := comm.Upload(path.Join(confDir, p.UserName+".pem"), pk); err != nil { @@ -535,14 +423,14 @@ func (p *ProvisionerS) deployConfigFiles( } // Copy the client config to the new instance - if err := comm.Upload(path.Join(confDir, clienrb), &buf); err != nil { + 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 fb := make(map[string]interface{}) - if p.attributes != nil { - fb = p.attributes + if p.Attributes != nil { + fb = p.Attributes } // Check if the run_list was also in the attributes and if so log a warning @@ -571,10 +459,7 @@ func (p *ProvisionerS) deployConfigFiles( return nil } -func (p *ProvisionerS) deployOhaiHints( - o terraform.UIOutput, - comm communicator.Communicator, - hintDir string) error { +func (p *provisioner) deployOhaiHints(o terraform.UIOutput, comm communicator.Communicator, hintDir string) error { for _, hint := range p.OhaiHints { // Open the hint file f, err := os.Open(hint) @@ -592,7 +477,7 @@ func (p *ProvisionerS) deployOhaiHints( return nil } -func (p *ProvisionerS) fetchChefCertificatesFunc( +func (p *provisioner) fetchChefCertificatesFunc( knifeCmd string, confDir string) func(terraform.UIOutput, communicator.Communicator) error { return func(o terraform.UIOutput, comm communicator.Communicator) error { @@ -603,10 +488,7 @@ func (p *ProvisionerS) fetchChefCertificatesFunc( } } -func (p *ProvisionerS) generateClientKeyFunc( - knifeCmd string, - confDir string, - noOutput string) func(terraform.UIOutput, communicator.Communicator) error { +func (p *provisioner) generateClientKeyFunc(knifeCmd string, confDir string, noOutput string) provisionFn { return func(o terraform.UIOutput, comm communicator.Communicator) error { options := fmt.Sprintf("-c %s -u %s --key %s", path.Join(confDir, clienrb), @@ -664,10 +546,7 @@ func (p *ProvisionerS) generateClientKeyFunc( } } -func (p *ProvisionerS) configureVaultsFunc( - gemCmd string, - knifeCmd string, - confDir string) func(terraform.UIOutput, communicator.Communicator) error { +func (p *provisioner) configureVaultsFunc(gemCmd string, knifeCmd string, confDir string) provisionFn { return func(o terraform.UIOutput, comm communicator.Communicator) error { if err := p.runCommand(o, comm, fmt.Sprintf("%s install chef-vault", gemCmd)); err != nil { return err @@ -679,7 +558,7 @@ func (p *ProvisionerS) configureVaultsFunc( path.Join(confDir, p.UserName+".pem"), ) - for vault, items := range p.vaults { + for vault, items := range p.Vaults { for _, item := range items { updateCmd := fmt.Sprintf("%s vault update %s %s -C %s -M client %s", knifeCmd, @@ -698,9 +577,7 @@ func (p *ProvisionerS) configureVaultsFunc( } } -func (p *ProvisionerS) runChefClientFunc( - chefCmd string, - confDir string) func(terraform.UIOutput, communicator.Communicator) error { +func (p *provisioner) runChefClientFunc(chefCmd string, confDir string) provisionFn { return func(o terraform.UIOutput, comm communicator.Communicator) error { fb := path.Join(confDir, firstBoot) var cmd string @@ -736,7 +613,7 @@ func (p *ProvisionerS) runChefClientFunc( } // Output implementation of terraform.UIOutput interface -func (p *ProvisionerS) Output(output string) { +func (p *provisioner) Output(output string) { logFile := path.Join(logfileDir, p.NodeName) f, err := os.OpenFile(logFile, os.O_APPEND|os.O_WRONLY, 0666) if err != nil { @@ -762,10 +639,7 @@ func (p *ProvisionerS) Output(output string) { } // runCommand is used to run already prepared commands -func (p *ProvisionerS) runCommand( - o terraform.UIOutput, - comm communicator.Communicator, - command string) error { +func (p *provisioner) runCommand(o terraform.UIOutput, comm communicator.Communicator, command string) error { // Unless prevented, prefix the command with sudo if p.useSudo { command = "sudo " + command @@ -804,7 +678,7 @@ func (p *ProvisionerS) runCommand( return err } -func (p *ProvisionerS) copyOutput(o terraform.UIOutput, r io.Reader, doneCh chan<- struct{}) { +func (p *provisioner) copyOutput(o terraform.UIOutput, r io.Reader, doneCh chan<- struct{}) { defer close(doneCh) lr := linereader.New(r) for line := range lr.Ch { @@ -830,9 +704,8 @@ func retryFunc(timeout time.Duration, f func() error) error { } } -func decodeDataToProvisioner(d *schema.ResourceData) *ProvisionerS { - return &ProvisionerS{ - AttributesJSON: d.Get("attributes_json").(string), +func decodeConfig(d *schema.ResourceData) (*provisioner, error) { + p := &provisioner{ ClientOptions: getStringList(d.Get("client_options")), DisableReporting: d.Get("disable_reporting").(bool), Environment: d.Get("environment").(string), @@ -858,13 +731,54 @@ func decodeDataToProvisioner(d *schema.ResourceData) *ProvisionerS { SSLVerifyMode: d.Get("ssl_verify_mode").(string), UserName: d.Get("user_name").(string), UserKey: d.Get("user_key").(string), - VaultJSON: d.Get("vault_json").(string), Version: d.Get("version").(string), - - // Deprecated - ValidationClientName: d.Get("validation_client_name").(string), - ValidationKey: d.Get("validation_key").(string), } + + // Make sure the supplied URL has a trailing slash + p.ServerURL = strings.TrimSuffix(p.ServerURL, "/") + "/" + + for i, hint := range p.OhaiHints { + hintPath, err := homedir.Expand(hint) + if err != nil { + return nil, fmt.Errorf("Error expanding the path %s: %v", hint, err) + } + p.OhaiHints[i] = hintPath + } + + if attrs, ok := d.GetOk("attributes_json"); ok { + var m map[string]interface{} + if err := json.Unmarshal([]byte(attrs.(string)), &m); err != nil { + return nil, fmt.Errorf("Error parsing attributes_json: %v", err) + } + p.Attributes = m + } + + if vaults, ok := d.GetOk("vault_json"); ok { + var m map[string]interface{} + if err := json.Unmarshal([]byte(vaults.(string)), &m); err != nil { + return nil, fmt.Errorf("Error parsing vault_json: %v", err) + } + + v := make(map[string][]string) + for vault, items := range m { + switch items := items.(type) { + case []interface{}: + for _, item := range items { + if item, ok := item.(string); ok { + v[vault] = append(v[vault], item) + } + } + case interface{}: + if item, ok := items.(string); ok { + v[vault] = append(v[vault], item) + } + } + } + + p.Vaults = v + } + + return p, nil } func getStringList(v interface{}) []string { diff --git a/builtin/provisioners/chef/resource_provisioner_test.go b/builtin/provisioners/chef/resource_provisioner_test.go index 3bb728d5d..f9e9f33fe 100644 --- a/builtin/provisioners/chef/resource_provisioner_test.go +++ b/builtin/provisioners/chef/resource_provisioner_test.go @@ -30,8 +30,8 @@ func TestResourceProvider_Validate_good(t *testing.T) { "user_name": "bob", "user_key": "USER-KEY", }) - r := Provisioner() - warn, errs := r.Validate(c) + + warn, errs := Provisioner().Validate(c) if len(warn) > 0 { t.Fatalf("Warnings: %v", warn) } @@ -44,8 +44,8 @@ func TestResourceProvider_Validate_bad(t *testing.T) { c := testConfig(t, map[string]interface{}{ "invalid": "nope", }) - p := Provisioner() - warn, errs := p.Validate(c) + + warn, errs := Provisioner().Validate(c) if len(warn) > 0 { t.Fatalf("Warnings: %v", warn) } @@ -66,8 +66,8 @@ func TestResourceProvider_Validate_computedValues(t *testing.T) { "user_key": "USER-KEY", "attributes_json": config.UnknownVariableValue, }) - r := Provisioner() - warn, errs := r.Validate(c) + + warn, errs := Provisioner().Validate(c) if len(warn) > 0 { t.Fatalf("Warnings: %v", warn) } @@ -76,30 +76,21 @@ func TestResourceProvider_Validate_computedValues(t *testing.T) { } } -func testConfig(t *testing.T, c map[string]interface{}) *terraform.ResourceConfig { - r, err := config.NewRawConfig(c) - if err != nil { - t.Fatalf("bad: %s", err) - } - - return terraform.NewResourceConfig(r) -} - func TestResourceProvider_runChefClient(t *testing.T) { cases := map[string]struct { - Config *terraform.ResourceConfig + Config map[string]interface{} ChefCmd string ConfDir string Commands map[string]bool }{ "Sudo": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "node_name": "nodename1", "run_list": []interface{}{"cookbook::recipe"}, "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, ChefCmd: linuxChefCmd, @@ -113,14 +104,14 @@ func TestResourceProvider_runChefClient(t *testing.T) { }, "NoSudo": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "node_name": "nodename1", "prevent_sudo": true, "run_list": []interface{}{"cookbook::recipe"}, "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, ChefCmd: linuxChefCmd, @@ -134,7 +125,7 @@ func TestResourceProvider_runChefClient(t *testing.T) { }, "Environment": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "environment": "production", "node_name": "nodename1", "prevent_sudo": true, @@ -142,7 +133,7 @@ func TestResourceProvider_runChefClient(t *testing.T) { "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, ChefCmd: windowsChefCmd, @@ -162,7 +153,9 @@ func TestResourceProvider_runChefClient(t *testing.T) { for k, tc := range cases { c.Commands = tc.Commands - p, err := decodeConfig(getTestResourceData(tc.Config)) + p, err := decodeConfig( + schema.TestResourceDataRaw(t, Provisioner().(*schema.Provisioner).Schema, tc.Config), + ) if err != nil { t.Fatalf("Error: %v", err) } @@ -179,20 +172,20 @@ func TestResourceProvider_runChefClient(t *testing.T) { func TestResourceProvider_fetchChefCertificates(t *testing.T) { cases := map[string]struct { - Config *terraform.ResourceConfig + Config map[string]interface{} KnifeCmd string ConfDir string Commands map[string]bool }{ "Sudo": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "fetch_chef_certificates": true, "node_name": "nodename1", "run_list": []interface{}{"cookbook::recipe"}, "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, KnifeCmd: linuxKnifeCmd, @@ -206,7 +199,7 @@ func TestResourceProvider_fetchChefCertificates(t *testing.T) { }, "NoSudo": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "fetch_chef_certificates": true, "node_name": "nodename1", "prevent_sudo": true, @@ -214,7 +207,7 @@ func TestResourceProvider_fetchChefCertificates(t *testing.T) { "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, KnifeCmd: windowsKnifeCmd, @@ -234,7 +227,9 @@ func TestResourceProvider_fetchChefCertificates(t *testing.T) { for k, tc := range cases { c.Commands = tc.Commands - p, err := decodeConfig(getTestResourceData(tc.Config)) + p, err := decodeConfig( + schema.TestResourceDataRaw(t, Provisioner().(*schema.Provisioner).Schema, tc.Config), + ) if err != nil { t.Fatalf("Error: %v", err) } @@ -251,14 +246,14 @@ func TestResourceProvider_fetchChefCertificates(t *testing.T) { func TestResourceProvider_configureVaults(t *testing.T) { cases := map[string]struct { - Config *terraform.ResourceConfig + Config map[string]interface{} GemCmd string KnifeCmd string ConfDir string Commands map[string]bool }{ "Linux Vault string": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "node_name": "nodename1", "prevent_sudo": true, "run_list": []interface{}{"cookbook::recipe"}, @@ -266,7 +261,7 @@ func TestResourceProvider_configureVaults(t *testing.T) { "user_name": "bob", "user_key": "USER-KEY", "vault_json": `{"vault1": "item1"}`, - }), + }, GemCmd: linuxGemCmd, KnifeCmd: linuxKnifeCmd, @@ -280,7 +275,7 @@ func TestResourceProvider_configureVaults(t *testing.T) { }, "Linux Vault []string": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "fetch_chef_certificates": true, "node_name": "nodename1", "prevent_sudo": true, @@ -289,7 +284,7 @@ func TestResourceProvider_configureVaults(t *testing.T) { "user_name": "bob", "user_key": "USER-KEY", "vault_json": `{"vault1": ["item1", "item2"]}`, - }), + }, GemCmd: linuxGemCmd, KnifeCmd: linuxKnifeCmd, @@ -305,7 +300,7 @@ func TestResourceProvider_configureVaults(t *testing.T) { }, "Windows Vault string": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "node_name": "nodename1", "prevent_sudo": true, "run_list": []interface{}{"cookbook::recipe"}, @@ -313,7 +308,7 @@ func TestResourceProvider_configureVaults(t *testing.T) { "user_name": "bob", "user_key": "USER-KEY", "vault_json": `{"vault1": "item1"}`, - }), + }, GemCmd: windowsGemCmd, KnifeCmd: windowsKnifeCmd, @@ -327,7 +322,7 @@ func TestResourceProvider_configureVaults(t *testing.T) { }, "Windows Vault []string": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "fetch_chef_certificates": true, "node_name": "nodename1", "prevent_sudo": true, @@ -336,7 +331,7 @@ func TestResourceProvider_configureVaults(t *testing.T) { "user_name": "bob", "user_key": "USER-KEY", "vault_json": `{"vault1": ["item1", "item2"]}`, - }), + }, GemCmd: windowsGemCmd, KnifeCmd: windowsKnifeCmd, @@ -358,7 +353,9 @@ func TestResourceProvider_configureVaults(t *testing.T) { for k, tc := range cases { c.Commands = tc.Commands - p, err := decodeConfig(getTestResourceData(tc.Config)) + p, err := decodeConfig( + schema.TestResourceDataRaw(t, Provisioner().(*schema.Provisioner).Schema, tc.Config), + ) if err != nil { t.Fatalf("Error: %v", err) } @@ -373,6 +370,11 @@ func TestResourceProvider_configureVaults(t *testing.T) { } } -func getTestResourceData(c *terraform.ResourceConfig) *schema.ResourceData { - return schema.TestResourceDataConfig(Provisioner().(*schema.Provisioner).Schema, c) +func testConfig(t *testing.T, c map[string]interface{}) *terraform.ResourceConfig { + r, err := config.NewRawConfig(c) + if err != nil { + t.Fatalf("bad: %s", err) + } + + return terraform.NewResourceConfig(r) } diff --git a/builtin/provisioners/chef/windows_provisioner.go b/builtin/provisioners/chef/windows_provisioner.go index 773d1b0f2..8713affcd 100644 --- a/builtin/provisioners/chef/windows_provisioner.go +++ b/builtin/provisioners/chef/windows_provisioner.go @@ -46,9 +46,7 @@ Write-Host 'Installing Chef Client...' Start-Process -FilePath msiexec -ArgumentList /qn, /i, $dest -Wait ` -func (p *ProvisionerS) windowsInstallChefClient( - o terraform.UIOutput, - comm communicator.Communicator) error { +func (p *provisioner) windowsInstallChefClient(o terraform.UIOutput, comm communicator.Communicator) error { script := path.Join(path.Dir(comm.ScriptPath()), "ChefClient.ps1") content := fmt.Sprintf(installScript, p.Version, p.HTTPProxy, strings.Join(p.NOProxy, ",")) @@ -62,9 +60,7 @@ func (p *ProvisionerS) windowsInstallChefClient( return p.runCommand(o, comm, installCmd) } -func (p *ProvisionerS) windowsCreateConfigFiles( - o terraform.UIOutput, - comm communicator.Communicator) error { +func (p *provisioner) windowsCreateConfigFiles(o terraform.UIOutput, comm communicator.Communicator) error { // Make sure the config directory exists cmd := fmt.Sprintf("cmd /c if not exist %q mkdir %q", windowsConfDir, windowsConfDir) if err := p.runCommand(o, comm, cmd); err != nil { diff --git a/builtin/provisioners/chef/windows_provisioner_test.go b/builtin/provisioners/chef/windows_provisioner_test.go index 0bb0dc126..566f8c457 100644 --- a/builtin/provisioners/chef/windows_provisioner_test.go +++ b/builtin/provisioners/chef/windows_provisioner_test.go @@ -6,23 +6,24 @@ import ( "testing" "github.com/hashicorp/terraform/communicator" + "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" ) func TestResourceProvider_windowsInstallChefClient(t *testing.T) { cases := map[string]struct { - Config *terraform.ResourceConfig + Config map[string]interface{} Commands map[string]bool UploadScripts map[string]string }{ "Default": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "node_name": "nodename1", "run_list": []interface{}{"cookbook::recipe"}, "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, Commands: map[string]bool{ "powershell -NoProfile -ExecutionPolicy Bypass -File ChefClient.ps1": true, @@ -34,7 +35,7 @@ func TestResourceProvider_windowsInstallChefClient(t *testing.T) { }, "Proxy": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "http_proxy": "http://proxy.local", "no_proxy": []interface{}{"http://local.local", "http://local.org"}, "node_name": "nodename1", @@ -42,7 +43,7 @@ func TestResourceProvider_windowsInstallChefClient(t *testing.T) { "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, Commands: map[string]bool{ "powershell -NoProfile -ExecutionPolicy Bypass -File ChefClient.ps1": true, @@ -54,14 +55,14 @@ func TestResourceProvider_windowsInstallChefClient(t *testing.T) { }, "Version": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "node_name": "nodename1", "run_list": []interface{}{"cookbook::recipe"}, "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", "version": "11.18.6", - }), + }, Commands: map[string]bool{ "powershell -NoProfile -ExecutionPolicy Bypass -File ChefClient.ps1": true, @@ -80,7 +81,9 @@ func TestResourceProvider_windowsInstallChefClient(t *testing.T) { c.Commands = tc.Commands c.UploadScripts = tc.UploadScripts - p, err := decodeConfig(getTestResourceData(tc.Config)) + p, err := decodeConfig( + schema.TestResourceDataRaw(t, Provisioner().(*schema.Provisioner).Schema, tc.Config), + ) if err != nil { t.Fatalf("Error: %v", err) } @@ -96,12 +99,12 @@ func TestResourceProvider_windowsInstallChefClient(t *testing.T) { func TestResourceProvider_windowsCreateConfigFiles(t *testing.T) { cases := map[string]struct { - Config *terraform.ResourceConfig + Config map[string]interface{} Commands map[string]bool Uploads map[string]string }{ "Default": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "ohai_hints": []interface{}{"test-fixtures/ohaihint.json"}, "node_name": "nodename1", "run_list": []interface{}{"cookbook::recipe"}, @@ -109,7 +112,7 @@ func TestResourceProvider_windowsCreateConfigFiles(t *testing.T) { "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, Commands: map[string]bool{ fmt.Sprintf("cmd /c if not exist %q mkdir %q", windowsConfDir, windowsConfDir): true, @@ -128,7 +131,7 @@ func TestResourceProvider_windowsCreateConfigFiles(t *testing.T) { }, "Proxy": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "http_proxy": "http://proxy.local", "https_proxy": "https://proxy.local", "no_proxy": []interface{}{"http://local.local", "https://local.local"}, @@ -139,7 +142,7 @@ func TestResourceProvider_windowsCreateConfigFiles(t *testing.T) { "ssl_verify_mode": "verify_none", "user_name": "bob", "user_key": "USER-KEY", - }), + }, Commands: map[string]bool{ fmt.Sprintf("cmd /c if not exist %q mkdir %q", windowsConfDir, windowsConfDir): true, @@ -154,7 +157,7 @@ func TestResourceProvider_windowsCreateConfigFiles(t *testing.T) { }, "Attributes JSON": { - Config: testConfig(t, map[string]interface{}{ + Config: map[string]interface{}{ "attributes_json": `{"key1":{"subkey1":{"subkey2a":["val1","val2","val3"],` + `"subkey2b":{"subkey3":"value3"}}},"key2":"value2"}`, "node_name": "nodename1", @@ -163,7 +166,7 @@ func TestResourceProvider_windowsCreateConfigFiles(t *testing.T) { "server_url": "https://chef.local", "user_name": "bob", "user_key": "USER-KEY", - }), + }, Commands: map[string]bool{ fmt.Sprintf("cmd /c if not exist %q mkdir %q", windowsConfDir, windowsConfDir): true, @@ -186,7 +189,9 @@ func TestResourceProvider_windowsCreateConfigFiles(t *testing.T) { c.Commands = tc.Commands c.Uploads = tc.Uploads - p, err := decodeConfig(getTestResourceData(tc.Config)) + p, err := decodeConfig( + schema.TestResourceDataRaw(t, Provisioner().(*schema.Provisioner).Schema, tc.Config), + ) if err != nil { t.Fatalf("Error: %v", err) } diff --git a/builtin/provisioners/file/resource_provisioner.go b/builtin/provisioners/file/resource_provisioner.go index eb29424b4..2c9c7298a 100644 --- a/builtin/provisioners/file/resource_provisioner.go +++ b/builtin/provisioners/file/resource_provisioner.go @@ -36,7 +36,7 @@ func Provisioner() terraform.ResourceProvisioner { }, ApplyFunc: applyFn, - ValidateFunc: Validate, + ValidateFunc: validateFn, } } @@ -78,8 +78,7 @@ func applyFn(ctx context.Context) error { } } -// Validate checks if the required arguments are configured -func Validate(d *schema.ResourceData) (ws []string, es []error) { +func validateFn(d *schema.ResourceData) (ws []string, es []error) { numSrc := 0 if _, ok := d.GetOk("source"); ok == true { numSrc++ diff --git a/builtin/provisioners/file/resource_provisioner_test.go b/builtin/provisioners/file/resource_provisioner_test.go index 96afc519f..2dacab35e 100644 --- a/builtin/provisioners/file/resource_provisioner_test.go +++ b/builtin/provisioners/file/resource_provisioner_test.go @@ -23,8 +23,8 @@ func TestResourceProvider_Validate_good_source(t *testing.T) { "source": "/tmp/foo", "destination": "/tmp/bar", }) - p := Provisioner() - warn, errs := p.Validate(c) + + warn, errs := Provisioner().Validate(c) if len(warn) > 0 { t.Fatalf("Warnings: %v", warn) } @@ -38,8 +38,8 @@ func TestResourceProvider_Validate_good_content(t *testing.T) { "content": "value to copy", "destination": "/tmp/bar", }) - p := Provisioner() - warn, errs := p.Validate(c) + + warn, errs := Provisioner().Validate(c) if len(warn) > 0 { t.Fatalf("Warnings: %v", warn) } @@ -52,8 +52,8 @@ func TestResourceProvider_Validate_bad_not_destination(t *testing.T) { c := testConfig(t, map[string]interface{}{ "source": "nope", }) - p := Provisioner() - warn, errs := p.Validate(c) + + warn, errs := Provisioner().Validate(c) if len(warn) > 0 { t.Fatalf("Warnings: %v", warn) } @@ -66,8 +66,8 @@ func TestResourceProvider_Validate_bad_no_source(t *testing.T) { c := testConfig(t, map[string]interface{}{ "destination": "/tmp/bar", }) - p := Provisioner() - warn, errs := p.Validate(c) + + warn, errs := Provisioner().Validate(c) if len(warn) > 0 { t.Fatalf("Warnings: %v", warn) } @@ -82,8 +82,8 @@ func TestResourceProvider_Validate_bad_to_many_src(t *testing.T) { "content": "value to copy", "destination": "/tmp/bar", }) - p := Provisioner() - warn, errs := p.Validate(c) + + warn, errs := Provisioner().Validate(c) if len(warn) > 0 { t.Fatalf("Warnings: %v", warn) } @@ -92,12 +92,11 @@ func TestResourceProvider_Validate_bad_to_many_src(t *testing.T) { } } -func testConfig( - t *testing.T, - c map[string]interface{}) *terraform.ResourceConfig { +func testConfig(t *testing.T, c map[string]interface{}) *terraform.ResourceConfig { r, err := config.NewRawConfig(c) if err != nil { t.Fatalf("bad: %s", err) } + return terraform.NewResourceConfig(r) } diff --git a/builtin/provisioners/local-exec/resource_provisioner_test.go b/builtin/provisioners/local-exec/resource_provisioner_test.go index 85fb6abcf..2fa17efc5 100644 --- a/builtin/provisioners/local-exec/resource_provisioner_test.go +++ b/builtin/provisioners/local-exec/resource_provisioner_test.go @@ -30,6 +30,7 @@ func TestResourceProvider_Apply(t *testing.T) { output := new(terraform.MockUIOutput) p := Provisioner() + if err := p.Apply(output, nil, c); err != nil { t.Fatalf("err: %v", err) } @@ -84,8 +85,8 @@ func TestResourceProvider_Validate_good(t *testing.T) { c := testConfig(t, map[string]interface{}{ "command": "echo foo", }) - p := Provisioner() - warn, errs := p.Validate(c) + + warn, errs := Provisioner().Validate(c) if len(warn) > 0 { t.Fatalf("Warnings: %v", warn) } @@ -96,8 +97,8 @@ func TestResourceProvider_Validate_good(t *testing.T) { func TestResourceProvider_Validate_missing(t *testing.T) { c := testConfig(t, map[string]interface{}{}) - p := Provisioner() - warn, errs := p.Validate(c) + + warn, errs := Provisioner().Validate(c) if len(warn) > 0 { t.Fatalf("Warnings: %v", warn) } @@ -106,9 +107,7 @@ func TestResourceProvider_Validate_missing(t *testing.T) { } } -func testConfig( - t *testing.T, - c map[string]interface{}) *terraform.ResourceConfig { +func testConfig(t *testing.T, c map[string]interface{}) *terraform.ResourceConfig { r, err := config.NewRawConfig(c) if err != nil { t.Fatalf("bad: %s", err) diff --git a/builtin/provisioners/remote-exec/resource_provisioner_test.go b/builtin/provisioners/remote-exec/resource_provisioner_test.go index 53e943680..67faf1fe4 100644 --- a/builtin/provisioners/remote-exec/resource_provisioner_test.go +++ b/builtin/provisioners/remote-exec/resource_provisioner_test.go @@ -30,8 +30,8 @@ func TestResourceProvider_Validate_good(t *testing.T) { c := testConfig(t, map[string]interface{}{ "inline": "echo foo", }) - p := Provisioner() - warn, errs := p.Validate(c) + + warn, errs := Provisioner().Validate(c) if len(warn) > 0 { t.Fatalf("Warnings: %v", warn) } @@ -44,8 +44,8 @@ func TestResourceProvider_Validate_bad(t *testing.T) { c := testConfig(t, map[string]interface{}{ "invalid": "nope", }) - p := Provisioner() - warn, errs := p.Validate(c) + + warn, errs := Provisioner().Validate(c) if len(warn) > 0 { t.Fatalf("Warnings: %v", warn) } @@ -60,7 +60,6 @@ exit 0 ` func TestResourceProvider_generateScript(t *testing.T) { - p := Provisioner().(*schema.Provisioner) conf := map[string]interface{}{ "inline": []interface{}{ "cd /tmp", @@ -68,8 +67,10 @@ func TestResourceProvider_generateScript(t *testing.T) { "exit 0", }, } - out, err := generateScripts(schema.TestResourceDataRaw( - t, p.Schema, conf)) + + out, err := generateScripts( + schema.TestResourceDataRaw(t, Provisioner().(*schema.Provisioner).Schema, conf), + ) if err != nil { t.Fatalf("err: %v", err) } @@ -101,7 +102,6 @@ func TestResourceProvider_generateScriptEmptyInline(t *testing.T) { } func TestResourceProvider_CollectScripts_inline(t *testing.T) { - p := Provisioner().(*schema.Provisioner) conf := map[string]interface{}{ "inline": []interface{}{ "cd /tmp", @@ -110,8 +110,9 @@ func TestResourceProvider_CollectScripts_inline(t *testing.T) { }, } - scripts, err := collectScripts(schema.TestResourceDataRaw( - t, p.Schema, conf)) + scripts, err := collectScripts( + schema.TestResourceDataRaw(t, Provisioner().(*schema.Provisioner).Schema, conf), + ) if err != nil { t.Fatalf("err: %v", err) } @@ -132,13 +133,13 @@ func TestResourceProvider_CollectScripts_inline(t *testing.T) { } func TestResourceProvider_CollectScripts_script(t *testing.T) { - p := Provisioner().(*schema.Provisioner) conf := map[string]interface{}{ "script": "test-fixtures/script1.sh", } - scripts, err := collectScripts(schema.TestResourceDataRaw( - t, p.Schema, conf)) + scripts, err := collectScripts( + schema.TestResourceDataRaw(t, Provisioner().(*schema.Provisioner).Schema, conf), + ) if err != nil { t.Fatalf("err: %v", err) } @@ -159,7 +160,6 @@ func TestResourceProvider_CollectScripts_script(t *testing.T) { } func TestResourceProvider_CollectScripts_scripts(t *testing.T) { - p := Provisioner().(*schema.Provisioner) conf := map[string]interface{}{ "scripts": []interface{}{ "test-fixtures/script1.sh", @@ -168,8 +168,9 @@ func TestResourceProvider_CollectScripts_scripts(t *testing.T) { }, } - scripts, err := collectScripts(schema.TestResourceDataRaw( - t, p.Schema, conf)) + scripts, err := collectScripts( + schema.TestResourceDataRaw(t, Provisioner().(*schema.Provisioner).Schema, conf), + ) if err != nil { t.Fatalf("err: %v", err) } @@ -234,9 +235,7 @@ func TestRetryFunc(t *testing.T) { } } -func testConfig( - t *testing.T, - c map[string]interface{}) *terraform.ResourceConfig { +func testConfig(t *testing.T, c map[string]interface{}) *terraform.ResourceConfig { r, err := config.NewRawConfig(c) if err != nil { t.Fatalf("bad: %s", err) diff --git a/helper/schema/provisioner.go b/helper/schema/provisioner.go index 926df8893..856c6758a 100644 --- a/helper/schema/provisioner.go +++ b/helper/schema/provisioner.go @@ -41,9 +41,8 @@ type Provisioner struct { // information. ApplyFunc func(ctx context.Context) error - // ValidateFunc is the function for extended validation. This is optional. - // It is given a resource data. - // Should be provided when Scheme is not enough. + // ValidateFunc is a function for extended validation. This is optional + // and should be used when individual field validation is not enough. ValidateFunc func(*ResourceData) ([]string, []error) stopCtx context.Context diff --git a/helper/schema/testing.go b/helper/schema/testing.go index bbc7b0cb8..9765bdbc6 100644 --- a/helper/schema/testing.go +++ b/helper/schema/testing.go @@ -28,10 +28,3 @@ func TestResourceDataRaw( return result } - -func TestResourceDataConfig(schema map[string]*Schema, config *terraform.ResourceConfig) *ResourceData { - return &ResourceData{ - schema: schema, - config: config, - } -} diff --git a/website/source/docs/provisioners/chef.html.markdown b/website/source/docs/provisioners/chef.html.markdown index 8b9557f81..34fff2cd2 100644 --- a/website/source/docs/provisioners/chef.html.markdown +++ b/website/source/docs/provisioners/chef.html.markdown @@ -157,9 +157,3 @@ The following arguments are supported: * `version (string)` - (Optional) The Chef Client version to install on the remote machine. If not set, the latest available version will be installed. - -These options are supported for backwards compatibility and may be removed in a -future version: - -* `validation_client_name (string)` - __Deprecated: please use `user_name` instead__. -* `validation_key (string)` - __Deprecated: please use `user_key` instead__.