diff --git a/builtin/providers/test/resource_diff_suppress.go b/builtin/providers/test/resource_diff_suppress.go index cb5f7358f..f5cfc9331 100644 --- a/builtin/providers/test/resource_diff_suppress.go +++ b/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 diff --git a/builtin/providers/test/resource_nested_set.go b/builtin/providers/test/resource_nested_set.go index d50c16da1..b808b7c98 100644 --- a/builtin/providers/test/resource_nested_set.go +++ b/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 { diff --git a/builtin/providers/test/resource_test.go b/builtin/providers/test/resource_test.go index dd33783ff..19c9f2611 100644 --- a/builtin/providers/test/resource_test.go +++ b/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, + }, + }, + }) +} diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index c7fbc8ebe..3e1e74520 100644 --- a/helper/plugin/grpc_provider.go +++ b/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) } } diff --git a/helper/plugin/grpc_provider_test.go b/helper/plugin/grpc_provider_test.go index 1dcc04df5..a9e65e818 100644 --- a/helper/plugin/grpc_provider_test.go +++ b/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) } diff --git a/plans/objchange/compatible.go b/plans/objchange/compatible.go index 8ac80ab05..ac11ebd02 100644 --- a/plans/objchange/compatible.go +++ b/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()