From 0b62de9f3d2f01c171620874ff59366185321f34 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 27 Jul 2022 12:19:50 -0400 Subject: [PATCH] handle nested types in StaticValidateTraversal The StaticValidateTraversal code was not taking into account nested structural types. Rather than create more special cases for checking Type vs NestedType, we move the ImpliedType method up to the Attribute to ensure both are used to generate the final type spec. --- internal/configs/configschema/empty_value.go | 5 +- internal/configs/configschema/implied_type.go | 14 +++++ .../configs/configschema/internal_validate.go | 2 +- .../configschema/validate_traversal.go | 8 +-- .../configschema/validate_traversal_test.go | 52 +++++++++++++++++++ 5 files changed, 72 insertions(+), 9 deletions(-) diff --git a/internal/configs/configschema/empty_value.go b/internal/configs/configschema/empty_value.go index ea8cdbbcc482..89e1f612f9b9 100644 --- a/internal/configs/configschema/empty_value.go +++ b/internal/configs/configschema/empty_value.go @@ -26,10 +26,7 @@ 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 { - return cty.NullVal(a.NestedType.ImpliedType()) - } - return cty.NullVal(a.Type) + return cty.NullVal(a.ImpliedType()) } // EmptyValue returns the "empty value" for when there are zero nested blocks diff --git a/internal/configs/configschema/implied_type.go b/internal/configs/configschema/implied_type.go index 0a1dc75f9631..9de5073f35ec 100644 --- a/internal/configs/configschema/implied_type.go +++ b/internal/configs/configschema/implied_type.go @@ -56,6 +56,20 @@ func (b *Block) ContainsSensitive() bool { return false } +// ImpliedType returns the cty.Type that would result from decoding a Block's +// ImpliedType and getting the resulting AttributeType. +// +// 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 (a *Attribute) ImpliedType() cty.Type { + if a.NestedType != nil { + return a.NestedType.specType().WithoutOptionalAttributesDeep() + } + return a.Type +} + // ImpliedType returns the cty.Type that would result from decoding a // NestedType Attribute using the receiving block schema. // diff --git a/internal/configs/configschema/internal_validate.go b/internal/configs/configschema/internal_validate.go index 8876672d793b..b113adfeccb7 100644 --- a/internal/configs/configschema/internal_validate.go +++ b/internal/configs/configschema/internal_validate.go @@ -135,7 +135,7 @@ func (a *Attribute) internalValidate(name, prefix string) error { // no validations to perform case NestingList, NestingSet: if a.NestedType.Nesting == NestingSet { - ety := a.NestedType.ImpliedType() + ety := a.ImpliedType() if ety.HasDynamicTypes() { // This is not permitted because the HCL (cty) set implementation // needs to know the exact type of set elements in order to diff --git a/internal/configs/configschema/validate_traversal.go b/internal/configs/configschema/validate_traversal.go index f8a9efe12bec..c98e3f7e152e 100644 --- a/internal/configs/configschema/validate_traversal.go +++ b/internal/configs/configschema/validate_traversal.go @@ -75,10 +75,10 @@ func (b *Block) StaticValidateTraversal(traversal hcl.Traversal) tfdiags.Diagnos if attrS, exists := b.Attributes[name]; exists { // For attribute validation we will just apply the rest of the - // traversal to an unknown value of the attribute type and pass - // through HCL's own errors, since we don't want to replicate all of - // HCL's type checking rules here. - val := cty.UnknownVal(attrS.Type) + // traversal to an unknown value of the attribute type and pass through + // HCL's own errors, since we don't want to replicate all of HCL's type + // checking rules here. + val := cty.UnknownVal(attrS.ImpliedType()) _, hclDiags := after.TraverseRel(val) diags = diags.Append(hclDiags) return diags diff --git a/internal/configs/configschema/validate_traversal_test.go b/internal/configs/configschema/validate_traversal_test.go index 2000d2e756e6..c49737d44350 100644 --- a/internal/configs/configschema/validate_traversal_test.go +++ b/internal/configs/configschema/validate_traversal_test.go @@ -13,6 +13,42 @@ func TestStaticValidateTraversal(t *testing.T) { "str": {Type: cty.String, Optional: true}, "list": {Type: cty.List(cty.String), Optional: true}, "dyn": {Type: cty.DynamicPseudoType, Optional: true}, + "nested_single": { + Optional: true, + NestedType: &Object{ + Nesting: NestingSingle, + Attributes: map[string]*Attribute{ + "optional": {Type: cty.String, Optional: true}, + }, + }, + }, + "nested_list": { + Optional: true, + NestedType: &Object{ + Nesting: NestingList, + Attributes: map[string]*Attribute{ + "optional": {Type: cty.String, Optional: true}, + }, + }, + }, + "nested_set": { + Optional: true, + NestedType: &Object{ + Nesting: NestingSet, + Attributes: map[string]*Attribute{ + "optional": {Type: cty.String, Optional: true}, + }, + }, + }, + "nested_map": { + Optional: true, + NestedType: &Object{ + Nesting: NestingMap, + Attributes: map[string]*Attribute{ + "optional": {Type: cty.String, Optional: true}, + }, + }, + }, } schema := &Block{ Attributes: attrs, @@ -168,6 +204,22 @@ func TestStaticValidateTraversal(t *testing.T) { `obj.map_block.anything.nonexist`, `Unsupported attribute: This object has no argument, nested block, or exported attribute named "nonexist".`, }, + { + `obj.nested_single.optional`, + ``, + }, + { + `obj.nested_list[0].optional`, + ``, + }, + { + `obj.nested_set[0].optional`, + `Invalid index: Elements of a set are identified only by their value and don't have any separate index or key to select with, so it's only possible to perform operations across all elements of the set.`, + }, + { + `obj.nested_map["key"].optional`, + ``, + }, } for _, test := range tests {