From 43100fbe39d6f800d6a44a13297b10bb25e4da06 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 ea8cdbbcc4..89e1f612f9 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 0a1dc75f96..9de5073f35 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 8876672d79..b113adfecc 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 f8a9efe12b..c98e3f7e15 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 2000d2e756..c49737d443 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 {