Skip to content

Commit fac60ab

Browse files
mildwonkeymildwonkey
authored
Add formatting for attributes with nested types (hashicorp#28069)
* format/diff: extract attributes-writing logic to a function This is a stepping-stone commit (for easier reviewability, and to prove that tests did not change) as part of writing a NestedType-specific diff printer. * command/format: add support for formatting attributes with NestedTypes This commit adds custom formatting for NestedType attributes. THe logic was mostly copied from the block diff printer, with minor tweaks here and there. I used the (excellent) existing test coverage and added a NestedType attribute to every test. Since the (nested-block specific) test schemas were nearly identical, I added a function that returns the schema with the requested NestingMode.
1 parent 783936f commit fac60ab

2 files changed

Lines changed: 612 additions & 441 deletions

File tree

‎command/format/diff.go‎

Lines changed: 271 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -213,56 +213,10 @@ const forcesNewResourceCaption = " [red]# forces replacement[reset]"
213213
// and returns true if any differences were found and written
214214
func (p *blockBodyDiffPrinter) writeBlockBodyDiff(schema *configschema.Block, old, new cty.Value, indent int, path cty.Path) blockBodyDiffResult {
215215
path = ctyEnsurePathCapacity(path, 1)
216-
217216
result := blockBodyDiffResult{}
218217

219-
blankBeforeBlocks := false
220-
{
221-
attrNames := make([]string, 0, len(schema.Attributes))
222-
attrNameLen := 0
223-
for name := range schema.Attributes {
224-
oldVal := ctyGetAttrMaybeNull(old, name)
225-
newVal := ctyGetAttrMaybeNull(new, name)
226-
if oldVal.IsNull() && newVal.IsNull() {
227-
// Skip attributes where both old and new values are null
228-
// (we do this early here so that we'll do our value alignment
229-
// based on the longest attribute name that has a change, rather
230-
// than the longest attribute name in the full set.)
231-
continue
232-
}
233-
234-
attrNames = append(attrNames, name)
235-
if len(name) > attrNameLen {
236-
attrNameLen = len(name)
237-
}
238-
}
239-
sort.Strings(attrNames)
240-
if len(attrNames) > 0 {
241-
blankBeforeBlocks = true
242-
}
243-
244-
for _, name := range attrNames {
245-
attrS := schema.Attributes[name]
246-
oldVal := ctyGetAttrMaybeNull(old, name)
247-
newVal := ctyGetAttrMaybeNull(new, name)
248-
249-
result.bodyWritten = true
250-
skipped := p.writeAttrDiff(name, attrS, oldVal, newVal, attrNameLen, indent, path)
251-
if skipped {
252-
result.skippedAttributes++
253-
}
254-
}
255-
256-
if result.skippedAttributes > 0 {
257-
noun := "attributes"
258-
if result.skippedAttributes == 1 {
259-
noun = "attribute"
260-
}
261-
p.buf.WriteString("\n")
262-
p.buf.WriteString(strings.Repeat(" ", indent+2))
263-
p.buf.WriteString(p.color.Color(fmt.Sprintf("[dark_gray]# (%d unchanged %s hidden)[reset]", result.skippedAttributes, noun)))
264-
}
265-
}
218+
// write the attributes diff
219+
blankBeforeBlocks := p.writeAttrsDiff(schema.Attributes, old, new, indent, path, &result)
266220

267221
{
268222
blockTypeNames := make([]string, 0, len(schema.BlockTypes))
@@ -299,6 +253,63 @@ func (p *blockBodyDiffPrinter) writeBlockBodyDiff(schema *configschema.Block, ol
299253
return result
300254
}
301255

256+
func (p *blockBodyDiffPrinter) writeAttrsDiff(
257+
attrsS map[string]*configschema.Attribute,
258+
old, new cty.Value,
259+
indent int,
260+
path cty.Path,
261+
result *blockBodyDiffResult) bool {
262+
263+
blankBeforeBlocks := false
264+
265+
attrNames := make([]string, 0, len(attrsS))
266+
attrNameLen := 0
267+
for name := range attrsS {
268+
oldVal := ctyGetAttrMaybeNull(old, name)
269+
newVal := ctyGetAttrMaybeNull(new, name)
270+
if oldVal.IsNull() && newVal.IsNull() {
271+
// Skip attributes where both old and new values are null
272+
// (we do this early here so that we'll do our value alignment
273+
// based on the longest attribute name that has a change, rather
274+
// than the longest attribute name in the full set.)
275+
continue
276+
}
277+
278+
attrNames = append(attrNames, name)
279+
if len(name) > attrNameLen {
280+
attrNameLen = len(name)
281+
}
282+
}
283+
sort.Strings(attrNames)
284+
if len(attrNames) > 0 {
285+
blankBeforeBlocks = true
286+
}
287+
288+
for _, name := range attrNames {
289+
attrS := attrsS[name]
290+
oldVal := ctyGetAttrMaybeNull(old, name)
291+
newVal := ctyGetAttrMaybeNull(new, name)
292+
293+
result.bodyWritten = true
294+
skipped := p.writeAttrDiff(name, attrS, oldVal, newVal, attrNameLen, indent, path)
295+
if skipped {
296+
result.skippedAttributes++
297+
}
298+
}
299+
300+
if result.skippedAttributes > 0 {
301+
noun := "attributes"
302+
if result.skippedAttributes == 1 {
303+
noun = "attribute"
304+
}
305+
p.buf.WriteString("\n")
306+
p.buf.WriteString(strings.Repeat(" ", indent+2))
307+
p.buf.WriteString(p.color.Color(fmt.Sprintf("[dark_gray]# (%d unchanged %s hidden)[reset]", result.skippedAttributes, noun)))
308+
}
309+
310+
return blankBeforeBlocks
311+
}
312+
302313
// getPlanActionAndShow returns the action value
303314
// and a boolean for showJustNew. In this function we
304315
// modify the old and new values to remove any possible marks
@@ -328,17 +339,9 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At
328339
return true
329340
}
330341

331-
// TODO: There will need to be an object-specific diff printer that handles
332-
// things like individual attribute sensitivity and pathing into
333-
// requiredReplace, but for now we will let writeAttrDiff handle attributes
334-
// with NestedTypes like regular (object) attributes.
335-
//
336-
// To avoid printing any sensitive nested fields inside attributes (until
337-
// the above is implemented) we will treat the entire attribute as
338-
// sensitive.
339-
var sensitive bool
340-
if attrS.NestedType != nil && attrS.NestedType.ContainsSensitive() {
341-
sensitive = true
342+
if attrS.NestedType != nil {
343+
p.writeNestedAttrDiff(name, attrS.NestedType, old, new, nameLen, indent, path, action, showJustNew)
344+
return false
342345
}
343346

344347
p.buf.WriteString("\n")
@@ -354,7 +357,7 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At
354357
p.buf.WriteString(strings.Repeat(" ", nameLen-len(name)))
355358
p.buf.WriteString(" = ")
356359

357-
if attrS.Sensitive || sensitive {
360+
if attrS.Sensitive {
358361
p.buf.WriteString("(sensitive value)")
359362
} else {
360363
switch {
@@ -374,19 +377,214 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At
374377
return false
375378
}
376379

377-
// TODO: writeNestedAttrDiff will be responsible for properly formatting
378-
// Attributes with NestedTypes in the diff. This function will be called from
379-
// writeAttrDiff when it recieves attribute with a NestedType. Right now, we are
380-
// letting the existing formatter "just" print these attributes like regular,
381-
// object-type attributes. Unlike the regular attribute printer, this function
382-
// will need to descend into the NestedType to ensure that we are properly
383-
// handling items such as:
384-
// - nested sensitive fields
385-
// - which nested field specifically requires replacement
386-
//
387-
// Examples of both can be seen in diff_test.go with FIXME comments.
388-
func (p *blockBodyDiffPrinter) writeNestedAttrDiff(name string, attrS *configschema.Attribute, old, new cty.Value, nameLen, indent int, path cty.Path) bool {
389-
panic("not implemented")
380+
// writeNestedAttrDiff is responsible for formatting Attributes with NestedTypes
381+
// in the diff.
382+
func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
383+
name string, objS *configschema.Object, old, new cty.Value,
384+
nameLen, indent int, path cty.Path, action plans.Action, showJustNew bool) {
385+
386+
p.buf.WriteString("\n")
387+
p.buf.WriteString(strings.Repeat(" ", indent))
388+
p.writeActionSymbol(action)
389+
390+
p.buf.WriteString(p.color.Color("[bold]"))
391+
p.buf.WriteString(name)
392+
p.buf.WriteString(p.color.Color("[reset]"))
393+
p.buf.WriteString(strings.Repeat(" ", nameLen-len(name)))
394+
395+
result := &blockBodyDiffResult{}
396+
switch objS.Nesting {
397+
case configschema.NestingSingle:
398+
p.buf.WriteString(" = {")
399+
if action != plans.NoOp && (p.pathForcesNewResource(path) || p.pathForcesNewResource(path[:len(path)-1])) {
400+
p.buf.WriteString(p.color.Color(forcesNewResourceCaption))
401+
}
402+
p.writeAttrsDiff(objS.Attributes, old, new, indent+2, path, result)
403+
404+
if result.skippedAttributes > 0 {
405+
noun := "attributes"
406+
if result.skippedAttributes == 1 {
407+
noun = "attribute"
408+
}
409+
p.buf.WriteString("\n")
410+
p.buf.WriteString(strings.Repeat(" ", indent+2))
411+
p.buf.WriteString(p.color.Color(fmt.Sprintf("[dark_gray]# (%d unchanged %s hidden)[reset]", result.skippedAttributes, noun)))
412+
}
413+
414+
p.buf.WriteString("\n")
415+
p.buf.WriteString(strings.Repeat(" ", indent))
416+
p.buf.WriteString("}")
417+
418+
case configschema.NestingList:
419+
p.buf.WriteString(" = [")
420+
if action != plans.NoOp && (p.pathForcesNewResource(path) || p.pathForcesNewResource(path[:len(path)-1])) {
421+
p.buf.WriteString(p.color.Color(forcesNewResourceCaption))
422+
}
423+
p.buf.WriteString("\n")
424+
p.buf.WriteString(strings.Repeat(" ", indent+4))
425+
p.writeActionSymbol(action)
426+
p.buf.WriteString("{")
427+
428+
oldItems := ctyCollectionValues(old)
429+
newItems := ctyCollectionValues(new)
430+
// Here we intentionally preserve the index-based correspondance
431+
// between old and new, rather than trying to detect insertions
432+
// and removals in the list, because this more accurately reflects
433+
// how Terraform Core and providers will understand the change,
434+
// particularly when the nested block contains computed attributes
435+
// that will themselves maintain correspondance by index.
436+
437+
// commonLen is number of elements that exist in both lists, which
438+
// will be presented as updates (~). Any additional items in one
439+
// of the lists will be presented as either creates (+) or deletes (-)
440+
// depending on which list they belong to.
441+
var commonLen int
442+
switch {
443+
case len(oldItems) < len(newItems):
444+
commonLen = len(oldItems)
445+
default:
446+
commonLen = len(newItems)
447+
}
448+
for i := 0; i < commonLen; i++ {
449+
path := append(path, cty.IndexStep{Key: cty.NumberIntVal(int64(i))})
450+
oldItem := oldItems[i]
451+
newItem := newItems[i]
452+
if oldItem.RawEquals(newItem) {
453+
action = plans.NoOp
454+
}
455+
p.writeAttrsDiff(objS.Attributes, oldItem, newItem, indent+6, path, result)
456+
}
457+
for i := commonLen; i < len(oldItems); i++ {
458+
path := append(path, cty.IndexStep{Key: cty.NumberIntVal(int64(i))})
459+
oldItem := oldItems[i]
460+
newItem := cty.NullVal(oldItem.Type())
461+
p.writeAttrsDiff(objS.Attributes, oldItem, newItem, indent+6, path, result)
462+
}
463+
for i := commonLen; i < len(newItems); i++ {
464+
path := append(path, cty.IndexStep{Key: cty.NumberIntVal(int64(i))})
465+
newItem := newItems[i]
466+
oldItem := cty.NullVal(newItem.Type())
467+
p.writeAttrsDiff(objS.Attributes, oldItem, newItem, indent+6, path, result)
468+
}
469+
470+
p.buf.WriteString("\n")
471+
p.buf.WriteString(strings.Repeat(" ", indent+4))
472+
p.buf.WriteString("},\n")
473+
p.buf.WriteString(strings.Repeat(" ", indent+2))
474+
p.buf.WriteString("]")
475+
476+
case configschema.NestingSet:
477+
oldItems := ctyCollectionValues(old)
478+
newItems := ctyCollectionValues(new)
479+
480+
allItems := make([]cty.Value, 0, len(oldItems)+len(newItems))
481+
allItems = append(allItems, oldItems...)
482+
allItems = append(allItems, newItems...)
483+
all := cty.SetVal(allItems)
484+
485+
p.buf.WriteString(" = [")
486+
487+
for it := all.ElementIterator(); it.Next(); {
488+
_, val := it.Element()
489+
var action plans.Action
490+
var oldValue, newValue cty.Value
491+
switch {
492+
case !val.IsKnown():
493+
action = plans.Update
494+
newValue = val
495+
case !old.HasElement(val).True():
496+
action = plans.Create
497+
oldValue = cty.NullVal(val.Type())
498+
newValue = val
499+
case !new.HasElement(val).True():
500+
action = plans.Delete
501+
oldValue = val
502+
newValue = cty.NullVal(val.Type())
503+
default:
504+
action = plans.NoOp
505+
oldValue = val
506+
newValue = val
507+
}
508+
509+
p.buf.WriteString("\n")
510+
p.buf.WriteString(strings.Repeat(" ", indent+4))
511+
p.writeActionSymbol(action)
512+
p.buf.WriteString("{")
513+
514+
if action != plans.NoOp && (p.pathForcesNewResource(path) || p.pathForcesNewResource(path[:len(path)-1])) {
515+
p.buf.WriteString(p.color.Color(forcesNewResourceCaption))
516+
}
517+
518+
path := append(path, cty.IndexStep{Key: val})
519+
p.writeAttrsDiff(objS.Attributes, oldValue, newValue, indent+6, path, result)
520+
521+
p.buf.WriteString("\n")
522+
p.buf.WriteString(strings.Repeat(" ", indent+4))
523+
p.buf.WriteString("},")
524+
}
525+
p.buf.WriteString("\n")
526+
p.buf.WriteString(strings.Repeat(" ", indent+2))
527+
p.buf.WriteString("]")
528+
529+
case configschema.NestingMap:
530+
oldItems := old.AsValueMap()
531+
newItems := new.AsValueMap()
532+
533+
allKeys := make(map[string]bool)
534+
for k := range oldItems {
535+
allKeys[k] = true
536+
}
537+
for k := range newItems {
538+
allKeys[k] = true
539+
}
540+
allKeysOrder := make([]string, 0, len(allKeys))
541+
for k := range allKeys {
542+
allKeysOrder = append(allKeysOrder, k)
543+
}
544+
sort.Strings(allKeysOrder)
545+
546+
p.buf.WriteString(" = {")
547+
548+
for _, k := range allKeysOrder {
549+
var action plans.Action
550+
oldValue := oldItems[k]
551+
newValue := newItems[k]
552+
switch {
553+
case oldValue == cty.NilVal:
554+
oldValue = cty.NullVal(newValue.Type())
555+
action = plans.Create
556+
case newValue == cty.NilVal:
557+
newValue = cty.NullVal(oldValue.Type())
558+
action = plans.Delete
559+
case !newValue.RawEquals(oldValue):
560+
action = plans.Update
561+
default:
562+
action = plans.NoOp
563+
}
564+
565+
p.buf.WriteString("\n")
566+
p.buf.WriteString(strings.Repeat(" ", indent+4))
567+
p.writeActionSymbol(action)
568+
569+
fmt.Fprintf(p.buf, "%q = {", k)
570+
if action != plans.NoOp && (p.pathForcesNewResource(path) || p.pathForcesNewResource(path[:len(path)-1])) {
571+
p.buf.WriteString(p.color.Color(forcesNewResourceCaption))
572+
}
573+
574+
path := append(path, cty.IndexStep{Key: cty.StringVal(k)})
575+
576+
p.writeAttrsDiff(objS.Attributes, oldValue, newValue, indent+6, path, result)
577+
p.buf.WriteString("\n")
578+
p.buf.WriteString(strings.Repeat(" ", indent+4))
579+
p.buf.WriteString("},")
580+
}
581+
582+
p.buf.WriteString("\n")
583+
p.buf.WriteString(strings.Repeat(" ", indent+2))
584+
p.buf.WriteString("}")
585+
}
586+
587+
return
390588
}
391589

392590
func (p *blockBodyDiffPrinter) writeNestedBlockDiffs(name string, blockS *configschema.NestedBlock, old, new cty.Value, blankBefore bool, indent int, path cty.Path) int {

0 commit comments

Comments
 (0)