Merge pull request #19475 from hashicorp/jbardin/computed-containers

Shim computed containers

builtin/providers/test/resource_diff_suppress.go

@@ -81,6 +81,10 @@ func testResourceDiffSuppressCreate(d *schema.ResourceData, meta interface{}) er
 	d.Set("network", "modified")
 	d.Set("subnetwork", "modified")
 
+	if _, ok := d.GetOk("node_pool"); !ok {
+		d.Set("node_pool", []string{})
+	}
+
 	id := fmt.Sprintf("%x", rand.Int63())
 	d.SetId(id)
 	return nil

builtin/providers/test/resource_nested_set.go

@@ -92,7 +92,6 @@ func testResourceNestedSet() *schema.Resource {
 			"with_list": {
 				Type:     schema.TypeSet,
 				Optional: true,
-				Computed: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"required": {
@@ -131,7 +130,7 @@ func testResourceNestedSetCreate(d *schema.ResourceData, meta interface{}) error
 
 	d.Set("single", set)
 
-	return testResourceNestedRead(d, meta)
+	return testResourceNestedSetRead(d, meta)
 }
 
 func testResourceNestedSetRead(d *schema.ResourceData, meta interface{}) error {

builtin/providers/test/resource_test.go

@@ -473,3 +473,23 @@ resource "test_resource" "foo" {
 		},
 	})
 }
+
+func TestResource_unknownFuncInMap(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 = {
+	  key = "${uuid()}"
+	}
+}
+				`),
+				ExpectNonEmptyPlan: true,
+			},
+		},
+	})
+}

helper/plugin/grpc_provider.go

@@ -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
 	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())
 	if err != nil {
@@ -481,9 +481,9 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl
 	priorState.Meta = priorPrivate
 
 	// 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 {
 		resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
 		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
 	plannedAttrs, err := diff.Apply(priorState.Attributes, block)
 
-	plannedAttrs = normalizeFlatmapContainers(plannedAttrs)
+	plannedAttrs = normalizeFlatmapContainers(priorState.Attributes, plannedAttrs)
 
 	plannedStateVal, err := hcl2shim.HCL2ValueFromFlatmap(plannedAttrs, block.ImpliedType())
 	if err != nil {
@@ -667,8 +667,12 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
 		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 {
-		newInstanceState.Attributes = normalizeFlatmapContainers(newInstanceState.Attributes)
+		newInstanceState.Attributes = normalizeFlatmapContainers(plannedState, newInstanceState.Attributes)
 	}
 
 	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
-// set of flatmapped attributes.
-func normalizeFlatmapContainers(attrs map[string]string) map[string]string {
+// 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(`.\.[%#]$`)
 
+	// 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
 	var keys []string
-	for k := range attrs {
-		if keyRx.MatchString(k) {
-			keys = append(keys, k)
+	for k, v := range attrs {
+		if !keyRx.MatchString(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
@@ -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))
-		} else {
+		default:
 			delete(attrs, k)
 		}
 	}

helper/plugin/grpc_provider_test.go

@@ -11,6 +11,7 @@ import (
 
 	"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"
@@ -642,6 +643,7 @@ 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
 	}{
@@ -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"},
 			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) {
-			got := normalizeFlatmapContainers(tc.attrs)
+			got := normalizeFlatmapContainers(tc.prior, tc.attrs)
 			if !reflect.DeepEqual(tc.expect, got) {
 				t.Fatalf("expected:\n%#v\ngot:\n%#v\n", tc.expect, got)
 			}

plans/objchange/compatible.go

@@ -197,9 +197,11 @@ func assertValueCompatible(planned, actual cty.Value, path cty.Path) []error {
 			return nil
 		}
 		errs = append(errs, path.NewErrorf("was %#v, but now null", planned))
+		return errs
 	}
 	if planned.IsNull() {
 		errs = append(errs, path.NewErrorf("was null, but now %#v", actual))
+		return errs
 	}
 
 	ty := planned.Type()