From 988059d5331e50851fbff77564bef0071ace972b Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 28 Oct 2020 13:47:04 -0400 Subject: [PATCH] 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 72734b788..cd91fcc7c 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 5bf06c4d0..a31a50469 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 1c8273e97..907842f25 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 bfdbd1efa..2e972ff21 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 13c5fac29..497d4fc96 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 731371920..14c06516b 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 24fb988dd..32d07b143 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 055492fe8..eb04b3954 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 1e27fd274..6f41232de 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 578b4276f..c83d16934 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 78ddb6e8f..b20f2a72c 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 00f39c80e..a2f11d3e9 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 63fd35214..3b518f046 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 c901eb417..3ea0485d4 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 aa6ba2c58..a89583cff 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 5c418bde9..9ce13b9df 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 c4df9c627..eaa6fc815 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 4606d5185..fa9f501ef 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 ff09ba614..02e71554a 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 0d902b837..90085997b 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 a841a3563..54b65ec4f 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 26dd56dee..9e9bfe87d 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 4a641dece..8fbfc2b9e 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 466c3a87d..a6eb70784 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 53db9f14b..839670852 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 ec653ff51..e870e3050 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 56edb1709..5eb3fe29b 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 f4396448e..57f6804d6 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 c5ae144ba..e3a8faa19 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 436d30a45..3a5bd6d0c 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 63fe81804..5e90e4ad6 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 2a410e212..6546395bd 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 6547f1a6d..941bf135a 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 9068a3654..362fd1862 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 d3b2248de..7a22008b6 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 638410d4a..80bc25f26 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 {