From cd0c4522eed540baf40e13e7c7fdb1b2195ecb2a Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Tue, 17 May 2016 07:58:11 -0700
Subject: [PATCH] core: honor "destroy" diffs for data resources

Previously the "planDestroy" pass would correctly produce a destroy diff,
but the "apply" pass would just ignore it and make a fresh diff, turning
it back into a "create" because data resources are always eager to
refresh.

Now we consider the previous diff when re-diffing during apply and so
we can preserve the plan to destroy and then ultimately actually "destroy"
the data resource (remove from the state) when we get to ReadDataApply.

This ensures that the state is left empty after "terraform destroy";
previously we would leave behind data resource states.
---
 terraform/eval_read_data.go     | 47 +++++++++++++++++++++------------
 terraform/transform_resource.go |  1 +
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/terraform/eval_read_data.go b/terraform/eval_read_data.go
index 9625bcffa..61fa3e125 100644
--- a/terraform/eval_read_data.go
+++ b/terraform/eval_read_data.go
@@ -12,12 +12,14 @@ type EvalReadDataDiff struct {
 	OutputState **InstanceState
 	Config      **ResourceConfig
 	Info        *InstanceInfo
+
+	// Set Previous when re-evaluating diff during apply, to ensure that
+	// the "Destroy" flag is preserved.
+	Previous **InstanceDiff
 }
 
 func (n *EvalReadDataDiff) Eval(ctx EvalContext) (interface{}, error) {
 	// TODO: test
-	provider := *n.Provider
-	config := *n.Config
 
 	err := ctx.Hook(func(h Hook) (HookAction, error) {
 		return h.PreDiff(n.Info, nil)
@@ -26,21 +28,32 @@ func (n *EvalReadDataDiff) Eval(ctx EvalContext) (interface{}, error) {
 		return nil, err
 	}
 
-	diff, err := provider.ReadDataDiff(n.Info, config)
-	if err != nil {
-		return nil, err
-	}
-	if diff == nil {
-		diff = new(InstanceDiff)
-	}
+	var diff *InstanceDiff
 
-	// id is always computed, because we're always "creating a new resource"
-	diff.init()
-	diff.Attributes["id"] = &ResourceAttrDiff{
-		Old:         "",
-		NewComputed: true,
-		RequiresNew: true,
-		Type:        DiffAttrOutput,
+	if n.Previous != nil && *n.Previous != nil && (*n.Previous).Destroy {
+		// If we're re-diffing for a diff that was already planning to
+		// destroy, then we'll just continue with that plan.
+		diff = &InstanceDiff{Destroy: true}
+	} else {
+		provider := *n.Provider
+		config := *n.Config
+
+		diff, err := provider.ReadDataDiff(n.Info, config)
+		if err != nil {
+			return nil, err
+		}
+		if diff == nil {
+			diff = new(InstanceDiff)
+		}
+
+		// id is always computed, because we're always "creating a new resource"
+		diff.init()
+		diff.Attributes["id"] = &ResourceAttrDiff{
+			Old:         "",
+			NewComputed: true,
+			RequiresNew: true,
+			Type:        DiffAttrOutput,
+		}
 	}
 
 	err = ctx.Hook(func(h Hook) (HookAction, error) {
@@ -83,7 +96,7 @@ func (n *EvalReadDataApply) Eval(ctx EvalContext) (interface{}, error) {
 	// If the diff is for *destroying* this resource then we'll
 	// just drop its state and move on, since data resources don't
 	// support an actual "destroy" action.
-	if diff.Destroy {
+	if diff != nil && diff.Destroy {
 		if n.Output != nil {
 			*n.Output = nil
 		}
diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go
index 14cd54f54..dccc3cd94 100644
--- a/terraform/transform_resource.go
+++ b/terraform/transform_resource.go
@@ -810,6 +810,7 @@ func (n *graphNodeExpandedResource) dataResourceEvalNodes(resource *Resource, in
 				&EvalReadDataDiff{
 					Info:     info,
 					Config:   &config,
+					Previous: &diff,
 					Provider: &provider,
 					Output:   &diff,
 				},