Refactor the provisioner validation function (#15273)

It turns out that `d.GetOk` also returns `false` when the user _did_ actually supply a value for it in the config, but the value itself needs to be evaluated before it can be used.

So instead of passing a `ResourceData` we now pass a `ResourceConfig`
which makes much more sense for doing the validation anyway.

builtin/provisioners/chef/resource_provisioner.go

@@ -259,7 +259,7 @@ func applyFn(ctx context.Context) error {
 	s := ctx.Value(schema.ProvRawStateKey).(*terraform.InstanceState)
 	d := ctx.Value(schema.ProvConfigDataKey).(*schema.ResourceData)
 
-	// Decode the raw config for this provisioner
+	// Decode the provisioner config
 	p, err := decodeConfig(d)
 	if err != nil {
 		return err
@@ -369,21 +369,20 @@ func applyFn(ctx context.Context) error {
 	return nil
 }
 
-func validateFn(d *schema.ResourceData) (ws []string, es []error) {
+func validateFn(c *terraform.ResourceConfig) (ws []string, es []error) {
-	p, err := decodeConfig(d)
+	usePolicyFile, ok := c.Get("use_policyfile")
-	if err != nil {
+	if !ok {
-		es = append(es, err)
+		usePolicyFile = false
-		return ws, es
 	}
 
-	if !p.UsePolicyfile && p.RunList == nil {
+	if !usePolicyFile.(bool) && !c.IsSet("run_list") {
-		es = append(es, errors.New("Key not found: run_list"))
+		es = append(es, errors.New("\"run_list\": required field is not set"))
 	}
-	if p.UsePolicyfile && p.PolicyName == "" {
+	if usePolicyFile.(bool) && !c.IsSet("policy_name") {
-		es = append(es, errors.New("Policyfile enabled but key not found: policy_name"))
+		es = append(es, errors.New("using policyfile, but \"policy_name\" not set"))
 	}
-	if p.UsePolicyfile && p.PolicyGroup == "" {
+	if usePolicyFile.(bool) && !c.IsSet("policy_group") {
-		es = append(es, errors.New("Policyfile enabled but key not found: policy_group"))
+		es = append(es, errors.New("using policyfile, but \"policy_group\" not set"))
 	}
 
 	return ws, es

builtin/provisioners/file/resource_provisioner.go

@@ -78,18 +78,12 @@ func applyFn(ctx context.Context) error {
 	}
 }
 
-func validateFn(d *schema.ResourceData) (ws []string, es []error) {
+func validateFn(c *terraform.ResourceConfig) (ws []string, es []error) {
-	numSrc := 0
+	if !c.IsSet("source") && !c.IsSet("content") {
-	if _, ok := d.GetOk("source"); ok == true {
+		es = append(es, fmt.Errorf("Must provide one of 'source' or 'content'"))
-		numSrc++
 	}
-	if _, ok := d.GetOk("content"); ok == true {
+
-		numSrc++
+	return ws, es
-	}
-	if numSrc != 1 {
-		es = append(es, fmt.Errorf("Must provide one of 'content' or 'source'"))
-	}
-	return
 }
 
 // getSrc returns the file to use as source

builtin/provisioners/file/resource_provisioner_test.go

@@ -48,6 +48,21 @@ func TestResourceProvider_Validate_good_content(t *testing.T) {
 	}
 }
 
+func TestResourceProvider_Validate_good_unknown_variable_value(t *testing.T) {
+	c := testConfig(t, map[string]interface{}{
+		"content":     config.UnknownVariableValue,
+		"destination": "/tmp/bar",
+	})
+
+	warn, errs := Provisioner().Validate(c)
+	if len(warn) > 0 {
+		t.Fatalf("Warnings: %v", warn)
+	}
+	if len(errs) > 0 {
+		t.Fatalf("Errors: %v", errs)
+	}
+}
+
 func TestResourceProvider_Validate_bad_not_destination(t *testing.T) {
 	c := testConfig(t, map[string]interface{}{
 		"source": "nope",

helper/schema/provisioner.go

@@ -43,7 +43,7 @@ type Provisioner struct {
 
 	// 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)
+	ValidateFunc func(*terraform.ResourceConfig) ([]string, []error)
 
 	stopCtx       context.Context
 	stopCtxCancel context.CancelFunc
@@ -121,32 +121,6 @@ func (p *Provisioner) Stop() error {
 	return nil
 }
 
-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.
 func (p *Provisioner) Apply(
 	o terraform.UIOutput,
@@ -204,3 +178,27 @@ func (p *Provisioner) Apply(
 	ctx = context.WithValue(ctx, ProvRawStateKey, s)
 	return p.ApplyFunc(ctx)
 }
+
+// Validate implements the terraform.ResourceProvisioner interface.
+func (p *Provisioner) Validate(c *terraform.ResourceConfig) (ws []string, es []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)}
+	}
+
+	if p.Schema != nil {
+		w, e := schemaMap(p.Schema).Validate(c)
+		ws = append(ws, w...)
+		es = append(es, e...)
+	}
+
+	if p.ValidateFunc != nil {
+		w, e := p.ValidateFunc(c)
+		ws = append(ws, w...)
+		es = append(es, e...)
+	}
+
+	return ws, es
+}

helper/schema/provisioner_test.go

@@ -112,7 +112,7 @@ func TestProvisionerValidate(t *testing.T) {
 			P: &Provisioner{
 				Schema:    nil,
 				ApplyFunc: noopApply,
-				ValidateFunc: func(*ResourceData) (ws []string, errors []error) {
+				ValidateFunc: func(*terraform.ResourceConfig) (ws []string, errors []error) {
 					ws = append(ws, "Simple warning from provisioner ValidateFunc")
 					return
 				},