command: Show diffs when only sensitivity changes

When an attribute's sensitivity changes, but its value remains the same,
we consider this an update operation for the plan. This commit updates
the diff renderer to match this, detecting and displaying the change in
sensitivity.

Previously, the renderer would detect no changes to the value of the
attribute, and consider it a no-op action. This resulted in suppression
of the attribute when the plan is in concise mode.

This is achieved with a new helper function, ctyEqualValueAndMarks. We
call this function whenever we want to check that two values are equal
in order to determine whether the action is update or no-op.
This commit is contained in:
Alisdair McDiarmid 2020-10-13 13:55:16 -04:00
parent fcae49611c
commit c798dc98db
2 changed files with 186 additions and 17 deletions

View File

@ -307,12 +307,6 @@ func (p *blockBodyDiffPrinter) writeBlockBodyDiff(schema *configschema.Block, ol
func getPlanActionAndShow(old cty.Value, new cty.Value) (plans.Action, bool) { func getPlanActionAndShow(old cty.Value, new cty.Value) (plans.Action, bool) {
var action plans.Action var action plans.Action
showJustNew := false showJustNew := false
if old.ContainsMarked() {
old, _ = old.UnmarkDeep()
}
if new.ContainsMarked() {
new, _ = new.UnmarkDeep()
}
switch { switch {
case old.IsNull(): case old.IsNull():
action = plans.Create action = plans.Create
@ -590,9 +584,6 @@ func (p *blockBodyDiffPrinter) writeNestedBlockDiffs(name string, blockS *config
} }
func (p *blockBodyDiffPrinter) writeSensitiveNestedBlockDiff(name string, old, new cty.Value, indent int, blankBefore bool, path cty.Path) { 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)
var action plans.Action var action plans.Action
switch { switch {
case old.IsNull(): case old.IsNull():
@ -604,7 +595,7 @@ func (p *blockBodyDiffPrinter) writeSensitiveNestedBlockDiff(name string, old, n
// that old values must never be unknown, but we'll allow it // that old values must never be unknown, but we'll allow it
// anyway to be robust. // anyway to be robust.
action = plans.Update action = plans.Update
case !eqV.IsKnown() || !eqV.True(): case !ctyEqualValueAndMarks(old, new):
action = plans.Update action = plans.Update
} }
@ -1162,12 +1153,8 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa
action = plans.Delete action = plans.Delete
} }
// Use unmarked values for equality testing
if old.HasIndex(kV).True() && new.HasIndex(kV).True() { if old.HasIndex(kV).True() && new.HasIndex(kV).True() {
unmarkedOld, _ := old.Index(kV).Unmark() if ctyEqualValueAndMarks(old.Index(kV), new.Index(kV)) {
unmarkedNew, _ := new.Index(kV).Unmark()
eqV := unmarkedOld.Equals(unmarkedNew)
if eqV.IsKnown() && eqV.True() {
action = plans.NoOp action = plans.NoOp
} else { } else {
action = plans.Update action = plans.Update
@ -1269,7 +1256,7 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa
action = plans.Create action = plans.Create
} else if !new.Type().HasAttribute(kV) { } else if !new.Type().HasAttribute(kV) {
action = plans.Delete action = plans.Delete
} else if eqV := old.GetAttr(kV).Equals(new.GetAttr(kV)); eqV.IsKnown() && eqV.True() { } else if ctyEqualValueAndMarks(old.GetAttr(kV), new.GetAttr(kV)) {
action = plans.NoOp action = plans.NoOp
} else { } else {
action = plans.Update action = plans.Update
@ -1493,11 +1480,23 @@ func ctySequenceDiff(old, new []cty.Value) []*plans.Change {
return ret return ret
} }
// ctyEqualValueAndMarks checks equality of two possibly-marked values,
// considering partially-unknown values and equal values with different marks
// as inequal
func ctyEqualWithUnknown(old, new cty.Value) bool { func ctyEqualWithUnknown(old, new cty.Value) bool {
if !old.IsWhollyKnown() || !new.IsWhollyKnown() { if !old.IsWhollyKnown() || !new.IsWhollyKnown() {
return false return false
} }
return old.Equals(new).True() return ctyEqualValueAndMarks(old, new)
}
// ctyEqualValueAndMarks checks equality of two possibly-marked values,
// considering equal values with different marks as inequal
func ctyEqualValueAndMarks(old, new cty.Value) bool {
oldUnmarked, oldMarks := old.UnmarkDeep()
newUnmarked, newMarks := new.UnmarkDeep()
sameValue := oldUnmarked.Equals(newUnmarked)
return sameValue.IsKnown() && sameValue.True() && oldMarks.Equal(newMarks)
} }
// ctyTypesEqual checks equality of two types more loosely // ctyTypesEqual checks equality of two types more loosely

View File

@ -4150,6 +4150,176 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
# so its contents will not be displayed. # so its contents will not be displayed.
} }
} }
`,
},
"in-place update - value unchanged, sensitivity changes": {
Action: plans.Update,
Mode: addrs.ManagedResourceMode,
Before: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-BEFORE"),
"special": cty.BoolVal(true),
"some_number": cty.NumberIntVal(1),
"list_field": cty.ListVal([]cty.Value{
cty.StringVal("hello"),
cty.StringVal("friends"),
cty.StringVal("!"),
}),
"map_key": cty.MapVal(map[string]cty.Value{
"breakfast": cty.NumberIntVal(800),
"dinner": cty.NumberIntVal(2000), // sensitive key
}),
"map_whole": cty.MapVal(map[string]cty.Value{
"breakfast": cty.StringVal("pizza"),
"dinner": cty.StringVal("pizza"),
}),
"nested_block": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("secretval"),
}),
}),
"nested_block_set": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("secretval"),
}),
}),
}),
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-BEFORE"),
"special": cty.BoolVal(true),
"some_number": cty.NumberIntVal(1),
"list_field": cty.ListVal([]cty.Value{
cty.StringVal("hello"),
cty.StringVal("friends"),
cty.StringVal("!"),
}),
"map_key": cty.MapVal(map[string]cty.Value{
"breakfast": cty.NumberIntVal(800),
"dinner": cty.NumberIntVal(2000), // sensitive key
}),
"map_whole": cty.MapVal(map[string]cty.Value{
"breakfast": cty.StringVal("pizza"),
"dinner": cty.StringVal("pizza"),
}),
"nested_block": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("secretval"),
}),
}),
"nested_block_set": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("secretval"),
}),
}),
}),
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"),
},
{
Path: cty.Path{cty.GetAttrStep{Name: "list_field"}, cty.IndexStep{Key: cty.NumberIntVal(2)}},
Marks: cty.NewValueMarks("sensitive"),
},
{
Path: cty.Path{cty.GetAttrStep{Name: "map_key"}, cty.IndexStep{Key: cty.StringVal("dinner")}},
Marks: cty.NewValueMarks("sensitive"),
},
{
Path: cty.Path{cty.GetAttrStep{Name: "map_whole"}},
Marks: cty.NewValueMarks("sensitive"),
},
{
Path: cty.Path{cty.GetAttrStep{Name: "nested_block"}},
Marks: cty.NewValueMarks("sensitive"),
},
{
Path: cty.Path{cty.GetAttrStep{Name: "nested_block_set"}},
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},
"list_field": {Type: cty.List(cty.String), Optional: true},
"special": {Type: cty.Bool, Optional: true},
"some_number": {Type: cty.Number, Optional: true},
"map_key": {Type: cty.Map(cty.Number), Optional: true},
"map_whole": {Type: cty.Map(cty.String), Optional: true},
},
BlockTypes: map[string]*configschema.NestedBlock{
"nested_block": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"an_attr": {Type: cty.String, Optional: true},
},
},
Nesting: configschema.NestingList,
},
"nested_block_set": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"an_attr": {Type: cty.String, Optional: true},
},
},
Nesting: configschema.NestingSet,
},
},
},
ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" {
# Warning: this attribute value will no longer be marked as sensitive
# after applying this change
~ ami = (sensitive)
id = "i-02ae66f368e8518a9"
~ list_field = [
# (1 unchanged element hidden)
"friends",
- (sensitive),
+ "!",
]
~ map_key = {
# Warning: this attribute value will no longer be marked as sensitive
# after applying this change
~ "dinner" = (sensitive)
# (1 unchanged element hidden)
}
# Warning: this attribute value will no longer be marked as sensitive
# after applying this change
~ map_whole = (sensitive)
# 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)
# Warning: this block will no longer be marked as sensitive
# after applying this change
~ nested_block {
# At least one attribute in this block is (or was) sensitive,
# so its contents will not be displayed.
}
# Warning: this block will no longer be marked as sensitive
# after applying this change
~ nested_block_set {
# At least one attribute in this block is (or was) sensitive,
# so its contents will not be displayed.
}
}
`, `,
}, },
"deletion": { "deletion": {