diff --git a/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-item/input/block-as-list-dynamic-item.tf b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-item/input/block-as-list-dynamic-item.tf new file mode 100644 index 000000000..bfbb18622 --- /dev/null +++ b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-item/input/block-as-list-dynamic-item.tf @@ -0,0 +1,8 @@ +resource "test_instance" "foo" { + network = [ + { + cidr_block = "10.1.2.0/24" + }, + "${var.baz}" + ] +} diff --git a/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-item/want/block-as-list-dynamic-item.tf b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-item/want/block-as-list-dynamic-item.tf new file mode 100644 index 000000000..d5b982b4f --- /dev/null +++ b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-item/want/block-as-list-dynamic-item.tf @@ -0,0 +1,23 @@ +resource "test_instance" "foo" { + network { + cidr_block = "10.1.2.0/24" + } + dynamic "network" { + for_each = [var.baz] + content { + # TF-UPGRADE-TODO: The automatic upgrade tool can't predict + # which keys might be set in maps assigned here, so it has + # produced a comprehensive set here. Consider simplifying + # this after confirming which keys can be set in practice. + + cidr_block = lookup(network.value, "cidr_block", null) + + dynamic "subnet" { + for_each = lookup(network.value, "subnet", []) + content { + number = subnet.value.number + } + } + } + } +} diff --git a/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-item/want/versions.tf b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-item/want/versions.tf new file mode 100644 index 000000000..d9b6f790b --- /dev/null +++ b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-item/want/versions.tf @@ -0,0 +1,3 @@ +terraform { + required_version = ">= 0.12" +} diff --git a/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-nested/input/block-as-list-dynamic-nested.tf b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-nested/input/block-as-list-dynamic-nested.tf new file mode 100644 index 000000000..df96a77c1 --- /dev/null +++ b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-nested/input/block-as-list-dynamic-nested.tf @@ -0,0 +1,5 @@ +resource "test_instance" "foo" { + network { + subnet = "${var.baz}" + } +} diff --git a/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-nested/want/block-as-list-dynamic-nested.tf b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-nested/want/block-as-list-dynamic-nested.tf new file mode 100644 index 000000000..f32824819 --- /dev/null +++ b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-nested/want/block-as-list-dynamic-nested.tf @@ -0,0 +1,15 @@ +resource "test_instance" "foo" { + network { + dynamic "subnet" { + for_each = var.baz + content { + # TF-UPGRADE-TODO: The automatic upgrade tool can't predict + # which keys might be set in maps assigned here, so it has + # produced a comprehensive set here. Consider simplifying + # this after confirming which keys can be set in practice. + + number = subnet.value.number + } + } + } +} diff --git a/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-nested/want/versions.tf b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-nested/want/versions.tf new file mode 100644 index 000000000..d9b6f790b --- /dev/null +++ b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic-nested/want/versions.tf @@ -0,0 +1,3 @@ +terraform { + required_version = ">= 0.12" +} diff --git a/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic/input/block-as-list-dynamic.tf b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic/input/block-as-list-dynamic.tf new file mode 100644 index 000000000..e436ad0db --- /dev/null +++ b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic/input/block-as-list-dynamic.tf @@ -0,0 +1,3 @@ +resource "test_instance" "foo" { + network = "${var.baz}" +} diff --git a/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic/want/block-as-list-dynamic.tf b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic/want/block-as-list-dynamic.tf new file mode 100644 index 000000000..1db25aba3 --- /dev/null +++ b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic/want/block-as-list-dynamic.tf @@ -0,0 +1,20 @@ +resource "test_instance" "foo" { + dynamic "network" { + for_each = var.baz + content { + # TF-UPGRADE-TODO: The automatic upgrade tool can't predict + # which keys might be set in maps assigned here, so it has + # produced a comprehensive set here. Consider simplifying + # this after confirming which keys can be set in practice. + + cidr_block = lookup(network.value, "cidr_block", null) + + dynamic "subnet" { + for_each = lookup(network.value, "subnet", []) + content { + number = subnet.value.number + } + } + } + } +} diff --git a/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic/want/versions.tf b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic/want/versions.tf new file mode 100644 index 000000000..d9b6f790b --- /dev/null +++ b/configs/configupgrade/test-fixtures/valid/block-as-list-dynamic/want/versions.tf @@ -0,0 +1,3 @@ +terraform { + required_version = ">= 0.12" +} diff --git a/configs/configupgrade/upgrade_body.go b/configs/configupgrade/upgrade_body.go index 0aa9dc6ba..2b1488779 100644 --- a/configs/configupgrade/upgrade_body.go +++ b/configs/configupgrade/upgrade_body.go @@ -5,7 +5,6 @@ import ( "fmt" hcl1ast "github.com/hashicorp/hcl/hcl/ast" - hcl1printer "github.com/hashicorp/hcl/hcl/printer" hcl1token "github.com/hashicorp/hcl/hcl/token" hcl2 "github.com/hashicorp/hcl2/hcl" hcl2syntax "github.com/hashicorp/hcl2/hcl/hclsyntax" @@ -184,7 +183,7 @@ func nestedBlockRule(filename string, nestedRules bodyContentRules, an *analysis } } -func nestedBlockRuleWithDynamic(filename string, nestedRules bodyContentRules, an *analysis, adhocComments *commentQueue) bodyItemRule { +func nestedBlockRuleWithDynamic(filename string, nestedRules bodyContentRules, nestedSchema *configschema.NestedBlock, an *analysis, adhocComments *commentQueue) bodyItemRule { return func(buf *bytes.Buffer, blockAddr string, item *hcl1ast.ObjectItem) tfdiags.Diagnostics { // In Terraform v0.11 it was possible in some cases to trick Terraform // and providers into accepting HCL's attribute syntax and some HIL @@ -218,10 +217,92 @@ func nestedBlockRuleWithDynamic(filename string, nestedRules bodyContentRules, a // generate a single "dynamic" block to iterate over the list at // runtime. - // TODO: Implement this - hcl1printer.Fprint(buf, item) - buf.WriteByte('\n') - return nil + var diags tfdiags.Diagnostics + blockType := item.Keys[0].Token.Value().(string) + labels := make([]string, len(item.Keys)-1) + for i, key := range item.Keys[1:] { + labels[i] = key.Token.Value().(string) + } + + var blockItems []hcl1ast.Node + + switch val := item.Val.(type) { + + case *hcl1ast.ObjectType: + blockItems = append(blockItems, val) + + case *hcl1ast.ListType: + for _, node := range val.List { + switch listItem := node.(type) { + case *hcl1ast.ObjectType: + blockItems = append(blockItems, listItem) + default: + // We're going to cheat a bit here and construct a synthetic + // HCL1 list just because that makes our logic + // simpler below where we can just treat all non-objects + // in the same way when producing "dynamic" blocks. + synthList := &hcl1ast.ListType{ + List: []hcl1ast.Node{listItem}, + Lbrack: listItem.Pos(), + Rbrack: hcl1NodeEndPos(listItem), + } + blockItems = append(blockItems, synthList) + } + } + + default: + blockItems = append(blockItems, item.Val) + } + + for _, blockItem := range blockItems { + switch ti := blockItem.(type) { + case *hcl1ast.ObjectType: + // If we have an object then we'll pass through its content + // as a block directly. This is the most straightforward mapping + // from the source input, since we know exactly which keys + // are present. + printBlockOpen(buf, blockType, labels, item.LineComment) + bodyDiags := upgradeBlockBody( + filename, fmt.Sprintf("%s.%s", blockAddr, blockType), buf, + ti.List.Items, nestedRules, adhocComments, + ) + diags = diags.Append(bodyDiags) + buf.WriteString("}\n") + default: + // For any other sort of value we can't predict what shape it + // will have at runtime, so we must generate a very conservative + // "dynamic" block that tries to assign everything from the + // schema. The result of this is likely to be pretty ugly. + printBlockOpen(buf, "dynamic", []string{blockType}, item.LineComment) + eachSrc, eachDiags := upgradeExpr(blockItem, filename, true, an) + diags = diags.Append(eachDiags) + printAttribute(buf, "for_each", eachSrc, nil) + if nestedSchema.Nesting == configschema.NestingMap { + // This is a pretty odd situation since map-based blocks + // didn't exist prior to Terraform v0.12, but we'll support + // this anyway in case we decide to add support in a later + // SDK release that is still somehow compatible with + // Terraform v0.11. + printAttribute(buf, "labels", []byte(fmt.Sprintf(`[%s.key]`, blockType)), nil) + } + printBlockOpen(buf, "content", nil, nil) + buf.WriteString("# TF-UPGRADE-TODO: The automatic upgrade tool can't predict\n") + buf.WriteString("# which keys might be set in maps assigned here, so it has\n") + buf.WriteString("# produced a comprehensive set here. Consider simplifying\n") + buf.WriteString("# this after confirming which keys can be set in practice.\n\n") + printDynamicBlockBody(buf, blockType, &nestedSchema.Block) + buf.WriteString("}\n") + buf.WriteString("}\n") + diags = diags.Append(&hcl2.Diagnostic{ + Severity: hcl2.DiagWarning, + Summary: "Approximate migration of invalid block type assignment", + Detail: fmt.Sprintf("In %s the name %q is a nested block type, but this configuration is exploiting some missing validation rules from Terraform v0.11 and prior to trick Terraform into creating blocks dynamically.\n\nThis has been upgraded to use the new Terraform v0.12 dynamic blocks feature, but since the upgrade tool cannot predict which map keys will be present a fully-comprehensive set has been generated.", blockAddr, blockType), + Subject: hcl1PosRange(filename, blockItem.Pos()).Ptr(), + }) + } + } + + return diags } } @@ -243,7 +324,7 @@ func schemaDefaultBodyRules(filename string, schema *configschema.Block, an *ana } for name, blockS := range schema.BlockTypes { nestedRules := schemaDefaultBodyRules(filename, &blockS.Block, an, adhocComments) - ret[name] = nestedBlockRule(filename, nestedRules, an, adhocComments) + ret[name] = nestedBlockRuleWithDynamic(filename, nestedRules, blockS, an, adhocComments) } return ret diff --git a/configs/configupgrade/upgrade_native.go b/configs/configupgrade/upgrade_native.go index 2eeb19310..952bcf871 100644 --- a/configs/configupgrade/upgrade_native.go +++ b/configs/configupgrade/upgrade_native.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/terraform/addrs" backendinit "github.com/hashicorp/terraform/backend/init" + "github.com/hashicorp/terraform/configs/configschema" "github.com/hashicorp/terraform/tfdiags" ) @@ -452,6 +453,76 @@ func upgradeBlockBody(filename string, blockAddr string, buf *bytes.Buffer, args return diags } +// printDynamicBody prints out a conservative, exhaustive dynamic block body +// for every attribute and nested block in the given schema, for situations +// when a dynamic expression was being assigned to a block type name in input +// configuration and so we can assume it's a list of maps but can't make +// any assumptions about what subset of the schema-specified keys might be +// present in the map values. +func printDynamicBlockBody(buf *bytes.Buffer, iterName string, schema *configschema.Block) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + + attrNames := make([]string, 0, len(schema.Attributes)) + for name := range schema.Attributes { + attrNames = append(attrNames, name) + } + sort.Strings(attrNames) + for _, name := range attrNames { + attrS := schema.Attributes[name] + if !(attrS.Required || attrS.Optional) { // no Computed-only attributes + continue + } + if attrS.Required { + // For required attributes we can generate a simpler expression + // that just assumes the presence of the key representing the + // attribute value. + printAttribute(buf, name, []byte(fmt.Sprintf(`%s.value.%s`, iterName, name)), nil) + } else { + // Otherwise we must be conservative and generate a conditional + // lookup that will just populate nothing at all if the expected + // key is not present. + printAttribute(buf, name, []byte(fmt.Sprintf(`lookup(%s.value, %q, null)`, iterName, name)), nil) + } + } + + blockTypeNames := make([]string, 0, len(schema.BlockTypes)) + for name := range schema.BlockTypes { + blockTypeNames = append(blockTypeNames, name) + } + sort.Strings(blockTypeNames) + for i, name := range blockTypeNames { + blockS := schema.BlockTypes[name] + + // We'll disregard any block type that consists only of computed + // attributes, since otherwise we'll just create weird empty blocks + // that do nothing except create confusion. + if !schemaHasSettableArguments(&blockS.Block) { + continue + } + + if i > 0 || len(attrNames) > 0 { + buf.WriteByte('\n') + } + printBlockOpen(buf, "dynamic", []string{name}, nil) + switch blockS.Nesting { + case configschema.NestingMap: + printAttribute(buf, "for_each", []byte(fmt.Sprintf(`lookup(%s.value, %q, {})`, iterName, name)), nil) + printAttribute(buf, "labels", []byte(fmt.Sprintf(`[%s.key]`, name)), nil) + case configschema.NestingSingle: + printAttribute(buf, "for_each", []byte(fmt.Sprintf(`lookup(%s.value, %q, null) != null ? [%s.value.%s] : []`, iterName, name, iterName, name)), nil) + default: + printAttribute(buf, "for_each", []byte(fmt.Sprintf(`lookup(%s.value, %q, [])`, iterName, name)), nil) + } + printBlockOpen(buf, "content", nil, nil) + moreDiags := printDynamicBlockBody(buf, name, &blockS.Block) + diags = diags.Append(moreDiags) + buf.WriteString("}\n") + buf.WriteString("}\n") + } + + return diags +} + func printComments(buf *bytes.Buffer, group *hcl1ast.CommentGroup) { if group == nil { return @@ -633,3 +704,17 @@ func passthruBlockTodo(w io.Writer, node hcl1ast.Node, msg string) { hcl1printer.Fprint(w, node) w.Write([]byte{'\n', '\n'}) } + +func schemaHasSettableArguments(schema *configschema.Block) bool { + for _, attrS := range schema.Attributes { + if attrS.Optional || attrS.Required { + return true + } + } + for _, blockS := range schema.BlockTypes { + if schemaHasSettableArguments(&blockS.Block) { + return true + } + } + return false +} diff --git a/configs/configupgrade/upgrade_test.go b/configs/configupgrade/upgrade_test.go index 7dab47e6a..1ef467e57 100644 --- a/configs/configupgrade/upgrade_test.go +++ b/configs/configupgrade/upgrade_test.go @@ -200,7 +200,27 @@ var testProviders = map[string]providers.Factory{ Nesting: configschema.NestingSet, Block: configschema.Block{ Attributes: map[string]*configschema.Attribute{ - "cidr_block": {Type: cty.String, Computed: true}, + "cidr_block": {Type: cty.String, Optional: true}, + "subnet_cidrs": {Type: cty.Map(cty.String), Computed: true}, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "subnet": { + Nesting: configschema.NestingSet, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "number": {Type: cty.Number, Required: true}, + }, + }, + }, + }, + }, + }, + "addresses": { + Nesting: configschema.NestingSingle, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "ipv4": {Type: cty.String, Computed: true}, + "ipv6": {Type: cty.String, Computed: true}, }, }, },