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.
This commit is contained in:
Kristin Laemmert 2019-10-17 11:33:04 -04:00 committed by GitHub
parent c1ea09141f
commit 4b10a6e1bf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 98 additions and 14 deletions

View File

@ -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
}
}
}
}
}

View File

@ -1,4 +1,7 @@
variable "ok" {
default = "something"
description = "description"
variable "test_var" {
default = "bar-var"
}
resource "test_instance" "test" {
ami = var.test_var
}

View File

@ -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 @@
}
}
}
}
}