Skip to content

Commit 4c321ab

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 4c321ab

File tree

2 files changed

+248
-28
lines changed

2 files changed

+248
-28
lines changed

‎internal/command/format/diff.go‎

Lines changed: 37 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,13 @@ 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+
// indicate if the entire map is unknown
695+
if new.IsKnown() {
696+
newItems = new.AsValueMap()
697+
}
692698

693699
allKeys := make(map[string]bool)
694700
for k := range oldItems {
@@ -710,6 +716,7 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
710716
for _, k := range allKeysOrder {
711717
var action plans.Action
712718
oldValue := oldItems[k]
719+
713720
newValue := newItems[k]
714721
switch {
715722
case oldValue == cty.NilVal:
@@ -745,6 +752,9 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
745752
p.writeSkippedElems(unchanged, indent+4)
746753
p.buf.WriteString(strings.Repeat(" ", indent+2))
747754
p.buf.WriteString("}")
755+
if !new.IsKnown() {
756+
p.buf.WriteString(" -> (known after apply)")
757+
}
748758
}
749759

750760
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)