From 477111e6b64a21a0e084ad1e88a21906e35d8c13 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 27 Oct 2020 18:16:28 -0400 Subject: [PATCH] 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