From 0c8b4ce755b0e2bee955b24ec78f6c8e20eeba20 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 28 Apr 2021 13:43:45 -0400 Subject: [PATCH] add context to UgpradeResourceState diags This path was not yet using diagnostics, and returned simple errors. --- terraform/node_resource_abstract.go | 45 ++++++++++++++-------- terraform/node_resource_abstract_test.go | 12 +++--- terraform/node_resource_apply_instance.go | 8 ++-- terraform/node_resource_destroy.go | 4 +- terraform/node_resource_destroy_deposed.go | 7 ++-- terraform/node_resource_plan_instance.go | 10 ++--- terraform/node_resource_plan_orphan.go | 7 +--- 7 files changed, 50 insertions(+), 43 deletions(-) diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index 85e9dfeb7f..c03103b9c3 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -347,10 +347,12 @@ func (n *NodeAbstractResource) writeResourceState(ctx EvalContext, addr addrs.Ab // readResourceInstanceState reads the current object for a specific instance in // the state. -func (n *NodeAbstractResource) readResourceInstanceState(ctx EvalContext, addr addrs.AbsResourceInstance) (*states.ResourceInstanceObject, error) { +func (n *NodeAbstractResource) readResourceInstanceState(ctx EvalContext, addr addrs.AbsResourceInstance) (*states.ResourceInstanceObject, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics provider, providerSchema, err := getProvider(ctx, n.ResolvedProvider) if err != nil { - return nil, err + diags = diags.Append(err) + return nil, diags } log.Printf("[TRACE] readResourceInstanceState: reading state for %s", addr) @@ -365,36 +367,41 @@ func (n *NodeAbstractResource) readResourceInstanceState(ctx EvalContext, addr a schema, currentVersion := (providerSchema).SchemaForResourceAddr(addr.Resource.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", addr) + return nil, diags.Append(fmt.Errorf("no schema available for %s while reading state; this is a bug in Terraform and should be reported", addr)) } - var diags tfdiags.Diagnostics - src, diags = upgradeResourceState(addr, provider, src, schema, currentVersion) + src, upgradeDiags := upgradeResourceState(addr, provider, src, schema, currentVersion) + if n.Config != nil { + upgradeDiags = upgradeDiags.InConfigBody(n.Config.Config, addr.String()) + } + diags = diags.Append(upgradeDiags) 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 nil, diags } obj, err := src.Decode(schema.ImpliedType()) if err != nil { - return nil, err + diags = diags.Append(err) } - return obj, nil + return obj, diags } // readResourceInstanceStateDeposed reads the deposed object for a specific // instance in the state. -func (n *NodeAbstractResource) readResourceInstanceStateDeposed(ctx EvalContext, addr addrs.AbsResourceInstance, key states.DeposedKey) (*states.ResourceInstanceObject, error) { +func (n *NodeAbstractResource) readResourceInstanceStateDeposed(ctx EvalContext, addr addrs.AbsResourceInstance, key states.DeposedKey) (*states.ResourceInstanceObject, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics provider, providerSchema, err := getProvider(ctx, n.ResolvedProvider) if err != nil { - return nil, err + diags = diags.Append(err) + return nil, diags } if key == states.NotDeposed { - return nil, fmt.Errorf("readResourceInstanceStateDeposed used with no instance key; this is a bug in Terraform and should be reported") + return nil, diags.Append(fmt.Errorf("readResourceInstanceStateDeposed used with no instance key; this is a bug in Terraform and should be reported")) } log.Printf("[TRACE] readResourceInstanceStateDeposed: reading state for %s deposed object %s", addr, key) @@ -403,31 +410,35 @@ func (n *NodeAbstractResource) readResourceInstanceStateDeposed(ctx EvalContext, if src == nil { // Presumably we only have deposed objects, then. log.Printf("[TRACE] readResourceInstanceStateDeposed: no state present for %s deposed object %s", addr, key) - return nil, nil + return nil, diags } schema, currentVersion := (providerSchema).SchemaForResourceAddr(addr.Resource.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", addr) + return nil, diags.Append(fmt.Errorf("no schema available for %s while reading state; this is a bug in Terraform and should be reported", addr)) } - src, diags := upgradeResourceState(addr, provider, src, schema, currentVersion) + src, upgradeDiags := upgradeResourceState(addr, provider, src, schema, currentVersion) + if n.Config != nil { + upgradeDiags = upgradeDiags.InConfigBody(n.Config.Config, addr.String()) + } + diags = diags.Append(upgradeDiags) 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 nil, diags } obj, err := src.Decode(schema.ImpliedType()) if err != nil { - return nil, err + diags = diags.Append(err) } - return obj, nil + return obj, diags } // graphNodesAreResourceInstancesInDifferentInstancesOfSameModule is an diff --git a/terraform/node_resource_abstract_test.go b/terraform/node_resource_abstract_test.go index a0075889dc..5c921f9cba 100644 --- a/terraform/node_resource_abstract_test.go +++ b/terraform/node_resource_abstract_test.go @@ -160,9 +160,9 @@ func TestNodeAbstractResource_ReadResourceInstanceState(t *testing.T) { ctx.ProviderSchemaSchema = mockProvider.ProviderSchema() ctx.ProviderProvider = providers.Interface(mockProvider) - got, err := test.Node.readResourceInstanceState(ctx, test.Node.Addr.Resource.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance)) - if err != nil { - t.Fatalf("[%s] Got err: %#v", k, err.Error()) + got, readDiags := test.Node.readResourceInstanceState(ctx, test.Node.Addr.Resource.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance)) + if readDiags.HasErrors() { + t.Fatalf("[%s] Got err: %#v", k, readDiags.Err()) } expected := test.ExpectedInstanceId @@ -223,9 +223,9 @@ func TestNodeAbstractResource_ReadResourceInstanceStateDeposed(t *testing.T) { key := states.DeposedKey("00000001") // shim from legacy state assigns 0th deposed index this key - got, err := test.Node.readResourceInstanceStateDeposed(ctx, test.Node.Addr.Resource.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), key) - if err != nil { - t.Fatalf("[%s] Got err: %#v", k, err.Error()) + got, readDiags := test.Node.readResourceInstanceStateDeposed(ctx, test.Node.Addr.Resource.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), key) + if readDiags.HasErrors() { + t.Fatalf("[%s] Got err: %#v", k, readDiags.Err()) } expected := test.ExpectedInstanceId diff --git a/terraform/node_resource_apply_instance.go b/terraform/node_resource_apply_instance.go index a678fef89f..d6cc55c515 100644 --- a/terraform/node_resource_apply_instance.go +++ b/terraform/node_resource_apply_instance.go @@ -217,8 +217,8 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) log.Printf("[TRACE] managedResourceExecute: prior object for %s now deposed with key %s", n.Addr, deposedKey) } - state, err = n.readResourceInstanceState(ctx, n.ResourceInstanceAddr()) - diags = diags.Append(err) + state, readDiags := n.readResourceInstanceState(ctx, n.ResourceInstanceAddr()) + diags = diags.Append(readDiags) if diags.HasErrors() { return diags } @@ -244,8 +244,8 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext) return diags } - state, err = n.readResourceInstanceState(ctx, n.ResourceInstanceAddr()) - diags = diags.Append(err) + state, readDiags = n.readResourceInstanceState(ctx, n.ResourceInstanceAddr()) + diags = diags.Append(readDiags) if diags.HasErrors() { return diags } diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index d725e40dfc..1027b67f35 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -177,8 +177,8 @@ func (n *NodeDestroyResourceInstance) managedResourceExecute(ctx EvalContext) (d return diags } - state, err = n.readResourceInstanceState(ctx, addr) - diags = diags.Append(err) + state, readDiags := n.readResourceInstanceState(ctx, addr) + diags = diags.Append(readDiags) if diags.HasErrors() { return diags } diff --git a/terraform/node_resource_destroy_deposed.go b/terraform/node_resource_destroy_deposed.go index 9c80c2b2a9..a63d49bf0d 100644 --- a/terraform/node_resource_destroy_deposed.go +++ b/terraform/node_resource_destroy_deposed.go @@ -184,9 +184,10 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w // Always write the resource back to the state deposed. If it // was successfully destroyed it will be pruned. If it was not, it will // be caught on the next run. - err = n.writeResourceInstanceState(ctx, state) - if err != nil { - return diags.Append(err) + writeDiags := n.writeResourceInstanceState(ctx, state) + diags.Append(writeDiags) + if diags.HasErrors() { + return diags } diags = diags.Append(n.postApplyHook(ctx, state, diags.Err())) diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index a958243770..e50a5cfcff 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -52,7 +52,6 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) (di addr := n.ResourceInstanceAddr() var change *plans.ResourceInstanceChange - var state *states.ResourceInstanceObject _, providerSchema, err := getProvider(ctx, n.ResolvedProvider) diags = diags.Append(err) @@ -60,8 +59,8 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) (di return diags } - state, err = n.readResourceInstanceState(ctx, addr) - diags = diags.Append(err) + state, readDiags := n.readResourceInstanceState(ctx, addr) + diags = diags.Append(readDiags) if diags.HasErrors() { return diags } @@ -97,7 +96,6 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) addr := n.ResourceInstanceAddr() var change *plans.ResourceInstanceChange - var instanceRefreshState *states.ResourceInstanceObject var instancePlanState *states.ResourceInstanceObject _, providerSchema, err := getProvider(ctx, n.ResolvedProvider) @@ -111,8 +109,8 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) return diags } - instanceRefreshState, err = n.readResourceInstanceState(ctx, addr) - diags = diags.Append(err) + instanceRefreshState, readDiags := n.readResourceInstanceState(ctx, addr) + diags = diags.Append(readDiags) if diags.HasErrors() { return diags } diff --git a/terraform/node_resource_plan_orphan.go b/terraform/node_resource_plan_orphan.go index 77cdffb161..94a4f92351 100644 --- a/terraform/node_resource_plan_orphan.go +++ b/terraform/node_resource_plan_orphan.go @@ -6,7 +6,6 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/plans" - "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" ) @@ -77,11 +76,9 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon // Declare a bunch of variables that are used for state during // evaluation. These are written to by-address below. var change *plans.ResourceInstanceChange - var state *states.ResourceInstanceObject - var err error - state, err = n.readResourceInstanceState(ctx, addr) - diags = diags.Append(err) + state, readDiags := n.readResourceInstanceState(ctx, addr) + diags = diags.Append(readDiags) if diags.HasErrors() { return diags }