configschema: Handle nested blocks containing dynamic-typed attributes

We need to make the collection itself be a tuple or object rather than
list or map in this case, since otherwise all of the elements of the
collection are constrained to be of the same type and that isn't the
intent of a provider indicating that it accepts any type.

configs/configschema/decoder_spec.go

@@ -45,13 +45,31 @@ func (b *Block) DecoderSpec() hcldec.Spec {
 				Required: blockS.MinItems == 1 && blockS.MaxItems >= 1,
 			}
 		case NestingList:
-			ret[name] = &hcldec.BlockListSpec{
-				TypeName: name,
-				Nested:   childSpec,
-				MinItems: blockS.MinItems,
-				MaxItems: blockS.MaxItems,
+			// We prefer to use a list where possible, since it makes our
+			// implied type more complete, but if there are any
+			// dynamically-typed attributes inside we must use a tuple
+			// instead, at the expense of our type then not being predictable.
+			if blockS.Block.ImpliedType().HasDynamicTypes() {
+				ret[name] = &hcldec.BlockTupleSpec{
+					TypeName: name,
+					Nested:   childSpec,
+					MinItems: blockS.MinItems,
+					MaxItems: blockS.MaxItems,
+				}
+			} else {
+				ret[name] = &hcldec.BlockListSpec{
+					TypeName: name,
+					Nested:   childSpec,
+					MinItems: blockS.MinItems,
+					MaxItems: blockS.MaxItems,
+				}
 			}
 		case NestingSet:
+			// We forbid dynamically-typed attributes inside NestingSet in
+			// InternalValidate, so we don't do anything special to handle
+			// that here. (There is no set analog to tuple and object types,
+			// because cty's set implementation depends on knowing the static
+			// type in order to properly compute its internal hashes.)
 			ret[name] = &hcldec.BlockSetSpec{
 				TypeName: name,
 				Nested:   childSpec,
@@ -59,10 +77,22 @@ func (b *Block) DecoderSpec() hcldec.Spec {
 				MaxItems: blockS.MaxItems,
 			}
 		case NestingMap:
-			ret[name] = &hcldec.BlockMapSpec{
-				TypeName:   name,
-				Nested:     childSpec,
-				LabelNames: mapLabelNames,
+			// We prefer to use a list where possible, since it makes our
+			// implied type more complete, but if there are any
+			// dynamically-typed attributes inside we must use a tuple
+			// instead, at the expense of our type then not being predictable.
+			if blockS.Block.ImpliedType().HasDynamicTypes() {
+				ret[name] = &hcldec.BlockObjectSpec{
+					TypeName:   name,
+					Nested:     childSpec,
+					LabelNames: mapLabelNames,
+				}
+			} else {
+				ret[name] = &hcldec.BlockMapSpec{
+					TypeName:   name,
+					Nested:     childSpec,
+					LabelNames: mapLabelNames,
+				}
 			}
 		default:
 			// Invalid nesting type is just ignored. It's checked by

configs/configschema/decoder_spec_test.go

@@ -3,6 +3,7 @@ package configschema
 import (
 	"testing"
 
+	"github.com/apparentlymart/go-dump/dump"
 	"github.com/davecgh/go-spew/spew"
 
 	"github.com/hashicorp/hcl2/hcl"
@@ -236,6 +237,95 @@ func TestBlockDecoderSpec(t *testing.T) {
 			}),
 			0,
 		},
+		"blocks with dynamically-typed attributes": {
+			&Block{
+				BlockTypes: map[string]*NestedBlock{
+					"single": {
+						Nesting: NestingSingle,
+						Block: Block{
+							Attributes: map[string]*Attribute{
+								"a": {
+									Type:     cty.DynamicPseudoType,
+									Optional: true,
+								},
+							},
+						},
+					},
+					"list": {
+						Nesting: NestingList,
+						Block: Block{
+							Attributes: map[string]*Attribute{
+								"a": {
+									Type:     cty.DynamicPseudoType,
+									Optional: true,
+								},
+							},
+						},
+					},
+					"map": {
+						Nesting: NestingMap,
+						Block: Block{
+							Attributes: map[string]*Attribute{
+								"a": {
+									Type:     cty.DynamicPseudoType,
+									Optional: true,
+								},
+							},
+						},
+					},
+				},
+			},
+			hcltest.MockBody(&hcl.BodyContent{
+				Blocks: hcl.Blocks{
+					&hcl.Block{
+						Type: "list",
+						Body: hcl.EmptyBody(),
+					},
+					&hcl.Block{
+						Type: "single",
+						Body: hcl.EmptyBody(),
+					},
+					&hcl.Block{
+						Type: "list",
+						Body: hcl.EmptyBody(),
+					},
+					&hcl.Block{
+						Type:        "map",
+						Labels:      []string{"foo"},
+						LabelRanges: []hcl.Range{hcl.Range{}},
+						Body:        hcl.EmptyBody(),
+					},
+					&hcl.Block{
+						Type:        "map",
+						Labels:      []string{"bar"},
+						LabelRanges: []hcl.Range{hcl.Range{}},
+						Body:        hcl.EmptyBody(),
+					},
+				},
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"single": cty.ObjectVal(map[string]cty.Value{
+					"a": cty.NullVal(cty.DynamicPseudoType),
+				}),
+				"list": cty.TupleVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"a": cty.NullVal(cty.DynamicPseudoType),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"a": cty.NullVal(cty.DynamicPseudoType),
+					}),
+				}),
+				"map": cty.ObjectVal(map[string]cty.Value{
+					"foo": cty.ObjectVal(map[string]cty.Value{
+						"a": cty.NullVal(cty.DynamicPseudoType),
+					}),
+					"bar": cty.ObjectVal(map[string]cty.Value{
+						"a": cty.NullVal(cty.DynamicPseudoType),
+					}),
+				}),
+			}),
+			0,
+		},
 		"too many list items": {
 			&Block{
 				BlockTypes: map[string]*NestedBlock{
@@ -294,7 +384,7 @@ func TestBlockDecoderSpec(t *testing.T) {
 
 			if !got.RawEquals(test.Want) {
 				t.Logf("[INFO] implied schema is %s", spew.Sdump(hcldec.ImpliedSchema(spec)))
-				t.Errorf("wrong result\ngot:  %#v\nwant: %#v", got, test.Want)
+				t.Errorf("wrong result\ngot:  %s\nwant: %s", dump.Value(got), dump.Value(test.Want))
 			}
 
 			// Double-check that we're producing consistent results for DecoderSpec

plans/objchange/compatible.go

@@ -80,31 +80,66 @@ func assertObjectCompatible(schema *configschema.Block, planned, actual cty.Valu
 			moreErrs := assertObjectCompatible(&blockS.Block, plannedV, actualV, path)
 			errs = append(errs, moreErrs...)
 		case configschema.NestingList:
+			// A NestingList might either be a list or a tuple, depending on
+			// whether there are dynamically-typed attributes inside. However,
+			// both support a similar-enough API that we can treat them the
+			// same for our purposes here.
 			plannedL := plannedV.LengthInt()
 			actualL := actualV.LengthInt()
 			if plannedL != actualL {
 				errs = append(errs, path.NewErrorf("block count changed from %d to %d", plannedL, actualL))
 				continue
 			}
-		case configschema.NestingMap:
-			plannedAtys := plannedV.Type().AttributeTypes()
-			actualAtys := actualV.Type().AttributeTypes()
-			for k := range plannedAtys {
-				if _, ok := actualAtys[k]; !ok {
-					errs = append(errs, path.NewErrorf("block key %q has vanished", k))
+			for it := plannedV.ElementIterator(); it.Next(); {
+				idx, plannedEV := it.Element()
+				if !actualV.HasIndex(idx).True() {
 					continue
 				}
-
-				plannedEV := plannedV.GetAttr(k)
-				actualEV := actualV.GetAttr(k)
-				moreErrs := assertObjectCompatible(&blockS.Block, plannedEV, actualEV, append(path, cty.GetAttrStep{Name: k}))
+				actualEV := actualV.Index(idx)
+				moreErrs := assertObjectCompatible(&blockS.Block, plannedEV, actualEV, append(path, cty.IndexStep{Key: idx}))
 				errs = append(errs, moreErrs...)
 			}
-			for k := range actualAtys {
-				if _, ok := plannedAtys[k]; !ok {
-					errs = append(errs, path.NewErrorf("new block key %q has appeared", k))
+		case configschema.NestingMap:
+			// A NestingMap might either be a map or an object, depending on
+			// whether there are dynamically-typed attributes inside, but
+			// that's decided statically and so both values will have the same
+			// kind.
+			if plannedV.Type().IsObjectType() {
+				plannedAtys := plannedV.Type().AttributeTypes()
+				actualAtys := actualV.Type().AttributeTypes()
+				for k := range plannedAtys {
+					if _, ok := actualAtys[k]; !ok {
+						errs = append(errs, path.NewErrorf("block key %q has vanished", k))
+						continue
+					}
+
+					plannedEV := plannedV.GetAttr(k)
+					actualEV := actualV.GetAttr(k)
+					moreErrs := assertObjectCompatible(&blockS.Block, plannedEV, actualEV, append(path, cty.GetAttrStep{Name: k}))
+					errs = append(errs, moreErrs...)
+				}
+				for k := range actualAtys {
+					if _, ok := plannedAtys[k]; !ok {
+						errs = append(errs, path.NewErrorf("new block key %q has appeared", k))
+						continue
+					}
+				}
+			} else {
+				plannedL := plannedV.LengthInt()
+				actualL := actualV.LengthInt()
+				if plannedL != actualL {
+					errs = append(errs, path.NewErrorf("block count changed from %d to %d", plannedL, actualL))
 					continue
 				}
+				for it := plannedV.ElementIterator(); it.Next(); {
+					idx, plannedEV := it.Element()
+					if !actualV.HasIndex(idx).True() {
+						continue
+					}
+					actualEV := actualV.Index(idx)
+					moreErrs := assertObjectCompatible(&blockS.Block, plannedEV, actualEV, append(path, cty.IndexStep{Key: idx}))
+					errs = append(errs, moreErrs...)
+				}
 			}
 		case configschema.NestingSet:
 			// We can't do any reasonable matching of set elements since their

plans/objchange/objchange.go

@@ -90,13 +90,6 @@ func ProposedNewObject(schema *configschema.Block, prior, config cty.Value) cty.
 			newV = ProposedNewObject(&blockType.Block, priorV, configV)
 
 		case configschema.NestingList:
-			if !configV.Type().IsTupleType() {
-				// Despite the name, we expect NestingList to produce a tuple
-				// type so that different elements may have dynamically-typed
-				// attributes that have a different actual type.
-				panic("configschema.NestingList value is not a tuple as expected")
-			}
-
 			// Nested blocks are correlated by index.
 			if l := configV.LengthInt(); l > 0 {
 				newVals := make([]cty.Value, 0, l)
@@ -113,45 +106,73 @@ func ProposedNewObject(schema *configschema.Block, prior, config cty.Value) cty.
 					newEV := ProposedNewObject(&blockType.Block, priorEV, configEV)
 					newVals = append(newVals, newEV)
 				}
-				// Although we call the nesting mode "list", we actually use
-				// tuple values so that elements might have different types
-				// in case of dynamically-typed attributes.
-				newV = cty.TupleVal(newVals)
+				// Despite the name, a NestingList might also be a tuple, if
+				// its nested schema contains dynamically-typed attributes.
+				if configV.Type().IsTupleType() {
+					newV = cty.TupleVal(newVals)
+				} else {
+					newV = cty.ListVal(newVals)
+				}
 			} else {
-				newV = cty.EmptyTupleVal
+				// Despite the name, a NestingList might also be a tuple, if
+				// its nested schema contains dynamically-typed attributes.
+				if configV.Type().IsTupleType() {
+					newV = cty.EmptyTupleVal
+				} else {
+					newV = cty.ListValEmpty(blockType.ImpliedType())
+				}
 			}
 
 		case configschema.NestingMap:
-			if !configV.Type().IsObjectType() {
-				// Despite the name, we expect NestingMap to produce an object
-				// type so that different elements may have dynamically-typed
-				// attributes that have a different actual type.
-				panic("configschema.NestingMap value is not an object as expected")
-			}
+			// Despite the name, a NestingMap may produce either a map or
+			// object value, depending on whether the nested schema contains
+			// dynamically-typed attributes.
+			if configV.Type().IsObjectType() {
+				// Nested blocks are correlated by key.
+				if l := configV.LengthInt(); l > 0 {
+					newVals := make(map[string]cty.Value, l)
+					atys := configV.Type().AttributeTypes()
+					for name := range atys {
+						configEV := configV.GetAttr(name)
+						if !priorV.Type().HasAttribute(name) {
+							// If there is no corresponding prior element then
+							// we just take the config value as-is.
+							newVals[name] = configEV
+							continue
+						}
+						priorEV := priorV.GetAttr(name)
 
-			// Nested blocks are correlated by key.
-			if l := configV.LengthInt(); l > 0 {
-				newVals := make(map[string]cty.Value, l)
-				atys := configV.Type().AttributeTypes()
-				for name := range atys {
-					configEV := configV.GetAttr(name)
-					if !priorV.Type().HasAttribute(name) {
-						// If there is no corresponding prior element then
-						// we just take the config value as-is.
-						newVals[name] = configEV
-						continue
+						newEV := ProposedNewObject(&blockType.Block, priorEV, configEV)
+						newVals[name] = newEV
 					}
-					priorEV := priorV.GetAttr(name)
-
-					newEV := ProposedNewObject(&blockType.Block, priorEV, configEV)
-					newVals[name] = newEV
+					// Although we call the nesting mode "map", we actually use
+					// object values so that elements might have different types
+					// in case of dynamically-typed attributes.
+					newV = cty.ObjectVal(newVals)
+				} else {
+					newV = cty.EmptyObjectVal
 				}
-				// Although we call the nesting mode "map", we actually use
-				// object values so that elements might have different types
-				// in case of dynamically-typed attributes.
-				newV = cty.ObjectVal(newVals)
 			} else {
-				newV = cty.EmptyObjectVal
+				if l := configV.LengthInt(); l > 0 {
+					newVals := make(map[string]cty.Value, l)
+					for it := configV.ElementIterator(); it.Next(); {
+						idx, configEV := it.Element()
+						k := idx.AsString()
+						if !priorV.HasIndex(idx).True() {
+							// If there is no corresponding prior element then
+							// we just take the config value as-is.
+							newVals[k] = configEV
+							continue
+						}
+						priorEV := priorV.Index(idx)
+
+						newEV := ProposedNewObject(&blockType.Block, priorEV, configEV)
+						newVals[k] = newEV
+					}
+					newV = cty.MapVal(newVals)
+				} else {
+					newV = cty.MapValEmpty(blockType.ImpliedType())
+				}
 			}
 
 		case configschema.NestingSet:

plans/objchange/objchange_test.go

@@ -170,6 +170,61 @@ func TestProposedNewObject(t *testing.T) {
 					},
 				},
 			},
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.ListVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.StringVal("beep"),
+						"baz": cty.StringVal("boop"),
+					}),
+				}),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.ListVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.StringVal("bap"),
+						"baz": cty.NullVal(cty.String),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.StringVal("blep"),
+						"baz": cty.NullVal(cty.String),
+					}),
+				}),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.ListVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.StringVal("bap"),
+						"baz": cty.StringVal("boop"),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.StringVal("blep"),
+						"baz": cty.NullVal(cty.String),
+					}),
+				}),
+			}),
+		},
+		"prior nested list with dynamic": {
+			&configschema.Block{
+				BlockTypes: map[string]*configschema.NestedBlock{
+					"foo": {
+						Nesting: configschema.NestingList,
+						Block: configschema.Block{
+							Attributes: map[string]*configschema.Attribute{
+								"bar": {
+									Type:     cty.String,
+									Optional: true,
+									Computed: true,
+								},
+								"baz": {
+									Type:     cty.DynamicPseudoType,
+									Optional: true,
+									Computed: true,
+								},
+							},
+						},
+					},
+				},
+			},
 			cty.ObjectVal(map[string]cty.Value{
 				"foo": cty.TupleVal([]cty.Value{
 					cty.ObjectVal(map[string]cty.Value{
@@ -225,6 +280,65 @@ func TestProposedNewObject(t *testing.T) {
 					},
 				},
 			},
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.MapVal(map[string]cty.Value{
+					"a": cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.StringVal("beep"),
+						"baz": cty.StringVal("boop"),
+					}),
+					"b": cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.StringVal("blep"),
+						"baz": cty.StringVal("boot"),
+					}),
+				}),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.MapVal(map[string]cty.Value{
+					"a": cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.StringVal("bap"),
+						"baz": cty.NullVal(cty.String),
+					}),
+					"c": cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.StringVal("bosh"),
+						"baz": cty.NullVal(cty.String),
+					}),
+				}),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.MapVal(map[string]cty.Value{
+					"a": cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.StringVal("bap"),
+						"baz": cty.StringVal("boop"),
+					}),
+					"c": cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.StringVal("bosh"),
+						"baz": cty.NullVal(cty.String),
+					}),
+				}),
+			}),
+		},
+		"prior nested map with dynamic": {
+			&configschema.Block{
+				BlockTypes: map[string]*configschema.NestedBlock{
+					"foo": {
+						Nesting: configschema.NestingMap,
+						Block: configschema.Block{
+							Attributes: map[string]*configschema.Attribute{
+								"bar": {
+									Type:     cty.String,
+									Optional: true,
+									Computed: true,
+								},
+								"baz": {
+									Type:     cty.DynamicPseudoType,
+									Optional: true,
+									Computed: true,
+								},
+							},
+						},
+					},
+				},
+			},
 			cty.ObjectVal(map[string]cty.Value{
 				"foo": cty.ObjectVal(map[string]cty.Value{
 					"a": cty.ObjectVal(map[string]cty.Value{