Merge pull request #342 from tmeckel/allof

RequiredWith schema check and changes in validation for optional schema fields
hashicorp · Apr 14, 2020 · e1ddbee · e1ddbee
2 parents aa2faf7 + e65a621
commit e1ddbee
Show file tree

Hide file tree

Showing 2 changed files with 307 additions and 0 deletions.
diff --git a/helper/schema/schema.go b/helper/schema/schema.go
@@ -223,9 +223,12 @@ 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.
+	//
+	// RequiredWith is a set of schema keys that must be set simultaneously.
 	ConflictsWith []string
 	ExactlyOneOf  []string
 	AtLeastOneOf  []string
+	RequiredWith  []string
 
 	// When Deprecated is set, this attribute is deprecated.
 	//
@@ -773,6 +776,13 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro
 			}
 		}
 
+		if len(v.RequiredWith) > 0 {
+			err := checkKeysAgainstSchemaFlags(k, v.RequiredWith, topSchemaMap)
+			if err != nil {
+				return fmt.Errorf("RequiredWith: %+v", err)
+			}
+		}
+
 		if len(v.ExactlyOneOf) > 0 {
 			err := checkKeysAgainstSchemaFlags(k, v.ExactlyOneOf, topSchemaMap)
 			if err != nil {
@@ -1414,6 +1424,11 @@ func (m schemaMap) validate(
 			"%q: this field cannot be set", k)}
 	}
 
+	err = validateRequiredWithAttribute(k, schema, c)
+	if err != nil {
+		return nil, []error{err}
+	}
+
 	// 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.
@@ -1494,6 +1509,27 @@ func removeDuplicates(elements []string) []string {
 	return result
 }
 
+func validateRequiredWithAttribute(
+	k string,
+	schema *Schema,
+	c *terraform.ResourceConfig) error {
+
+	if len(schema.RequiredWith) == 0 {
+		return nil
+	}
+
+	allKeys := removeDuplicates(append(schema.RequiredWith, k))
+	sort.Strings(allKeys)
+
+	for _, key := range allKeys {
+		if _, ok := c.Get(key); !ok {
+			return fmt.Errorf("%q: all of `%s` must be specified", k, strings.Join(allKeys, ","))
+		}
+	}
+
+	return nil
+}
+
 func validateExactlyOneAttribute(
 	k string,
 	schema *Schema,

diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go
@@ -6635,3 +6635,274 @@ func TestValidateAtLeastOneOfAttributes(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateRequiredWithAttributes(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": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"blacklist"},
+				},
+				"blacklist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"whitelist"},
+				},
+			},
+
+			Config: map[string]interface{}{
+				"whitelist": true,
+				"blacklist": true,
+			},
+			Err: false,
+		},
+
+		"one attributes specified": {
+			Key: "whitelist",
+			Schema: map[string]*Schema{
+				"whitelist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"blacklist"},
+				},
+				"blacklist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"whitelist"},
+				},
+			},
+
+			Config: map[string]interface{}{
+				"whitelist": true,
+			},
+			Err: true,
+		},
+
+		"no attributes specified": {
+			Key: "whitelist",
+			Schema: map[string]*Schema{
+				"whitelist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"blacklist"},
+				},
+				"blacklist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"whitelist"},
+				},
+			},
+
+			Config: map[string]interface{}{},
+			Err:    false,
+		},
+
+		"two attributes of three specified": {
+			Key: "whitelist",
+			Schema: map[string]*Schema{
+				"whitelist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"purplelist"},
+				},
+				"blacklist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"whitelist", "purplelist"},
+				},
+				"purplelist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"whitelist"},
+				},
+			},
+
+			Config: map[string]interface{}{
+				"whitelist":  true,
+				"purplelist": true,
+			},
+			Err: false,
+		},
+
+		"three attributes of three specified": {
+			Key: "whitelist",
+			Schema: map[string]*Schema{
+				"whitelist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"blacklist", "purplelist"},
+				},
+				"blacklist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"whitelist", "purplelist"},
+				},
+				"purplelist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []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": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"blacklist", "purplelist"},
+				},
+				"blacklist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"whitelist", "purplelist"},
+				},
+				"purplelist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"whitelist", "blacklist"},
+				},
+			},
+
+			Config: map[string]interface{}{
+				"purplelist": true,
+			},
+			Err: true,
+		},
+
+		"no attributes of three specified": {
+			Key: "whitelist",
+			Schema: map[string]*Schema{
+				"whitelist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"whitelist", "blacklist", "purplelist"},
+				},
+				"blacklist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"whitelist", "blacklist", "purplelist"},
+				},
+				"purplelist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"whitelist", "blacklist", "purplelist"},
+				},
+			},
+
+			Config: map[string]interface{}{},
+			Err:    false,
+		},
+
+		"Only Unknown Variable Value": {
+			Schema: map[string]*Schema{
+				"whitelist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"whitelist", "blacklist", "purplelist"},
+				},
+				"blacklist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"whitelist", "blacklist", "purplelist"},
+				},
+				"purplelist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"whitelist", "blacklist", "purplelist"},
+				},
+			},
+
+			Config: map[string]interface{}{
+				"whitelist": hcl2shim.UnknownVariableValue,
+			},
+
+			Err: true,
+		},
+
+		"only unknown list value": {
+			Schema: map[string]*Schema{
+				"whitelist": {
+					Type:         TypeList,
+					Optional:     true,
+					Elem:         &Schema{Type: TypeString},
+					RequiredWith: []string{"whitelist", "blacklist"},
+				},
+				"blacklist": {
+					Type:         TypeList,
+					Optional:     true,
+					Elem:         &Schema{Type: TypeString},
+					RequiredWith: []string{"whitelist", "blacklist"},
+				},
+			},
+
+			Config: map[string]interface{}{
+				"whitelist": []interface{}{hcl2shim.UnknownVariableValue},
+			},
+
+			Err: true,
+		},
+
+		"Unknown Variable Value and Known Value": {
+			Schema: map[string]*Schema{
+				"whitelist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"whitelist", "blacklist", "purplelist"},
+				},
+				"blacklist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []string{"whitelist", "blacklist", "purplelist"},
+				},
+				"purplelist": {
+					Type:         TypeBool,
+					Optional:     true,
+					RequiredWith: []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()
+			}
+		})
+	}
+}