diff --git a/builtin/providers/test/resource_nested_set.go b/builtin/providers/test/resource_nested_set.go index b808b7c98..81d7ab0f5 100644 --- a/builtin/providers/test/resource_nested_set.go +++ b/builtin/providers/test/resource_nested_set.go @@ -28,6 +28,19 @@ func testResourceNestedSet() *schema.Resource { Optional: true, ForceNew: true, }, + "type_list": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "value": { + Type: schema.TypeString, + ForceNew: true, + Optional: true, + }, + }, + }, + }, "single": { Type: schema.TypeSet, Optional: true, @@ -98,7 +111,6 @@ func testResourceNestedSet() *schema.Resource { Type: schema.TypeString, Required: true, }, - "list": { Type: schema.TypeList, Optional: true, @@ -106,6 +118,18 @@ func testResourceNestedSet() *schema.Resource { Type: schema.TypeString, }, }, + "list_block": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "unused": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + }, }, }, }, diff --git a/builtin/providers/test/resource_nested_set_test.go b/builtin/providers/test/resource_nested_set_test.go index f85e2a190..99e7651ce 100644 --- a/builtin/providers/test/resource_nested_set_test.go +++ b/builtin/providers/test/resource_nested_set_test.go @@ -3,6 +3,7 @@ package test import ( "errors" "fmt" + "regexp" "strings" "testing" @@ -56,6 +57,118 @@ resource "test_resource_nested_set" "foo" { }) } +// the empty type_list must be passed to the provider with 1 nil element +func TestResourceNestedSet_emptyBlock(t *testing.T) { + checkFunc := func(s *terraform.State) error { + root := s.ModuleByPath(addrs.RootModuleInstance) + res := root.Resources["test_resource_nested_set.foo"] + for k, v := range res.Primary.Attributes { + if strings.HasPrefix(k, "type_list") && v != "1" { + return fmt.Errorf("unexpected set value: %s:%s", k, v) + } + } + return nil + } + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_nested_set" "foo" { + type_list { + } +} + `), + Check: checkFunc, + }, + }, + }) +} + +func TestResourceNestedSet_emptyNestedListBlock(t *testing.T) { + checkFunc := func(s *terraform.State) error { + root := s.ModuleByPath(addrs.RootModuleInstance) + res := root.Resources["test_resource_nested_set.foo"] + found := false + for k, v := range res.Primary.Attributes { + if !regexp.MustCompile(`^with_list\.\d+\.list_block\.`).MatchString(k) { + continue + } + found = true + + if strings.HasSuffix(k, ".#") { + if v != "1" { + return fmt.Errorf("expected block with no objects: got %s:%s", k, v) + } + continue + } + + // there should be no other attribute values for an empty block + return fmt.Errorf("unexpected attribute: %s:%s", k, v) + } + if !found { + return fmt.Errorf("with_list.X.list_block not found") + } + return nil + } + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_nested_set" "foo" { + with_list { + required = "ok" + list_block { + } + } +} + `), + Check: checkFunc, + }, + }, + }) +} +func TestResourceNestedSet_emptyNestedList(t *testing.T) { + checkFunc := func(s *terraform.State) error { + root := s.ModuleByPath(addrs.RootModuleInstance) + res := root.Resources["test_resource_nested_set.foo"] + found := false + for k, v := range res.Primary.Attributes { + if regexp.MustCompile(`^with_list\.\d+\.list\.#$`).MatchString(k) { + found = true + if v != "0" { + return fmt.Errorf("expected empty list: %s, got %s", k, v) + } + break + } + } + if !found { + return fmt.Errorf("with_list.X.nested_list not found") + } + return nil + } + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_nested_set" "foo" { + with_list { + required = "ok" + list = [] + } +} + `), + Check: checkFunc, + }, + }, + }) +} + func TestResourceNestedSet_addRemove(t *testing.T) { var id string checkFunc := func(s *terraform.State) error { @@ -339,3 +452,50 @@ resource "test_resource_nested_set" "foo" { }, }) } + +// This is the same as forceNewEmptyString, but we start with the empty value, +// instead of changing it. +func TestResourceNestedSet_nestedSetEmptyString(t *testing.T) { + checkFunc := func(s *terraform.State) error { + return nil + } + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_nested_set" "foo" { + multi { + set { + required = "" + } + } +} + `), + Check: checkFunc, + }, + }, + }) +} + +func TestResourceNestedSet_emptySet(t *testing.T) { + checkFunc := func(s *terraform.State) error { + return nil + } + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_nested_set" "foo" { + multi { + } +} + `), + Check: checkFunc, + }, + }, + }) +} diff --git a/builtin/providers/test/resource_nested_test.go b/builtin/providers/test/resource_nested_test.go index d1a9c1b73..9237b52fc 100644 --- a/builtin/providers/test/resource_nested_test.go +++ b/builtin/providers/test/resource_nested_test.go @@ -142,6 +142,7 @@ resource "test_resource_nested" "foo" { "nested.0.nested_again.0.string": "a", "nested.1.string": "", "nested.1.optional": "false", + "nested.1.nested_again.#": "0", } delete(got, "id") // it's random, so not useful for testing diff --git a/builtin/providers/test/resource_test.go b/builtin/providers/test/resource_test.go index b9eb06455..12bf568fa 100644 --- a/builtin/providers/test/resource_test.go +++ b/builtin/providers/test/resource_test.go @@ -521,3 +521,23 @@ resource "test_resource" "two" { }, }) } + +func TestResource_emptyMapValue(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource" "foo" { + required = "ok" + required_map = { + a = "a" + b = "" + } +} + `), + }, + }, + }) +} diff --git a/config/hcl2shim/flatmap.go b/config/hcl2shim/flatmap.go index bcecb30df..bb4228d98 100644 --- a/config/hcl2shim/flatmap.go +++ b/config/hcl2shim/flatmap.go @@ -347,10 +347,8 @@ func hcl2ValueFromFlatmapSet(m map[string]string, prefix string, ty cty.Type) (c return cty.UnknownVal(ty), nil } - // We actually don't really care about the "count" of a set for our - // purposes here, but we do need to check if it _exists_ in order to - // recognize the difference between null (not set at all) and empty. - if strCount, exists := m[prefix+"#"]; !exists { + strCount, exists := m[prefix+"#"] + if !exists { return cty.NullVal(ty), nil } else if strCount == UnknownVariableValue { return cty.UnknownVal(ty), nil @@ -394,7 +392,31 @@ func hcl2ValueFromFlatmapSet(m map[string]string, prefix string, ty cty.Type) (c vals = append(vals, val) } - if len(vals) == 0 { + if len(vals) == 0 && strCount == "1" { + // An empty set wouldn't be represented in the flatmap, so this must be + // a single empty object since the count is actually 1. + // Add an appropriately typed null value to the set. + var val cty.Value + switch { + case ety.IsMapType(): + val = cty.MapValEmpty(ety) + case ety.IsListType(): + val = cty.ListValEmpty(ety) + case ety.IsSetType(): + val = cty.SetValEmpty(ety) + case ety.IsObjectType(): + // TODO: cty.ObjectValEmpty + objectMap := map[string]cty.Value{} + for attr, ty := range ety.AttributeTypes() { + objectMap[attr] = cty.NullVal(ty) + } + val = cty.ObjectVal(objectMap) + default: + val = cty.NullVal(ety) + } + vals = append(vals, val) + + } else if len(vals) == 0 { return cty.SetValEmpty(ety), nil } diff --git a/config/hcl2shim/flatmap_test.go b/config/hcl2shim/flatmap_test.go index 07f93ac26..e2ae8a70f 100644 --- a/config/hcl2shim/flatmap_test.go +++ b/config/hcl2shim/flatmap_test.go @@ -679,10 +679,52 @@ func TestHCL2ValueFromFlatmap(t *testing.T) { }), }), }, + { + Flatmap: map[string]string{ + "foo.#": "1", + }, + Type: cty.Object(map[string]cty.Type{ + "foo": cty.Set(cty.Object(map[string]cty.Type{ + "bar": cty.String, + })), + }), + Want: cty.ObjectVal(map[string]cty.Value{ + "foo": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "bar": cty.NullVal(cty.String), + }), + }), + }), + }, + { + Flatmap: map[string]string{ + "multi.#": "1", + "multi.2.set.#": "1", + "multi.2.set.3.required": "val", + }, + Type: cty.Object(map[string]cty.Type{ + "multi": cty.Set(cty.Object(map[string]cty.Type{ + "set": cty.Set(cty.Object(map[string]cty.Type{ + "required": cty.String, + })), + })), + }), + Want: cty.ObjectVal(map[string]cty.Value{ + "multi": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "set": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "required": cty.StringVal("val"), + }), + }), + }), + }), + }), + }, } - for _, test := range tests { - t.Run(fmt.Sprintf("%#v as %#v", test.Flatmap, test.Type), func(t *testing.T) { + for i, test := range tests { + t.Run(fmt.Sprintf("%d %#v as %#v", i, test.Flatmap, test.Type), func(t *testing.T) { got, err := HCL2ValueFromFlatmap(test.Flatmap, test.Type) if test.WantErr != "" { diff --git a/config/hcl2shim/values.go b/config/hcl2shim/values.go index 2c5b2907e..000ad7ba8 100644 --- a/config/hcl2shim/values.go +++ b/config/hcl2shim/values.go @@ -80,11 +80,6 @@ func ConfigValueFromHCL2Block(v cty.Value, schema *configschema.Block) map[strin case configschema.NestingList, configschema.NestingSet: l := bv.LengthInt() - if l == 0 { - // skip empty collections to better mimic how HCL1 would behave - continue - } - elems := make([]interface{}, 0, l) for it := bv.ElementIterator(); it.Next(); { _, ev := it.Element() @@ -97,11 +92,6 @@ func ConfigValueFromHCL2Block(v cty.Value, schema *configschema.Block) map[strin ret[name] = elems case configschema.NestingMap: - if bv.LengthInt() == 0 { - // skip empty collections to better mimic how HCL1 would behave - continue - } - elems := make(map[string]interface{}) for it := bv.ElementIterator(); it.Next(); { ek, ev := it.Element() diff --git a/config/hcl2shim/values_test.go b/config/hcl2shim/values_test.go index 7c3011da0..805155f0b 100644 --- a/config/hcl2shim/values_test.go +++ b/config/hcl2shim/values_test.go @@ -151,7 +151,7 @@ func TestConfigValueFromHCL2Block(t *testing.T) { }, { cty.ObjectVal(map[string]cty.Value{ - "address": cty.ListValEmpty(cty.EmptyObject), // should be omitted altogether in result + "address": cty.ListValEmpty(cty.EmptyObject), }), &configschema.Block{ BlockTypes: map[string]*configschema.NestedBlock{ @@ -161,7 +161,9 @@ func TestConfigValueFromHCL2Block(t *testing.T) { }, }, }, - map[string]interface{}{}, + map[string]interface{}{ + "address": []interface{}{}, + }, }, { cty.ObjectVal(map[string]cty.Value{ @@ -193,7 +195,9 @@ func TestConfigValueFromHCL2Block(t *testing.T) { }, }, }, - map[string]interface{}{}, + map[string]interface{}{ + "address": []interface{}{}, + }, }, { cty.ObjectVal(map[string]cty.Value{ @@ -225,7 +229,9 @@ func TestConfigValueFromHCL2Block(t *testing.T) { }, }, }, - map[string]interface{}{}, + map[string]interface{}{ + "address": map[string]interface{}{}, + }, }, { cty.NullVal(cty.EmptyObject), @@ -234,8 +240,8 @@ func TestConfigValueFromHCL2Block(t *testing.T) { }, } - for _, test := range tests { - t.Run(fmt.Sprintf("%#v", test.Input), func(t *testing.T) { + for i, test := range tests { + t.Run(fmt.Sprintf("%d-%#v", i, test.Input), func(t *testing.T) { got := ConfigValueFromHCL2Block(test.Input, test.Schema) if !reflect.DeepEqual(got, test.Want) { t.Errorf("wrong result\ninput: %#v\ngot: %#v\nwant: %#v", test.Input, got, test.Want) diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index 0c95539f7..c9ffa1a23 100644 --- a/helper/plugin/grpc_provider.go +++ b/helper/plugin/grpc_provider.go @@ -438,8 +438,6 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso // helper/schema should always copy the ID over, but do it again just to be safe newInstanceState.Attributes["id"] = newInstanceState.ID - newInstanceState.Attributes = normalizeFlatmapContainers(instanceState.Attributes, newInstanceState.Attributes) - newStateVal, err := hcl2shim.HCL2ValueFromFlatmap(newInstanceState.Attributes, block.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) @@ -488,7 +486,6 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) return resp, nil } - priorPrivate := make(map[string]interface{}) if len(req.PriorPrivate) > 0 { if err := json.Unmarshal(req.PriorPrivate, &priorPrivate); err != nil { @@ -536,7 +533,7 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl // now we need to apply the diff to the prior state, so get the planned state plannedAttrs, err := diff.Apply(priorState.Attributes, block) - plannedAttrs = normalizeFlatmapContainers(priorState.Attributes, plannedAttrs) + plannedAttrs = normalizeFlatmapContainers(priorState.Attributes, plannedAttrs, false) plannedStateVal, err := hcl2shim.HCL2ValueFromFlatmap(plannedAttrs, block.ImpliedType()) if err != nil { @@ -544,6 +541,8 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl return resp, nil } + plannedStateVal = copyMissingValues(plannedStateVal, proposedNewStateVal) + plannedStateVal, err = block.CoerceValue(plannedStateVal) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) @@ -683,6 +682,13 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A } } + // strip out non-diffs + for k, v := range diff.Attributes { + if v.New == v.Old && !v.NewComputed && !v.NewRemoved { + delete(diff.Attributes, k) + } + } + // add NewExtra Fields that may have been stored in the private data if newExtra := private[newExtraKey]; newExtra != nil { for k, v := range newExtra.(map[string]interface{}) { @@ -718,7 +724,7 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A // here we use the planned state to check for unknown/zero containers values // when normalizing the flatmap. plannedState := hcl2shim.FlatmapValueFromHCL2(plannedStateVal) - newInstanceState.Attributes = normalizeFlatmapContainers(plannedState, newInstanceState.Attributes) + newInstanceState.Attributes = normalizeFlatmapContainers(plannedState, newInstanceState.Attributes, true) } newStateVal := cty.NullVal(block.ImpliedType()) @@ -733,6 +739,8 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A } } + newStateVal = copyMissingValues(newStateVal, plannedStateVal) + newStateVal = copyTimeoutValues(newStateVal, plannedStateVal) newStateMP, err := msgpack.Marshal(newStateVal, block.ImpliedType()) @@ -898,24 +906,62 @@ func pathToAttributePath(path cty.Path) *proto.AttributePath { // set of flatmapped attributes. The prior value is used to determine if there // could be zero-length flatmap containers which we need to preserve. This // allows a provider to set an empty computed container in the state without -// creating perpetual diff. -func normalizeFlatmapContainers(prior map[string]string, attrs map[string]string) map[string]string { - keyRx := regexp.MustCompile(`.\.[%#]$`) +// creating perpetual diff. This can differ slightly between plan and apply, so +// the apply flag is passed when called from ApplyResourceChange. +func normalizeFlatmapContainers(prior map[string]string, attrs map[string]string, apply bool) map[string]string { + isCount := regexp.MustCompile(`.\.[%#]$`).MatchString - // while we can't determine if the value was actually computed here, we will + // While we can't determine if the value was actually computed here, we will // trust that our shims stored and retrieved a zero-value container // correctly. zeros := map[string]bool{} + // Empty blocks have a count of 1 with no other attributes. Just record all + // "1"s here to override 0-length blocks when setting the count below. + ones := map[string]bool{} for k, v := range prior { - if keyRx.MatchString(k) && (v == "0" || v == hcl2shim.UnknownVariableValue) { + if isCount(k) && (v == "0" || v == hcl2shim.UnknownVariableValue) { zeros[k] = true } + + // fixup any 1->0 conversions that happened during Apply + if apply && isCount(k) && v == "1" && attrs[k] == "0" { + attrs[k] = "1" + } + } + + for k, v := range attrs { + // store any "1" values, since if the length was 1 and there are no + // items, it was probably an empty set block. Hopefully checking for a 1 + // value with no items is sufficient, without cross-referencing the + // schema. + if isCount(k) && v == "1" { + ones[k] = true + } + } + + // The "ones" were stored to look for sets with an empty value, so we need + // to verify that we only store ones with no attrs. + expectedEmptySets := map[string]bool{} + for one := range ones { + prefix := one[:len(one)-1] + + found := 0 + for k := range attrs { + // since this can be recursive, we check that the attrs isn't also a #. + if strings.HasPrefix(k, prefix) && !isCount(k) { + found++ + } + } + + if found == 0 { + expectedEmptySets[one] = true + } } // find container keys var keys []string for k, v := range attrs { - if !keyRx.MatchString(k) { + if !isCount(k) { continue } @@ -967,6 +1013,9 @@ func normalizeFlatmapContainers(prior map[string]string, attrs map[string]string // must have set the computed value to an empty container, and we // need to leave it in the flatmap. attrs[k] = "0" + case len(indexes) == 0 && ones[k]: + // We need to retain any empty blocks that had a 1 count with no attributes. + attrs[k] = "1" case len(indexes) > 0: attrs[k] = strconv.Itoa(len(indexes)) default: @@ -974,6 +1023,12 @@ func normalizeFlatmapContainers(prior map[string]string, attrs map[string]string } } + for k := range expectedEmptySets { + if _, ok := attrs[k]; !ok { + attrs[k] = "1" + } + } + return attrs } @@ -1043,3 +1098,86 @@ func stripSchema(s *schema.Schema) *schema.Schema { return newSchema } + +// Zero values and empty containers may be lost during apply. Copy zero values +// and empty containers from src to dst when they are missing in dst. +// This takes a little more liberty with set types, since we can't correlate +// modified set values. In the case of sets, if the src set was wholly known we +// assume the value was correctly applied and copy that entirely to the new +// value. +func copyMissingValues(dst, src cty.Value) cty.Value { + ty := dst.Type() + + // In this case the provider set an empty string which was lost in + // conversion. Since src is unknown, there must have been a corresponding + // value set. + if ty == cty.String && dst.IsNull() && !src.IsKnown() { + return cty.StringVal("") + } + + if src.IsNull() || !src.IsKnown() || !dst.IsKnown() { + return dst + } + + switch { + case ty.IsMapType(), ty.IsObjectType(): + var dstMap map[string]cty.Value + if dst.IsNull() { + dstMap = map[string]cty.Value{} + } else { + dstMap = dst.AsValueMap() + } + + ei := src.ElementIterator() + for ei.Next() { + k, v := ei.Element() + key := k.AsString() + + dstVal := dstMap[key] + if dstVal == cty.NilVal { + dstVal = cty.NullVal(ty.ElementType()) + } + dstMap[key] = copyMissingValues(dstVal, v) + } + + // 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() { + return src + } + return dst + } + + if ty.IsMapType() { + return cty.MapVal(dstMap) + } + + return cty.ObjectVal(dstMap) + + case ty.IsSetType(): + // 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() { + return src + } + + case ty.IsListType(), ty.IsTupleType(): + // If the dst is nil, and the src is known, then we lost an empty value + // so take the original. This doesn't attempt to descend into the list + // values, since missing empty values may prevent us from correlating + // the correct src and dst indexes. + if dst.IsNull() && src.IsWhollyKnown() { + return src + } + + case ty.IsPrimitiveType(): + if dst.IsNull() && src.IsWhollyKnown() { + return src + } + } + + return dst +} diff --git a/helper/plugin/grpc_provider_test.go b/helper/plugin/grpc_provider_test.go index 8f807c51d..4df64cdac 100644 --- a/helper/plugin/grpc_provider_test.go +++ b/helper/plugin/grpc_provider_test.go @@ -673,7 +673,7 @@ func TestNormalizeFlatmapContainers(t *testing.T) { }{ { attrs: map[string]string{"id": "1", "multi.2.set.#": "1", "multi.1.set.#": "0", "single.#": "0"}, - expect: map[string]string{"id": "1"}, + expect: map[string]string{"id": "1", "multi.2.set.#": "1"}, }, { attrs: map[string]string{"id": "1", "multi.2.set.#": "2", "multi.2.set.1.foo": "bar", "multi.1.set.#": "0", "single.#": "0"}, @@ -681,11 +681,15 @@ func TestNormalizeFlatmapContainers(t *testing.T) { }, { attrs: map[string]string{"id": "78629a0f5f3f164f", "multi.#": "1"}, + expect: map[string]string{"id": "78629a0f5f3f164f", "multi.#": "1"}, + }, + { + attrs: map[string]string{"id": "78629a0f5f3f164f", "multi.#": "0"}, expect: map[string]string{"id": "78629a0f5f3f164f"}, }, { - attrs: map[string]string{"multi.529860700.set.#": "1", "multi.#": "1", "id": "78629a0f5f3f164f"}, - expect: map[string]string{"id": "78629a0f5f3f164f"}, + attrs: map[string]string{"multi.529860700.set.#": "0", "multi.#": "1", "id": "78629a0f5f3f164f"}, + expect: map[string]string{"id": "78629a0f5f3f164f", "multi.#": "1"}, }, { attrs: map[string]string{"set.2.required": "bar", "set.2.list.#": "1", "set.2.list.0": "x", "set.1.list.#": "0", "set.#": "2"}, @@ -707,10 +711,143 @@ func TestNormalizeFlatmapContainers(t *testing.T) { }, } { t.Run(strconv.Itoa(i), func(t *testing.T) { - got := normalizeFlatmapContainers(tc.prior, tc.attrs) + got := normalizeFlatmapContainers(tc.prior, tc.attrs, false) if !reflect.DeepEqual(tc.expect, got) { t.Fatalf("expected:\n%#v\ngot:\n%#v\n", tc.expect, got) } }) } } + +func TestCopyMissingValues(t *testing.T) { + for i, tc := range []struct { + Src, Dst, Expect cty.Value + }{ + { + // The known set value is copied over the null set value + Src: cty.ObjectVal(map[string]cty.Value{ + "set": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.NullVal(cty.String), + }), + }), + }), + Dst: cty.ObjectVal(map[string]cty.Value{ + "set": cty.NullVal(cty.Set(cty.Object(map[string]cty.Type{ + "foo": cty.String, + }))), + }), + Expect: cty.ObjectVal(map[string]cty.Value{ + "set": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.NullVal(cty.String), + }), + }), + }), + }, + { + // The empty map is copied over the null map + Src: cty.ObjectVal(map[string]cty.Value{ + "map": cty.MapValEmpty(cty.String), + }), + Dst: cty.ObjectVal(map[string]cty.Value{ + "map": cty.NullVal(cty.Map(cty.String)), + }), + Expect: cty.ObjectVal(map[string]cty.Value{ + "map": cty.MapValEmpty(cty.String), + }), + }, + { + // A zerp value primitive is copied over a null primitive + Src: cty.ObjectVal(map[string]cty.Value{ + "string": cty.StringVal(""), + }), + Dst: cty.ObjectVal(map[string]cty.Value{ + "string": cty.NullVal(cty.String), + }), + Expect: cty.ObjectVal(map[string]cty.Value{ + "string": cty.StringVal(""), + }), + }, + { + // The null map is retained, because the src was unknown + Src: cty.ObjectVal(map[string]cty.Value{ + "map": cty.UnknownVal(cty.Map(cty.String)), + }), + Dst: cty.ObjectVal(map[string]cty.Value{ + "map": cty.NullVal(cty.Map(cty.String)), + }), + Expect: cty.ObjectVal(map[string]cty.Value{ + "map": cty.NullVal(cty.Map(cty.String)), + }), + }, + { + // the nul set is retained, because the src set contains an unknown value + Src: cty.ObjectVal(map[string]cty.Value{ + "set": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.UnknownVal(cty.String), + }), + }), + }), + Dst: cty.ObjectVal(map[string]cty.Value{ + "set": cty.NullVal(cty.Set(cty.Object(map[string]cty.Type{ + "foo": cty.String, + }))), + }), + Expect: cty.ObjectVal(map[string]cty.Value{ + "set": cty.NullVal(cty.Set(cty.Object(map[string]cty.Type{ + "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(""), + }), + }), + }, + { + // Recover the lost unknown key, assuming it was set to an empty + // string and lost. + Src: cty.ObjectVal(map[string]cty.Value{ + "map": cty.MapVal(map[string]cty.Value{ + "a": cty.StringVal("a"), + "b": cty.UnknownVal(cty.String), + }), + }), + 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(""), + }), + }), + }, + } { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + got := copyMissingValues(tc.Dst, tc.Src) + if !got.RawEquals(tc.Expect) { + t.Fatalf("\nexpected: %#v\ngot: %#v\n", tc.Expect, got) + } + }) + } +} diff --git a/plugin/grpc_provider.go b/plugin/grpc_provider.go index 6bab719c8..373e07394 100644 --- a/plugin/grpc_provider.go +++ b/plugin/grpc_provider.go @@ -420,11 +420,17 @@ func (p *GRPCProvider) ApplyResourceChange(r providers.ApplyResourceChangeReques resp.Diagnostics = resp.Diagnostics.Append(err) return resp } + configMP, err := msgpack.Marshal(r.Config, resSchema.Block.ImpliedType()) + if err != nil { + resp.Diagnostics = resp.Diagnostics.Append(err) + return resp + } protoReq := &proto.ApplyResourceChange_Request{ TypeName: r.TypeName, PriorState: &proto.DynamicValue{Msgpack: priorMP}, PlannedState: &proto.DynamicValue{Msgpack: plannedMP}, + Config: &proto.DynamicValue{Msgpack: configMP}, PlannedPrivate: r.PlannedPrivate, }