configs/configschema: fix missing "computed" attributes from NestedObject's...

configs/configschema: fix missing "computed" attributes from NestedObject's ImpliedType (#29172) (#29177)

* configs/configschema: fix missing "computed" attributes from NestedObject's ImpliedType

listOptionalAttrsFromObject was not including "computed" attributes in the list of optional object attributes. This is now fixed. I've also added some tests and fixed some panics and otherwise bad behavior when bad input is given. One natable change is in ImpliedType, which was panicking on an invalid nesting mode. The comment expressly states that it will return a result even when the schema is inconsistent, so I removed the panic and instead return an empty object.

internal/configs/configschema/decoder_spec.go

@@ -207,9 +207,15 @@ func (a *Attribute) decoderSpec(name string) hcldec.Spec {
 // belong to their own cty.Object definitions. It is used in other functions
 // which themselves handle that recursion.
 func listOptionalAttrsFromObject(o *Object) []string {
-	var ret []string
+	ret := make([]string, 0)
+
+	// This is unlikely to happen outside of tests.
+	if o == nil {
+		return ret
+	}
+
 	for name, attr := range o.Attributes {
-		if attr.Optional == true {
+		if attr.Optional || attr.Computed {
 			ret = append(ret, name)
 		}
 	}

internal/configs/configschema/decoder_spec_test.go

 package configschema
 
 import (
+	"sort"
 	"testing"
 
 	"github.com/apparentlymart/go-dump/dump"
 	"github.com/davecgh/go-spew/spew"
+	"github.com/google/go-cmp/cmp"
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/hcldec"
@@ -885,3 +887,43 @@ func TestAttributeDecoderSpec_panic(t *testing.T) {
 	attrS.decoderSpec("attr")
 	t.Errorf("expected panic")
 }
+
+func TestListOptionalAttrsFromObject(t *testing.T) {
+	tests := []struct {
+		input *Object
+		want  []string
+	}{
+		{
+			nil,
+			[]string{},
+		},
+		{
+			&Object{},
+			[]string{},
+		},
+		{
+			&Object{
+				Nesting: NestingSingle,
+				Attributes: map[string]*Attribute{
+					"optional":          {Type: cty.String, Optional: true},
+					"required":          {Type: cty.Number, Required: true},
+					"computed":          {Type: cty.List(cty.Bool), Computed: true},
+					"optional_computed": {Type: cty.Map(cty.Bool), Optional: true, Computed: true},
+				},
+			},
+			[]string{"optional", "computed", "optional_computed"},
+		},
+	}
+
+	for _, test := range tests {
+		got := listOptionalAttrsFromObject(test.input)
+
+		// order is irrelevant
+		sort.Strings(got)
+		sort.Strings(test.want)
+
+		if diff := cmp.Diff(got, test.want); diff != "" {
+			t.Fatalf("wrong result: %s\n", diff)
+		}
+	}
+}

internal/configs/configschema/implied_type.go

@@ -79,7 +79,7 @@ func (o *Object) ImpliedType() cty.Type {
 	case NestingSet:
 		return cty.Set(ret)
 	default: // Should never happen
-		panic("invalid Nesting")
+		return cty.EmptyObject
 	}
 }
 

internal/configs/configschema/implied_type_test.go

@@ -37,6 +37,7 @@ func TestBlockImpliedType(t *testing.T) {
 					"optional_computed": {
 						Type:     cty.Map(cty.Bool),
 						Optional: true,
+						Computed: true,
 					},
 				},
 			},
@@ -132,26 +133,18 @@ func TestObjectImpliedType(t *testing.T) {
 			nil,
 			cty.EmptyObject,
 		},
+		"empty": {
+			&Object{},
+			cty.EmptyObject,
+		},
 		"attributes": {
 			&Object{
 				Nesting: NestingSingle,
 				Attributes: map[string]*Attribute{
-					"optional": {
-						Type:     cty.String,
-						Optional: true,
-					},
-					"required": {
-						Type:     cty.Number,
-						Required: true,
-					},
-					"computed": {
-						Type:     cty.List(cty.Bool),
-						Computed: true,
-					},
-					"optional_computed": {
-						Type:     cty.Map(cty.Bool),
-						Optional: true,
-					},
+					"optional":          {Type: cty.String, Optional: true},
+					"required":          {Type: cty.Number, Required: true},
+					"computed":          {Type: cty.List(cty.Bool), Computed: true},
+					"optional_computed": {Type: cty.Map(cty.Bool), Optional: true, Computed: true},
 				},
 			},
 			cty.ObjectWithOptionalAttrs(
@@ -161,7 +154,7 @@ func TestObjectImpliedType(t *testing.T) {
 					"computed":          cty.List(cty.Bool),
 					"optional_computed": cty.Map(cty.Bool),
 				},
-				[]string{"optional", "optional_computed"},
+				[]string{"optional", "computed", "optional_computed"},
 			),
 		},
 		"nested attributes": {
@@ -172,21 +165,42 @@ func TestObjectImpliedType(t *testing.T) {
 						NestedType: &Object{
 							Nesting: NestingSingle,
 							Attributes: map[string]*Attribute{
-								"optional": {
-									Type:     cty.String,
-									Optional: true,
-								},
-								"required": {
-									Type:     cty.Number,
-									Required: true,
-								},
-								"computed": {
-									Type:     cty.List(cty.Bool),
-									Computed: true,
-								},
-								"optional_computed": {
-									Type:     cty.Map(cty.Bool),
-									Optional: true,
+								"optional":          {Type: cty.String, Optional: true},
+								"required":          {Type: cty.Number, Required: true},
+								"computed":          {Type: cty.List(cty.Bool), Computed: true},
+								"optional_computed": {Type: cty.Map(cty.Bool), Optional: true, Computed: true},
+							},
+						},
+						Optional: true,
+					},
+				},
+			},
+			cty.ObjectWithOptionalAttrs(map[string]cty.Type{
+				"nested_type": cty.ObjectWithOptionalAttrs(map[string]cty.Type{
+					"optional":          cty.String,
+					"required":          cty.Number,
+					"computed":          cty.List(cty.Bool),
+					"optional_computed": cty.Map(cty.Bool),
+				}, []string{"optional", "computed", "optional_computed"}),
+			}, []string{"nested_type"}),
+		},
+		"nested object-type attributes": {
+			&Object{
+				Nesting: NestingSingle,
+				Attributes: map[string]*Attribute{
+					"nested_type": {
+						NestedType: &Object{
+							Nesting: NestingSingle,
+							Attributes: map[string]*Attribute{
+								"optional":          {Type: cty.String, Optional: true},
+								"required":          {Type: cty.Number, Required: true},
+								"computed":          {Type: cty.List(cty.Bool), Computed: true},
+								"optional_computed": {Type: cty.Map(cty.Bool), Optional: true, Computed: true},
+								"object": {
+									Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{
+										"optional": cty.String,
+										"required": cty.Number,
+									}, []string{"optional"}),
 								},
 							},
 						},
@@ -200,7 +214,8 @@ func TestObjectImpliedType(t *testing.T) {
 					"required":          cty.Number,
 					"computed":          cty.List(cty.Bool),
 					"optional_computed": cty.Map(cty.Bool),
-				}, []string{"optional", "optional_computed"}),
+					"object":            cty.ObjectWithOptionalAttrs(map[string]cty.Type{"optional": cty.String, "required": cty.Number}, []string{"optional"}),
+				}, []string{"optional", "computed", "optional_computed"}),
 			}, []string{"nested_type"}),
 		},
 		"NestingList": {

internal/plans/objchange/objchange_test.go

@@ -1457,7 +1457,7 @@ func TestProposedNew(t *testing.T) {
 						"computed":          cty.String,
 						"optional_computed": cty.String,
 						"required":          cty.String,
-					}, []string{"optional", "optional_computed"}),
+					}, []string{"computed", "optional", "optional_computed"}),
 				}))),
 			}),
 		},