From 8111050c664078ff2bf198d3209ca74f7ce2bd8f Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Tue, 16 Jul 2019 18:58:40 -0400
Subject: [PATCH] ensure we record diagnostics from nested modules

When loading nested modules, the child module diagnostics were dropped
in the recursive function. This mean that the config from the submodules
wasn't fully loaded, even though no errors were reported to the user.

This caused further problems if the plan was stored in a plan file, when
means only the partial configuration was stored for the subsequent apply
operation, which would result in unexplained "Resource node has no
configuration attached" errors later on.

Also due to the child module diagnostics being lost, any newly added
nested modules would be silently ignored until `init` was run again
manually.
---
 configs/config_build.go                       |  1 +
 configs/config_build_test.go                  | 45 +++++++++++++++++++
 .../testdata/nested-errors/child_a/child_a.tf |  4 ++
 .../testdata/nested-errors/child_c/child_c.tf |  6 +++
 configs/testdata/nested-errors/root.tf        |  3 ++
 5 files changed, 59 insertions(+)
 create mode 100644 configs/testdata/nested-errors/child_a/child_a.tf
 create mode 100644 configs/testdata/nested-errors/child_c/child_c.tf
 create mode 100644 configs/testdata/nested-errors/root.tf

diff --git a/configs/config_build.go b/configs/config_build.go
index 948b2c8ff..1ca1d77e5 100644
--- a/configs/config_build.go
+++ b/configs/config_build.go
@@ -76,6 +76,7 @@ func buildChildModules(parent *Config, walker ModuleWalker) (map[string]*Config,
 		}
 
 		child.Children, modDiags = buildChildModules(child, walker)
+		diags = append(diags, modDiags...)
 
 		ret[call.Name] = child
 	}
diff --git a/configs/config_build_test.go b/configs/config_build_test.go
index 95abe94d2..b244ea4cf 100644
--- a/configs/config_build_test.go
+++ b/configs/config_build_test.go
@@ -69,3 +69,48 @@ func TestBuildConfig(t *testing.T) {
 		t.Fatalf("child_a.child_c is same object as child_b.child_c; should not be")
 	}
 }
+
+func TestBuildConfigDiags(t *testing.T) {
+	parser := NewParser(nil)
+	mod, diags := parser.LoadConfigDir("testdata/nested-errors")
+	assertNoDiagnostics(t, diags)
+	if mod == nil {
+		t.Fatal("got nil root module; want non-nil")
+	}
+
+	versionI := 0
+	cfg, diags := BuildConfig(mod, ModuleWalkerFunc(
+		func(req *ModuleRequest) (*Module, *version.Version, hcl.Diagnostics) {
+			// For the sake of this test we're going to just treat our
+			// SourceAddr as a path relative to our fixture directory.
+			// A "real" implementation of ModuleWalker should accept the
+			// various different source address syntaxes Terraform supports.
+			sourcePath := filepath.Join("testdata/nested-errors", req.SourceAddr)
+
+			mod, diags := parser.LoadConfigDir(sourcePath)
+			version, _ := version.NewVersion(fmt.Sprintf("1.0.%d", versionI))
+			versionI++
+			return mod, version, diags
+		},
+	))
+
+	wantDiag := `testdata/nested-errors/child_c/child_c.tf:5,1-8: ` +
+		`Unsupported block type; Blocks of type "invalid" are not expected here.`
+	assertExactDiagnostics(t, diags, []string{wantDiag})
+
+	// we should still have module structure loaded
+	var got []string
+	cfg.DeepEach(func(c *Config) {
+		got = append(got, fmt.Sprintf("%s %s", strings.Join(c.Path, "."), c.Version))
+	})
+	sort.Strings(got)
+	want := []string{
+		" <nil>",
+		"child_a 1.0.0",
+		"child_a.child_c 1.0.1",
+	}
+
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("wrong result\ngot: %swant: %s", spew.Sdump(got), spew.Sdump(want))
+	}
+}
diff --git a/configs/testdata/nested-errors/child_a/child_a.tf b/configs/testdata/nested-errors/child_a/child_a.tf
new file mode 100644
index 000000000..6c5759701
--- /dev/null
+++ b/configs/testdata/nested-errors/child_a/child_a.tf
@@ -0,0 +1,4 @@
+
+module "child_c" {
+  source = "child_c"
+}
diff --git a/configs/testdata/nested-errors/child_c/child_c.tf b/configs/testdata/nested-errors/child_c/child_c.tf
new file mode 100644
index 000000000..19ba443c8
--- /dev/null
+++ b/configs/testdata/nested-errors/child_c/child_c.tf
@@ -0,0 +1,6 @@
+output "hello" {
+  value = "hello"
+}
+
+invalid "block" "type " {
+}
diff --git a/configs/testdata/nested-errors/root.tf b/configs/testdata/nested-errors/root.tf
new file mode 100644
index 000000000..c9db02049
--- /dev/null
+++ b/configs/testdata/nested-errors/root.tf
@@ -0,0 +1,3 @@
+module "child_a" {
+  source = "child_a"
+}