plans/objchange: Don't consider refinements when validating plans

Providers that existed prior to refinements (all of them, at the time of
writing) cannot preserve refinements sent in unknown values in the
configuration, and even if one day providers _are_ aware of refinements
there we might add new ones that existing providers don't know how to
handle.

For that reason we'll absolve providers of the responsibility of
preserving refinements from config into plan by fixing some cases where
we were incorrectly using RawEquals to compare values; that function isn't
appropriate for comparing values that might be unknown.

However, to avoid a disruptive change right now this initial fix just
strips off the refinements before comparing. Ideally this should be using
Value.Equals and handling unknown values more explicitly, but we'll save
that for a possible later improvement.

This does not include a similar exception for validating whether a final
value conforms to a plan because the plan value and the final value are
both produced by the same provider and so providers ought to be able to
be consistent with their _own_ treatment of refinements, if any.
Configuration is special because Terraform itself generates that, and so
it can potentially contain refinements that a particular provider has no
awareness of.

internal/plans/objchange/objchange.go

@@ -429,7 +429,7 @@ func optionalValueNotComputable(schema *configschema.Attribute, val cty.Value) b
 // values have been added. This function is only used to correlated
 // configuration with possible valid prior values within sets.
 func validPriorFromConfig(schema nestedSchema, prior, config cty.Value) bool {
-	if config.RawEquals(prior) {
+	if unrefinedValue(config).RawEquals(unrefinedValue(prior)) {
 		return true
 	}
 
@@ -446,7 +446,7 @@ func validPriorFromConfig(schema nestedSchema, prior, config cty.Value) bool {
 		}
 
 		// we don't need to know the schema if both are equal
-		if configV.RawEquals(priorV) {
+		if unrefinedValue(configV).RawEquals(unrefinedValue(priorV)) {
 			// we know they are equal, so no need to descend further
 			return false, nil
 		}

internal/plans/objchange/plan_valid.go

@@ -270,11 +270,11 @@ func assertPlannedAttrValid(name string, attrS *configschema.Attribute, priorSta
 func assertPlannedValueValid(attrS *configschema.Attribute, priorV, configV, plannedV cty.Value, path cty.Path) []error {
 
 	var errs []error
-	if plannedV.RawEquals(configV) {
+	if unrefinedValue(plannedV).RawEquals(unrefinedValue(configV)) {
 		// This is the easy path: provider didn't change anything at all.
 		return errs
 	}
-	if plannedV.RawEquals(priorV) && !priorV.IsNull() && !configV.IsNull() {
+	if unrefinedValue(plannedV).RawEquals(unrefinedValue(priorV)) && !priorV.IsNull() && !configV.IsNull() {
 		// Also pretty easy: there is a prior value and the provider has
 		// returned it unchanged. This indicates that configV and plannedV
 		// are functionally equivalent and so the provider wishes to disregard
@@ -463,3 +463,12 @@ func assertPlannedObjectValid(schema *configschema.Object, prior, config, planne
 
 	return errs
 }
+
+// unrefinedValue returns the given value with any unknown value refinements
+// stripped away, making it a basic unknown value with only a type constraint.
+func unrefinedValue(v cty.Value) cty.Value {
+	if !v.IsKnown() {
+		return cty.UnknownVal(v.Type())
+	}
+	return v
+}

internal/plans/objchange/plan_valid_test.go

@@ -1796,11 +1796,39 @@ func TestAssertPlanValid(t *testing.T) {
 				)),
 			}),
 			[]string{
-				`.set: count in plan (cty.UnknownVal(cty.Number)) disagrees with count in config (cty.NumberIntVal(1))`,
-				`.list: count in plan (cty.UnknownVal(cty.Number)) disagrees with count in config (cty.NumberIntVal(1))`,
-				`.map: count in plan (cty.UnknownVal(cty.Number)) disagrees with count in config (cty.NumberIntVal(1))`,
+				`.set: count in plan (cty.UnknownVal(cty.Number).Refine().NotNull().NumberLowerBound(cty.NumberIntVal(0), true).NumberUpperBound(cty.NumberIntVal(9.223372036854775807e+18), true).NewValue()) disagrees with count in config (cty.NumberIntVal(1))`,
+				`.list: count in plan (cty.UnknownVal(cty.Number).Refine().NotNull().NumberLowerBound(cty.NumberIntVal(0), true).NumberUpperBound(cty.NumberIntVal(9.223372036854775807e+18), true).NewValue()) disagrees with count in config (cty.NumberIntVal(1))`,
+				`.map: count in plan (cty.UnknownVal(cty.Number).Refine().NotNull().NumberLowerBound(cty.NumberIntVal(0), true).NumberUpperBound(cty.NumberIntVal(9.223372036854775807e+18), true).NewValue()) disagrees with count in config (cty.NumberIntVal(1))`,
 			},
 		},
+
+		"refined unknown values can become less refined": {
+			// Providers often can't preserve refinements through the provider
+			// wire protocol: although we do have a defined serialization for
+			// it, most providers were written before there was any such
+			// thing as refinements, and in future there might be new
+			// refinements that even refinement-aware providers don't know
+			// how to preserve, so we allow them to get dropped here as
+			// a concession to backward-compatibility.
+			&configschema.Block{
+				Attributes: map[string]*configschema.Attribute{
+					"a": {
+						Type:     cty.String,
+						Required: true,
+					},
+				},
+			},
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.StringVal("old"),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.UnknownVal(cty.String).RefineNotNull(),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.UnknownVal(cty.String),
+			}),
+			nil,
+		},
 	}
 
 	for name, test := range tests {