command/format: fix repetitive "unchanged attribute hidden" message (#28589)

writeNestedAttrDiff and writeAttrDiff were both printing the "unchanged attribute" message.  This removes one of the redundant prints.

Fixing this led me (in a very roundabout way) to realize that NestedType attributes were printing a sum total of unchanged attributes, including those in entirely unchanged elements, while *not* printing the total of unchanged elements. I added the necessary logic to count and print the number of unchanged elements for maps and lists.
This commit is contained in:
Kristin Laemmert 2021-05-04 10:23:50 -04:00 committed by GitHub
parent 5813620412
commit 3679de0630
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 111 additions and 42 deletions

View File

@ -219,6 +219,7 @@ func (p *blockBodyDiffPrinter) writeBlockBodyDiff(schema *configschema.Block, ol
// write the attributes diff // write the attributes diff
blankBeforeBlocks := p.writeAttrsDiff(schema.Attributes, old, new, indent, path, &result) blankBeforeBlocks := p.writeAttrsDiff(schema.Attributes, old, new, indent, path, &result)
p.writeSkippedAttr(result.skippedAttributes, indent+2)
{ {
blockTypeNames := make([]string, 0, len(schema.BlockTypes)) blockTypeNames := make([]string, 0, len(schema.BlockTypes))
@ -299,16 +300,6 @@ func (p *blockBodyDiffPrinter) writeAttrsDiff(
} }
} }
if result.skippedAttributes > 0 {
noun := "attributes"
if result.skippedAttributes == 1 {
noun = "attribute"
}
p.buf.WriteString("\n")
p.buf.WriteString(strings.Repeat(" ", indent+2))
p.buf.WriteString(p.color.Color(fmt.Sprintf("[dark_gray]# (%d unchanged %s hidden)[reset]", result.skippedAttributes, noun)))
}
return blankBeforeBlocks return blankBeforeBlocks
} }
@ -405,17 +396,7 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
p.buf.WriteString(p.color.Color(forcesNewResourceCaption)) p.buf.WriteString(p.color.Color(forcesNewResourceCaption))
} }
p.writeAttrsDiff(objS.Attributes, old, new, indent+2, path, result) p.writeAttrsDiff(objS.Attributes, old, new, indent+2, path, result)
p.writeSkippedAttr(result.skippedAttributes, indent+4)
if result.skippedAttributes > 0 {
noun := "attributes"
if result.skippedAttributes == 1 {
noun = "attribute"
}
p.buf.WriteString("\n")
p.buf.WriteString(strings.Repeat(" ", indent+2))
p.buf.WriteString(p.color.Color(fmt.Sprintf("[dark_gray]# (%d unchanged %s hidden)[reset]", result.skippedAttributes, noun)))
}
p.buf.WriteString("\n") p.buf.WriteString("\n")
p.buf.WriteString(strings.Repeat(" ", indent)) p.buf.WriteString(strings.Repeat(" ", indent))
p.buf.WriteString("}") p.buf.WriteString("}")
@ -444,6 +425,9 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
// of the lists will be presented as either creates (+) or deletes (-) // of the lists will be presented as either creates (+) or deletes (-)
// depending on which list they belong to. // depending on which list they belong to.
var commonLen int var commonLen int
// unchanged is the number of unchanged elements
var unchanged int
switch { switch {
case len(oldItems) < len(newItems): case len(oldItems) < len(newItems):
commonLen = len(oldItems) commonLen = len(oldItems)
@ -456,25 +440,31 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
newItem := newItems[i] newItem := newItems[i]
if oldItem.RawEquals(newItem) { if oldItem.RawEquals(newItem) {
action = plans.NoOp action = plans.NoOp
unchanged++
} }
if action != plans.NoOp {
p.writeAttrsDiff(objS.Attributes, oldItem, newItem, indent+6, path, result) p.writeAttrsDiff(objS.Attributes, oldItem, newItem, indent+6, path, result)
p.writeSkippedAttr(result.skippedAttributes, indent+8)
p.buf.WriteString("\n")
}
} }
for i := commonLen; i < len(oldItems); i++ { for i := commonLen; i < len(oldItems); i++ {
path := append(path, cty.IndexStep{Key: cty.NumberIntVal(int64(i))}) path := append(path, cty.IndexStep{Key: cty.NumberIntVal(int64(i))})
oldItem := oldItems[i] oldItem := oldItems[i]
newItem := cty.NullVal(oldItem.Type()) newItem := cty.NullVal(oldItem.Type())
p.writeAttrsDiff(objS.Attributes, oldItem, newItem, indent+6, path, result) p.writeAttrsDiff(objS.Attributes, oldItem, newItem, indent+6, path, result)
p.buf.WriteString("\n")
} }
for i := commonLen; i < len(newItems); i++ { for i := commonLen; i < len(newItems); i++ {
path := append(path, cty.IndexStep{Key: cty.NumberIntVal(int64(i))}) path := append(path, cty.IndexStep{Key: cty.NumberIntVal(int64(i))})
newItem := newItems[i] newItem := newItems[i]
oldItem := cty.NullVal(newItem.Type()) oldItem := cty.NullVal(newItem.Type())
p.writeAttrsDiff(objS.Attributes, oldItem, newItem, indent+6, path, result) p.writeAttrsDiff(objS.Attributes, oldItem, newItem, indent+6, path, result)
}
p.buf.WriteString("\n") p.buf.WriteString("\n")
}
p.buf.WriteString(strings.Repeat(" ", indent+4)) p.buf.WriteString(strings.Repeat(" ", indent+4))
p.buf.WriteString("},\n") p.buf.WriteString("},\n")
p.writeSkippedElems(unchanged, indent+4)
p.buf.WriteString(strings.Repeat(" ", indent+2)) p.buf.WriteString(strings.Repeat(" ", indent+2))
p.buf.WriteString("]") p.buf.WriteString("]")
@ -548,8 +538,10 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
} }
sort.Strings(allKeysOrder) sort.Strings(allKeysOrder)
p.buf.WriteString(" = {") p.buf.WriteString(" = {\n")
// unchanged tracks the number of unchanged elements
unchanged := 0
for _, k := range allKeysOrder { for _, k := range allKeysOrder {
var action plans.Action var action plans.Action
oldValue := oldItems[k] oldValue := oldItems[k]
@ -565,26 +557,27 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
action = plans.Update action = plans.Update
default: default:
action = plans.NoOp action = plans.NoOp
unchanged++
} }
p.buf.WriteString("\n") if action != plans.NoOp {
p.buf.WriteString(strings.Repeat(" ", indent+4)) p.buf.WriteString(strings.Repeat(" ", indent+4))
p.writeActionSymbol(action) p.writeActionSymbol(action)
fmt.Fprintf(p.buf, "%q = {", k) fmt.Fprintf(p.buf, "%q = {", k)
if action != plans.NoOp && (p.pathForcesNewResource(path) || p.pathForcesNewResource(path[:len(path)-1])) { if p.pathForcesNewResource(path) || p.pathForcesNewResource(path[:len(path)-1]) {
p.buf.WriteString(p.color.Color(forcesNewResourceCaption)) p.buf.WriteString(p.color.Color(forcesNewResourceCaption))
} }
path := append(path, cty.IndexStep{Key: cty.StringVal(k)}) path := append(path, cty.IndexStep{Key: cty.StringVal(k)})
p.writeAttrsDiff(objS.Attributes, oldValue, newValue, indent+6, path, result) p.writeAttrsDiff(objS.Attributes, oldValue, newValue, indent+6, path, result)
p.writeSkippedAttr(result.skippedAttributes, indent+8)
p.buf.WriteString("\n") p.buf.WriteString("\n")
p.buf.WriteString(strings.Repeat(" ", indent+4)) p.buf.WriteString(strings.Repeat(" ", indent+4))
p.buf.WriteString("},") p.buf.WriteString("},\n")
}
} }
p.buf.WriteString("\n") p.writeSkippedElems(unchanged, indent+4)
p.buf.WriteString(strings.Repeat(" ", indent+2)) p.buf.WriteString(strings.Repeat(" ", indent+2))
p.buf.WriteString("}") p.buf.WriteString("}")
} }
@ -1836,3 +1829,27 @@ func DiffActionSymbol(action plans.Action) string {
func identifyingAttribute(name string, attrSchema *configschema.Attribute) bool { func identifyingAttribute(name string, attrSchema *configschema.Attribute) bool {
return name == "id" || name == "tags" || name == "name" return name == "id" || name == "tags" || name == "name"
} }
func (p *blockBodyDiffPrinter) writeSkippedAttr(skipped, indent int) {
if skipped > 0 {
noun := "attributes"
if skipped == 1 {
noun = "attribute"
}
p.buf.WriteString("\n")
p.buf.WriteString(strings.Repeat(" ", indent))
p.buf.WriteString(p.color.Color(fmt.Sprintf("[dark_gray]# (%d unchanged %s hidden)[reset]", skipped, noun)))
}
}
func (p *blockBodyDiffPrinter) writeSkippedElems(skipped, indent int) {
if skipped > 0 {
noun := "elements"
if skipped == 1 {
noun = "element"
}
p.buf.WriteString(strings.Repeat(" ", indent))
p.buf.WriteString(p.color.Color(fmt.Sprintf("[dark_gray]# (%d unchanged %s hidden)[reset]", skipped, noun)))
p.buf.WriteString("\n")
}
}

View File

@ -2309,6 +2309,10 @@ func TestResourceChange_nestedList(t *testing.T) {
"mount_point": cty.StringVal("/var/diska"), "mount_point": cty.StringVal("/var/diska"),
"size": cty.NullVal(cty.String), "size": cty.NullVal(cty.String),
}), }),
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diskb"),
"size": cty.StringVal("50GB"),
}),
}), }),
"root_block_device": cty.ListVal([]cty.Value{ "root_block_device": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{ cty.ObjectVal(map[string]cty.Value{
@ -2325,6 +2329,10 @@ func TestResourceChange_nestedList(t *testing.T) {
"mount_point": cty.StringVal("/var/diska"), "mount_point": cty.StringVal("/var/diska"),
"size": cty.StringVal("50GB"), "size": cty.StringVal("50GB"),
}), }),
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diskb"),
"size": cty.StringVal("50GB"),
}),
}), }),
"root_block_device": cty.ListVal([]cty.Value{ "root_block_device": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{ cty.ObjectVal(map[string]cty.Value{
@ -2343,6 +2351,7 @@ func TestResourceChange_nestedList(t *testing.T) {
+ size = "50GB" + size = "50GB"
# (1 unchanged attribute hidden) # (1 unchanged attribute hidden)
}, },
# (1 unchanged element hidden)
] ]
id = "i-02ae66f368e8518a9" id = "i-02ae66f368e8518a9"
@ -2992,9 +3001,7 @@ func TestResourceChange_nestedMap(t *testing.T) {
+ mount_point = "/var/disk2" + mount_point = "/var/disk2"
+ size = "50GB" + size = "50GB"
}, },
"disk_a" = { # (1 unchanged element hidden)
# (2 unchanged attributes hidden)
},
} }
id = "i-02ae66f368e8518a9" id = "i-02ae66f368e8518a9"
@ -4019,6 +4026,51 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
~ ami = (sensitive value) # forces replacement ~ ami = (sensitive value) # forces replacement
id = "i-02ae66f368e8518a9" id = "i-02ae66f368e8518a9"
} }
`,
},
"update with sensitive nested type attribute forcing replacement": {
Action: plans.DeleteThenCreate,
Mode: addrs.ManagedResourceMode,
Before: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"conn_info": cty.ObjectVal(map[string]cty.Value{
"user": cty.StringVal("not-secret"),
"password": cty.StringVal("top-secret"),
}),
}),
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"conn_info": cty.ObjectVal(map[string]cty.Value{
"user": cty.StringVal("not-secret"),
"password": cty.StringVal("new-secret"),
}),
}),
Schema: &configschema.Block{
Attributes: map[string]*configschema.Attribute{
"id": {Type: cty.String, Optional: true, Computed: true},
"conn_info": {
NestedType: &configschema.Object{
Nesting: configschema.NestingSingle,
Attributes: map[string]*configschema.Attribute{
"user": {Type: cty.String, Optional: true},
"password": {Type: cty.String, Optional: true, Sensitive: true},
},
},
},
},
},
RequiredReplace: cty.NewPathSet(
cty.GetAttrPath("conn_info"),
cty.GetAttrPath("password"),
),
ExpectedOutput: ` # test_instance.example must be replaced
-/+ resource "test_instance" "example" {
~ conn_info = { # forces replacement
~ password = (sensitive value)
# (1 unchanged attribute hidden)
}
id = "i-02ae66f368e8518a9"
}
`, `,
}, },
} }