Merge pull request #35090 from hashicorp/jbardin/plan-valid-nested-null

core: prevent panics with null objects in nested attrs
hashicorp · May 6, 2024 · 147186c · 147186c
2 parents a72ff6e + 2562420
commit 147186c
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 3 deletions.
diff --git a/internal/plans/objchange/plan_valid.go b/internal/plans/objchange/plan_valid.go
@@ -256,12 +256,31 @@ func assertPlannedAttrsValid(schema map[string]*configschema.Attribute, priorSta
 }
 
 func assertPlannedAttrValid(name string, attrS *configschema.Attribute, priorState, config, plannedState cty.Value, path cty.Path) []error {
-	plannedV := plannedState.GetAttr(name)
-	configV := config.GetAttr(name)
-	priorV := cty.NullVal(attrS.Type)
+	// any of the config, prior or planned values may be null at this point if
+	// we are in nested structural attributes.
+	var plannedV, configV, priorV cty.Value
+	if attrS.NestedType != nil {
+		configV = cty.NullVal(attrS.NestedType.ImpliedType())
+		priorV = cty.NullVal(attrS.NestedType.ImpliedType())
+		plannedV = cty.NullVal(attrS.NestedType.ImpliedType())
+	} else {
+		configV = cty.NullVal(attrS.Type)
+		priorV = cty.NullVal(attrS.Type)
+		plannedV = cty.NullVal(attrS.Type)
+	}
+
+	if !config.IsNull() {
+		configV = config.GetAttr(name)
+	}
+
 	if !priorState.IsNull() {
 		priorV = priorState.GetAttr(name)
 	}
+
+	if !plannedState.IsNull() {
+		plannedV = plannedState.GetAttr(name)
+	}
+
 	path = append(path, cty.GetAttrStep{Name: name})
 
 	return assertPlannedValueValid(attrS, priorV, configV, plannedV, path)

diff --git a/internal/plans/objchange/plan_valid_test.go b/internal/plans/objchange/plan_valid_test.go
@@ -1510,6 +1510,7 @@ func TestAssertPlanValid(t *testing.T) {
 					// When an object has dynamic attrs, the map may be
 					// handled as an object.
 					"map_as_obj": {
+						Optional: true,
 						NestedType: &configschema.Object{
 							Nesting: configschema.NestingMap,
 							Attributes: map[string]*configschema.Attribute{
@@ -1522,6 +1523,7 @@ func TestAssertPlanValid(t *testing.T) {
 						},
 					},
 					"list": {
+						Optional: true,
 						NestedType: &configschema.Object{
 							Nesting: configschema.NestingList,
 							Attributes: map[string]*configschema.Attribute{
@@ -1586,11 +1588,23 @@ func TestAssertPlanValid(t *testing.T) {
 					"one": cty.ObjectVal(map[string]cty.Value{
 						"name": cty.NullVal(cty.DynamicPseudoType),
 					}),
+					"two": cty.NullVal(cty.Object(map[string]cty.Type{
+						"name": cty.DynamicPseudoType,
+					})),
+					"three": cty.NullVal(cty.Object(map[string]cty.Type{
+						"name": cty.DynamicPseudoType,
+					})),
 				}),
 				"list": cty.ListVal([]cty.Value{
 					cty.ObjectVal(map[string]cty.Value{
 						"name": cty.NullVal(cty.String),
 					}),
+					cty.NullVal(cty.Object(map[string]cty.Type{
+						"name": cty.String,
+					})),
+					cty.NullVal(cty.Object(map[string]cty.Type{
+						"name": cty.String,
+					})),
 				}),
 				"set": cty.SetVal([]cty.Value{
 					cty.ObjectVal(map[string]cty.Value{
@@ -1611,11 +1625,26 @@ func TestAssertPlanValid(t *testing.T) {
 					"one": cty.ObjectVal(map[string]cty.Value{
 						"name": cty.StringVal("computed"),
 					}),
+					// The config was null, but some providers may return a
+					// non-null object here, so we need to accept this for
+					// compatibility.
+					"two": cty.ObjectVal(map[string]cty.Value{
+						"name": cty.NullVal(cty.String),
+					}),
+					"three": cty.NullVal(cty.Object(map[string]cty.Type{
+						"name": cty.DynamicPseudoType,
+					})),
 				}),
 				"list": cty.ListVal([]cty.Value{
 					cty.ObjectVal(map[string]cty.Value{
 						"name": cty.StringVal("computed"),
 					}),
+					cty.ObjectVal(map[string]cty.Value{
+						"name": cty.NullVal(cty.String),
+					}),
+					cty.NullVal(cty.Object(map[string]cty.Type{
+						"name": cty.String,
+					})),
 				}),
 				"set": cty.SetVal([]cty.Value{
 					cty.ObjectVal(map[string]cty.Value{