Repository: pulumi/pulumi-terraform-bridge
Pull request #319 (2021-01-19 03:25:05)

Correctly handle replacements for sets and lists with nested elements
There is a bug in our diffing function's handling of sets and lists with nested elements when the set or list itself has `ForceNew` but the nested element does not.

To correct this, we also check if the list/set count diff has `ForceNew` and treat `A`/`D` as `AR`/`DR` if so.

Fixes: https://github.com/pulumi/pulumi-aws/issues/1297

github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/e80cc8e..e32c406) with merge commit a4e6fc97345a150ad499cd7a7ca8a6ea4442e019
github-actions[bot]: Diff for [pulumi-azure](https://github.com/pulumi-bot/pulumi-azure/compare/5e19d1c62..a9d1a66c8) with merge commit a4e6fc97345a150ad499cd7a7ca8a6ea4442e019
github-actions[bot]: Diff for [pulumi-random](https://github.com/pulumi-bot/pulumi-random/compare/02847f1..36b8fd1) with merge commit b3f157dae7e7a4b624d442e79e8a9412b84bda4f
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/a611dad..9752a18) with merge commit b3f157dae7e7a4b624d442e79e8a9412b84bda4f
github-actions[bot]: Diff for [pulumi-gcp](https://github.com/pulumi-bot/pulumi-gcp/compare/c102c164..c87ff954) with merge commit b3f157dae7e7a4b624d442e79e8a9412b84bda4f
github-actions[bot]: Diff for [pulumi-azure](https://github.com/pulumi-bot/pulumi-azure/compare/47f8fdc9e..c1ce2f0f3) with merge commit b3f157dae7e7a4b624d442e79e8a9412b84bda4f
github-actions[bot]: Diff for [pulumi-aws](https://github.com/pulumi-bot/pulumi-aws/compare/9479c2526..91f7ca68a) with merge commit b3f157dae7e7a4b624d442e79e8a9412b84bda4f
github-actions[bot]: Diff for [pulumi-random](https://github.com/pulumi-bot/pulumi-random/compare/ea47f2b..e59f483) with merge commit a960491ccee00e8220601e0c124c91699bbc1a11
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/27d2383..8e02335) with merge commit a960491ccee00e8220601e0c124c91699bbc1a11
github-actions[bot]: Diff for [pulumi-gcp](https://github.com/pulumi-bot/pulumi-gcp/compare/495edf9e..01739b10) with merge commit a960491ccee00e8220601e0c124c91699bbc1a11
github-actions[bot]: Diff for [pulumi-azure](https://github.com/pulumi-bot/pulumi-azure/compare/ef959f6fc..42a340406) with merge commit a960491ccee00e8220601e0c124c91699bbc1a11
github-actions[bot]: Diff for [pulumi-aws](https://github.com/pulumi-bot/pulumi-aws/compare/a382ac32d..ea5d66c25) with merge commit a960491ccee00e8220601e0c124c91699bbc1a11
github-actions[bot]: Diff for [pulumi-random](https://github.com/pulumi-bot/pulumi-random/compare/be7b8da..8e22ca4) with merge commit f322a623e277ff7f513d2e41889aab3bf050583f
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/d858619..a746d3a) with merge commit f322a623e277ff7f513d2e41889aab3bf050583f
github-actions[bot]: Diff for [pulumi-gcp](https://github.com/pulumi-bot/pulumi-gcp/compare/e6bb3a94..4188e409) with merge commit f322a623e277ff7f513d2e41889aab3bf050583f
github-actions[bot]: Diff for [pulumi-azure](https://github.com/pulumi-bot/pulumi-azure/compare/0cab948c5..8ee626297) with merge commit f322a623e277ff7f513d2e41889aab3bf050583f
github-actions[bot]: Diff for [pulumi-aws](https://github.com/pulumi-bot/pulumi-aws/compare/086f7312d..5b59f93c8) with merge commit f322a623e277ff7f513d2e41889aab3bf050583f
github-actions[bot]: Diff for [pulumi-random](https://github.com/pulumi-bot/pulumi-random/compare/bd02dbc..f2535f1) with merge commit 8c7533bb1e3739deb58f373e5aba8465a50ff57b
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/f1d5f11..5eac8cb) with merge commit 8c7533bb1e3739deb58f373e5aba8465a50ff57b
github-actions[bot]: Diff for [pulumi-gcp](https://github.com/pulumi-bot/pulumi-gcp/compare/bc67a97e..fbfe25db) with merge commit 8c7533bb1e3739deb58f373e5aba8465a50ff57b
github-actions[bot]: Diff for [pulumi-aws](https://github.com/pulumi-bot/pulumi-aws/compare/05a5cdd84..ac7e6f510) with merge commit 8c7533bb1e3739deb58f373e5aba8465a50ff57b
github-actions[bot]: Diff for [pulumi-azure](https://github.com/pulumi-bot/pulumi-azure/compare/8cbb30d55..cf7e634b0) with merge commit 8c7533bb1e3739deb58f373e5aba8465a50ff57b
github-actions[bot]: Diff for [pulumi-random](https://github.com/pulumi-bot/pulumi-random/compare/51765b2..0bd9f5c) with merge commit b8587991c1a4735b1241ef1c696275cbaf87d5bd
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/ff14194..5086602) with merge commit b8587991c1a4735b1241ef1c696275cbaf87d5bd
github-actions[bot]: Diff for [pulumi-gcp](https://github.com/pulumi-bot/pulumi-gcp/compare/308e49be..1174bac4) with merge commit b8587991c1a4735b1241ef1c696275cbaf87d5bd
github-actions[bot]: Diff for [pulumi-azure](https://github.com/pulumi-bot/pulumi-azure/compare/3fcc266d5..bb2f9cd3f) with merge commit b8587991c1a4735b1241ef1c696275cbaf87d5bd
github-actions[bot]: Diff for [pulumi-aws](https://github.com/pulumi-bot/pulumi-aws/compare/709084640..31ce2c8e3) with merge commit b8587991c1a4735b1241ef1c696275cbaf87d5bd
komalali: > I think it might be cleaner to pass a bit through the property visitor that indicates whether or not we're inside an add/delete of an array element.

@pgavlin I'm not really clear on how exactly to do this, but please feel free to make a follow-up change with what you're suggesting.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a13e917d..df45bcdf 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,9 @@ CHANGELOG
 
 ## HEAD (Unreleased)
 
+- Fixes a panic caused by improper diffing of lists and sets with nested object elements.
+  [#319](https://github.com/pulumi/pulumi-terraform-bridge/pull/319)
+
 - Add go package info options to schema
   [#321](https://github.com/pulumi/pulumi-terraform-bridge/pull/321)
 
diff --git a/pkg/tfbridge/diff.go b/pkg/tfbridge/diff.go
index 8f9020f4..234a5bb5 100644
--- a/pkg/tfbridge/diff.go
+++ b/pkg/tfbridge/diff.go
@@ -184,6 +184,14 @@ func makePropertyDiff(name, path string, v resource.PropertyValue, tfDiff shim.I
 		if d := tfDiff.Attribute(name); d != nil && d.Old != d.New {
 			other, hasOtherDiff := diff[path]
 
+			var arrDiff *shim.ResourceAttrDiff
+			// If this is an element of a list or set, get the count diff if it exists.
+			if strings.Contains(name, ".") {
+				arrName := strings.Split(name, ".")[0]
+				arrName += ".#"
+				arrDiff = tfDiff.Attribute(arrName)
+			}
+
 			// If we're finalizing the diff, we want to remove any ADD diffs that were only present in the state.
 			// These diffs are typically changes to output properties that we don't care about.
 			if finalize {
@@ -197,13 +205,13 @@ func makePropertyDiff(name, path string, v resource.PropertyValue, tfDiff shim.I
 			var kind pulumirpc.PropertyDiff_Kind
 			switch {
 			case d.NewRemoved:
-				if d.RequiresNew {
+				if d.RequiresNew || (arrDiff != nil && arrDiff.RequiresNew) {
 					kind = pulumirpc.PropertyDiff_DELETE_REPLACE
 				} else {
 					kind = pulumirpc.PropertyDiff_DELETE
 				}
 			case !hasOtherDiff:
-				if d.RequiresNew {
+				if d.RequiresNew || (arrDiff != nil && arrDiff.RequiresNew) {
 					kind = pulumirpc.PropertyDiff_ADD_REPLACE
 				} else {
 					kind = pulumirpc.PropertyDiff_ADD


Test patch:
diff --git a/pkg/tfbridge/diff_test.go b/pkg/tfbridge/diff_test.go
index 787565a2..9948233c 100644
--- a/pkg/tfbridge/diff_test.go
+++ b/pkg/tfbridge/diff_test.go
@@ -1474,3 +1474,186 @@ func TestCollectionsWithMultipleItems(t *testing.T) {
 		}
 	})
 }
+
+func TestSetNestedAddReplace(t *testing.T) {
+	diffTest(t,
+		map[string]*schema.Schema{
+			"prop": {
+				Type: schema.TypeSet,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"nest": {Type: schema.TypeString, Required: true},
+					},
+				},
+				ForceNew: true,
+			},
+			"outp": {Type: schema.TypeString, Computed: true},
+		},
+		map[string]*SchemaInfo{},
+		map[string]interface{}{
+			"prop": []interface{}{map[string]interface{}{"nest": "baz"}},
+		},
+		map[string]interface{}{
+			"prop": nil,
+			"outp": "bar",
+		},
+		map[string]DiffKind{
+			"prop[0].nest": AR,
+		})
+}
+
+func TestListNestedAddReplace(t *testing.T) {
+	diffTest(t,
+		// tfSchema
+		map[string]*schema.Schema{
+			"prop": {
+				Type: schema.TypeList,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"nest": {Type: schema.TypeString, Required: true},
+					},
+				},
+				ForceNew: true,
+			},
+			"outp": {Type: schema.TypeString, Computed: true},
+		},
+		// info
+		map[string]*SchemaInfo{},
+		// inputs
+		map[string]interface{}{
+			"prop": []interface{}{map[string]interface{}{"nest": "foo"}},
+		},
+		// state
+		map[string]interface{}{
+			"prop": nil,
+			"outp": "bar",
+		},
+		// expected
+		map[string]DiffKind{
+			"prop[0].nest": AR,
+		})
+}
+
+func TestListNestedUpdate(t *testing.T) {
+	diffTest(t,
+		// tfSchema
+		map[string]*schema.Schema{
+			"prop": {
+				Type: schema.TypeList,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"nest": {Type: schema.TypeString, Required: true},
+					},
+				},
+				ForceNew: true,
+			},
+			"outp": {Type: schema.TypeString, Computed: true},
+		},
+		// info
+		map[string]*SchemaInfo{},
+		// inputs
+		map[string]interface{}{
+			"prop": []interface{}{map[string]interface{}{"nest": "foo"}},
+		},
+		// state
+		map[string]interface{}{
+			"prop": []interface{}{map[string]interface{}{"nest": "bar"}},
+			"outp": "bar",
+		},
+		// expected
+		map[string]DiffKind{
+			"prop[0].nest": U,
+		})
+}
+
+func TestListNestedDeleteReplace(t *testing.T) {
+	diffTest(t,
+		// tfSchema
+		map[string]*schema.Schema{
+			"prop": {
+				Type: schema.TypeList,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"nest": {Type: schema.TypeString, Required: true},
+					},
+				},
+				ForceNew: true,
+			},
+			"outp": {Type: schema.TypeString, Computed: true},
+		},
+		// info
+		map[string]*SchemaInfo{},
+		// inputs
+		map[string]interface{}{},
+		// state
+		map[string]interface{}{
+			"prop": []interface{}{map[string]interface{}{"nest": "bar"}},
+			"outp": "bar",
+		},
+		// expected
+		map[string]DiffKind{
+			"prop[0].nest": DR,
+		})
+}
+
+func TestSetNestedDeleteReplace(t *testing.T) {
+	diffTest(t,
+		// tfSchema
+		map[string]*schema.Schema{
+			"prop": {
+				Type: schema.TypeSet,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"nest": {Type: schema.TypeString, Required: true},
+					},
+				},
+				ForceNew: true,
+			},
+			"outp": {Type: schema.TypeString, Computed: true},
+		},
+		// info
+		map[string]*SchemaInfo{},
+		// inputs
+		map[string]interface{}{},
+		// state
+		map[string]interface{}{
+			"prop": []interface{}{map[string]interface{}{"nest": "bar"}},
+			"outp": "bar",
+		},
+		// expected
+		map[string]DiffKind{
+			"prop[0].nest": DR,
+		})
+}
+
+func TestListNestedAddMaxItemsOne(t *testing.T) {
+	diffTest(t,
+		// tfSchema
+		map[string]*schema.Schema{
+			"prop": {
+				Type: schema.TypeList,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"nest": {Type: schema.TypeString, Required: true},
+					},
+				},
+				MaxItems: 1,
+				ForceNew: true,
+			},
+			"outp": {Type: schema.TypeString, Computed: true},
+		},
+		// info
+		map[string]*SchemaInfo{},
+		// inputs
+		map[string]interface{}{
+			"prop": map[string]interface{}{"nest": "foo"},
+		},
+		// state
+		map[string]interface{}{
+			"outp": "bar",
+		},
+		// expected
+		map[string]DiffKind{
+			"prop.nest": AR,
+		})
+}