optional+computed can contain nested objects

When structural attributes were added, optional+computed were not
correctly handled when containing nested values which could themselves
be computed. This would cause terraform to ignore previously computed
values from state when generating the proposed plan.

The special case for optional+computed was incorrect, but isn't needed
in the context of planning new values anyway. Attributes are either
computed, or not computed. When optional+computed is set and there is
no configuration, the attribute is treated as computed. It is up to the
provider to determine how and when to deal with any changes to that
computed value.
This commit is contained in:
James Bardin 2023-01-17 17:07:39 -05:00
parent 95782f2491
commit 93f739e927
2 changed files with 145 additions and 32 deletions

View File

@ -286,42 +286,37 @@ func proposedNewAttributes(attrs map[string]*configschema.Attribute, prior, conf
}
configV := config.GetAttr(name)
// required isn't considered when constructing the plan, so attributes
// are essentially either computed or not computed. In the case of
// optional+computed, they are only computed when there is no
// configuration.
computed := attr.Computed
if attr.Optional && !configV.IsNull() {
computed = false
}
var newV cty.Value
switch {
case attr.Computed && attr.Optional:
// This is the trickiest scenario: we want to keep the prior value
// if the config isn't overriding it. Note that due to some
// ambiguity here, setting an optional+computed attribute from
// config and then later switching the config to null in a
// subsequent change causes the initial config value to be "sticky"
// unless the provider specifically overrides it during its own
// plan customization step.
if configV.IsNull() {
newV = priorV
} else {
newV = configV
}
case attr.Computed:
case computed:
// configV will always be null in this case, by definition.
// priorV may also be null, but that's okay.
newV = priorV
default:
if attr.NestedType != nil {
// For non-computed NestedType attributes, we need to descend
// into the individual nested attributes to build the final
// value, unless the entire nested attribute is unknown.
if !configV.IsKnown() {
newV = configV
} else {
newV = proposedNewNestedType(attr.NestedType, priorV, configV)
}
} else {
// For non-computed attributes, we always take the config value,
// even if it is null. If it's _required_ then null values
// should've been caught during an earlier validation step, and
// so we don't really care about that here.
case attr.NestedType != nil:
// For non-computed NestedType attributes, we need to descend
// into the individual nested attributes to build the final
// value, unless the entire nested attribute is unknown.
if !configV.IsKnown() {
newV = configV
} else {
newV = proposedNewNestedType(attr.NestedType, priorV, configV)
}
default:
// For non-computed attributes, we always take the config value,
// even if it is null. If it's _required_ then null values
// should've been caught during an earlier validation step, and
// so we don't really care about that here.
newV = configV
}
newAttrs[name] = newV
}

View File

@ -1738,27 +1738,145 @@ func TestProposedNew(t *testing.T) {
},
},
cty.UnknownVal(cty.Object(map[string]cty.Type{
"List": cty.List(cty.Object(map[string]cty.Type{
"list": cty.List(cty.Object(map[string]cty.Type{
"list": cty.List(cty.Object(map[string]cty.Type{
"foo": cty.String,
})),
})),
})),
cty.NullVal(cty.Object(map[string]cty.Type{
"List": cty.List(cty.Object(map[string]cty.Type{
"list": cty.List(cty.Object(map[string]cty.Type{
"list": cty.List(cty.Object(map[string]cty.Type{
"foo": cty.String,
})),
})),
})),
cty.UnknownVal(cty.Object(map[string]cty.Type{
"List": cty.List(cty.Object(map[string]cty.Type{
"list": cty.List(cty.Object(map[string]cty.Type{
"list": cty.List(cty.Object(map[string]cty.Type{
"foo": cty.String,
})),
})),
})),
},
// A nested object with computed attributes, which is contained in an
// optional+computed container. The nested computed values should be
// represented in the proposed new object.
"config within optional+computed": {
&configschema.Block{
Attributes: map[string]*configschema.Attribute{
"list_obj": {
Optional: true,
Computed: true,
NestedType: &configschema.Object{
Nesting: configschema.NestingList,
Attributes: map[string]*configschema.Attribute{
"obj": {
Optional: true,
NestedType: &configschema.Object{
Nesting: configschema.NestingSingle,
Attributes: map[string]*configschema.Attribute{
"optional": {Type: cty.String, Optional: true},
"computed": {Type: cty.String, Computed: true},
},
},
},
},
},
},
},
},
cty.ObjectVal(map[string]cty.Value{
"list_obj": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"obj": cty.ObjectVal(map[string]cty.Value{
"optional": cty.StringVal("prior"),
"computed": cty.StringVal("prior computed"),
}),
}),
}),
}),
cty.ObjectVal(map[string]cty.Value{
"list_obj": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"obj": cty.ObjectVal(map[string]cty.Value{
"optional": cty.StringVal("prior"),
"computed": cty.NullVal(cty.String),
}),
}),
}),
}),
cty.ObjectVal(map[string]cty.Value{
"list_obj": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"obj": cty.ObjectVal(map[string]cty.Value{
"optional": cty.StringVal("prior"),
"computed": cty.StringVal("prior computed"),
}),
}),
}),
}),
},
// A nested object with computed attributes, which is contained in an
// optional+computed container. The entire prior nested value should be
// represented in the proposed new object if the configuration is null.
"computed within optional+computed": {
&configschema.Block{
Attributes: map[string]*configschema.Attribute{
"list_obj": {
Optional: true,
Computed: true,
NestedType: &configschema.Object{
Nesting: configschema.NestingList,
Attributes: map[string]*configschema.Attribute{
"obj": {
Optional: true,
NestedType: &configschema.Object{
Nesting: configschema.NestingSingle,
Attributes: map[string]*configschema.Attribute{
"optional": {Type: cty.String, Optional: true},
"computed": {Type: cty.String, Computed: true},
},
},
},
},
},
},
},
},
cty.ObjectVal(map[string]cty.Value{
"list_obj": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"obj": cty.ObjectVal(map[string]cty.Value{
"optional": cty.StringVal("prior"),
"computed": cty.StringVal("prior computed"),
}),
}),
}),
}),
cty.ObjectVal(map[string]cty.Value{
"list_obj": cty.NullVal(cty.List(
cty.Object(map[string]cty.Type{
"obj": cty.Object(map[string]cty.Type{
"optional": cty.String,
"computed": cty.String,
}),
}),
)),
}),
cty.ObjectVal(map[string]cty.Value{
"list_obj": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"obj": cty.ObjectVal(map[string]cty.Value{
"optional": cty.StringVal("prior"),
"computed": cty.StringVal("prior computed"),
}),
}),
}),
}),
},
}
for name, test := range tests {