From ded2641c1a4d6ad6e78c04c87bca4ce6d6637dfa Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Wed, 18 Mar 2020 23:30:09 -0400 Subject: [PATCH] helper/schema: Additional validation for schema attribute references Reference: https://github.com/hashicorp/terraform-plugin-sdk/issues/71 --- helper/schema/schema.go | 36 ++++++++++++++++++++++++++-------- helper/schema/schema_test.go | 38 +++++++++++++++++++++++++++++------- 2 files changed, 59 insertions(+), 15 deletions(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 881b2ebba2..78cfb546c7 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -770,7 +770,7 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro } if len(v.ConflictsWith) > 0 { - err := checkKeysAgainstSchemaFlags(k, v.ConflictsWith, topSchemaMap) + err := checkKeysAgainstSchemaFlags(k, v.ConflictsWith, topSchemaMap, v) if err != nil { return fmt.Errorf("ConflictsWith: %+v", err) } @@ -784,14 +784,14 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro } if len(v.ExactlyOneOf) > 0 { - err := checkKeysAgainstSchemaFlags(k, v.ExactlyOneOf, topSchemaMap) + err := checkKeysAgainstSchemaFlags(k, v.ExactlyOneOf, topSchemaMap, v) if err != nil { return fmt.Errorf("ExactlyOneOf: %+v", err) } } if len(v.AtLeastOneOf) > 0 { - err := checkKeysAgainstSchemaFlags(k, v.AtLeastOneOf, topSchemaMap) + err := checkKeysAgainstSchemaFlags(k, v.AtLeastOneOf, topSchemaMap, v) if err != nil { return fmt.Errorf("AtLeastOneOf: %+v", err) } @@ -860,14 +860,20 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro return nil } -func checkKeysAgainstSchemaFlags(k string, keys []string, topSchemaMap schemaMap) error { +func checkKeysAgainstSchemaFlags(k string, keys []string, topSchemaMap schemaMap, self *Schema) error { for _, key := range keys { parts := strings.Split(key, ".") sm := topSchemaMap var target *Schema for _, part := range parts { - // Skip index fields - if _, err := strconv.Atoi(part); err == nil { + // Skip index fields if 0 + partInt, err := strconv.Atoi(part) + + if err == nil { + if partInt != 0 { + return fmt.Errorf("%s configuration block reference (%s) can only use the .0. index for TypeList and MaxItems: 1 configuration blocks", k, key) + } + continue } @@ -876,13 +882,27 @@ func checkKeysAgainstSchemaFlags(k string, keys []string, topSchemaMap schemaMap return fmt.Errorf("%s references unknown attribute (%s) at part (%s)", k, key, part) } - if subResource, ok := target.Elem.(*Resource); ok { - sm = schemaMap(subResource.Schema) + subResource, ok := target.Elem.(*Resource) + + if !ok { + continue + } + + if target.Type == TypeSet || target.MaxItems != 1 { + return fmt.Errorf("%s configuration block reference (%s) can only be used with TypeList and MaxItems: 1 configuration blocks", k, key) } + + sm = schemaMap(subResource.Schema) } + if target == nil { return fmt.Errorf("%s cannot find target attribute (%s), sm: %#v", k, key, sm) } + + if target == self { + return fmt.Errorf("%s cannot reference self (%s)", k, key) + } + if target.Required { return fmt.Errorf("%s cannot contain Required attribute (%s)", k, key) } diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 2f21f536cc..87d1603255 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -3611,7 +3611,7 @@ func TestSchemaMap_InternalValidate(t *testing.T) { }, }, }, - false, // TODO: This should return an error as it will always error during runtime + true, }, "ConflictsWith list index syntax with list configuration block existing attribute": { @@ -3619,6 +3619,7 @@ func TestSchemaMap_InternalValidate(t *testing.T) { "config_block_attr": &Schema{ Type: TypeList, Optional: true, + MaxItems: 1, Elem: &Resource{ Schema: map[string]*Schema{ "nested_attr": &Schema{ @@ -3660,6 +3661,29 @@ func TestSchemaMap_InternalValidate(t *testing.T) { true, }, + "ConflictsWith list index syntax with list configuration block missing MaxItems": { + map[string]*Schema{ + "config_block_attr": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_attr": &Schema{ + Type: TypeString, + Optional: true, + }, + }, + }, + }, + "test": &Schema{ + Type: TypeBool, + Optional: true, + ConflictsWith: []string{"config_block_attr.0.missing_attr"}, + }, + }, + true, + }, + "ConflictsWith list index syntax with set configuration block existing attribute": { map[string]*Schema{ "config_block_attr": &Schema{ @@ -3680,7 +3704,7 @@ func TestSchemaMap_InternalValidate(t *testing.T) { ConflictsWith: []string{"config_block_attr.0.nested_attr"}, }, }, - false, // TODO: This should return an error as sets cannot be indexed + true, }, "ConflictsWith list index syntax with set configuration block missing attribute": { @@ -3726,7 +3750,7 @@ func TestSchemaMap_InternalValidate(t *testing.T) { ConflictsWith: []string{"config_block_attr.nested_attr"}, }, }, - false, // TODO: This should return an error as validateConflictingAttributes will never error + true, }, "ConflictsWith map key syntax with list configuration block self reference": { @@ -3745,7 +3769,7 @@ func TestSchemaMap_InternalValidate(t *testing.T) { }, }, }, - false, // TODO: This should return an error as validateConflictingAttributes will never error + true, }, "ConflictsWith map key syntax with set configuration block existing attribute": { @@ -3768,7 +3792,7 @@ func TestSchemaMap_InternalValidate(t *testing.T) { ConflictsWith: []string{"config_block_attr.nested_attr"}, }, }, - false, // TODO: This should return an error as validateConflictingAttributes will never error and sets cannot be indexed + true, }, "ConflictsWith map key syntax with set configuration block self reference": { @@ -3787,7 +3811,7 @@ func TestSchemaMap_InternalValidate(t *testing.T) { }, }, }, - false, // TODO: This should return an error as validateConflictingAttributes will never error and sets cannot be indexed + true, }, "ConflictsWith map key syntax with map attribute": { @@ -3830,7 +3854,7 @@ func TestSchemaMap_InternalValidate(t *testing.T) { ConflictsWith: []string{"test"}, }, }, - false, // TODO: This should return an error as it will always error during runtime + true, }, "Sub-resource invalid": {