Merge pull request #28753 from hashicorp/jbardin/provisioner-diags

return diagnostics from provisioners

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) {