From a6a08fe84aaaeee31252122dde544f2d35855aba Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Mon, 25 Jul 2022 14:58:40 -0400
Subject: [PATCH] AssertPlanValid for computed with config value

Return early from AssertPlanValid for any attribute which is only
computed. We currently fail if there's a config value, but that could
only happen because of core, not because of the provider.
---
 internal/plans/objchange/plan_valid.go      | 31 +++++++++++++--------
 internal/plans/objchange/plan_valid_test.go | 26 +++++++++++++++++
 2 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/internal/plans/objchange/plan_valid.go b/internal/plans/objchange/plan_valid.go
index 4f35f53478f1..1977ea7dd933 100644
--- a/internal/plans/objchange/plan_valid.go
+++ b/internal/plans/objchange/plan_valid.go
@@ -257,6 +257,7 @@ 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) {
 		// This is the easy path: provider didn't change anything at all.
@@ -270,22 +271,28 @@ func assertPlannedValueValid(attrS *configschema.Attribute, priorV, configV, pla
 		return errs
 	}
 
-	// the provider is allowed to insert values when the config is
+	switch {
+	// The provider can plan any value for a computed-only attribute. There may
+	// be a config value here in the case where a user used `ignore_changes` on
+	// a computed attribute and ignored the warning, or we failed to validate
+	// computed attributes in the config, but regardless it's not a plan error
+	// caused by the provider.
+	case attrS.Computed && !attrS.Optional:
+		return errs
+
+	// The provider is allowed to insert optional values when the config is
 	// null, but only if the attribute is computed.
-	if configV.IsNull() {
-		if attrS.Computed {
-			return errs
-		}
+	case configV.IsNull() && attrS.Computed:
+		return errs
 
+	case configV.IsNull() && !plannedV.IsNull():
 		// if the attribute is not computed, then any planned value is incorrect
-		if !plannedV.IsNull() {
-			if attrS.Sensitive {
-				errs = append(errs, path.NewErrorf("sensitive planned value for a non-computed attribute"))
-			} else {
-				errs = append(errs, path.NewErrorf("planned value %#v for a non-computed attribute", plannedV))
-			}
-			return errs
+		if attrS.Sensitive {
+			errs = append(errs, path.NewErrorf("sensitive planned value for a non-computed attribute"))
+		} else {
+			errs = append(errs, path.NewErrorf("planned value %#v for a non-computed attribute", plannedV))
 		}
+		return errs
 	}
 
 	// If this attribute has a NestedType, validate the nested object
diff --git a/internal/plans/objchange/plan_valid_test.go b/internal/plans/objchange/plan_valid_test.go
index 5c054cb907e6..8ba1927eeca7 100644
--- a/internal/plans/objchange/plan_valid_test.go
+++ b/internal/plans/objchange/plan_valid_test.go
@@ -1628,6 +1628,32 @@ func TestAssertPlanValid(t *testing.T) {
 			}),
 			[]string{`.map.one.name: planned value cty.StringVal("from_provider") does not match config value cty.StringVal("from_config")`},
 		},
+
+		// If a config value ended up in a computed-only attribute it can still
+		// be a valid plan. We either got here because the user ignore warnings
+		// about ignore_changes on computed attributes, or we failed to
+		// validate a config with computed values. Either way, we don't want to
+		// indicate an error with the provider.
+		"computed only value with config": {
+			&configschema.Block{
+				Attributes: map[string]*configschema.Attribute{
+					"a": {
+						Type:     cty.String,
+						Computed: true,
+					},
+				},
+			},
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.StringVal("old"),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.StringVal("old"),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.UnknownVal(cty.String),
+			}),
+			nil,
+		},
 	}
 
 	for name, test := range tests {