Merge pull request #29410 from hashicorp/jbardin/format-empty-nested-attrs

handle null and unknown values in attr diffs

internal/command/format/diff.go

@@ -471,28 +471,8 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At
 	}
 
 	if attrS.NestedType != nil {
-		renderNested := true
+		p.writeNestedAttrDiff(name, attrS.NestedType, old, new, nameLen, indent, path, action, showJustNew)
-
+		return false
-		// If the collection values are empty or null, we render them as single attributes
-		switch attrS.NestedType.Nesting {
-		case configschema.NestingList, configschema.NestingSet, configschema.NestingMap:
-			var oldLen, newLen int
-			if !old.IsNull() && old.IsKnown() {
-				oldLen = old.LengthInt()
-			}
-			if !new.IsNull() && new.IsKnown() {
-				newLen = new.LengthInt()
-			}
-
-			if oldLen+newLen == 0 {
-				renderNested = false
-			}
-		}
-
-		if renderNested {
-			p.writeNestedAttrDiff(name, attrS.NestedType, old, new, nameLen, indent, path, action, showJustNew)
-			return false
-		}
 	}
 
 	p.buf.WriteString("\n")
@@ -626,15 +606,24 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
 		p.buf.WriteString(strings.Repeat(" ", indent+2))
 		p.buf.WriteString("]")
 
+		if !new.IsKnown() {
+			p.buf.WriteString(" -> (known after apply)")
+		}
+
 	case configschema.NestingSet:
 		oldItems := ctyCollectionValues(old)
 		newItems := ctyCollectionValues(new)
 
-		allItems := make([]cty.Value, 0, len(oldItems)+len(newItems))
+		var all cty.Value
-		allItems = append(allItems, oldItems...)
+		if len(oldItems)+len(newItems) > 0 {
-		allItems = append(allItems, newItems...)
+			allItems := make([]cty.Value, 0, len(oldItems)+len(newItems))
+			allItems = append(allItems, oldItems...)
+			allItems = append(allItems, newItems...)
 
-		all := cty.SetVal(allItems)
+			all = cty.SetVal(allItems)
+		} else {
+			all = cty.SetValEmpty(old.Type().ElementType())
+		}
 
 		p.buf.WriteString(" = [")
 
@@ -646,6 +635,13 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
 			case !val.IsKnown():
 				action = plans.Update
 				newValue = val
+			case !new.IsKnown():
+				action = plans.Delete
+				// the value must have come from the old set
+				oldValue = val
+				// Mark the new val as null, but the entire set will be
+				// displayed as "(unknown after apply)"
+				newValue = cty.NullVal(val.Type())
 			case old.IsNull() || !old.HasElement(val).True():
 				action = plans.Create
 				oldValue = cty.NullVal(val.Type())
@@ -680,6 +676,10 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
 		p.buf.WriteString(strings.Repeat(" ", indent+2))
 		p.buf.WriteString("]")
 
+		if !new.IsKnown() {
+			p.buf.WriteString(" -> (known after apply)")
+		}
+
 	case configschema.NestingMap:
 		// For the sake of handling nested blocks, we'll treat a null map
 		// the same as an empty map since the config language doesn't
@@ -688,7 +688,12 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
 		new = ctyNullBlockMapAsEmpty(new)
 
 		oldItems := old.AsValueMap()
-		newItems := new.AsValueMap()
+
+		newItems := map[string]cty.Value{}
+
+		if new.IsKnown() {
+			newItems = new.AsValueMap()
+		}
 
 		allKeys := make(map[string]bool)
 		for k := range oldItems {
@@ -710,6 +715,7 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
 		for _, k := range allKeysOrder {
 			var action plans.Action
 			oldValue := oldItems[k]
+
 			newValue := newItems[k]
 			switch {
 			case oldValue == cty.NilVal:
@@ -745,6 +751,9 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
 		p.writeSkippedElems(unchanged, indent+4)
 		p.buf.WriteString(strings.Repeat(" ", indent+2))
 		p.buf.WriteString("}")
+		if !new.IsKnown() {
+			p.buf.WriteString(" -> (known after apply)")
+		}
 	}
 
 	return

internal/command/format/diff_test.go

@@ -2633,6 +2633,56 @@ func TestResourceChange_nestedList(t *testing.T) {
           ~ attr = "y" -> "z"
         }
     }
+`,
+		},
+		"in-place update - unknown": {
+			Action: plans.Update,
+			Mode:   addrs.ManagedResourceMode,
+			Before: cty.ObjectVal(map[string]cty.Value{
+				"id":  cty.StringVal("i-02ae66f368e8518a9"),
+				"ami": cty.StringVal("ami-BEFORE"),
+				"disks": cty.ListVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"mount_point": cty.StringVal("/var/diska"),
+						"size":        cty.StringVal("50GB"),
+					}),
+				}),
+				"root_block_device": cty.ListVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"volume_type": cty.StringVal("gp2"),
+						"new_field":   cty.StringVal("new_value"),
+					}),
+				}),
+			}),
+			After: cty.ObjectVal(map[string]cty.Value{
+				"id":  cty.StringVal("i-02ae66f368e8518a9"),
+				"ami": cty.StringVal("ami-AFTER"),
+				"disks": cty.UnknownVal(cty.List(cty.Object(map[string]cty.Type{
+					"mount_point": cty.String,
+					"size":        cty.String,
+				}))),
+				"root_block_device": cty.ListVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"volume_type": cty.StringVal("gp2"),
+						"new_field":   cty.StringVal("new_value"),
+					}),
+				}),
+			}),
+			RequiredReplace: cty.NewPathSet(),
+			Schema:          testSchemaPlus(configschema.NestingList),
+			ExpectedOutput: `  # test_instance.example will be updated in-place
+  ~ resource "test_instance" "example" {
+      ~ ami   = "ami-BEFORE" -> "ami-AFTER"
+      ~ disks = [
+          ~ {
+            - mount_point = "/var/diska" -> null
+            - size        = "50GB" -> null
+          },
+        ] -> (known after apply)
+        id    = "i-02ae66f368e8518a9"
+
+        # (1 unchanged block hidden)
+    }
 `,
 		},
 	}
@@ -2893,7 +2943,8 @@ func TestResourceChange_nestedSet(t *testing.T) {
 			ExpectedOutput: `  # test_instance.example will be updated in-place
   ~ resource "test_instance" "example" {
       ~ ami   = "ami-BEFORE" -> "ami-AFTER"
-      + disks = []
+      + disks = [
+        ]
         id    = "i-02ae66f368e8518a9"
     }
 `,
@@ -2952,6 +3003,56 @@ func TestResourceChange_nestedSet(t *testing.T) {
           - volume_type = "gp2" -> null
         }
     }
+`,
+		},
+		"in-place update - unknown": {
+			Action: plans.Update,
+			Mode:   addrs.ManagedResourceMode,
+			Before: cty.ObjectVal(map[string]cty.Value{
+				"id":  cty.StringVal("i-02ae66f368e8518a9"),
+				"ami": cty.StringVal("ami-BEFORE"),
+				"disks": cty.SetVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"mount_point": cty.StringVal("/var/diska"),
+						"size":        cty.StringVal("50GB"),
+					}),
+				}),
+				"root_block_device": cty.SetVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"volume_type": cty.StringVal("gp2"),
+						"new_field":   cty.StringVal("new_value"),
+					}),
+				}),
+			}),
+			After: cty.ObjectVal(map[string]cty.Value{
+				"id":  cty.StringVal("i-02ae66f368e8518a9"),
+				"ami": cty.StringVal("ami-AFTER"),
+				"disks": cty.UnknownVal(cty.Set(cty.Object(map[string]cty.Type{
+					"mount_point": cty.String,
+					"size":        cty.String,
+				}))),
+				"root_block_device": cty.SetVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"volume_type": cty.StringVal("gp2"),
+						"new_field":   cty.StringVal("new_value"),
+					}),
+				}),
+			}),
+			RequiredReplace: cty.NewPathSet(),
+			Schema:          testSchemaPlus(configschema.NestingSet),
+			ExpectedOutput: `  # test_instance.example will be updated in-place
+  ~ resource "test_instance" "example" {
+      ~ ami   = "ami-BEFORE" -> "ami-AFTER"
+      ~ disks = [
+          - {
+            - mount_point = "/var/diska" -> null
+            - size        = "50GB" -> null
+          },
+        ] -> (known after apply)
+        id    = "i-02ae66f368e8518a9"
+
+        # (1 unchanged block hidden)
+    }
 `,
 		},
 	}
@@ -3288,6 +3389,115 @@ func TestResourceChange_nestedMap(t *testing.T) {
           - volume_type = "gp2" -> null
         }
     }
+`,
+		},
+		"in-place update - unknown": {
+			Action: plans.Update,
+			Mode:   addrs.ManagedResourceMode,
+			Before: cty.ObjectVal(map[string]cty.Value{
+				"id":  cty.StringVal("i-02ae66f368e8518a9"),
+				"ami": cty.StringVal("ami-BEFORE"),
+				"disks": cty.MapVal(map[string]cty.Value{
+					"disk_a": cty.ObjectVal(map[string]cty.Value{
+						"mount_point": cty.StringVal("/var/diska"),
+						"size":        cty.StringVal("50GB"),
+					}),
+				}),
+				"root_block_device": cty.MapVal(map[string]cty.Value{
+					"a": cty.ObjectVal(map[string]cty.Value{
+						"volume_type": cty.StringVal("gp2"),
+						"new_field":   cty.StringVal("new_value"),
+					}),
+				}),
+			}),
+			After: cty.ObjectVal(map[string]cty.Value{
+				"id":  cty.StringVal("i-02ae66f368e8518a9"),
+				"ami": cty.StringVal("ami-AFTER"),
+				"disks": cty.UnknownVal(cty.Map(cty.Object(map[string]cty.Type{
+					"mount_point": cty.String,
+					"size":        cty.String,
+				}))),
+				"root_block_device": cty.MapVal(map[string]cty.Value{
+					"a": cty.ObjectVal(map[string]cty.Value{
+						"volume_type": cty.StringVal("gp2"),
+						"new_field":   cty.StringVal("new_value"),
+					}),
+				}),
+			}),
+			RequiredReplace: cty.NewPathSet(),
+			Schema:          testSchemaPlus(configschema.NestingMap),
+			ExpectedOutput: `  # test_instance.example will be updated in-place
+  ~ resource "test_instance" "example" {
+      ~ ami   = "ami-BEFORE" -> "ami-AFTER"
+      ~ disks = {
+          - "disk_a" = {
+            - mount_point = "/var/diska" -> null
+            - size        = "50GB" -> null
+          },
+        } -> (known after apply)
+        id    = "i-02ae66f368e8518a9"
+
+        # (1 unchanged block hidden)
+    }
+`,
+		},
+		"in-place update - insertion sensitive": {
+			Action: plans.Update,
+			Mode:   addrs.ManagedResourceMode,
+			Before: cty.ObjectVal(map[string]cty.Value{
+				"id":  cty.StringVal("i-02ae66f368e8518a9"),
+				"ami": cty.StringVal("ami-BEFORE"),
+				"disks": cty.MapValEmpty(cty.Object(map[string]cty.Type{
+					"mount_point": cty.String,
+					"size":        cty.String,
+				})),
+				"root_block_device": cty.MapVal(map[string]cty.Value{
+					"a": cty.ObjectVal(map[string]cty.Value{
+						"volume_type": cty.StringVal("gp2"),
+						"new_field":   cty.StringVal("new_value"),
+					}),
+				}),
+			}),
+			After: cty.ObjectVal(map[string]cty.Value{
+				"id":  cty.StringVal("i-02ae66f368e8518a9"),
+				"ami": cty.StringVal("ami-AFTER"),
+				"disks": cty.MapVal(map[string]cty.Value{
+					"disk_a": cty.ObjectVal(map[string]cty.Value{
+						"mount_point": cty.StringVal("/var/diska"),
+						"size":        cty.StringVal("50GB"),
+					}),
+				}),
+				"root_block_device": cty.MapVal(map[string]cty.Value{
+					"a": cty.ObjectVal(map[string]cty.Value{
+						"volume_type": cty.StringVal("gp2"),
+						"new_field":   cty.StringVal("new_value"),
+					}),
+				}),
+			}),
+			AfterValMarks: []cty.PathValueMarks{
+				{
+					Path: cty.Path{cty.GetAttrStep{Name: "disks"},
+						cty.IndexStep{Key: cty.StringVal("disk_a")},
+						cty.GetAttrStep{Name: "mount_point"},
+					},
+					Marks: cty.NewValueMarks(marks.Sensitive),
+				},
+			},
+			RequiredReplace: cty.NewPathSet(),
+			Schema:          testSchemaPlus(configschema.NestingMap),
+			ExpectedOutput: `  # test_instance.example will be updated in-place
+  ~ resource "test_instance" "example" {
+      ~ ami   = "ami-BEFORE" -> "ami-AFTER"
+      ~ disks = {
+          + "disk_a" = {
+            + mount_point = (sensitive)
+            + size        = "50GB"
+          },
+        }
+        id    = "i-02ae66f368e8518a9"
+
+        # (1 unchanged block hidden)
+    }
 `,
 		},
 	}