change apply Eval methods to use diags

terraform/eval_apply.go

@@ -39,7 +39,7 @@ type EvalApply struct {
 }
 
 // TODO: test
-func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
+func (n *EvalApply) Eval(ctx EvalContext) tfdiags.Diagnostics {
 	var diags tfdiags.Diagnostics
 
 	change := *n.Change
@@ -54,7 +54,8 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
 	schema, _ := (*n.ProviderSchema).SchemaForResourceType(n.Addr.Resource.Mode, n.Addr.Resource.Type)
 	if schema == nil {
 		// Should be caught during validation, so we don't bother with a pretty error here
-		return nil, fmt.Errorf("provider does not support resource type %q", n.Addr.Resource.Type)
+		diags = diags.Append(fmt.Errorf("provider does not support resource type %q", n.Addr.Resource.Type))
+		return diags
 	}
 
 	if n.CreateNew != nil {
@@ -69,15 +70,16 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
 		configVal, _, configDiags = ctx.EvaluateBlock(n.Config.Config, schema, nil, keyData)
 		diags = diags.Append(configDiags)
 		if configDiags.HasErrors() {
-			return nil, diags.Err()
+			return diags
 		}
 	}
 
 	if !configVal.IsWhollyKnown() {
-		return nil, fmt.Errorf(
+		diags = diags.Append(fmt.Errorf(
 			"configuration for %s still contains unknown values during apply (this is a bug in Terraform; please report it!)",
 			absAddr,
-		)
+		))
+		return diags
 	}
 
 	metaConfigVal := cty.NullVal(cty.DynamicPseudoType)
@@ -99,7 +101,7 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
 				metaConfigVal, _, configDiags = ctx.EvaluateBlock(m.Config, (*n.ProviderSchema).ProviderMeta, nil, EvalDataForNoInstanceKey)
 				diags = diags.Append(configDiags)
 				if configDiags.HasErrors() {
-					return nil, diags.Err()
+					return diags
 				}
 			}
 		}
@@ -120,7 +122,7 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
 	eqV := unmarkedBefore.Equals(unmarkedAfter)
 	eq := eqV.IsKnown() && eqV.True()
 	if change.Action == plans.Update && eq && !reflect.DeepEqual(beforePaths, afterPaths) {
-		return nil, diags.ErrWithWarnings()
+		return diags
 	}
 
 	resp := provider.ApplyResourceChange(providers.ApplyResourceChangeRequest{
@@ -189,7 +191,7 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
 		// Bail early in this particular case, because an object that doesn't
 		// conform to the schema can't be saved in the state anyway -- the
 		// serializer will reject it.
-		return nil, diags.Err()
+		return diags
 	}
 
 	// After this point we have a type-conforming result object and so we
@@ -350,21 +352,11 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
 			err := diags.Err()
 			*n.Error = err
 			log.Printf("[DEBUG] %s: apply errored, but we're indicating that via the Error pointer rather than returning it: %s", n.Addr.Absolute(ctx.Path()), err)
-			return nil, nil
+			return nil
 		}
 	}
 
-	// we have to drop warning-only diagnostics for now
+	return diags
-	if diags.HasErrors() {
-		return nil, diags.ErrWithWarnings()
-	}
-
-	// log any warnings since we can't return them
-	if e := diags.ErrWithWarnings(); e != nil {
-		log.Printf("[WARN] EvalApply %s: %v", n.Addr, e)
-	}
-
-	return nil, nil
 }
 
 // EvalApplyPre is an EvalNode implementation that does the pre-Apply work
@@ -376,7 +368,8 @@ type EvalApplyPre struct {
 }
 
 // TODO: test
-func (n *EvalApplyPre) Eval(ctx EvalContext) (interface{}, error) {
+func (n *EvalApplyPre) Eval(ctx EvalContext) tfdiags.Diagnostics {
+	var diags tfdiags.Diagnostics
 	change := *n.Change
 	absAddr := n.Addr.Absolute(ctx.Path())
 
@@ -388,15 +381,15 @@ func (n *EvalApplyPre) Eval(ctx EvalContext) (interface{}, error) {
 		priorState := change.Before
 		plannedNewState := change.After
 
-		err := ctx.Hook(func(h Hook) (HookAction, error) {
+		diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
 			return h.PreApply(absAddr, n.Gen, change.Action, priorState, plannedNewState)
-		})
+		}))
-		if err != nil {
+		if diags.HasErrors() {
-			return nil, err
+			return diags
 		}
 	}
 
-	return nil, nil
+	return nil
 }
 
 // EvalApplyPost is an EvalNode implementation that does the post-Apply work
@@ -408,7 +401,8 @@ type EvalApplyPost struct {
 }
 
 // TODO: test
-func (n *EvalApplyPost) Eval(ctx EvalContext) (interface{}, error) {
+func (n *EvalApplyPost) Eval(ctx EvalContext) tfdiags.Diagnostics {
+	var diags tfdiags.Diagnostics
 	state := *n.State
 
 	if resourceHasUserVisibleApply(n.Addr) {
@@ -419,20 +413,14 @@ func (n *EvalApplyPost) Eval(ctx EvalContext) (interface{}, error) {
 		} else {
 			newState = cty.NullVal(cty.DynamicPseudoType)
 		}
-		var err error
+		diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
-		if n.Error != nil {
+			return h.PostApply(absAddr, n.Gen, newState, *n.Error)
-			err = *n.Error
+		}))
 	}
 
-		hookErr := ctx.Hook(func(h Hook) (HookAction, error) {
+	diags = diags.Append(*n.Error)
-			return h.PostApply(absAddr, n.Gen, newState, err)
-		})
-		if hookErr != nil {
-			return nil, hookErr
-		}
-	}
 
-	return nil, *n.Error
+	return diags
 }
 
 // EvalMaybeTainted is an EvalNode that takes the planned change, new value,
@@ -449,9 +437,9 @@ type EvalMaybeTainted struct {
 	Error  *error
 }
 
-func (n *EvalMaybeTainted) Eval(ctx EvalContext) (interface{}, error) {
+func (n *EvalMaybeTainted) Eval(ctx EvalContext) tfdiags.Diagnostics {
 	if n.State == nil || n.Change == nil || n.Error == nil {
-		return nil, nil
+		return nil
 	}
 
 	state := *n.State
@@ -460,12 +448,12 @@ func (n *EvalMaybeTainted) Eval(ctx EvalContext) (interface{}, error) {
 
 	// nothing to do if everything went as planned
 	if err == nil {
-		return nil, nil
+		return nil
 	}
 
 	if state != nil && state.Status == states.ObjectTainted {
 		log.Printf("[TRACE] EvalMaybeTainted: %s was already tainted, so nothing to do", n.Addr.Absolute(ctx.Path()))
-		return nil, nil
+		return nil
 	}
 
 	if change.Action == plans.Create {
@@ -482,7 +470,7 @@ func (n *EvalMaybeTainted) Eval(ctx EvalContext) (interface{}, error) {
 		*n.State = state.AsTainted()
 	}
 
-	return nil, nil
+	return nil
 }
 
 // resourceHasUserVisibleApply returns true if the given resource is one where
@@ -516,44 +504,44 @@ type EvalApplyProvisioners struct {
 }
 
 // TODO: test
-func (n *EvalApplyProvisioners) Eval(ctx EvalContext) (interface{}, error) {
+func (n *EvalApplyProvisioners) Eval(ctx EvalContext) tfdiags.Diagnostics {
+	var diags tfdiags.Diagnostics
+
 	absAddr := n.Addr.Absolute(ctx.Path())
 	state := *n.State
 	if state == nil {
 		log.Printf("[TRACE] EvalApplyProvisioners: %s has no state, so skipping provisioners", n.Addr)
-		return nil, nil
+		return nil
 	}
 	if n.When == configs.ProvisionerWhenCreate && n.CreateNew != nil && !*n.CreateNew {
 		// If we're not creating a new resource, then don't run provisioners
 		log.Printf("[TRACE] EvalApplyProvisioners: %s is not freshly-created, so no provisioning is required", n.Addr)
-		return nil, nil
+		return nil
 	}
 	if state.Status == states.ObjectTainted {
 		// No point in provisioning an object that is already tainted, since
 		// it's going to get recreated on the next apply anyway.
 		log.Printf("[TRACE] EvalApplyProvisioners: %s is tainted, so skipping provisioning", n.Addr)
-		return nil, nil
+		return nil
 	}
 
 	provs := n.filterProvisioners()
 	if len(provs) == 0 {
 		// We have no provisioners, so don't do anything
-		return nil, nil
+		return nil
 	}
 
 	if n.Error != nil && *n.Error != nil {
 		// We're already tainted, so just return out
-		return nil, nil
+		return nil
 	}
 
-	{
 	// Call pre hook
-		err := ctx.Hook(func(h Hook) (HookAction, error) {
+	diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
 		return h.PreProvisionInstance(absAddr, state.Value)
-		})
+	}))
-		if err != nil {
+	if diags.HasErrors() {
-			return nil, err
+		return diags
-		}
 	}
 
 	// If there are no errors, then we append it to our output error
@@ -561,25 +549,19 @@ func (n *EvalApplyProvisioners) Eval(ctx EvalContext) (interface{}, error) {
 	err := n.apply(ctx, provs)
 	if err != nil {
 		*n.Error = multierror.Append(*n.Error, err)
-		if n.Error == nil {
-			return nil, err
-		} else {
 		log.Printf("[TRACE] EvalApplyProvisioners: %s provisioning failed, but we will continue anyway at the caller's request", absAddr)
-			return nil, nil
+		return nil
-		}
 	}
 
-	{
 	// Call post hook
-		err := ctx.Hook(func(h Hook) (HookAction, error) {
+	diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
 		return h.PostProvisionInstance(absAddr, state.Value)
-		})
+	}))
-		if err != nil {
+	if diags.HasErrors() {
-			return nil, err
+		return diags
-		}
 	}
 
-	return nil, nil
+	return diags
 }
 
 // filterProvisioners filters the provisioners on the resource to only

terraform/graph_walk_context.go

@@ -145,7 +145,6 @@ func (w *ContextGraphWalker) Execute(ctx EvalContext, n GraphNodeExecutable) tfd
 		return nil
 	}
 
-	//  If we early exit, it isn't an error.
 	if _, isEarlyExit := err.(EvalEarlyExitError); isEarlyExit {
 		return nil
 	}

terraform/node_resource_apply_instance.go

@@ -197,6 +197,8 @@ func (n *NodeApplyableResourceInstance) dataResourceExecute(ctx EvalContext) err
 }
 
 func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) error {
+	var diags tfdiags.Diagnostics
+
 	// Declare a bunch of variables that are used for state during
 	// evaluation. Most of this are written to by-address below.
 	var state *states.ResourceInstanceObject
@@ -327,9 +329,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext)
 		State:  &state,
 		Change: &diffApply,
 	}
-	_, err = evalApplyPre.Eval(ctx)
+	diags = evalApplyPre.Eval(ctx)
-	if err != nil {
+	if diags.HasErrors() {
-		return err
+		return diags.ErrWithWarnings()
 	}
 
 	var applyError error
@@ -347,9 +349,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext)
 		CreateNew:           &createNew,
 		CreateBeforeDestroy: n.CreateBeforeDestroy(),
 	}
-	_, err = evalApply.Eval(ctx)
+	diags = evalApply.Eval(ctx)
-	if err != nil {
+	if diags.HasErrors() {
-		return err
+		return diags.ErrWithWarnings()
 	}
 
 	// We clear the change out here so that future nodes don't see a change
@@ -370,9 +372,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext)
 		Change: &diffApply,
 		Error:  &applyError,
 	}
-	_, err = evalMaybeTainted.Eval(ctx)
+	diags = evalMaybeTainted.Eval(ctx)
-	if err != nil {
+	if diags.HasErrors() {
-		return err
+		return diags.ErrWithWarnings()
 	}
 
 	writeState := &EvalWriteState{
@@ -395,9 +397,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext)
 		Error:          &applyError,
 		When:           configs.ProvisionerWhenCreate,
 	}
-	_, err = applyProvisioners.Eval(ctx)
+	diags = applyProvisioners.Eval(ctx)
-	if err != nil {
+	if diags.HasErrors() {
-		return err
+		return diags.ErrWithWarnings()
 	}
 
 	evalMaybeTainted = &EvalMaybeTainted{
@@ -406,9 +408,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext)
 		Change: &diffApply,
 		Error:  &applyError,
 	}
-	_, err = evalMaybeTainted.Eval(ctx)
+	diags = evalMaybeTainted.Eval(ctx)
-	if err != nil {
+	if diags.HasErrors() {
-		return err
+		return diags.ErrWithWarnings()
 	}
 
 	writeState = &EvalWriteState{
@@ -440,9 +442,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext)
 		State: &state,
 		Error: &applyError,
 	}
-	_, err = applyPost.Eval(ctx)
+	diags = applyPost.Eval(ctx)
-	if err != nil {
+	if diags.HasErrors() {
-		return err
+		return diags.ErrWithWarnings()
 	}
 
 	UpdateStateHook(ctx)

terraform/node_resource_destroy.go

@@ -5,6 +5,7 @@ import (
 	"log"
 
 	"github.com/hashicorp/terraform/plans"
+	"github.com/hashicorp/terraform/tfdiags"
 
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/configs"
@@ -123,6 +124,8 @@ func (n *NodeDestroyResourceInstance) References() []*addrs.Reference {
 
 // GraphNodeExecutable
 func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) error {
+	var diags tfdiags.Diagnostics
+
 	addr := n.ResourceInstanceAddr()
 
 	// Get our state
@@ -178,9 +181,9 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation)
 		State:  &state,
 		Change: &changeApply,
 	}
-	_, err = evalApplyPre.Eval(ctx)
+	diags = evalApplyPre.Eval(ctx)
-	if err != nil {
+	if diags.HasErrors() {
-		return err
+		return diags.ErrWithWarnings()
 	}
 
 	// Run destroy provisioners if not tainted
@@ -192,9 +195,9 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation)
 			Error:          &provisionerErr,
 			When:           configs.ProvisionerWhenDestroy,
 		}
-		_, err := evalApplyProvisioners.Eval(ctx)
+		diags = evalApplyProvisioners.Eval(ctx)
-		if err != nil {
+		if diags.HasErrors() {
-			return err
+			return diags.ErrWithWarnings()
 		}
 		if provisionerErr != nil {
 			// If we have a provisioning error, then we just call
@@ -204,9 +207,9 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation)
 				State: &state,
 				Error: &provisionerErr,
 			}
-			_, err = evalApplyPost.Eval(ctx)
+			diags = evalApplyPost.Eval(ctx)
-			if err != nil {
+			if diags.HasErrors() {
-				return err
+				return diags.ErrWithWarnings()
 			}
 		}
 	}
@@ -226,9 +229,9 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation)
 			Output:         &state,
 			Error:          &provisionerErr,
 		}
-		_, err = evalApply.Eval(ctx)
+		diags = evalApply.Eval(ctx)
-		if err != nil {
+		if diags.HasErrors() {
-			return err
+			return diags.ErrWithWarnings()
 		}
 
 		evalWriteState := &EvalWriteState{
@@ -252,9 +255,9 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation)
 		State: &state,
 		Error: &provisionerErr,
 	}
-	_, err = evalApplyPost.Eval(ctx)
+	diags = evalApplyPost.Eval(ctx)
-	if err != nil {
+	if diags.HasErrors() {
-		return err
+		return diags.ErrWithWarnings()
 	}
 
 	err = UpdateStateHook(ctx)

terraform/node_resource_destroy_deposed.go

@@ -7,6 +7,7 @@ import (
 	"github.com/hashicorp/terraform/dag"
 	"github.com/hashicorp/terraform/plans"
 	"github.com/hashicorp/terraform/states"
+	"github.com/hashicorp/terraform/tfdiags"
 )
 
 // ConcreteResourceInstanceDeposedNodeFunc is a callback type used to convert
@@ -182,6 +183,8 @@ func (n *NodeDestroyDeposedResourceInstanceObject) ModifyCreateBeforeDestroy(v b
 
 // GraphNodeExecutable impl.
 func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op walkOperation) error {
+	var diags tfdiags.Diagnostics
+
 	addr := n.ResourceInstanceAddr().Resource
 
 	var state *states.ResourceInstanceObject
@@ -222,9 +225,9 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w
 		State:  &state,
 		Change: &change,
 	}
-	_, err = applyPre.Eval(ctx)
+	diags = applyPre.Eval(ctx)
-	if err != nil {
+	if diags.HasErrors() {
-		return err
+		return diags.ErrWithWarnings()
 	}
 
 	apply := &EvalApply{
@@ -238,9 +241,9 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w
 		Output:         &state,
 		Error:          &applyError,
 	}
-	_, err = apply.Eval(ctx)
+	diags = apply.Eval(ctx)
-	if err != nil {
+	if diags.HasErrors() {
-		return err
+		return diags.ErrWithWarnings()
 	}
 
 	// Always write the resource back to the state deposed. If it
@@ -263,9 +266,9 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w
 		State: &state,
 		Error: &applyError,
 	}
-	_, err = applyPost.Eval(ctx)
+	diags = applyPost.Eval(ctx)
-	if err != nil {
+	if diags.HasErrors() {
-		return err
+		return diags.ErrWithWarnings()
 	}
 	if applyError != nil {
 		return applyError