From 89f7e3b79fc7e2cca22e02bd9eae143302dbe260 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Tue, 13 Dec 2016 21:22:21 -0800
Subject: [PATCH] terraform: add module vars after providers to see references

Fixes #10711

The `ModuleVariablesTransformer` only adds module variables in use. This
was missing module variables used by providers since we ran the provider
too late. This moves the transformer and adds a test for this.
---
 terraform/context_plan_test.go                | 23 +++++++++++++++++++
 terraform/graph_builder_plan.go               | 18 ++++++---------
 terraform/terraform_test.go                   | 13 +++++++++++
 .../plan-module-provider-var/child/main.tf    |  9 ++++++++
 .../plan-module-provider-var/main.tf          |  6 +++++
 terraform/transform_module_variable.go        |  2 +-
 6 files changed, 59 insertions(+), 12 deletions(-)
 create mode 100644 terraform/test-fixtures/plan-module-provider-var/child/main.tf
 create mode 100644 terraform/test-fixtures/plan-module-provider-var/main.tf

diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go
index 4902def11..f6d310761 100644
--- a/terraform/context_plan_test.go
+++ b/terraform/context_plan_test.go
@@ -657,6 +657,29 @@ func TestContext2Plan_moduleProviderDefaultsVar(t *testing.T) {
 	}
 }
 
+func TestContext2Plan_moduleProviderVar(t *testing.T) {
+	m := testModule(t, "plan-module-provider-var")
+	p := testProvider("aws")
+	p.DiffFn = testDiffFn
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+	})
+
+	plan, err := ctx.Plan()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(plan.String())
+	expected := strings.TrimSpace(testTerraformPlanModuleProviderVarStr)
+	if actual != expected {
+		t.Fatalf("bad:\n%s", actual)
+	}
+}
+
 func TestContext2Plan_moduleVar(t *testing.T) {
 	m := testModule(t, "plan-module-var")
 	p := testProvider("aws")
diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go
index 640245fc6..35961ee46 100644
--- a/terraform/graph_builder_plan.go
+++ b/terraform/graph_builder_plan.go
@@ -92,6 +92,13 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
 		// Add root variables
 		&RootVariableTransformer{Module: b.Module},
 
+		// Create all the providers
+		&MissingProviderTransformer{Providers: b.Providers, Concrete: concreteProvider},
+		&ProviderTransformer{},
+		&DisableProviderTransformer{},
+		&ParentProviderTransformer{},
+		&AttachProviderConfigTransformer{Module: b.Module},
+
 		// Add module variables
 		&ModuleVariableTransformer{Module: b.Module},
 
@@ -102,17 +109,6 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
 		// Target
 		&TargetsTransformer{Targets: b.Targets},
 
-		// Create all the providers
-		&MissingProviderTransformer{Providers: b.Providers, Concrete: concreteProvider},
-		&ProviderTransformer{},
-		&DisableProviderTransformer{},
-		&ParentProviderTransformer{},
-		&AttachProviderConfigTransformer{Module: b.Module},
-
-		// Connect references again to connect the providers, module variables,
-		// etc. This is idempotent.
-		&ReferenceTransformer{},
-
 		// Single root
 		&RootTransformer{},
 	}
diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go
index 023da95b1..142123e53 100644
--- a/terraform/terraform_test.go
+++ b/terraform/terraform_test.go
@@ -1381,6 +1381,19 @@ module.child:
     ID = baz
 `
 
+const testTerraformPlanModuleProviderVarStr = `
+DIFF:
+
+module.child:
+  CREATE: aws_instance.test
+    type:  "" => "aws_instance"
+    value: "" => "hello"
+
+STATE:
+
+<no state>
+`
+
 const testTerraformPlanModuleVarStr = `
 DIFF:
 
diff --git a/terraform/test-fixtures/plan-module-provider-var/child/main.tf b/terraform/test-fixtures/plan-module-provider-var/child/main.tf
new file mode 100644
index 000000000..599cb99db
--- /dev/null
+++ b/terraform/test-fixtures/plan-module-provider-var/child/main.tf
@@ -0,0 +1,9 @@
+variable "foo" {}
+
+provider "aws" {
+    value = "${var.foo}"
+}
+
+resource "aws_instance" "test" {
+    value = "hello"
+}
diff --git a/terraform/test-fixtures/plan-module-provider-var/main.tf b/terraform/test-fixtures/plan-module-provider-var/main.tf
new file mode 100644
index 000000000..57e8faec5
--- /dev/null
+++ b/terraform/test-fixtures/plan-module-provider-var/main.tf
@@ -0,0 +1,6 @@
+variable "foo" { default = "bar" }
+
+module "child" {
+    source = "./child"
+    foo    = "${var.foo}"
+}
diff --git a/terraform/transform_module_variable.go b/terraform/transform_module_variable.go
index 50f00bff1..467950bdc 100644
--- a/terraform/transform_module_variable.go
+++ b/terraform/transform_module_variable.go
@@ -11,7 +11,7 @@ import (
 // ModuleVariableTransformer is a GraphTransformer that adds all the variables
 // in the configuration to the graph.
 //
-// This only adds variables that are referenced by other thigns in the graph.
+// This only adds variables that are referenced by other things in the graph.
 // If a module variable is not referenced, it won't be added to the graph.
 type ModuleVariableTransformer struct {
 	Module *module.Tree