core: Re-implement EvalReadDiff for the new plan types

This also includes passing in the provider schema to a few more EvalNodes
that were expecting it but not getting it, in order to be able to
successfully test the implementation of EvalReadDiff here.

plans/changes_sync.go

@@ -1,7 +1,11 @@
 package plans
 
 import (
+	"fmt"
 	"sync"
+
+	"github.com/hashicorp/terraform/addrs"
+	"github.com/hashicorp/terraform/states"
 )
 
 // ChangesSync is a wrapper around a Changes that provides a concurrency-safe
@@ -33,3 +37,27 @@ func (cs *ChangesSync) AppendResourceInstanceChange(changeSrc *ResourceInstanceC
 	s := changeSrc.DeepCopy()
 	cs.changes.Resources = append(cs.changes.Resources, s)
 }
+
+// GetResourceInstanceChange searches the set of resource instance changes for
+// one matching the given address and generation, returning it if it exists.
+//
+// If no such change exists, nil is returned.
+//
+// The returned object is a deep copy of the change recorded in the plan, so
+// callers may mutate it although it's generally better (less confusing) to
+// treat planned changes as immutable after they've been initially constructed.
+func (cs *ChangesSync) GetResourceInstanceChange(addr addrs.AbsResourceInstance, gen states.Generation) *ResourceInstanceChangeSrc {
+	if cs == nil {
+		panic("GetResourceInstanceChange on nil ChangesSync")
+	}
+	cs.lock.Lock()
+	defer cs.lock.Unlock()
+
+	if gen == states.CurrentGen {
+		return cs.changes.ResourceInstance(addr)
+	}
+	if dk, ok := gen.(states.DeposedKey); ok {
+		return cs.changes.ResourceInstanceDeposed(addr, dk)
+	}
+	panic(fmt.Sprintf("unsupported generation value %#v", gen))
+}

terraform/eval_diff.go

@@ -703,30 +703,44 @@ func (n *EvalDiffDestroyModule) Eval(ctx EvalContext) (interface{}, error) {
 // EvalReadDiff is an EvalNode implementation that retrieves the planned
 // change for a particular resource instance object.
 type EvalReadDiff struct {
-	Addr       addrs.ResourceInstance
+	Addr           addrs.ResourceInstance
-	DeposedKey states.DeposedKey
+	DeposedKey     states.DeposedKey
-	Change     **plans.ResourceInstanceChange
+	ProviderSchema **ProviderSchema
+	Change         **plans.ResourceInstanceChange
 }
 
 func (n *EvalReadDiff) Eval(ctx EvalContext) (interface{}, error) {
-	return nil, fmt.Errorf("EvalReadDiff not yet updated for new plan types")
+	providerSchema := *n.ProviderSchema
-	/*
+	changes := ctx.Changes()
-		diff, lock := ctx.Diff()
+	addr := n.Addr.Absolute(ctx.Path())
 
-		// Acquire the lock so that we can do this safely concurrently
+	schema := providerSchema.ResourceTypes[n.Addr.Resource.Type]
-		lock.Lock()
+	if schema == nil {
-		defer lock.Unlock()
+		// Should be caught during validation, so we don't bother with a pretty error here
-
+		return nil, fmt.Errorf("provider does not support resource type %q", n.Addr.Resource.Type)
-		// Write the diff
+	}
-		modDiff := diff.ModuleByPath(ctx.Path())
-		if modDiff == nil {
-			return nil, nil
-		}
-
-		*n.Diff = modDiff.Resources[n.Name]
 
+	gen := states.CurrentGen
+	if n.DeposedKey != states.NotDeposed {
+		gen = n.DeposedKey
+	}
+	csrc := changes.GetResourceInstanceChange(addr, gen)
+	if csrc == nil {
+		log.Printf("[TRACE] EvalReadDiff: No planned change recorded for %s", addr)
 		return nil, nil
-	*/
+	}
+
+	change, err := csrc.Decode(schema.ImpliedType())
+	if err != nil {
+		return nil, fmt.Errorf("failed to decode planned changes for %s: %s", addr, err)
+	}
+	if n.Change != nil {
+		*n.Change = change
+	}
+
+	log.Printf("[TRACE] EvalReadDiff: Read %s change from plan for %s", change.Action, addr)
+
+	return nil, nil
 }
 
 // EvalWriteDiff is an EvalNode implementation that saves a planned change
@@ -742,6 +756,7 @@ type EvalWriteDiff struct {
 func (n *EvalWriteDiff) Eval(ctx EvalContext) (interface{}, error) {
 	providerSchema := *n.ProviderSchema
 	change := *n.Change
+	addr := n.Addr.Absolute(ctx.Path())
 
 	if change.Addr.String() != n.Addr.String() || change.DeposedKey != n.DeposedKey {
 		// Should never happen, and indicates a bug in the caller.
@@ -756,15 +771,15 @@ func (n *EvalWriteDiff) Eval(ctx EvalContext) (interface{}, error) {
 
 	csrc, err := change.Encode(schema.ImpliedType())
 	if err != nil {
-		return nil, fmt.Errorf("failed to encode planned changes for %s: %s", n.Addr, err)
+		return nil, fmt.Errorf("failed to encode planned changes for %s: %s", addr, err)
 	}
 
 	changes := ctx.Changes()
 	changes.AppendResourceInstanceChange(csrc)
 	if n.DeposedKey == states.NotDeposed {
-		log.Printf("[TRACE] EvalWriteDiff: recorded %s change for %s", change.Action, n.Addr)
+		log.Printf("[TRACE] EvalWriteDiff: recorded %s change for %s", change.Action, addr)
 	} else {
-		log.Printf("[TRACE] EvalWriteDiff: recorded %s change for %s deposed object %s", change.Action, n.Addr, n.DeposedKey)
+		log.Printf("[TRACE] EvalWriteDiff: recorded %s change for %s deposed object %s", change.Action, addr, n.DeposedKey)
 	}
 
 	return nil, nil

terraform/node_resource_apply.go

@@ -129,10 +129,17 @@ func (n *NodeApplyableResourceInstance) evalTreeDataResource(addr addrs.AbsResou
 
 	return &EvalSequence{
 		Nodes: []EvalNode{
+			&EvalGetProvider{
+				Addr:   n.ResolvedProvider,
+				Output: &provider,
+				Schema: &providerSchema,
+			},
+
 			// Get the saved diff for apply
 			&EvalReadDiff{
-				Addr:   addr.Resource,
+				Addr:           addr.Resource,
-				Change: &change,
+				ProviderSchema: &providerSchema,
+				Change:         &change,
 			},
 
 			// Stop early if we don't actually have a diff
@@ -146,12 +153,6 @@ func (n *NodeApplyableResourceInstance) evalTreeDataResource(addr addrs.AbsResou
 				Then: EvalNoop{},
 			},
 
-			&EvalGetProvider{
-				Addr:   n.ResolvedProvider,
-				Output: &provider,
-				Schema: &providerSchema,
-			},
-
 			// Make a new diff, in case we've learned new values in the state
 			// during apply which we can now incorporate.
 			&EvalReadDataDiff{
@@ -206,10 +207,17 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe
 
 	return &EvalSequence{
 		Nodes: []EvalNode{
+			&EvalGetProvider{
+				Addr:   n.ResolvedProvider,
+				Output: &provider,
+				Schema: &providerSchema,
+			},
+
 			// Get the saved diff for apply
 			&EvalReadDiff{
-				Addr:   addr.Resource,
+				Addr:           addr.Resource,
-				Change: &diffApply,
+				ProviderSchema: &providerSchema,
+				Change:         &diffApply,
 			},
 
 			// We don't want to do any destroys
@@ -244,11 +252,6 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe
 				},
 			},
 
-			&EvalGetProvider{
-				Addr:   n.ResolvedProvider,
-				Output: &provider,
-				Schema: &providerSchema,
-			},
 			&EvalReadState{
 				Addr:           addr.Resource,
 				Provider:       &provider,
@@ -273,16 +276,18 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe
 
 			// Get the saved diff
 			&EvalReadDiff{
-				Addr:   addr.Resource,
+				Addr:           addr.Resource,
-				Change: &diff,
+				ProviderSchema: &providerSchema,
+				Change:         &diff,
 			},
 
 			// Compare the diffs
 			&EvalCheckPlannedChange{
-				Addr:         addr.Resource,
+				Addr:           addr.Resource,
-				ProviderAddr: n.ResolvedProvider,
+				ProviderAddr:   n.ResolvedProvider,
-				Planned:      &diff,
+				ProviderSchema: &providerSchema,
-				Actual:       &diffApply,
+				Planned:        &diff,
+				Actual:         &diffApply,
 			},
 
 			&EvalGetProvider{
@@ -305,14 +310,16 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe
 				Change: &diffApply,
 			},
 			&EvalApply{
-				Addr:      addr.Resource,
+				Addr:           addr.Resource,
-				Config:    n.Config,
+				Config:         n.Config,
-				State:     &state,
+				State:          &state,
-				Change:    &diffApply,
+				Change:         &diffApply,
-				Provider:  &provider,
+				Provider:       &provider,
-				Output:    &state,
+				ProviderAddr:   n.ResolvedProvider,
-				Error:     &err,
+				ProviderSchema: &providerSchema,
-				CreateNew: &createNew,
+				Output:         &state,
+				Error:          &err,
+				CreateNew:      &createNew,
 			},
 			&EvalWriteState{
 				Addr:           addr.Resource,

terraform/node_resource_destroy.go

@@ -176,10 +176,17 @@ func (n *NodeDestroyResourceInstance) EvalTree() EvalNode {
 		Ops: []walkOperation{walkApply, walkDestroy},
 		Node: &EvalSequence{
 			Nodes: []EvalNode{
+				&EvalGetProvider{
+					Addr:   n.ResolvedProvider,
+					Output: &provider,
+					Schema: &providerSchema,
+				},
+
 				// Get the saved diff for apply
 				&EvalReadDiff{
-					Addr:   addr.Resource,
+					Addr:           addr.Resource,
-					Change: &changeApply,
+					ProviderSchema: &providerSchema,
+					Change:         &changeApply,
 				},
 
 				// If we're not destroying, then compare diffs
@@ -196,11 +203,6 @@ func (n *NodeDestroyResourceInstance) EvalTree() EvalNode {
 					Then: EvalNoop{},
 				},
 
-				&EvalGetProvider{
-					Addr:   n.ResolvedProvider,
-					Output: &provider,
-					Schema: &providerSchema,
-				},
 				&EvalReadState{
 					Addr:           addr.Resource,
 					Output:         &state,
@@ -264,13 +266,15 @@ func (n *NodeDestroyResourceInstance) EvalTree() EvalNode {
 						Output:   &state,
 					},
 					Else: &EvalApply{
-						Addr:     addr.Resource,
+						Addr:           addr.Resource,
-						Config:   nil, // No configuration because we are destroying
+						Config:         nil, // No configuration because we are destroying
-						State:    &state,
+						State:          &state,
-						Change:   &changeApply,
+						Change:         &changeApply,
-						Provider: &provider,
+						Provider:       &provider,
-						Output:   &state,
+						ProviderAddr:   n.ResolvedProvider,
-						Error:    &err,
+						ProviderSchema: &providerSchema,
+						Output:         &state,
+						Error:          &err,
 					},
 				},
 				&EvalWriteState{

terraform/transform_deposed.go

@@ -151,13 +151,15 @@ func (n *graphNodeDeposedResource) EvalTree() EvalNode {
 					Change: &change,
 				},
 				&EvalApply{
-					Addr:     addr.Resource,
+					Addr:           addr.Resource,
-					Config:   nil, // No configuration because we are destroying
+					Config:         nil, // No configuration because we are destroying
-					State:    &state,
+					State:          &state,
-					Change:   &change,
+					Change:         &change,
-					Provider: &provider,
+					Provider:       &provider,
-					Output:   &state,
+					ProviderAddr:   n.ResolvedProvider,
-					Error:    &err,
+					ProviderSchema: &providerSchema,
+					Output:         &state,
+					Error:          &err,
 				},
 				// Always write the resource back to the state deposed... if it
 				// was successfully destroyed it will be pruned. If it was not, it will