Skip to content

Commit

Permalink
prevent panics with null objects in nested attrs
Browse files Browse the repository at this point in the history
When descending into nested structural attributes, don't try to extract
attributes from null objects. Unlike with blocks, nested attributes
allow the possibility of assigning null values. While these technically
aren't allowed to be altered, we need to accept these for compatibility.
  • Loading branch information
jbardin committed Apr 26, 2024
1 parent c77898c commit f511356
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 3 deletions.
23 changes: 20 additions & 3 deletions internal/plans/objchange/plan_valid.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,12 +256,29 @@ func assertPlannedAttrsValid(schema map[string]*configschema.Attribute, priorSta
}

func assertPlannedAttrValid(name string, attrS *configschema.Attribute, priorState, config, plannedState cty.Value, path cty.Path) []error {
plannedV := plannedState.GetAttr(name)
configV := config.GetAttr(name)
priorV := cty.NullVal(attrS.Type)
// any of the config, prior or planned values may be null at this point if
// we are in nested structural attributes.
var plannedV, configV, priorV cty.Value
if attrS.NestedType != nil {
configV = cty.NullVal(attrS.NestedType.ImpliedType())
priorV = cty.NullVal(attrS.NestedType.ImpliedType())
} else {
configV = cty.NullVal(attrS.Type)
priorV = cty.NullVal(attrS.Type)
}

if !config.IsNull() {
configV = config.GetAttr(name)
}

if !priorState.IsNull() {
priorV = priorState.GetAttr(name)
}

if !plannedState.IsNull() {
plannedV = plannedState.GetAttr(name)
}

path = append(path, cty.GetAttrStep{Name: name})

return assertPlannedValueValid(attrS, priorV, configV, plannedV, path)
Expand Down
29 changes: 29 additions & 0 deletions internal/plans/objchange/plan_valid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1510,6 +1510,7 @@ func TestAssertPlanValid(t *testing.T) {
// When an object has dynamic attrs, the map may be
// handled as an object.
"map_as_obj": {
Optional: true,
NestedType: &configschema.Object{
Nesting: configschema.NestingMap,
Attributes: map[string]*configschema.Attribute{
Expand All @@ -1522,6 +1523,7 @@ func TestAssertPlanValid(t *testing.T) {
},
},
"list": {
Optional: true,
NestedType: &configschema.Object{
Nesting: configschema.NestingList,
Attributes: map[string]*configschema.Attribute{
Expand Down Expand Up @@ -1586,11 +1588,23 @@ func TestAssertPlanValid(t *testing.T) {
"one": cty.ObjectVal(map[string]cty.Value{
"name": cty.NullVal(cty.DynamicPseudoType),
}),
"two": cty.NullVal(cty.Object(map[string]cty.Type{
"name": cty.DynamicPseudoType,
})),
"three": cty.NullVal(cty.Object(map[string]cty.Type{
"name": cty.DynamicPseudoType,
})),
}),
"list": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"name": cty.NullVal(cty.String),
}),
cty.NullVal(cty.Object(map[string]cty.Type{
"name": cty.String,
})),
cty.NullVal(cty.Object(map[string]cty.Type{
"name": cty.String,
})),
}),
"set": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
Expand All @@ -1611,11 +1625,26 @@ func TestAssertPlanValid(t *testing.T) {
"one": cty.ObjectVal(map[string]cty.Value{
"name": cty.StringVal("computed"),
}),
// The config was null, but some providers may return a
// non-null object here, so we need to accept this for
// compatibility.
"two": cty.ObjectVal(map[string]cty.Value{
"name": cty.NullVal(cty.String),
}),
"three": cty.NullVal(cty.Object(map[string]cty.Type{
"name": cty.DynamicPseudoType,
})),
}),
"list": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"name": cty.StringVal("computed"),
}),
cty.ObjectVal(map[string]cty.Value{
"name": cty.NullVal(cty.String),
}),
cty.NullVal(cty.Object(map[string]cty.Type{
"name": cty.String,
})),
}),
"set": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
Expand Down

0 comments on commit f511356

Please sign in to comment.