From 6b4e73af48f1cb78e51a634b077f73548db39ddc Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 22 Sep 2021 12:09:26 -0400 Subject: [PATCH 1/2] skip the blocktoattr fixup with nested types If structural types are being used, we can be assured that the legacy SDK SchemaConfigModeAttr is not being used, and the fixup is not needed. This prevents inadvertent mapping of blocks to structural attributes, and allows us to skip the fixup overhead when possible. --- internal/lang/blocktoattr/fixup.go | 27 +++++++++++++++++++++++ internal/lang/blocktoattr/fixup_test.go | 29 +++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/internal/lang/blocktoattr/fixup.go b/internal/lang/blocktoattr/fixup.go index 1864e3e5016c..90eb260d7289 100644 --- a/internal/lang/blocktoattr/fixup.go +++ b/internal/lang/blocktoattr/fixup.go @@ -12,6 +12,10 @@ import ( // type in the schema to be written with HCL block syntax as multiple nested // blocks with the attribute name as the block type. // +// The fixup is only applied in the absence of structural attribute types. The +// presence of these types indicate the use of a provider which does not +// support mapping blocks to attributes. +// // This partially restores some of the block/attribute confusion from HCL 1 // so that existing patterns that depended on that confusion can continue to // be used in the short term while we settle on a longer-term strategy. @@ -28,6 +32,10 @@ func FixUpBlockAttrs(body hcl.Body, schema *configschema.Block) hcl.Body { schema = &configschema.Block{} } + if skipFixup(schema) { + return body + } + return &fixupBody{ original: body, schema: schema, @@ -35,6 +43,25 @@ func FixUpBlockAttrs(body hcl.Body, schema *configschema.Block) hcl.Body { } } +// skipFixup detects any use of Attribute.NestedType. Because the fixup was +// only supported for the legacy SDK, there is no situation where structural +// attributes are used where the fixup is expected. +func skipFixup(schema *configschema.Block) bool { + for _, attr := range schema.Attributes { + if attr.NestedType != nil { + return true + } + } + + for _, block := range schema.BlockTypes { + if skipFixup(&block.Block) { + return true + } + } + + return false +} + type fixupBody struct { original hcl.Body schema *configschema.Block diff --git a/internal/lang/blocktoattr/fixup_test.go b/internal/lang/blocktoattr/fixup_test.go index 8c7640521b13..92799394fa95 100644 --- a/internal/lang/blocktoattr/fixup_test.go +++ b/internal/lang/blocktoattr/fixup_test.go @@ -400,6 +400,35 @@ container { }), wantErrs: true, }, + "no fixup allowed": { + src: ` + container { + foo = "one" + } + `, + schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "container": { + NestedType: &configschema.Object{ + Nesting: configschema.NestingList, + Attributes: map[string]*configschema.Attribute{ + "foo": { + Type: cty.String, + }, + }, + }, + }, + }, + }, + want: cty.ObjectVal(map[string]cty.Value{ + "container": cty.NullVal(cty.List( + cty.Object(map[string]cty.Type{ + "foo": cty.String, + }), + )), + }), + wantErrs: true, + }, } ctx := &hcl.EvalContext{ From 8706a18c4b69850cee1736f3af26869317313f3b Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 22 Sep 2021 15:59:25 -0400 Subject: [PATCH 2/2] refine the skipFixup heuristic We can also rule out some attribute types as indicating something other than the legacy SDK. - Tuple types were not generated at all. - There were no single objects types, the convention was to use a block list or set of length 1. - Maps of objects were not possible to generate, since named blocks were not implemented. - Nested collections were not supported, but when they were generated they would have primitive types. --- internal/lang/blocktoattr/fixup.go | 34 ++++++++++++++++++++++--- internal/lang/blocktoattr/fixup_test.go | 34 ++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/internal/lang/blocktoattr/fixup.go b/internal/lang/blocktoattr/fixup.go index 90eb260d7289..5d05a86f2f5f 100644 --- a/internal/lang/blocktoattr/fixup.go +++ b/internal/lang/blocktoattr/fixup.go @@ -1,6 +1,8 @@ package blocktoattr import ( + "log" + "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hcldec" "github.com/hashicorp/terraform/internal/configs/configschema" @@ -33,6 +35,10 @@ func FixUpBlockAttrs(body hcl.Body, schema *configschema.Block) hcl.Body { } if skipFixup(schema) { + // we don't have any context for the resource name or type, but + // hopefully this could help locate the evaluation in the logs if there + // were a problem + log.Println("[DEBUG] skipping FixUpBlockAttrs") return body } @@ -43,14 +49,36 @@ func FixUpBlockAttrs(body hcl.Body, schema *configschema.Block) hcl.Body { } } -// skipFixup detects any use of Attribute.NestedType. Because the fixup was -// only supported for the legacy SDK, there is no situation where structural -// attributes are used where the fixup is expected. +// skipFixup detects any use of Attribute.NestedType, or Types which could not +// be generate by the legacy SDK when taking SchemaConfigModeAttr into account. func skipFixup(schema *configschema.Block) bool { for _, attr := range schema.Attributes { if attr.NestedType != nil { return true } + ty := attr.Type + + // Lists and sets of objects could be generated by + // SchemaConfigModeAttr, but some other combinations can be ruled out. + + // Tuples and objects could not be generated at all. + if ty.IsTupleType() || ty.IsObjectType() { + return true + } + + // A map of objects was not possible. + if ty.IsMapType() && ty.ElementType().IsObjectType() { + return true + } + + // Nested collections were not really supported, but could be generated + // with string types (though we conservatively limit this to primitive types) + if ty.IsCollectionType() { + ety := ty.ElementType() + if ety.IsCollectionType() && !ety.ElementType().IsPrimitiveType() { + return true + } + } } for _, block := range schema.BlockTypes { diff --git a/internal/lang/blocktoattr/fixup_test.go b/internal/lang/blocktoattr/fixup_test.go index 92799394fa95..36ab48041c9a 100644 --- a/internal/lang/blocktoattr/fixup_test.go +++ b/internal/lang/blocktoattr/fixup_test.go @@ -400,7 +400,7 @@ container { }), wantErrs: true, }, - "no fixup allowed": { + "no fixup allowed with NestedType": { src: ` container { foo = "one" @@ -429,6 +429,38 @@ container { }), wantErrs: true, }, + "no fixup allowed new types": { + src: ` + container { + foo = "one" + } + `, + schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + // This could be a ConfigModeAttr fixup + "container": { + Type: cty.List(cty.Object(map[string]cty.Type{ + "foo": cty.String, + })), + }, + // But the presence of this type means it must have been + // declared by a new SDK + "new_type": { + Type: cty.Object(map[string]cty.Type{ + "boo": cty.String, + }), + }, + }, + }, + want: cty.ObjectVal(map[string]cty.Value{ + "container": cty.NullVal(cty.List( + cty.Object(map[string]cty.Type{ + "foo": cty.String, + }), + )), + }), + wantErrs: true, + }, } ctx := &hcl.EvalContext{