Add special diff for nested blocks

When a value in a nested block is marked as sensitive,
it will result in the behavior of every member of
that block being sensitive. As such, show a
specialized diff that reduces noise of (sensitive)
for many attributes that we won't show. Also update
the warning to differentiate between showing a warning
for an attribute or warning for blocks.
This commit is contained in:
Pam Selle 2020-09-29 12:59:30 -04:00
parent 6617c2729c
commit 73b1d8b0d1
2 changed files with 163 additions and 32 deletions

View File

@ -338,7 +338,7 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At
p.buf.WriteString("\n") p.buf.WriteString("\n")
p.writeSensitivityWarning(old, new, indent, action) p.writeSensitivityWarning(old, new, indent, action, false)
p.buf.WriteString(strings.Repeat(" ", indent)) p.buf.WriteString(strings.Repeat(" ", indent))
p.writeActionSymbol(action) p.writeActionSymbol(action)
@ -377,6 +377,49 @@ func (p *blockBodyDiffPrinter) writeNestedBlockDiffs(name string, blockS *config
return skippedBlocks return skippedBlocks
} }
// If either the old or the new value is marked,
// Display a special diff because it is irrelevant
// to list all obfuscated attributes as (sensitive)
if old.IsMarked() || new.IsMarked() {
unmarkedOld, _ := old.Unmark()
unmarkedNew, _ := new.Unmark()
eqV := unmarkedNew.Equals(unmarkedOld)
var action plans.Action
switch {
case old.IsNull():
action = plans.Create
case new.IsNull():
action = plans.Delete
case !new.IsWhollyKnown() || !old.IsWhollyKnown():
// "old" should actually always be known due to our contract
// that old values must never be unknown, but we'll allow it
// anyway to be robust.
action = plans.Update
case !eqV.IsKnown() || !eqV.True():
action = plans.Update
}
if blankBefore {
p.buf.WriteRune('\n')
}
// New line before warning printing
p.buf.WriteRune('\n')
p.writeSensitivityWarning(old, new, indent, action, true)
p.buf.WriteString(strings.Repeat(" ", indent))
p.writeActionSymbol(action)
fmt.Fprintf(p.buf, "%s {", name)
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")
p.buf.WriteString(strings.Repeat(" ", indent+4))
p.buf.WriteString("# so its contents will not be displayed.")
p.buf.WriteRune('\n')
p.buf.WriteString(strings.Repeat(" ", indent+2))
p.buf.WriteString("}")
return 0
}
// Where old/new are collections representing a nesting mode other than // Where old/new are collections representing a nesting mode other than
// NestingSingle, we assume the collection value can never be unknown // NestingSingle, we assume the collection value can never be unknown
// since we always produce the container for the nested objects, even if // since we always produce the container for the nested objects, even if
@ -1129,7 +1172,7 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa
oldV := old.Index(kV) oldV := old.Index(kV)
newV := new.Index(kV) newV := new.Index(kV)
p.writeSensitivityWarning(oldV, newV, indent+2, action) p.writeSensitivityWarning(oldV, newV, indent+2, action, false)
p.buf.WriteString(strings.Repeat(" ", indent+2)) p.buf.WriteString(strings.Repeat(" ", indent+2))
p.writeActionSymbol(action) p.writeActionSymbol(action)
@ -1307,15 +1350,21 @@ func (p *blockBodyDiffPrinter) writeActionSymbol(action plans.Action) {
} }
} }
func (p *blockBodyDiffPrinter) writeSensitivityWarning(old, new cty.Value, indent int, action plans.Action) { func (p *blockBodyDiffPrinter) writeSensitivityWarning(old, new cty.Value, indent int, action plans.Action, isBlock bool) {
// Dont' show this warning for create or delete // Dont' show this warning for create or delete
if action == plans.Create || action == plans.Delete { if action == plans.Create || action == plans.Delete {
return return
} }
// Customize the warning based on if it is an attribute or block
diffType := "attribute value"
if isBlock {
diffType = "block"
}
if new.IsMarked() && !old.IsMarked() { if new.IsMarked() && !old.IsMarked() {
p.buf.WriteString(strings.Repeat(" ", indent)) p.buf.WriteString(strings.Repeat(" ", indent))
p.buf.WriteString(p.color.Color("# [yellow]Warning:[reset] this attribute value will be marked as sensitive and will\n")) p.buf.WriteString(p.color.Color(fmt.Sprintf("# [yellow]Warning:[reset] this %s will be marked as sensitive and will\n", diffType)))
p.buf.WriteString(strings.Repeat(" ", indent)) p.buf.WriteString(strings.Repeat(" ", indent))
p.buf.WriteString(p.color.Color("# not display in UI output after applying this change\n")) p.buf.WriteString(p.color.Color("# not display in UI output after applying this change\n"))
} }
@ -1323,7 +1372,7 @@ func (p *blockBodyDiffPrinter) writeSensitivityWarning(old, new cty.Value, inden
// Note if changing this attribute will change its sensitivity // Note if changing this attribute will change its sensitivity
if old.IsMarked() && !new.IsMarked() { if old.IsMarked() && !new.IsMarked() {
p.buf.WriteString(strings.Repeat(" ", indent)) p.buf.WriteString(strings.Repeat(" ", indent))
p.buf.WriteString(p.color.Color("# [yellow]Warning:[reset] this attribute value will no longer be marked as sensitive\n")) p.buf.WriteString(p.color.Color(fmt.Sprintf("# [yellow]Warning:[reset] this %s will no longer be marked as sensitive\n", diffType)))
p.buf.WriteString(strings.Repeat(" ", indent)) p.buf.WriteString(strings.Repeat(" ", indent))
p.buf.WriteString(p.color.Color("# after applying this change\n")) p.buf.WriteString(p.color.Color("# after applying this change\n"))
} }
@ -1373,15 +1422,10 @@ func ctyCollectionValues(val cty.Value) []cty.Value {
return nil return nil
} }
// Sets with marks mark the whole set as marked,
// so when we unmark it, apply those marks to all members
// of the set
val, marks := val.Unmark()
ret := make([]cty.Value, 0, val.LengthInt()) ret := make([]cty.Value, 0, val.LengthInt())
for it := val.ElementIterator(); it.Next(); { for it := val.ElementIterator(); it.Next(); {
_, value := it.Element() _, value := it.Element()
ret = append(ret, value.WithMarks(marks)) ret = append(ret, value)
} }
return ret return ret
} }

View File

@ -3644,7 +3644,7 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
cty.StringVal("friends"), cty.StringVal("friends"),
cty.StringVal("!"), cty.StringVal("!"),
}), }),
"nested_block": cty.ListVal([]cty.Value{ "nested_block_list": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{ cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("secretval"), "an_attr": cty.StringVal("secretval"),
"another": cty.StringVal("not secret"), "another": cty.StringVal("not secret"),
@ -3670,7 +3670,7 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
}, },
{ {
// Nested blocks/sets will mark the whole set/block as sensitive // Nested blocks/sets will mark the whole set/block as sensitive
Path: cty.Path{cty.GetAttrStep{Name: "nested_block"}}, Path: cty.Path{cty.GetAttrStep{Name: "nested_block_list"}},
Marks: cty.NewValueMarks("sensitive"), Marks: cty.NewValueMarks("sensitive"),
}, },
}, },
@ -3685,7 +3685,7 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"list_field": {Type: cty.List(cty.String), Optional: true}, "list_field": {Type: cty.List(cty.String), Optional: true},
}, },
BlockTypes: map[string]*configschema.NestedBlock{ BlockTypes: map[string]*configschema.NestedBlock{
"nested_block": { "nested_block_list": {
Block: configschema.Block{ Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{ Attributes: map[string]*configschema.Attribute{
"an_attr": {Type: cty.String, Optional: true}, "an_attr": {Type: cty.String, Optional: true},
@ -3711,9 +3711,9 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
} }
+ map_whole = (sensitive) + map_whole = (sensitive)
+ nested_block { + nested_block_list {
+ an_attr = (sensitive) # At least one attribute in this block is (or was) sensitive,
+ another = (sensitive) # so its contents will not be displayed.
} }
} }
`, `,
@ -3742,7 +3742,11 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"nested_block": cty.ListVal([]cty.Value{ "nested_block": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{ cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("secretval"), "an_attr": cty.StringVal("secretval"),
"another": cty.StringVal("not secret"), }),
}),
"nested_block_set": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("secretval"),
}), }),
}), }),
}), }),
@ -3767,7 +3771,11 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"nested_block": cty.ListVal([]cty.Value{ "nested_block": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{ cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("changed"), "an_attr": cty.StringVal("changed"),
"another": cty.StringVal("not secret"), }),
}),
"nested_block_set": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("changed"),
}), }),
}), }),
}), }),
@ -3800,6 +3808,10 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
Path: cty.Path{cty.GetAttrStep{Name: "nested_block"}}, Path: cty.Path{cty.GetAttrStep{Name: "nested_block"}},
Marks: cty.NewValueMarks("sensitive"), Marks: cty.NewValueMarks("sensitive"),
}, },
{
Path: cty.Path{cty.GetAttrStep{Name: "nested_block_set"}},
Marks: cty.NewValueMarks("sensitive"),
},
}, },
RequiredReplace: cty.NewPathSet(), RequiredReplace: cty.NewPathSet(),
Tainted: false, Tainted: false,
@ -3814,14 +3826,13 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"map_whole": {Type: cty.Map(cty.String), Optional: true}, "map_whole": {Type: cty.Map(cty.String), Optional: true},
}, },
BlockTypes: map[string]*configschema.NestedBlock{ BlockTypes: map[string]*configschema.NestedBlock{
"nested_block": { "nested_block_set": {
Block: configschema.Block{ Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{ Attributes: map[string]*configschema.Attribute{
"an_attr": {Type: cty.String, Optional: true}, "an_attr": {Type: cty.String, Optional: true},
"another": {Type: cty.String, Optional: true},
}, },
}, },
Nesting: configschema.NestingList, Nesting: configschema.NestingSet,
}, },
}, },
}, },
@ -3853,11 +3864,11 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
# after applying this change # after applying this change
~ special = (sensitive) ~ special = (sensitive)
~ nested_block { # Warning: this block will no longer be marked as sensitive
# Warning: this attribute value will no longer be marked as sensitive
# after applying this change # after applying this change
~ an_attr = (sensitive) ~ nested_block_set {
# (1 unchanged attribute hidden) # At least one attribute in this block is (or was) sensitive,
# so its contents will not be displayed.
} }
} }
`, `,
@ -3879,6 +3890,9 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"breakfast": cty.StringVal("pizza"), "breakfast": cty.StringVal("pizza"),
"dinner": cty.StringVal("pizza"), "dinner": cty.StringVal("pizza"),
}), }),
"nested_block_single": cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("original"),
}),
}), }),
After: cty.ObjectVal(map[string]cty.Value{ After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"), "id": cty.StringVal("i-02ae66f368e8518a9"),
@ -3894,6 +3908,9 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"breakfast": cty.StringVal("cereal"), "breakfast": cty.StringVal("cereal"),
"dinner": cty.StringVal("pizza"), "dinner": cty.StringVal("pizza"),
}), }),
"nested_block_single": cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("changed"),
}),
}), }),
AfterValMarks: []cty.PathValueMarks{ AfterValMarks: []cty.PathValueMarks{
{ {
@ -3912,6 +3929,13 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
Path: cty.Path{cty.GetAttrStep{Name: "map_whole"}}, Path: cty.Path{cty.GetAttrStep{Name: "map_whole"}},
Marks: cty.NewValueMarks("sensitive"), Marks: cty.NewValueMarks("sensitive"),
}, },
{
Path: cty.Path{cty.GetAttrStep{Name: "nested_block_single"}},
Marks: cty.NewValueMarks("sensitive"),
},
{
Marks: cty.NewValueMarks("sensitive"),
},
}, },
RequiredReplace: cty.NewPathSet(), RequiredReplace: cty.NewPathSet(),
Tainted: false, Tainted: false,
@ -3922,6 +3946,16 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"map_key": {Type: cty.Map(cty.Number), Optional: true}, "map_key": {Type: cty.Map(cty.Number), Optional: true},
"map_whole": {Type: cty.Map(cty.String), Optional: true}, "map_whole": {Type: cty.Map(cty.String), Optional: true},
}, },
BlockTypes: map[string]*configschema.NestedBlock{
"nested_block_single": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"an_attr": {Type: cty.String, Optional: true},
},
},
Nesting: configschema.NestingSingle,
},
},
}, },
ExpectedOutput: ` # test_instance.example will be updated in-place ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" { ~ resource "test_instance" "example" {
@ -3940,6 +3974,13 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
# Warning: this attribute value will be marked as sensitive and will # Warning: this attribute value will be marked as sensitive and will
# not display in UI output after applying this change # not display in UI output after applying this change
~ map_whole = (sensitive) ~ map_whole = (sensitive)
# Warning: this block will be marked as sensitive and will
# not display in UI output after applying this change
~ nested_block_single {
# At least one attribute in this block is (or was) sensitive,
# so its contents will not be displayed.
}
} }
`, `,
}, },
@ -3961,6 +4002,11 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"breakfast": cty.StringVal("pizza"), "breakfast": cty.StringVal("pizza"),
"dinner": cty.StringVal("pizza"), "dinner": cty.StringVal("pizza"),
}), }),
"nested_block_map": cty.MapVal(map[string]cty.Value{
"foo": cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("original"),
}),
}),
}), }),
After: cty.ObjectVal(map[string]cty.Value{ After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"), "id": cty.StringVal("i-02ae66f368e8518a9"),
@ -3977,6 +4023,11 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"breakfast": cty.StringVal("cereal"), "breakfast": cty.StringVal("cereal"),
"dinner": cty.StringVal("pizza"), "dinner": cty.StringVal("pizza"),
}), }),
"nested_block_map": cty.MapVal(map[string]cty.Value{
"foo": cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("changed"),
}),
}),
}), }),
BeforeValMarks: []cty.PathValueMarks{ BeforeValMarks: []cty.PathValueMarks{
{ {
@ -3995,6 +4046,13 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
Path: cty.Path{cty.GetAttrStep{Name: "map_whole"}}, Path: cty.Path{cty.GetAttrStep{Name: "map_whole"}},
Marks: cty.NewValueMarks("sensitive"), Marks: cty.NewValueMarks("sensitive"),
}, },
{
Path: cty.Path{cty.GetAttrStep{Name: "nested_block_map"}},
Marks: cty.NewValueMarks("sensitive"),
},
{
Marks: cty.NewValueMarks("sensitive"),
},
}, },
AfterValMarks: []cty.PathValueMarks{ AfterValMarks: []cty.PathValueMarks{
{ {
@ -4013,6 +4071,10 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
Path: cty.Path{cty.GetAttrStep{Name: "map_whole"}}, Path: cty.Path{cty.GetAttrStep{Name: "map_whole"}},
Marks: cty.NewValueMarks("sensitive"), Marks: cty.NewValueMarks("sensitive"),
}, },
{
Path: cty.Path{cty.GetAttrStep{Name: "nested_block_map"}},
Marks: cty.NewValueMarks("sensitive"),
},
}, },
RequiredReplace: cty.NewPathSet(), RequiredReplace: cty.NewPathSet(),
Tainted: false, Tainted: false,
@ -4024,6 +4086,16 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"map_key": {Type: cty.Map(cty.Number), Optional: true}, "map_key": {Type: cty.Map(cty.Number), Optional: true},
"map_whole": {Type: cty.Map(cty.String), Optional: true}, "map_whole": {Type: cty.Map(cty.String), Optional: true},
}, },
BlockTypes: map[string]*configschema.NestedBlock{
"nested_block_map": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"an_attr": {Type: cty.String, Optional: true},
},
},
Nesting: configschema.NestingMap,
},
},
}, },
ExpectedOutput: ` # test_instance.example will be updated in-place ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" { ~ resource "test_instance" "example" {
@ -4039,6 +4111,11 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
# (1 unchanged element hidden) # (1 unchanged element hidden)
} }
~ map_whole = (sensitive) ~ map_whole = (sensitive)
~ nested_block_map {
# At least one attribute in this block is (or was) sensitive,
# so its contents will not be displayed.
}
} }
`, `,
}, },
@ -4066,6 +4143,12 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"another": cty.StringVal("not secret"), "another": cty.StringVal("not secret"),
}), }),
}), }),
"nested_block_set": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("secret"),
"another": cty.StringVal("not secret"),
}),
}),
}), }),
After: cty.NullVal(cty.EmptyObject), After: cty.NullVal(cty.EmptyObject),
BeforeValMarks: []cty.PathValueMarks{ BeforeValMarks: []cty.PathValueMarks{
@ -4089,6 +4172,10 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
Path: cty.Path{cty.GetAttrStep{Name: "nested_block"}}, Path: cty.Path{cty.GetAttrStep{Name: "nested_block"}},
Marks: cty.NewValueMarks("sensitive"), Marks: cty.NewValueMarks("sensitive"),
}, },
{
Path: cty.Path{cty.GetAttrStep{Name: "nested_block_set"}},
Marks: cty.NewValueMarks("sensitive"),
},
}, },
RequiredReplace: cty.NewPathSet(), RequiredReplace: cty.NewPathSet(),
Tainted: false, Tainted: false,
@ -4101,14 +4188,14 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"map_whole": {Type: cty.Map(cty.String), Optional: true}, "map_whole": {Type: cty.Map(cty.String), Optional: true},
}, },
BlockTypes: map[string]*configschema.NestedBlock{ BlockTypes: map[string]*configschema.NestedBlock{
"nested_block": { "nested_block_set": {
Block: configschema.Block{ Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{ Attributes: map[string]*configschema.Attribute{
"an_attr": {Type: cty.String, Optional: true}, "an_attr": {Type: cty.String, Optional: true},
"another": {Type: cty.String, Optional: true}, "another": {Type: cty.String, Optional: true},
}, },
}, },
Nesting: configschema.NestingList, Nesting: configschema.NestingSet,
}, },
}, },
}, },
@ -4126,9 +4213,9 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
} -> null } -> null
- map_whole = (sensitive) -> null - map_whole = (sensitive) -> null
- nested_block { - nested_block_set {
- an_attr = (sensitive) -> null # At least one attribute in this block is (or was) sensitive,
- another = (sensitive) -> null # so its contents will not be displayed.
} }
} }
`, `,