Merge pull request #21077 from hashicorp/jbardin/map-shims

Fixes for maps in shims
This commit is contained in:
James Bardin 2019-04-23 14:41:26 -04:00 committed by GitHub
commit 64f419ac76
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 146 additions and 54 deletions

View File

@ -23,6 +23,16 @@ func testResourceMap() *schema.Resource {
Optional: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
"map_values": {
Type: schema.TypeMap,
Optional: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
"computed_map": {
Type: schema.TypeMap,
Computed: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
},
}
}
@ -35,15 +45,20 @@ func testResourceMapCreate(d *schema.ResourceData, meta interface{}) error {
}
d.SetId("testId")
return nil
return testResourceMapRead(d, meta)
}
func testResourceMapRead(d *schema.ResourceData, meta interface{}) error {
var computedMap map[string]interface{}
if v, ok := d.GetOk("map_values"); ok {
computedMap = v.(map[string]interface{})
}
d.Set("computed_map", computedMap)
return nil
}
func testResourceMapUpdate(d *schema.ResourceData, meta interface{}) error {
return nil
return testResourceMapRead(d, meta)
}
func testResourceMapDelete(d *schema.ResourceData, meta interface{}) error {

View File

@ -30,3 +30,80 @@ resource "test_resource_map" "foobar" {
},
})
}
func TestResourceMap_computedMap(t *testing.T) {
resource.UnitTest(t, resource.TestCase{
Providers: testAccProviders,
CheckDestroy: testAccCheckResourceDestroy,
Steps: []resource.TestStep{
{
Config: `
resource "test_resource_map" "foobar" {
name = "test"
map_of_three = {
one = "one"
two = "two"
empty = ""
}
map_values = {
a = "1"
b = "2"
}
}`,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"test_resource_map.foobar", "computed_map.a", "1",
),
resource.TestCheckResourceAttr(
"test_resource_map.foobar", "computed_map.b", "2",
),
),
},
{
Config: `
resource "test_resource_map" "foobar" {
name = "test"
map_of_three = {
one = "one"
two = "two"
empty = ""
}
map_values = {
a = "3"
b = "4"
}
}`,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"test_resource_map.foobar", "computed_map.a", "3",
),
resource.TestCheckResourceAttr(
"test_resource_map.foobar", "computed_map.b", "4",
),
),
},
{
Config: `
resource "test_resource_map" "foobar" {
name = "test"
map_of_three = {
one = "one"
two = "two"
empty = ""
}
map_values = {
a = "3"
}
}`,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"test_resource_map.foobar", "computed_map.a", "3",
),
resource.TestCheckNoResourceAttr(
"test_resource_map.foobar", "computed_map.b",
),
),
},
},
})
}

View File

@ -490,7 +490,7 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso
return resp, nil
}
newStateVal = normalizeNullValues(newStateVal, stateVal, true)
newStateVal = normalizeNullValues(newStateVal, stateVal, false)
newStateVal = copyTimeoutValues(newStateVal, stateVal)
newStateMP, err := msgpack.Marshal(newStateVal, blockForCore.ImpliedType())
@ -614,7 +614,7 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl
return resp, nil
}
plannedStateVal = normalizeNullValues(plannedStateVal, proposedNewStateVal, true)
plannedStateVal = normalizeNullValues(plannedStateVal, proposedNewStateVal, false)
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
@ -839,7 +839,7 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
return resp, nil
}
newStateVal = normalizeNullValues(newStateVal, plannedStateVal, false)
newStateVal = normalizeNullValues(newStateVal, plannedStateVal, true)
newStateVal = copyTimeoutValues(newStateVal, plannedStateVal)
@ -1108,16 +1108,13 @@ func stripSchema(s *schema.Schema) *schema.Schema {
// however it sees fit. This however means that a CustomizeDiffFunction may not
// be able to change a null to an empty value or vice versa, but that should be
// very uncommon nor was it reliable before 0.12 either.
func normalizeNullValues(dst, src cty.Value, preferDst bool) cty.Value {
func normalizeNullValues(dst, src cty.Value, apply bool) cty.Value {
ty := dst.Type()
if !src.IsNull() && !src.IsKnown() {
// While this seems backwards to return src when preferDst is set, it
// means this might be a plan scenario, and it must retain unknown
// interpolated placeholders, which could be lost if we're only updating
// a resource. If this is a read scenario, then there shouldn't be any
// unknowns all.
if dst.IsNull() && preferDst {
// Return src during plan to retain unknown interpolated placeholders,
// which could be lost if we're only updating a resource. If this is a
// read scenario, then there shouldn't be any unknowns all.
if dst.IsNull() && !apply {
return src
}
return dst
@ -1154,22 +1151,27 @@ func normalizeNullValues(dst, src cty.Value, preferDst bool) cty.Value {
srcMap := src.AsValueMap()
for key, v := range srcMap {
dstVal := dstMap[key]
dstVal, ok := dstMap[key]
if !ok && apply && ty.IsMapType() {
// don't transfer old map values to dst during apply
continue
}
if dstVal == cty.NilVal {
if preferDst && ty.IsMapType() {
if !apply && ty.IsMapType() {
// let plan shape this map however it wants
continue
}
dstVal = cty.NullVal(v.Type())
}
dstMap[key] = normalizeNullValues(dstVal, v, preferDst)
dstMap[key] = normalizeNullValues(dstVal, v, apply)
}
// you can't call MapVal/ObjectVal with empty maps, but nothing was
// copied in anyway. If the dst is nil, and the src is known, assume the
// src is correct.
if len(dstMap) == 0 {
if dst.IsNull() && src.IsWhollyKnown() && !preferDst {
if dst.IsNull() && src.IsWhollyKnown() && apply {
return src
}
return dst
@ -1203,7 +1205,7 @@ func normalizeNullValues(dst, src cty.Value, preferDst bool) cty.Value {
// If the original was wholly known, then we expect that is what the
// provider applied. The apply process loses too much information to
// reliably re-create the set.
if src.IsWhollyKnown() && !preferDst {
if src.IsWhollyKnown() && apply {
return src
}
@ -1211,14 +1213,13 @@ func normalizeNullValues(dst, src cty.Value, preferDst bool) cty.Value {
// If the dst is null, and the src is known, then we lost an empty value
// so take the original.
if dst.IsNull() {
if src.IsWhollyKnown() && src.LengthInt() == 0 && !preferDst {
if src.IsWhollyKnown() && src.LengthInt() == 0 && apply {
return src
}
// if dst is null and src only contains unknown values, then we lost
// those during a plan (which is when preferDst is set, there would
// be no unknowns during read).
if preferDst && !src.IsNull() {
// those during a read or plan.
if !apply && !src.IsNull() {
allUnknown := true
for _, v := range src.AsValueSlice() {
if v.IsKnown() {
@ -1242,7 +1243,7 @@ func normalizeNullValues(dst, src cty.Value, preferDst bool) cty.Value {
dsts := dst.AsValueSlice()
for i := 0; i < srcLen; i++ {
dsts[i] = normalizeNullValues(dsts[i], srcs[i], preferDst)
dsts[i] = normalizeNullValues(dsts[i], srcs[i], apply)
}
if ty.IsTupleType() {
@ -1252,7 +1253,7 @@ func normalizeNullValues(dst, src cty.Value, preferDst bool) cty.Value {
}
case ty.IsPrimitiveType():
if dst.IsNull() && src.IsWhollyKnown() && !preferDst {
if dst.IsNull() && src.IsWhollyKnown() && apply {
return src
}
}

View File

@ -667,7 +667,7 @@ func TestGetSchemaTimeouts(t *testing.T) {
func TestNormalizeNullValues(t *testing.T) {
for i, tc := range []struct {
Src, Dst, Expect cty.Value
Plan bool
Apply bool
}{
{
// The known set value is copied over the null set value
@ -690,6 +690,7 @@ func TestNormalizeNullValues(t *testing.T) {
}),
}),
}),
Apply: true,
},
{
// A zero set value is kept
@ -702,7 +703,6 @@ func TestNormalizeNullValues(t *testing.T) {
Expect: cty.ObjectVal(map[string]cty.Value{
"set": cty.SetValEmpty(cty.String),
}),
Plan: true,
},
{
// The known set value is copied over the null set value
@ -724,7 +724,6 @@ func TestNormalizeNullValues(t *testing.T) {
"foo": cty.String,
}))),
}),
Plan: true,
},
{
// The empty map is copied over the null map
@ -737,6 +736,7 @@ func TestNormalizeNullValues(t *testing.T) {
Expect: cty.ObjectVal(map[string]cty.Value{
"map": cty.MapValEmpty(cty.String),
}),
Apply: true,
},
{
// A zero value primitive is copied over a null primitive
@ -749,6 +749,7 @@ func TestNormalizeNullValues(t *testing.T) {
Expect: cty.ObjectVal(map[string]cty.Value{
"string": cty.StringVal(""),
}),
Apply: true,
},
{
// Plan primitives are kept
@ -761,7 +762,6 @@ func TestNormalizeNullValues(t *testing.T) {
Expect: cty.ObjectVal(map[string]cty.Value{
"string": cty.NullVal(cty.String),
}),
Plan: true,
},
{
// The null map is retained, because the src was unknown
@ -774,6 +774,7 @@ func TestNormalizeNullValues(t *testing.T) {
Expect: cty.ObjectVal(map[string]cty.Value{
"map": cty.NullVal(cty.Map(cty.String)),
}),
Apply: true,
},
{
// the nul set is retained, because the src set contains an unknown value
@ -794,26 +795,7 @@ func TestNormalizeNullValues(t *testing.T) {
"foo": cty.String,
}))),
}),
},
{
// Retain the zero value within the map
Src: cty.ObjectVal(map[string]cty.Value{
"map": cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("a"),
"b": cty.StringVal(""),
}),
}),
Dst: cty.ObjectVal(map[string]cty.Value{
"map": cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("a"),
}),
}),
Expect: cty.ObjectVal(map[string]cty.Value{
"map": cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("a"),
"b": cty.StringVal(""),
}),
}),
Apply: true,
},
{
// Retain don't re-add unexpected planned values in a map
@ -833,7 +815,26 @@ func TestNormalizeNullValues(t *testing.T) {
"a": cty.StringVal("a"),
}),
}),
Plan: true,
},
{
// Remove extra values after apply
Src: cty.ObjectVal(map[string]cty.Value{
"map": cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("a"),
"b": cty.StringVal("b"),
}),
}),
Dst: cty.ObjectVal(map[string]cty.Value{
"map": cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("a"),
}),
}),
Expect: cty.ObjectVal(map[string]cty.Value{
"map": cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("a"),
}),
}),
Apply: true,
},
{
Src: cty.ObjectVal(map[string]cty.Value{
@ -843,7 +844,6 @@ func TestNormalizeNullValues(t *testing.T) {
Expect: cty.ObjectVal(map[string]cty.Value{
"a": cty.NullVal(cty.String),
}),
Plan: true,
},
// a list in an object in a list, going from null to empty
@ -877,6 +877,7 @@ func TestNormalizeNullValues(t *testing.T) {
}),
}),
}),
Apply: true,
},
// a list in an object in a list, going from empty to null
@ -910,6 +911,7 @@ func TestNormalizeNullValues(t *testing.T) {
}),
}),
}),
Apply: true,
},
// the empty list should be transferred, but the new unknown should not be overridden
{
@ -942,7 +944,6 @@ func TestNormalizeNullValues(t *testing.T) {
}),
}),
}),
Plan: true,
},
{
// fix unknowns added to a map
@ -964,7 +965,6 @@ func TestNormalizeNullValues(t *testing.T) {
"b": cty.StringVal(""),
}),
}),
Plan: true,
},
{
// fix unknowns lost from a list
@ -1001,11 +1001,10 @@ func TestNormalizeNullValues(t *testing.T) {
}),
}),
}),
Plan: true,
},
} {
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
got := normalizeNullValues(tc.Dst, tc.Src, tc.Plan)
got := normalizeNullValues(tc.Dst, tc.Src, tc.Apply)
if !got.RawEquals(tc.Expect) {
t.Fatalf("\nexpected: %#v\ngot: %#v\n", tc.Expect, got)
}