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

@@ -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.

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)
+			}
+		})
+	}
+}