From 9722686b62aa85943739f96ec09e6b2f9d512b3b Mon Sep 17 00:00:00 2001 From: James Bardin Date: Sat, 6 Jun 2020 21:27:29 -0400 Subject: [PATCH 1/3] validation test with multiple nested modules --- terraform/context_validate_test.go | 80 ++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/terraform/context_validate_test.go b/terraform/context_validate_test.go index d9a9a3af6..bfe964a6c 100644 --- a/terraform/context_validate_test.go +++ b/terraform/context_validate_test.go @@ -1552,3 +1552,83 @@ resource "aws_instance" "foo" { t.Fatalf("wrong error:\ngot: %s\nwant: message containing %q", got, want) } } + +func TestContext2Validate_expandMultipleNestedModules(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` +module "modA" { + for_each = { + first = "m" + second = "n" + } + source = "./modA" +} +`, + "modA/main.tf": ` +locals { + m = { + first = "m" + second = "n" + } +} + +module "modB" { + for_each = local.m + source = "./modB" + y = each.value +} + +module "modC" { + for_each = local.m + source = "./modC" + x = module.modB[each.key].out + y = module.modB[each.key].out +} + +`, + "modA/modB/main.tf": ` +variable "y" { + type = string +} + +resource "aws_instance" "foo" { + foo = var.y +} + +output "out" { + value = aws_instance.foo.id +} +`, + "modA/modC/main.tf": ` +variable "x" { + type = string +} + +variable "y" { + type = string +} + +resource "aws_instance" "foo" { + foo = var.x +} + +output "out" { + value = var.y +} +`, + }) + + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Config: m, + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p), + }, + }) + + diags := ctx.Validate() + if diags.HasErrors() { + t.Fatal(diags.ErrWithWarnings()) + } +} From 242a916a17373474ee7df4215a5bdc394cc5f2e9 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Sat, 6 Jun 2020 21:38:08 -0400 Subject: [PATCH 2/3] variable ModulePath must return configured path The parent path case is handled ReferenceOutside --- terraform/node_module_variable.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraform/node_module_variable.go b/terraform/node_module_variable.go index a4574e92b..c1ec01e27 100644 --- a/terraform/node_module_variable.go +++ b/terraform/node_module_variable.go @@ -150,7 +150,7 @@ func (n *nodeModuleVariable) Path() addrs.ModuleInstance { // GraphNodeModulePath func (n *nodeModuleVariable) ModulePath() addrs.Module { - return n.Addr.Module.Parent().Module() + return n.Addr.Module.Module() } // RemovableIfNotTargeted From 198c632e0416ae4e08ad85cf4b5745666023af5a Mon Sep 17 00:00:00 2001 From: James Bardin Date: Sat, 6 Jun 2020 21:42:01 -0400 Subject: [PATCH 3/3] incorrect early return during module transformer The recursive call should only return immediately on error. The switch statement to find the current path should not use ReferenceOutside, as we are getting the path for configuration, not for references. This case would not have been taken currently, since all GraphNodeReferenceOutside are also GraphNodeModulePath. --- terraform/node_output.go | 1 + terraform/transform_module_expansion.go | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/terraform/node_output.go b/terraform/node_output.go index 729710f22..1039c3ec4 100644 --- a/terraform/node_output.go +++ b/terraform/node_output.go @@ -22,6 +22,7 @@ var ( _ RemovableIfNotTargeted = (*nodeExpandOutput)(nil) _ GraphNodeReferenceable = (*nodeExpandOutput)(nil) _ GraphNodeReferencer = (*nodeExpandOutput)(nil) + _ GraphNodeReferenceOutside = (*nodeExpandOutput)(nil) _ GraphNodeDynamicExpandable = (*nodeExpandOutput)(nil) _ graphNodeTemporaryValue = (*nodeExpandOutput)(nil) _ graphNodeExpandsInstances = (*nodeExpandOutput)(nil) diff --git a/terraform/transform_module_expansion.go b/terraform/transform_module_expansion.go index 752755204..2ca6a888b 100644 --- a/terraform/transform_module_expansion.go +++ b/terraform/transform_module_expansion.go @@ -120,8 +120,6 @@ func (t *ModuleExpansionTransformer) transform(g *Graph, c *configs.Config, pare case GraphNodeModulePath: path = t.ModulePath() - case GraphNodeReferenceOutside: - path, _ = t.ReferenceOutside() default: continue } @@ -134,7 +132,9 @@ func (t *ModuleExpansionTransformer) transform(g *Graph, c *configs.Config, pare // Also visit child modules, recursively. for _, cc := range c.Children { - return t.transform(g, cc, v) + if err := t.transform(g, cc, v); err != nil { + return err + } } return nil