resilien
/
terraform
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

use new dynamic decoding behavior 

Dynamic blocks with unknown for_each expressions are now decoded into an
unknown value rather than using a sentinel object with unknown
and null attributes. This will allow providers to precisely plan the
block values, rather than trying to heuristically paper over the
incorrect plans when dynamic is in use.
This commit is contained in:
James Bardin 2021-04-13 18:43:32 -04:00
parent b515ab583a
commit 59c8281378
3 changed files with 25 additions and 244 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

114
plans/objchange/compatible.go
View File

@ -75,20 +75,12 @@ func assertObjectCompatible(schema *configschema.Block, planned, actual cty.Valu
plannedV, _ := planned.GetAttr(name).Unmark()
actualV, _ := actual.GetAttr(name).Unmark()
// 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() {
if !plannedV.IsKnown() && actualV.IsNull() {
continue
}
moreErrs := assertObjectCompatible(&blockS.Block, plannedV, actualV, path)
@ -102,14 +94,6 @@ 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 {
@ -144,7 +128,7 @@ 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...)
}
if !maybeUnknownBlocks { // new blocks may appear if unknown blocks were present in the plan
if plannedV.IsKnown() { // 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))
@ -158,7 +142,7 @@ func assertObjectCompatible(schema *configschema.Block, planned, actual cty.Valu
}
plannedL := plannedV.LengthInt()
actualL := actualV.LengthInt()
if plannedL != actualL && !maybeUnknownBlocks { // new blocks may appear if unknown blocks were persent in the plan
if plannedL != actualL && plannedV.IsKnown() { // 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
}
@ -177,7 +161,7 @@ func assertObjectCompatible(schema *configschema.Block, planned, actual cty.Valu
continue
}
if maybeUnknownBlocks {
if !plannedV.IsKnown() {
// When unknown blocks are present the final number of blocks
// may be different, either because the unknown set values
// become equal and are collapsed, or the count is unknown due
@ -328,96 +312,6 @@ func indexStrForErrors(v cty.Value) string {
}
}
// 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
}
return couldBeUnknownBlockPlaceholderElement(v, blockS)
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
}
if v.IsNull() {
return false // treated as if the list were empty, so we would see zero iterations below
}
// Unmark before we call ElementIterator in case this iterable is marked sensitive.
// This can arise in the case where a member of a Set is sensitive, and thus the
// whole Set is marked sensitive
v, _ := v.Unmark()
// For all other nesting modes, our value should be something iterable.
for it := v.ElementIterator(); it.Next(); {
_, ev := it.Element()
if couldBeUnknownBlockPlaceholderElement(ev, blockS) {
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.NestedBlock) 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() {
// FIXME: only required for the legacy SDK, but we don't have a
// separate codepath to switch the comparisons, and we still want
// the rest of the checks from AssertObjectCompatible to apply.
//
// The legacy SDK cannot handle missing strings from set elements,
// and will insert an empty string into the planned value.
// Skipping these treats them as null values in this case,
// preventing false alerts from AssertObjectCompatible.
if schema.Nesting == configschema.NestingSet && av.Type() == cty.String && av.AsString() == "" {
continue
}
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
// be correlated with at least one equivalent element in b and vice-versa,
// using the given correlation function.

147
plans/objchange/compatible_test.go
View File

@ -30,10 +30,6 @@ func TestAssertObjectCompatible(t *testing.T) {
"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
@ -919,13 +915,11 @@ func TestAssertObjectCompatible(t *testing.T) {
},
},
},
cty.ObjectVal(map[string]cty.Value{
"key": cty.ObjectVal(map[string]cty.Value{
// One wholly unknown block is what "dynamic" blocks
// generate when the for_each expression is unknown.
"foo": cty.UnknownVal(cty.String),
cty.UnknownVal(cty.Object(map[string]cty.Type{
"key": cty.Object(map[string]cty.Type{
"foo": cty.String,
}),
}),
})),
cty.ObjectVal(map[string]cty.Value{
"key": cty.NullVal(cty.Object(map[string]cty.Type{
"foo": cty.String,
@ -1011,11 +1005,9 @@ func TestAssertObjectCompatible(t *testing.T) {
},
},
},
cty.ObjectVal(map[string]cty.Value{
"key": cty.ListVal([]cty.Value{
fooBarBlockDynamicPlaceholder, // the presence of this disables some of our checks
}),
}),
cty.UnknownVal(cty.Object(map[string]cty.Type{
"key": cty.List(fooBarBlockValue.Type()),
})),
cty.ObjectVal(map[string]cty.Value{
"key": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
@ -1026,35 +1018,7 @@ func TestAssertObjectCompatible(t *testing.T) {
}),
}),
}),
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
nil, // an unknown block is allowed to expand into multiple, because that's how dynamic blocks behave when for_each is unknown
},
{
&configschema.Block{
@ -1195,14 +1159,11 @@ func TestAssertObjectCompatible(t *testing.T) {
},
},
cty.ObjectVal(map[string]cty.Value{
"block": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"foo": cty.UnknownVal(cty.String),
"block": cty.UnknownVal(cty.Set(
cty.Object(map[string]cty.Type{
"foo": cty.String,
}),
cty.ObjectVal(map[string]cty.Value{
"foo": cty.UnknownVal(cty.String),
}),
}),
)),
}),
cty.ObjectVal(map[string]cty.Value{
"block": cty.SetVal([]cty.Value{
@ -1221,47 +1182,6 @@ func TestAssertObjectCompatible(t *testing.T) {
// indicates this may be a dynamic block, and the length is unknown
nil,
},
{
&configschema.Block{
BlockTypes: map[string]*configschema.NestedBlock{
"block": {
Nesting: configschema.NestingSet,
Block: schemaWithFooBar,
},
},
},
// The legacy SDK cannot handle missing strings in sets, and will
// insert empty strings to the planned value. Empty strings should
// be handled as nulls, and this object should represent a possible
// dynamic block.
cty.ObjectVal(map[string]cty.Value{
"block": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"foo": cty.UnknownVal(cty.String),
"bar": cty.StringVal(""),
}),
}),
}),
cty.ObjectVal(map[string]cty.Value{
"block": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"foo": cty.StringVal("hello"),
"bar": cty.StringVal(""),
}),
cty.ObjectVal(map[string]cty.Value{
"foo": cty.StringVal("world"),
"bar": cty.StringVal(""),
}),
cty.ObjectVal(map[string]cty.Value{
"foo": cty.StringVal("nope"),
"bar": cty.StringVal(""),
}),
}),
}),
// there is no error here, because the presence of unknowns
// indicates this may be a dynamic block, and the length is unknown
nil,
},
{
&configschema.Block{
BlockTypes: map[string]*configschema.NestedBlock{
@ -1335,11 +1255,7 @@ func TestAssertObjectCompatible(t *testing.T) {
},
},
cty.ObjectVal(map[string]cty.Value{
"block": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"foo": cty.UnknownVal(cty.String),
}),
}),
"block": cty.UnknownVal(cty.Set(fooBlockValue.Type())),
}),
cty.ObjectVal(map[string]cty.Value{
"block": cty.SetVal([]cty.Value{
@ -1364,11 +1280,7 @@ func TestAssertObjectCompatible(t *testing.T) {
},
},
cty.ObjectVal(map[string]cty.Value{
"block2": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"foo": cty.UnknownVal(cty.String),
}),
}),
"block2": cty.UnknownVal(cty.Set(fooBlockValue.Type())),
}),
cty.ObjectVal(map[string]cty.Value{
"block2": cty.SetValEmpty(cty.Object(map[string]cty.Type{
@ -1406,37 +1318,6 @@ func TestAssertObjectCompatible(t *testing.T) {
}),
nil,
},
{
&configschema.Block{
BlockTypes: map[string]*configschema.NestedBlock{
"block": {
Nesting: configschema.NestingSet,
Block: schemaWithFooBar,
},
},
},
cty.ObjectVal(map[string]cty.Value{
"block": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"foo": cty.UnknownVal(cty.String),
"bar": cty.NullVal(cty.String),
}),
}),
}),
cty.ObjectVal(map[string]cty.Value{
"block": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"foo": cty.StringVal("a"),
"bar": cty.StringVal(""),
}),
cty.ObjectVal(map[string]cty.Value{
"foo": cty.StringVal("b"),
"bar": cty.StringVal(""),
}),
}),
}),
nil,
},
{
&configschema.Block{
BlockTypes: map[string]*configschema.NestedBlock{

8
plans/objchange/objchange.go
View File

@ -93,6 +93,12 @@ func proposedNew(schema *configschema.Block, prior, config cty.Value) cty.Value
}
func proposedNewNestedBlock(schema *configschema.NestedBlock, prior, config cty.Value) cty.Value {
// The only time we should encounter an entirely unknown block is from the
// use of dynamic with an unknown for_each expression.
if !config.IsKnown() {
return config
}
var newV cty.Value
switch schema.Nesting {
@ -103,7 +109,7 @@ func proposedNewNestedBlock(schema *configschema.NestedBlock, prior, config cty.
case configschema.NestingList:
// Nested blocks are correlated by index.
configVLen := 0
if config.IsKnown() && !config.IsNull() {
if !config.IsNull() {
configVLen = config.LengthInt()
}
if configVLen > 0 {