From d8e996436313c032376f3429050a09968aff960e Mon Sep 17 00:00:00 2001
From: Alisdair McDiarmid <alisdair@users.noreply.github.com>
Date: Mon, 17 Aug 2020 17:03:25 -0400
Subject: [PATCH] terraform: Eval module call arguments for import

Include the import walk in the list of operations for which we create an
EvalModuleCallArgument node. This causes module call arguments to be
evaluated even if the module variables have defaults, ensuring that
invalid default values (such as the common "{}" for variables thought of
as maps) do not cause failures specific to import.

This fixes a bug where a child module evaluates an input variable in its
locals block, assuming that it is a nested object structure. The bug
report includes a default value of "{}", which is overridden by a root
variable value. Without the eval node added in this commit, the default
value is used and the local evaluation errors.
---
 command/import_test.go                        | 70 +++++++++++++++++++
 .../child/main.tf                             | 11 +++
 .../import-module-input-variable/main.tf      |  8 +++
 .../terraform.tfvars                          |  1 +
 terraform/node_module_variable.go             |  2 +-
 5 files changed, 91 insertions(+), 1 deletion(-)
 create mode 100644 command/testdata/import-module-input-variable/child/main.tf
 create mode 100644 command/testdata/import-module-input-variable/main.tf
 create mode 100644 command/testdata/import-module-input-variable/terraform.tfvars

diff --git a/command/import_test.go b/command/import_test.go
index 67277c1ec..21174727c 100644
--- a/command/import_test.go
+++ b/command/import_test.go
@@ -802,6 +802,76 @@ func TestImportModuleVarFile(t *testing.T) {
 	}
 }
 
+// This test covers an edge case where a module with a complex input variable
+// of nested objects has an invalid default which is overridden by the calling
+// context, and is used in locals. If we don't evaluate module call variables
+// for the import walk, this results in an error.
+//
+// The specific example has a variable "foo" which is a nested object:
+//
+//   foo = { bar = { baz = true } }
+//
+// This is used as foo = var.foo in the call to the child module, which then
+// uses the traversal foo.bar.baz in a local. A default value in the child
+// module of {} causes this local evaluation to error, breaking import.
+func TestImportModuleInputVariableEvaluation(t *testing.T) {
+	td := tempDir(t)
+	copy.CopyDir(testFixturePath("import-module-input-variable"), td)
+	defer os.RemoveAll(td)
+	defer testChdir(t, td)()
+
+	statePath := testTempFile(t)
+
+	p := testProvider()
+	p.GetSchemaReturn = &terraform.ProviderSchema{
+		ResourceTypes: map[string]*configschema.Block{
+			"test_instance": {
+				Attributes: map[string]*configschema.Attribute{
+					"foo": {Type: cty.String, Optional: true},
+				},
+			},
+		},
+	}
+
+	providerSource, close := newMockProviderSource(t, map[string][]string{
+		"test": {"1.2.3"},
+	})
+	defer close()
+
+	// init to install the module
+	ui := new(cli.MockUi)
+	m := Meta{
+		testingOverrides: metaOverridesForProvider(testProvider()),
+		Ui:               ui,
+		ProviderSource:   providerSource,
+	}
+
+	ic := &InitCommand{
+		Meta: m,
+	}
+	if code := ic.Run([]string{}); code != 0 {
+		t.Fatalf("init failed\n%s", ui.ErrorWriter)
+	}
+
+	// import
+	ui = new(cli.MockUi)
+	c := &ImportCommand{
+		Meta: Meta{
+			testingOverrides: metaOverridesForProvider(p),
+			Ui:               ui,
+		},
+	}
+	args := []string{
+		"-state", statePath,
+		"module.child.test_instance.foo",
+		"bar",
+	}
+	code := c.Run(args)
+	if code != 0 {
+		t.Fatalf("import failed; expected success")
+	}
+}
+
 func TestImport_dataResource(t *testing.T) {
 	defer testChdir(t, testFixturePath("import-missing-resource-config"))()
 
diff --git a/command/testdata/import-module-input-variable/child/main.tf b/command/testdata/import-module-input-variable/child/main.tf
new file mode 100644
index 000000000..6327b7c1e
--- /dev/null
+++ b/command/testdata/import-module-input-variable/child/main.tf
@@ -0,0 +1,11 @@
+variable "foo" {
+  default = {}
+}
+
+locals {
+  baz = var.foo.bar.baz
+}
+
+resource "test_instance" "foo" {
+    foo = local.baz
+}
diff --git a/command/testdata/import-module-input-variable/main.tf b/command/testdata/import-module-input-variable/main.tf
new file mode 100644
index 000000000..13334c11e
--- /dev/null
+++ b/command/testdata/import-module-input-variable/main.tf
@@ -0,0 +1,8 @@
+variable "foo" {
+  default = {}
+}
+
+module "child" {
+    source = "./child"
+    foo = var.foo
+}
diff --git a/command/testdata/import-module-input-variable/terraform.tfvars b/command/testdata/import-module-input-variable/terraform.tfvars
new file mode 100644
index 000000000..39ab5cb5d
--- /dev/null
+++ b/command/testdata/import-module-input-variable/terraform.tfvars
@@ -0,0 +1 @@
+foo = { bar = { baz = true } }
diff --git a/terraform/node_module_variable.go b/terraform/node_module_variable.go
index 5009f54d6..36742212b 100644
--- a/terraform/node_module_variable.go
+++ b/terraform/node_module_variable.go
@@ -154,7 +154,7 @@ func (n *nodeModuleVariable) EvalTree() EvalNode {
 		Nodes: []EvalNode{
 			&EvalOpFilter{
 				Ops: []walkOperation{walkRefresh, walkPlan, walkApply,
-					walkDestroy},
+					walkDestroy, walkImport},
 				Node: &EvalModuleCallArgument{
 					Addr:           n.Addr.Variable,
 					Config:         n.Config,