diff --git a/builtin/providers/test/resource_list_test.go b/builtin/providers/test/resource_list_test.go index aeb56e989f..94c697ceeb 100644 --- a/builtin/providers/test/resource_list_test.go +++ b/builtin/providers/test/resource_list_test.go @@ -447,3 +447,37 @@ resource "test_resource_list" "bar" { }, }) } + +func TestResourceList_dynamicList(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_list" "a" { + dependent_list { + val = "a" + } + + dependent_list { + val = "b" + } +} +resource "test_resource_list" "b" { + list_block { + string = "constant" + } + dynamic "list_block" { + for_each = test_resource_list.a.computed_list + content { + string = list_block.value + } + } +} + `), + Check: resource.ComposeTestCheckFunc(), + }, + }, + }) +} diff --git a/plans/objchange/compatible.go b/plans/objchange/compatible.go index 58e943042c..8b7ef43fdd 100644 --- a/plans/objchange/compatible.go +++ b/plans/objchange/compatible.go @@ -61,30 +61,22 @@ func assertObjectCompatible(schema *configschema.Block, planned, actual cty.Valu plannedV := planned.GetAttr(name) actualV := actual.GetAttr(name) - // As a special case, we permit a "planned" block with exactly one - // element where all of the "leaf" values are unknown, since that's - // what HCL's dynamic block extension generates if the for_each - // expression is itself unknown and thus it cannot predict how many - // child blocks will get created. - switch blockS.Nesting { - case configschema.NestingSingle, configschema.NestingGroup: - if allLeafValuesUnknown(plannedV) && !plannedV.IsNull() { - return errs - } - case configschema.NestingList, configschema.NestingMap, configschema.NestingSet: - if plannedV.IsKnown() && !plannedV.IsNull() && plannedV.LengthInt() == 1 { - elemVs := plannedV.AsValueSlice() - if allLeafValuesUnknown(elemVs[0]) { - return errs - } - } - default: - panic(fmt.Sprintf("unsupported nesting mode %s", blockS.Nesting)) - } + // As a special case, if there were any blocks whose leaf attributes + // are all unknown then we assume (possibly incorrectly) that the + // HCL dynamic block extension is in use with an unknown for_each + // argument, and so we will do looser validation here that allows + // for those blocks to have expanded into a different number of blocks + // if the for_each value is now known. + maybeUnknownBlocks := couldHaveUnknownBlockPlaceholder(plannedV, blockS, false) path := append(path, cty.GetAttrStep{Name: name}) switch blockS.Nesting { case configschema.NestingSingle, configschema.NestingGroup: + // If an unknown block placeholder was present then the placeholder + // may have expanded out into zero blocks, which is okay. + if maybeUnknownBlocks && actualV.IsNull() { + continue + } moreErrs := assertObjectCompatible(&blockS.Block, plannedV, actualV, path) errs = append(errs, moreErrs...) case configschema.NestingList: @@ -96,6 +88,14 @@ func assertObjectCompatible(schema *configschema.Block, planned, actual cty.Valu continue } + if maybeUnknownBlocks { + // When unknown blocks are present the final blocks may be + // at different indices than the planned blocks, so unfortunately + // we can't do our usual checks in this case without generating + // false negatives. + continue + } + plannedL := plannedV.LengthInt() actualL := actualV.LengthInt() if plannedL != actualL { @@ -130,10 +130,12 @@ func assertObjectCompatible(schema *configschema.Block, planned, actual cty.Valu 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 + if !maybeUnknownBlocks { // new blocks may appear if unknown blocks were present in the plan + for k := range actualAtys { + if _, ok := plannedAtys[k]; !ok { + errs = append(errs, path.NewErrorf("new block key %q has appeared", k)) + continue + } } } } else { @@ -142,7 +144,7 @@ func assertObjectCompatible(schema *configschema.Block, planned, actual cty.Valu } plannedL := plannedV.LengthInt() actualL := actualV.LengthInt() - if plannedL != actualL { + if plannedL != actualL && !maybeUnknownBlocks { // new blocks may appear if unknown blocks were persent in the plan errs = append(errs, path.NewErrorf("block count changed from %d to %d", plannedL, actualL)) continue } @@ -302,18 +304,77 @@ func indexStrForErrors(v cty.Value) string { } } -func allLeafValuesUnknown(v cty.Value) bool { - seenKnownValue := false - cty.Walk(v, func(path cty.Path, cv cty.Value) (bool, error) { - if cv.IsNull() { - seenKnownValue = true +// couldHaveUnknownBlockPlaceholder is a heuristic that recognizes how the +// HCL dynamic block extension behaves when it's asked to expand a block whose +// for_each argument is unknown. In such cases, it generates a single placeholder +// block with all leaf attribute values unknown, and once the for_each +// expression becomes known the placeholder may be replaced with any number +// of blocks, so object compatibility checks would need to be more liberal. +// +// Set "nested" if testing a block that is nested inside a candidate block +// placeholder; this changes the interpretation of there being no blocks of +// a type to allow for there being zero nested blocks. +func couldHaveUnknownBlockPlaceholder(v cty.Value, blockS *configschema.NestedBlock, nested bool) bool { + switch blockS.Nesting { + case configschema.NestingSingle, configschema.NestingGroup: + if nested && v.IsNull() { + return true // for nested blocks, a single block being unset doesn't disqualify from being an unknown block placeholder } - if cv.Type().IsPrimitiveType() && cv.IsKnown() { - seenKnownValue = true + return couldBeUnknownBlockPlaceholderElement(v, &blockS.Block) + default: + // These situations should be impossible for correct providers, but + // we permit the legacy SDK to produce some incorrect outcomes + // for compatibility with its existing logic, and so we must be + // tolerant here. + if !v.IsKnown() { + return true } - return true, nil - }) - return !seenKnownValue + if v.IsNull() { + return false // treated as if the list were empty, so we would see zero iterations below + } + + // For all other nesting modes, our value should be something iterable. + for it := v.ElementIterator(); it.Next(); { + _, ev := it.Element() + if couldBeUnknownBlockPlaceholderElement(ev, &blockS.Block) { + return true + } + } + + // Our default changes depending on whether we're testing the candidate + // block itself or something nested inside of it: zero blocks of a type + // can never contain a dynamic block placeholder, but a dynamic block + // placeholder might contain zero blocks of one of its own nested block + // types, if none were set in the config at all. + return nested + } +} + +func couldBeUnknownBlockPlaceholderElement(v cty.Value, schema *configschema.Block) bool { + if v.IsNull() { + return false // null value can never be a placeholder element + } + if !v.IsKnown() { + return true // this should never happen for well-behaved providers, but can happen with the legacy SDK opt-outs + } + for name := range schema.Attributes { + av := v.GetAttr(name) + + // Unknown block placeholders contain only unknown or null attribute + // values, depending on whether or not a particular attribute was set + // explicitly inside the content block. Note that this is imprecise: + // non-placeholders can also match this, so this function can generate + // false positives. + if av.IsKnown() && !av.IsNull() { + return false + } + } + for name, blockS := range schema.BlockTypes { + if !couldHaveUnknownBlockPlaceholder(v.GetAttr(name), blockS, true) { + return false + } + } + return true } // assertSetValuesCompatible checks that each of the elements in a can diff --git a/plans/objchange/compatible_test.go b/plans/objchange/compatible_test.go index 900edc08ad..e8af5458b5 100644 --- a/plans/objchange/compatible_test.go +++ b/plans/objchange/compatible_test.go @@ -12,6 +12,29 @@ import ( ) func TestAssertObjectCompatible(t *testing.T) { + schemaWithFoo := configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "foo": {Type: cty.String, Optional: true}, + }, + } + fooBlockValue := cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("bar"), + }) + schemaWithFooBar := configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "foo": {Type: cty.String, Optional: true}, + "bar": {Type: cty.String, Optional: true}, + }, + } + fooBarBlockValue := cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("bar"), + "bar": cty.NullVal(cty.String), // simulating the situation where bar isn't set in the config at all + }) + fooBarBlockDynamicPlaceholder := cty.ObjectVal(map[string]cty.Value{ + "foo": cty.UnknownVal(cty.String), + "bar": cty.NullVal(cty.String), // simulating the situation where bar isn't set in the config at all + }) + tests := []struct { Schema *configschema.Block Planned cty.Value @@ -681,11 +704,9 @@ func TestAssertObjectCompatible(t *testing.T) { }, }, cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), "key": cty.EmptyObjectVal, }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), "key": cty.EmptyObjectVal, }), nil, @@ -700,11 +721,9 @@ func TestAssertObjectCompatible(t *testing.T) { }, }, cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), "key": cty.UnknownVal(cty.EmptyObject), }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), "key": cty.EmptyObjectVal, }), nil, @@ -806,20 +825,18 @@ func TestAssertObjectCompatible(t *testing.T) { BlockTypes: map[string]*configschema.NestedBlock{ "key": { Nesting: configschema.NestingList, - Block: configschema.Block{}, + Block: schemaWithFoo, }, }, }, cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), "key": cty.ListVal([]cty.Value{ - cty.EmptyObjectVal, + fooBlockValue, }), }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), "key": cty.ListVal([]cty.Value{ - cty.EmptyObjectVal, + fooBlockValue, }), }), nil, @@ -829,21 +846,19 @@ func TestAssertObjectCompatible(t *testing.T) { BlockTypes: map[string]*configschema.NestedBlock{ "key": { Nesting: configschema.NestingList, - Block: configschema.Block{}, + Block: schemaWithFoo, }, }, }, cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), "key": cty.TupleVal([]cty.Value{ - cty.EmptyObjectVal, - cty.EmptyObjectVal, + fooBlockValue, + fooBlockValue, }), }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), "key": cty.TupleVal([]cty.Value{ - cty.EmptyObjectVal, + fooBlockValue, }), }), []string{ @@ -855,25 +870,77 @@ func TestAssertObjectCompatible(t *testing.T) { BlockTypes: map[string]*configschema.NestedBlock{ "key": { Nesting: configschema.NestingList, - Block: configschema.Block{}, + Block: schemaWithFoo, }, }, }, cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), "key": cty.TupleVal([]cty.Value{}), }), cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), "key": cty.TupleVal([]cty.Value{ - cty.EmptyObjectVal, - cty.EmptyObjectVal, + fooBlockValue, + fooBlockValue, }), }), []string{ `.key: block count changed from 0 to 2`, }, }, + { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "key": { + Nesting: configschema.NestingList, + Block: schemaWithFooBar, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "key": cty.ListVal([]cty.Value{ + fooBarBlockDynamicPlaceholder, // the presence of this disables some of our checks + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "key": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("hello"), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("world"), + }), + }), + }), + nil, // a single block whose attrs are all unknown is allowed to expand into multiple, because that's how dynamic blocks behave when for_each is unknown + }, + { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "key": { + Nesting: configschema.NestingList, + Block: schemaWithFooBar, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "key": cty.ListVal([]cty.Value{ + fooBarBlockValue, // the presence of one static block does not negate that the following element looks like a dynamic placeholder + fooBarBlockDynamicPlaceholder, // the presence of this disables some of our checks + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "key": cty.ListVal([]cty.Value{ + fooBlockValue, + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("hello"), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("world"), + }), + }), + }), + nil, // as above, the presence of a block whose attrs are all unknown indicates dynamic block expansion, so our usual count checks don't apply + }, // NestingSet blocks { @@ -881,14 +948,7 @@ func TestAssertObjectCompatible(t *testing.T) { BlockTypes: map[string]*configschema.NestedBlock{ "block": { Nesting: configschema.NestingSet, - Block: configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "foo": { - Type: cty.String, - Optional: true, - }, - }, - }, + Block: schemaWithFoo, }, }, }, @@ -919,14 +979,7 @@ func TestAssertObjectCompatible(t *testing.T) { BlockTypes: map[string]*configschema.NestedBlock{ "block": { Nesting: configschema.NestingSet, - Block: configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "foo": { - Type: cty.String, - Optional: true, - }, - }, - }, + Block: schemaWithFoo, }, }, }, @@ -957,14 +1010,7 @@ func TestAssertObjectCompatible(t *testing.T) { BlockTypes: map[string]*configschema.NestedBlock{ "block": { Nesting: configschema.NestingSet, - Block: configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "foo": { - Type: cty.String, - Optional: true, - }, - }, - }, + Block: schemaWithFoo, }, }, }, @@ -995,14 +1041,7 @@ func TestAssertObjectCompatible(t *testing.T) { BlockTypes: map[string]*configschema.NestedBlock{ "block": { Nesting: configschema.NestingSet, - Block: configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "foo": { - Type: cty.String, - Optional: true, - }, - }, - }, + Block: schemaWithFoo, }, }, }, @@ -1038,14 +1077,7 @@ func TestAssertObjectCompatible(t *testing.T) { BlockTypes: map[string]*configschema.NestedBlock{ "block": { Nesting: configschema.NestingSet, - Block: configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "foo": { - Type: cty.String, - Optional: true, - }, - }, - }, + Block: schemaWithFoo, }, }, }, @@ -1082,14 +1114,7 @@ func TestAssertObjectCompatible(t *testing.T) { BlockTypes: map[string]*configschema.NestedBlock{ "block": { Nesting: configschema.NestingSet, - Block: configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "foo": { - Type: cty.String, - Optional: true, - }, - }, - }, + Block: schemaWithFoo, }, }, }, @@ -1112,8 +1137,8 @@ func TestAssertObjectCompatible(t *testing.T) { }, } - for _, test := range tests { - t.Run(fmt.Sprintf("%#v and %#v", test.Planned, test.Actual), func(t *testing.T) { + for i, test := range tests { + t.Run(fmt.Sprintf("%02d: %#v and %#v", i, test.Planned, test.Actual), func(t *testing.T) { errs := AssertObjectCompatible(test.Schema, test.Planned, test.Actual) wantErrs := make(map[string]struct{})