plans/objchange: extended ProposedNewObject to descend into attributes

with NestedType objects.

There are a handful of mostly cosmetic changes in this PR which likely
make the diff awkward to read; I renamed several functions to
(hopefully) clarifiy which funcs worked with Blocks vs other types. I
also extracted some small code snippets into their own functions for
reusability.

The code that descends into attributes with NestedTypes is similar to
the block-handling code, and differs in all the ways blocks and
attributes differ: null is valid for attributes, unlike blocks which can
only be present or empty.

plans/objchange/all_null.go

@@ -5,14 +5,29 @@ import (
 	"github.com/zclconf/go-cty/cty"
 )
 
-// AllAttributesNull constructs a non-null cty.Value of the object type implied
+// AllBlockAttributesNull constructs a non-null cty.Value of the object type implied
 // by the given schema that has all of its leaf attributes set to null and all
 // of its nested block collections set to zero-length.
 //
 // This simulates what would result from decoding an empty configuration block
 // with the given schema, except that it does not produce errors
-func AllAttributesNull(schema *configschema.Block) cty.Value {
+func AllBlockAttributesNull(schema *configschema.Block) cty.Value {
 	// "All attributes null" happens to be the definition of EmptyValue for
 	// a Block, so we can just delegate to that.
 	return schema.EmptyValue()
 }
+
+// AllAttributesNull returns a cty.Value of the object type implied by the given
+// attriubutes that has all of its leaf attributes set to null.
+func AllAttributesNull(attrs map[string]*configschema.Attribute) cty.Value {
+	newAttrs := make(map[string]cty.Value, len(attrs))
+
+	for name, attr := range attrs {
+		if attr.NestedType != nil {
+			newAttrs[name] = AllAttributesNull(attr.NestedType.Attributes)
+		} else {
+			newAttrs[name] = cty.NullVal(attr.Type)
+		}
+	}
+	return cty.ObjectVal(newAttrs)
+}

plans/objchange/objchange.go

@@ -37,7 +37,7 @@ func ProposedNew(schema *configschema.Block, prior, config cty.Value) cty.Value
 		// similar to the result of decoding an empty configuration block,
 		// which simplifies our handling of the top-level attributes/blocks
 		// below by giving us one non-null level of object to pull values from.
-		prior = AllAttributesNull(schema)
+		prior = AllBlockAttributesNull(schema)
 	}
 	return proposedNew(schema, prior, config)
 }
@@ -77,38 +77,7 @@ func proposedNew(schema *configschema.Block, prior, config cty.Value) cty.Value
 	// From this point onwards, we can assume that both values are non-null
 	// object types, and that the config value itself is known (though it
 	// may contain nested values that are unknown.)
-	newAttrs := map[string]cty.Value{}
-	for name, attr := range schema.Attributes {
-		priorV := prior.GetAttr(name)
-		configV := config.GetAttr(name)
-		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:
-			// configV will always be null in this case, by definition.
-			// priorV may also be null, but that's okay.
-			newV = priorV
-		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
-	}
+	newAttrs := proposedNewAttributes(schema.Attributes, prior, config)
 
 	// Merging nested blocks is a little more complex, since we need to
 	// correlate blocks between both objects and then recursively propose
@@ -282,6 +251,206 @@ func proposedNewNestedBlock(schema *configschema.NestedBlock, prior, config cty.
 	return newV
 }
 
+func proposedNewAttributes(attrs map[string]*configschema.Attribute, prior, config cty.Value) map[string]cty.Value {
+	if prior.IsNull() {
+		prior = AllAttributesNull(attrs)
+	}
+	newAttrs := make(map[string]cty.Value, len(attrs))
+	for name, attr := range attrs {
+		priorV := prior.GetAttr(name)
+		configV := config.GetAttr(name)
+		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:
+			// 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.
+				newV = configV
+			}
+		}
+		newAttrs[name] = newV
+	}
+	return newAttrs
+}
+
+func proposedNewNestedType(schema *configschema.Object, prior, config cty.Value) cty.Value {
+	var newV cty.Value
+	switch schema.Nesting {
+	case configschema.NestingSingle:
+		newAttrs := proposedNewAttributes(schema.Attributes, prior, config)
+		newV = cty.ObjectVal(newAttrs)
+
+	case configschema.NestingList:
+		// Nested blocks are correlated by index.
+		configVLen := 0
+		if config.IsKnown() && !config.IsNull() {
+			configVLen = config.LengthInt()
+		}
+		if configVLen > 0 {
+			newVals := make([]cty.Value, 0, configVLen)
+			for it := config.ElementIterator(); it.Next(); {
+				idx, configEV := it.Element()
+				if prior.IsKnown() && (prior.IsNull() || !prior.HasIndex(idx).True()) {
+					// If there is no corresponding prior element then
+					// we just take the config value as-is.
+					newVals = append(newVals, configEV)
+					continue
+				}
+				priorEV := prior.Index(idx)
+
+				newEV := proposedNewAttributes(schema.Attributes, priorEV, configEV)
+				newVals = append(newVals, cty.ObjectVal(newEV))
+			}
+			// Despite the name, a NestingList might also be a tuple, if
+			// its nested schema contains dynamically-typed attributes.
+			if config.Type().IsTupleType() {
+				newV = cty.TupleVal(newVals)
+			} else {
+				newV = cty.ListVal(newVals)
+			}
+		} else {
+			// Despite the name, a NestingList might also be a tuple, if
+			// its nested schema contains dynamically-typed attributes.
+			if config.Type().IsTupleType() {
+				newV = cty.EmptyTupleVal
+			} else {
+				newV = cty.ListValEmpty(schema.ImpliedType())
+			}
+		}
+
+	case configschema.NestingMap:
+		// Despite the name, a NestingMap may produce either a map or
+		// object value, depending on whether the nested schema contains
+		// dynamically-typed attributes.
+		if config.Type().IsObjectType() {
+			// Nested blocks are correlated by key.
+			configVLen := 0
+			if config.IsKnown() && !config.IsNull() {
+				configVLen = config.LengthInt()
+			}
+			if configVLen > 0 {
+				newVals := make(map[string]cty.Value, configVLen)
+				atys := config.Type().AttributeTypes()
+				for name := range atys {
+					configEV := config.GetAttr(name)
+					if !prior.IsKnown() || prior.IsNull() || !prior.Type().HasAttribute(name) {
+						// If there is no corresponding prior element then
+						// we just take the config value as-is.
+						newVals[name] = configEV
+						continue
+					}
+					priorEV := prior.GetAttr(name)
+					newEV := proposedNewAttributes(schema.Attributes, priorEV, configEV)
+					newVals[name] = cty.ObjectVal(newEV)
+				}
+				// Although we call the nesting mode "map", we actually use
+				// object values so that elements might have different types
+				// in case of dynamically-typed attributes.
+				newV = cty.ObjectVal(newVals)
+			} else {
+				newV = cty.EmptyObjectVal
+			}
+		} else {
+			configVLen := 0
+			if config.IsKnown() && !config.IsNull() {
+				configVLen = config.LengthInt()
+			}
+			if configVLen > 0 {
+				newVals := make(map[string]cty.Value, configVLen)
+				for it := config.ElementIterator(); it.Next(); {
+					idx, configEV := it.Element()
+					k := idx.AsString()
+					if prior.IsKnown() && (prior.IsNull() || !prior.HasIndex(idx).True()) {
+						// If there is no corresponding prior element then
+						// we just take the config value as-is.
+						newVals[k] = configEV
+						continue
+					}
+					priorEV := prior.Index(idx)
+
+					newEV := proposedNewAttributes(schema.Attributes, priorEV, configEV)
+					newVals[k] = cty.ObjectVal(newEV)
+				}
+				newV = cty.MapVal(newVals)
+			} else {
+				newV = cty.MapValEmpty(schema.ImpliedType())
+			}
+		}
+
+	case configschema.NestingSet:
+		// Nested blocks are correlated by comparing the element values
+		// after eliminating all of the computed attributes. In practice,
+		// this means that any config change produces an entirely new
+		// nested object, and we only propagate prior computed values
+		// if the non-computed attribute values are identical.
+		var cmpVals [][2]cty.Value
+		if prior.IsKnown() && !prior.IsNull() {
+			cmpVals = setElementCompareValuesFromObject(schema, prior)
+		}
+		configVLen := 0
+		if config.IsKnown() && !config.IsNull() {
+			configVLen = config.LengthInt()
+		}
+		if configVLen > 0 {
+			used := make([]bool, len(cmpVals)) // track used elements in case multiple have the same compare value
+			newVals := make([]cty.Value, 0, configVLen)
+			for it := config.ElementIterator(); it.Next(); {
+				_, configEV := it.Element()
+				var priorEV cty.Value
+				for i, cmp := range cmpVals {
+					if used[i] {
+						continue
+					}
+					if cmp[1].RawEquals(configEV) {
+						priorEV = cmp[0]
+						used[i] = true // we can't use this value on a future iteration
+						break
+					}
+				}
+				if priorEV == cty.NilVal {
+					newVals = append(newVals, configEV)
+				} else {
+					newEV := proposedNewAttributes(schema.Attributes, priorEV, configEV)
+					newVals = append(newVals, cty.ObjectVal(newEV))
+				}
+			}
+			newV = cty.SetVal(newVals)
+		} else {
+			newV = cty.SetValEmpty(schema.ImpliedType())
+		}
+	}
+
+	return newV
+}
+
 // setElementCompareValues takes a known, non-null value of a cty.Set type and
 // returns a table -- constructed of two-element arrays -- that maps original
 // set element values to corresponding values that have all of the computed
@@ -410,3 +579,51 @@ func setElementCompareValue(schema *configschema.Block, v cty.Value, isConfig bo
 
 	return cty.ObjectVal(attrs)
 }
+
+// setElementCompareValues takes a known, non-null value of a cty.Set type and
+// returns a table -- constructed of two-element arrays -- that maps original
+// set element values to corresponding values that have all of the computed
+// values removed, making them suitable for comparison with values obtained
+// from configuration. The element type of the set must conform to the implied
+// type of the given schema, or this function will panic.
+//
+// In the resulting slice, the zeroth element of each array is the original
+// value and the one-indexed element is the corresponding "compare value".
+//
+// This is intended to help correlate prior elements with configured elements
+// in proposedNewBlock. The result is a heuristic rather than an exact science,
+// since e.g. two separate elements may reduce to the same value through this
+// process. The caller must therefore be ready to deal with duplicates.
+func setElementCompareValuesFromObject(schema *configschema.Object, set cty.Value) [][2]cty.Value {
+	ret := make([][2]cty.Value, 0, set.LengthInt())
+	for it := set.ElementIterator(); it.Next(); {
+		_, ev := it.Element()
+		ret = append(ret, [2]cty.Value{ev, setElementCompareValueFromObject(schema, ev)})
+	}
+	return ret
+}
+
+// setElementCompareValue creates a new value that has all of the same
+// non-computed attribute values as the one given but has all computed
+// attribute values forced to null.
+//
+// The input value must conform to the schema's implied type, and the return
+// value is guaranteed to conform to it.
+func setElementCompareValueFromObject(schema *configschema.Object, v cty.Value) cty.Value {
+	if v.IsNull() || !v.IsKnown() {
+		return v
+	}
+	attrs := map[string]cty.Value{}
+
+	for name, attr := range schema.Attributes {
+		attrV := v.GetAttr(name)
+		switch {
+		case attr.Computed:
+			attrs[name] = cty.NullVal(attr.Type)
+		default:
+			attrs[name] = attrV
+		}
+	}
+
+	return cty.ObjectVal(attrs)
+}

plans/objchange/objchange_test.go

@@ -9,7 +9,7 @@ import (
 	"github.com/hashicorp/terraform/configs/configschema"
 )
 
-func TestProposedNewObject(t *testing.T) {
+func TestProposedNew(t *testing.T) {
 	tests := map[string]struct {
 		Schema *configschema.Block
 		Prior  cty.Value
@@ -1198,6 +1198,167 @@ func TestProposedNewObject(t *testing.T) {
 				}),
 			}),
 		},
+		// This example has a mixture of optional, computed and required in a deeply-nested NestedType attribute
+		"deeply NestedType": {
+			&configschema.Block{
+				Attributes: map[string]*configschema.Attribute{
+					"foo": {
+						NestedType: &configschema.Object{
+							Nesting: configschema.NestingSingle,
+							Attributes: map[string]*configschema.Attribute{
+								"bar": {
+									NestedType: &configschema.Object{
+										Nesting:    configschema.NestingSingle,
+										Attributes: testAttributes,
+									},
+									Required: true,
+								},
+								"baz": {
+									NestedType: &configschema.Object{
+										Nesting:    configschema.NestingSingle,
+										Attributes: testAttributes,
+									},
+									Optional: true,
+								},
+							},
+						},
+						Optional: true,
+					},
+				},
+			},
+			// prior
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.ObjectVal(map[string]cty.Value{
+					"bar": cty.NullVal(cty.DynamicPseudoType),
+					"baz": cty.ObjectVal(map[string]cty.Value{
+						"optional":          cty.NullVal(cty.String),
+						"computed":          cty.StringVal("hello"),
+						"optional_computed": cty.StringVal("prior"),
+						"required":          cty.StringVal("present"),
+					}),
+				}),
+			}),
+			// config
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.ObjectVal(map[string]cty.Value{
+					"bar": cty.UnknownVal(cty.Object(map[string]cty.Type{ // explicit unknown from the config
+						"optional":          cty.String,
+						"computed":          cty.String,
+						"optional_computed": cty.String,
+						"required":          cty.String,
+					})),
+					"baz": cty.ObjectVal(map[string]cty.Value{
+						"optional":          cty.NullVal(cty.String),
+						"computed":          cty.NullVal(cty.String),
+						"optional_computed": cty.StringVal("hello"),
+						"required":          cty.StringVal("present"),
+					}),
+				}),
+			}),
+			// want
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.ObjectVal(map[string]cty.Value{
+					"bar": cty.UnknownVal(cty.Object(map[string]cty.Type{ // explicit unknown preserved from the config
+						"optional":          cty.String,
+						"computed":          cty.String,
+						"optional_computed": cty.String,
+						"required":          cty.String,
+					})),
+					"baz": cty.ObjectVal(map[string]cty.Value{
+						"optional":          cty.NullVal(cty.String),  // config is null
+						"computed":          cty.StringVal("hello"),   // computed values come from prior
+						"optional_computed": cty.StringVal("hello"),   // config takes precedent over prior in opt+computed
+						"required":          cty.StringVal("present"), // value from config
+					}),
+				}),
+			}),
+		},
+		"deeply nested set": {
+			&configschema.Block{
+				Attributes: map[string]*configschema.Attribute{
+					"foo": {
+						NestedType: &configschema.Object{
+							Nesting: configschema.NestingSet,
+							Attributes: map[string]*configschema.Attribute{
+								"bar": {
+									NestedType: &configschema.Object{
+										Nesting:    configschema.NestingSet,
+										Attributes: testAttributes,
+									},
+									Required: true,
+								},
+							},
+						},
+						Optional: true,
+					},
+				},
+			},
+			// prior values
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.SetVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.SetVal([]cty.Value{
+							cty.ObjectVal(map[string]cty.Value{
+								"optional":          cty.StringVal("prior"),
+								"computed":          cty.StringVal("prior"),
+								"optional_computed": cty.StringVal("prior"),
+								"required":          cty.StringVal("prior"),
+							}),
+						}),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.SetVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{
+							"optional":          cty.StringVal("other_prior"),
+							"computed":          cty.StringVal("other_prior"),
+							"optional_computed": cty.StringVal("other_prior"),
+							"required":          cty.StringVal("other_prior"),
+						})}),
+					}),
+				}),
+			}),
+			// config differs from prior
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.SetVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.SetVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{
+							"optional":          cty.StringVal("configured"),
+							"computed":          cty.NullVal(cty.String), // computed attrs are null in config
+							"optional_computed": cty.StringVal("configured"),
+							"required":          cty.StringVal("configured"),
+						})}),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.SetVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{
+							"optional":          cty.NullVal(cty.String), // explicit null in config
+							"computed":          cty.NullVal(cty.String), // computed attrs are null in config
+							"optional_computed": cty.StringVal("other_configured"),
+							"required":          cty.StringVal("other_configured"),
+						})}),
+					}),
+				}),
+			}),
+			// want:
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.SetVal([]cty.Value{
+					cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.SetVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{
+							"optional":          cty.StringVal("configured"),
+							"computed":          cty.NullVal(cty.String),
+							"optional_computed": cty.StringVal("configured"),
+							"required":          cty.StringVal("configured"),
+						})}),
+					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"bar": cty.SetVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{
+							"optional":          cty.NullVal(cty.String), // explicit null in config is preserved
+							"computed":          cty.NullVal(cty.String),
+							"optional_computed": cty.StringVal("other_configured"),
+							"required":          cty.StringVal("other_configured"),
+						})}),
+					}),
+				}),
+			}),
+		},
 	}
 
 	for name, test := range tests {
@@ -1209,3 +1370,23 @@ func TestProposedNewObject(t *testing.T) {
 		})
 	}
 }
+
+var testAttributes = map[string]*configschema.Attribute{
+	"optional": {
+		Type:     cty.String,
+		Optional: true,
+	},
+	"computed": {
+		Type:     cty.String,
+		Computed: true,
+	},
+	"optional_computed": {
+		Type:     cty.String,
+		Computed: true,
+		Optional: true,
+	},
+	"required": {
+		Type:     cty.String,
+		Required: true,
+	},
+}