From cd7fb9bd5a09d422a1dbe55d91dfde377e917188 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Wed, 24 Feb 2021 12:13:12 -0500
Subject: [PATCH] catch invalidly planned attributes earlier

Catch attributes which are planed but not computed separately to provide
a clearer error to provider developers.

The error conditions were previously caught, however it was unclear from
the error text as to _why_ the change was an error. The statements about
value inequality would be incorrect when planning no changes for a value
which should not have been set in the first place.
---
 plans/objchange/plan_valid.go      | 23 +++++++++++++++++++----
 plans/objchange/plan_valid_test.go |  4 ++--
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/plans/objchange/plan_valid.go b/plans/objchange/plan_valid.go
index 6a91428f8..07dd24865 100644
--- a/plans/objchange/plan_valid.go
+++ b/plans/objchange/plan_valid.go
@@ -255,10 +255,23 @@ func assertPlannedValueValid(attrS *configschema.Attribute, priorV, configV, pla
 		// the configuration value in favor of the prior.
 		return errs
 	}
-	if attrS.Computed && configV.IsNull() {
-		// The provider is allowed to change the value of any computed
-		// attribute that isn't explicitly set in the config.
-		return errs
+
+	// the provider is allowed to insert values when the config is
+	// null, but only if the attribute is computed.
+	if configV.IsNull() {
+		if attrS.Computed {
+			return errs
+		}
+
+		// 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 this attribute has a NestedType, validate the nested object
@@ -276,11 +289,13 @@ func assertPlannedValueValid(attrS *configschema.Attribute, priorV, configV, pla
 		}
 		return errs
 	}
+
 	if attrS.Sensitive {
 		errs = append(errs, path.NewErrorf("sensitive planned value does not match config value nor prior value"))
 	} else {
 		errs = append(errs, path.NewErrorf("planned value %#v does not match config value %#v nor prior value %#v", plannedV, configV, priorV))
 	}
+
 	return errs
 }
 
diff --git a/plans/objchange/plan_valid_test.go b/plans/objchange/plan_valid_test.go
index 1abc15677..5fca9b5d8 100644
--- a/plans/objchange/plan_valid_test.go
+++ b/plans/objchange/plan_valid_test.go
@@ -1077,7 +1077,7 @@ func TestAssertPlanValid(t *testing.T) {
 					}),
 				}),
 			}),
-			[]string{".bloop: planned for existence but config wants absense"},
+			[]string{`.bloop: planned value cty.ListVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{"blop":cty.StringVal("ok")})}) for a non-computed attribute`},
 		},
 		"NestedType nested set attribute to null": {
 			&configschema.Block{
@@ -1116,7 +1116,7 @@ func TestAssertPlanValid(t *testing.T) {
 					}),
 				}),
 			}),
-			[]string{".bloop: planned for existence but config wants absense"},
+			[]string{`.bloop: planned value cty.ListVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{"blop":cty.StringVal("ok")})}) for a non-computed attribute`},
 		},
 	}