Merge pull request #26367 from hashicorp/pselle/sensitive-diff-format

Warnings and specialized diffs when switching between sensitive values

command/format/diff.go

@@ -337,6 +337,9 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At
 	}
 
 	p.buf.WriteString("\n")
+
+	p.writeSensitivityWarning(old, new, indent, action)
+
 	p.buf.WriteString(strings.Repeat(" ", indent))
 	p.writeActionSymbol(action)
 
@@ -767,12 +770,6 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa
 	ty := old.Type()
 	typesEqual := ctyTypesEqual(ty, new.Type())
 
-	// If either the old or new value is marked, don't display the value
-	if old.ContainsMarked() || new.ContainsMarked() {
-		p.buf.WriteString("(sensitive)")
-		return
-	}
-
 	// We have some specialized diff implementations for certain complex
 	// values where it's useful to see a visualization of the diff of
 	// the nested elements rather than just showing the entire old and
@@ -780,13 +777,26 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa
 	// However, these specialized implementations can apply only if both
 	// values are known and non-null.
 	if old.IsKnown() && new.IsKnown() && !old.IsNull() && !new.IsNull() && typesEqual {
+		// Create unmarked values for comparisons
+		unmarkedOld, oldMarks := old.UnmarkDeep()
+		unmarkedNew, newMarks := new.UnmarkDeep()
 		switch {
+		case ty == cty.Bool || ty == cty.Number:
+			if len(oldMarks) > 0 || len(newMarks) > 0 {
+				p.buf.WriteString("(sensitive)")
+				return
+			}
 		case ty == cty.String:
 			// We have special behavior for both multi-line strings in general
 			// and for strings that can parse as JSON. For the JSON handling
 			// to apply, both old and new must be valid JSON.
 			// For single-line strings that don't parse as JSON we just fall
 			// out of this switch block and do the default old -> new rendering.
+
+			if len(oldMarks) > 0 || len(newMarks) > 0 {
+				p.buf.WriteString("(sensitive)")
+				return
+			}
 			oldS := old.AsString()
 			newS := new.AsString()
 
@@ -811,7 +821,7 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa
 							p.buf.WriteString(strings.Repeat(" ", indent))
 							p.buf.WriteByte(')')
 						} else {
-							// if they differ only in insigificant whitespace
+							// if they differ only in insignificant whitespace
 							// then we'll note that but still expand out the
 							// effective value.
 							if p.pathForcesNewResource(path) {
@@ -1104,7 +1114,7 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa
 					action = plans.Create
 				} else if new.HasIndex(kV).False() {
 					action = plans.Delete
-				} else if eqV := old.Index(kV).Equals(new.Index(kV)); eqV.IsKnown() && eqV.True() {
+				} else if eqV := unmarkedOld.Index(kV).Equals(unmarkedNew.Index(kV)); eqV.IsKnown() && eqV.True() {
 					action = plans.NoOp
 				} else {
 					action = plans.Update
@@ -1117,6 +1127,10 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa
 
 				path := append(path, cty.IndexStep{Key: kV})
 
+				oldV := old.Index(kV)
+				newV := new.Index(kV)
+				p.writeSensitivityWarning(oldV, newV, indent+2, action)
+
 				p.buf.WriteString(strings.Repeat(" ", indent+2))
 				p.writeActionSymbol(action)
 				p.writeValue(kV, action, indent+4)
@@ -1125,15 +1139,21 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa
 				switch action {
 				case plans.Create, plans.NoOp:
 					v := new.Index(kV)
-					p.writeValue(v, action, indent+4)
+					if v.IsMarked() {
+						p.buf.WriteString("(sensitive)")
+					} else {
+						p.writeValue(v, action, indent+4)
+					}
 				case plans.Delete:
 					oldV := old.Index(kV)
 					newV := cty.NullVal(oldV.Type())
 					p.writeValueDiff(oldV, newV, indent+4, path)
 				default:
-					oldV := old.Index(kV)
-					newV := new.Index(kV)
-					p.writeValueDiff(oldV, newV, indent+4, path)
+					if oldV.IsMarked() || newV.IsMarked() {
+						p.buf.WriteString("(sensitive)")
+					} else {
+						p.writeValueDiff(oldV, newV, indent+4, path)
+					}
 				}
 
 				p.buf.WriteByte('\n')
@@ -1287,6 +1307,28 @@ func (p *blockBodyDiffPrinter) writeActionSymbol(action plans.Action) {
 	}
 }
 
+func (p *blockBodyDiffPrinter) writeSensitivityWarning(old, new cty.Value, indent int, action plans.Action) {
+	// Dont' show this warning for create or delete
+	if action == plans.Create || action == plans.Delete {
+		return
+	}
+
+	if new.IsMarked() && !old.IsMarked() {
+		p.buf.WriteString(strings.Repeat(" ", indent))
+		p.buf.WriteString(p.color.Color("[yellow]# Warning: this attribute value will be marked as sensitive and will\n"))
+		p.buf.WriteString(strings.Repeat(" ", indent))
+		p.buf.WriteString(p.color.Color("[yellow]# not display in UI output after applying this change[reset]\n"))
+	}
+
+	// Note if changing this attribute will change its sensitivity
+	if old.IsMarked() && !new.IsMarked() {
+		p.buf.WriteString(strings.Repeat(" ", indent))
+		p.buf.WriteString(p.color.Color("[yellow]# Warning: this attribute value will no longer be marked as sensitive\n"))
+		p.buf.WriteString(strings.Repeat(" ", indent))
+		p.buf.WriteString(p.color.Color("[yellow]# after applying this change[reset]\n"))
+	}
+}
+
 func (p *blockBodyDiffPrinter) pathForcesNewResource(path cty.Path) bool {
 	if !p.action.IsReplace() || p.requiredReplace.Empty() {
 		// "requiredReplace" only applies when the instance is being replaced,

command/format/diff_test.go

@@ -3652,34 +3652,57 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
     }
 `,
 		},
-		"in-place update - before sensitive": {
+		"in-place update - before sensitive, primitive types": {
 			Action: plans.Update,
 			Mode:   addrs.ManagedResourceMode,
 			Before: cty.ObjectVal(map[string]cty.Value{
-				"id":  cty.StringVal("i-02ae66f368e8518a9"),
-				"ami": cty.StringVal("ami-BEFORE"),
+				"id":          cty.StringVal("i-02ae66f368e8518a9"),
+				"ami":         cty.StringVal("ami-BEFORE"),
+				"special":     cty.BoolVal(true),
+				"some_number": cty.NumberIntVal(1),
 			}),
 			After: cty.ObjectVal(map[string]cty.Value{
-				"id":  cty.StringVal("i-02ae66f368e8518a9"),
-				"ami": cty.StringVal("ami-AFTER"),
+				"id":          cty.StringVal("i-02ae66f368e8518a9"),
+				"ami":         cty.StringVal("ami-AFTER"),
+				"special":     cty.BoolVal(false),
+				"some_number": cty.NumberIntVal(2),
 			}),
 			BeforeValMarks: []cty.PathValueMarks{
 				{
 					Path:  cty.Path{cty.GetAttrStep{Name: "ami"}},
 					Marks: cty.NewValueMarks("sensitive"),
-				}},
+				},
+				{
+					Path:  cty.Path{cty.GetAttrStep{Name: "special"}},
+					Marks: cty.NewValueMarks("sensitive"),
+				},
+				{
+					Path:  cty.Path{cty.GetAttrStep{Name: "some_number"}},
+					Marks: cty.NewValueMarks("sensitive"),
+				},
+			},
 			RequiredReplace: cty.NewPathSet(),
 			Tainted:         false,
 			Schema: &configschema.Block{
 				Attributes: map[string]*configschema.Attribute{
-					"id":  {Type: cty.String, Optional: true, Computed: true},
-					"ami": {Type: cty.String, Optional: true},
+					"id":          {Type: cty.String, Optional: true, Computed: true},
+					"ami":         {Type: cty.String, Optional: true},
+					"special":     {Type: cty.Bool, Optional: true},
+					"some_number": {Type: cty.Number, Optional: true},
 				},
 			},
 			ExpectedOutput: `  # test_instance.example will be updated in-place
   ~ resource "test_instance" "example" {
-      ~ ami = (sensitive)
-        id  = "i-02ae66f368e8518a9"
+      # Warning: this attribute value will no longer be marked as sensitive
+      # after applying this change
+      ~ ami         = (sensitive)
+        id          = "i-02ae66f368e8518a9"
+      # Warning: this attribute value will no longer be marked as sensitive
+      # after applying this change
+      ~ some_number = (sensitive)
+      # Warning: this attribute value will no longer be marked as sensitive
+      # after applying this change
+      ~ special     = (sensitive)
     }
 `,
 		},
@@ -3714,7 +3737,11 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
 			ExpectedOutput: `  # test_instance.example will be updated in-place
   ~ resource "test_instance" "example" {
         id   = "i-02ae66f368e8518a9"
-      ~ tags = (sensitive)
+      ~ tags = {
+          # Warning: this attribute value will be marked as sensitive and will
+          # not display in UI output after applying this change
+          ~ "name" = (sensitive)
+        }
     }
 `,
 		},
@@ -3777,7 +3804,7 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
 			},
 			ExpectedOutput: `  # test_instance.example will be destroyed
   - resource "test_instance" "example" {
-      - ami = (sensitive)
+      - ami = (sensitive) -> null
       - id  = "i-02ae66f368e8518a9" -> null
     }
 `,