preserve possible zero values when normalizing

When normalizing flatmapped containers, compare the attributes to the
prior state and preserve pre-existing zero-length or unknown values. A
zero-length value that was previously unknown is preserved as a
zero-length value, as that may have been computed as such by the
provider.
This commit is contained in:
James Bardin 2018-11-26 18:07:23 -05:00
parent e68377b1f8
commit 6f4d86094f
2 changed files with 60 additions and 13 deletions

View File

@ -419,7 +419,7 @@ 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 // helper/schema should always copy the ID over, but do it again just to be safe
newInstanceState.Attributes["id"] = newInstanceState.ID newInstanceState.Attributes["id"] = newInstanceState.ID
newInstanceState.Attributes = normalizeFlatmapContainers(newInstanceState.Attributes) newInstanceState.Attributes = normalizeFlatmapContainers(instanceState.Attributes, newInstanceState.Attributes)
newStateVal, err := hcl2shim.HCL2ValueFromFlatmap(newInstanceState.Attributes, block.ImpliedType()) newStateVal, err := hcl2shim.HCL2ValueFromFlatmap(newInstanceState.Attributes, block.ImpliedType())
if err != nil { if err != nil {
@ -481,9 +481,9 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl
priorState.Meta = priorPrivate priorState.Meta = priorPrivate
// turn the proposed state into a legacy configuration // turn the proposed state into a legacy configuration
config := terraform.NewResourceConfigShimmed(proposedNewStateVal, block) cfg := terraform.NewResourceConfigShimmed(proposedNewStateVal, block)
diff, err := s.provider.SimpleDiff(info, priorState, config) diff, err := s.provider.SimpleDiff(info, priorState, cfg)
if err != nil { if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil return resp, nil
@ -517,7 +517,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 // now we need to apply the diff to the prior state, so get the planned state
plannedAttrs, err := diff.Apply(priorState.Attributes, block) plannedAttrs, err := diff.Apply(priorState.Attributes, block)
plannedAttrs = normalizeFlatmapContainers(plannedAttrs) plannedAttrs = normalizeFlatmapContainers(priorState.Attributes, plannedAttrs)
plannedStateVal, err := hcl2shim.HCL2ValueFromFlatmap(plannedAttrs, block.ImpliedType()) plannedStateVal, err := hcl2shim.HCL2ValueFromFlatmap(plannedAttrs, block.ImpliedType())
if err != nil { if err != nil {
@ -667,8 +667,12 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
return resp, nil return resp, nil
} }
// here we use the planned state to check for unknown/zero containers values
// when normalizing the flatmap.
plannedState := hcl2shim.FlatmapValueFromHCL2(plannedStateVal)
if newInstanceState != nil { if newInstanceState != nil {
newInstanceState.Attributes = normalizeFlatmapContainers(newInstanceState.Attributes) newInstanceState.Attributes = normalizeFlatmapContainers(plannedState, newInstanceState.Attributes)
} }
newStateVal := cty.NullVal(block.ImpliedType()) newStateVal := cty.NullVal(block.ImpliedType())
@ -845,16 +849,37 @@ func pathToAttributePath(path cty.Path) *proto.AttributePath {
} }
// normalizeFlatmapContainers removes empty containers, and fixes counts in a // normalizeFlatmapContainers removes empty containers, and fixes counts in a
// set of flatmapped attributes. // set of flatmapped attributes. The prior value is used to determine if there
func normalizeFlatmapContainers(attrs map[string]string) map[string]string { // 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(`.\.[%#]$`) keyRx := regexp.MustCompile(`.\.[%#]$`)
// 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{}
for k, v := range prior {
if keyRx.MatchString(k) && (v == "0" || v == hcl2shim.UnknownVariableValue) {
zeros[k] = true
}
}
// find container keys // find container keys
var keys []string var keys []string
for k := range attrs { for k, v := range attrs {
if keyRx.MatchString(k) { if !keyRx.MatchString(k) {
keys = append(keys, 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 the keys in reverse, so that we check the longest subkeys first
@ -890,9 +915,15 @@ func normalizeFlatmapContainers(attrs map[string]string) map[string]string {
} }
} }
if len(indexes) > 0 { 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:
attrs[k] = strconv.Itoa(len(indexes)) attrs[k] = strconv.Itoa(len(indexes))
} else { default:
delete(attrs, k) delete(attrs, k)
} }
} }

View File

@ -11,6 +11,7 @@ import (
"github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts" "github.com/google/go-cmp/cmp/cmpopts"
"github.com/hashicorp/terraform/config/hcl2shim"
"github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/schema"
proto "github.com/hashicorp/terraform/internal/tfplugin5" proto "github.com/hashicorp/terraform/internal/tfplugin5"
"github.com/hashicorp/terraform/terraform" "github.com/hashicorp/terraform/terraform"
@ -642,6 +643,7 @@ func TestGetSchemaTimeouts(t *testing.T) {
func TestNormalizeFlatmapContainers(t *testing.T) { func TestNormalizeFlatmapContainers(t *testing.T) {
for i, tc := range []struct { for i, tc := range []struct {
prior map[string]string
attrs map[string]string attrs map[string]string
expect map[string]string expect map[string]string
}{ }{
@ -665,9 +667,23 @@ func TestNormalizeFlatmapContainers(t *testing.T) {
attrs: 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{"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.list.#": "1", "set.2.list.0": "x", "set.2.required": "bar", "set.#": "1"}, expect: map[string]string{"set.2.list.#": "1", "set.2.list.0": "x", "set.2.required": "bar", "set.#": "1"},
}, },
{
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", "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"},
},
} { } {
t.Run(strconv.Itoa(i), func(t *testing.T) { t.Run(strconv.Itoa(i), func(t *testing.T) {
got := normalizeFlatmapContainers(tc.attrs) got := normalizeFlatmapContainers(tc.prior, tc.attrs)
if !reflect.DeepEqual(tc.expect, got) { if !reflect.DeepEqual(tc.expect, got) {
t.Fatalf("expected:\n%#v\ngot:\n%#v\n", tc.expect, got) t.Fatalf("expected:\n%#v\ngot:\n%#v\n", tc.expect, got)
} }