From 79a3e33c4d98240ebdea3edabff3be78ee73bd6d Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Tue, 6 Oct 2020 13:05:09 -0400 Subject: [PATCH 1/2] command: Fix missing force new for sensitive vars If a value rendered for the diff is sensitive and results in replacement of the resource, we should render the standard "forces replacement" text after the "(sensitive)" value display. --- command/format/diff.go | 3 +++ command/format/diff_test.go | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/command/format/diff.go b/command/format/diff.go index f8d7c56568..84527f6be3 100644 --- a/command/format/diff.go +++ b/command/format/diff.go @@ -827,6 +827,9 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa if old.IsKnown() && new.IsKnown() && !old.IsNull() && !new.IsNull() && typesEqual { if old.IsMarked() || new.IsMarked() { p.buf.WriteString("(sensitive)") + if p.pathForcesNewResource(path) { + p.buf.WriteString(p.color.Color(forcesNewResourceCaption)) + } return } diff --git a/command/format/diff_test.go b/command/format/diff_test.go index 8e31ce355a..63047e4650 100644 --- a/command/format/diff_test.go +++ b/command/format/diff_test.go @@ -4251,6 +4251,46 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { # so its contents will not be displayed. } } +`, + }, + "update with sensitive value forcing replacement": { + Action: plans.DeleteThenCreate, + Mode: addrs.ManagedResourceMode, + Before: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "ami": cty.StringVal("ami-BEFORE"), + }), + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("i-02ae66f368e8518a9"), + "ami": cty.StringVal("ami-AFTER"), + }), + BeforeValMarks: []cty.PathValueMarks{ + { + Path: cty.Path{cty.GetAttrStep{Name: "ami"}}, + Marks: cty.NewValueMarks("sensitive"), + }, + }, + AfterValMarks: []cty.PathValueMarks{ + { + Path: cty.Path{cty.GetAttrStep{Name: "ami"}}, + Marks: cty.NewValueMarks("sensitive"), + }, + }, + Schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Optional: true, Computed: true}, + "ami": {Type: cty.String, Optional: true}, + }, + }, + RequiredReplace: cty.NewPathSet(cty.Path{ + cty.GetAttrStep{Name: "ami"}, + }), + Tainted: false, + ExpectedOutput: ` # test_instance.example must be replaced +-/+ resource "test_instance" "example" { + ~ ami = (sensitive) # forces replacement + id = "i-02ae66f368e8518a9" + } `, }, } From 62e6f56a50c5503226392f73fa6e4291ce35457d Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Wed, 7 Oct 2020 10:50:54 -0400 Subject: [PATCH 2/2] command: Fix missing force new for sensitive blocks If an entire block is marked sensitive (possibly because it is of type NestedSet) and results in replacement of the resource, we should render the standard "forces replacement" text after the opening line of the block. --- command/format/diff.go | 7 ++++-- command/format/diff_test.go | 44 ++++++++++++++++++++++++++++++++----- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/command/format/diff.go b/command/format/diff.go index 84527f6be3..a8e37a31ce 100644 --- a/command/format/diff.go +++ b/command/format/diff.go @@ -381,7 +381,7 @@ func (p *blockBodyDiffPrinter) writeNestedBlockDiffs(name string, blockS *config // Display a special diff because it is irrelevant // to list all obfuscated attributes as (sensitive) if old.IsMarked() || new.IsMarked() { - p.writeSensitiveNestedBlockDiff(name, old, new, indent, blankBefore) + p.writeSensitiveNestedBlockDiff(name, old, new, indent, blankBefore, path) return 0 } @@ -589,7 +589,7 @@ func (p *blockBodyDiffPrinter) writeNestedBlockDiffs(name string, blockS *config return skippedBlocks } -func (p *blockBodyDiffPrinter) writeSensitiveNestedBlockDiff(name string, old, new cty.Value, indent int, blankBefore bool) { +func (p *blockBodyDiffPrinter) writeSensitiveNestedBlockDiff(name string, old, new cty.Value, indent int, blankBefore bool, path cty.Path) { unmarkedOld, _ := old.Unmark() unmarkedNew, _ := new.Unmark() eqV := unmarkedNew.Equals(unmarkedOld) @@ -618,6 +618,9 @@ func (p *blockBodyDiffPrinter) writeSensitiveNestedBlockDiff(name string, old, n p.buf.WriteString(strings.Repeat(" ", indent)) p.writeActionSymbol(action) fmt.Fprintf(p.buf, "%s {", name) + if action != plans.NoOp && p.pathForcesNewResource(path) { + p.buf.WriteString(p.color.Color(forcesNewResourceCaption)) + } p.buf.WriteRune('\n') p.buf.WriteString(strings.Repeat(" ", indent+4)) p.buf.WriteString("# At least one attribute in this block is (or was) sensitive,\n") diff --git a/command/format/diff_test.go b/command/format/diff_test.go index 63047e4650..ec08e0079d 100644 --- a/command/format/diff_test.go +++ b/command/format/diff_test.go @@ -4259,20 +4259,38 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { Before: cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("i-02ae66f368e8518a9"), "ami": cty.StringVal("ami-BEFORE"), + "nested_block_set": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "an_attr": cty.StringVal("secret"), + }), + }), }), After: cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("i-02ae66f368e8518a9"), "ami": cty.StringVal("ami-AFTER"), + "nested_block_set": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "an_attr": cty.StringVal("changed"), + }), + }), }), BeforeValMarks: []cty.PathValueMarks{ { - Path: cty.Path{cty.GetAttrStep{Name: "ami"}}, + Path: cty.GetAttrPath("ami"), + Marks: cty.NewValueMarks("sensitive"), + }, + { + Path: cty.GetAttrPath("nested_block_set"), Marks: cty.NewValueMarks("sensitive"), }, }, AfterValMarks: []cty.PathValueMarks{ { - Path: cty.Path{cty.GetAttrStep{Name: "ami"}}, + Path: cty.GetAttrPath("ami"), + Marks: cty.NewValueMarks("sensitive"), + }, + { + Path: cty.GetAttrPath("nested_block_set"), Marks: cty.NewValueMarks("sensitive"), }, }, @@ -4281,15 +4299,31 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { "id": {Type: cty.String, Optional: true, Computed: true}, "ami": {Type: cty.String, Optional: true}, }, + BlockTypes: map[string]*configschema.NestedBlock{ + "nested_block_set": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "an_attr": {Type: cty.String, Required: true}, + }, + }, + Nesting: configschema.NestingSet, + }, + }, }, - RequiredReplace: cty.NewPathSet(cty.Path{ - cty.GetAttrStep{Name: "ami"}, - }), + RequiredReplace: cty.NewPathSet( + cty.GetAttrPath("ami"), + cty.GetAttrPath("nested_block_set"), + ), Tainted: false, ExpectedOutput: ` # test_instance.example must be replaced -/+ resource "test_instance" "example" { ~ ami = (sensitive) # forces replacement id = "i-02ae66f368e8518a9" + + ~ nested_block_set { # forces replacement + # At least one attribute in this block is (or was) sensitive, + # so its contents will not be displayed. + } } `, },