From 970e7c192330ad6a4b3ab5de923ba681e11740f9 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Mon, 20 Mar 2017 17:17:14 -0400
Subject: [PATCH] Add a failing test for missing keys in diff

ignore_changes is causing changes in other flatmapped sets to be
filtered out incorrectly.

This required fixing the testDiffFn to create diffs which include the
old value, breaking one other test.
---
 terraform/context_plan_test.go                | 50 +++++++++++++++++++
 terraform/context_test.go                     |  5 ++
 terraform/terraform_test.go                   | 11 ++++
 .../plan-ignore-changes-with-flatmaps/main.tf | 16 ++++++
 4 files changed, 82 insertions(+)
 create mode 100644 terraform/test-fixtures/plan-ignore-changes-with-flatmaps/main.tf

diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go
index bf3ff4f41..e27b90c40 100644
--- a/terraform/context_plan_test.go
+++ b/terraform/context_plan_test.go
@@ -3095,3 +3095,53 @@ func TestContext2Plan_listOrder(t *testing.T) {
 		t.Fatal("aws_instance.a and aws_instance.b diffs should match:\n", plan)
 	}
 }
+
+// Make sure ignore-changes doesn't interfere with set/list/map diffs.
+// If a resource was being replaced by a RequiresNew attribute that gets
+// ignores, we need to filter out the diff properly.
+func TestContext2Plan_ignoreChangesWithFlatmaps(t *testing.T) {
+	m := testModule(t, "plan-ignore-changes-with-flatmaps")
+	p := testProvider("aws")
+	p.DiffFn = testDiffFn
+	s := &State{
+		Modules: []*ModuleState{
+			&ModuleState{
+				Path: rootModulePath,
+				Resources: map[string]*ResourceState{
+					"aws_instance.foo": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID: "bar",
+							Attributes: map[string]string{
+								"user_data":   "x",
+								"require_new": "",
+								"set.#":       "1",
+								"set.0.a":     "1",
+								"lst.#":       "1",
+								"lst.0":       "j",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		State: s,
+	})
+
+	plan, err := ctx.Plan()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(plan.Diff.String())
+	expected := strings.TrimSpace(testTFPlanDiffIgnoreChangesWithFlatmaps)
+	if actual != expected {
+		t.Fatalf("bad:\n%s\n\nexpected\n\n%s", actual, expected)
+	}
+}
diff --git a/terraform/context_test.go b/terraform/context_test.go
index 91babd75f..3534e9aa3 100644
--- a/terraform/context_test.go
+++ b/terraform/context_test.go
@@ -262,6 +262,11 @@ func testDiffFn(
 			if _, ok := c.Raw["__"+k+"_requires_new"]; ok {
 				attrDiff.RequiresNew = true
 			}
+
+			if attr, ok := s.Attributes[k]; ok {
+				attrDiff.Old = attr
+			}
+
 			diff.Attributes[k] = attrDiff
 		}
 	}
diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go
index f1075b7f1..1c5eb8717 100644
--- a/terraform/terraform_test.go
+++ b/terraform/terraform_test.go
@@ -1570,6 +1570,17 @@ aws_instance.foo:
   ami = ami-abcd1234
 `
 
+const testTFPlanDiffIgnoreChangesWithFlatmaps = `
+UPDATE: aws_instance.foo
+  lst.#:   "1" => "2"
+  lst.0:   "j" => "j"
+  lst.1:   "" => "k"
+  set.#:   "1" => "1"
+  set.0.a: "1" => "1"
+  set.0.b: "" => "2"
+  type:    "" => "aws_instance"
+`
+
 const testTerraformPlanIgnoreChangesWildcardStr = `
 DIFF:
 
diff --git a/terraform/test-fixtures/plan-ignore-changes-with-flatmaps/main.tf b/terraform/test-fixtures/plan-ignore-changes-with-flatmaps/main.tf
new file mode 100644
index 000000000..49885194e
--- /dev/null
+++ b/terraform/test-fixtures/plan-ignore-changes-with-flatmaps/main.tf
@@ -0,0 +1,16 @@
+resource "aws_instance" "foo" {
+  id = "bar"
+  user_data = "x"
+  require_new = "yes"
+
+  set = {
+	  a = "1"
+	  b = "2"
+  }
+
+  lst = ["j", "k"]
+
+  lifecycle {
+    ignore_changes = ["require_new"]
+  }
+}