From 4b10a6e1bfcbab33aeb680da6a94089b1f6a7d32 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Thu, 17 Oct 2019 11:33:04 -0400 Subject: [PATCH] command/jsonplan: fix bug with nested modules output (#23092) `marshalPlannedValues` builds a map of modules to their children in order to output the resource changes in a tree. The map was built from the list of resource changes. However if a module had no resources itself, and only called another module (a very normal case), that module would not get added to the map causing none of its children to be output in `planned_values`. This PR adds a walk up through a given module's ancestors to ensure that each module, even those without resources, would be added. --- command/jsonplan/values.go | 26 ++++++- .../modules/more-modules/main.tf | 9 ++- .../show-json/nested-modules/output.json | 77 +++++++++++++++++-- 3 files changed, 98 insertions(+), 14 deletions(-) diff --git a/command/jsonplan/values.go b/command/jsonplan/values.go index a6e96d1d899c..498b0f6f750f 100644 --- a/command/jsonplan/values.go +++ b/command/jsonplan/values.go @@ -96,16 +96,36 @@ func marshalPlannedValues(changes *plans.Changes, schemas *terraform.Schemas) (m containingModule := resource.Addr.Module.String() moduleResourceMap[containingModule] = append(moduleResourceMap[containingModule], resource.Addr) - // root has no parents. - if containingModule != "" { + // the root module has no parents + if !resource.Addr.Module.IsRoot() { parent := resource.Addr.Module.Parent().String() - // we likely will see multiple resources in one module, so we + // we expect to see multiple resources in one module, so we // only need to report the "parent" module for each child module // once. if !seenModules[containingModule] { moduleMap[parent] = append(moduleMap[parent], resource.Addr.Module) seenModules[containingModule] = true } + + // If any given parent module has no resources, it needs to be + // added to the moduleMap. This walks through the current + // resources' modules' ancestors, taking advantage of the fact + // that Ancestors() returns an ordered slice, and verifies that + // each one is in the map. + ancestors := resource.Addr.Module.Ancestors() + for i, ancestor := range ancestors[:len(ancestors)-1] { + aStr := ancestor.String() + + // childStr here is the immediate child of the current step + childStr := ancestors[i+1].String() + // we likely will see multiple resources in one module, so we + // only need to report the "parent" module for each child module + // once. + if !seenModules[childStr] { + moduleMap[aStr] = append(moduleMap[aStr], ancestors[i+1]) + seenModules[childStr] = true + } + } } } } diff --git a/command/testdata/show-json/nested-modules/modules/more-modules/main.tf b/command/testdata/show-json/nested-modules/modules/more-modules/main.tf index 7e1ffafe1a57..2e5273a5726b 100644 --- a/command/testdata/show-json/nested-modules/modules/more-modules/main.tf +++ b/command/testdata/show-json/nested-modules/modules/more-modules/main.tf @@ -1,4 +1,7 @@ -variable "ok" { - default = "something" - description = "description" +variable "test_var" { + default = "bar-var" +} + +resource "test_instance" "test" { + ami = var.test_var } diff --git a/command/testdata/show-json/nested-modules/output.json b/command/testdata/show-json/nested-modules/output.json index 2dcc54773a53..d4304dff5b65 100644 --- a/command/testdata/show-json/nested-modules/output.json +++ b/command/testdata/show-json/nested-modules/output.json @@ -1,9 +1,54 @@ { "format_version": "0.1", - "terraform_version": "0.12.1-dev", "planned_values": { - "root_module": {} + "root_module": { + "child_modules": [ + { + "address": "module.my_module", + "child_modules": [ + { + "resources": [ + { + "address": "module.my_module.module.more.test_instance.test", + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "test", + "schema_version": 0, + "values": { + "ami": "bar-var" + } + } + ], + "address": "module.my_module.module.more" + } + ] + } + ] + } }, + "resource_changes": [ + { + "address": "module.my_module.module.more.test_instance.test", + "module_address": "module.my_module.module.more", + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_name": "test", + "change": { + "actions": [ + "create" + ], + "before": null, + "after": { + "ami": "bar-var" + }, + "after_unknown": { + "id": true + } + } + } + ], "configuration": { "root_module": { "module_calls": { @@ -12,15 +57,31 @@ "module": { "module_calls": { "more": { + "source": "./more-modules", "module": { + "resources": [ + { + "address": "test_instance.test", + "mode": "managed", + "type": "test_instance", + "name": "test", + "provider_config_key": "more:test", + "expressions": { + "ami": { + "references": [ + "var.test_var" + ] + } + }, + "schema_version": 0 + } + ], "variables": { - "ok": { - "default": "something", - "description": "description" + "test_var": { + "default": "bar-var" } } - }, - "source": "./more-modules" + } } } } @@ -28,4 +89,4 @@ } } } -} +} \ No newline at end of file