From 477111e6b64a21a0e084ad1e88a21906e35d8c13 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 27 Oct 2020 18:16:28 -0400 Subject: [PATCH 01/12] change apply Eval methods to use diags --- terraform/eval_apply.go | 128 +++++++++------------ terraform/graph_walk_context.go | 1 - terraform/node_resource_apply_instance.go | 38 +++--- terraform/node_resource_destroy.go | 33 +++--- terraform/node_resource_destroy_deposed.go | 21 ++-- 5 files changed, 105 insertions(+), 116 deletions(-) diff --git a/terraform/eval_apply.go b/terraform/eval_apply.go index 67e559484e..a62fcaa8b9 100644 --- a/terraform/eval_apply.go +++ b/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 - 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 + return diags } // 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 { - return nil, err + })) + if diags.HasErrors() { + 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 - if n.Error != nil { - err = *n.Error - } - - hookErr := ctx.Hook(func(h Hook) (HookAction, error) { - return h.PostApply(absAddr, n.Gen, newState, err) - }) - if hookErr != nil { - return nil, hookErr - } + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PostApply(absAddr, n.Gen, newState, *n.Error) + })) } - return nil, *n.Error + diags = diags.Append(*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) { - return h.PreProvisionInstance(absAddr, state.Value) - }) - if err != nil { - return nil, err - } + // Call pre hook + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PreProvisionInstance(absAddr, state.Value) + })) + if diags.HasErrors() { + 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 - } + log.Printf("[TRACE] EvalApplyProvisioners: %s provisioning failed, but we will continue anyway at the caller's request", absAddr) + return nil } - { - // Call post hook - err := ctx.Hook(func(h Hook) (HookAction, error) { - return h.PostProvisionInstance(absAddr, state.Value) - }) - if err != nil { - return nil, err - } + // Call post hook + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PostProvisionInstance(absAddr, state.Value) + })) + if diags.HasErrors() { + return diags } - return nil, nil + return diags } // filterProvisioners filters the provisioners on the resource to only diff --git a/terraform/graph_walk_context.go b/terraform/graph_walk_context.go index c7492ed065..26ab193623 100644 --- a/terraform/graph_walk_context.go +++ b/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 } diff --git a/terraform/node_resource_apply_instance.go b/terraform/node_resource_apply_instance.go index a1d35b81ad..453387af60 100644 --- a/terraform/node_resource_apply_instance.go +++ b/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) - if err != nil { - return err + diags = evalApplyPre.Eval(ctx) + if diags.HasErrors() { + 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) - if err != nil { - return err + diags = evalApply.Eval(ctx) + if diags.HasErrors() { + 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) - if err != nil { - return err + diags = evalMaybeTainted.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } writeState := &EvalWriteState{ @@ -395,9 +397,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) Error: &applyError, When: configs.ProvisionerWhenCreate, } - _, err = applyProvisioners.Eval(ctx) - if err != nil { - return err + diags = applyProvisioners.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } evalMaybeTainted = &EvalMaybeTainted{ @@ -406,9 +408,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) Change: &diffApply, Error: &applyError, } - _, err = evalMaybeTainted.Eval(ctx) - if err != nil { - return err + diags = evalMaybeTainted.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } writeState = &EvalWriteState{ @@ -440,9 +442,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) State: &state, Error: &applyError, } - _, err = applyPost.Eval(ctx) - if err != nil { - return err + diags = applyPost.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } UpdateStateHook(ctx) diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 872df3654a..94c8d2d805 100644 --- a/terraform/node_resource_destroy.go +++ b/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) - if err != nil { - return err + diags = evalApplyPre.Eval(ctx) + if diags.HasErrors() { + 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) - if err != nil { - return err + diags = evalApplyProvisioners.Eval(ctx) + if diags.HasErrors() { + 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) - if err != nil { - return err + diags = evalApplyPost.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } } } @@ -226,9 +229,9 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) Output: &state, Error: &provisionerErr, } - _, err = evalApply.Eval(ctx) - if err != nil { - return err + diags = evalApply.Eval(ctx) + if diags.HasErrors() { + 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) - if err != nil { - return err + diags = evalApplyPost.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } err = UpdateStateHook(ctx) diff --git a/terraform/node_resource_destroy_deposed.go b/terraform/node_resource_destroy_deposed.go index a77cdf4761..ebe04fa794 100644 --- a/terraform/node_resource_destroy_deposed.go +++ b/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) - if err != nil { - return err + diags = applyPre.Eval(ctx) + if diags.HasErrors() { + 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) - if err != nil { - return err + diags = apply.Eval(ctx) + if diags.HasErrors() { + 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) - if err != nil { - return err + diags = applyPost.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } if applyError != nil { return applyError From fe9c93b9f957bbdb71c9906ea7f9635c7d9a51a6 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 28 Oct 2020 10:59:32 -0400 Subject: [PATCH 02/12] handle wrapped EvalEarlyExitErrors --- terraform/graph_walk_context.go | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/terraform/graph_walk_context.go b/terraform/graph_walk_context.go index 26ab193623..1c8273e977 100644 --- a/terraform/graph_walk_context.go +++ b/terraform/graph_walk_context.go @@ -2,6 +2,7 @@ package terraform import ( "context" + "errors" "sync" "github.com/zclconf/go-cty/cty" @@ -145,16 +146,34 @@ func (w *ContextGraphWalker) Execute(ctx EvalContext, n GraphNodeExecutable) tfd return nil } - if _, isEarlyExit := err.(EvalEarlyExitError); isEarlyExit { + var diags tfdiags.Diagnostics + + // Handle a simple early exit error + if errors.Is(err, EvalEarlyExitError{}) { return nil } + // we need to see if this wraps only EarlyExitErrors + if wrapper, ok := err.(interface{ WrappedErrors() []error }); ok { + //WrappedErrors only returns native error values, so we can't extract + //them from diagnostics. Just return the whole thing if we have a + //combination. + errs := wrapper.WrappedErrors() + earlyExit := true + for _, err := range errs { + if err != (EvalEarlyExitError{}) { + earlyExit = false + } + } + if len(errs) > 0 && earlyExit { + return nil + } + } + // Otherwise, we'll let our usual diagnostics machinery figure out how to // unpack this as one or more diagnostic messages and return that. If we // get down here then the returned diagnostics will contain at least one // error, causing the graph walk to halt. - var diags tfdiags.Diagnostics diags = diags.Append(err) return diags - } From b42aad585683c88194b563cb7b602e1cb851335a Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 28 Oct 2020 11:46:07 -0400 Subject: [PATCH 03/12] add diags to eval_diff --- terraform/eval_diff.go | 103 ++++++++++++--------- terraform/node_resource_apply_instance.go | 30 +++--- terraform/node_resource_destroy.go | 6 +- terraform/node_resource_destroy_deposed.go | 18 ++-- terraform/node_resource_plan_destroy.go | 10 +- terraform/node_resource_plan_instance.go | 14 +-- terraform/node_resource_plan_orphan.go | 10 +- 7 files changed, 101 insertions(+), 90 deletions(-) diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index cc3a92175a..0e49346d3b 100644 --- a/terraform/eval_diff.go +++ b/terraform/eval_diff.go @@ -37,7 +37,9 @@ type EvalCheckPlannedChange struct { Planned, Actual **plans.ResourceInstanceChange } -func (n *EvalCheckPlannedChange) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalCheckPlannedChange) Eval(ctx EvalContext) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + providerSchema := *n.ProviderSchema plannedChange := *n.Planned actualChange := *n.Actual @@ -45,10 +47,10 @@ func (n *EvalCheckPlannedChange) Eval(ctx EvalContext) (interface{}, error) { schema, _ := providerSchema.SchemaForResourceAddr(n.Addr.ContainingResource()) 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 %q", n.Addr.Resource.Type) + diags = diags.Append(fmt.Errorf("provider does not support %q", n.Addr.Resource.Type)) + return diags } - var diags tfdiags.Diagnostics absAddr := n.Addr.Absolute(ctx.Path()) log.Printf("[TRACE] EvalCheckPlannedChange: Verifying that actual change (action %s) matches planned change (action %s)", actualChange.Action, plannedChange.Action) @@ -96,7 +98,7 @@ func (n *EvalCheckPlannedChange) Eval(ctx EvalContext) (interface{}, error) { ), )) } - return nil, diags.Err() + return diags } // EvalDiff is an EvalNode implementation that detects changes for a given @@ -124,7 +126,9 @@ type EvalDiff struct { } // TODO: test -func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalDiff) Eval(ctx EvalContext) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + state := *n.State config := *n.Config provider := *n.Provider @@ -137,26 +141,26 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { } if providerSchema == nil { - return nil, fmt.Errorf("provider schema is unavailable for %s", n.Addr) + diags = diags.Append(fmt.Errorf("provider schema is unavailable for %s", n.Addr)) + return diags } if n.ProviderAddr.Provider.Type == "" { panic(fmt.Sprintf("EvalDiff for %s does not have ProviderAddr set", n.Addr.Absolute(ctx.Path()))) } - var diags tfdiags.Diagnostics - // Evaluate the configuration schema, _ := providerSchema.SchemaForResourceAddr(n.Addr.ContainingResource()) 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 } forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx) keyData := EvalDataForInstanceKey(n.Addr.Key, forEach) origConfigVal, _, configDiags := ctx.EvaluateBlock(config.Config, schema, nil, keyData) diags = diags.Append(configDiags) if configDiags.HasErrors() { - return nil, diags.Err() + return diags } metaConfigVal := cty.NullVal(cty.DynamicPseudoType) @@ -175,7 +179,7 @@ func (n *EvalDiff) 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 } } } @@ -216,18 +220,18 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { configValIgnored, ignoreChangeDiags := n.processIgnoreChanges(unmarkedPriorVal, unmarkedConfigVal) diags = diags.Append(ignoreChangeDiags) if ignoreChangeDiags.HasErrors() { - return nil, diags.Err() + return diags } proposedNewVal := objchange.ProposedNewObject(schema, unmarkedPriorVal, configValIgnored) // Call pre-diff hook if !n.Stub { - err := ctx.Hook(func(h Hook) (HookAction, error) { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { return h.PreDiff(absAddr, states.CurrentGen, priorVal, proposedNewVal) - }) - if err != nil { - return nil, err + })) + if diags.HasErrors() { + return diags } } @@ -242,7 +246,8 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { }, ) if validateResp.Diagnostics.HasErrors() { - return nil, validateResp.Diagnostics.InConfigBody(config.Config).Err() + diags = diags.Append(validateResp.Diagnostics.InConfigBody(config.Config)) + return diags } resp := provider.PlanResourceChange(providers.PlanResourceChangeRequest{ @@ -255,7 +260,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { }) diags = diags.Append(resp.Diagnostics.InConfigBody(config.Config)) if diags.HasErrors() { - return nil, diags.Err() + return diags } plannedNewVal := resp.PlannedState @@ -283,7 +288,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { )) } if diags.HasErrors() { - return nil, diags.Err() + return diags } if errs := objchange.AssertPlanValid(schema, unmarkedPriorVal, configValIgnored, plannedNewVal); len(errs) > 0 { @@ -313,7 +318,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { ), )) } - return nil, diags.Err() + return diags } } @@ -380,7 +385,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { } } if diags.HasErrors() { - return nil, diags.Err() + return diags } } @@ -447,7 +452,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { // append these new diagnostics if there's at least one error inside. if resp.Diagnostics.HasErrors() { diags = diags.Append(resp.Diagnostics.InConfigBody(config.Config)) - return nil, diags.Err() + return diags } plannedNewVal = resp.PlannedState plannedPrivate = resp.PlannedPrivate @@ -467,7 +472,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { )) } if diags.HasErrors() { - return nil, diags.Err() + return diags } } @@ -506,11 +511,11 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { // Call post-refresh hook if !n.Stub { - err := ctx.Hook(func(h Hook) (HookAction, error) { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { return h.PostDiff(absAddr, states.CurrentGen, action, priorVal, plannedNewVal) - }) - if err != nil { - return nil, err + })) + if diags.HasErrors() { + return diags } } @@ -547,7 +552,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { } } - return nil, nil + return diags } func (n *EvalDiff) processIgnoreChanges(prior, config cty.Value) (cty.Value, tfdiags.Diagnostics) { @@ -738,7 +743,9 @@ type EvalDiffDestroy struct { } // TODO: test -func (n *EvalDiffDestroy) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalDiffDestroy) Eval(ctx EvalContext) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + absAddr := n.Addr.Absolute(ctx.Path()) state := *n.State @@ -753,19 +760,19 @@ func (n *EvalDiffDestroy) Eval(ctx EvalContext) (interface{}, error) { // If there is no state or our attributes object is null then we're already // destroyed. if state == nil || state.Value.IsNull() { - return nil, nil + return nil } // Call pre-diff hook - err := ctx.Hook(func(h Hook) (HookAction, error) { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { return h.PreDiff( absAddr, n.DeposedKey.Generation(), state.Value, cty.NullVal(cty.DynamicPseudoType), ) - }) - if err != nil { - return nil, err + })) + if diags.HasErrors() { + return diags } // Change is always the same for a destroy. We don't need the provider's @@ -784,7 +791,7 @@ func (n *EvalDiffDestroy) Eval(ctx EvalContext) (interface{}, error) { } // Call post-diff hook - err = ctx.Hook(func(h Hook) (HookAction, error) { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { return h.PostDiff( absAddr, n.DeposedKey.Generation(), @@ -792,9 +799,9 @@ func (n *EvalDiffDestroy) Eval(ctx EvalContext) (interface{}, error) { change.Before, change.After, ) - }) - if err != nil { - return nil, err + })) + if diags.HasErrors() { + return diags } // Update our output @@ -805,7 +812,7 @@ func (n *EvalDiffDestroy) Eval(ctx EvalContext) (interface{}, error) { *n.OutputState = nil } - return nil, nil + return diags } // EvalReduceDiff is an EvalNode implementation that takes a planned resource @@ -829,7 +836,7 @@ type EvalReduceDiff struct { } // TODO: test -func (n *EvalReduceDiff) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalReduceDiff) Eval(ctx EvalContext) tfdiags.Diagnostics { in := *n.InChange out := in.Simplify(n.Destroy) if n.OutChange != nil { @@ -842,7 +849,7 @@ func (n *EvalReduceDiff) Eval(ctx EvalContext) (interface{}, error) { log.Printf("[TRACE] EvalReduceDiff: %s change simplified from %s to %s for apply node", n.Addr, in.Action, out.Action) } } - return nil, nil + return nil } // EvalWriteDiff is an EvalNode implementation that saves a planned change @@ -855,7 +862,9 @@ type EvalWriteDiff struct { } // TODO: test -func (n *EvalWriteDiff) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalWriteDiff) Eval(ctx EvalContext) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + changes := ctx.Changes() addr := n.Addr.Absolute(ctx.Path()) if n.Change == nil || *n.Change == nil { @@ -866,7 +875,7 @@ func (n *EvalWriteDiff) Eval(ctx EvalContext) (interface{}, error) { gen = n.DeposedKey } changes.RemoveResourceInstanceChange(addr, gen) - return nil, nil + return nil } providerSchema := *n.ProviderSchema @@ -880,12 +889,14 @@ func (n *EvalWriteDiff) Eval(ctx EvalContext) (interface{}, error) { schema, _ := providerSchema.SchemaForResourceAddr(n.Addr.ContainingResource()) 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 } csrc, err := change.Encode(schema.ImpliedType()) if err != nil { - return nil, fmt.Errorf("failed to encode planned changes for %s: %s", addr, err) + diags = diags.Append(fmt.Errorf("failed to encode planned changes for %s: %s", addr, err)) + return diags } changes.AppendResourceInstanceChange(csrc) @@ -895,5 +906,5 @@ func (n *EvalWriteDiff) Eval(ctx EvalContext) (interface{}, error) { log.Printf("[TRACE] EvalWriteDiff: recorded %s change for %s deposed object %s", change.Action, addr, n.DeposedKey) } - return nil, nil + return diags } diff --git a/terraform/node_resource_apply_instance.go b/terraform/node_resource_apply_instance.go index 453387af60..7a109855f0 100644 --- a/terraform/node_resource_apply_instance.go +++ b/terraform/node_resource_apply_instance.go @@ -187,9 +187,9 @@ func (n *NodeApplyableResourceInstance) dataResourceExecute(ctx EvalContext) err ProviderSchema: &providerSchema, Change: nil, } - _, err = writeDiff.Eval(ctx) - if err != nil { - return err + diags := writeDiff.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } UpdateStateHook(ctx) @@ -276,9 +276,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) OutputChange: &diffApply, OutputState: &state, } - _, err = evalDiff.Eval(ctx) - if err != nil { - return err + diags = evalDiff.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } // Compare the diffs @@ -289,9 +289,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) Planned: &diff, Actual: &diffApply, } - _, err = checkPlannedChange.Eval(ctx) - if err != nil { - return err + diags = checkPlannedChange.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } readState = &EvalReadState{ @@ -312,9 +312,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) Destroy: false, OutChange: &diffApply, } - _, err = reduceDiff.Eval(ctx) - if err != nil { - return err + diags = reduceDiff.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } // EvalReduceDiff may have simplified our planned change @@ -361,9 +361,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) ProviderSchema: &providerSchema, Change: nil, } - _, err = writeDiff.Eval(ctx) - if err != nil { - return err + diags = writeDiff.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } evalMaybeTainted := &EvalMaybeTainted{ diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 94c8d2d805..b483bcbe04 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -155,9 +155,9 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) Destroy: true, OutChange: &changeApply, } - _, err = evalReduceDiff.Eval(ctx) - if err != nil { - return err + diags = evalReduceDiff.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } // EvalReduceDiff may have simplified our planned change diff --git a/terraform/node_resource_destroy_deposed.go b/terraform/node_resource_destroy_deposed.go index ebe04fa794..289b3a7ab7 100644 --- a/terraform/node_resource_destroy_deposed.go +++ b/terraform/node_resource_destroy_deposed.go @@ -96,9 +96,9 @@ func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walk State: &state, Output: &change, } - _, err = diffDestroy.Eval(ctx) - if err != nil { - return err + diags := diffDestroy.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } writeDiff := &EvalWriteDiff{ @@ -107,9 +107,9 @@ func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walk ProviderSchema: &providerSchema, Change: &change, } - _, err = writeDiff.Eval(ctx) - if err != nil { - return err + diags = writeDiff.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } return nil @@ -214,9 +214,9 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w State: &state, Output: &change, } - _, err = diffDestroy.Eval(ctx) - if err != nil { - return err + diags = diffDestroy.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } // Call pre-apply hook diff --git a/terraform/node_resource_plan_destroy.go b/terraform/node_resource_plan_destroy.go index 0118815bd8..53db9f14b6 100644 --- a/terraform/node_resource_plan_destroy.go +++ b/terraform/node_resource_plan_destroy.go @@ -57,9 +57,9 @@ func (n *NodePlanDestroyableResourceInstance) Execute(ctx EvalContext, op walkOp State: &state, Output: &change, } - _, err = diffDestroy.Eval(ctx) - if err != nil { - return err + diags := diffDestroy.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } err = n.checkPreventDestroy(change) @@ -72,6 +72,6 @@ func (n *NodePlanDestroyableResourceInstance) Execute(ctx EvalContext, op walkOp ProviderSchema: &providerSchema, Change: &change, } - _, err = writeDiff.Eval(ctx) - return err + diags = writeDiff.Eval(ctx) + return diags.ErrWithWarnings() } diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index 3810484d05..836f1328e4 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -119,8 +119,8 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) err ProviderSchema: &providerSchema, Change: &change, } - _, err = writeDiff.Eval(ctx) - return err + diags := writeDiff.Eval(ctx) + return diags.ErrWithWarnings() } func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) error { @@ -204,9 +204,9 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) OutputChange: &change, OutputState: &instancePlanState, } - _, err = diff.Eval(ctx) - if err != nil { - return err + diags := diff.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } err = n.checkPreventDestroy(change) @@ -230,6 +230,6 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) ProviderSchema: &providerSchema, Change: &change, } - _, err = writeDiff.Eval(ctx) - return err + diags = writeDiff.Eval(ctx) + return diags.ErrWithWarnings() } diff --git a/terraform/node_resource_plan_orphan.go b/terraform/node_resource_plan_orphan.go index 515caec58d..ca9b1000bf 100644 --- a/terraform/node_resource_plan_orphan.go +++ b/terraform/node_resource_plan_orphan.go @@ -114,9 +114,9 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon Output: &change, OutputState: &state, // Will point to a nil state after this complete, signalling destroyed } - _, err = diffDestroy.Eval(ctx) + diags := diffDestroy.Eval(ctx) if err != nil { - return err + return diags.ErrWithWarnings() } err = n.checkPreventDestroy(change) @@ -129,9 +129,9 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon ProviderSchema: &providerSchema, Change: &change, } - _, err = writeDiff.Eval(ctx) - if err != nil { - return err + diags = writeDiff.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } writeState := &EvalWriteState{ From 64491df856e9e4ae8ccdb56a6475c77f824c1017 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 28 Oct 2020 11:57:45 -0400 Subject: [PATCH 04/12] add diags to data eval --- terraform/eval_read_data_apply.go | 32 +++++++++++--------- terraform/eval_read_data_plan.go | 37 +++++++++++------------ terraform/node_resource_apply_instance.go | 9 +++--- terraform/node_resource_plan_instance.go | 10 +++--- 4 files changed, 45 insertions(+), 43 deletions(-) diff --git a/terraform/eval_read_data_apply.go b/terraform/eval_read_data_apply.go index 179fe368d0..c9e235250d 100644 --- a/terraform/eval_read_data_apply.go +++ b/terraform/eval_read_data_apply.go @@ -15,7 +15,7 @@ type evalReadDataApply struct { evalReadData } -func (n *evalReadDataApply) Eval(ctx EvalContext) (interface{}, error) { +func (n *evalReadDataApply) Eval(ctx EvalContext) tfdiags.Diagnostics { absAddr := n.Addr.Absolute(ctx.Path()) var diags tfdiags.Diagnostics @@ -26,22 +26,25 @@ func (n *evalReadDataApply) Eval(ctx EvalContext) (interface{}, error) { } if n.ProviderSchema == nil || *n.ProviderSchema == nil { - return nil, fmt.Errorf("provider schema not available for %s", n.Addr) + diags = diags.Append(fmt.Errorf("provider schema not available for %s", n.Addr)) + return diags } if planned != nil && planned.Action != plans.Read { // If any other action gets in here then that's always a bug; this // EvalNode only deals with reading. - return nil, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "invalid action %s for %s: only Read is supported (this is a bug in Terraform; please report it!)", planned.Action, absAddr, - ) + )) + return diags } - if err := ctx.Hook(func(h Hook) (HookAction, error) { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { return h.PreApply(absAddr, states.CurrentGen, planned.Action, planned.Before, planned.After) - }); err != nil { - return nil, err + })) + if diags.HasErrors() { + return diags } config := *n.Config @@ -49,7 +52,8 @@ func (n *evalReadDataApply) Eval(ctx EvalContext) (interface{}, error) { schema, _ := providerSchema.SchemaForResourceAddr(n.Addr.ContainingResource()) if schema == nil { // Should be caught during validation, so we don't bother with a pretty error here - return nil, fmt.Errorf("provider %q does not support data source %q", n.ProviderAddr.Provider.String(), n.Addr.Resource.Type) + diags = diags.Append(fmt.Errorf("provider %q does not support data source %q", n.ProviderAddr.Provider.String(), n.Addr.Resource.Type)) + return diags } forEach, _ := evaluateForEachExpression(config.ForEach, ctx) @@ -58,13 +62,13 @@ func (n *evalReadDataApply) Eval(ctx EvalContext) (interface{}, error) { configVal, _, configDiags := ctx.EvaluateBlock(config.Config, schema, nil, keyData) diags = diags.Append(configDiags) if configDiags.HasErrors() { - return nil, diags.ErrWithWarnings() + return diags } newVal, readDiags := n.readDataSource(ctx, configVal) diags = diags.Append(readDiags) if diags.HasErrors() { - return nil, diags.ErrWithWarnings() + return diags } *n.State = &states.ResourceInstanceObject{ @@ -72,11 +76,9 @@ func (n *evalReadDataApply) Eval(ctx EvalContext) (interface{}, error) { Status: states.ObjectReady, } - if err := ctx.Hook(func(h Hook) (HookAction, error) { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { return h.PostApply(absAddr, states.CurrentGen, newVal, diags.Err()) - }); err != nil { - diags = diags.Append(err) - } + })) - return nil, diags.ErrWithWarnings() + return diags } diff --git a/terraform/eval_read_data_plan.go b/terraform/eval_read_data_plan.go index 2265942992..a1d7ccce6c 100644 --- a/terraform/eval_read_data_plan.go +++ b/terraform/eval_read_data_plan.go @@ -21,14 +21,15 @@ type evalReadDataPlan struct { evalReadData } -func (n *evalReadDataPlan) Eval(ctx EvalContext) (interface{}, error) { +func (n *evalReadDataPlan) Eval(ctx EvalContext) tfdiags.Diagnostics { absAddr := n.Addr.Absolute(ctx.Path()) var diags tfdiags.Diagnostics var configVal cty.Value if n.ProviderSchema == nil || *n.ProviderSchema == nil { - return nil, fmt.Errorf("provider schema not available for %s", n.Addr) + diags = diags.Append(fmt.Errorf("provider schema not available for %s", n.Addr)) + return diags } config := *n.Config @@ -36,7 +37,8 @@ func (n *evalReadDataPlan) Eval(ctx EvalContext) (interface{}, error) { schema, _ := providerSchema.SchemaForResourceAddr(n.Addr.ContainingResource()) if schema == nil { // Should be caught during validation, so we don't bother with a pretty error here - return nil, fmt.Errorf("provider %q does not support data source %q", n.ProviderAddr.Provider.String(), n.Addr.Resource.Type) + diags = diags.Append(fmt.Errorf("provider %q does not support data source %q", n.ProviderAddr.Provider.String(), n.Addr.Resource.Type)) + return diags } objTy := schema.ImpliedType() @@ -52,7 +54,7 @@ func (n *evalReadDataPlan) Eval(ctx EvalContext) (interface{}, error) { configVal, _, configDiags = ctx.EvaluateBlock(config.Config, schema, nil, keyData) diags = diags.Append(configDiags) if configDiags.HasErrors() { - return nil, diags.ErrWithWarnings() + return diags } configKnown := configVal.IsWhollyKnown() @@ -69,11 +71,11 @@ func (n *evalReadDataPlan) Eval(ctx EvalContext) (interface{}, error) { proposedNewVal := objchange.PlannedDataResourceObject(schema, configVal) - if err := ctx.Hook(func(h Hook) (HookAction, error) { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { return h.PreDiff(absAddr, states.CurrentGen, priorVal, proposedNewVal) - }); err != nil { - diags = diags.Append(err) - return nil, diags.ErrWithWarnings() + })) + if diags.HasErrors() { + return diags } // Apply detects that the data source will need to be read by the After @@ -93,13 +95,11 @@ func (n *evalReadDataPlan) Eval(ctx EvalContext) (interface{}, error) { Status: states.ObjectPlanned, } - if err := ctx.Hook(func(h Hook) (HookAction, error) { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { return h.PostDiff(absAddr, states.CurrentGen, plans.Read, priorVal, proposedNewVal) - }); err != nil { - diags = diags.Append(err) - } + })) - return nil, diags.ErrWithWarnings() + return diags } // We have a complete configuration with no dependencies to wait on, so we @@ -107,7 +107,7 @@ func (n *evalReadDataPlan) Eval(ctx EvalContext) (interface{}, error) { newVal, readDiags := n.readDataSource(ctx, configVal) diags = diags.Append(readDiags) if diags.HasErrors() { - return nil, diags.ErrWithWarnings() + return diags } // if we have a prior value, we can check for any irregularities in the response @@ -137,13 +137,10 @@ func (n *evalReadDataPlan) Eval(ctx EvalContext) (interface{}, error) { Status: states.ObjectReady, } - if err := ctx.Hook(func(h Hook) (HookAction, error) { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { return h.PostDiff(absAddr, states.CurrentGen, plans.Update, priorVal, newVal) - }); err != nil { - return nil, err - } - - return nil, diags.ErrWithWarnings() + })) + return diags } // forcePlanRead determines if we need to override the usual behavior of diff --git a/terraform/node_resource_apply_instance.go b/terraform/node_resource_apply_instance.go index 7a109855f0..82f5b2f464 100644 --- a/terraform/node_resource_apply_instance.go +++ b/terraform/node_resource_apply_instance.go @@ -134,6 +134,7 @@ func (n *NodeApplyableResourceInstance) Execute(ctx EvalContext, op walkOperatio } func (n *NodeApplyableResourceInstance) dataResourceExecute(ctx EvalContext) error { + var diags tfdiags.Diagnostics addr := n.ResourceInstanceAddr().Resource provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) @@ -166,9 +167,9 @@ func (n *NodeApplyableResourceInstance) dataResourceExecute(ctx EvalContext) err State: &state, }, } - _, err = readDataApply.Eval(ctx) - if err != nil { - return err + diags = readDataApply.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } writeState := &EvalWriteState{ @@ -187,7 +188,7 @@ func (n *NodeApplyableResourceInstance) dataResourceExecute(ctx EvalContext) err ProviderSchema: &providerSchema, Change: nil, } - diags := writeDiff.Eval(ctx) + diags = writeDiff.Eval(ctx) if diags.HasErrors() { return diags.ErrWithWarnings() } diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index 836f1328e4..64ab930ec5 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -5,6 +5,7 @@ import ( "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/states" + "github.com/hashicorp/terraform/tfdiags" "github.com/hashicorp/terraform/addrs" ) @@ -45,6 +46,7 @@ func (n *NodePlannableResourceInstance) Execute(ctx EvalContext, op walkOperatio } func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) error { + var diags tfdiags.Diagnostics config := n.Config addr := n.ResourceInstanceAddr() @@ -84,9 +86,9 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) err dependsOn: n.dependsOn, }, } - _, err = readDataPlan.Eval(ctx) - if err != nil { - return err + diags = readDataPlan.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } // write the data source into both the refresh state and the @@ -119,7 +121,7 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) err ProviderSchema: &providerSchema, Change: &change, } - diags := writeDiff.Eval(ctx) + diags = writeDiff.Eval(ctx) return diags.ErrWithWarnings() } From 524505830fbb3aa3ed614b3b396d961843089685 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 28 Oct 2020 12:03:00 -0400 Subject: [PATCH 05/12] add diags to eval_refresh --- terraform/eval_refresh.go | 31 ++++++++++++------------ terraform/node_resource_plan_instance.go | 6 ++--- terraform/node_resource_plan_orphan.go | 11 ++++++--- terraform/transform_import_state.go | 6 ++--- 4 files changed, 29 insertions(+), 25 deletions(-) diff --git a/terraform/eval_refresh.go b/terraform/eval_refresh.go index 737484c45b..d69bedd64c 100644 --- a/terraform/eval_refresh.go +++ b/terraform/eval_refresh.go @@ -29,7 +29,7 @@ type EvalRefresh struct { } // TODO: test -func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalRefresh) Eval(ctx EvalContext) tfdiags.Diagnostics { state := *n.State absAddr := n.Addr.Absolute(ctx.Path()) @@ -38,13 +38,14 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) { // If we have no state, we don't do any refreshing if state == nil { log.Printf("[DEBUG] refresh: %s: no state, so not refreshing", n.Addr.Absolute(ctx.Path())) - return nil, diags.ErrWithWarnings() + return diags } schema, _ := (*n.ProviderSchema).SchemaForResourceAddr(n.Addr.ContainingResource()) 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 } metaConfigVal := cty.NullVal(cty.DynamicPseudoType) @@ -66,18 +67,18 @@ func (n *EvalRefresh) 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 } } } } // Call pre-refresh hook - err := ctx.Hook(func(h Hook) (HookAction, error) { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { return h.PreRefresh(absAddr, states.CurrentGen, state.Value) - }) - if err != nil { - return nil, diags.ErrWithWarnings() + })) + if diags.HasErrors() { + return diags } // Refresh! @@ -100,7 +101,7 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) { resp := provider.ReadResource(req) diags = diags.Append(resp.Diagnostics) if diags.HasErrors() { - return nil, diags.Err() + return diags } if resp.NewState == cty.NilVal { @@ -121,7 +122,7 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) { )) } if diags.HasErrors() { - return nil, diags.Err() + return diags } // We have no way to exempt provider using the legacy SDK from this check, @@ -142,11 +143,11 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) { newState.CreateBeforeDestroy = state.CreateBeforeDestroy // Call post-refresh hook - err = ctx.Hook(func(h Hook) (HookAction, error) { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { return h.PostRefresh(absAddr, states.CurrentGen, priorVal, newState.Value) - }) - if err != nil { - return nil, err + })) + if diags.HasErrors() { + return diags } // Mark the value if necessary @@ -158,5 +159,5 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) { *n.Output = newState } - return nil, diags.ErrWithWarnings() + return diags } diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index 64ab930ec5..86b8b0f67c 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -174,9 +174,9 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) State: &instanceRefreshState, Output: &instanceRefreshState, } - _, err = refresh.Eval(ctx) - if err != nil { - return err + diags := refresh.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } writeRefreshState := &EvalWriteState{ diff --git a/terraform/node_resource_plan_orphan.go b/terraform/node_resource_plan_orphan.go index ca9b1000bf..da2a890b2a 100644 --- a/terraform/node_resource_plan_orphan.go +++ b/terraform/node_resource_plan_orphan.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/states" + "github.com/hashicorp/terraform/tfdiags" ) // NodePlannableResourceInstanceOrphan represents a resource that is "applyable": @@ -56,6 +57,8 @@ func (n *NodePlannableResourceInstanceOrphan) dataResourceExecute(ctx EvalContex } func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalContext) error { + var diags tfdiags.Diagnostics + addr := n.ResourceInstanceAddr() // Declare a bunch of variables that are used for state during @@ -89,9 +92,9 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon State: &state, Output: &state, } - _, err = refresh.Eval(ctx) - if err != nil { - return err + diags = refresh.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } writeRefreshState := &EvalWriteState{ @@ -114,7 +117,7 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon Output: &change, OutputState: &state, // Will point to a nil state after this complete, signalling destroyed } - diags := diffDestroy.Eval(ctx) + diags = diffDestroy.Eval(ctx) if err != nil { return diags.ErrWithWarnings() } diff --git a/terraform/transform_import_state.go b/terraform/transform_import_state.go index bbeba7aa77..717beacb4c 100644 --- a/terraform/transform_import_state.go +++ b/terraform/transform_import_state.go @@ -280,9 +280,9 @@ func (n *graphNodeImportStateSub) Execute(ctx EvalContext, op walkOperation) err State: &state, Output: &state, } - _, err = evalRefresh.Eval(ctx) - if err != nil { - return err + diags := evalRefresh.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } // Verify the existance of the imported resource From c81fd833bb86853d37ed67488e032cbb95fbaa3a Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 28 Oct 2020 12:23:03 -0400 Subject: [PATCH 06/12] add diags to eval_state --- terraform/eval_state.go | 96 +++++++++++++--------- terraform/eval_state_test.go | 30 +++---- terraform/node_resource_apply_instance.go | 42 +++++----- terraform/node_resource_destroy.go | 6 +- terraform/node_resource_destroy_deposed.go | 22 ++--- terraform/node_resource_plan_instance.go | 33 ++++---- terraform/node_resource_plan_orphan.go | 12 +-- terraform/transform_import_state.go | 8 +- 8 files changed, 133 insertions(+), 116 deletions(-) diff --git a/terraform/eval_state.go b/terraform/eval_state.go index 1edab45168..b531f86c9e 100644 --- a/terraform/eval_state.go +++ b/terraform/eval_state.go @@ -37,7 +37,9 @@ type EvalReadState struct { Output **states.ResourceInstanceObject } -func (n *EvalReadState) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalReadState) Eval(ctx EvalContext) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + if n.Provider == nil || *n.Provider == nil { panic("EvalReadState used with no Provider object") } @@ -52,33 +54,35 @@ func (n *EvalReadState) Eval(ctx EvalContext) (interface{}, error) { if src == nil { // Presumably we only have deposed objects, then. log.Printf("[TRACE] EvalReadState: no state present for %s", absAddr) - return nil, nil + return nil } schema, currentVersion := (*n.ProviderSchema).SchemaForResourceAddr(n.Addr.ContainingResource()) if schema == nil { // Shouldn't happen since we should've failed long ago if no schema is present - return nil, fmt.Errorf("no schema available for %s while reading state; this is a bug in Terraform and should be reported", absAddr) + diags = diags.Append(fmt.Errorf("no schema available for %s while reading state; this is a bug in Terraform and should be reported", absAddr)) + return diags } - var diags tfdiags.Diagnostics + src, diags = UpgradeResourceState(absAddr, *n.Provider, src, schema, currentVersion) if diags.HasErrors() { // Note that we don't have any channel to return warnings here. We'll // accept that for now since warnings during a schema upgrade would // be pretty weird anyway, since this operation is supposed to seem // invisible to the user. - return nil, diags.Err() + return diags } obj, err := src.Decode(schema.ImpliedType()) if err != nil { - return nil, err + diags = diags.Append(err) + return diags } if n.Output != nil { *n.Output = obj } - return obj, nil + return diags } // EvalReadStateDeposed is an EvalNode implementation that reads the @@ -102,7 +106,9 @@ type EvalReadStateDeposed struct { Output **states.ResourceInstanceObject } -func (n *EvalReadStateDeposed) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalReadStateDeposed) Eval(ctx EvalContext) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + if n.Provider == nil || *n.Provider == nil { panic("EvalReadStateDeposed used with no Provider object") } @@ -112,7 +118,8 @@ func (n *EvalReadStateDeposed) Eval(ctx EvalContext) (interface{}, error) { key := n.Key if key == states.NotDeposed { - return nil, fmt.Errorf("EvalReadStateDeposed used with no instance key; this is a bug in Terraform and should be reported") + diags = diags.Append(fmt.Errorf("EvalReadStateDeposed used with no instance key; this is a bug in Terraform and should be reported")) + return diags } absAddr := n.Addr.Absolute(ctx.Path()) log.Printf("[TRACE] EvalReadStateDeposed: reading state for %s deposed object %s", absAddr, n.Key) @@ -121,32 +128,34 @@ func (n *EvalReadStateDeposed) Eval(ctx EvalContext) (interface{}, error) { if src == nil { // Presumably we only have deposed objects, then. log.Printf("[TRACE] EvalReadStateDeposed: no state present for %s deposed object %s", absAddr, n.Key) - return nil, nil + return diags } schema, currentVersion := (*n.ProviderSchema).SchemaForResourceAddr(n.Addr.ContainingResource()) if schema == nil { // Shouldn't happen since we should've failed long ago if no schema is present - return nil, fmt.Errorf("no schema available for %s while reading state; this is a bug in Terraform and should be reported", absAddr) + diags = diags.Append(fmt.Errorf("no schema available for %s while reading state; this is a bug in Terraform and should be reported", absAddr)) + return diags } - var diags tfdiags.Diagnostics + src, diags = UpgradeResourceState(absAddr, *n.Provider, src, schema, currentVersion) if diags.HasErrors() { // Note that we don't have any channel to return warnings here. We'll // accept that for now since warnings during a schema upgrade would // be pretty weird anyway, since this operation is supposed to seem // invisible to the user. - return nil, diags.Err() + return diags } obj, err := src.Decode(schema.ImpliedType()) if err != nil { - return nil, err + diags = diags.Append(err) + return diags } if n.Output != nil { *n.Output = obj } - return obj, nil + return diags } // UpdateStateHook calls the PostStateUpdate hook with the current state. @@ -173,7 +182,7 @@ type evalWriteEmptyState struct { EvalWriteState } -func (n *evalWriteEmptyState) Eval(ctx EvalContext) (interface{}, error) { +func (n *evalWriteEmptyState) Eval(ctx EvalContext) tfdiags.Diagnostics { var state *states.ResourceInstanceObject n.State = &state return n.EvalWriteState.Eval(ctx) @@ -204,7 +213,9 @@ type EvalWriteState struct { targetState phaseState } -func (n *EvalWriteState) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalWriteState) Eval(ctx EvalContext) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + if n.State == nil { // Note that a pointer _to_ nil is valid here, indicating the total // absense of an object as we'd see during destroy. @@ -223,14 +234,15 @@ func (n *EvalWriteState) Eval(ctx EvalContext) (interface{}, error) { } if n.ProviderAddr.Provider.Type == "" { - return nil, fmt.Errorf("failed to write state for %s: missing provider type", absAddr) + diags = diags.Append(fmt.Errorf("failed to write state for %s: missing provider type", absAddr)) + return diags } obj := *n.State if obj == nil || obj.Value.IsNull() { // No need to encode anything: we'll just write it directly. state.SetResourceInstanceCurrent(absAddr, nil, n.ProviderAddr) log.Printf("[TRACE] EvalWriteState: removing state object for %s", absAddr) - return nil, nil + return diags } // store the new deps in the state @@ -255,15 +267,17 @@ func (n *EvalWriteState) Eval(ctx EvalContext) (interface{}, error) { // It shouldn't be possible to get this far in any real scenario // without a schema, but we might end up here in contrived tests that // fail to set up their world properly. - return nil, fmt.Errorf("failed to encode %s in state: no resource type schema available", absAddr) + diags = diags.Append(fmt.Errorf("failed to encode %s in state: no resource type schema available", absAddr)) + return diags } src, err := obj.Encode(schema.ImpliedType(), currentVersion) if err != nil { - return nil, fmt.Errorf("failed to encode %s in state: %s", absAddr, err) + diags = diags.Append(fmt.Errorf("failed to encode %s in state: %s", absAddr, err)) + return diags } state.SetResourceInstanceCurrent(absAddr, src, n.ProviderAddr) - return nil, nil + return diags } // EvalWriteStateDeposed is an EvalNode implementation that writes @@ -286,7 +300,9 @@ type EvalWriteStateDeposed struct { ProviderAddr addrs.AbsProviderConfig } -func (n *EvalWriteStateDeposed) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalWriteStateDeposed) Eval(ctx EvalContext) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + if n.State == nil { // Note that a pointer _to_ nil is valid here, indicating the total // absense of an object as we'd see during destroy. @@ -299,7 +315,8 @@ func (n *EvalWriteStateDeposed) Eval(ctx EvalContext) (interface{}, error) { if key == states.NotDeposed { // should never happen - return nil, fmt.Errorf("can't save deposed object for %s without a deposed key; this is a bug in Terraform that should be reported", absAddr) + diags = diags.Append(fmt.Errorf("can't save deposed object for %s without a deposed key; this is a bug in Terraform that should be reported", absAddr)) + return diags } obj := *n.State @@ -307,7 +324,7 @@ func (n *EvalWriteStateDeposed) Eval(ctx EvalContext) (interface{}, error) { // No need to encode anything: we'll just write it directly. state.SetResourceInstanceDeposed(absAddr, key, nil, n.ProviderAddr) log.Printf("[TRACE] EvalWriteStateDeposed: removing state object for %s deposed %s", absAddr, key) - return nil, nil + return diags } if n.ProviderSchema == nil || *n.ProviderSchema == nil { // Should never happen, unless our state object is nil @@ -319,16 +336,18 @@ func (n *EvalWriteStateDeposed) Eval(ctx EvalContext) (interface{}, error) { // It shouldn't be possible to get this far in any real scenario // without a schema, but we might end up here in contrived tests that // fail to set up their world properly. - return nil, fmt.Errorf("failed to encode %s in state: no resource type schema available", absAddr) + diags = diags.Append(fmt.Errorf("failed to encode %s in state: no resource type schema available", absAddr)) + return diags } src, err := obj.Encode(schema.ImpliedType(), currentVersion) if err != nil { - return nil, fmt.Errorf("failed to encode %s in state: %s", absAddr, err) + diags = diags.Append(fmt.Errorf("failed to encode %s in state: %s", absAddr, err)) + return diags } log.Printf("[TRACE] EvalWriteStateDeposed: writing state object for %s deposed %s", absAddr, key) state.SetResourceInstanceDeposed(absAddr, key, src, n.ProviderAddr) - return nil, nil + return diags } // EvalDeposeState is an EvalNode implementation that moves the current object @@ -353,7 +372,7 @@ type EvalDeposeState struct { } // TODO: test -func (n *EvalDeposeState) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalDeposeState) Eval(ctx EvalContext) tfdiags.Diagnostics { absAddr := n.Addr.Absolute(ctx.Path()) state := ctx.State() @@ -370,7 +389,7 @@ func (n *EvalDeposeState) Eval(ctx EvalContext) (interface{}, error) { *n.OutputKey = key } - return nil, nil + return nil } // EvalMaybeRestoreDeposedObject is an EvalNode implementation that will @@ -405,7 +424,9 @@ type EvalMaybeRestoreDeposedObject struct { } // TODO: test -func (n *EvalMaybeRestoreDeposedObject) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalMaybeRestoreDeposedObject) Eval(ctx EvalContext) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + absAddr := n.Addr.Absolute(ctx.Path()) dk := *n.Key state := ctx.State() @@ -414,7 +435,6 @@ func (n *EvalMaybeRestoreDeposedObject) Eval(ctx EvalContext) (interface{}, erro // This should never happen, and so it always indicates a bug. // We should evaluate this node only if we've previously deposed // an object as part of the same operation. - var diags tfdiags.Diagnostics if n.PlannedChange != nil && *n.PlannedChange != nil { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, @@ -434,7 +454,7 @@ func (n *EvalMaybeRestoreDeposedObject) Eval(ctx EvalContext) (interface{}, erro ), )) } - return nil, diags.Err() + return diags } restored := state.MaybeRestoreResourceInstanceDeposed(absAddr, dk) @@ -444,7 +464,7 @@ func (n *EvalMaybeRestoreDeposedObject) Eval(ctx EvalContext) (interface{}, erro log.Printf("[TRACE] EvalMaybeRestoreDeposedObject: %s deposed object %s remains deposed", absAddr, dk) } - return nil, nil + return diags } // EvalRefreshLifecycle is an EvalNode implementation that updates @@ -461,21 +481,21 @@ type EvalRefreshLifecycle struct { ForceCreateBeforeDestroy bool } -func (n *EvalRefreshLifecycle) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalRefreshLifecycle) Eval(ctx EvalContext) tfdiags.Diagnostics { state := *n.State if state == nil { // no existing state - return nil, nil + return nil } // In 0.13 we could be refreshing a resource with no config. // We should be operating on managed resource, but check here to be certain if n.Config == nil || n.Config.Managed == nil { log.Printf("[WARN] EvalRefreshLifecycle: no Managed config value found in instance state for %q", n.Addr) - return nil, nil + return nil } state.CreateBeforeDestroy = n.Config.Managed.CreateBeforeDestroy || n.ForceCreateBeforeDestroy - return nil, nil + return nil } diff --git a/terraform/eval_state_test.go b/terraform/eval_state_test.go index a9c73cded7..50519bedcd 100644 --- a/terraform/eval_state_test.go +++ b/terraform/eval_state_test.go @@ -67,15 +67,12 @@ func TestEvalReadState(t *testing.T) { ctx.StateState = state.SyncWrapper() ctx.PathPath = addrs.RootModuleInstance - result, err := c.Node.Eval(ctx) - if err != nil { - t.Fatalf("[%s] Got err: %#v", k, err) + diags := c.Node.Eval(ctx) + if diags.HasErrors() { + t.Fatalf("[%s] Got err: %#v", k, diags.ErrWithWarnings()) } expected := c.ExpectedInstanceId - if !(result != nil && instanceObjectIdForTests(result.(*states.ResourceInstanceObject)) == expected) { - t.Fatalf("[%s] Expected return with ID %#v, got: %#v", k, expected, result) - } if !(output != nil && output.Value.GetAttr("id") == cty.StringVal(expected)) { t.Fatalf("[%s] Expected output with ID %#v, got: %#v", k, expected, output) @@ -141,15 +138,12 @@ func TestEvalReadStateDeposed(t *testing.T) { ctx.StateState = state.SyncWrapper() ctx.PathPath = addrs.RootModuleInstance - result, err := c.Node.Eval(ctx) - if err != nil { - t.Fatalf("[%s] Got err: %#v", k, err) + diags := c.Node.Eval(ctx) + if diags.HasErrors() { + t.Fatalf("[%s] Got err: %#v", k, diags.ErrWithWarnings()) } expected := c.ExpectedInstanceId - if !(result != nil && instanceObjectIdForTests(result.(*states.ResourceInstanceObject)) == expected) { - t.Fatalf("[%s] Expected return with ID %#v, got: %#v", k, expected, result) - } if !(output != nil && output.Value.GetAttr("id") == cty.StringVal(expected)) { t.Fatalf("[%s] Expected output with ID %#v, got: %#v", k, expected, output) @@ -194,9 +188,9 @@ func TestEvalWriteState(t *testing.T) { ProviderSchema: &providerSchema, ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("aws")), } - _, err := node.Eval(ctx) - if err != nil { - t.Fatalf("Got err: %#v", err) + diags := node.Eval(ctx) + if diags.HasErrors() { + t.Fatalf("Got err: %#v", diags.ErrWithWarnings()) } checkStateString(t, state, ` @@ -241,9 +235,9 @@ func TestEvalWriteStateDeposed(t *testing.T) { ProviderSchema: &providerSchema, ProviderAddr: addrs.RootModuleInstance.ProviderConfigDefault(addrs.NewDefaultProvider("aws")), } - _, err := node.Eval(ctx) - if err != nil { - t.Fatalf("Got err: %#v", err) + diags := node.Eval(ctx) + if diags.HasErrors() { + t.Fatalf("Got err: %#v", diags.ErrWithWarnings()) } checkStateString(t, state, ` diff --git a/terraform/node_resource_apply_instance.go b/terraform/node_resource_apply_instance.go index 82f5b2f464..0d902b837f 100644 --- a/terraform/node_resource_apply_instance.go +++ b/terraform/node_resource_apply_instance.go @@ -178,9 +178,9 @@ func (n *NodeApplyableResourceInstance) dataResourceExecute(ctx EvalContext) err ProviderSchema: &providerSchema, State: &state, } - _, err = writeState.Eval(ctx) - if err != nil { - return err + diags = writeState.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } writeDiff := &EvalWriteDiff{ @@ -239,9 +239,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) ForceKey: n.PreallocatedDeposedKey, OutputKey: &deposedKey, } - _, err = deposeState.Eval(ctx) - if err != nil { - return err + diags = deposeState.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } } @@ -252,9 +252,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) Output: &state, } - _, err = readState.Eval(ctx) - if err != nil { - return err + diags = readState.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } // Get the saved diff @@ -302,9 +302,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) Output: &state, } - _, err = readState.Eval(ctx) - if err != nil { - return err + diags = readState.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } reduceDiff := &EvalReduceDiff{ @@ -385,9 +385,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) State: &state, Dependencies: &n.Dependencies, } - _, err = writeState.Eval(ctx) - if err != nil { - return err + diags = writeState.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } applyProvisioners := &EvalApplyProvisioners{ @@ -421,9 +421,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) State: &state, Dependencies: &n.Dependencies, } - _, err = writeState.Eval(ctx) - if err != nil { - return err + diags = writeState.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } if createBeforeDestroyEnabled && applyError != nil { @@ -432,9 +432,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) PlannedChange: &diffApply, Key: &deposedKey, } - _, err := maybeRestoreDesposedObject.Eval(ctx) - if err != nil { - return err + diags := maybeRestoreDesposedObject.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } } diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index b483bcbe04..26dd56dee4 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -240,9 +240,9 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) ProviderSchema: &providerSchema, State: &state, } - _, err = evalWriteState.Eval(ctx) - if err != nil { - return err + diags = evalWriteState.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } } else { log.Printf("[TRACE] NodeDestroyResourceInstance: removing state object for %s", n.Addr) diff --git a/terraform/node_resource_destroy_deposed.go b/terraform/node_resource_destroy_deposed.go index 289b3a7ab7..4a641dece6 100644 --- a/terraform/node_resource_destroy_deposed.go +++ b/terraform/node_resource_destroy_deposed.go @@ -65,6 +65,8 @@ func (n *NodePlanDeposedResourceInstanceObject) References() []*addrs.Reference // GraphNodeEvalable impl. func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walkOperation) error { + var diags tfdiags.Diagnostics + addr := n.ResourceInstanceAddr() provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) @@ -84,9 +86,9 @@ func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walk Provider: &provider, ProviderSchema: &providerSchema, } - _, err = readStateDeposed.Eval(ctx) - if err != nil { - return err + diags = readStateDeposed.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } diffDestroy := &EvalDiffDestroy{ @@ -96,7 +98,7 @@ func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walk State: &state, Output: &change, } - diags := diffDestroy.Eval(ctx) + diags = diffDestroy.Eval(ctx) if diags.HasErrors() { return diags.ErrWithWarnings() } @@ -203,9 +205,9 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w Provider: &provider, ProviderSchema: &providerSchema, } - _, err = readStateDeposed.Eval(ctx) - if err != nil { - return err + diags = readStateDeposed.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } diffDestroy := &EvalDiffDestroy{ @@ -256,9 +258,9 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w ProviderSchema: &providerSchema, State: &state, } - _, err = writeStateDeposed.Eval(ctx) - if err != nil { - return err + diags = writeStateDeposed.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } applyPost := &EvalApplyPost{ diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index 86b8b0f67c..935f7caa01 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -100,9 +100,9 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) err State: &state, targetState: refreshState, } - _, err = writeRefreshState.Eval(ctx) - if err != nil { - return err + diags = writeRefreshState.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } writeState := &EvalWriteState{ @@ -111,9 +111,9 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) err ProviderSchema: &providerSchema, State: &state, } - _, err = writeState.Eval(ctx) - if err != nil { - return err + diags = writeState.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } writeDiff := &EvalWriteDiff{ @@ -126,6 +126,7 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) err } func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) error { + var diags tfdiags.Diagnostics config := n.Config addr := n.ResourceInstanceAddr() @@ -158,9 +159,9 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) State: &instanceRefreshState, ForceCreateBeforeDestroy: n.ForceCreateBeforeDestroy, } - _, err = refreshLifecycle.Eval(ctx) - if err != nil { - return err + diags = refreshLifecycle.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } // Refresh, maybe @@ -187,9 +188,9 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) targetState: refreshState, Dependencies: &n.Dependencies, } - _, err = writeRefreshState.Eval(ctx) - if err != nil { - return err + diags = writeRefreshState.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } } @@ -206,7 +207,7 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) OutputChange: &change, OutputState: &instancePlanState, } - diags := diff.Eval(ctx) + diags = diff.Eval(ctx) if diags.HasErrors() { return diags.ErrWithWarnings() } @@ -222,9 +223,9 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) State: &instancePlanState, ProviderSchema: &providerSchema, } - _, err = writeState.Eval(ctx) - if err != nil { - return err + diags = writeState.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } writeDiff := &EvalWriteDiff{ diff --git a/terraform/node_resource_plan_orphan.go b/terraform/node_resource_plan_orphan.go index da2a890b2a..56edb17098 100644 --- a/terraform/node_resource_plan_orphan.go +++ b/terraform/node_resource_plan_orphan.go @@ -104,9 +104,9 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon State: &state, targetState: refreshState, } - _, err = writeRefreshState.Eval(ctx) - if err != nil { - return err + diags = writeRefreshState.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } } @@ -143,9 +143,9 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon ProviderSchema: &providerSchema, State: &state, } - _, err = writeState.Eval(ctx) - if err != nil { - return err + diags = writeState.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } return nil } diff --git a/terraform/transform_import_state.go b/terraform/transform_import_state.go index 717beacb4c..6547f1a6d7 100644 --- a/terraform/transform_import_state.go +++ b/terraform/transform_import_state.go @@ -260,6 +260,7 @@ func (n *graphNodeImportStateSub) Path() addrs.ModuleInstance { // GraphNodeExecutable impl. func (n *graphNodeImportStateSub) Execute(ctx EvalContext, op walkOperation) error { + var diags tfdiags.Diagnostics // If the Ephemeral type isn't set, then it is an error if n.State.TypeName == "" { return fmt.Errorf("import of %s didn't set type", n.TargetAddr.String()) @@ -280,14 +281,13 @@ func (n *graphNodeImportStateSub) Execute(ctx EvalContext, op walkOperation) err State: &state, Output: &state, } - diags := evalRefresh.Eval(ctx) + diags = evalRefresh.Eval(ctx) if diags.HasErrors() { return diags.ErrWithWarnings() } // Verify the existance of the imported resource if state.Value.IsNull() { - var diags tfdiags.Diagnostics diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, "Cannot import non-existent remote object", @@ -306,6 +306,6 @@ func (n *graphNodeImportStateSub) Execute(ctx EvalContext, op walkOperation) err ProviderSchema: &providerSchema, State: &state, } - _, err = evalWriteState.Eval(ctx) - return err + diags = diags.Append(evalWriteState.Eval(ctx)) + return diags.ErrWithWarnings() } From 786a7291bf38e8f85c2a75bb23ddb038e83e350a Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 28 Oct 2020 12:26:37 -0400 Subject: [PATCH 07/12] remove unused EvalValidateCount --- terraform/eval_validate.go | 48 -------------------------------------- 1 file changed, 48 deletions(-) diff --git a/terraform/eval_validate.go b/terraform/eval_validate.go index c9f47b1b72..c5daf01efb 100644 --- a/terraform/eval_validate.go +++ b/terraform/eval_validate.go @@ -15,54 +15,6 @@ import ( "github.com/zclconf/go-cty/cty/gocty" ) -// EvalValidateCount is an EvalNode implementation that validates -// the count of a resource. -type EvalValidateCount struct { - Resource *configs.Resource -} - -// TODO: test -func (n *EvalValidateCount) Eval(ctx EvalContext) (interface{}, error) { - var diags tfdiags.Diagnostics - var count int - var err error - - val, valDiags := ctx.EvaluateExpr(n.Resource.Count, cty.Number, nil) - diags = diags.Append(valDiags) - if valDiags.HasErrors() { - goto RETURN - } - if val.IsNull() || !val.IsKnown() { - goto RETURN - } - - err = gocty.FromCtyValue(val, &count) - if err != nil { - // The EvaluateExpr call above already guaranteed us a number value, - // so if we end up here then we have something that is out of range - // for an int, and the error message will include a description of - // the valid range. - rawVal := val.AsBigFloat() - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid count value", - Detail: fmt.Sprintf("The number %s is not a valid count value: %s.", rawVal, err), - Subject: n.Resource.Count.Range().Ptr(), - }) - } else if count < 0 { - rawVal := val.AsBigFloat() - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid count value", - Detail: fmt.Sprintf("The number %s is not a valid count value: count must not be negative.", rawVal), - Subject: n.Resource.Count.Range().Ptr(), - }) - } - -RETURN: - return nil, diags.NonFatalErr() -} - // EvalValidateProvisioner validates the configuration of a provisioner // belonging to a resource. The provisioner config is expected to contain the // merged connection configurations. From 77cabe187dec2781b383881b9dd0b4a80db8cc73 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 28 Oct 2020 12:32:49 -0400 Subject: [PATCH 08/12] last Evals without diagnostics --- terraform/eval_validate_selfref.go | 10 ++++++---- terraform/eval_validate_selfref_test.go | 7 +------ terraform/node_resource_plan_instance.go | 12 ++++++------ 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/terraform/eval_validate_selfref.go b/terraform/eval_validate_selfref.go index dd5e4018d0..22ba8a889d 100644 --- a/terraform/eval_validate_selfref.go +++ b/terraform/eval_validate_selfref.go @@ -20,7 +20,7 @@ type EvalValidateSelfRef struct { ProviderSchema **ProviderSchema } -func (n *EvalValidateSelfRef) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalValidateSelfRef) Eval(ctx EvalContext) tfdiags.Diagnostics { var diags tfdiags.Diagnostics addr := n.Addr @@ -33,7 +33,8 @@ func (n *EvalValidateSelfRef) Eval(ctx EvalContext) (interface{}, error) { } if n.ProviderSchema == nil || *n.ProviderSchema == nil { - return nil, fmt.Errorf("provider schema unavailable while validating %s for self-references; this is a bug in Terraform and should be reported", addr) + diags = diags.Append(fmt.Errorf("provider schema unavailable while validating %s for self-references; this is a bug in Terraform and should be reported", addr)) + return diags } providerSchema := *n.ProviderSchema @@ -46,7 +47,8 @@ func (n *EvalValidateSelfRef) Eval(ctx EvalContext) (interface{}, error) { } if schema == nil { - return nil, fmt.Errorf("no schema available for %s to validate for self-references; this is a bug in Terraform and should be reported", addr) + diags = diags.Append(fmt.Errorf("no schema available for %s to validate for self-references; this is a bug in Terraform and should be reported", addr)) + return diags } refs, _ := lang.ReferencesInBlock(n.Config, schema) @@ -63,5 +65,5 @@ func (n *EvalValidateSelfRef) Eval(ctx EvalContext) (interface{}, error) { } } - return nil, diags.NonFatalErr() + return diags } diff --git a/terraform/eval_validate_selfref_test.go b/terraform/eval_validate_selfref_test.go index de3b9a34b2..0e0fa188c7 100644 --- a/terraform/eval_validate_selfref_test.go +++ b/terraform/eval_validate_selfref_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/hashicorp/terraform/configs/configschema" - "github.com/hashicorp/terraform/tfdiags" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hcltest" @@ -98,11 +97,7 @@ func TestEvalValidateSelfRef(t *testing.T) { Config: body, ProviderSchema: &ps, } - result, err := n.Eval(nil) - if result != nil { - t.Fatal("result should always be nil") - } - diags := tfdiags.Diagnostics(nil).Append(err) + diags := n.Eval(nil) if diags.HasErrors() != test.Err { if test.Err { t.Errorf("unexpected success; want error") diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index 935f7caa01..ec653ff513 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -68,9 +68,9 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) err Config: config.Config, ProviderSchema: &providerSchema, } - _, err = validateSelfRef.Eval(ctx) - if err != nil { - return err + diags = validateSelfRef.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } readDataPlan := &evalReadDataPlan{ @@ -144,9 +144,9 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) Config: config.Config, ProviderSchema: &providerSchema, } - _, err = validateSelfRef.Eval(ctx) - if err != nil { - return err + diags = validateSelfRef.Eval(ctx) + if diags.HasErrors() { + return diags.ErrWithWarnings() } instanceRefreshState, err = n.ReadResourceInstanceState(ctx, addr) From 988059d5331e50851fbff77564bef0071ace972b Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 28 Oct 2020 13:47:04 -0400 Subject: [PATCH 09/12] make GraphNodeExecutable return diagnostics --- terraform/eval_variable.go | 6 +- terraform/execute.go | 4 +- terraform/graph_walk_context.go | 18 +-- terraform/node_count_boundary.go | 10 +- terraform/node_count_boundary_test.go | 6 +- terraform/node_data_destroy.go | 8 +- terraform/node_data_destroy_test.go | 6 +- terraform/node_local.go | 14 +-- terraform/node_module_expand.go | 28 ++--- terraform/node_module_expand_test.go | 18 +-- terraform/node_module_variable.go | 13 +- terraform/node_output.go | 28 +++-- terraform/node_output_test.go | 18 +-- terraform/node_provider.go | 40 +++--- terraform/node_provider_eval.go | 8 +- terraform/node_provider_test.go | 6 +- terraform/node_provisioner.go | 5 +- terraform/node_resource_abstract.go | 9 +- terraform/node_resource_apply.go | 6 +- terraform/node_resource_apply_instance.go | 130 ++++++++++---------- terraform/node_resource_apply_test.go | 12 +- terraform/node_resource_destroy.go | 61 +++++---- terraform/node_resource_destroy_deposed.go | 65 +++++----- terraform/node_resource_plan.go | 5 +- terraform/node_resource_plan_destroy.go | 27 ++-- terraform/node_resource_plan_instance.go | 82 ++++++------ terraform/node_resource_plan_orphan.go | 49 ++++---- terraform/node_resource_plan_orphan_test.go | 6 +- terraform/node_resource_plan_test.go | 12 +- terraform/node_resource_validate.go | 28 +++-- terraform/node_root_variable.go | 3 +- terraform/node_root_variable_test.go | 6 +- terraform/transform_import_state.go | 43 +++---- terraform/transform_import_state_test.go | 12 +- terraform/transform_provider.go | 5 +- terraform/transform_provisioner.go | 7 +- 36 files changed, 415 insertions(+), 389 deletions(-) diff --git a/terraform/eval_variable.go b/terraform/eval_variable.go index 72734b7881..cd91fcc7c8 100644 --- a/terraform/eval_variable.go +++ b/terraform/eval_variable.go @@ -18,14 +18,12 @@ import ( // This must be used only after any side-effects that make the value of the // variable available for use in expression evaluation, such as // EvalModuleCallArgument for variables in descendent modules. -func evalVariableValidations(addr addrs.AbsInputVariableInstance, config *configs.Variable, expr hcl.Expression, ctx EvalContext) error { +func evalVariableValidations(addr addrs.AbsInputVariableInstance, config *configs.Variable, expr hcl.Expression, ctx EvalContext) (diags tfdiags.Diagnostics) { if config == nil || len(config.Validations) == 0 { log.Printf("[TRACE] evalVariableValidations: not active for %s, so skipping", addr) return nil } - var diags tfdiags.Diagnostics - // Variable nodes evaluate in the parent module to where they were declared // because the value expression (n.Expr, if set) comes from the calling // "module" block in the parent module. @@ -105,5 +103,5 @@ func evalVariableValidations(addr addrs.AbsInputVariableInstance, config *config } } - return diags.ErrWithWarnings() + return diags } diff --git a/terraform/execute.go b/terraform/execute.go index 5bf06c4d04..a31a504691 100644 --- a/terraform/execute.go +++ b/terraform/execute.go @@ -1,9 +1,11 @@ package terraform +import "github.com/hashicorp/terraform/tfdiags" + // GraphNodeExecutable is the interface that graph nodes must implement to // enable execution. This is an alternative to GraphNodeEvalable, which is in // the process of being removed. A given graph node should _not_ implement both // GraphNodeExecutable and GraphNodeEvalable. type GraphNodeExecutable interface { - Execute(EvalContext, walkOperation) error + Execute(EvalContext, walkOperation) tfdiags.Diagnostics } diff --git a/terraform/graph_walk_context.go b/terraform/graph_walk_context.go index 1c8273e977..907842f252 100644 --- a/terraform/graph_walk_context.go +++ b/terraform/graph_walk_context.go @@ -125,29 +125,20 @@ func (w *ContextGraphWalker) Execute(ctx EvalContext, n GraphNodeExecutable) tfd // Acquire a lock on the semaphore w.Context.parallelSem.Acquire() - err := n.Execute(ctx, w.Operation) + diags := n.Execute(ctx, w.Operation) // Release the semaphore w.Context.parallelSem.Release() - if err == nil { - return nil + if !diags.HasErrors() { + return diags } // Acquire the lock because anything is going to require a lock. w.errorLock.Lock() defer w.errorLock.Unlock() - // If the error is non-fatal then we'll accumulate its diagnostics in our - // non-fatal list, rather than returning it directly, so that the graph - // walk can continue. - if nferr, ok := err.(tfdiags.NonFatalError); ok { - w.NonFatalDiagnostics = w.NonFatalDiagnostics.Append(nferr.Diagnostics) - return nil - } - - var diags tfdiags.Diagnostics - + err := diags.Err() // Handle a simple early exit error if errors.Is(err, EvalEarlyExitError{}) { return nil @@ -174,6 +165,5 @@ func (w *ContextGraphWalker) Execute(ctx EvalContext, n GraphNodeExecutable) tfd // unpack this as one or more diagnostic messages and return that. If we // get down here then the returned diagnostics will contain at least one // error, causing the graph walk to halt. - diags = diags.Append(err) return diags } diff --git a/terraform/node_count_boundary.go b/terraform/node_count_boundary.go index bfdbd1efa5..2e972ff216 100644 --- a/terraform/node_count_boundary.go +++ b/terraform/node_count_boundary.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" + "github.com/hashicorp/terraform/tfdiags" ) // NodeCountBoundary fixes up any transitions between "each modes" in objects @@ -14,12 +15,14 @@ type NodeCountBoundary struct { Config *configs.Config } +var _ GraphNodeExecutable = (*NodeCountBoundary)(nil) + func (n *NodeCountBoundary) Name() string { return "meta.count-boundary (EachMode fixup)" } // GraphNodeExecutable -func (n *NodeCountBoundary) Execute(ctx EvalContext, op walkOperation) error { +func (n *NodeCountBoundary) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { // We'll temporarily lock the state to grab the modules, then work on each // one separately while taking a lock again for each separate resource. // This means that if another caller concurrently adds a module here while @@ -42,10 +45,11 @@ func (n *NodeCountBoundary) Execute(ctx EvalContext, op walkOperation) error { continue } if err := n.fixModule(ctx, addr); err != nil { - return err + diags = diags.Append(err) + return diags } } - return nil + return diags } func (n *NodeCountBoundary) fixModule(ctx EvalContext, moduleAddr addrs.ModuleInstance) error { diff --git a/terraform/node_count_boundary_test.go b/terraform/node_count_boundary_test.go index 13c5fac295..497d4fc96d 100644 --- a/terraform/node_count_boundary_test.go +++ b/terraform/node_count_boundary_test.go @@ -53,9 +53,9 @@ func TestNodeCountBoundaryExecute(t *testing.T) { } node := NodeCountBoundary{Config: config} - err := node.Execute(ctx, walkApply) - if err != nil { - t.Fatalf("unexpected error: %s", err.Error()) + diags := node.Execute(ctx, walkApply) + if diags.HasErrors() { + t.Fatalf("unexpected error: %s", diags.Err()) } if !state.HasResources() { t.Fatal("resources missing from state") diff --git a/terraform/node_data_destroy.go b/terraform/node_data_destroy.go index 7313719209..14c06516b4 100644 --- a/terraform/node_data_destroy.go +++ b/terraform/node_data_destroy.go @@ -1,6 +1,10 @@ package terraform -import "log" +import ( + "log" + + "github.com/hashicorp/terraform/tfdiags" +) // NodeDestroyableDataResourceInstance represents a resource that is "destroyable": // it is ready to be destroyed. @@ -13,7 +17,7 @@ var ( ) // GraphNodeExecutable -func (n *NodeDestroyableDataResourceInstance) Execute(ctx EvalContext, op walkOperation) error { +func (n *NodeDestroyableDataResourceInstance) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics { log.Printf("[TRACE] NodeDestroyableDataResourceInstance: removing state object for %s", n.Addr) ctx.State().SetResourceInstanceCurrent(n.Addr, nil, n.ResolvedProvider) return nil diff --git a/terraform/node_data_destroy_test.go b/terraform/node_data_destroy_test.go index 24fb988dd9..32d07b1431 100644 --- a/terraform/node_data_destroy_test.go +++ b/terraform/node_data_destroy_test.go @@ -36,9 +36,9 @@ func TestNodeDataDestroyExecute(t *testing.T) { }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), }} - err := node.Execute(ctx, walkApply) - if err != nil { - t.Fatalf("unexpected error: %s", err.Error()) + diags := node.Execute(ctx, walkApply) + if diags.HasErrors() { + t.Fatalf("unexpected error: %v", diags.Err()) } // verify resource removed from state diff --git a/terraform/node_local.go b/terraform/node_local.go index 055492fe83..eb04b39549 100644 --- a/terraform/node_local.go +++ b/terraform/node_local.go @@ -128,10 +128,7 @@ func (n *NodeLocal) References() []*addrs.Reference { // NodeLocal.Execute is an Execute implementation that evaluates the // expression for a local value and writes it into a transient part of // the state. -func (n *NodeLocal) Execute(ctx EvalContext, op walkOperation) error { - - var diags tfdiags.Diagnostics - +func (n *NodeLocal) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { expr := n.Config.Expr addr := n.Addr.LocalValue @@ -150,23 +147,24 @@ func (n *NodeLocal) Execute(ctx EvalContext, op walkOperation) error { } } if diags.HasErrors() { - return diags.Err() + return diags } val, moreDiags := ctx.EvaluateExpr(expr, cty.DynamicPseudoType, nil) diags = diags.Append(moreDiags) if moreDiags.HasErrors() { - return diags.Err() + return diags } state := ctx.State() if state == nil { - return fmt.Errorf("cannot write local value to nil state") + diags = diags.Append(fmt.Errorf("cannot write local value to nil state")) + return diags } state.SetLocalValue(addr.Absolute(ctx.Path()), val) - return nil + return diags } // dag.GraphNodeDotter impl. diff --git a/terraform/node_module_expand.go b/terraform/node_module_expand.go index 1e27fd274d..6f41232dee 100644 --- a/terraform/node_module_expand.go +++ b/terraform/node_module_expand.go @@ -99,7 +99,7 @@ func (n *nodeExpandModule) ReferenceOutside() (selfPath, referencePath addrs.Mod } // GraphNodeExecutable -func (n *nodeExpandModule) Execute(ctx EvalContext, op walkOperation) error { +func (n *nodeExpandModule) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { expander := ctx.InstanceExpander() _, call := n.Addr.Call() @@ -110,16 +110,18 @@ func (n *nodeExpandModule) Execute(ctx EvalContext, op walkOperation) error { ctx = ctx.WithPath(module) switch { case n.ModuleCall.Count != nil: - count, diags := evaluateCountExpression(n.ModuleCall.Count, ctx) + count, ctDiags := evaluateCountExpression(n.ModuleCall.Count, ctx) + diags = diags.Append(ctDiags) if diags.HasErrors() { - return diags.Err() + return diags } expander.SetModuleCount(module, call, count) case n.ModuleCall.ForEach != nil: - forEach, diags := evaluateForEachExpression(n.ModuleCall.ForEach, ctx) + forEach, feDiags := evaluateForEachExpression(n.ModuleCall.ForEach, ctx) + diags = diags.Append(feDiags) if diags.HasErrors() { - return diags.Err() + return diags } expander.SetModuleForEach(module, call, forEach) @@ -128,7 +130,7 @@ func (n *nodeExpandModule) Execute(ctx EvalContext, op walkOperation) error { } } - return nil + return diags } @@ -146,6 +148,7 @@ type nodeCloseModule struct { var ( _ GraphNodeReferenceable = (*nodeCloseModule)(nil) _ GraphNodeReferenceOutside = (*nodeCloseModule)(nil) + _ GraphNodeExecutable = (*nodeCloseModule)(nil) ) func (n *nodeCloseModule) ModulePath() addrs.Module { @@ -170,7 +173,7 @@ func (n *nodeCloseModule) Name() string { return n.Addr.String() + " (close)" } -func (n *nodeCloseModule) Execute(ctx EvalContext, op walkOperation) error { +func (n *nodeCloseModule) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { switch op { case walkApply, walkDestroy: state := ctx.State().Lock() @@ -206,10 +209,11 @@ type nodeValidateModule struct { nodeExpandModule } +var _ GraphNodeExecutable = (*nodeValidateModule)(nil) + // GraphNodeEvalable -func (n *nodeValidateModule) Execute(ctx EvalContext, op walkOperation) error { +func (n *nodeValidateModule) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { _, call := n.Addr.Call() - var diags tfdiags.Diagnostics expander := ctx.InstanceExpander() // Modules all evaluate to single instances during validation, only to @@ -238,9 +242,5 @@ func (n *nodeValidateModule) Execute(ctx EvalContext, op walkOperation) error { expander.SetModuleSingle(module, call) } - if diags.HasErrors() { - return diags.ErrWithWarnings() - } - - return nil + return diags } diff --git a/terraform/node_module_expand_test.go b/terraform/node_module_expand_test.go index 578b4276f7..c83d169346 100644 --- a/terraform/node_module_expand_test.go +++ b/terraform/node_module_expand_test.go @@ -42,9 +42,9 @@ func TestNodeCloseModuleExecute(t *testing.T) { StateState: state.SyncWrapper(), } node := nodeCloseModule{addrs.Module{"child"}} - err := node.Execute(ctx, walkApply) - if err != nil { - t.Fatalf("unexpected error: %s", err.Error()) + diags := node.Execute(ctx, walkApply) + if diags.HasErrors() { + t.Fatalf("unexpected error: %s", diags.Err()) } // Since module.child has no resources, it should be removed @@ -62,9 +62,9 @@ func TestNodeCloseModuleExecute(t *testing.T) { } node := nodeCloseModule{addrs.Module{"child"}} - err := node.Execute(ctx, walkImport) - if err != nil { - t.Fatalf("unexpected error: %s", err.Error()) + diags := node.Execute(ctx, walkImport) + if diags.HasErrors() { + t.Fatalf("unexpected error: %s", diags.Err()) } if _, ok := state.Modules["module.child"]; !ok { t.Fatal("module.child was removed from state, expected no-op") @@ -87,9 +87,9 @@ func TestNodeValidateModuleExecute(t *testing.T) { }, } - err := node.Execute(ctx, walkApply) - if err != nil { - t.Fatalf("unexpected error: %s", err.Error()) + diags := node.Execute(ctx, walkApply) + if diags.HasErrors() { + t.Fatalf("unexpected error: %v", diags.Err()) } }) diff --git a/terraform/node_module_variable.go b/terraform/node_module_variable.go index 78ddb6e8f2..b20f2a72ca 100644 --- a/terraform/node_module_variable.go +++ b/terraform/node_module_variable.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform/dag" "github.com/hashicorp/terraform/instances" "github.com/hashicorp/terraform/lang" + "github.com/hashicorp/terraform/tfdiags" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/convert" ) @@ -141,7 +142,7 @@ func (n *nodeModuleVariable) ModulePath() addrs.Module { } // GraphNodeExecutable -func (n *nodeModuleVariable) Execute(ctx EvalContext, op walkOperation) error { +func (n *nodeModuleVariable) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { // If we have no value, do nothing if n.Expr == nil { return nil @@ -155,13 +156,15 @@ func (n *nodeModuleVariable) Execute(ctx EvalContext, op walkOperation) error { switch op { case walkValidate: vals, err = n.EvalModuleCallArgument(ctx, true) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } default: vals, err = n.EvalModuleCallArgument(ctx, false) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } } diff --git a/terraform/node_output.go b/terraform/node_output.go index 00f39c80e3..a2f11d3e9b 100644 --- a/terraform/node_output.go +++ b/terraform/node_output.go @@ -246,11 +246,10 @@ func (n *NodeApplyableOutput) References() []*addrs.Reference { } // GraphNodeExecutable -func (n *NodeApplyableOutput) Execute(ctx EvalContext, op walkOperation) error { - var diags tfdiags.Diagnostics +func (n *NodeApplyableOutput) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { state := ctx.State() if state == nil { - return nil + return } changes := ctx.Changes() // may be nil, if we're not working on a changeset @@ -297,9 +296,24 @@ func (n *NodeApplyableOutput) Execute(ctx EvalContext, op walkOperation) error { // marked as unknown. If the evaluator was able to find a type // for the value in spite of the error then we'll use it. n.setValue(state, changes, cty.UnknownVal(val.Type())) - return EvalEarlyExitError{} + + // Keep existing warnings, while converting errors to warnings. + // This is not meant to be the normal path, so there no need to + // make the errors pretty. + var warnings tfdiags.Diagnostics + for _, d := range diags { + switch d.Severity() { + case tfdiags.Warning: + warnings = warnings.Append(d) + case tfdiags.Error: + desc := d.Description() + warnings = warnings.Append(tfdiags.SimpleWarning(fmt.Sprintf("%s:%s", desc.Summary, desc.Detail))) + } + } + + return warnings } - return diags.Err() + return diags } n.setValue(state, changes, val) @@ -309,7 +323,7 @@ func (n *NodeApplyableOutput) Execute(ctx EvalContext, op walkOperation) error { n.setValue(state, changes, val) } - return nil + return diags } // dag.GraphNodeDotter impl. @@ -350,7 +364,7 @@ func (n *NodeDestroyableOutput) temporaryValue() bool { } // GraphNodeExecutable -func (n *NodeDestroyableOutput) Execute(ctx EvalContext, op walkOperation) error { +func (n *NodeDestroyableOutput) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics { state := ctx.State() if state == nil { return nil diff --git a/terraform/node_output_test.go b/terraform/node_output_test.go index 63fd35214e..3b518f0461 100644 --- a/terraform/node_output_test.go +++ b/terraform/node_output_test.go @@ -81,11 +81,11 @@ func TestNodeApplyableOutputExecute_invalidDependsOn(t *testing.T) { }) ctx.EvaluateExprResult = val - err := node.Execute(ctx, walkApply) - if err == nil { + diags := node.Execute(ctx, walkApply) + if !diags.HasErrors() { t.Fatal("expected execute error, but there was none") } - if got, want := err.Error(), "Invalid depends_on reference"; !strings.Contains(got, want) { + if got, want := diags.Err().Error(), "Invalid depends_on reference"; !strings.Contains(got, want) { t.Errorf("expected error to include %q, but was: %s", want, got) } } @@ -102,11 +102,11 @@ func TestNodeApplyableOutputExecute_sensitiveValueNotOutput(t *testing.T) { }) ctx.EvaluateExprResult = val - err := node.Execute(ctx, walkApply) - if err == nil { + diags := node.Execute(ctx, walkApply) + if !diags.HasErrors() { t.Fatal("expected execute error, but there was none") } - if got, want := err.Error(), "Output refers to sensitive values"; !strings.Contains(got, want) { + if got, want := diags.Err().Error(), "Output refers to sensitive values"; !strings.Contains(got, want) { t.Errorf("expected error to include %q, but was: %s", want, got) } } @@ -151,9 +151,9 @@ func TestNodeDestroyableOutputExecute(t *testing.T) { } node := NodeDestroyableOutput{Addr: outputAddr} - err := node.Execute(ctx, walkApply) - if err != nil { - t.Fatalf("Unexpected error: %s", err.Error()) + diags := node.Execute(ctx, walkApply) + if diags.HasErrors() { + t.Fatalf("Unexpected error: %s", diags.Err()) } if state.OutputValue(outputAddr) != nil { t.Fatal("Unexpected outputs in state after removal") diff --git a/terraform/node_provider.go b/terraform/node_provider.go index c901eb417c..3ea0485d4c 100644 --- a/terraform/node_provider.go +++ b/terraform/node_provider.go @@ -20,36 +20,37 @@ var ( ) // GraphNodeExecutable -func (n *NodeApplyableProvider) Execute(ctx EvalContext, op walkOperation) error { +func (n *NodeApplyableProvider) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { _, err := ctx.InitProvider(n.Addr) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } provider, _, err := GetProvider(ctx, n.Addr) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } switch op { case walkValidate: - return n.ValidateProvider(ctx, provider) + return diags.Append(n.ValidateProvider(ctx, provider)) case walkPlan, walkApply, walkDestroy: - return n.ConfigureProvider(ctx, provider, false) + return diags.Append(n.ConfigureProvider(ctx, provider, false)) case walkImport: - return n.ConfigureProvider(ctx, provider, true) + return diags.Append(n.ConfigureProvider(ctx, provider, true)) } - return nil + return diags } -func (n *NodeApplyableProvider) ValidateProvider(ctx EvalContext, provider providers.Interface) error { - var diags tfdiags.Diagnostics +func (n *NodeApplyableProvider) ValidateProvider(ctx EvalContext, provider providers.Interface) (diags tfdiags.Diagnostics) { configBody := buildProviderConfig(ctx, n.Addr, n.ProviderConfig()) resp := provider.GetSchema() diags = diags.Append(resp.Diagnostics) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } configSchema := resp.Provider.Block @@ -63,7 +64,7 @@ func (n *NodeApplyableProvider) ValidateProvider(ctx EvalContext, provider provi configVal, configBody, evalDiags := ctx.EvaluateBlock(configBody, configSchema, nil, EvalDataForNoInstanceKey) diags = diags.Append(evalDiags) if evalDiags.HasErrors() { - return diags.ErrWithWarnings() + return diags } req := providers.PrepareProviderConfigRequest{ @@ -73,14 +74,13 @@ func (n *NodeApplyableProvider) ValidateProvider(ctx EvalContext, provider provi validateResp := provider.PrepareProviderConfig(req) diags = diags.Append(validateResp.Diagnostics) - return diags.ErrWithWarnings() + return diags } // ConfigureProvider configures a provider that is already initialized and retrieved. // If verifyConfigIsKnown is true, ConfigureProvider will return an error if the // provider configVal is not wholly known and is meant only for use during import. -func (n *NodeApplyableProvider) ConfigureProvider(ctx EvalContext, provider providers.Interface, verifyConfigIsKnown bool) error { - var diags tfdiags.Diagnostics +func (n *NodeApplyableProvider) ConfigureProvider(ctx EvalContext, provider providers.Interface, verifyConfigIsKnown bool) (diags tfdiags.Diagnostics) { config := n.ProviderConfig() configBody := buildProviderConfig(ctx, n.Addr, config) @@ -88,14 +88,14 @@ func (n *NodeApplyableProvider) ConfigureProvider(ctx EvalContext, provider prov resp := provider.GetSchema() diags = diags.Append(resp.Diagnostics) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } configSchema := resp.Provider.Block configVal, configBody, evalDiags := ctx.EvaluateBlock(configBody, configSchema, nil, EvalDataForNoInstanceKey) diags = diags.Append(evalDiags) if evalDiags.HasErrors() { - return diags.ErrWithWarnings() + return diags } if verifyConfigIsKnown && !configVal.IsWhollyKnown() { @@ -105,11 +105,11 @@ func (n *NodeApplyableProvider) ConfigureProvider(ctx EvalContext, provider prov Detail: fmt.Sprintf("The configuration for %s depends on values that cannot be determined until apply.", n.Addr), Subject: &config.DeclRange, }) - return diags.ErrWithWarnings() + return diags } configDiags := ctx.ConfigureProvider(n.Addr, configVal) configDiags = configDiags.InConfigBody(configBody) - return configDiags.ErrWithWarnings() + return configDiags } diff --git a/terraform/node_provider_eval.go b/terraform/node_provider_eval.go index aa6ba2c585..a89583cff5 100644 --- a/terraform/node_provider_eval.go +++ b/terraform/node_provider_eval.go @@ -1,5 +1,7 @@ package terraform +import "github.com/hashicorp/terraform/tfdiags" + // NodeEvalableProvider represents a provider during an "eval" walk. // This special provider node type just initializes a provider and // fetches its schema, without configuring it or otherwise interacting @@ -8,8 +10,10 @@ type NodeEvalableProvider struct { *NodeAbstractProvider } +var _ GraphNodeExecutable = (*NodeEvalableProvider)(nil) + // GraphNodeExecutable -func (n *NodeEvalableProvider) Execute(ctx EvalContext, op walkOperation) error { +func (n *NodeEvalableProvider) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { _, err := ctx.InitProvider(n.Addr) - return err + return diags.Append(err) } diff --git a/terraform/node_provider_test.go b/terraform/node_provider_test.go index 5c418bde90..9ce13b9df8 100644 --- a/terraform/node_provider_test.go +++ b/terraform/node_provider_test.go @@ -65,13 +65,13 @@ func TestNodeApplyableProviderExecute_unknownImport(t *testing.T) { ctx := &MockEvalContext{ProviderProvider: provider} ctx.installSimpleEval() - err := n.Execute(ctx, walkImport) - if err == nil { + diags := n.Execute(ctx, walkImport) + if !diags.HasErrors() { t.Fatal("expected error, got success") } detail := `Invalid provider configuration: The configuration for provider["registry.terraform.io/hashicorp/foo"] depends on values that cannot be determined until apply.` - if got, want := err.Error(), detail; got != want { + if got, want := diags.Err().Error(), detail; got != want { t.Errorf("wrong diagnostic detail\n got: %q\nwant: %q", got, want) } diff --git a/terraform/node_provisioner.go b/terraform/node_provisioner.go index c4df9c6271..eaa6fc815b 100644 --- a/terraform/node_provisioner.go +++ b/terraform/node_provisioner.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/tfdiags" ) // NodeProvisioner represents a provider that has no associated operations. @@ -39,6 +40,6 @@ func (n *NodeProvisioner) ProvisionerName() string { } // GraphNodeExecutable impl. -func (n *NodeProvisioner) Execute(ctx EvalContext, op walkOperation) error { - return ctx.InitProvisioner(n.NameValue) +func (n *NodeProvisioner) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { + return diags.Append(ctx.InitProvisioner(n.NameValue)) } diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index 4606d5185d..fa9f501ef7 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -305,8 +305,7 @@ func (n *NodeAbstractResource) DotNode(name string, opts *dag.DotOpts) *dag.DotN // eval is the only change we get to set the resource "each mode" to list // in that case, allowing expression evaluation to see it as a zero-element list // rather than as not set at all. -func (n *NodeAbstractResource) writeResourceState(ctx EvalContext, addr addrs.AbsResource) error { - var diags tfdiags.Diagnostics +func (n *NodeAbstractResource) writeResourceState(ctx EvalContext, addr addrs.AbsResource) (diags tfdiags.Diagnostics) { state := ctx.State() // We'll record our expansion decision in the shared "expander" object @@ -320,7 +319,7 @@ func (n *NodeAbstractResource) writeResourceState(ctx EvalContext, addr addrs.Ab count, countDiags := evaluateCountExpression(n.Config.Count, ctx) diags = diags.Append(countDiags) if countDiags.HasErrors() { - return diags.Err() + return diags } state.SetResourceProvider(addr, n.ResolvedProvider) @@ -330,7 +329,7 @@ func (n *NodeAbstractResource) writeResourceState(ctx EvalContext, addr addrs.Ab forEach, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx) diags = diags.Append(forEachDiags) if forEachDiags.HasErrors() { - return diags.Err() + return diags } // This method takes care of all of the business logic of updating this @@ -343,7 +342,7 @@ func (n *NodeAbstractResource) writeResourceState(ctx EvalContext, addr addrs.Ab expander.SetResourceSingle(addr.Module, n.Addr.Resource) } - return nil + return diags } // ReadResourceInstanceState reads the current object for a specific instance in diff --git a/terraform/node_resource_apply.go b/terraform/node_resource_apply.go index ff09ba6145..02e71554a7 100644 --- a/terraform/node_resource_apply.go +++ b/terraform/node_resource_apply.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/dag" "github.com/hashicorp/terraform/lang" + "github.com/hashicorp/terraform/tfdiags" ) // nodeExpandApplyableResource handles the first layer of resource @@ -102,13 +103,12 @@ func (n *NodeApplyableResource) References() []*addrs.Reference { } // GraphNodeExecutable -func (n *NodeApplyableResource) Execute(ctx EvalContext, op walkOperation) error { +func (n *NodeApplyableResource) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics { if n.Config == nil { // Nothing to do, then. log.Printf("[TRACE] NodeApplyableResource: no configuration present for %s", n.Name()) return nil } - err := n.writeResourceState(ctx, n.Addr) - return err + return n.writeResourceState(ctx, n.Addr) } diff --git a/terraform/node_resource_apply_instance.go b/terraform/node_resource_apply_instance.go index 0d902b837f..90085997b5 100644 --- a/terraform/node_resource_apply_instance.go +++ b/terraform/node_resource_apply_instance.go @@ -101,7 +101,7 @@ func (n *NodeApplyableResourceInstance) AttachDependencies(deps []addrs.ConfigRe } // GraphNodeExecutable -func (n *NodeApplyableResourceInstance) Execute(ctx EvalContext, op walkOperation) error { +func (n *NodeApplyableResourceInstance) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { addr := n.ResourceInstanceAddr() if n.Config == nil { @@ -110,7 +110,6 @@ func (n *NodeApplyableResourceInstance) Execute(ctx EvalContext, op walkOperatio // https://github.com/hashicorp/terraform/issues/21258 // To avoid an outright crash here, we'll instead return an explicit // error. - var diags tfdiags.Diagnostics diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, "Resource node has no configuration attached", @@ -119,7 +118,7 @@ func (n *NodeApplyableResourceInstance) Execute(ctx EvalContext, op walkOperatio addr, ), )) - return diags.Err() + return diags } // Eval info is different depending on what kind of resource this is @@ -133,22 +132,24 @@ func (n *NodeApplyableResourceInstance) Execute(ctx EvalContext, op walkOperatio } } -func (n *NodeApplyableResourceInstance) dataResourceExecute(ctx EvalContext) error { - var diags tfdiags.Diagnostics +func (n *NodeApplyableResourceInstance) dataResourceExecute(ctx EvalContext) (diags tfdiags.Diagnostics) { addr := n.ResourceInstanceAddr().Resource provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } change, err := n.readDiff(ctx, providerSchema) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } // Stop early if we don't actually have a diff if change == nil { - return EvalEarlyExitError{} + diags = diags.Append(EvalEarlyExitError{}) + return diags } // In this particular call to EvalReadData we include our planned @@ -167,9 +168,9 @@ func (n *NodeApplyableResourceInstance) dataResourceExecute(ctx EvalContext) err State: &state, }, } - diags = readDataApply.Eval(ctx) + diags = diags.Append(readDataApply.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } writeState := &EvalWriteState{ @@ -178,9 +179,9 @@ func (n *NodeApplyableResourceInstance) dataResourceExecute(ctx EvalContext) err ProviderSchema: &providerSchema, State: &state, } - diags = writeState.Eval(ctx) + diags = diags.Append(writeState.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } writeDiff := &EvalWriteDiff{ @@ -188,18 +189,16 @@ func (n *NodeApplyableResourceInstance) dataResourceExecute(ctx EvalContext) err ProviderSchema: &providerSchema, Change: nil, } - diags = writeDiff.Eval(ctx) + diags = diags.Append(writeDiff.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } - UpdateStateHook(ctx) - return nil + diags = diags.Append(UpdateStateHook(ctx)) + return diags } -func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) error { - var diags tfdiags.Diagnostics - +func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) (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 @@ -209,20 +208,23 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) addr := n.ResourceInstanceAddr().Resource provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } // Get the saved diff for apply diffApply, err := n.readDiff(ctx, providerSchema) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } // We don't want to do any destroys // (these are handled by NodeDestroyResourceInstance instead) if diffApply == nil || diffApply.Action == plans.Delete { - return EvalEarlyExitError{} + diags = diags.Append(EvalEarlyExitError{}) + return diags } destroy := (diffApply.Action == plans.Delete || diffApply.Action.IsReplace()) @@ -239,9 +241,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) ForceKey: n.PreallocatedDeposedKey, OutputKey: &deposedKey, } - diags = deposeState.Eval(ctx) + diags = diags.Append(deposeState.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } } @@ -252,15 +254,16 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) Output: &state, } - diags = readState.Eval(ctx) + diags = diags.Append(readState.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } // Get the saved diff diff, err := n.readDiff(ctx, providerSchema) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } // Make a new diff, in case we've learned new values in the state @@ -277,9 +280,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) OutputChange: &diffApply, OutputState: &state, } - diags = evalDiff.Eval(ctx) + diags = diags.Append(evalDiff.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } // Compare the diffs @@ -290,9 +293,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) Planned: &diff, Actual: &diffApply, } - diags = checkPlannedChange.Eval(ctx) + diags = diags.Append(checkPlannedChange.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } readState = &EvalReadState{ @@ -302,9 +305,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) Output: &state, } - diags = readState.Eval(ctx) + diags = diags.Append(readState.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } reduceDiff := &EvalReduceDiff{ @@ -313,16 +316,17 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) Destroy: false, OutChange: &diffApply, } - diags = reduceDiff.Eval(ctx) + diags = diags.Append(reduceDiff.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } // EvalReduceDiff may have simplified our planned change // into a NoOp if it only requires destroying, since destroying // is handled by NodeDestroyResourceInstance. if diffApply == nil || diffApply.Action == plans.NoOp { - return EvalEarlyExitError{} + diags = diags.Append(EvalEarlyExitError{}) + return diags } evalApplyPre := &EvalApplyPre{ @@ -330,9 +334,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) State: &state, Change: &diffApply, } - diags = evalApplyPre.Eval(ctx) + diags = diags.Append(evalApplyPre.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } var applyError error @@ -350,9 +354,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) CreateNew: &createNew, CreateBeforeDestroy: n.CreateBeforeDestroy(), } - diags = evalApply.Eval(ctx) + diags = diags.Append(evalApply.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } // We clear the change out here so that future nodes don't see a change @@ -362,9 +366,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) ProviderSchema: &providerSchema, Change: nil, } - diags = writeDiff.Eval(ctx) + diags = diags.Append(writeDiff.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } evalMaybeTainted := &EvalMaybeTainted{ @@ -373,9 +377,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) Change: &diffApply, Error: &applyError, } - diags = evalMaybeTainted.Eval(ctx) + diags = diags.Append(evalMaybeTainted.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } writeState := &EvalWriteState{ @@ -385,9 +389,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) State: &state, Dependencies: &n.Dependencies, } - diags = writeState.Eval(ctx) + diags = diags.Append(writeState.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } applyProvisioners := &EvalApplyProvisioners{ @@ -398,9 +402,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) Error: &applyError, When: configs.ProvisionerWhenCreate, } - diags = applyProvisioners.Eval(ctx) + diags = diags.Append(applyProvisioners.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } evalMaybeTainted = &EvalMaybeTainted{ @@ -409,9 +413,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) Change: &diffApply, Error: &applyError, } - diags = evalMaybeTainted.Eval(ctx) + diags = diags.Append(evalMaybeTainted.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } writeState = &EvalWriteState{ @@ -421,9 +425,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) State: &state, Dependencies: &n.Dependencies, } - diags = writeState.Eval(ctx) + diags = diags.Append(writeState.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } if createBeforeDestroyEnabled && applyError != nil { @@ -432,9 +436,9 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) PlannedChange: &diffApply, Key: &deposedKey, } - diags := maybeRestoreDesposedObject.Eval(ctx) + diags := diags.Append(maybeRestoreDesposedObject.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } } @@ -443,11 +447,11 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) State: &state, Error: &applyError, } - diags = applyPost.Eval(ctx) + diags = diags.Append(applyPost.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } - UpdateStateHook(ctx) - return nil + diags = diags.Append(UpdateStateHook(ctx)) + return diags } diff --git a/terraform/node_resource_apply_test.go b/terraform/node_resource_apply_test.go index a841a35630..54b65ec4f8 100644 --- a/terraform/node_resource_apply_test.go +++ b/terraform/node_resource_apply_test.go @@ -23,9 +23,9 @@ func TestNodeApplyableResourceExecute(t *testing.T) { }, Addr: mustAbsResourceAddr("test_instance.foo"), } - err := node.Execute(ctx, walkApply) - if err != nil { - t.Fatalf("unexpected error: %s", err.Error()) + diags := node.Execute(ctx, walkApply) + if diags.HasErrors() { + t.Fatalf("unexpected error: %s", diags.Err()) } if !state.Empty() { t.Fatalf("expected no state, got:\n %s", state.String()) @@ -48,9 +48,9 @@ func TestNodeApplyableResourceExecute(t *testing.T) { }, Addr: mustAbsResourceAddr("test_instance.foo"), } - err := node.Execute(ctx, walkApply) - if err != nil { - t.Fatalf("unexpected error: %s", err.Error()) + diags := node.Execute(ctx, walkApply) + if diags.HasErrors() { + t.Fatalf("unexpected error: %s", diags.Err()) } if state.Empty() { t.Fatal("expected resources in state, got empty state") diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 26dd56dee4..9e9bfe87db 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -123,9 +123,7 @@ func (n *NodeDestroyResourceInstance) References() []*addrs.Reference { } // GraphNodeExecutable -func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) error { - var diags tfdiags.Diagnostics - +func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { addr := n.ResourceInstanceAddr() // Get our state @@ -140,13 +138,15 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) var provisionerErr error provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } changeApply, err = n.readDiff(ctx, providerSchema) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } evalReduceDiff := &EvalReduceDiff{ @@ -155,25 +155,28 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) Destroy: true, OutChange: &changeApply, } - diags = evalReduceDiff.Eval(ctx) + diags = diags.Append(evalReduceDiff.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } // EvalReduceDiff may have simplified our planned change // into a NoOp if it does not require destroying. if changeApply == nil || changeApply.Action == plans.NoOp { - return EvalEarlyExitError{} + diags = diags.Append(EvalEarlyExitError{}) + return diags } state, err = n.ReadResourceInstanceState(ctx, addr) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } // Exit early if the state object is null after reading the state if state == nil || state.Value.IsNull() { - return EvalEarlyExitError{} + diags = diags.Append(EvalEarlyExitError{}) + return diags } evalApplyPre := &EvalApplyPre{ @@ -181,9 +184,9 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) State: &state, Change: &changeApply, } - diags = evalApplyPre.Eval(ctx) + diags = diags.Append(evalApplyPre.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } // Run destroy provisioners if not tainted @@ -195,9 +198,9 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) Error: &provisionerErr, When: configs.ProvisionerWhenDestroy, } - diags = evalApplyProvisioners.Eval(ctx) + diags = diags.Append(evalApplyProvisioners.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } if provisionerErr != nil { // If we have a provisioning error, then we just call @@ -207,9 +210,9 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) State: &state, Error: &provisionerErr, } - diags = evalApplyPost.Eval(ctx) + diags = diags.Append(evalApplyPost.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } } } @@ -229,9 +232,9 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) Output: &state, Error: &provisionerErr, } - diags = evalApply.Eval(ctx) + diags = diags.Append(evalApply.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } evalWriteState := &EvalWriteState{ @@ -240,9 +243,9 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) ProviderSchema: &providerSchema, State: &state, } - diags = evalWriteState.Eval(ctx) + diags = diags.Append(evalWriteState.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } } else { log.Printf("[TRACE] NodeDestroyResourceInstance: removing state object for %s", n.Addr) @@ -255,15 +258,11 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) State: &state, Error: &provisionerErr, } - diags = evalApplyPost.Eval(ctx) + diags = diags.Append(evalApplyPost.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } - err = UpdateStateHook(ctx) - if err != nil { - return err - } - - return nil + diags = diags.Append(UpdateStateHook(ctx)) + return diags } diff --git a/terraform/node_resource_destroy_deposed.go b/terraform/node_resource_destroy_deposed.go index 4a641dece6..8fbfc2b9e5 100644 --- a/terraform/node_resource_destroy_deposed.go +++ b/terraform/node_resource_destroy_deposed.go @@ -64,14 +64,13 @@ func (n *NodePlanDeposedResourceInstanceObject) References() []*addrs.Reference } // GraphNodeEvalable impl. -func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walkOperation) error { - var diags tfdiags.Diagnostics - +func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { addr := n.ResourceInstanceAddr() provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } // During the plan walk we always produce a planned destroy change, because @@ -86,9 +85,9 @@ func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walk Provider: &provider, ProviderSchema: &providerSchema, } - diags = readStateDeposed.Eval(ctx) + diags = diags.Append(readStateDeposed.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } diffDestroy := &EvalDiffDestroy{ @@ -98,9 +97,9 @@ func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walk State: &state, Output: &change, } - diags = diffDestroy.Eval(ctx) + diags = diags.Append(diffDestroy.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } writeDiff := &EvalWriteDiff{ @@ -109,12 +108,8 @@ func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walk ProviderSchema: &providerSchema, Change: &change, } - diags = writeDiff.Eval(ctx) - if diags.HasErrors() { - return diags.ErrWithWarnings() - } - - return nil + diags = diags.Append(writeDiff.Eval(ctx)) + return diags } // NodeDestroyDeposedResourceInstanceObject represents deposed resource @@ -184,9 +179,7 @@ func (n *NodeDestroyDeposedResourceInstanceObject) ModifyCreateBeforeDestroy(v b } // GraphNodeExecutable impl. -func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op walkOperation) error { - var diags tfdiags.Diagnostics - +func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { addr := n.ResourceInstanceAddr().Resource var state *states.ResourceInstanceObject @@ -194,8 +187,9 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w var applyError error provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } readStateDeposed := &EvalReadStateDeposed{ @@ -205,9 +199,9 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w Provider: &provider, ProviderSchema: &providerSchema, } - diags = readStateDeposed.Eval(ctx) + diags = diags.Append(readStateDeposed.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } diffDestroy := &EvalDiffDestroy{ @@ -216,9 +210,9 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w State: &state, Output: &change, } - diags = diffDestroy.Eval(ctx) + diags = diags.Append(diffDestroy.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } // Call pre-apply hook @@ -227,9 +221,9 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w State: &state, Change: &change, } - diags = applyPre.Eval(ctx) + diags = diags.Append(applyPre.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } apply := &EvalApply{ @@ -243,9 +237,9 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w Output: &state, Error: &applyError, } - diags = apply.Eval(ctx) + diags = diags.Append(apply.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } // Always write the resource back to the state deposed. If it @@ -258,9 +252,9 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w ProviderSchema: &providerSchema, State: &state, } - diags = writeStateDeposed.Eval(ctx) + diags = diags.Append(writeStateDeposed.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } applyPost := &EvalApplyPost{ @@ -268,15 +262,16 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w State: &state, Error: &applyError, } - diags = applyPost.Eval(ctx) + diags = diags.Append(applyPost.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } if applyError != nil { - return applyError + diags = diags.Append(applyError) + return diags } - UpdateStateHook(ctx) - return nil + diags = diags.Append(UpdateStateHook(ctx)) + return diags } // GraphNodeDeposer is an optional interface implemented by graph nodes that diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index 466c3a87d9..a6eb707845 100644 --- a/terraform/node_resource_plan.go +++ b/terraform/node_resource_plan.go @@ -178,15 +178,14 @@ func (n *NodePlannableResource) ModuleInstance() addrs.ModuleInstance { } // GraphNodeExecutable -func (n *NodePlannableResource) Execute(ctx EvalContext, op walkOperation) error { +func (n *NodePlannableResource) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics { if n.Config == nil { // Nothing to do, then. log.Printf("[TRACE] NodeApplyableResource: no configuration present for %s", n.Name()) return nil } - err := n.writeResourceState(ctx, n.Addr) - return err + return n.writeResourceState(ctx, n.Addr) } // GraphNodeDestroyerCBD diff --git a/terraform/node_resource_plan_destroy.go b/terraform/node_resource_plan_destroy.go index 53db9f14b6..8396708525 100644 --- a/terraform/node_resource_plan_destroy.go +++ b/terraform/node_resource_plan_destroy.go @@ -4,6 +4,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/states" + "github.com/hashicorp/terraform/tfdiags" ) // NodePlanDestroyableResourceInstance represents a resource that is ready @@ -32,7 +33,7 @@ func (n *NodePlanDestroyableResourceInstance) DestroyAddr() *addrs.AbsResourceIn } // GraphNodeEvalable -func (n *NodePlanDestroyableResourceInstance) Execute(ctx EvalContext, op walkOperation) error { +func (n *NodePlanDestroyableResourceInstance) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { addr := n.ResourceInstanceAddr() // Declare a bunch of variables that are used for state during @@ -42,13 +43,15 @@ func (n *NodePlanDestroyableResourceInstance) Execute(ctx EvalContext, op walkOp var state *states.ResourceInstanceObject _, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } state, err = n.ReadResourceInstanceState(ctx, addr) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } diffDestroy := &EvalDiffDestroy{ @@ -57,14 +60,14 @@ func (n *NodePlanDestroyableResourceInstance) Execute(ctx EvalContext, op walkOp State: &state, Output: &change, } - diags := diffDestroy.Eval(ctx) + diags = diags.Append(diffDestroy.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } - err = n.checkPreventDestroy(change) - if err != nil { - return err + diags = diags.Append(n.checkPreventDestroy(change)) + if diags.HasErrors() { + return diags } writeDiff := &EvalWriteDiff{ @@ -72,6 +75,6 @@ func (n *NodePlanDestroyableResourceInstance) Execute(ctx EvalContext, op walkOp ProviderSchema: &providerSchema, Change: &change, } - diags = writeDiff.Eval(ctx) - return diags.ErrWithWarnings() + diags = diags.Append(writeDiff.Eval(ctx)) + return diags } diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index ec653ff513..e870e30508 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -31,7 +31,7 @@ var ( ) // GraphNodeEvalable -func (n *NodePlannableResourceInstance) Execute(ctx EvalContext, op walkOperation) error { +func (n *NodePlannableResourceInstance) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics { addr := n.ResourceInstanceAddr() // Eval info is different depending on what kind of resource this is @@ -45,8 +45,7 @@ func (n *NodePlannableResourceInstance) Execute(ctx EvalContext, op walkOperatio } } -func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) error { - var diags tfdiags.Diagnostics +func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) (diags tfdiags.Diagnostics) { config := n.Config addr := n.ResourceInstanceAddr() @@ -54,13 +53,15 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) err var state *states.ResourceInstanceObject provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } state, err = n.ReadResourceInstanceState(ctx, addr) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } validateSelfRef := &EvalValidateSelfRef{ @@ -68,9 +69,9 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) err Config: config.Config, ProviderSchema: &providerSchema, } - diags = validateSelfRef.Eval(ctx) + diags = diags.Append(validateSelfRef.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } readDataPlan := &evalReadDataPlan{ @@ -86,9 +87,9 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) err dependsOn: n.dependsOn, }, } - diags = readDataPlan.Eval(ctx) + diags = diags.Append(readDataPlan.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } // write the data source into both the refresh state and the @@ -100,9 +101,9 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) err State: &state, targetState: refreshState, } - diags = writeRefreshState.Eval(ctx) + diags = diags.Append(writeRefreshState.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } writeState := &EvalWriteState{ @@ -111,9 +112,9 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) err ProviderSchema: &providerSchema, State: &state, } - diags = writeState.Eval(ctx) + diags = diags.Append(writeState.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } writeDiff := &EvalWriteDiff{ @@ -121,12 +122,11 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) err ProviderSchema: &providerSchema, Change: &change, } - diags = writeDiff.Eval(ctx) - return diags.ErrWithWarnings() + diags = diags.Append(writeDiff.Eval(ctx)) + return diags } -func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) error { - var diags tfdiags.Diagnostics +func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) (diags tfdiags.Diagnostics) { config := n.Config addr := n.ResourceInstanceAddr() @@ -135,8 +135,9 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) var instancePlanState *states.ResourceInstanceObject provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } validateSelfRef := &EvalValidateSelfRef{ @@ -144,14 +145,15 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) Config: config.Config, ProviderSchema: &providerSchema, } - diags = validateSelfRef.Eval(ctx) + diags = diags.Append(validateSelfRef.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } instanceRefreshState, err = n.ReadResourceInstanceState(ctx, addr) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } refreshLifecycle := &EvalRefreshLifecycle{ Addr: addr, @@ -159,9 +161,9 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) State: &instanceRefreshState, ForceCreateBeforeDestroy: n.ForceCreateBeforeDestroy, } - diags = refreshLifecycle.Eval(ctx) + diags = diags.Append(refreshLifecycle.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } // Refresh, maybe @@ -175,9 +177,9 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) State: &instanceRefreshState, Output: &instanceRefreshState, } - diags := refresh.Eval(ctx) + diags := diags.Append(refresh.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } writeRefreshState := &EvalWriteState{ @@ -188,9 +190,9 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) targetState: refreshState, Dependencies: &n.Dependencies, } - diags = writeRefreshState.Eval(ctx) + diags = diags.Append(writeRefreshState.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } } @@ -207,14 +209,14 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) OutputChange: &change, OutputState: &instancePlanState, } - diags = diff.Eval(ctx) + diags = diags.Append(diff.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } - err = n.checkPreventDestroy(change) - if err != nil { - return err + diags = diags.Append(n.checkPreventDestroy(change)) + if diags.HasErrors() { + return diags } writeState := &EvalWriteState{ @@ -223,9 +225,9 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) State: &instancePlanState, ProviderSchema: &providerSchema, } - diags = writeState.Eval(ctx) + diags = diags.Append(writeState.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } writeDiff := &EvalWriteDiff{ @@ -233,6 +235,6 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) ProviderSchema: &providerSchema, Change: &change, } - diags = writeDiff.Eval(ctx) - return diags.ErrWithWarnings() + diags = diags.Append(writeDiff.Eval(ctx)) + return diags } diff --git a/terraform/node_resource_plan_orphan.go b/terraform/node_resource_plan_orphan.go index 56edb17098..5eb3fe29b5 100644 --- a/terraform/node_resource_plan_orphan.go +++ b/terraform/node_resource_plan_orphan.go @@ -34,7 +34,7 @@ func (n *NodePlannableResourceInstanceOrphan) Name() string { } // GraphNodeExecutable -func (n *NodePlannableResourceInstanceOrphan) Execute(ctx EvalContext, op walkOperation) error { +func (n *NodePlannableResourceInstanceOrphan) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics { addr := n.ResourceInstanceAddr() // Eval info is different depending on what kind of resource this is @@ -48,7 +48,7 @@ func (n *NodePlannableResourceInstanceOrphan) Execute(ctx EvalContext, op walkOp } } -func (n *NodePlannableResourceInstanceOrphan) dataResourceExecute(ctx EvalContext) error { +func (n *NodePlannableResourceInstanceOrphan) dataResourceExecute(ctx EvalContext) tfdiags.Diagnostics { // A data source that is no longer in the config is removed from the state log.Printf("[TRACE] NodePlannableResourceInstanceOrphan: removing state object for %s", n.Addr) state := ctx.RefreshState() @@ -56,9 +56,7 @@ func (n *NodePlannableResourceInstanceOrphan) dataResourceExecute(ctx EvalContex return nil } -func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalContext) error { - var diags tfdiags.Diagnostics - +func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalContext) (diags tfdiags.Diagnostics) { addr := n.ResourceInstanceAddr() // Declare a bunch of variables that are used for state during @@ -67,13 +65,15 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon var state *states.ResourceInstanceObject provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } state, err = n.ReadResourceInstanceState(ctx, addr) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } if !n.skipRefresh { @@ -92,9 +92,9 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon State: &state, Output: &state, } - diags = refresh.Eval(ctx) + diags = diags.Append(refresh.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } writeRefreshState := &EvalWriteState{ @@ -104,9 +104,9 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon State: &state, targetState: refreshState, } - diags = writeRefreshState.Eval(ctx) + diags = diags.Append(writeRefreshState.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } } @@ -117,14 +117,14 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon Output: &change, OutputState: &state, // Will point to a nil state after this complete, signalling destroyed } - diags = diffDestroy.Eval(ctx) - if err != nil { - return diags.ErrWithWarnings() + diags = diags.Append(diffDestroy.Eval(ctx)) + if diags.HasErrors() { + return diags } - err = n.checkPreventDestroy(change) - if err != nil { - return err + diags = diags.Append(n.checkPreventDestroy(change)) + if diags.HasErrors() { + return diags } writeDiff := &EvalWriteDiff{ @@ -132,9 +132,9 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon ProviderSchema: &providerSchema, Change: &change, } - diags = writeDiff.Eval(ctx) + diags = diags.Append(writeDiff.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } writeState := &EvalWriteState{ @@ -143,9 +143,6 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon ProviderSchema: &providerSchema, State: &state, } - diags = writeState.Eval(ctx) - if diags.HasErrors() { - return diags.ErrWithWarnings() - } - return nil + diags = diags.Append(writeState.Eval(ctx)) + return diags } diff --git a/terraform/node_resource_plan_orphan_test.go b/terraform/node_resource_plan_orphan_test.go index f4396448ed..57f6804d6b 100644 --- a/terraform/node_resource_plan_orphan_test.go +++ b/terraform/node_resource_plan_orphan_test.go @@ -55,9 +55,9 @@ func TestNodeResourcePlanOrphanExecute(t *testing.T) { Addr: mustResourceInstanceAddr("test_object.foo"), }, } - err := node.Execute(ctx, walkApply) - if err != nil { - t.Fatalf("unexpected error: %s", err.Error()) + diags := node.Execute(ctx, walkApply) + if diags.HasErrors() { + t.Fatalf("unexpected error: %s", diags.Err()) } if !state.Empty() { t.Fatalf("expected empty state, got %s", state.String()) diff --git a/terraform/node_resource_plan_test.go b/terraform/node_resource_plan_test.go index c5ae144ba2..e3a8faa19c 100644 --- a/terraform/node_resource_plan_test.go +++ b/terraform/node_resource_plan_test.go @@ -23,9 +23,9 @@ func TestNodePlannableResourceExecute(t *testing.T) { }, Addr: mustAbsResourceAddr("test_instance.foo"), } - err := node.Execute(ctx, walkApply) - if err != nil { - t.Fatalf("unexpected error: %s", err.Error()) + diags := node.Execute(ctx, walkApply) + if diags.HasErrors() { + t.Fatalf("unexpected error: %s", diags.Err()) } if !state.Empty() { t.Fatalf("expected no state, got:\n %s", state.String()) @@ -48,9 +48,9 @@ func TestNodePlannableResourceExecute(t *testing.T) { }, Addr: mustAbsResourceAddr("test_instance.foo"), } - err := node.Execute(ctx, walkApply) - if err != nil { - t.Fatalf("unexpected error: %s", err.Error()) + diags := node.Execute(ctx, walkApply) + if diags.HasErrors() { + t.Fatalf("unexpected error: %s", diags.Err()) } if state.Empty() { t.Fatal("expected resources in state, got empty state") diff --git a/terraform/node_resource_validate.go b/terraform/node_resource_validate.go index 436d30a455..3a5bd6d0c9 100644 --- a/terraform/node_resource_validate.go +++ b/terraform/node_resource_validate.go @@ -5,6 +5,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" + "github.com/hashicorp/terraform/tfdiags" "github.com/zclconf/go-cty/cty" ) @@ -31,7 +32,7 @@ func (n *NodeValidatableResource) Path() addrs.ModuleInstance { } // GraphNodeEvalable -func (n *NodeValidatableResource) Execute(ctx EvalContext, op walkOperation) error { +func (n *NodeValidatableResource) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { addr := n.ResourceAddr() config := n.Config @@ -40,8 +41,9 @@ func (n *NodeValidatableResource) Execute(ctx EvalContext, op walkOperation) err // passed to the EvalNodes. var configVal cty.Value provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } evalValidateResource := &EvalValidateResource{ @@ -52,9 +54,9 @@ func (n *NodeValidatableResource) Execute(ctx EvalContext, op walkOperation) err Config: config, ConfigVal: &configVal, } - err = evalValidateResource.Validate(ctx) - if err != nil { - return err + diags = diags.Append(evalValidateResource.Validate(ctx)) + if diags.HasErrors() { + return diags } if managed := n.Config.Managed; managed != nil { @@ -71,11 +73,13 @@ func (n *NodeValidatableResource) Execute(ctx EvalContext, op walkOperation) err provisioner := ctx.Provisioner(p.Type) if provisioner == nil { - return fmt.Errorf("provisioner %s not initialized", p.Type) + diags = diags.Append(fmt.Errorf("provisioner %s not initialized", p.Type)) + return diags } provisionerSchema := ctx.ProvisionerSchema(p.Type) if provisionerSchema == nil { - return fmt.Errorf("provisioner %s not initialized", p.Type) + diags = diags.Append(fmt.Errorf("provisioner %s not initialized", p.Type)) + return diags } // Validate Provisioner Config @@ -87,11 +91,11 @@ func (n *NodeValidatableResource) Execute(ctx EvalContext, op walkOperation) err ResourceHasCount: hasCount, ResourceHasForEach: hasForEach, } - err := validateProvisioner.Validate(ctx) - if err != nil { - return err + diags = diags.Append(validateProvisioner.Validate(ctx)) + if diags.HasErrors() { + return diags } } } - return nil + return diags } diff --git a/terraform/node_root_variable.go b/terraform/node_root_variable.go index 63fe818046..5e90e4ad67 100644 --- a/terraform/node_root_variable.go +++ b/terraform/node_root_variable.go @@ -4,6 +4,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/dag" + "github.com/hashicorp/terraform/tfdiags" ) // NodeRootVariable represents a root variable input. @@ -36,7 +37,7 @@ func (n *NodeRootVariable) ReferenceableAddrs() []addrs.Referenceable { } // GraphNodeExecutable -func (n *NodeRootVariable) Execute(ctx EvalContext, op walkOperation) error { +func (n *NodeRootVariable) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics { // We don't actually need to _evaluate_ a root module variable, because // its value is always constant and already stashed away in our EvalContext. // However, we might need to run some user-defined validation rules against diff --git a/terraform/node_root_variable_test.go b/terraform/node_root_variable_test.go index 2a410e212e..6546395bde 100644 --- a/terraform/node_root_variable_test.go +++ b/terraform/node_root_variable_test.go @@ -17,9 +17,9 @@ func TestNodeRootVariableExecute(t *testing.T) { }, } - err := n.Execute(ctx, walkApply) - if err != nil { - t.Fatalf("unexpected error: %s", err.Error()) + diags := n.Execute(ctx, walkApply) + if diags.HasErrors() { + t.Fatalf("unexpected error: %s", diags.Err()) } } diff --git a/terraform/transform_import_state.go b/terraform/transform_import_state.go index 6547f1a6d7..941bf135aa 100644 --- a/terraform/transform_import_state.go +++ b/terraform/transform_import_state.go @@ -116,25 +116,25 @@ func (n *graphNodeImportState) ModulePath() addrs.Module { } // GraphNodeExecutable impl. -func (n *graphNodeImportState) Execute(ctx EvalContext, op walkOperation) error { +func (n *graphNodeImportState) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { // Reset our states n.states = nil provider, _, err := GetProvider(ctx, n.ResolvedProvider) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } // import state absAddr := n.Addr.Resource.Absolute(ctx.Path()) - var diags tfdiags.Diagnostics // Call pre-import hook - err = ctx.Hook(func(h Hook) (HookAction, error) { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { return h.PreImportState(absAddr, n.ID) - }) - if err != nil { - return err + })) + if diags.HasErrors() { + return diags } resp := provider.ImportResourceState(providers.ImportResourceStateRequest{ @@ -143,7 +143,7 @@ func (n *graphNodeImportState) Execute(ctx EvalContext, op walkOperation) error }) diags = diags.Append(resp.Diagnostics) if diags.HasErrors() { - return diags.Err() + return diags } imported := resp.ImportedResources @@ -153,10 +153,10 @@ func (n *graphNodeImportState) Execute(ctx EvalContext, op walkOperation) error n.states = imported // Call post-import hook - err = ctx.Hook(func(h Hook) (HookAction, error) { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { return h.PostImportState(absAddr, imported) - }) - return err + })) + return diags } // GraphNodeDynamicExpandable impl. @@ -259,17 +259,18 @@ func (n *graphNodeImportStateSub) Path() addrs.ModuleInstance { } // GraphNodeExecutable impl. -func (n *graphNodeImportStateSub) Execute(ctx EvalContext, op walkOperation) error { - var diags tfdiags.Diagnostics +func (n *graphNodeImportStateSub) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { // If the Ephemeral type isn't set, then it is an error if n.State.TypeName == "" { - return fmt.Errorf("import of %s didn't set type", n.TargetAddr.String()) + diags = diags.Append(fmt.Errorf("import of %s didn't set type", n.TargetAddr.String())) + return diags } state := n.State.AsInstanceObject() provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) - if err != nil { - return err + diags = diags.Append(err) + if diags.HasErrors() { + return diags } // EvalRefresh @@ -281,9 +282,9 @@ func (n *graphNodeImportStateSub) Execute(ctx EvalContext, op walkOperation) err State: &state, Output: &state, } - diags = evalRefresh.Eval(ctx) + diags = diags.Append(evalRefresh.Eval(ctx)) if diags.HasErrors() { - return diags.ErrWithWarnings() + return diags } // Verify the existance of the imported resource @@ -296,7 +297,7 @@ func (n *graphNodeImportStateSub) Execute(ctx EvalContext, op walkOperation) err n.TargetAddr.Resource.String(), ), )) - return diags.Err() + return diags } //EvalWriteState @@ -307,5 +308,5 @@ func (n *graphNodeImportStateSub) Execute(ctx EvalContext, op walkOperation) err State: &state, } diags = diags.Append(evalWriteState.Eval(ctx)) - return diags.ErrWithWarnings() + return diags } diff --git a/terraform/transform_import_state_test.go b/terraform/transform_import_state_test.go index 9068a3654d..362fd1862a 100644 --- a/terraform/transform_import_state_test.go +++ b/terraform/transform_import_state_test.go @@ -41,9 +41,9 @@ func TestGraphNodeImportStateExecute(t *testing.T) { }, } - err := node.Execute(ctx, walkImport) - if err != nil { - t.Fatalf("Unexpected error: %s", err.Error()) + diags := node.Execute(ctx, walkImport) + if diags.HasErrors() { + t.Fatalf("Unexpected error: %s", diags.Err()) } if len(node.states) != 1 { @@ -93,9 +93,9 @@ func TestGraphNodeImportStateSubExecute(t *testing.T) { Module: addrs.RootModule, }, } - err := node.Execute(ctx, walkImport) - if err != nil { - t.Fatalf("Unexpected error: %s", err.Error()) + diags := node.Execute(ctx, walkImport) + if diags.HasErrors() { + t.Fatalf("Unexpected error: %s", diags.Err()) } // check for resource in state diff --git a/terraform/transform_provider.go b/terraform/transform_provider.go index d3b2248deb..7a22008b64 100644 --- a/terraform/transform_provider.go +++ b/terraform/transform_provider.go @@ -449,6 +449,7 @@ type graphNodeCloseProvider struct { var ( _ GraphNodeCloseProvider = (*graphNodeCloseProvider)(nil) + _ GraphNodeExecutable = (*graphNodeCloseProvider)(nil) ) func (n *graphNodeCloseProvider) Name() string { @@ -461,8 +462,8 @@ func (n *graphNodeCloseProvider) ModulePath() addrs.Module { } // GraphNodeExecutable impl. -func (n *graphNodeCloseProvider) Execute(ctx EvalContext, op walkOperation) error { - return ctx.CloseProvider(n.Addr) +func (n *graphNodeCloseProvider) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { + return diags.Append(ctx.CloseProvider(n.Addr)) } // GraphNodeDependable impl. diff --git a/terraform/transform_provisioner.go b/terraform/transform_provisioner.go index 638410d4af..80bc25f26e 100644 --- a/terraform/transform_provisioner.go +++ b/terraform/transform_provisioner.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/dag" + "github.com/hashicorp/terraform/tfdiags" ) // GraphNodeProvisioner is an interface that nodes that can be a provisioner @@ -165,13 +166,15 @@ type graphNodeCloseProvisioner struct { ProvisionerNameValue string } +var _ GraphNodeExecutable = (*graphNodeCloseProvisioner)(nil) + func (n *graphNodeCloseProvisioner) Name() string { return fmt.Sprintf("provisioner.%s (close)", n.ProvisionerNameValue) } // GraphNodeExecutable impl. -func (n *graphNodeCloseProvisioner) Execute(ctx EvalContext, op walkOperation) error { - return ctx.CloseProvisioner(n.ProvisionerNameValue) +func (n *graphNodeCloseProvisioner) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { + return diags.Append(ctx.CloseProvisioner(n.ProvisionerNameValue)) } func (n *graphNodeCloseProvisioner) CloseProvisionerName() string { From 95f30451d96b6b77a0ccea0de83818891229cd02 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 28 Oct 2020 14:12:52 -0400 Subject: [PATCH 10/12] get rid of EvalEarlyExitError This was mostly unused now, since we no longer needed to interrupt a series of eval node executions. The exception was the stopHook, which is still used to halt execution when there's an interrupt. Since interrupting execution should not complete successfully, we use a normal opaque error to halt everything, and return it to the UI. We can work on coalescing or hiding these if necessary in a separate PR. --- terraform/context_apply_test.go | 37 ++++++++++++++++--- terraform/eval_context_builtin.go | 2 +- terraform/eval_error.go | 7 ---- terraform/graph_walk_context.go | 45 +---------------------- terraform/hook_stop.go | 7 +--- terraform/node_resource_apply_instance.go | 3 -- terraform/node_resource_destroy.go | 2 - 7 files changed, 37 insertions(+), 66 deletions(-) delete mode 100644 terraform/eval_error.go diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 8d76edc6bb..8b7254f0bd 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -1734,8 +1734,16 @@ func TestContext2Apply_cancel(t *testing.T) { }() state := <-stateCh - if applyDiags.HasErrors() { - t.Fatalf("unexpected errors: %s", applyDiags.Err()) + // only expecting an early exit error + if !applyDiags.HasErrors() { + t.Fatal("expected early exit error") + } + + for _, d := range applyDiags { + desc := d.Description() + if desc.Summary != "execution halted" { + t.Fatalf("unexpected error: %v", applyDiags.Err()) + } } actual := strings.TrimSpace(state.String()) @@ -1812,8 +1820,16 @@ func TestContext2Apply_cancelBlock(t *testing.T) { // Wait for apply to complete state := <-stateCh - if applyDiags.HasErrors() { - t.Fatalf("unexpected error: %s", applyDiags.Err()) + // only expecting an early exit error + if !applyDiags.HasErrors() { + t.Fatal("expected early exit error") + } + + for _, d := range applyDiags { + desc := d.Description() + if desc.Summary != "execution halted" { + t.Fatalf("unexpected error: %v", applyDiags.Err()) + } } checkStateString(t, state, ` @@ -1882,7 +1898,18 @@ func TestContext2Apply_cancelProvisioner(t *testing.T) { // Wait for completion state := <-stateCh - assertNoErrors(t, applyDiags) + + // we are expecting only an early exit error + if !applyDiags.HasErrors() { + t.Fatal("expected early exit error") + } + + for _, d := range applyDiags { + desc := d.Description() + if desc.Summary != "execution halted" { + t.Fatalf("unexpected error: %v", applyDiags.Err()) + } + } checkStateString(t, state, ` aws_instance.foo: (tainted) diff --git a/terraform/eval_context_builtin.go b/terraform/eval_context_builtin.go index 67264948ef..63f87cc274 100644 --- a/terraform/eval_context_builtin.go +++ b/terraform/eval_context_builtin.go @@ -107,7 +107,7 @@ func (ctx *BuiltinEvalContext) Hook(fn func(Hook) (HookAction, error)) error { case HookActionHalt: // Return an early exit error to trigger an early exit log.Printf("[WARN] Early exit triggered by hook: %T", h) - return EvalEarlyExitError{} + return nil } } diff --git a/terraform/eval_error.go b/terraform/eval_error.go deleted file mode 100644 index 853ea2cc8e..0000000000 --- a/terraform/eval_error.go +++ /dev/null @@ -1,7 +0,0 @@ -package terraform - -// EvalEarlyExitError is a special error return value that can be returned -// by eval nodes that does an early exit. -type EvalEarlyExitError struct{} - -func (EvalEarlyExitError) Error() string { return "early exit" } diff --git a/terraform/graph_walk_context.go b/terraform/graph_walk_context.go index 907842f252..70cf4aff41 100644 --- a/terraform/graph_walk_context.go +++ b/terraform/graph_walk_context.go @@ -2,7 +2,6 @@ package terraform import ( "context" - "errors" "sync" "github.com/zclconf/go-cty/cty" @@ -36,7 +35,6 @@ type ContextGraphWalker struct { // is in progress. NonFatalDiagnostics tfdiags.Diagnostics - errorLock sync.Mutex once sync.Once contexts map[string]*BuiltinEvalContext contextLock sync.Mutex @@ -124,46 +122,7 @@ func (w *ContextGraphWalker) init() { func (w *ContextGraphWalker) Execute(ctx EvalContext, n GraphNodeExecutable) tfdiags.Diagnostics { // Acquire a lock on the semaphore w.Context.parallelSem.Acquire() + defer w.Context.parallelSem.Release() - diags := n.Execute(ctx, w.Operation) - - // Release the semaphore - w.Context.parallelSem.Release() - - if !diags.HasErrors() { - return diags - } - - // Acquire the lock because anything is going to require a lock. - w.errorLock.Lock() - defer w.errorLock.Unlock() - - err := diags.Err() - // Handle a simple early exit error - if errors.Is(err, EvalEarlyExitError{}) { - return nil - } - - // we need to see if this wraps only EarlyExitErrors - if wrapper, ok := err.(interface{ WrappedErrors() []error }); ok { - //WrappedErrors only returns native error values, so we can't extract - //them from diagnostics. Just return the whole thing if we have a - //combination. - errs := wrapper.WrappedErrors() - earlyExit := true - for _, err := range errs { - if err != (EvalEarlyExitError{}) { - earlyExit = false - } - } - if len(errs) > 0 && earlyExit { - return nil - } - } - - // Otherwise, we'll let our usual diagnostics machinery figure out how to - // unpack this as one or more diagnostic messages and return that. If we - // get down here then the returned diagnostics will contain at least one - // error, causing the graph walk to halt. - return diags + return n.Execute(ctx, w.Operation) } diff --git a/terraform/hook_stop.go b/terraform/hook_stop.go index 811fb337cc..86f2211426 100644 --- a/terraform/hook_stop.go +++ b/terraform/hook_stop.go @@ -1,6 +1,7 @@ package terraform import ( + "errors" "sync/atomic" "github.com/zclconf/go-cty/cty" @@ -76,11 +77,7 @@ func (h *stopHook) PostStateUpdate(new *states.State) (HookAction, error) { func (h *stopHook) hook() (HookAction, error) { if h.Stopped() { - // FIXME: This should really return an error since stopping partway - // through is not a successful run-to-completion, but we'll need to - // introduce that cautiously since existing automation solutions may - // be depending on this behavior. - return HookActionHalt, nil + return HookActionHalt, errors.New("execution halted") } return HookActionContinue, nil diff --git a/terraform/node_resource_apply_instance.go b/terraform/node_resource_apply_instance.go index 90085997b5..bd6a528967 100644 --- a/terraform/node_resource_apply_instance.go +++ b/terraform/node_resource_apply_instance.go @@ -148,7 +148,6 @@ func (n *NodeApplyableResourceInstance) dataResourceExecute(ctx EvalContext) (di } // Stop early if we don't actually have a diff if change == nil { - diags = diags.Append(EvalEarlyExitError{}) return diags } @@ -223,7 +222,6 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) // We don't want to do any destroys // (these are handled by NodeDestroyResourceInstance instead) if diffApply == nil || diffApply.Action == plans.Delete { - diags = diags.Append(EvalEarlyExitError{}) return diags } @@ -325,7 +323,6 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) // into a NoOp if it only requires destroying, since destroying // is handled by NodeDestroyResourceInstance. if diffApply == nil || diffApply.Action == plans.NoOp { - diags = diags.Append(EvalEarlyExitError{}) return diags } diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 9e9bfe87db..163aa24dad 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -163,7 +163,6 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) // EvalReduceDiff may have simplified our planned change // into a NoOp if it does not require destroying. if changeApply == nil || changeApply.Action == plans.NoOp { - diags = diags.Append(EvalEarlyExitError{}) return diags } @@ -175,7 +174,6 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) // Exit early if the state object is null after reading the state if state == nil || state.Value.IsNull() { - diags = diags.Append(EvalEarlyExitError{}) return diags } From f987b697776dda3e387da9f90dac55989120ecea Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 28 Oct 2020 14:33:27 -0400 Subject: [PATCH 11/12] interrupted execution now exits with an error --- command/apply_test.go | 2 +- command/plan_test.go | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/command/apply_test.go b/command/apply_test.go index 280568b4f0..407d003054 100644 --- a/command/apply_test.go +++ b/command/apply_test.go @@ -924,7 +924,7 @@ func TestApply_shutdown(t *testing.T) { "-auto-approve", testFixturePath("apply-shutdown"), } - if code := c.Run(args); code != 0 { + if code := c.Run(args); code != 1 { t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) } diff --git a/command/plan_test.go b/command/plan_test.go index 7a99446a56..99525e32db 100644 --- a/command/plan_test.go +++ b/command/plan_test.go @@ -845,12 +845,8 @@ func TestPlan_shutdown(t *testing.T) { "-state=nonexistent.tfstate", testFixturePath("apply-shutdown"), }) - if code != 0 { - // FIXME: In retrospect cancellation ought to be an unsuccessful exit - // case, but we need to do that cautiously in case it impacts automation - // wrappers. See the note about this in the terraform.stopHook - // implementation for more. - t.Errorf("wrong exit code %d; want 0\noutput:\n%s", code, ui.OutputWriter.String()) + if code != 1 { + t.Errorf("wrong exit code %d; want 1\noutput:\n%s", code, ui.OutputWriter.String()) } select { From b8bed97ef45410ed9a6e65309706323119dbc4b3 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 28 Oct 2020 14:51:04 -0400 Subject: [PATCH 12/12] test for RPC warnings with no errors --- terraform/context_apply_test.go | 49 ++++++++++++++++++++++++++++++ terraform/context_plan_test.go | 43 ++++++++++++++++++++++++++ terraform/context_validate_test.go | 43 ++++++++++++++++++++++++++ 3 files changed, 135 insertions(+) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 8b7254f0bd..f65ce9bd56 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -12230,3 +12230,52 @@ resource "test_resource" "foo" { t.Fatal("missing 'test_resource.foo' in state:", state) } } + +func TestContext2Apply_rpcDiagnostics(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` +resource "test_instance" "a" { +} +`, + }) + + p := testProvider("test") + p.PlanResourceChangeFn = testDiffFn + p.ApplyResourceChangeFn = testApplyFn + p.GetSchemaReturn = &ProviderSchema{ + ResourceTypes: map[string]*configschema.Block{ + "test_instance": { + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Computed: true}, + }, + }, + }, + } + + p.ValidateResourceTypeConfigResponse = providers.ValidateResourceTypeConfigResponse{ + Diagnostics: tfdiags.Diagnostics(nil).Append(tfdiags.SimpleWarning("don't frobble")), + } + + ctx := testContext2(t, &ContextOpts{ + Config: m, + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + _, diags := ctx.Plan() + if diags.HasErrors() { + t.Fatal(diags.Err()) + } + + _, diags = ctx.Apply() + if diags.HasErrors() { + t.Fatal(diags.Err()) + } + + for _, d := range diags { + des := d.Description().Summary + if !strings.Contains(des, "frobble") { + t.Fatalf(`expected frobble, got %q`, des) + } + } +} diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 819870f962..33d0c091a8 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -6421,3 +6421,46 @@ data "test_data_source" "b" { t.Fatal("data source b was not read during plan") } } + +func TestContext2Plan_rpcDiagnostics(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` +resource "test_instance" "a" { +} +`, + }) + + p := testProvider("test") + p.PlanResourceChangeFn = testDiffFn + p.GetSchemaReturn = &ProviderSchema{ + ResourceTypes: map[string]*configschema.Block{ + "test_instance": { + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Computed: true}, + }, + }, + }, + } + + p.ValidateResourceTypeConfigResponse = providers.ValidateResourceTypeConfigResponse{ + Diagnostics: tfdiags.Diagnostics(nil).Append(tfdiags.SimpleWarning("don't herd cats")), + } + + ctx := testContext2(t, &ContextOpts{ + Config: m, + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + _, diags := ctx.Plan() + if diags.HasErrors() { + t.Fatal(diags.Err()) + } + + for _, d := range diags { + des := d.Description().Summary + if !strings.Contains(des, "cats") { + t.Fatalf(`expected cats, got %q`, des) + } + } +} diff --git a/terraform/context_validate_test.go b/terraform/context_validate_test.go index ec1f9b0919..4f5f45a9f3 100644 --- a/terraform/context_validate_test.go +++ b/terraform/context_validate_test.go @@ -1795,3 +1795,46 @@ resource "test_instance" "a" { } } } + +func TestContext2Validate_rpcDiagnostics(t *testing.T) { + // validate module and output depends_on + m := testModuleInline(t, map[string]string{ + "main.tf": ` +resource "test_instance" "a" { +} +`, + }) + + p := testProvider("test") + p.GetSchemaReturn = &ProviderSchema{ + ResourceTypes: map[string]*configschema.Block{ + "test_instance": { + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Computed: true}, + }, + }, + }, + } + + p.ValidateResourceTypeConfigResponse = providers.ValidateResourceTypeConfigResponse{ + Diagnostics: tfdiags.Diagnostics(nil).Append(tfdiags.SimpleWarning("don't frobble")), + } + + ctx := testContext2(t, &ContextOpts{ + Config: m, + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + diags := ctx.Validate() + if diags.HasErrors() { + t.Fatal(diags.Err()) + } + + for _, d := range diags { + des := d.Description().Summary + if !strings.Contains(des, "frobble") { + t.Fatalf(`expected frobble, got %q`, des) + } + } +}