diff --git a/internal/plans/objchange/objchange.go b/internal/plans/objchange/objchange.go index 8c5d25f51f..d277b7aa95 100644 --- a/internal/plans/objchange/objchange.go +++ b/internal/plans/objchange/objchange.go @@ -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 } diff --git a/internal/plans/objchange/objchange_test.go b/internal/plans/objchange/objchange_test.go index 3a1d3661c0..363bdd2e39 100644 --- a/internal/plans/objchange/objchange_test.go +++ b/internal/plans/objchange/objchange_test.go @@ -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 {