From 3ad720e9dca5503ff3be9def0d0e33ab9ef7fb33 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Fri, 5 Feb 2021 13:34:55 -0500 Subject: [PATCH] configs/configschema: add new NestedType to attribute This commit adds a new field, NestedType, to the Attribute schema, and extends the current Attribute decoderSpec to account for the new type. The codepaths are mostly unused and included in a separate commit to verify that the included changes do not impact any other tests yet. --- configs/configschema/decoder_spec.go | 48 ++- configs/configschema/decoder_spec_test.go | 360 ++++++++++++++++++++++ configs/configschema/empty_value.go | 12 + configs/configschema/empty_value_test.go | 113 +++++++ configs/configschema/implied_type.go | 34 ++ configs/configschema/implied_type_test.go | 56 ++++ configs/configschema/schema.go | 26 ++ 7 files changed, 645 insertions(+), 4 deletions(-) diff --git a/configs/configschema/decoder_spec.go b/configs/configschema/decoder_spec.go index f7193ec07..8325eaf60 100644 --- a/configs/configschema/decoder_spec.go +++ b/configs/configschema/decoder_spec.go @@ -6,6 +6,7 @@ import ( "unsafe" "github.com/hashicorp/hcl/v2/hcldec" + "github.com/zclconf/go-cty/cty" ) var mapLabelNames = []string{"key"} @@ -183,9 +184,48 @@ func (b *Block) DecoderSpec() hcldec.Spec { } func (a *Attribute) decoderSpec(name string) hcldec.Spec { - return &hcldec.AttrSpec{ - Name: name, - Type: a.Type, - Required: a.Required, + ret := &hcldec.AttrSpec{Name: name} + if a == nil { + return ret } + + if a.NestedType != nil { + // FIXME: a panic() is a bad UX. Fix this, probably by extending + // InternalValidate() to check Attribute schemas as well and calling it + // when we get the schema from the provider in Context(). + if a.Type != cty.NilType { + panic("Invalid attribute schema: NestedType and Type cannot both be set. This is a bug in the provider.") + } + + var optAttrs []string + optAttrs = listOptionalAttrsFromObject(a.NestedType, optAttrs) + ty := a.NestedType.ImpliedType() + + switch a.NestedType.Nesting { + case NestingList: + ret.Type = cty.List(ty) + case NestingSet: + ret.Type = cty.Set(ty) + case NestingMap: + ret.Type = cty.Map(ty) + default: // NestingSingle, NestingGroup, or no NestingMode + ret.Type = ty + } + ret.Required = a.NestedType.MinItems > 0 + return ret + } + + ret.Type = a.Type + ret.Required = a.Required + return ret +} + +func listOptionalAttrsFromObject(o *Object, optAttrs []string) []string { + for name, attr := range o.Attributes { + if attr.Optional == true { + optAttrs = append(optAttrs, name) + } + } + + return optAttrs } diff --git a/configs/configschema/decoder_spec_test.go b/configs/configschema/decoder_spec_test.go index 492ee5dad..1f3691ed2 100644 --- a/configs/configschema/decoder_spec_test.go +++ b/configs/configschema/decoder_spec_test.go @@ -426,3 +426,363 @@ func TestBlockDecoderSpec(t *testing.T) { }) } } + +func TestAttributeDecoderSpec(t *testing.T) { + tests := map[string]struct { + Schema *Attribute + TestBody hcl.Body + Want cty.Value + DiagCount int + }{ + "empty": { + &Attribute{}, + hcl.EmptyBody(), + cty.NilVal, + 0, + }, + "nil": { + nil, + hcl.EmptyBody(), + cty.NilVal, + 0, + }, + "optional string (null)": { + &Attribute{ + Type: cty.String, + Optional: true, + }, + hcl.EmptyBody(), + cty.NullVal(cty.String), + 0, + }, + "optional string": { + &Attribute{ + Type: cty.String, + Optional: true, + }, + hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcl.Attributes{ + "attr": { + Name: "attr", + Expr: hcltest.MockExprLiteral(cty.StringVal("bar")), + }, + }, + }), + cty.StringVal("bar"), + 0, + }, + "NestedType with required string": { + &Attribute{ + NestedType: &Object{ + Attributes: map[string]*Attribute{ + "foo": { + Type: cty.String, + Required: true, + }, + }, + }, + Optional: true, + }, + hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcl.Attributes{ + "attr": { + Name: "attr", + Expr: hcltest.MockExprLiteral(cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("bar"), + })), + }, + }, + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("bar"), + }), + 0, + }, + "NestedType with optional attributes": { + &Attribute{ + NestedType: &Object{ + Attributes: map[string]*Attribute{ + "foo": { + Type: cty.String, + Optional: true, + }, + "bar": { + Type: cty.String, + Optional: true, + }, + }, + }, + Optional: true, + }, + hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcl.Attributes{ + "attr": { + Name: "attr", + Expr: hcltest.MockExprLiteral(cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("bar"), + })), + }, + }, + }), + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("bar"), + "bar": cty.NullVal(cty.String), + }), + 0, + }, + "NestedType with missing required string": { + &Attribute{ + NestedType: &Object{ + Attributes: map[string]*Attribute{ + "foo": { + Type: cty.String, + Required: true, + }, + }, + }, + Optional: true, + }, + hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcl.Attributes{ + "attr": { + Name: "attr", + Expr: hcltest.MockExprLiteral(cty.EmptyObjectVal), + }, + }, + }), + cty.UnknownVal(cty.Object(map[string]cty.Type{ + "foo": cty.String, + })), + 1, + }, + // NestedModes + "NestedType NestingModeList valid": { + &Attribute{ + NestedType: &Object{ + Nesting: NestingList, + Attributes: map[string]*Attribute{ + "foo": { + Type: cty.String, + Required: true, + }, + }, + }, + Optional: true, + }, + hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcl.Attributes{ + "attr": { + Name: "attr", + Expr: hcltest.MockExprLiteral(cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + // "foo" should be a string, not a list + "foo": cty.StringVal("bar"), + }), + cty.ObjectVal(map[string]cty.Value{ + // "foo" should be a string, not a list + "foo": cty.StringVal("baz"), + }), + })), + }, + }, + }), + cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{"foo": cty.StringVal("bar")}), + cty.ObjectVal(map[string]cty.Value{"foo": cty.StringVal("baz")}), + }), + 0, + }, + "NestedType NestingModeList invalid": { + &Attribute{ + NestedType: &Object{ + Nesting: NestingList, + Attributes: map[string]*Attribute{ + "foo": { + Type: cty.String, + Required: true, + }, + }, + }, + Optional: true, + }, + hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcl.Attributes{ + "attr": { + Name: "attr", + Expr: hcltest.MockExprLiteral(cty.ListVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{ + // "foo" should be a string, not a list + "foo": cty.ListVal([]cty.Value{cty.StringVal("bar"), cty.StringVal("baz")}), + })})), + }, + }, + }), + cty.UnknownVal(cty.List(cty.Object(map[string]cty.Type{"foo": cty.String}))), + 1, + }, + "NestedType NestingModeSet valid": { + &Attribute{ + NestedType: &Object{ + Nesting: NestingSet, + Attributes: map[string]*Attribute{ + "foo": { + Type: cty.String, + Required: true, + }, + }, + }, + Optional: true, + }, + hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcl.Attributes{ + "attr": { + Name: "attr", + Expr: hcltest.MockExprLiteral(cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + // "foo" should be a string, not a list + "foo": cty.StringVal("bar"), + }), + cty.ObjectVal(map[string]cty.Value{ + // "foo" should be a string, not a list + "foo": cty.StringVal("baz"), + }), + })), + }, + }, + }), + cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{"foo": cty.StringVal("bar")}), + cty.ObjectVal(map[string]cty.Value{"foo": cty.StringVal("baz")}), + }), + 0, + }, + "NestedType NestingModeSet invalid": { + &Attribute{ + NestedType: &Object{ + Nesting: NestingSet, + Attributes: map[string]*Attribute{ + "foo": { + Type: cty.String, + Required: true, + }, + }, + }, + Optional: true, + }, + hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcl.Attributes{ + "attr": { + Name: "attr", + Expr: hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{ + // "foo" should be a string, not a list + "foo": cty.ListVal([]cty.Value{cty.StringVal("bar"), cty.StringVal("baz")}), + })})), + }, + }, + }), + cty.UnknownVal(cty.Set(cty.Object(map[string]cty.Type{"foo": cty.String}))), + 1, + }, + "NestedType NestingModeMap valid": { + &Attribute{ + NestedType: &Object{ + Nesting: NestingMap, + Attributes: map[string]*Attribute{ + "foo": { + Type: cty.String, + Required: true, + }, + }, + }, + Optional: true, + }, + hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcl.Attributes{ + "attr": { + Name: "attr", + Expr: hcltest.MockExprLiteral(cty.MapVal(map[string]cty.Value{ + "one": cty.ObjectVal(map[string]cty.Value{ + // "foo" should be a string, not a list + "foo": cty.StringVal("bar"), + }), + "two": cty.ObjectVal(map[string]cty.Value{ + // "foo" should be a string, not a list + "foo": cty.StringVal("baz"), + }), + })), + }, + }, + }), + cty.MapVal(map[string]cty.Value{ + "one": cty.ObjectVal(map[string]cty.Value{"foo": cty.StringVal("bar")}), + "two": cty.ObjectVal(map[string]cty.Value{"foo": cty.StringVal("baz")}), + }), + 0, + }, + "NestedType NestingModeMap invalid": { + &Attribute{ + NestedType: &Object{ + Nesting: NestingMap, + Attributes: map[string]*Attribute{ + "foo": { + Type: cty.String, + Required: true, + }, + }, + }, + Optional: true, + }, + hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcl.Attributes{ + "attr": { + Name: "attr", + Expr: hcltest.MockExprLiteral(cty.MapVal(map[string]cty.Value{ + "one": cty.ObjectVal(map[string]cty.Value{ + // "foo" should be a string, not a list + "foo": cty.ListVal([]cty.Value{cty.StringVal("bar"), cty.StringVal("baz")}), + }), + })), + }, + }, + }), + cty.UnknownVal(cty.Map(cty.Object(map[string]cty.Type{"foo": cty.String}))), + 1, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + spec := test.Schema.decoderSpec("attr") + got, diags := hcldec.Decode(test.TestBody, spec, nil) + if len(diags) != test.DiagCount { + t.Errorf("wrong number of diagnostics %d; want %d", len(diags), test.DiagCount) + for _, diag := range diags { + t.Logf("- %s", diag.Error()) + } + } + + if !got.RawEquals(test.Want) { + t.Logf("[INFO] implied schema is %s", spew.Sdump(hcldec.ImpliedSchema(spec))) + t.Errorf("wrong result\ngot: %s\nwant: %s", dump.Value(got), dump.Value(test.Want)) + } + }) + } + +} + +// TestAttributeDecodeSpec_panic is a temporary test which verifies that +// decoderSpec panics when an invalid Attribute schema is encountered. It will +// be removed when InternalValidate() is extended to validate Attribute specs +// (and is used). See the #FIXME in decoderSpec. +func TestAttributeDecoderSpec_panic(t *testing.T) { + attrS := &Attribute{ + Type: cty.Object(map[string]cty.Type{ + "nested_attribute": cty.String, + }), + NestedType: &Object{}, + Optional: true, + } + + defer func() { recover() }() + attrS.decoderSpec("attr") + t.Errorf("expected panic") +} diff --git a/configs/configschema/empty_value.go b/configs/configschema/empty_value.go index 005da56bf..a51468494 100644 --- a/configs/configschema/empty_value.go +++ b/configs/configschema/empty_value.go @@ -26,6 +26,18 @@ func (b *Block) EmptyValue() cty.Value { // the value that would be returned if there were no definition of the attribute // at all, ignoring any required constraint. func (a *Attribute) EmptyValue() cty.Value { + if a.NestedType != nil { + switch a.NestedType.Nesting { + case NestingList: + return cty.NullVal(cty.List(a.NestedType.ImpliedType())) + case NestingSet: + return cty.NullVal(cty.Set(a.NestedType.ImpliedType())) + case NestingMap: + return cty.NullVal(cty.Map(a.NestedType.ImpliedType())) + default: // NestingSingle, NestingGroup, or no NestingMode + return cty.NullVal(a.NestedType.ImpliedType()) + } + } return cty.NullVal(a.Type) } diff --git a/configs/configschema/empty_value_test.go b/configs/configschema/empty_value_test.go index 44d27fe71..a11ab4cdb 100644 --- a/configs/configschema/empty_value_test.go +++ b/configs/configschema/empty_value_test.go @@ -168,3 +168,116 @@ func TestBlockEmptyValue(t *testing.T) { }) } } + +// Attribute EmptyValue() is well covered by the Block tests above; these tests +// focus on the behavior with NestedType field inside an Attribute +func TestAttributeEmptyValue(t *testing.T) { + tests := []struct { + Schema *Attribute + Want cty.Value + }{ + { + &Attribute{}, + cty.NilVal, + }, + { + &Attribute{ + Type: cty.String, + }, + cty.NullVal(cty.String), + }, + { + &Attribute{ + NestedType: &Object{ + // no Nesting set should behave the same as NestingSingle + Attributes: map[string]*Attribute{ + "str": {Type: cty.String, Required: true}, + }, + }, + }, + cty.NullVal(cty.Object(map[string]cty.Type{ + "str": cty.String, + })), + }, + { + &Attribute{ + NestedType: &Object{ + Nesting: NestingSingle, + Attributes: map[string]*Attribute{ + "str": {Type: cty.String, Required: true}, + }, + }, + }, + cty.NullVal(cty.Object(map[string]cty.Type{ + "str": cty.String, + })), + }, + { + &Attribute{ + NestedType: &Object{ + Nesting: NestingGroup, // functionally equivalent to NestingSingle in a NestedType + Attributes: map[string]*Attribute{ + "str": {Type: cty.String, Required: true}, + }, + }, + }, + cty.NullVal(cty.Object(map[string]cty.Type{ + "str": cty.String, + })), + }, + { + &Attribute{ + NestedType: &Object{ + Nesting: NestingList, + Attributes: map[string]*Attribute{ + "str": {Type: cty.String, Required: true}, + }, + }, + }, + cty.NullVal(cty.List( + cty.Object(map[string]cty.Type{ + "str": cty.String, + }), + )), + }, + { + &Attribute{ + NestedType: &Object{ + Nesting: NestingMap, + Attributes: map[string]*Attribute{ + "str": {Type: cty.String, Required: true}, + }, + }, + }, + cty.NullVal(cty.Map( + cty.Object(map[string]cty.Type{ + "str": cty.String, + }), + )), + }, + { + &Attribute{ + NestedType: &Object{ + Nesting: NestingSet, + Attributes: map[string]*Attribute{ + "str": {Type: cty.String, Required: true}, + }, + }, + }, + cty.NullVal(cty.Set( + cty.Object(map[string]cty.Type{ + "str": cty.String, + }), + )), + }, + } + + for _, test := range tests { + t.Run(fmt.Sprintf("%#v", test.Schema), func(t *testing.T) { + got := test.Schema.EmptyValue() + if !test.Want.RawEquals(got) { + t.Errorf("wrong result\nschema: %s\ngot: %s\nwant: %s", spew.Sdump(test.Schema), dump.Value(got), dump.Value(test.Want)) + } + }) + } +} diff --git a/configs/configschema/implied_type.go b/configs/configschema/implied_type.go index a81b7eab4..729881a03 100644 --- a/configs/configschema/implied_type.go +++ b/configs/configschema/implied_type.go @@ -40,3 +40,37 @@ func (b *Block) ContainsSensitive() bool { } return false } + +// ImpliedType returns the cty.Type that would result from decoding a NestedType +// Attribute using the receiving block schema. +// +// ImpliedType always returns a result, even if the given schema is +// inconsistent. Code that creates configschema.Object objects should be tested +// using the InternalValidate method to detect any inconsistencies that would +// cause this method to fall back on defaults and assumptions. +func (o *Object) ImpliedType() cty.Type { + if o == nil { + return cty.EmptyObject + } + + attrTys := make(map[string]cty.Type, len(o.Attributes)) + for name, attrS := range o.Attributes { + attrTys[name] = attrS.Type + } + + var optAttrs []string + optAttrs = listOptionalAttrsFromObject(o, optAttrs) + + return cty.ObjectWithOptionalAttrs(attrTys, optAttrs) +} + +// ContainsSensitive returns true if any of the attributes of the receiving +// Object are marked as sensitive. +func (o *Object) ContainsSensitive() bool { + for _, attrS := range o.Attributes { + if attrS.Sensitive { + return true + } + } + return false +} diff --git a/configs/configschema/implied_type_test.go b/configs/configschema/implied_type_test.go index 85e4a88d5..56316eead 100644 --- a/configs/configschema/implied_type_test.go +++ b/configs/configschema/implied_type_test.go @@ -122,3 +122,59 @@ func TestBlockImpliedType(t *testing.T) { }) } } + +func TestObjectImpliedType(t *testing.T) { + tests := map[string]struct { + Schema *Object + Want cty.Type + }{ + "nil": { + nil, + cty.EmptyObject, + }, + "empty": { + &Object{}, + cty.EmptyObject, + }, + "attributes": { + &Object{ + 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, + }, + }, + }, + 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", "optional_computed"}, + ), + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + got := test.Schema.ImpliedType() + if !got.Equals(test.Want) { + t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, test.Want) + } + }) + } +} diff --git a/configs/configschema/schema.go b/configs/configschema/schema.go index 4d3e7cabc..581bead8b 100644 --- a/configs/configschema/schema.go +++ b/configs/configschema/schema.go @@ -38,8 +38,13 @@ type Block struct { // Attribute represents a configuration attribute, within a block. type Attribute struct { // Type is a type specification that the attribute's value must conform to. + // It conflicts with NestedType. Type cty.Type + // NestedType indicates that the attribute is a NestedBlock-style object. + // This field conflicts with Type. + NestedType *Object + // Description is an English-language description of the purpose and // usage of the attribute. A description should be concise and use only // one or two sentences, leaving full definition to longer-form @@ -72,6 +77,25 @@ type Attribute struct { Deprecated bool } +// Object represents the embedding of a structural object inside an Attribute. +type Object struct { + // Attributes describes the nested attributes which may appear inside the + // Object. + Attributes map[string]*Attribute + + // Nesting provides the nesting mode for this Object, which determines how + // many instances of the Object are allowed, how many labels it expects, and + // how the resulting data will be converted into a data structure. + Nesting NestingMode + + // MinItems and MaxItems set, for the NestingList and NestingSet nesting + // modes, lower and upper limits on the number of child blocks allowed + // of the given type. If both are left at zero, no limit is applied. + // These fields are ignored for other nesting modes and must both be left + // at zero. + MinItems, MaxItems int +} + // NestedBlock represents the embedding of one block within another. type NestedBlock struct { // Block is the description of the block that's nested. @@ -98,6 +122,8 @@ type NestedBlock struct { // blocks. type NestingMode int +// Object represents the embedding of a NestedBl + //go:generate go run golang.org/x/tools/cmd/stringer -type=NestingMode const (