Merge pull request #20577 from hashicorp/jbardin/shim-list-elements

don't re-add removed list values even when planned

builtin/providers/test/resource_list.go

@@ -58,21 +58,54 @@ func testResourceList() *schema.Resource {
 					},
 				},
 			},
+			"dependent_list": {
+				Type:     schema.TypeList,
+				Optional: true,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"val": {
+							Type:     schema.TypeString,
+							Required: true,
+						},
+					},
+				},
+			},
+			"computed_list": {
+				Type:     schema.TypeList,
+				Computed: true,
+				Elem:     &schema.Schema{Type: schema.TypeString},
+			},
 		},
 	}
 }
 
 func testResourceListCreate(d *schema.ResourceData, meta interface{}) error {
 	d.SetId("testId")
-	return nil
+	return testResourceListRead(d, meta)
 }
 
 func testResourceListRead(d *schema.ResourceData, meta interface{}) error {
+	fixedIps := d.Get("dependent_list")
+
+	// all_fixed_ips should be set as computed with a CustomizeDiff func, but
+	// we're trying to emulate legacy provider behavior, and updating a
+	// computed field was a common case.
+	ips := []interface{}{}
+	if fixedIps != nil {
+		for _, v := range fixedIps.([]interface{}) {
+			m := v.(map[string]interface{})
+			ips = append(ips, m["val"])
+		}
+	}
+	if err := d.Set("computed_list", ips); err != nil {
+		return err
+	}
+
 	return nil
 }
 
 func testResourceListUpdate(d *schema.ResourceData, meta interface{}) error {
-	return nil
+	return testResourceListRead(d, meta)
 }
 
 func testResourceListDelete(d *schema.ResourceData, meta interface{}) error {

builtin/providers/test/resource_list_test.go

@@ -276,3 +276,45 @@ resource "test_resource_list" "foo" {
 		},
 	})
 }
+
+func TestResourceList_addRemove(t *testing.T) {
+	resource.UnitTest(t, resource.TestCase{
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckResourceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_list" "foo" {
+}
+				`),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("test_resource_list.foo", "computed_list.#", "0"),
+					resource.TestCheckResourceAttr("test_resource_list.foo", "dependent_list.#", "0"),
+				),
+			},
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_list" "foo" {
+	dependent_list {
+		val = "a"
+	}
+}
+				`),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("test_resource_list.foo", "computed_list.#", "1"),
+					resource.TestCheckResourceAttr("test_resource_list.foo", "dependent_list.#", "1"),
+				),
+			},
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_list" "foo" {
+}
+				`),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("test_resource_list.foo", "computed_list.#", "0"),
+					resource.TestCheckResourceAttr("test_resource_list.foo", "dependent_list.#", "0"),
+				),
+			},
+		},
+	})
+}

helper/plugin/grpc_provider.go

@@ -431,7 +431,7 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso
 		// here we use the prior state to check for unknown/zero containers values
 		// when normalizing the flatmap.
 		stateAttrs := hcl2shim.FlatmapValueFromHCL2(stateVal)
-		newInstanceState.Attributes = normalizeFlatmapContainers(stateAttrs, newInstanceState.Attributes, false)
+		newInstanceState.Attributes = normalizeFlatmapContainers(stateAttrs, newInstanceState.Attributes)
 	}
 
 	if newInstanceState == nil || newInstanceState.ID == "" {
@@ -572,7 +572,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(priorState.Attributes, plannedAttrs, false)
+	plannedAttrs = normalizeFlatmapContainers(priorState.Attributes, plannedAttrs)
 
 	plannedStateVal, err := hcl2shim.HCL2ValueFromFlatmap(plannedAttrs, block.ImpliedType())
 	if err != nil {
@@ -808,7 +808,7 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
 	// here we use the planned state to check for unknown/zero containers values
 	// when normalizing the flatmap.
 	plannedState := hcl2shim.FlatmapValueFromHCL2(plannedStateVal)
-	newInstanceState.Attributes = normalizeFlatmapContainers(plannedState, newInstanceState.Attributes, true)
+	newInstanceState.Attributes = normalizeFlatmapContainers(plannedState, newInstanceState.Attributes)
 
 	// We keep the null val if we destroyed the resource, otherwise build the
 	// entire object, even if the new state was nil.
@@ -819,6 +819,7 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
 	}
 
 	newStateVal = normalizeNullValues(newStateVal, plannedStateVal, false)
+
 	newStateVal = copyTimeoutValues(newStateVal, plannedStateVal)
 
 	newStateMP, err := msgpack.Marshal(newStateVal, block.ImpliedType())
@@ -995,7 +996,7 @@ func pathToAttributePath(path cty.Path) *proto.AttributePath {
 // allows a provider to set an empty computed container in the state without
 // creating perpetual diff. This can differ slightly between plan and apply, so
 // the apply flag is passed when called from ApplyResourceChange.
-func normalizeFlatmapContainers(prior map[string]string, attrs map[string]string, apply bool) map[string]string {
+func normalizeFlatmapContainers(prior map[string]string, attrs map[string]string) map[string]string {
 	isCount := regexp.MustCompile(`.\.[%#]$`).MatchString
 
 	// While we can't determine if the value was actually computed here, we will
@@ -1009,11 +1010,6 @@ func normalizeFlatmapContainers(prior map[string]string, attrs map[string]string
 		if isCount(k) && (v == "0" || v == hcl2shim.UnknownVariableValue) {
 			zeros[k] = true
 		}
-
-		// fixup any 1->0 conversions that happened during Apply
-		if apply && isCount(k) && v == "1" && attrs[k] == "0" {
-			attrs[k] = "1"
-		}
 	}
 
 	for k, v := range attrs {
@@ -1291,7 +1287,7 @@ func normalizeNullValues(dst, src cty.Value, plan bool) cty.Value {
 		// If the dst is nil, and the src is known, then we lost an empty value
 		// so take the original.
 		if dst.IsNull() {
-			if src.IsWhollyKnown() && !plan {
+			if src.IsWhollyKnown() && src.LengthInt() == 0 && !plan {
 				return src
 			}
 			return dst

helper/plugin/grpc_provider_test.go

@@ -716,7 +716,7 @@ func TestNormalizeFlatmapContainers(t *testing.T) {
 		},
 	} {
 		t.Run(strconv.Itoa(i), func(t *testing.T) {
-			got := normalizeFlatmapContainers(tc.prior, tc.attrs, false)
+			got := normalizeFlatmapContainers(tc.prior, tc.attrs)
 			if !reflect.DeepEqual(tc.expect, got) {
 				t.Fatalf("expected:\n%#v\ngot:\n%#v\n", tc.expect, got)
 			}