From 17ecda53b5f552626a4a1eb844df5bf7c35019aa Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 16 Nov 2018 09:55:34 -0500 Subject: [PATCH] strip empty containers from flatmap attributes In order to prevent mismatched states between read/plan/apply, we need to ensure that the attributes are generated consistently each time. Because of the various ways in which helper/schema and the hcl2 shims interpret empty values, the only way to ensure consistency is to always remove them altogether. --- helper/plugin/grpc_provider.go | 82 +++++++++++++++++++++++++++++ helper/plugin/grpc_provider_test.go | 33 ++++++++++++ 2 files changed, 115 insertions(+) diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index 4d6aa7307..7cb51035a 100644 --- a/helper/plugin/grpc_provider.go +++ b/helper/plugin/grpc_provider.go @@ -3,7 +3,10 @@ package plugin import ( "encoding/json" "errors" + "regexp" + "sort" "strconv" + "strings" "github.com/zclconf/go-cty/cty" ctyconvert "github.com/zclconf/go-cty/cty/convert" @@ -416,6 +419,8 @@ 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(newInstanceState.Attributes) + newStateVal, err := hcl2shim.HCL2ValueFromFlatmap(newInstanceState.Attributes, block.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) @@ -498,12 +503,22 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl return resp, nil } + // strip out non-diffs + for k, v := range diff.Attributes { + if v.New == v.Old && !v.NewComputed && !v.NewRemoved { + delete(diff.Attributes, k) + } + } + if priorState == nil { priorState = &terraform.InstanceState{} } // 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(plannedAttrs) + plannedStateVal, err := hcl2shim.HCL2ValueFromFlatmap(plannedAttrs, block.ImpliedType()) if err != nil { resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err) @@ -636,6 +651,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) + } + } + if private != nil { diff.Meta = private } @@ -646,6 +668,10 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A return resp, nil } + if newInstanceState != nil { + newInstanceState.Attributes = normalizeFlatmapContainers(newInstanceState.Attributes) + } + newStateVal := cty.NullVal(block.ImpliedType()) // We keep the null val if we destroyed the resource, otherwise build the @@ -819,6 +845,62 @@ 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. +func normalizeFlatmapContainers(attrs map[string]string) map[string]string { + keyRx := regexp.MustCompile(`.*\.[%#]$`) + + // find container keys + var keys []string + for k := range attrs { + if keyRx.MatchString(k) { + 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]++ + } + } + + if len(indexes) > 0 { + attrs[k] = strconv.Itoa(len(indexes)) + } else { + delete(attrs, k) + } + } + + 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 03f059a0b..a06185fc4 100644 --- a/helper/plugin/grpc_provider_test.go +++ b/helper/plugin/grpc_provider_test.go @@ -3,6 +3,8 @@ package plugin import ( "context" "fmt" + "reflect" + "strconv" "strings" "testing" "time" @@ -637,3 +639,34 @@ func TestGetSchemaTimeouts(t *testing.T) { t.Fatal("missing default timeout in schema") } } + +func TestNormalizeFlatmapContainers(t *testing.T) { + for i, tc := range []struct { + attrs map[string]string + expect map[string]string + }{ + { + attrs: map[string]string{"id": "1", "multi.2.set.#": "1", "multi.1.set.#": "0", "single.#": "0"}, + expect: map[string]string{"id": "1"}, + }, + { + 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"}, + }, + { + attrs: map[string]string{"id": "78629a0f5f3f164f", "multi.#": "1"}, + expect: map[string]string{"id": "78629a0f5f3f164f"}, + }, + { + attrs: map[string]string{"multi.529860700.set.#": "1", "multi.#": "1", "id": "78629a0f5f3f164f"}, + expect: map[string]string{"id": "78629a0f5f3f164f"}, + }, + } { + t.Run(strconv.Itoa(i), func(t *testing.T) { + got := normalizeFlatmapContainers(tc.attrs) + if !reflect.DeepEqual(tc.expect, got) { + t.Fatalf("expected:\n%#v\ngot:\n%#v\n", tc.expect, got) + } + }) + } +}