Skip to content

Commit

Permalink
helper/schema: Adjust checkKeysAgainstSchemaFlags from require to all…
Browse files Browse the repository at this point in the history
…ow, add InternalValidate unit testing for RequiredWhen

Reference: hashicorp#418 (review)
  • Loading branch information
bflad committed Apr 30, 2020
1 parent ddd2b16 commit 40c7f00
Show file tree
Hide file tree
Showing 2 changed files with 271 additions and 18 deletions.
18 changes: 4 additions & 14 deletions helper/schema/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro
}

if len(v.RequiredWith) > 0 {
err := checkKeysAgainstSchemaFlags(k, v.RequiredWith, topSchemaMap, v, false)
err := checkKeysAgainstSchemaFlags(k, v.RequiredWith, topSchemaMap, v, true)
if err != nil {
return fmt.Errorf("RequiredWith: %+v", err)
}
Expand Down Expand Up @@ -860,9 +860,7 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro
return nil
}

func checkKeysAgainstSchemaFlags(k string, keys []string, topSchemaMap schemaMap, self *Schema, requireSelfReference bool) error {
var foundSelfReference bool

func checkKeysAgainstSchemaFlags(k string, keys []string, topSchemaMap schemaMap, self *Schema, allowSelfReference bool) error {
for _, key := range keys {
parts := strings.Split(key, ".")
sm := topSchemaMap
Expand Down Expand Up @@ -901,12 +899,8 @@ func checkKeysAgainstSchemaFlags(k string, keys []string, topSchemaMap schemaMap
return fmt.Errorf("%s cannot find target attribute (%s), sm: %#v", k, key, sm)
}

if target == self {
if !requireSelfReference {
return fmt.Errorf("%s cannot reference self (%s)", k, key)
}

foundSelfReference = true
if target == self && !allowSelfReference {
return fmt.Errorf("%s cannot reference self (%s)", k, key)
}

if target.Required {
Expand All @@ -918,10 +912,6 @@ func checkKeysAgainstSchemaFlags(k string, keys []string, topSchemaMap schemaMap
}
}

if requireSelfReference && !foundSelfReference {
return fmt.Errorf("%s must reference self for AtLeastOneOf/ExactlyOneOf", k)
}

return nil
}

Expand Down
271 changes: 267 additions & 4 deletions helper/schema/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3636,7 +3636,7 @@ func TestSchemaMap_InternalValidate(t *testing.T) {
AtLeastOneOf: []string{"config_block_attr.0.nested_attr"},
},
},
true,
false,
},

"AtLeastOneOf list index syntax with list configuration block missing attribute": {
Expand Down Expand Up @@ -3844,7 +3844,7 @@ func TestSchemaMap_InternalValidate(t *testing.T) {
AtLeastOneOf: []string{"map_attr"},
},
},
true,
false,
},

"AtLeastOneOf string syntax with self reference": {
Expand Down Expand Up @@ -4161,7 +4161,7 @@ func TestSchemaMap_InternalValidate(t *testing.T) {
ExactlyOneOf: []string{"config_block_attr.0.nested_attr"},
},
},
true,
false,
},

"ExactlyOneOf list index syntax with list configuration block missing attribute": {
Expand Down Expand Up @@ -4369,7 +4369,7 @@ func TestSchemaMap_InternalValidate(t *testing.T) {
ExactlyOneOf: []string{"map_attr"},
},
},
true,
false,
},

"ExactlyOneOf string syntax with self reference": {
Expand All @@ -4383,6 +4383,269 @@ func TestSchemaMap_InternalValidate(t *testing.T) {
false,
},

"RequiredWith list index syntax with self reference": {
map[string]*Schema{
"config_block_attr": {
Type: TypeList,
Optional: true,
MaxItems: 1,
Elem: &Resource{
Schema: map[string]*Schema{
"nested_attr": {
Type: TypeString,
Optional: true,
RequiredWith: []string{"config_block_attr.0.nested_attr"},
},
},
},
},
},
false,
},

"RequiredWith list index syntax with list configuration block existing attribute": {
map[string]*Schema{
"config_block_attr": {
Type: TypeList,
Optional: true,
MaxItems: 1,
Elem: &Resource{
Schema: map[string]*Schema{
"nested_attr": {
Type: TypeString,
Optional: true,
},
},
},
},
"test": {
Type: TypeBool,
Optional: true,
RequiredWith: []string{"config_block_attr.0.nested_attr"},
},
},
false,
},

"RequiredWith list index syntax with list configuration block missing attribute": {
map[string]*Schema{
"config_block_attr": {
Type: TypeList,
Optional: true,
Elem: &Resource{
Schema: map[string]*Schema{
"nested_attr": {
Type: TypeString,
Optional: true,
},
},
},
},
"test": {
Type: TypeBool,
Optional: true,
RequiredWith: []string{"config_block_attr.0.missing_attr"},
},
},
true,
},

"RequiredWith list index syntax with list configuration block missing MaxItems": {
map[string]*Schema{
"config_block_attr": {
Type: TypeList,
Optional: true,
Elem: &Resource{
Schema: map[string]*Schema{
"nested_attr": {
Type: TypeString,
Optional: true,
},
},
},
},
"test": {
Type: TypeBool,
Optional: true,
RequiredWith: []string{"config_block_attr.0.missing_attr"},
},
},
true,
},

"RequiredWith list index syntax with set configuration block existing attribute": {
map[string]*Schema{
"config_block_attr": {
Type: TypeSet,
Optional: true,
Elem: &Resource{
Schema: map[string]*Schema{
"nested_attr": {
Type: TypeString,
Optional: true,
},
},
},
},
"test": {
Type: TypeBool,
Optional: true,
RequiredWith: []string{"config_block_attr.0.nested_attr"},
},
},
true,
},

"RequiredWith list index syntax with set configuration block missing attribute": {
map[string]*Schema{
"config_block_attr": {
Type: TypeSet,
Optional: true,
Elem: &Resource{
Schema: map[string]*Schema{
"nested_attr": {
Type: TypeString,
Optional: true,
},
},
},
},
"test": {
Type: TypeBool,
Optional: true,
RequiredWith: []string{"config_block_attr.0.missing_attr"},
},
},
true,
},

"RequiredWith map key syntax with list configuration block existing attribute": {
map[string]*Schema{
"config_block_attr": {
Type: TypeList,
Optional: true,
Elem: &Resource{
Schema: map[string]*Schema{
"nested_attr": {
Type: TypeString,
Optional: true,
},
},
},
},
"test": {
Type: TypeBool,
Optional: true,
RequiredWith: []string{"config_block_attr.nested_attr"},
},
},
true,
},

"RequiredWith map key syntax with list configuration block self reference": {
map[string]*Schema{
"config_block_attr": {
Type: TypeList,
Optional: true,
Elem: &Resource{
Schema: map[string]*Schema{
"nested_attr": {
Type: TypeString,
Optional: true,
RequiredWith: []string{"config_block_attr.nested_attr"},
},
},
},
},
},
true,
},

"RequiredWith map key syntax with set configuration block existing attribute": {
map[string]*Schema{
"config_block_attr": {
Type: TypeSet,
Optional: true,
Elem: &Resource{
Schema: map[string]*Schema{
"nested_attr": {
Type: TypeString,
Optional: true,
},
},
},
},
"test": {
Type: TypeBool,
Optional: true,
RequiredWith: []string{"config_block_attr.nested_attr"},
},
},
true,
},

"RequiredWith map key syntax with set configuration block self reference": {
map[string]*Schema{
"config_block_attr": {
Type: TypeSet,
Optional: true,
Elem: &Resource{
Schema: map[string]*Schema{
"nested_attr": {
Type: TypeString,
Optional: true,
RequiredWith: []string{"config_block_attr.nested_attr"},
},
},
},
},
},
true,
},

"RequiredWith map key syntax with map attribute": {
map[string]*Schema{
"map_attr": {
Type: TypeMap,
Optional: true,
Elem: &Schema{Type: TypeString},
},
"test": {
Type: TypeBool,
Optional: true,
RequiredWith: []string{"map_attr.some_key"},
},
},
true,
},

"RequiredWith string syntax with map attribute": {
map[string]*Schema{
"map_attr": {
Type: TypeMap,
Optional: true,
Elem: &Schema{Type: TypeString},
},
"test": {
Type: TypeBool,
Optional: true,
RequiredWith: []string{"map_attr"},
},
},
false,
},

"RequiredWith string syntax with self reference": {
map[string]*Schema{
"test": {
Type: TypeBool,
Optional: true,
RequiredWith: []string{"test"},
},
},
false,
},

"Sub-resource invalid": {
map[string]*Schema{
"foo": {
Expand Down

0 comments on commit 40c7f00

Please sign in to comment.