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.
This commit is contained in:
James Bardin 2021-08-18 13:17:03 -04:00
parent c351f41e47
commit 68ed50616e
2 changed files with 247 additions and 28 deletions

View File

@ -471,28 +471,8 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At
} }
if attrS.NestedType != nil { if attrS.NestedType != nil {
renderNested := true p.writeNestedAttrDiff(name, attrS.NestedType, old, new, nameLen, indent, path, action, showJustNew)
return false
// If the collection values are empty or null, we render them as single attributes
switch attrS.NestedType.Nesting {
case configschema.NestingList, configschema.NestingSet, configschema.NestingMap:
var oldLen, newLen int
if !old.IsNull() && old.IsKnown() {
oldLen = old.LengthInt()
}
if !new.IsNull() && new.IsKnown() {
newLen = new.LengthInt()
}
if oldLen+newLen == 0 {
renderNested = false
}
}
if renderNested {
p.writeNestedAttrDiff(name, attrS.NestedType, old, new, nameLen, indent, path, action, showJustNew)
return false
}
} }
p.buf.WriteString("\n") p.buf.WriteString("\n")
@ -626,15 +606,24 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
p.buf.WriteString(strings.Repeat(" ", indent+2)) p.buf.WriteString(strings.Repeat(" ", indent+2))
p.buf.WriteString("]") p.buf.WriteString("]")
if !new.IsKnown() {
p.buf.WriteString(" -> (known after apply)")
}
case configschema.NestingSet: case configschema.NestingSet:
oldItems := ctyCollectionValues(old) oldItems := ctyCollectionValues(old)
newItems := ctyCollectionValues(new) newItems := ctyCollectionValues(new)
allItems := make([]cty.Value, 0, len(oldItems)+len(newItems)) var all cty.Value
allItems = append(allItems, oldItems...) if len(oldItems)+len(newItems) > 0 {
allItems = append(allItems, newItems...) allItems := make([]cty.Value, 0, len(oldItems)+len(newItems))
allItems = append(allItems, oldItems...)
allItems = append(allItems, newItems...)
all := cty.SetVal(allItems) all = cty.SetVal(allItems)
} else {
all = cty.SetValEmpty(old.Type().ElementType())
}
p.buf.WriteString(" = [") p.buf.WriteString(" = [")
@ -646,6 +635,13 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
case !val.IsKnown(): case !val.IsKnown():
action = plans.Update action = plans.Update
newValue = val newValue = val
case !new.IsKnown():
action = plans.Delete
// the value must have come from the old set
oldValue = val
// Mark the new val as null, but the entire set will be
// displayed as "(unknown after apply)"
newValue = cty.NullVal(val.Type())
case old.IsNull() || !old.HasElement(val).True(): case old.IsNull() || !old.HasElement(val).True():
action = plans.Create action = plans.Create
oldValue = cty.NullVal(val.Type()) oldValue = cty.NullVal(val.Type())
@ -680,6 +676,10 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
p.buf.WriteString(strings.Repeat(" ", indent+2)) p.buf.WriteString(strings.Repeat(" ", indent+2))
p.buf.WriteString("]") p.buf.WriteString("]")
if !new.IsKnown() {
p.buf.WriteString(" -> (known after apply)")
}
case configschema.NestingMap: case configschema.NestingMap:
// For the sake of handling nested blocks, we'll treat a null map // For the sake of handling nested blocks, we'll treat a null map
// the same as an empty map since the config language doesn't // the same as an empty map since the config language doesn't
@ -688,7 +688,12 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
new = ctyNullBlockMapAsEmpty(new) new = ctyNullBlockMapAsEmpty(new)
oldItems := old.AsValueMap() oldItems := old.AsValueMap()
newItems := new.AsValueMap()
newItems := map[string]cty.Value{}
if new.IsKnown() {
newItems = new.AsValueMap()
}
allKeys := make(map[string]bool) allKeys := make(map[string]bool)
for k := range oldItems { for k := range oldItems {
@ -710,6 +715,7 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
for _, k := range allKeysOrder { for _, k := range allKeysOrder {
var action plans.Action var action plans.Action
oldValue := oldItems[k] oldValue := oldItems[k]
newValue := newItems[k] newValue := newItems[k]
switch { switch {
case oldValue == cty.NilVal: case oldValue == cty.NilVal:
@ -745,6 +751,9 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
p.writeSkippedElems(unchanged, indent+4) 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("}")
if !new.IsKnown() {
p.buf.WriteString(" -> (known after apply)")
}
} }
return return

View File

@ -2633,6 +2633,56 @@ func TestResourceChange_nestedList(t *testing.T) {
~ attr = "y" -> "z" ~ attr = "y" -> "z"
} }
} }
`,
},
"in-place update - unknown": {
Action: plans.Update,
Mode: addrs.ManagedResourceMode,
Before: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-BEFORE"),
"disks": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diska"),
"size": cty.StringVal("50GB"),
}),
}),
"root_block_device": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("gp2"),
"new_field": cty.StringVal("new_value"),
}),
}),
}),
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-AFTER"),
"disks": cty.UnknownVal(cty.List(cty.Object(map[string]cty.Type{
"mount_point": cty.String,
"size": cty.String,
}))),
"root_block_device": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("gp2"),
"new_field": cty.StringVal("new_value"),
}),
}),
}),
RequiredReplace: cty.NewPathSet(),
Schema: testSchemaPlus(configschema.NestingList),
ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = [
~ {
- mount_point = "/var/diska" -> null
- size = "50GB" -> null
},
] -> (known after apply)
id = "i-02ae66f368e8518a9"
# (1 unchanged block hidden)
}
`, `,
}, },
} }
@ -2893,7 +2943,8 @@ func TestResourceChange_nestedSet(t *testing.T) {
ExpectedOutput: ` # test_instance.example will be updated in-place ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" { ~ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER" ~ ami = "ami-BEFORE" -> "ami-AFTER"
+ disks = [] + disks = [
]
id = "i-02ae66f368e8518a9" id = "i-02ae66f368e8518a9"
} }
`, `,
@ -2952,6 +3003,56 @@ func TestResourceChange_nestedSet(t *testing.T) {
- volume_type = "gp2" -> null - volume_type = "gp2" -> null
} }
} }
`,
},
"in-place update - unknown": {
Action: plans.Update,
Mode: addrs.ManagedResourceMode,
Before: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-BEFORE"),
"disks": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diska"),
"size": cty.StringVal("50GB"),
}),
}),
"root_block_device": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("gp2"),
"new_field": cty.StringVal("new_value"),
}),
}),
}),
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-AFTER"),
"disks": cty.UnknownVal(cty.Set(cty.Object(map[string]cty.Type{
"mount_point": cty.String,
"size": cty.String,
}))),
"root_block_device": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("gp2"),
"new_field": cty.StringVal("new_value"),
}),
}),
}),
RequiredReplace: cty.NewPathSet(),
Schema: testSchemaPlus(configschema.NestingSet),
ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = [
- {
- mount_point = "/var/diska" -> null
- size = "50GB" -> null
},
] -> (known after apply)
id = "i-02ae66f368e8518a9"
# (1 unchanged block hidden)
}
`, `,
}, },
} }
@ -3288,6 +3389,115 @@ func TestResourceChange_nestedMap(t *testing.T) {
- volume_type = "gp2" -> null - volume_type = "gp2" -> null
} }
} }
`,
},
"in-place update - unknown": {
Action: plans.Update,
Mode: addrs.ManagedResourceMode,
Before: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-BEFORE"),
"disks": cty.MapVal(map[string]cty.Value{
"disk_a": cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diska"),
"size": cty.StringVal("50GB"),
}),
}),
"root_block_device": cty.MapVal(map[string]cty.Value{
"a": cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("gp2"),
"new_field": cty.StringVal("new_value"),
}),
}),
}),
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-AFTER"),
"disks": cty.UnknownVal(cty.Map(cty.Object(map[string]cty.Type{
"mount_point": cty.String,
"size": cty.String,
}))),
"root_block_device": cty.MapVal(map[string]cty.Value{
"a": cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("gp2"),
"new_field": cty.StringVal("new_value"),
}),
}),
}),
RequiredReplace: cty.NewPathSet(),
Schema: testSchemaPlus(configschema.NestingMap),
ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = {
- "disk_a" = {
- mount_point = "/var/diska" -> null
- size = "50GB" -> null
},
} -> (known after apply)
id = "i-02ae66f368e8518a9"
# (1 unchanged block hidden)
}
`,
},
"in-place update - insertion sensitive": {
Action: plans.Update,
Mode: addrs.ManagedResourceMode,
Before: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-BEFORE"),
"disks": cty.MapValEmpty(cty.Object(map[string]cty.Type{
"mount_point": cty.String,
"size": cty.String,
})),
"root_block_device": cty.MapVal(map[string]cty.Value{
"a": cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("gp2"),
"new_field": cty.StringVal("new_value"),
}),
}),
}),
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-AFTER"),
"disks": cty.MapVal(map[string]cty.Value{
"disk_a": cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diska"),
"size": cty.StringVal("50GB"),
}),
}),
"root_block_device": cty.MapVal(map[string]cty.Value{
"a": cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("gp2"),
"new_field": cty.StringVal("new_value"),
}),
}),
}),
AfterValMarks: []cty.PathValueMarks{
{
Path: cty.Path{cty.GetAttrStep{Name: "disks"},
cty.IndexStep{Key: cty.StringVal("disk_a")},
cty.GetAttrStep{Name: "mount_point"},
},
Marks: cty.NewValueMarks(marks.Sensitive),
},
},
RequiredReplace: cty.NewPathSet(),
Schema: testSchemaPlus(configschema.NestingMap),
ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = {
+ "disk_a" = {
+ mount_point = (sensitive)
+ size = "50GB"
},
}
id = "i-02ae66f368e8518a9"
# (1 unchanged block hidden)
}
`, `,
}, },
} }