From 8fbb4d01634c4ebb389fbd926064d983e47e13e6 Mon Sep 17 00:00:00 2001 From: Alexander Ovechkin Date: Fri, 4 Sep 2020 15:46:40 +0300 Subject: [PATCH 1/5] Converting ListVal to ListVal instead of TupleVal in setElementCompareValue --- plans/objchange/objchange.go | 4 +- plans/objchange/objchange_test.go | 64 +++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/plans/objchange/objchange.go b/plans/objchange/objchange.go index 5a8af1481f..6d88de97cb 100644 --- a/plans/objchange/objchange.go +++ b/plans/objchange/objchange.go @@ -357,13 +357,13 @@ func setElementCompareValue(schema *configschema.Block, v cty.Value, isConfig bo // set and we've not changed the types of any elements here. attrs[name] = cty.SetVal(elems) } else { - attrs[name] = cty.TupleVal(elems) + attrs[name] = cty.ListVal(elems) } } else { if blockType.Nesting == configschema.NestingSet { attrs[name] = cty.SetValEmpty(blockType.Block.ImpliedType()) } else { - attrs[name] = cty.EmptyTupleVal + attrs[name] = cty.ListValEmpty(blockType.Block.ImpliedType()) } } diff --git a/plans/objchange/objchange_test.go b/plans/objchange/objchange_test.go index 97f749acae..3e0784808f 100644 --- a/plans/objchange/objchange_test.go +++ b/plans/objchange/objchange_test.go @@ -572,6 +572,70 @@ func TestProposedNewObject(t *testing.T) { }), }), }, + "nested list in sed": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "foo": { + Nesting: configschema.NestingSet, + Block: configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "bar": { + Nesting: configschema.NestingList, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "baz": { + Type: cty.String, + }, + "qux": { + Type: cty.String, + Computed: true, + Optional: true, + }, + }, + }, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "baz": cty.StringVal("beep"), + "qux": cty.StringVal("boop"), + }), + }), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "baz": cty.StringVal("beep"), + "qux": cty.NullVal(cty.String), + }), + }), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "baz": cty.StringVal("beep"), + "qux": cty.StringVal("boop"), + }), + }), + }), + }), + }), + }, } for name, test := range tests { From d7db008df2ffb63b38ff3f96bc51e334e8c104a7 Mon Sep 17 00:00:00 2001 From: Alexander Ovechkin Date: Mon, 7 Sep 2020 05:16:31 +0300 Subject: [PATCH 2/5] added empty list test case --- plans/objchange/objchange_test.go | 40 ++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/plans/objchange/objchange_test.go b/plans/objchange/objchange_test.go index 3e0784808f..5106feb1fb 100644 --- a/plans/objchange/objchange_test.go +++ b/plans/objchange/objchange_test.go @@ -572,7 +572,7 @@ func TestProposedNewObject(t *testing.T) { }), }), }, - "nested list in sed": { + "nested list in set": { &configschema.Block{ BlockTypes: map[string]*configschema.NestedBlock{ "foo": { @@ -636,6 +636,44 @@ func TestProposedNewObject(t *testing.T) { }), }), }, + "empty nested list in set": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "foo": { + Nesting: configschema.NestingSet, + Block: configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "bar": { + Nesting: configschema.NestingList, + Block: configschema.Block{}, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.ListValEmpty((&configschema.Block{}).ImpliedType()), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.ListValEmpty((&configschema.Block{}).ImpliedType()), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.ListValEmpty((&configschema.Block{}).ImpliedType()), + }), + }), + }), + }, } for name, test := range tests { From f128b8c4faca0c57296455d601298dc208e69e6a Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 15 Oct 2020 20:03:02 -0400 Subject: [PATCH 3/5] take dynamic types into account when comparing set If a NestingList or NestingMap contains a dynamic type, they must be handled as a cty.Tuple and cty.Object respectively, because the elements may not have precisely matching types. --- plans/objchange/objchange.go | 18 +++- plans/objchange/objchange_test.go | 131 +++++++++++++++++++++++++++++- 2 files changed, 143 insertions(+), 6 deletions(-) diff --git a/plans/objchange/objchange.go b/plans/objchange/objchange.go index 6d88de97cb..27ef709b01 100644 --- a/plans/objchange/objchange.go +++ b/plans/objchange/objchange.go @@ -357,13 +357,21 @@ func setElementCompareValue(schema *configschema.Block, v cty.Value, isConfig bo // set and we've not changed the types of any elements here. attrs[name] = cty.SetVal(elems) } else { - attrs[name] = cty.ListVal(elems) + if blockType.Block.ImpliedType().HasDynamicTypes() { + attrs[name] = cty.TupleVal(elems) + } else { + attrs[name] = cty.ListVal(elems) + } } } else { if blockType.Nesting == configschema.NestingSet { attrs[name] = cty.SetValEmpty(blockType.Block.ImpliedType()) } else { - attrs[name] = cty.ListValEmpty(blockType.Block.ImpliedType()) + if blockType.Block.ImpliedType().HasDynamicTypes() { + attrs[name] = cty.EmptyTupleVal + } else { + attrs[name] = cty.ListValEmpty(blockType.Block.ImpliedType()) + } } } @@ -378,7 +386,11 @@ func setElementCompareValue(schema *configschema.Block, v cty.Value, isConfig bo kv, ev := it.Element() elems[kv.AsString()] = setElementCompareValue(&blockType.Block, ev, isConfig) } - attrs[name] = cty.ObjectVal(elems) + if blockType.Block.ImpliedType().HasDynamicTypes() { + attrs[name] = cty.ObjectVal(elems) + } else { + attrs[name] = cty.MapVal(elems) + } default: // Should never happen, since the above cases are comprehensive. diff --git a/plans/objchange/objchange_test.go b/plans/objchange/objchange_test.go index 5106feb1fb..d6352337be 100644 --- a/plans/objchange/objchange_test.go +++ b/plans/objchange/objchange_test.go @@ -440,7 +440,7 @@ func TestProposedNewObject(t *testing.T) { }), "b": cty.ObjectVal(map[string]cty.Value{ "bar": cty.StringVal("blep"), - "baz": cty.StringVal("boot"), + "baz": cty.ListVal([]cty.Value{cty.StringVal("boot")}), }), }), }), @@ -452,7 +452,7 @@ func TestProposedNewObject(t *testing.T) { }), "c": cty.ObjectVal(map[string]cty.Value{ "bar": cty.StringVal("bosh"), - "baz": cty.NullVal(cty.String), + "baz": cty.NullVal(cty.List(cty.String)), }), }), }), @@ -464,7 +464,7 @@ func TestProposedNewObject(t *testing.T) { }), "c": cty.ObjectVal(map[string]cty.Value{ "bar": cty.StringVal("bosh"), - "baz": cty.NullVal(cty.String), + "baz": cty.NullVal(cty.List(cty.String)), }), }), }), @@ -674,6 +674,131 @@ func TestProposedNewObject(t *testing.T) { }), }), }, + "nested list with dynamic in set": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "foo": { + Nesting: configschema.NestingSet, + Block: configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "bar": { + Nesting: configschema.NestingList, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "baz": { + Type: cty.DynamicPseudoType, + }, + }, + }, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.TupleVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "baz": cty.StringVal("true"), + }), + cty.ObjectVal(map[string]cty.Value{ + "baz": cty.ListVal([]cty.Value{cty.StringVal("true")}), + }), + }), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.TupleVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "baz": cty.StringVal("true"), + }), + cty.ObjectVal(map[string]cty.Value{ + "baz": cty.ListVal([]cty.Value{cty.StringVal("true")}), + }), + }), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.TupleVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "baz": cty.StringVal("true"), + }), + cty.ObjectVal(map[string]cty.Value{ + "baz": cty.ListVal([]cty.Value{cty.StringVal("true")}), + }), + }), + }), + }), + }), + }, + "nested map with dynamic in set": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "foo": { + Nesting: configschema.NestingSet, + Block: configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "bar": { + Nesting: configschema.NestingMap, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "baz": { + Type: cty.DynamicPseudoType, + Optional: true, + }, + }, + }, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.ObjectVal(map[string]cty.Value{ + "bing": cty.ObjectVal(map[string]cty.Value{ + "baz": cty.StringVal("true"), + }), + "bang": cty.ObjectVal(map[string]cty.Value{ + "baz": cty.ListVal([]cty.Value{cty.StringVal("true")}), + }), + }), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.ObjectVal(map[string]cty.Value{ + "bing": cty.ObjectVal(map[string]cty.Value{ + "baz": cty.ListVal([]cty.Value{cty.StringVal("true")}), + }), + }), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.ObjectVal(map[string]cty.Value{ + "bing": cty.ObjectVal(map[string]cty.Value{ + "baz": cty.ListVal([]cty.Value{cty.StringVal("true")}), + }), + }), + }), + }), + }), + }, } for name, test := range tests { From b59c64245bea9d22c3f6d0b44414e86dd32e7780 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 15 Oct 2020 20:55:56 -0400 Subject: [PATCH 4/5] refactor ifs to reduce indentation --- plans/objchange/objchange.go | 37 ++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/plans/objchange/objchange.go b/plans/objchange/objchange.go index 27ef709b01..b833e85202 100644 --- a/plans/objchange/objchange.go +++ b/plans/objchange/objchange.go @@ -344,34 +344,38 @@ func setElementCompareValue(schema *configschema.Block, v cty.Value, isConfig bo attrs[name] = cv continue } + if l := cv.LengthInt(); l > 0 { elems := make([]cty.Value, 0, l) for it := cv.ElementIterator(); it.Next(); { _, ev := it.Element() elems = append(elems, setElementCompareValue(&blockType.Block, ev, isConfig)) } - if blockType.Nesting == configschema.NestingSet { + + switch { + case blockType.Nesting == configschema.NestingSet: // SetValEmpty would panic if given elements that are not // all of the same type, but that's guaranteed not to // happen here because our input value was _already_ a // set and we've not changed the types of any elements here. attrs[name] = cty.SetVal(elems) - } else { - if blockType.Block.ImpliedType().HasDynamicTypes() { - attrs[name] = cty.TupleVal(elems) - } else { - attrs[name] = cty.ListVal(elems) - } + + // NestingList cases + case blockType.Block.ImpliedType().HasDynamicTypes(): + attrs[name] = cty.TupleVal(elems) + default: + attrs[name] = cty.ListVal(elems) } } else { - if blockType.Nesting == configschema.NestingSet { + switch { + case blockType.Nesting == configschema.NestingSet: attrs[name] = cty.SetValEmpty(blockType.Block.ImpliedType()) - } else { - if blockType.Block.ImpliedType().HasDynamicTypes() { - attrs[name] = cty.EmptyTupleVal - } else { - attrs[name] = cty.ListValEmpty(blockType.Block.ImpliedType()) - } + + // NestingList cases + case blockType.Block.ImpliedType().HasDynamicTypes(): + attrs[name] = cty.EmptyTupleVal + default: + attrs[name] = cty.ListValEmpty(blockType.Block.ImpliedType()) } } @@ -386,9 +390,10 @@ func setElementCompareValue(schema *configschema.Block, v cty.Value, isConfig bo kv, ev := it.Element() elems[kv.AsString()] = setElementCompareValue(&blockType.Block, ev, isConfig) } - if blockType.Block.ImpliedType().HasDynamicTypes() { + switch { + case blockType.Block.ImpliedType().HasDynamicTypes(): attrs[name] = cty.ObjectVal(elems) - } else { + default: attrs[name] = cty.MapVal(elems) } From 77af322c1ce7bfc873734877f6312852b7c4207b Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 15 Oct 2020 21:21:14 -0400 Subject: [PATCH 5/5] handle non-null, but empty NestingMap in a set --- plans/objchange/objchange.go | 16 +++++---- plans/objchange/objchange_test.go | 55 +++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/plans/objchange/objchange.go b/plans/objchange/objchange.go index b833e85202..ab16f68a4b 100644 --- a/plans/objchange/objchange.go +++ b/plans/objchange/objchange.go @@ -333,8 +333,9 @@ func setElementCompareValue(schema *configschema.Block, v cty.Value, isConfig bo } for name, blockType := range schema.BlockTypes { - switch blockType.Nesting { + elementType := blockType.Block.ImpliedType() + switch blockType.Nesting { case configschema.NestingSingle, configschema.NestingGroup: attrs[name] = setElementCompareValue(&blockType.Block, v.GetAttr(name), isConfig) @@ -361,7 +362,7 @@ func setElementCompareValue(schema *configschema.Block, v cty.Value, isConfig bo attrs[name] = cty.SetVal(elems) // NestingList cases - case blockType.Block.ImpliedType().HasDynamicTypes(): + case elementType.HasDynamicTypes(): attrs[name] = cty.TupleVal(elems) default: attrs[name] = cty.ListVal(elems) @@ -369,19 +370,19 @@ func setElementCompareValue(schema *configschema.Block, v cty.Value, isConfig bo } else { switch { case blockType.Nesting == configschema.NestingSet: - attrs[name] = cty.SetValEmpty(blockType.Block.ImpliedType()) + attrs[name] = cty.SetValEmpty(elementType) // NestingList cases - case blockType.Block.ImpliedType().HasDynamicTypes(): + case elementType.HasDynamicTypes(): attrs[name] = cty.EmptyTupleVal default: - attrs[name] = cty.ListValEmpty(blockType.Block.ImpliedType()) + attrs[name] = cty.ListValEmpty(elementType) } } case configschema.NestingMap: cv := v.GetAttr(name) - if cv.IsNull() || !cv.IsKnown() { + if cv.IsNull() || !cv.IsKnown() || cv.LengthInt() == 0 { attrs[name] = cv continue } @@ -390,8 +391,9 @@ func setElementCompareValue(schema *configschema.Block, v cty.Value, isConfig bo kv, ev := it.Element() elems[kv.AsString()] = setElementCompareValue(&blockType.Block, ev, isConfig) } + switch { - case blockType.Block.ImpliedType().HasDynamicTypes(): + case elementType.HasDynamicTypes(): attrs[name] = cty.ObjectVal(elems) default: attrs[name] = cty.MapVal(elems) diff --git a/plans/objchange/objchange_test.go b/plans/objchange/objchange_test.go index d6352337be..065e4add88 100644 --- a/plans/objchange/objchange_test.go +++ b/plans/objchange/objchange_test.go @@ -799,6 +799,61 @@ func TestProposedNewObject(t *testing.T) { }), }), }, + "empty nested map in set": { + &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "foo": { + Nesting: configschema.NestingSet, + Block: configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "bar": { + Nesting: configschema.NestingMap, + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "baz": { + Type: cty.String, + Optional: true, + }, + }, + }, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.MapValEmpty(cty.Object(map[string]cty.Type{ + "baz": cty.String, + })), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.MapVal(map[string]cty.Value{ + "bing": cty.ObjectVal(map[string]cty.Value{ + "baz": cty.StringVal("true"), + }), + }), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.MapVal(map[string]cty.Value{ + "bing": cty.ObjectVal(map[string]cty.Value{ + "baz": cty.StringVal("true"), + }), + }), + }), + }), + }), + }, } for name, test := range tests {