Skip to content

handle null and unknown values in attr diffs #29410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jbardin merged 1 commit into from
Aug 18, 2021
Merged

handle null and unknown values in attr diffs #29410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 36 additions & 27 deletions internal/command/format/diff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,28 +471,8 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At
}

if attrS.NestedType != nil {
renderNested := true

// 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.writeNestedAttrDiff(name, attrS.NestedType, old, new, nameLen, indent, path, action, showJustNew)
return false
}

p.buf.WriteString("\n")
Expand Down Expand Up @@ -626,15 +606,24 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
p.buf.WriteString(strings.Repeat(" ", indent+2))
p.buf.WriteString("]")

if !new.IsKnown() {
p.buf.WriteString(" -> (known after apply)")
}

case configschema.NestingSet:
oldItems := ctyCollectionValues(old)
newItems := ctyCollectionValues(new)

allItems := make([]cty.Value, 0, len(oldItems)+len(newItems))
allItems = append(allItems, oldItems...)
allItems = append(allItems, newItems...)
var all cty.Value
if len(oldItems)+len(newItems) > 0 {
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(" = [")

Expand All @@ -646,6 +635,13 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
case !val.IsKnown():
action = plans.Update
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():
action = plans.Create
oldValue = cty.NullVal(val.Type())
Expand Down Expand Up @@ -680,6 +676,10 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
p.buf.WriteString(strings.Repeat(" ", indent+2))
p.buf.WriteString("]")

if !new.IsKnown() {
p.buf.WriteString(" -> (known after apply)")
}

case configschema.NestingMap:
// 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
Expand All @@ -688,7 +688,12 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
new = ctyNullBlockMapAsEmpty(new)

oldItems := old.AsValueMap()
newItems := new.AsValueMap()

newItems := map[string]cty.Value{}

if new.IsKnown() {
newItems = new.AsValueMap()
}

allKeys := make(map[string]bool)
for k := range oldItems {
Expand All @@ -710,6 +715,7 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
for _, k := range allKeysOrder {
var action plans.Action
oldValue := oldItems[k]

newValue := newItems[k]
switch {
case oldValue == cty.NilVal:
Expand Down Expand Up @@ -745,6 +751,9 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
p.writeSkippedElems(unchanged, indent+4)
p.buf.WriteString(strings.Repeat(" ", indent+2))
p.buf.WriteString("}")
if !new.IsKnown() {
p.buf.WriteString(" -> (known after apply)")
}
}

return
Expand Down
212 changes: 211 additions & 1 deletion internal/command/format/diff_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2633,6 +2633,56 @@ func TestResourceChange_nestedList(t *testing.T) {
~ 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)
}
`,
},
}
Expand Down Expand Up @@ -2893,7 +2943,8 @@ func TestResourceChange_nestedSet(t *testing.T) {
ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
+ disks = []
+ disks = [
]
id = "i-02ae66f368e8518a9"
}
`,
Expand Down Expand Up @@ -2952,6 +3003,56 @@ func TestResourceChange_nestedSet(t *testing.T) {
- 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)
}
`,
},
}
Expand Down Expand Up @@ -3288,6 +3389,115 @@ func TestResourceChange_nestedMap(t *testing.T) {
- 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)
}
`,
},
}
Expand Down