return diagnostics from provisioners

Do not convert provisioner diagnostics to errors so that users can get
context from provisioner failures.

Return diagnostics from the builtin provisioners that can be annotated
with configuration context and instance addresses.

internal/builtin/provisioners/file/resource_provisioner.go

@@ -10,6 +10,7 @@ import (
 	"github.com/hashicorp/terraform/internal/communicator"
 	"github.com/hashicorp/terraform/internal/configs/configschema"
 	"github.com/hashicorp/terraform/internal/provisioners"
+	"github.com/hashicorp/terraform/internal/tfdiags"
 	"github.com/mitchellh/go-homedir"
 	"github.com/zclconf/go-cty/cty"
 )
@@ -75,20 +76,32 @@ func (p *provisioner) ValidateProvisionerConfig(req provisioners.ValidateProvisi
 
 func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceRequest) (resp provisioners.ProvisionResourceResponse) {
 	if req.Connection.IsNull() {
-		resp.Diagnostics = resp.Diagnostics.Append(errors.New("missing connection configuration for provisioner"))
+		resp.Diagnostics = resp.Diagnostics.Append(tfdiags.WholeContainingBody(
+			tfdiags.Error,
+			"file provisioner error",
+			"Missing connection configuration for provisioner.",
+		))
 		return resp
 	}
 
 	comm, err := communicator.New(req.Connection)
 	if err != nil {
-		resp.Diagnostics = resp.Diagnostics.Append(err)
+		resp.Diagnostics = resp.Diagnostics.Append(tfdiags.WholeContainingBody(
+			tfdiags.Error,
+			"file provisioner error",
+			err.Error(),
+		))
 		return resp
 	}
 
 	// Get the source
 	src, deleteSource, err := getSrc(req.Config)
 	if err != nil {
-		resp.Diagnostics = resp.Diagnostics.Append(err)
+		resp.Diagnostics = resp.Diagnostics.Append(tfdiags.WholeContainingBody(
+			tfdiags.Error,
+			"file provisioner error",
+			err.Error(),
+		))
 		return resp
 	}
 	if deleteSource {
@@ -98,7 +111,11 @@ func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceReques
 	// Begin the file copy
 	dst := req.Config.GetAttr("destination").AsString()
 	if err := copyFiles(p.ctx, comm, src, dst); err != nil {
-		resp.Diagnostics = resp.Diagnostics.Append(err)
+		resp.Diagnostics = resp.Diagnostics.Append(tfdiags.WholeContainingBody(
+			tfdiags.Error,
+			"file provisioner error",
+			err.Error(),
+		))
 		return resp
 	}
 

internal/builtin/provisioners/file/resource_provisioner_test.go

@@ -112,7 +112,7 @@ func TestResourceProvisioner_connectionRequired(t *testing.T) {
 	}
 
 	got := resp.Diagnostics.Err().Error()
-	if !strings.Contains(got, "missing connection") {
+	if !strings.Contains(got, "Missing connection") {
-		t.Fatalf("expected 'missing connection' error: got %q", got)
+		t.Fatalf("expected 'Missing connection' error: got %q", got)
 	}
 }

internal/builtin/provisioners/local-exec/resource_provisioner.go

@@ -11,6 +11,7 @@ import (
 	"github.com/armon/circbuf"
 	"github.com/hashicorp/terraform/internal/configs/configschema"
 	"github.com/hashicorp/terraform/internal/provisioners"
+	"github.com/hashicorp/terraform/internal/tfdiags"
 	"github.com/mitchellh/go-linereader"
 	"github.com/zclconf/go-cty/cty"
 )
@@ -65,7 +66,11 @@ func (p *provisioner) GetSchema() (resp provisioners.GetSchemaResponse) {
 
 func (p *provisioner) ValidateProvisionerConfig(req provisioners.ValidateProvisionerConfigRequest) (resp provisioners.ValidateProvisionerConfigResponse) {
 	if _, err := p.GetSchema().Provisioner.CoerceValue(req.Config); err != nil {
-		resp.Diagnostics = resp.Diagnostics.Append(err)
+		resp.Diagnostics = resp.Diagnostics.Append(tfdiags.WholeContainingBody(
+			tfdiags.Error,
+			"Invalid local-exec provisioner configuration",
+			err.Error(),
+		))
 	}
 	return resp
 }
@@ -73,7 +78,11 @@ func (p *provisioner) ValidateProvisionerConfig(req provisioners.ValidateProvisi
 func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceRequest) (resp provisioners.ProvisionResourceResponse) {
 	command := req.Config.GetAttr("command").AsString()
 	if command == "" {
-		resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("local-exec provisioner command must be a non-empty string"))
+		resp.Diagnostics = resp.Diagnostics.Append(tfdiags.WholeContainingBody(
+			tfdiags.Error,
+			"Invalid local-exec provisioner command",
+			"The command must be a non-empty string.",
+		))
 		return resp
 	}
 
@@ -120,7 +129,11 @@ func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceReques
 	// See golang.org/issue/18874
 	pr, pw, err := os.Pipe()
 	if err != nil {
-		resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("failed to initialize pipe for output: %s", err))
+		resp.Diagnostics = resp.Diagnostics.Append(tfdiags.WholeContainingBody(
+			tfdiags.Error,
+			"local-exec provisioner error",
+			fmt.Sprintf("Failed to initialize pipe for output: %s", err),
+		))
 		return resp
 	}
 
@@ -171,8 +184,11 @@ func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceReques
 	}
 
 	if err != nil {
-		resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("Error running command '%s': %v. Output: %s",
+		resp.Diagnostics = resp.Diagnostics.Append(tfdiags.WholeContainingBody(
-			command, err, output.Bytes()))
+			tfdiags.Error,
+			"local-exec provisioner error",
+			fmt.Sprintf("Error running command '%s': %v. Output: %s", command, err, output.Bytes()),
+		))
 		return resp
 	}
 

internal/builtin/provisioners/remote-exec/resource_provisioner.go

@@ -15,6 +15,7 @@ import (
 	"github.com/hashicorp/terraform/internal/communicator/remote"
 	"github.com/hashicorp/terraform/internal/configs/configschema"
 	"github.com/hashicorp/terraform/internal/provisioners"
+	"github.com/hashicorp/terraform/internal/tfdiags"
 	"github.com/mitchellh/go-linereader"
 	"github.com/zclconf/go-cty/cty"
 )
@@ -59,7 +60,11 @@ func (p *provisioner) GetSchema() (resp provisioners.GetSchemaResponse) {
 func (p *provisioner) ValidateProvisionerConfig(req provisioners.ValidateProvisionerConfigRequest) (resp provisioners.ValidateProvisionerConfigResponse) {
 	cfg, err := p.GetSchema().Provisioner.CoerceValue(req.Config)
 	if err != nil {
-		resp.Diagnostics = resp.Diagnostics.Append(err)
+		resp.Diagnostics = resp.Diagnostics.Append(tfdiags.WholeContainingBody(
+			tfdiags.Error,
+			"Invalid remote-exec provisioner configuration",
+			err.Error(),
+		))
 		return resp
 	}
 
@@ -78,28 +83,43 @@ func (p *provisioner) ValidateProvisionerConfig(req provisioners.ValidateProvisi
 		set++
 	}
 	if set != 1 {
-		resp.Diagnostics = resp.Diagnostics.Append(errors.New(
+		resp.Diagnostics = resp.Diagnostics.Append(tfdiags.WholeContainingBody(
-			`only one of "inline", "script", or "scripts" must be set`))
+			tfdiags.Error,
+			"Invalid remote-exec provisioner configuration",
+			`Only one of "inline", "script", or "scripts" must be set`,
+		))
 	}
 	return resp
 }
 
 func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceRequest) (resp provisioners.ProvisionResourceResponse) {
 	if req.Connection.IsNull() {
-		resp.Diagnostics = resp.Diagnostics.Append(errors.New("missing connection configuration for provisioner"))
+		resp.Diagnostics = resp.Diagnostics.Append(tfdiags.WholeContainingBody(
+			tfdiags.Error,
+			"remote-exec provisioner error",
+			"Missing connection configuration for provisioner.",
+		))
 		return resp
 	}
 
 	comm, err := communicator.New(req.Connection)
 	if err != nil {
-		resp.Diagnostics = resp.Diagnostics.Append(err)
+		resp.Diagnostics = resp.Diagnostics.Append(tfdiags.WholeContainingBody(
+			tfdiags.Error,
+			"remote-exec provisioner error",
+			err.Error(),
+		))
 		return resp
 	}
 
 	// Collect the scripts
 	scripts, err := collectScripts(req.Config)
 	if err != nil {
-		resp.Diagnostics = resp.Diagnostics.Append(err)
+		resp.Diagnostics = resp.Diagnostics.Append(tfdiags.WholeContainingBody(
+			tfdiags.Error,
+			"remote-exec provisioner error",
+			err.Error(),
+		))
 		return resp
 	}
 	for _, s := range scripts {
@@ -108,7 +128,11 @@ func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceReques
 
 	// Copy and execute each script
 	if err := runScripts(p.ctx, req.UIOutput, comm, scripts); err != nil {
-		resp.Diagnostics = resp.Diagnostics.Append(err)
+		resp.Diagnostics = resp.Diagnostics.Append(tfdiags.WholeContainingBody(
+			tfdiags.Error,
+			"remote-exec provisioner error",
+			err.Error(),
+		))
 		return resp
 	}
 

internal/builtin/provisioners/remote-exec/resource_provisioner_test.go

@@ -271,8 +271,8 @@ func TestResourceProvisioner_connectionRequired(t *testing.T) {
 	}
 
 	got := resp.Diagnostics.Err().Error()
-	if !strings.Contains(got, "missing connection") {
+	if !strings.Contains(got, "Missing connection") {
-		t.Fatalf("expected 'missing connection' error: got %q", got)
+		t.Fatalf("expected 'Missing connection' error: got %q", got)
 	}
 }
 

internal/terraform/node_resource_abstract_instance.go

@@ -1701,9 +1701,8 @@ func (n *NodeAbstractResourceInstance) evalApplyProvisioners(ctx EvalContext, st
 
 	// If there are no errors, then we append it to our output error
 	// if we have one, otherwise we just output it.
-	err := n.applyProvisioners(ctx, state, when, provs)
+	diags = diags.Append(n.applyProvisioners(ctx, state, when, provs))
-	if err != nil {
+	if diags.HasErrors() {
-		diags = diags.Append(err)
 		log.Printf("[TRACE] evalApplyProvisioners: %s provisioning failed, but we will continue anyway at the caller's request", n.Addr)
 		return diags
 	}
@@ -1737,7 +1736,7 @@ func filterProvisioners(config *configs.Resource, when configs.ProvisionerWhen)
 }
 
 // applyProvisioners executes the provisioners for a resource.
-func (n *NodeAbstractResourceInstance) applyProvisioners(ctx EvalContext, state *states.ResourceInstanceObject, when configs.ProvisionerWhen, provs []*configs.Provisioner) error {
+func (n *NodeAbstractResourceInstance) applyProvisioners(ctx EvalContext, state *states.ResourceInstanceObject, when configs.ProvisionerWhen, provs []*configs.Provisioner) tfdiags.Diagnostics {
 	var diags tfdiags.Diagnostics
 
 	// this self is only used for destroy provisioner evaluation, and must
@@ -1766,8 +1765,7 @@ func (n *NodeAbstractResourceInstance) applyProvisioners(ctx EvalContext, state
 		// Get the provisioner
 		provisioner, err := ctx.Provisioner(prov.Type)
 		if err != nil {
-			diags = diags.Append(err)
+			return diags.Append(err)
-			return diags.Err()
 		}
 
 		schema := ctx.ProvisionerSchema(prov.Type)
@@ -1775,7 +1773,7 @@ func (n *NodeAbstractResourceInstance) applyProvisioners(ctx EvalContext, state
 		config, configDiags := evalScope(ctx, prov.Config, self, schema)
 		diags = diags.Append(configDiags)
 		if diags.HasErrors() {
-			return diags.Err()
+			return diags
 		}
 
 		// If the provisioner block contains a connection block of its own then
@@ -1807,7 +1805,7 @@ func (n *NodeAbstractResourceInstance) applyProvisioners(ctx EvalContext, state
 			connInfo, connInfoDiags = evalScope(ctx, connBody, self, connectionBlockSupersetSchema)
 			diags = diags.Append(connInfoDiags)
 			if diags.HasErrors() {
-				return diags.Err()
+				return diags
 			}
 		}
 
@@ -1817,7 +1815,7 @@ func (n *NodeAbstractResourceInstance) applyProvisioners(ctx EvalContext, state
 				return h.PreProvisionInstanceStep(n.Addr, prov.Type)
 			})
 			if err != nil {
-				return err
+				return diags.Append(err)
 			}
 		}
 
@@ -1874,27 +1872,17 @@ func (n *NodeAbstractResourceInstance) applyProvisioners(ctx EvalContext, state
 			diags = diags.Append(applyDiags)
 			if applyDiags.HasErrors() {
 				log.Printf("[WARN] Errors while provisioning %s with %q, so aborting", n.Addr, prov.Type)
-				return diags.Err()
+				return diags
 			}
 		}
 
 		// Deal with the hook
 		if hookErr != nil {
-			return hookErr
+			return diags.Append(hookErr)
 		}
 	}
 
-	// we have to drop warning-only diagnostics for now
+	return diags
-	if diags.HasErrors() {
-		return diags.ErrWithWarnings()
-	}
-
-	// log any warnings since we can't return them
-	if e := diags.ErrWithWarnings(); e != nil {
-		log.Printf("[WARN] applyProvisioners %s: %v", n.Addr, e)
-	}
-
-	return nil
 }
 
 func (n *NodeAbstractResourceInstance) evalProvisionerConfig(ctx EvalContext, body hcl.Body, self cty.Value, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) {