From ad9c89fc1991105dd627180060fc68abccf3a16d Mon Sep 17 00:00:00 2001
From: Alisdair McDiarmid <alisdair@users.noreply.github.com>
Date: Wed, 16 Mar 2022 10:34:08 -0400
Subject: [PATCH] cli: Fix missing identifying attributes in diff

When rendering a diff for an object value within a resource, Terraform
should always display the value of attributes which may be identifying.
At present, this is a simple rule: render attributes named "id", "name",
or "tags".

Prior to this commit, Terraform would only apply this rule to top-level
resource attributes and those inside nested blocks. Here we extend the
implementation to include object values in other contexts as well.
---
 internal/command/format/diff.go      |  5 +-
 internal/command/format/diff_test.go | 86 ++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/internal/command/format/diff.go b/internal/command/format/diff.go
index 1c1da14f4..43167772b 100644
--- a/internal/command/format/diff.go
+++ b/internal/command/format/diff.go
@@ -1620,7 +1620,10 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa
 					action = plans.Update
 				}
 
-				if action == plans.NoOp && !p.verbose {
+				// TODO: If in future we have a schema associated with this
+				// object, we should pass the attribute's schema to
+				// identifyingAttribute here.
+				if action == plans.NoOp && !p.verbose && !identifyingAttribute(k, nil) {
 					suppressedElements++
 					continue
 				}
diff --git a/internal/command/format/diff_test.go b/internal/command/format/diff_test.go
index 466e72fd8..334db2e5b 100644
--- a/internal/command/format/diff_test.go
+++ b/internal/command/format/diff_test.go
@@ -1131,6 +1131,92 @@ func TestResourceChange_JSON(t *testing.T) {
 	runTestCases(t, testCases)
 }
 
+func TestResourceChange_listObject(t *testing.T) {
+	testCases := map[string]testCase{
+		// https://github.com/hashicorp/terraform/issues/30641
+		"updating non-identifying attribute": {
+			Action: plans.Update,
+			Mode:   addrs.ManagedResourceMode,
+			Before: cty.ObjectVal(map[string]cty.Value{
+				"id": cty.StringVal("i-02ae66f368e8518a9"),
+				"accounts": cty.ListVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"id":     cty.StringVal("1"),
+						"name":   cty.StringVal("production"),
+						"status": cty.StringVal("ACTIVE"),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"id":     cty.StringVal("2"),
+						"name":   cty.StringVal("staging"),
+						"status": cty.StringVal("ACTIVE"),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"id":     cty.StringVal("3"),
+						"name":   cty.StringVal("disaster-recovery"),
+						"status": cty.StringVal("ACTIVE"),
+					}),
+				}),
+			}),
+			After: cty.ObjectVal(map[string]cty.Value{
+				"id": cty.UnknownVal(cty.String),
+				"accounts": cty.ListVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"id":     cty.StringVal("1"),
+						"name":   cty.StringVal("production"),
+						"status": cty.StringVal("ACTIVE"),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"id":     cty.StringVal("2"),
+						"name":   cty.StringVal("staging"),
+						"status": cty.StringVal("EXPLODED"),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"id":     cty.StringVal("3"),
+						"name":   cty.StringVal("disaster-recovery"),
+						"status": cty.StringVal("ACTIVE"),
+					}),
+				}),
+			}),
+			Schema: &configschema.Block{
+				Attributes: map[string]*configschema.Attribute{
+					"id": {Type: cty.String, Optional: true, Computed: true},
+					"accounts": {
+						Type: cty.List(cty.Object(map[string]cty.Type{
+							"id":     cty.String,
+							"name":   cty.String,
+							"status": cty.String,
+						})),
+					},
+				},
+			},
+			RequiredReplace: cty.NewPathSet(),
+			ExpectedOutput: `  # test_instance.example will be updated in-place
+  ~ resource "test_instance" "example" {
+      ~ accounts = [
+            {
+                id     = "1"
+                name   = "production"
+                status = "ACTIVE"
+            },
+          ~ {
+                id     = "2"
+                name   = "staging"
+              ~ status = "ACTIVE" -> "EXPLODED"
+            },
+            {
+                id     = "3"
+                name   = "disaster-recovery"
+                status = "ACTIVE"
+            },
+        ]
+      ~ id       = "i-02ae66f368e8518a9" -> (known after apply)
+    }
+`,
+		},
+	}
+	runTestCases(t, testCases)
+}
+
 func TestResourceChange_primitiveList(t *testing.T) {
 	testCases := map[string]testCase{
 		"in-place update - creation": {