diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index b8dacc9bfd..fc2bfea2f7 100644 --- a/helper/plugin/grpc_provider.go +++ b/helper/plugin/grpc_provider.go @@ -4,8 +4,6 @@ import ( "encoding/json" "errors" "fmt" - "regexp" - "sort" "strconv" "strings" @@ -427,13 +425,6 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso return resp, nil } - if newInstanceState != nil { - // here we use the prior state to check for unknown/zero containers values - // when normalizing the flatmap. - stateAttrs := hcl2shim.FlatmapValueFromHCL2(stateVal) - newInstanceState.Attributes = normalizeFlatmapContainers(stateAttrs, newInstanceState.Attributes) - } - if newInstanceState == nil || newInstanceState.ID == "" { // The old provider API used an empty id to signal that the remote // object appears to have been deleted, but our new protocol expects @@ -572,8 +563,6 @@ 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) - plannedStateVal, err := hcl2shim.HCL2ValueFromFlatmap(plannedAttrs, block.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) @@ -817,8 +806,6 @@ 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) // We keep the null val if we destroyed the resource, otherwise build the // entire object, even if the new state was nil. @@ -1000,131 +987,6 @@ func pathToAttributePath(path cty.Path) *proto.AttributePath { return &proto.AttributePath{Steps: steps} } -// normalizeFlatmapContainers removes empty containers, and fixes counts in a -// 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. 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) map[string]string { - isCount := regexp.MustCompile(`.\.[%#]$`).MatchString - - // 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 isCount(k) && (v == "0" || v == hcl2shim.UnknownVariableValue) { - zeros[k] = true - } - } - - 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 - // make sure we don't have the same key under both categories. - delete(zeros, k) - } - } - - // 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 !isCount(k) { - continue - } - - if v == hcl2shim.UnknownVariableValue { - // if the index value indicates the container is unknown, skip - // updating the counts. - continue - } - - keys = append(keys, k) - } - - // sort the keys in reverse, so that we check the longest subkeys first - sort.Slice(keys, func(i, j int) bool { - a, b := keys[i], keys[j] - - if strings.HasPrefix(a, b) { - return true - } - - if strings.HasPrefix(b, a) { - return false - } - - return a > b - }) - - for _, k := range keys { - prefix := k[:len(k)-1] - indexes := map[string]int{} - for cand := range attrs { - if cand == k { - continue - } - - if strings.HasPrefix(cand, prefix) { - idx := cand[len(prefix):] - dot := strings.Index(idx, ".") - if dot > 0 { - idx = idx[:dot] - } - indexes[idx]++ - } - } - - switch { - case len(indexes) == 0 && zeros[k]: - // if there were no keys, but the value was known to be zero, the provider - // 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)) - } - } - - for k := range expectedEmptySets { - if _, ok := attrs[k]; !ok { - attrs[k] = "1" - } - } - - return attrs -} - // helper/schema throws away timeout values from the config and stores them in // the Private/Meta fields. we need to copy those values into the planned state // so that core doesn't see a perpetual diff with the timeout block. diff --git a/helper/plugin/grpc_provider_test.go b/helper/plugin/grpc_provider_test.go index cf0b805fac..a31fe57c50 100644 --- a/helper/plugin/grpc_provider_test.go +++ b/helper/plugin/grpc_provider_test.go @@ -3,15 +3,12 @@ package plugin import ( "context" "fmt" - "reflect" - "strconv" "strings" "testing" "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "github.com/hashicorp/terraform/config/hcl2shim" "github.com/hashicorp/terraform/helper/schema" proto "github.com/hashicorp/terraform/internal/tfplugin5" "github.com/hashicorp/terraform/terraform" @@ -665,57 +662,6 @@ func TestGetSchemaTimeouts(t *testing.T) { } } -func TestNormalizeFlatmapContainers(t *testing.T) { - for i, tc := range []struct { - prior map[string]string - attrs map[string]string - expect map[string]string - }{ - { - attrs: map[string]string{"id": "1", "multi.2.set.#": "2", "multi.2.set.1.foo": "bar", "multi.1.set.#": "0", "single.#": "0"}, - expect: map[string]string{"id": "1", "multi.2.set.#": "1", "multi.2.set.1.foo": "bar", "multi.1.set.#": "0", "single.#": "0"}, - }, - { - attrs: map[string]string{"id": "78629a0f5f3f164f", "multi.#": "1"}, - expect: map[string]string{"id": "78629a0f5f3f164f", "multi.#": "1"}, - }, - { - attrs: map[string]string{"multi.529860700.set.#": "0", "multi.#": "1", "id": "78629a0f5f3f164f"}, - expect: map[string]string{"multi.529860700.set.#": "0", "multi.#": "1", "id": "78629a0f5f3f164f"}, - }, - { - attrs: map[string]string{"set.2.required": "bar", "set.2.list.#": "1", "set.2.list.0": "x", "set.1.list.#": "0", "set.#": "2"}, - expect: map[string]string{"set.2.required": "bar", "set.2.list.#": "1", "set.2.list.0": "x", "set.1.list.#": "0", "set.#": "2"}, - }, - { - attrs: map[string]string{"map.%": hcl2shim.UnknownVariableValue, "list.#": hcl2shim.UnknownVariableValue, "id": "1"}, - expect: map[string]string{"id": "1", "map.%": hcl2shim.UnknownVariableValue, "list.#": hcl2shim.UnknownVariableValue}, - }, - { - prior: map[string]string{"map.%": "0"}, - attrs: map[string]string{"map.%": "0", "list.#": "0", "id": "1"}, - expect: map[string]string{"id": "1", "list.#": "0", "map.%": "0"}, - }, - { - prior: map[string]string{"map.%": hcl2shim.UnknownVariableValue, "list.#": "0"}, - attrs: map[string]string{"map.%": "0", "list.#": "0", "id": "1"}, - expect: map[string]string{"id": "1", "map.%": "0", "list.#": "0"}, - }, - { - prior: map[string]string{"list.#": "1", "list.0": "old value"}, - attrs: map[string]string{"list.#": "0"}, - expect: map[string]string{"list.#": "0"}, - }, - } { - t.Run(strconv.Itoa(i), func(t *testing.T) { - got := normalizeFlatmapContainers(tc.prior, tc.attrs) - if !reflect.DeepEqual(tc.expect, got) { - t.Fatalf("expected:\n%#v\ngot:\n%#v\n", tc.expect, got) - } - }) - } -} - func TestNormalizeNullValues(t *testing.T) { for i, tc := range []struct { Src, Dst, Expect cty.Value @@ -743,6 +689,19 @@ func TestNormalizeNullValues(t *testing.T) { }), }), }, + { + // A zero set value is kept + Src: cty.ObjectVal(map[string]cty.Value{ + "set": cty.SetValEmpty(cty.String), + }), + Dst: cty.ObjectVal(map[string]cty.Value{ + "set": cty.SetValEmpty(cty.String), + }), + 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 Src: cty.ObjectVal(map[string]cty.Value{