add context to UgpradeResourceState diags

This path was not yet using diagnostics, and returned simple errors.

Author: jbardin

Diff for: 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 for: 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 for: 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 for: 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 for: 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 for: terraform/node_resource_plan_instance.go

@@ -52,16 +52,15 @@ 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)
 	if diags.HasErrors() {
 		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 for: 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
 	}