From dfe5e1ddc4d5970e79fea8d45c07b50c4f23715b Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Wed, 8 Feb 2023 17:08:25 -0800
Subject: [PATCH] plans/objchange: Providers must honor their unknown value
 refinements

If the original value was unknown but its range was refined then the
provider must return a value that is within the refined range, because
otherwise downstream planning decisions could be invalidated.

This relies on cty's definition of whether a value is in a refined range,
which has pretty good coverage for the "false" case and so should give a
pretty good signal, but it'll probably improve over time and so providers
must not rely on any loopholes in the current implementation and must
keep their promises even if Terraform can't currently check them.
---
 internal/plans/objchange/compatible.go      |  9 +++-
 internal/plans/objchange/compatible_test.go | 59 +++++++++++++++++++++
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/internal/plans/objchange/compatible.go b/internal/plans/objchange/compatible.go
index c6f0c2bc50..65609ef154 100644
--- a/internal/plans/objchange/compatible.go
+++ b/internal/plans/objchange/compatible.go
@@ -34,7 +34,7 @@ func assertObjectCompatible(schema *configschema.Block, planned, actual cty.Valu
 	var errs []error
 	var atRoot string
 	if len(path) == 0 {
-		atRoot = "Root resource "
+		atRoot = "Root object "
 	}
 
 	if planned.IsNull() && !actual.IsNull() {
@@ -216,7 +216,12 @@ func assertValueCompatible(planned, actual cty.Value, path cty.Path) []error {
 
 	if !planned.IsKnown() {
 		// We didn't know what were going to end up with during plan, so
-		// anything goes during apply.
+		// the final value needs only to match the type and refinements of
+		// the unknown value placeholder.
+		plannedRng := planned.Range()
+		if ok := plannedRng.Includes(actual); ok.IsKnown() && ok.False() {
+			errs = append(errs, path.NewErrorf("final value %#v does not conform to planning placeholder %#v", actual, planned))
+		}
 		return errs
 	}
 
diff --git a/internal/plans/objchange/compatible_test.go b/internal/plans/objchange/compatible_test.go
index 60c9876702..eab1e35844 100644
--- a/internal/plans/objchange/compatible_test.go
+++ b/internal/plans/objchange/compatible_test.go
@@ -118,6 +118,65 @@ func TestAssertObjectCompatible(t *testing.T) {
 				`.name: was cty.StringVal("wotsit"), but now cty.StringVal("thingy")`,
 			},
 		},
+		{
+			&configschema.Block{
+				Attributes: map[string]*configschema.Attribute{
+					"name": {
+						Type:     cty.String,
+						Required: true,
+					},
+				},
+			},
+			cty.ObjectVal(map[string]cty.Value{
+				"name": cty.UnknownVal(cty.String),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"name": cty.Zero,
+			}),
+			[]string{
+				`.name: wrong final value type: string required`,
+			},
+		},
+		{
+			&configschema.Block{
+				Attributes: map[string]*configschema.Attribute{
+					"name": {
+						Type:     cty.String,
+						Required: true,
+					},
+				},
+			},
+			cty.ObjectVal(map[string]cty.Value{
+				"name": cty.UnknownVal(cty.String).RefineNotNull(),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"name": cty.NullVal(cty.String),
+			}),
+			[]string{
+				`.name: final value cty.NullVal(cty.String) does not conform to planning placeholder cty.UnknownVal(cty.String).RefineNotNull()`,
+			},
+		},
+		{
+			&configschema.Block{
+				Attributes: map[string]*configschema.Attribute{
+					"name": {
+						Type:     cty.String,
+						Required: true,
+					},
+				},
+			},
+			cty.ObjectVal(map[string]cty.Value{
+				"name": cty.UnknownVal(cty.String).Refine().
+					StringPrefix("boop:").
+					NewValue(),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"name": cty.StringVal("thingy"),
+			}),
+			[]string{
+				`.name: final value cty.StringVal("thingy") does not conform to planning placeholder cty.UnknownVal(cty.String).Refine().StringPrefixFull("boop:").NewValue()`,
+			},
+		},
 		{
 			&configschema.Block{
 				Attributes: map[string]*configschema.Attribute{