Skip to content

Commit

Permalink
command/jsonplan: fix bug with nested modules output (#23092)
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
mildwonkey authored Oct 17, 2019
1 parent c1ea091 commit 4b10a6e
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 14 deletions.
26 changes: 23 additions & 3 deletions command/jsonplan/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
variable "ok" {
default = "something"
description = "description"
variable "test_var" {
default = "bar-var"
}

resource "test_instance" "test" {
ami = var.test_var
}
77 changes: 69 additions & 8 deletions command/testdata/show-json/nested-modules/output.json
Original file line number Diff line number Diff line change
@@ -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": {
Expand All @@ -12,20 +57,36 @@
"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"
}
}
}
}
}
}
}
}
}
}

0 comments on commit 4b10a6e

Please sign in to comment.