From 1ee69761d41de4a7dc8382a4c3af60f3d6d62c00 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 24 Jul 2014 07:58:45 -0700
Subject: [PATCH] terraform: self-referencing variables in provisoiners work

---
 terraform/context.go                          | 25 +++++-----
 terraform/context_test.go                     | 46 +++++++++++++++++++
 terraform/graph.go                            | 20 ++++++--
 terraform/terraform_test.go                   |  7 +++
 .../apply-provisioner-resource-ref/main.tf    |  7 +++
 5 files changed, 89 insertions(+), 16 deletions(-)
 create mode 100644 terraform/test-fixtures/apply-provisioner-resource-ref/main.tf

diff --git a/terraform/context.go b/terraform/context.go
index 0e7dbcc7e..d088da574 100644
--- a/terraform/context.go
+++ b/terraform/context.go
@@ -567,6 +567,16 @@ func (c *Context) applyWalkFn() depgraph.WalkFunc {
 			}
 		}
 
+		// Update the resulting diff
+		c.sl.Lock()
+		if rs.ID == "" {
+			delete(c.state.Resources, r.Id)
+			delete(c.state.Tainted, r.Id)
+		} else {
+			c.state.Resources[r.Id] = rs
+		}
+		c.sl.Unlock()
+
 		// Invoke any provisioners we have defined. This is only done
 		// if the resource was created, as updates or deletes do not
 		// invoke provisioners.
@@ -581,18 +591,11 @@ func (c *Context) applyWalkFn() depgraph.WalkFunc {
 			}
 		}
 
-		// Update the resulting diff
-		c.sl.Lock()
-		if rs.ID == "" {
-			delete(c.state.Resources, r.Id)
-		} else {
-			c.state.Resources[r.Id] = rs
-
-			if tainted {
-				c.state.Tainted[r.Id] = struct{}{}
-			}
+		if tainted {
+			c.sl.Lock()
+			c.state.Tainted[r.Id] = struct{}{}
+			c.sl.Unlock()
 		}
-		c.sl.Unlock()
 
 		// Update the state for the resource itself
 		r.State = rs
diff --git a/terraform/context_test.go b/terraform/context_test.go
index 6abdbbb24..3516bc5cd 100644
--- a/terraform/context_test.go
+++ b/terraform/context_test.go
@@ -550,6 +550,52 @@ func TestContextApply_provisionerFail(t *testing.T) {
 	}
 }
 
+func TestContextApply_provisionerResourceRef(t *testing.T) {
+	c := testConfig(t, "apply-provisioner-resource-ref")
+	p := testProvider("aws")
+	pr := testProvisioner()
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+	pr.ApplyFn = func(rs *ResourceState, c *ResourceConfig) error {
+		val, ok := c.Config["foo"]
+		if !ok || val != "2" {
+			t.Fatalf("bad value for foo: %v %#v", val, c)
+		}
+
+		return nil
+	}
+
+	ctx := testContext(t, &ContextOpts{
+		Config: c,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		Provisioners: map[string]ResourceProvisionerFactory{
+			"shell": testProvisionerFuncFixed(pr),
+		},
+	})
+
+	if _, err := ctx.Plan(nil); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state, err := ctx.Apply()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(state.String())
+	expected := strings.TrimSpace(testTerraformApplyProvisionerResourceRefStr)
+	if actual != expected {
+		t.Fatalf("bad: \n%s", actual)
+	}
+
+	// Verify apply was invoked
+	if !pr.ApplyCalled {
+		t.Fatalf("provisioner not invoked")
+	}
+}
+
 func TestContextApply_outputDiffVars(t *testing.T) {
 	c := testConfig(t, "apply-good")
 	p := testProvider("aws")
diff --git a/terraform/graph.go b/terraform/graph.go
index de0be65a8..5c9dfdfa2 100644
--- a/terraform/graph.go
+++ b/terraform/graph.go
@@ -611,20 +611,20 @@ func graphAddVariableDeps(g *depgraph.Graph) {
 
 			// Handle the resource variables
 			vars = m.Config.RawConfig.Variables
-			nounAddVariableDeps(g, n, vars)
+			nounAddVariableDeps(g, n, vars, false)
 
 			// Handle the variables of the resource provisioners
 			for _, p := range m.Resource.Provisioners {
 				vars = p.RawConfig.Variables
-				nounAddVariableDeps(g, n, vars)
+				nounAddVariableDeps(g, n, vars, true)
 
 				vars = p.ConnInfo.Variables
-				nounAddVariableDeps(g, n, vars)
+				nounAddVariableDeps(g, n, vars, true)
 			}
 
 		case *GraphNodeResourceProvider:
 			vars = m.Config.RawConfig.Variables
-			nounAddVariableDeps(g, n, vars)
+			nounAddVariableDeps(g, n, vars, false)
 
 		default:
 			continue
@@ -634,7 +634,11 @@ func graphAddVariableDeps(g *depgraph.Graph) {
 
 // nounAddVariableDeps updates the dependencies of a noun given
 // a set of associated variable values
-func nounAddVariableDeps(g *depgraph.Graph, n *depgraph.Noun, vars map[string]config.InterpolatedVariable) {
+func nounAddVariableDeps(
+	g *depgraph.Graph,
+	n *depgraph.Noun,
+	vars map[string]config.InterpolatedVariable,
+	removeSelf bool) {
 	for _, v := range vars {
 		// Only resource variables impose dependencies
 		rv, ok := v.(*config.ResourceVariable)
@@ -648,6 +652,12 @@ func nounAddVariableDeps(g *depgraph.Graph, n *depgraph.Noun, vars map[string]co
 			continue
 		}
 
+		// If we're ignoring self-references, then don't add that
+		// dependency.
+		if removeSelf && n == target {
+			continue
+		}
+
 		// Build the dependency
 		dep := &depgraph.Dependency{
 			Name:   rv.ResourceId(),
diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go
index d9f9027fd..05be1c85a 100644
--- a/terraform/terraform_test.go
+++ b/terraform/terraform_test.go
@@ -139,6 +139,13 @@ aws_instance.foo:
   type = aws_instance
 `
 
+const testTerraformApplyProvisionerResourceRefStr = `
+aws_instance.bar:
+  ID = foo
+  num = 2
+  type = aws_instance
+`
+
 const testTerraformApplyDestroyStr = `
 <no state>
 `
diff --git a/terraform/test-fixtures/apply-provisioner-resource-ref/main.tf b/terraform/test-fixtures/apply-provisioner-resource-ref/main.tf
new file mode 100644
index 000000000..056a6304a
--- /dev/null
+++ b/terraform/test-fixtures/apply-provisioner-resource-ref/main.tf
@@ -0,0 +1,7 @@
+resource "aws_instance" "bar" {
+    num = "2"
+
+    provisioner "shell" {
+        foo = "${aws_instance.bar.num}"
+    }
+}