Skip to content

Commit 68ed506

Browse files
jbardinjbardin
committed
handle null and unknown values in attr diffs
The code adopted from block diffs was not set to handle null and unknown values, as those are not allowed for blocks. We also revert the change to formatting nested object types as single attributes, because the attribute formatter cannot handle sensitive values from the schema. This presents some awkward syntax for diffs for now, but should suffice until the entire formatter can be refactored to better handle these new nested types.
1 parent c351f41 commit 68ed506

2 files changed

Lines changed: 247 additions & 28 deletions

File tree

‎internal/command/format/diff.go‎

Lines changed: 36 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -471,28 +471,8 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At
471471
}
472472

473473
if attrS.NestedType != nil {
474-
renderNested := true
475-
476-
// If the collection values are empty or null, we render them as single attributes
477-
switch attrS.NestedType.Nesting {
478-
case configschema.NestingList, configschema.NestingSet, configschema.NestingMap:
479-
var oldLen, newLen int
480-
if !old.IsNull() && old.IsKnown() {
481-
oldLen = old.LengthInt()
482-
}
483-
if !new.IsNull() && new.IsKnown() {
484-
newLen = new.LengthInt()
485-
}
486-
487-
if oldLen+newLen == 0 {
488-
renderNested = false
489-
}
490-
}
491-
492-
if renderNested {
493-
p.writeNestedAttrDiff(name, attrS.NestedType, old, new, nameLen, indent, path, action, showJustNew)
494-
return false
495-
}
474+
p.writeNestedAttrDiff(name, attrS.NestedType, old, new, nameLen, indent, path, action, showJustNew)
475+
return false
496476
}
497477

498478
p.buf.WriteString("\n")
@@ -626,15 +606,24 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
626606
p.buf.WriteString(strings.Repeat(" ", indent+2))
627607
p.buf.WriteString("]")
628608

609+
if !new.IsKnown() {
610+
p.buf.WriteString(" -> (known after apply)")
611+
}
612+
629613
case configschema.NestingSet:
630614
oldItems := ctyCollectionValues(old)
631615
newItems := ctyCollectionValues(new)
632616

633-
allItems := make([]cty.Value, 0, len(oldItems)+len(newItems))
634-
allItems = append(allItems, oldItems...)
635-
allItems = append(allItems, newItems...)
617+
var all cty.Value
618+
if len(oldItems)+len(newItems) > 0 {
619+
allItems := make([]cty.Value, 0, len(oldItems)+len(newItems))
620+
allItems = append(allItems, oldItems...)
621+
allItems = append(allItems, newItems...)
636622

637-
all := cty.SetVal(allItems)
623+
all = cty.SetVal(allItems)
624+
} else {
625+
all = cty.SetValEmpty(old.Type().ElementType())
626+
}
638627

639628
p.buf.WriteString(" = [")
640629

@@ -646,6 +635,13 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
646635
case !val.IsKnown():
647636
action = plans.Update
648637
newValue = val
638+
case !new.IsKnown():
639+
action = plans.Delete
640+
// the value must have come from the old set
641+
oldValue = val
642+
// Mark the new val as null, but the entire set will be
643+
// displayed as "(unknown after apply)"
644+
newValue = cty.NullVal(val.Type())
649645
case old.IsNull() || !old.HasElement(val).True():
650646
action = plans.Create
651647
oldValue = cty.NullVal(val.Type())
@@ -680,6 +676,10 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
680676
p.buf.WriteString(strings.Repeat(" ", indent+2))
681677
p.buf.WriteString("]")
682678

679+
if !new.IsKnown() {
680+
p.buf.WriteString(" -> (known after apply)")
681+
}
682+
683683
case configschema.NestingMap:
684684
// For the sake of handling nested blocks, we'll treat a null map
685685
// the same as an empty map since the config language doesn't
@@ -688,7 +688,12 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
688688
new = ctyNullBlockMapAsEmpty(new)
689689

690690
oldItems := old.AsValueMap()
691-
newItems := new.AsValueMap()
691+
692+
newItems := map[string]cty.Value{}
693+
694+
if new.IsKnown() {
695+
newItems = new.AsValueMap()
696+
}
692697

693698
allKeys := make(map[string]bool)
694699
for k := range oldItems {
@@ -710,6 +715,7 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
710715
for _, k := range allKeysOrder {
711716
var action plans.Action
712717
oldValue := oldItems[k]
718+
713719
newValue := newItems[k]
714720
switch {
715721
case oldValue == cty.NilVal:
@@ -745,6 +751,9 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
745751
p.writeSkippedElems(unchanged, indent+4)
746752
p.buf.WriteString(strings.Repeat(" ", indent+2))
747753
p.buf.WriteString("}")
754+
if !new.IsKnown() {
755+
p.buf.WriteString(" -> (known after apply)")
756+
}
748757
}
749758

750759
return

‎internal/command/format/diff_test.go‎

Lines changed: 211 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2633,6 +2633,56 @@ func TestResourceChange_nestedList(t *testing.T) {
26332633
~ attr = "y" -> "z"
26342634
}
26352635
}
2636+
`,
2637+
},
2638+
"in-place update - unknown": {
2639+
Action: plans.Update,
2640+
Mode: addrs.ManagedResourceMode,
2641+
Before: cty.ObjectVal(map[string]cty.Value{
2642+
"id": cty.StringVal("i-02ae66f368e8518a9"),
2643+
"ami": cty.StringVal("ami-BEFORE"),
2644+
"disks": cty.ListVal([]cty.Value{
2645+
cty.ObjectVal(map[string]cty.Value{
2646+
"mount_point": cty.StringVal("/var/diska"),
2647+
"size": cty.StringVal("50GB"),
2648+
}),
2649+
}),
2650+
"root_block_device": cty.ListVal([]cty.Value{
2651+
cty.ObjectVal(map[string]cty.Value{
2652+
"volume_type": cty.StringVal("gp2"),
2653+
"new_field": cty.StringVal("new_value"),
2654+
}),
2655+
}),
2656+
}),
2657+
After: cty.ObjectVal(map[string]cty.Value{
2658+
"id": cty.StringVal("i-02ae66f368e8518a9"),
2659+
"ami": cty.StringVal("ami-AFTER"),
2660+
"disks": cty.UnknownVal(cty.List(cty.Object(map[string]cty.Type{
2661+
"mount_point": cty.String,
2662+
"size": cty.String,
2663+
}))),
2664+
"root_block_device": cty.ListVal([]cty.Value{
2665+
cty.ObjectVal(map[string]cty.Value{
2666+
"volume_type": cty.StringVal("gp2"),
2667+
"new_field": cty.StringVal("new_value"),
2668+
}),
2669+
}),
2670+
}),
2671+
RequiredReplace: cty.NewPathSet(),
2672+
Schema: testSchemaPlus(configschema.NestingList),
2673+
ExpectedOutput: ` # test_instance.example will be updated in-place
2674+
~ resource "test_instance" "example" {
2675+
~ ami = "ami-BEFORE" -> "ami-AFTER"
2676+
~ disks = [
2677+
~ {
2678+
- mount_point = "/var/diska" -> null
2679+
- size = "50GB" -> null
2680+
},
2681+
] -> (known after apply)
2682+
id = "i-02ae66f368e8518a9"
2683+
2684+
# (1 unchanged block hidden)
2685+
}
26362686
`,
26372687
},
26382688
}
@@ -2893,7 +2943,8 @@ func TestResourceChange_nestedSet(t *testing.T) {
28932943
ExpectedOutput: ` # test_instance.example will be updated in-place
28942944
~ resource "test_instance" "example" {
28952945
~ ami = "ami-BEFORE" -> "ami-AFTER"
2896-
+ disks = []
2946+
+ disks = [
2947+
]
28972948
id = "i-02ae66f368e8518a9"
28982949
}
28992950
`,
@@ -2952,6 +3003,56 @@ func TestResourceChange_nestedSet(t *testing.T) {
29523003
- volume_type = "gp2" -> null
29533004
}
29543005
}
3006+
`,
3007+
},
3008+
"in-place update - unknown": {
3009+
Action: plans.Update,
3010+
Mode: addrs.ManagedResourceMode,
3011+
Before: cty.ObjectVal(map[string]cty.Value{
3012+
"id": cty.StringVal("i-02ae66f368e8518a9"),
3013+
"ami": cty.StringVal("ami-BEFORE"),
3014+
"disks": cty.SetVal([]cty.Value{
3015+
cty.ObjectVal(map[string]cty.Value{
3016+
"mount_point": cty.StringVal("/var/diska"),
3017+
"size": cty.StringVal("50GB"),
3018+
}),
3019+
}),
3020+
"root_block_device": cty.SetVal([]cty.Value{
3021+
cty.ObjectVal(map[string]cty.Value{
3022+
"volume_type": cty.StringVal("gp2"),
3023+
"new_field": cty.StringVal("new_value"),
3024+
}),
3025+
}),
3026+
}),
3027+
After: cty.ObjectVal(map[string]cty.Value{
3028+
"id": cty.StringVal("i-02ae66f368e8518a9"),
3029+
"ami": cty.StringVal("ami-AFTER"),
3030+
"disks": cty.UnknownVal(cty.Set(cty.Object(map[string]cty.Type{
3031+
"mount_point": cty.String,
3032+
"size": cty.String,
3033+
}))),
3034+
"root_block_device": cty.SetVal([]cty.Value{
3035+
cty.ObjectVal(map[string]cty.Value{
3036+
"volume_type": cty.StringVal("gp2"),
3037+
"new_field": cty.StringVal("new_value"),
3038+
}),
3039+
}),
3040+
}),
3041+
RequiredReplace: cty.NewPathSet(),
3042+
Schema: testSchemaPlus(configschema.NestingSet),
3043+
ExpectedOutput: ` # test_instance.example will be updated in-place
3044+
~ resource "test_instance" "example" {
3045+
~ ami = "ami-BEFORE" -> "ami-AFTER"
3046+
~ disks = [
3047+
- {
3048+
- mount_point = "/var/diska" -> null
3049+
- size = "50GB" -> null
3050+
},
3051+
] -> (known after apply)
3052+
id = "i-02ae66f368e8518a9"
3053+
3054+
# (1 unchanged block hidden)
3055+
}
29553056
`,
29563057
},
29573058
}
@@ -3288,6 +3389,115 @@ func TestResourceChange_nestedMap(t *testing.T) {
32883389
- volume_type = "gp2" -> null
32893390
}
32903391
}
3392+
`,
3393+
},
3394+
"in-place update - unknown": {
3395+
Action: plans.Update,
3396+
Mode: addrs.ManagedResourceMode,
3397+
Before: cty.ObjectVal(map[string]cty.Value{
3398+
"id": cty.StringVal("i-02ae66f368e8518a9"),
3399+
"ami": cty.StringVal("ami-BEFORE"),
3400+
"disks": cty.MapVal(map[string]cty.Value{
3401+
"disk_a": cty.ObjectVal(map[string]cty.Value{
3402+
"mount_point": cty.StringVal("/var/diska"),
3403+
"size": cty.StringVal("50GB"),
3404+
}),
3405+
}),
3406+
"root_block_device": cty.MapVal(map[string]cty.Value{
3407+
"a": cty.ObjectVal(map[string]cty.Value{
3408+
"volume_type": cty.StringVal("gp2"),
3409+
"new_field": cty.StringVal("new_value"),
3410+
}),
3411+
}),
3412+
}),
3413+
After: cty.ObjectVal(map[string]cty.Value{
3414+
"id": cty.StringVal("i-02ae66f368e8518a9"),
3415+
"ami": cty.StringVal("ami-AFTER"),
3416+
"disks": cty.UnknownVal(cty.Map(cty.Object(map[string]cty.Type{
3417+
"mount_point": cty.String,
3418+
"size": cty.String,
3419+
}))),
3420+
"root_block_device": cty.MapVal(map[string]cty.Value{
3421+
"a": cty.ObjectVal(map[string]cty.Value{
3422+
"volume_type": cty.StringVal("gp2"),
3423+
"new_field": cty.StringVal("new_value"),
3424+
}),
3425+
}),
3426+
}),
3427+
RequiredReplace: cty.NewPathSet(),
3428+
Schema: testSchemaPlus(configschema.NestingMap),
3429+
ExpectedOutput: ` # test_instance.example will be updated in-place
3430+
~ resource "test_instance" "example" {
3431+
~ ami = "ami-BEFORE" -> "ami-AFTER"
3432+
~ disks = {
3433+
- "disk_a" = {
3434+
- mount_point = "/var/diska" -> null
3435+
- size = "50GB" -> null
3436+
},
3437+
} -> (known after apply)
3438+
id = "i-02ae66f368e8518a9"
3439+
3440+
# (1 unchanged block hidden)
3441+
}
3442+
`,
3443+
},
3444+
"in-place update - insertion sensitive": {
3445+
Action: plans.Update,
3446+
Mode: addrs.ManagedResourceMode,
3447+
Before: cty.ObjectVal(map[string]cty.Value{
3448+
"id": cty.StringVal("i-02ae66f368e8518a9"),
3449+
"ami": cty.StringVal("ami-BEFORE"),
3450+
"disks": cty.MapValEmpty(cty.Object(map[string]cty.Type{
3451+
"mount_point": cty.String,
3452+
"size": cty.String,
3453+
})),
3454+
"root_block_device": cty.MapVal(map[string]cty.Value{
3455+
"a": cty.ObjectVal(map[string]cty.Value{
3456+
"volume_type": cty.StringVal("gp2"),
3457+
"new_field": cty.StringVal("new_value"),
3458+
}),
3459+
}),
3460+
}),
3461+
After: cty.ObjectVal(map[string]cty.Value{
3462+
"id": cty.StringVal("i-02ae66f368e8518a9"),
3463+
"ami": cty.StringVal("ami-AFTER"),
3464+
"disks": cty.MapVal(map[string]cty.Value{
3465+
"disk_a": cty.ObjectVal(map[string]cty.Value{
3466+
"mount_point": cty.StringVal("/var/diska"),
3467+
"size": cty.StringVal("50GB"),
3468+
}),
3469+
}),
3470+
"root_block_device": cty.MapVal(map[string]cty.Value{
3471+
"a": cty.ObjectVal(map[string]cty.Value{
3472+
"volume_type": cty.StringVal("gp2"),
3473+
"new_field": cty.StringVal("new_value"),
3474+
}),
3475+
}),
3476+
}),
3477+
AfterValMarks: []cty.PathValueMarks{
3478+
{
3479+
Path: cty.Path{cty.GetAttrStep{Name: "disks"},
3480+
cty.IndexStep{Key: cty.StringVal("disk_a")},
3481+
cty.GetAttrStep{Name: "mount_point"},
3482+
},
3483+
Marks: cty.NewValueMarks(marks.Sensitive),
3484+
},
3485+
},
3486+
RequiredReplace: cty.NewPathSet(),
3487+
Schema: testSchemaPlus(configschema.NestingMap),
3488+
ExpectedOutput: ` # test_instance.example will be updated in-place
3489+
~ resource "test_instance" "example" {
3490+
~ ami = "ami-BEFORE" -> "ami-AFTER"
3491+
~ disks = {
3492+
+ "disk_a" = {
3493+
+ mount_point = (sensitive)
3494+
+ size = "50GB"
3495+
},
3496+
}
3497+
id = "i-02ae66f368e8518a9"
3498+
3499+
# (1 unchanged block hidden)
3500+
}
32913501
`,
32923502
},
32933503
}

0 commit comments

Comments
 (0)