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

Merge pull request #28424 from hashicorp/jbardin/dynamic 

Better planing of unknown dynamic blocks
This commit is contained in:
James Bardin 2021-04-23 18:26:21 -04:00 committed by GitHub
parent 0830e8be70 f8493bf5cd
commit 4997b9b5bb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 152 additions and 268 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
configs/configschema/decoder_spec.go
View File

@ -101,15 +101,6 @@ func (b *Block) DecoderSpec() hcldec.Spec {
childSpec := blockS.Block.DecoderSpec()
// We can only validate 0 or 1 for MinItems, because a dynamic block
// may satisfy any number of min items while only having a single
// block in the config. We cannot validate MaxItems because a
// configuration may have any number of dynamic blocks.
minItems := 0
if blockS.MinItems > 1 {
minItems = 1
}
switch blockS.Nesting {
case NestingSingle, NestingGroup:
ret[name] = &hcldec.BlockSpec{
@ -134,13 +125,15 @@ func (b *Block) DecoderSpec() hcldec.Spec {
ret[name] = &hcldec.BlockTupleSpec{
TypeName: name,
Nested: childSpec,
MinItems: minItems,
MinItems: blockS.MinItems,
MaxItems: blockS.MaxItems,
}
} else {
ret[name] = &hcldec.BlockListSpec{
TypeName: name,
Nested: childSpec,
MinItems: minItems,
MinItems: blockS.MinItems,
MaxItems: blockS.MaxItems,
}
}
case NestingSet:
@ -154,7 +147,8 @@ func (b *Block) DecoderSpec() hcldec.Spec {
ret[name] = &hcldec.BlockSetSpec{
TypeName: name,
Nested: childSpec,
MinItems: minItems,
MinItems: blockS.MinItems,
MaxItems: blockS.MaxItems,
}
case NestingMap:
// We prefer to use a list where possible, since it makes our

24
configs/configschema/decoder_spec_test.go
View File

@ -344,15 +344,12 @@ func TestBlockDecoderSpec(t *testing.T) {
},
&hcl.Block{
Type: "foo",
Body: hcl.EmptyBody(),
Body: unknownBody{hcl.EmptyBody()},
},
},
}),
cty.ObjectVal(map[string]cty.Value{
"foo": cty.ListVal([]cty.Value{
cty.EmptyObjectVal,
cty.EmptyObjectVal,
}),
"foo": cty.UnknownVal(cty.List(cty.EmptyObject)),
}),
0, // max items cannot be validated during decode
},
@ -372,14 +369,12 @@ func TestBlockDecoderSpec(t *testing.T) {
Blocks: hcl.Blocks{
&hcl.Block{
Type: "foo",
Body: hcl.EmptyBody(),
Body: unknownBody{hcl.EmptyBody()},
},
},
}),
cty.ObjectVal(map[string]cty.Value{
"foo": cty.ListVal([]cty.Value{
cty.EmptyObjectVal,
}),
"foo": cty.UnknownVal(cty.List(cty.EmptyObject)),
}),
0,
},
@ -401,6 +396,7 @@ func TestBlockDecoderSpec(t *testing.T) {
for name, test := range tests {
t.Run(name, func(t *testing.T) {
spec := test.Schema.DecoderSpec()
got, diags := hcldec.Decode(test.TestBody, spec, nil)
if len(diags) != test.DiagCount {
t.Errorf("wrong number of diagnostics %d; want %d", len(diags), test.DiagCount)
@ -427,6 +423,16 @@ func TestBlockDecoderSpec(t *testing.T) {
}
}
// this satisfies hcldec.UnknownBody to simulate a dynamic block with an
// unknown number of values.
type unknownBody struct {
hcl.Body
}
func (b unknownBody) Unknown() bool {
return true
}
func TestAttributeDecoderSpec(t *testing.T) {
tests := map[string]struct {
Schema *Attribute

2
go.mod
View File

@ -65,7 +65,7 @@ require (
github.com/hashicorp/go-uuid v1.0.1
github.com/hashicorp/go-version v1.2.1
github.com/hashicorp/hcl v0.0.0-20170504190234-a4b07c25de5f
github.com/hashicorp/hcl/v2 v2.9.1
github.com/hashicorp/hcl/v2 v2.10.0
github.com/hashicorp/memberlist v0.1.0 // indirect
github.com/hashicorp/serf v0.0.0-20160124182025-e4ec8cc423bb // indirect
github.com/hashicorp/terraform-config-inspect v0.0.0-20210209133302-4fd17a0faac2

4
go.sum
View File

@ -358,8 +358,8 @@ github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ
github.com/hashicorp/hcl v0.0.0-20170504190234-a4b07c25de5f h1:UdxlrJz4JOnY8W+DbLISwf2B8WXEolNRA8BGCwI9jws=
github.com/hashicorp/hcl v0.0.0-20170504190234-a4b07c25de5f/go.mod h1:oZtUIOe8dh44I2q6ScRibXws4Ajl+d+nod3AaR9vL5w=
github.com/hashicorp/hcl/v2 v2.0.0/go.mod h1:oVVDG71tEinNGYCxinCYadcmKU9bglqW9pV3txagJ90=
github.com/hashicorp/hcl/v2 v2.9.1 h1:eOy4gREY0/ZQHNItlfuEZqtcQbXIxzojlP301hDpnac=
github.com/hashicorp/hcl/v2 v2.9.1/go.mod h1:FwWsfWEjyV/CMj8s/gqAuiviY72rJ1/oayI9WftqcKg=
github.com/hashicorp/hcl/v2 v2.10.0 h1:1S1UnuhDGlv3gRFV4+0EdwB+znNP5HmcGbIqwnSCByg=
github.com/hashicorp/hcl/v2 v2.10.0/go.mod h1:FwWsfWEjyV/CMj8s/gqAuiviY72rJ1/oayI9WftqcKg=
github.com/hashicorp/memberlist v0.1.0 h1:qSsCiC0WYD39lbSitKNt40e30uorm2Ss/d4JGU1hzH8=
github.com/hashicorp/memberlist v0.1.0/go.mod h1:ncdBp14cuox2iFOq3kDiquKU6fqsTBc3W6JvZwjxxsE=
github.com/hashicorp/serf v0.0.0-20160124182025-e4ec8cc423bb h1:ZbgmOQt8DOg796figP87/EFCVx2v2h9yRvwHF/zceX4=

11
lang/blocktoattr/fixup.go
View File

@ -41,6 +41,17 @@ type fixupBody struct {
names map[string]struct{}
}
type unknownBlock interface {
Unknown() bool
}
func (b *fixupBody) Unknown() bool {
if u, ok := b.original.(unknownBlock); ok {
return u.Unknown()
}
return false
}
// Content decodes content from the body. The given schema must be the lower-level
// representation of the same schema that was previously passed to FixUpBlockAttrs,
// or else the result is undefined.

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 {

92
terraform/context_validate_test.go
View File

@ -2000,3 +2000,95 @@ func TestContext2Validate_sensitiveProvisionerConfig(t *testing.T) {
t.Fatal("ValidateProvisionerConfig not called")
}
}
func TestContext2Plan_validateMinMaxDynamicBlock(t *testing.T) {
p := new(MockProvider)
p.GetProviderSchemaResponse = getProviderSchemaResponseFromProviderSchema(&ProviderSchema{
ResourceTypes: map[string]*configschema.Block{
"test_instance": {
Attributes: map[string]*configschema.Attribute{
"id": {
Type: cty.String,
Computed: true,
},
"things": {
Type: cty.List(cty.String),
Computed: true,
},
},
BlockTypes: map[string]*configschema.NestedBlock{
"foo": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"bar": {Type: cty.String, Optional: true},
},
},
Nesting: configschema.NestingList,
MinItems: 2,
MaxItems: 3,
},
},
},
},
})
m := testModuleInline(t, map[string]string{
"main.tf": `
resource "test_instance" "a" {
// MinItems 2
foo {
bar = "a"
}
foo {
bar = "b"
}
}
resource "test_instance" "b" {
// one dymamic block can satisfy MinItems of 2
dynamic "foo" {
for_each = test_instance.a.things
content {
bar = foo.value
}
}
}
resource "test_instance" "c" {
// we may have more than MaxItems dynamic blocks when they are unknown
foo {
bar = "b"
}
dynamic "foo" {
for_each = test_instance.a.things
content {
bar = foo.value
}
}
dynamic "foo" {
for_each = test_instance.a.things
content {
bar = "${foo.value}-2"
}
}
dynamic "foo" {
for_each = test_instance.b.things
content {
bar = foo.value
}
}
}
`})
ctx := testContext2(t, &ContextOpts{
Config: m,
Providers: map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
},
})
diags := ctx.Validate()
if diags.HasErrors() {
t.Fatal(diags.ErrWithWarnings())
}
}