Merge pull request #32551 from hashicorp/jbardin/optional-computed-null

better determine when to plan optional+computed

internal/plans/objchange/objchange.go

@@ -297,6 +297,14 @@ func proposedNewAttributes(attrs map[string]*configschema.Attribute, prior, conf
 			// configV will always be null in this case, by definition.
 			// priorV may also be null, but that's okay.
 			newV = priorV
+
+			// the exception to the above is that if the config is optional and
+			// the _prior_ value contains non-computed values, we can infer
+			// that the config must have been non-null previously.
+			if optionalValueNotComputable(attr, priorV) {
+				newV = configV
+			}
+
 		case attr.NestedType != nil:
 			// For non-computed NestedType attributes, we need to descend
 			// into the individual nested attributes to build the final
@@ -518,3 +526,43 @@ func setElementComputedAsNull(schema attrPath, elem cty.Value) cty.Value {
 
 	return elem
 }
+
+// optionalValueNotComputable is used to check if an object in state must
+// have at least partially come from configuration. If the prior value has any
+// non-null attributes which are not computed in the schema, then we know there
+// was previously a configuration value which set those.
+//
+// This is used when the configuration contains a null optional+computed value,
+// and we want to know if we should plan to send the null value or the prior
+// state.
+func optionalValueNotComputable(schema *configschema.Attribute, val cty.Value) bool {
+	if !schema.Optional {
+		return false
+	}
+
+	// We must have a NestedType for complex nested attributes in order
+	// to find nested computed values in the first place.
+	if schema.NestedType == nil {
+		return false
+	}
+
+	foundNonComputedAttr := false
+	cty.Walk(val, func(path cty.Path, v cty.Value) (bool, error) {
+		if v.IsNull() {
+			return true, nil
+		}
+
+		attr := schema.NestedType.AttributeByPath(path)
+		if attr == nil {
+			return true, nil
+		}
+
+		if !attr.Computed {
+			foundNonComputedAttr = true
+			return false, nil
+		}
+		return true, nil
+	})
+
+	return foundNonComputedAttr
+}

internal/plans/objchange/objchange_test.go

@@ -460,10 +460,10 @@ func TestProposedNew(t *testing.T) {
 				})),
 			}),
 			cty.ObjectVal(map[string]cty.Value{
-				"bloop": cty.ObjectVal(map[string]cty.Value{
+				"bloop": cty.NullVal(cty.Object(map[string]cty.Type{
-					"blop":  cty.StringVal("glub"),
+					"blop":  cty.String,
-					"bleep": cty.NullVal(cty.String),
+					"bleep": cty.String,
-				}),
+				})),
 			}),
 		},
 
@@ -1989,8 +1989,9 @@ func TestProposedNew(t *testing.T) {
 		},
 
 		// A nested object with computed attributes, which is contained in an
-		// optional+computed container. The entire prior nested value should be
+		// optional+computed container. The prior nested object contains values
-		// represented in the proposed new object if the configuration is null.
+		// which could not be computed, therefor the proposed new value must be
+		// the null value from the configuration.
 		"computed within optional+computed": {
 			&configschema.Block{
 				Attributes: map[string]*configschema.Attribute{
@@ -2036,14 +2037,14 @@ func TestProposedNew(t *testing.T) {
 				)),
 			}),
 			cty.ObjectVal(map[string]cty.Value{
-				"list_obj": cty.ListVal([]cty.Value{
+				"list_obj": cty.NullVal(cty.List(
-					cty.ObjectVal(map[string]cty.Value{
+					cty.Object(map[string]cty.Type{
-						"obj": cty.ObjectVal(map[string]cty.Value{
+						"obj": cty.Object(map[string]cty.Type{
-							"optional": cty.StringVal("prior"),
+							"optional": cty.String,
-							"computed": cty.StringVal("prior computed"),
+							"computed": cty.String,
 						}),
 					}),
-				}),
+				)),
 			}),
 		},