From d5e3d5a196209019c8b52f43f7223d8a2e0760cc Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Tue, 22 Jul 2014 19:32:46 -0700
Subject: [PATCH] terraform: validate diffs are the same

---
 terraform/context.go      | 30 +++++++++++++++++++++++-------
 terraform/context_test.go | 29 +++++++++++++++++++++++++++++
 terraform/diff.go         | 32 +++++++++++++++++++++++++++++++-
 3 files changed, 83 insertions(+), 8 deletions(-)

diff --git a/terraform/context.go b/terraform/context.go
index 9ef0050d84..24710cc114 100644
--- a/terraform/context.go
+++ b/terraform/context.go
@@ -485,14 +485,30 @@ func (c *Context) applyWalkFn() depgraph.WalkFunc {
 			if err != nil {
 				return err
 			}
-		}
 
-		// This should never happen because we check if Diff.Empty above.
-		// If this happened, then the diff above returned a bad diff.
-		if diff == nil {
-			return fmt.Errorf(
-				"%s: diff became nil during Apply. This is a bug with " +
-					"the resource provider. Please report a bug.")
+			// This should never happen because we check if Diff.Empty above.
+			// If this happened, then the diff above returned a bad diff.
+			if diff == nil {
+				return fmt.Errorf(
+					"%s: diff became nil during Apply. This is a bug with " +
+						"the resource provider. Please report a bug.")
+			}
+
+			// Delete id from the diff because it is dependent on
+			// our internal plan function.
+			delete(r.Diff.Attributes, "id")
+			delete(diff.Attributes, "id")
+
+			// Verify the diffs are the same
+			if !r.Diff.Same(diff) {
+				log.Printf(
+					"[ERROR] Diffs don't match.\n\nDiff 1: %#v"+
+						"\n\nDiff 2: %#v",
+					r.Diff, diff)
+				return fmt.Errorf(
+					"%s: diffs didn't match during apply. This is a " +
+						"bug with the resource provider, please report a bug.")
+			}
 		}
 
 		// If we do not have any connection info, initialize
diff --git a/terraform/context_test.go b/terraform/context_test.go
index 5560248e9b..6abdbbb243 100644
--- a/terraform/context_test.go
+++ b/terraform/context_test.go
@@ -309,6 +309,35 @@ func TestContextApply_Minimal(t *testing.T) {
 	}
 }
 
+func TestContextApply_badDiff(t *testing.T) {
+	c := testConfig(t, "apply-good")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+	ctx := testContext(t, &ContextOpts{
+		Config: c,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+	})
+
+	if _, err := ctx.Plan(nil); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	p.DiffFn = func(*ResourceState, *ResourceConfig) (*ResourceDiff, error) {
+		return &ResourceDiff{
+			Attributes: map[string]*ResourceAttrDiff{
+				"newp": nil,
+			},
+		}, nil
+	}
+
+	if _, err := ctx.Apply(); err == nil {
+		t.Fatal("should error")
+	}
+}
+
 func TestContextApply_cancel(t *testing.T) {
 	stopped := false
 
diff --git a/terraform/diff.go b/terraform/diff.go
index 7a08fb8516..706197a2b2 100644
--- a/terraform/diff.go
+++ b/terraform/diff.go
@@ -209,10 +209,40 @@ func (d *ResourceDiff) RequiresNew() bool {
 	}
 
 	for _, rd := range d.Attributes {
-		if rd.RequiresNew {
+		if rd != nil && rd.RequiresNew {
 			return true
 		}
 	}
 
 	return false
 }
+
+// Same checks whether or not to ResourceDiffs are the "same." When
+// we say "same", it is not necessarily exactly equal. Instead, it is
+// just checking that the same attributes are changing, a destroy
+// isn't suddenly happening, etc.
+func (d *ResourceDiff) Same(d2 *ResourceDiff) bool {
+	if d.Destroy != d2.Destroy {
+		return false
+	}
+	if d.RequiresNew() != d2.RequiresNew() {
+		return false
+	}
+	if len(d.Attributes) != len(d2.Attributes) {
+		return false
+	}
+
+	ks := make(map[string]struct{})
+	for k, _ := range d.Attributes {
+		ks[k] = struct{}{}
+	}
+	for k, _ := range d2.Attributes {
+		delete(ks, k)
+	}
+
+	if len(ks) > 0 {
+		return false
+	}
+
+	return true
+}