From 81ae4668141ffcaf228927bd2983acb577bb5612 Mon Sep 17 00:00:00 2001 From: Thomas Meckel Date: Mon, 24 Feb 2020 20:06:56 +0100 Subject: [PATCH 1/6] Added implementation for AllOf validation and moved validation functions after checking of an optional element has a value --- helper/schema/schema.go | 56 +++++++++++++++++++++++++++++++++++------ 1 file changed, 48 insertions(+), 8 deletions(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 69ab09b7c5a..d4ca93c274a 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -223,9 +223,13 @@ type Schema struct { // // AtLeastOneOf is a set of schema keys that, when set, at least one of // the keys in that list must be specified. + // + // AllOf is a set of schema keys that must be set simultaneously. This + // setting conflicts with ExactlyOneOf or AtLeastOneOf ConflictsWith []string ExactlyOneOf []string AtLeastOneOf []string + AllOf []string // When Deprecated is set, this attribute is deprecated. // @@ -773,6 +777,16 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro } } + if len(v.AllOf) > 0 { + if len(v.ExactlyOneOf) > 0 || len(v.AtLeastOneOf) > 0 { + return fmt.Errorf("AllOf cannot be used simultaneously with ExactlyOneOf or AtLeastOneOf") + } + err := checkKeysAgainstSchemaFlags(k, v.AllOf, topSchemaMap) + if err != nil { + return fmt.Errorf("AllOf: %+v", err) + } + } + if len(v.ExactlyOneOf) > 0 { err := checkKeysAgainstSchemaFlags(k, v.ExactlyOneOf, topSchemaMap) if err != nil { @@ -1390,22 +1404,27 @@ func (m schemaMap) validate( ok = raw != nil } - err := validateExactlyOneAttribute(k, schema, c) + if !ok { + if schema.Required { + return nil, []error{fmt.Errorf( + "%q: required field is not set", k)} + } + return nil, nil + } + + err := validateAllOfAttribute(k, schema, c) if err != nil { return nil, []error{err} } - err = validateAtLeastOneAttribute(k, schema, c) + err = validateExactlyOneAttribute(k, schema, c) if err != nil { return nil, []error{err} } - if !ok { - if schema.Required { - return nil, []error{fmt.Errorf( - "%q: required field is not set", k)} - } - return nil, nil + err = validateAtLeastOneAttribute(k, schema, c) + if err != nil { + return nil, []error{err} } if !schema.Required && !schema.Optional { @@ -1494,6 +1513,27 @@ func removeDuplicates(elements []string) []string { return result } +func validateAllOfAttribute( + k string, + schema *Schema, + c *terraform.ResourceConfig) error { + + if len(schema.AllOf) == 0 { + return nil + } + + allKeys := removeDuplicates(append(schema.AllOf, k)) + sort.Strings(allKeys) + + for _, allOfKey := range allKeys { + if _, ok := c.Get(allOfKey); !ok { + return fmt.Errorf("%q: all of `%s` must be specified", k, strings.Join(allKeys, ",")) + } + } + + return nil +} + func validateExactlyOneAttribute( k string, schema *Schema, From 6cc84bf72646523ecdf30b9736478aefbc2de0ab Mon Sep 17 00:00:00 2001 From: Thomas Meckel Date: Mon, 24 Feb 2020 20:41:14 +0100 Subject: [PATCH 2/6] Added tests for AllOf setting and corrected tests for AtLeastOneOf in helper\schema\schema_test.go --- helper/schema/schema_test.go | 273 ++++++++++++++++++++++++++++++++++- 1 file changed, 272 insertions(+), 1 deletion(-) diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 4e3cb2f991c..f3bba102045 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -6537,7 +6537,7 @@ func TestValidateAtLeastOneOfAttributes(t *testing.T) { }, Config: map[string]interface{}{}, - Err: true, + Err: false, }, "Only Unknown Variable Value": { @@ -6635,3 +6635,274 @@ func TestValidateAtLeastOneOfAttributes(t *testing.T) { }) } } + +func TestValidateAllOfAttributes(t *testing.T) { + cases := map[string]struct { + Key string + Schema map[string]*Schema + Config map[string]interface{} + Err bool + }{ + + "two attributes specified": { + Key: "whitelist", + Schema: map[string]*Schema{ + "whitelist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"blacklist"}, + }, + "blacklist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist"}, + }, + }, + + Config: map[string]interface{}{ + "whitelist": true, + "blacklist": true, + }, + Err: false, + }, + + "one attributes specified": { + Key: "whitelist", + Schema: map[string]*Schema{ + "whitelist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"blacklist"}, + }, + "blacklist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist"}, + }, + }, + + Config: map[string]interface{}{ + "whitelist": true, + }, + Err: true, + }, + + "no attributes specified": { + Key: "whitelist", + Schema: map[string]*Schema{ + "whitelist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"blacklist"}, + }, + "blacklist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist"}, + }, + }, + + Config: map[string]interface{}{}, + Err: false, + }, + + "two attributes of three specified": { + Key: "whitelist", + Schema: map[string]*Schema{ + "whitelist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"purplelist"}, + }, + "blacklist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist", "purplelist"}, + }, + "purplelist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist"}, + }, + }, + + Config: map[string]interface{}{ + "whitelist": true, + "purplelist": true, + }, + Err: false, + }, + + "three attributes of three specified": { + Key: "whitelist", + Schema: map[string]*Schema{ + "whitelist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"blacklist", "purplelist"}, + }, + "blacklist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist", "purplelist"}, + }, + "purplelist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist", "blacklist"}, + }, + }, + + Config: map[string]interface{}{ + "whitelist": true, + "purplelist": true, + "blacklist": true, + }, + Err: false, + }, + + "one attributes of three specified": { + Key: "whitelist", + Schema: map[string]*Schema{ + "whitelist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"blacklist", "purplelist"}, + }, + "blacklist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist", "purplelist"}, + }, + "purplelist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist", "blacklist"}, + }, + }, + + Config: map[string]interface{}{ + "purplelist": true, + }, + Err: true, + }, + + "no attributes of three specified": { + Key: "whitelist", + Schema: map[string]*Schema{ + "whitelist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist", "blacklist", "purplelist"}, + }, + "blacklist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist", "blacklist", "purplelist"}, + }, + "purplelist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist", "blacklist", "purplelist"}, + }, + }, + + Config: map[string]interface{}{}, + Err: false, + }, + + "Only Unknown Variable Value": { + Schema: map[string]*Schema{ + "whitelist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist", "blacklist", "purplelist"}, + }, + "blacklist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist", "blacklist", "purplelist"}, + }, + "purplelist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist", "blacklist", "purplelist"}, + }, + }, + + Config: map[string]interface{}{ + "whitelist": hcl2shim.UnknownVariableValue, + }, + + Err: true, + }, + + "only unknown list value": { + Schema: map[string]*Schema{ + "whitelist": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Schema{Type: TypeString}, + AllOf: []string{"whitelist", "blacklist"}, + }, + "blacklist": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Schema{Type: TypeString}, + AllOf: []string{"whitelist", "blacklist"}, + }, + }, + + Config: map[string]interface{}{ + "whitelist": []interface{}{hcl2shim.UnknownVariableValue}, + }, + + Err: true, + }, + + "Unknown Variable Value and Known Value": { + Schema: map[string]*Schema{ + "whitelist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist", "blacklist", "purplelist"}, + }, + "blacklist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist", "blacklist", "purplelist"}, + }, + "purplelist": &Schema{ + Type: TypeBool, + Optional: true, + AllOf: []string{"whitelist", "blacklist", "purplelist"}, + }, + }, + + Config: map[string]interface{}{ + "whitelist": hcl2shim.UnknownVariableValue, + "blacklist": true, + }, + + Err: true, + }, + } + + for tn, tc := range cases { + t.Run(tn, func(t *testing.T) { + c := terraform.NewResourceConfigRaw(tc.Config) + _, es := schemaMap(tc.Schema).Validate(c) + if len(es) > 0 != tc.Err { + if len(es) == 0 { + t.Fatalf("expected error") + } + + for _, e := range es { + t.Fatalf("didn't expect error, got error: %+v", e) + } + + t.FailNow() + } + }) + } +} From 0b04412156df5283e2f048d58b5fcd241f05042c Mon Sep 17 00:00:00 2001 From: Thomas Meckel Date: Mon, 24 Feb 2020 21:05:50 +0100 Subject: [PATCH 3/6] Corrected formatting of helper/schema/schema.go --- helper/schema/schema.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index d4ca93c274a..13431b8f504 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -223,13 +223,13 @@ type Schema struct { // // AtLeastOneOf is a set of schema keys that, when set, at least one of // the keys in that list must be specified. - // - // AllOf is a set of schema keys that must be set simultaneously. This + // + // AllOf is a set of schema keys that must be set simultaneously. This // setting conflicts with ExactlyOneOf or AtLeastOneOf ConflictsWith []string ExactlyOneOf []string AtLeastOneOf []string - AllOf []string + AllOf []string // When Deprecated is set, this attribute is deprecated. // @@ -1521,7 +1521,7 @@ func validateAllOfAttribute( if len(schema.AllOf) == 0 { return nil } - + allKeys := removeDuplicates(append(schema.AllOf, k)) sort.Strings(allKeys) From 61abb2942d3128bbec95e89dd71e3d4197393cf4 Mon Sep 17 00:00:00 2001 From: Thomas Meckel Date: Sat, 21 Mar 2020 19:46:45 +0100 Subject: [PATCH 4/6] Fixed formatting issues in helper/schema/schema_test.go --- helper/schema/schema_test.go | 52 ++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index f3bba102045..f0adee924bc 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -6647,12 +6647,12 @@ func TestValidateAllOfAttributes(t *testing.T) { "two attributes specified": { Key: "whitelist", Schema: map[string]*Schema{ - "whitelist": &Schema{ + "whitelist": { Type: TypeBool, Optional: true, AllOf: []string{"blacklist"}, }, - "blacklist": &Schema{ + "blacklist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist"}, @@ -6669,12 +6669,12 @@ func TestValidateAllOfAttributes(t *testing.T) { "one attributes specified": { Key: "whitelist", Schema: map[string]*Schema{ - "whitelist": &Schema{ + "whitelist": { Type: TypeBool, Optional: true, AllOf: []string{"blacklist"}, }, - "blacklist": &Schema{ + "blacklist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist"}, @@ -6690,12 +6690,12 @@ func TestValidateAllOfAttributes(t *testing.T) { "no attributes specified": { Key: "whitelist", Schema: map[string]*Schema{ - "whitelist": &Schema{ + "whitelist": { Type: TypeBool, Optional: true, AllOf: []string{"blacklist"}, }, - "blacklist": &Schema{ + "blacklist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist"}, @@ -6709,17 +6709,17 @@ func TestValidateAllOfAttributes(t *testing.T) { "two attributes of three specified": { Key: "whitelist", Schema: map[string]*Schema{ - "whitelist": &Schema{ + "whitelist": { Type: TypeBool, Optional: true, AllOf: []string{"purplelist"}, }, - "blacklist": &Schema{ + "blacklist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist", "purplelist"}, }, - "purplelist": &Schema{ + "purplelist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist"}, @@ -6736,17 +6736,17 @@ func TestValidateAllOfAttributes(t *testing.T) { "three attributes of three specified": { Key: "whitelist", Schema: map[string]*Schema{ - "whitelist": &Schema{ + "whitelist": { Type: TypeBool, Optional: true, AllOf: []string{"blacklist", "purplelist"}, }, - "blacklist": &Schema{ + "blacklist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist", "purplelist"}, }, - "purplelist": &Schema{ + "purplelist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist", "blacklist"}, @@ -6764,17 +6764,17 @@ func TestValidateAllOfAttributes(t *testing.T) { "one attributes of three specified": { Key: "whitelist", Schema: map[string]*Schema{ - "whitelist": &Schema{ + "whitelist": { Type: TypeBool, Optional: true, AllOf: []string{"blacklist", "purplelist"}, }, - "blacklist": &Schema{ + "blacklist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist", "purplelist"}, }, - "purplelist": &Schema{ + "purplelist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist", "blacklist"}, @@ -6790,17 +6790,17 @@ func TestValidateAllOfAttributes(t *testing.T) { "no attributes of three specified": { Key: "whitelist", Schema: map[string]*Schema{ - "whitelist": &Schema{ + "whitelist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist", "blacklist", "purplelist"}, }, - "blacklist": &Schema{ + "blacklist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist", "blacklist", "purplelist"}, }, - "purplelist": &Schema{ + "purplelist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist", "blacklist", "purplelist"}, @@ -6813,17 +6813,17 @@ func TestValidateAllOfAttributes(t *testing.T) { "Only Unknown Variable Value": { Schema: map[string]*Schema{ - "whitelist": &Schema{ + "whitelist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist", "blacklist", "purplelist"}, }, - "blacklist": &Schema{ + "blacklist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist", "blacklist", "purplelist"}, }, - "purplelist": &Schema{ + "purplelist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist", "blacklist", "purplelist"}, @@ -6839,13 +6839,13 @@ func TestValidateAllOfAttributes(t *testing.T) { "only unknown list value": { Schema: map[string]*Schema{ - "whitelist": &Schema{ + "whitelist": { Type: TypeList, Optional: true, Elem: &Schema{Type: TypeString}, AllOf: []string{"whitelist", "blacklist"}, }, - "blacklist": &Schema{ + "blacklist": { Type: TypeList, Optional: true, Elem: &Schema{Type: TypeString}, @@ -6862,17 +6862,17 @@ func TestValidateAllOfAttributes(t *testing.T) { "Unknown Variable Value and Known Value": { Schema: map[string]*Schema{ - "whitelist": &Schema{ + "whitelist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist", "blacklist", "purplelist"}, }, - "blacklist": &Schema{ + "blacklist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist", "blacklist", "purplelist"}, }, - "purplelist": &Schema{ + "purplelist": { Type: TypeBool, Optional: true, AllOf: []string{"whitelist", "blacklist", "purplelist"}, From e193bdbc248f19855988107b91abd6cf33695a54 Mon Sep 17 00:00:00 2001 From: Thomas Meckel Date: Thu, 9 Apr 2020 21:57:39 +0200 Subject: [PATCH 5/6] Renamed AllOf to RequiredWith and made RequiredWith usable with other property constraints; updated tests accordingly --- helper/schema/schema.go | 38 ++++---- helper/schema/schema_test.go | 162 +++++++++++++++++------------------ 2 files changed, 98 insertions(+), 102 deletions(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 13431b8f504..10e45dacf7b 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -224,12 +224,11 @@ type Schema struct { // AtLeastOneOf is a set of schema keys that, when set, at least one of // the keys in that list must be specified. // - // AllOf is a set of schema keys that must be set simultaneously. This - // setting conflicts with ExactlyOneOf or AtLeastOneOf + // RequiredWith is a set of schema keys that must be set simultaneously. ConflictsWith []string ExactlyOneOf []string AtLeastOneOf []string - AllOf []string + RequiredWith []string // When Deprecated is set, this attribute is deprecated. // @@ -777,13 +776,10 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro } } - if len(v.AllOf) > 0 { - if len(v.ExactlyOneOf) > 0 || len(v.AtLeastOneOf) > 0 { - return fmt.Errorf("AllOf cannot be used simultaneously with ExactlyOneOf or AtLeastOneOf") - } - err := checkKeysAgainstSchemaFlags(k, v.AllOf, topSchemaMap) + if len(v.RequiredWith) > 0 { + err := checkKeysAgainstSchemaFlags(k, v.RequiredWith, topSchemaMap) if err != nil { - return fmt.Errorf("AllOf: %+v", err) + return fmt.Errorf("RequiredWith: %+v", err) } } @@ -1412,7 +1408,13 @@ func (m schemaMap) validate( return nil, nil } - err := validateAllOfAttribute(k, schema, c) + if !schema.Required && !schema.Optional { + // This is a computed-only field + return nil, []error{fmt.Errorf( + "%q: this field cannot be set", k)} + } + + err := validateRequiredWithAttribute(k, schema, c) if err != nil { return nil, []error{err} } @@ -1427,12 +1429,6 @@ func (m schemaMap) validate( return nil, []error{err} } - if !schema.Required && !schema.Optional { - // This is a computed-only field - return nil, []error{fmt.Errorf( - "%q: this field cannot be set", k)} - } - // If the value is unknown then we can't validate it yet. // In particular, this avoids spurious type errors where downstream // validation code sees UnknownVariableValue as being just a string. @@ -1513,20 +1509,20 @@ func removeDuplicates(elements []string) []string { return result } -func validateAllOfAttribute( +func validateRequiredWithAttribute( k string, schema *Schema, c *terraform.ResourceConfig) error { - if len(schema.AllOf) == 0 { + if len(schema.RequiredWith) == 0 { return nil } - allKeys := removeDuplicates(append(schema.AllOf, k)) + allKeys := removeDuplicates(append(schema.RequiredWith, k)) sort.Strings(allKeys) - for _, allOfKey := range allKeys { - if _, ok := c.Get(allOfKey); !ok { + for _, key := range allKeys { + if _, ok := c.Get(key); !ok { return fmt.Errorf("%q: all of `%s` must be specified", k, strings.Join(allKeys, ",")) } } diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index f0adee924bc..7dcffef88d2 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -6636,7 +6636,7 @@ func TestValidateAtLeastOneOfAttributes(t *testing.T) { } } -func TestValidateAllOfAttributes(t *testing.T) { +func TestValidateRequiredWithAttributes(t *testing.T) { cases := map[string]struct { Key string Schema map[string]*Schema @@ -6648,14 +6648,14 @@ func TestValidateAllOfAttributes(t *testing.T) { Key: "whitelist", Schema: map[string]*Schema{ "whitelist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"blacklist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"blacklist"}, }, "blacklist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist"}, }, }, @@ -6670,14 +6670,14 @@ func TestValidateAllOfAttributes(t *testing.T) { Key: "whitelist", Schema: map[string]*Schema{ "whitelist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"blacklist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"blacklist"}, }, "blacklist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist"}, }, }, @@ -6691,14 +6691,14 @@ func TestValidateAllOfAttributes(t *testing.T) { Key: "whitelist", Schema: map[string]*Schema{ "whitelist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"blacklist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"blacklist"}, }, "blacklist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist"}, }, }, @@ -6710,19 +6710,19 @@ func TestValidateAllOfAttributes(t *testing.T) { Key: "whitelist", Schema: map[string]*Schema{ "whitelist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"purplelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"purplelist"}, }, "blacklist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist", "purplelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist", "purplelist"}, }, "purplelist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist"}, }, }, @@ -6737,19 +6737,19 @@ func TestValidateAllOfAttributes(t *testing.T) { Key: "whitelist", Schema: map[string]*Schema{ "whitelist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"blacklist", "purplelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"blacklist", "purplelist"}, }, "blacklist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist", "purplelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist", "purplelist"}, }, "purplelist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist", "blacklist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist", "blacklist"}, }, }, @@ -6765,19 +6765,19 @@ func TestValidateAllOfAttributes(t *testing.T) { Key: "whitelist", Schema: map[string]*Schema{ "whitelist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"blacklist", "purplelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"blacklist", "purplelist"}, }, "blacklist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist", "purplelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist", "purplelist"}, }, "purplelist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist", "blacklist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist", "blacklist"}, }, }, @@ -6791,19 +6791,19 @@ func TestValidateAllOfAttributes(t *testing.T) { Key: "whitelist", Schema: map[string]*Schema{ "whitelist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist", "blacklist", "purplelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist", "blacklist", "purplelist"}, }, "blacklist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist", "blacklist", "purplelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist", "blacklist", "purplelist"}, }, "purplelist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist", "blacklist", "purplelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist", "blacklist", "purplelist"}, }, }, @@ -6814,19 +6814,19 @@ func TestValidateAllOfAttributes(t *testing.T) { "Only Unknown Variable Value": { Schema: map[string]*Schema{ "whitelist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist", "blacklist", "purplelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist", "blacklist", "purplelist"}, }, "blacklist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist", "blacklist", "purplelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist", "blacklist", "purplelist"}, }, "purplelist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist", "blacklist", "purplelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist", "blacklist", "purplelist"}, }, }, @@ -6840,16 +6840,16 @@ func TestValidateAllOfAttributes(t *testing.T) { "only unknown list value": { Schema: map[string]*Schema{ "whitelist": { - Type: TypeList, - Optional: true, - Elem: &Schema{Type: TypeString}, - AllOf: []string{"whitelist", "blacklist"}, + Type: TypeList, + Optional: true, + Elem: &Schema{Type: TypeString}, + RequiredWith: []string{"whitelist", "blacklist"}, }, "blacklist": { - Type: TypeList, - Optional: true, - Elem: &Schema{Type: TypeString}, - AllOf: []string{"whitelist", "blacklist"}, + Type: TypeList, + Optional: true, + Elem: &Schema{Type: TypeString}, + RequiredWith: []string{"whitelist", "blacklist"}, }, }, @@ -6863,19 +6863,19 @@ func TestValidateAllOfAttributes(t *testing.T) { "Unknown Variable Value and Known Value": { Schema: map[string]*Schema{ "whitelist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist", "blacklist", "purplelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist", "blacklist", "purplelist"}, }, "blacklist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist", "blacklist", "purplelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist", "blacklist", "purplelist"}, }, "purplelist": { - Type: TypeBool, - Optional: true, - AllOf: []string{"whitelist", "blacklist", "purplelist"}, + Type: TypeBool, + Optional: true, + RequiredWith: []string{"whitelist", "blacklist", "purplelist"}, }, }, From e65a621e6a1fe07cfb8bcdde2d0c37c3f138945d Mon Sep 17 00:00:00 2001 From: Thomas Meckel Date: Sat, 11 Apr 2020 16:33:24 +0200 Subject: [PATCH 6/6] Moved validateExactlyOneAttribute and validateAtLeastOneAttribute back to their old positions as requested by review in helper/schema/schema.go --- helper/schema/schema.go | 22 +++++++++++----------- helper/schema/schema_test.go | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 10e45dacf7b..881b2ebba2c 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -1400,6 +1400,16 @@ func (m schemaMap) validate( ok = raw != nil } + err := validateExactlyOneAttribute(k, schema, c) + if err != nil { + return nil, []error{err} + } + + err = validateAtLeastOneAttribute(k, schema, c) + if err != nil { + return nil, []error{err} + } + if !ok { if schema.Required { return nil, []error{fmt.Errorf( @@ -1414,17 +1424,7 @@ func (m schemaMap) validate( "%q: this field cannot be set", k)} } - err := validateRequiredWithAttribute(k, schema, c) - if err != nil { - return nil, []error{err} - } - - err = validateExactlyOneAttribute(k, schema, c) - if err != nil { - return nil, []error{err} - } - - err = validateAtLeastOneAttribute(k, schema, c) + err = validateRequiredWithAttribute(k, schema, c) if err != nil { return nil, []error{err} } diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 7dcffef88d2..e94c5164b59 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -6537,7 +6537,7 @@ func TestValidateAtLeastOneOfAttributes(t *testing.T) { }, Config: map[string]interface{}{}, - Err: false, + Err: true, }, "Only Unknown Variable Value": {