From d49c14b1388adb05b8eb60a5e7bb9d9cd3d4056d Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Tue, 7 Jan 2020 12:05:57 +0000 Subject: [PATCH 1/3] schema/ExactlyOneOf: Fix handling of unknowns in complex types --- helper/schema/schema.go | 14 +- helper/schema/schema_test.go | 577 +++++++++++++++++++++++++++++++++++ 2 files changed, 584 insertions(+), 7 deletions(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index c6061da9ee0..0cd64635e7e 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -1508,13 +1508,13 @@ func validateExactlyOneAttribute( specified := make([]string, 0) unknownVariableValueCount := 0 for _, exactlyOneOfKey := range allKeys { - if raw, ok := c.Get(exactlyOneOfKey); ok { - if raw == hcl2shim.UnknownVariableValue { - // This aims to do a best effort check that at least one value is specified whether - // it's known or not. - unknownVariableValueCount++ - continue - } + if c.IsComputed(exactlyOneOfKey) { + unknownVariableValueCount++ + continue + } + + _, ok := c.Get(exactlyOneOfKey) + if ok { specified = append(specified, exactlyOneOfKey) } } diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index b14e61d218e..78f7eae37a0 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -5765,6 +5765,583 @@ func TestValidateExactlyOneOfAttributes(t *testing.T) { }, Err: true, }, + + "unknown list values": { + Key: "allow", + Schema: map[string]*Schema{ + "allow": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "protocol": { + Type: TypeString, + Required: true, + }, + "ports": { + Type: TypeList, + Optional: true, + Elem: &Schema{ + Type: TypeString, + }, + }, + }, + }, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "deny": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "protocol": { + Type: TypeString, + Required: true, + }, + "ports": { + Type: TypeList, + Optional: true, + Elem: &Schema{ + Type: TypeString, + }, + }, + }, + }, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "purplelist": &Schema{ + Type: TypeString, + Optional: true, + }, + }, + Config: map[string]interface{}{ + "allow": []interface{}{map[string]interface{}{ + "ports": hcl2shim.UnknownVariableValue, + "protocol": hcl2shim.UnknownVariableValue, + }}, + "deny": []interface{}{map[string]interface{}{ + "ports": hcl2shim.UnknownVariableValue, + "protocol": hcl2shim.UnknownVariableValue, + }}, + "purplelist": "blah", + }, + Err: false, + }, + + // This should probably fail, but we let it pass and rely on 2nd + // validation phase when unknowns become known, which will then fail. + "partially known list values": { + Key: "allow", + Schema: map[string]*Schema{ + "allow": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "protocol": { + Type: TypeString, + Required: true, + }, + "ports": { + Type: TypeList, + Optional: true, + Elem: &Schema{ + Type: TypeString, + }, + }, + }, + }, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "deny": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "protocol": { + Type: TypeString, + Required: true, + }, + "ports": { + Type: TypeList, + Optional: true, + Elem: &Schema{ + Type: TypeString, + }, + }, + }, + }, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "purplelist": &Schema{ + Type: TypeString, + Optional: true, + }, + }, + Config: map[string]interface{}{ + "allow": []interface{}{map[string]interface{}{ + "ports": hcl2shim.UnknownVariableValue, + "protocol": "TCP", + }}, + "deny": []interface{}{map[string]interface{}{ + "ports": hcl2shim.UnknownVariableValue, + "protocol": "TCP", + }}, + "purplelist": "blah", + }, + Err: false, + }, + + "known list values": { + Key: "allow", + Schema: map[string]*Schema{ + "allow": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "protocol": { + Type: TypeString, + Required: true, + }, + "ports": { + Type: TypeList, + Optional: true, + Elem: &Schema{ + Type: TypeString, + }, + }, + }, + }, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "deny": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "protocol": { + Type: TypeString, + Required: true, + }, + "ports": { + Type: TypeList, + Optional: true, + Elem: &Schema{ + Type: TypeString, + }, + }, + }, + }, + ExactlyOneOf: []string{"allow", "deny"}, + }, + }, + Config: map[string]interface{}{ + "allow": []interface{}{map[string]interface{}{ + "ports": []interface{}{"80"}, + "protocol": "TCP", + }}, + "deny": []interface{}{map[string]interface{}{ + "ports": []interface{}{"80"}, + "protocol": "TCP", + }}, + }, + Err: true, + }, + + "wholly unknown set values": { + Key: "allow", + Schema: map[string]*Schema{ + "allow": &Schema{ + Type: TypeSet, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "protocol": { + Type: TypeString, + Required: true, + }, + "ports": { + Type: TypeList, + Optional: true, + Elem: &Schema{ + Type: TypeString, + }, + }, + }, + }, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "deny": &Schema{ + Type: TypeSet, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "protocol": { + Type: TypeString, + Required: true, + }, + "ports": { + Type: TypeList, + Optional: true, + Elem: &Schema{ + Type: TypeString, + }, + }, + }, + }, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "purplelist": &Schema{ + Type: TypeString, + Optional: true, + }, + }, + Config: map[string]interface{}{ + "allow": []interface{}{map[string]interface{}{ + "ports": hcl2shim.UnknownVariableValue, + "protocol": hcl2shim.UnknownVariableValue, + }}, + "deny": []interface{}{map[string]interface{}{ + "ports": hcl2shim.UnknownVariableValue, + "protocol": hcl2shim.UnknownVariableValue, + }}, + "purplelist": "blah", + }, + Err: false, + }, + + // This should probably fail, but we let it pass and rely on 2nd + // validation phase when unknowns become known, which will then fail. + "partially known set values": { + Key: "allow", + Schema: map[string]*Schema{ + "allow": &Schema{ + Type: TypeSet, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "protocol": { + Type: TypeString, + Required: true, + }, + "ports": { + Type: TypeList, + Optional: true, + Elem: &Schema{ + Type: TypeString, + }, + }, + }, + }, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "deny": &Schema{ + Type: TypeSet, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "protocol": { + Type: TypeString, + Required: true, + }, + "ports": { + Type: TypeList, + Optional: true, + Elem: &Schema{ + Type: TypeString, + }, + }, + }, + }, + ExactlyOneOf: []string{"allow", "deny"}, + }, + }, + Config: map[string]interface{}{ + "allow": []interface{}{map[string]interface{}{ + "ports": hcl2shim.UnknownVariableValue, + "protocol": "TCP", + }}, + "deny": []interface{}{map[string]interface{}{ + "ports": hcl2shim.UnknownVariableValue, + "protocol": "UDP", + }}, + }, + Err: false, + }, + + "known set values": { + Key: "allow", + Schema: map[string]*Schema{ + "allow": &Schema{ + Type: TypeSet, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "protocol": { + Type: TypeString, + Required: true, + }, + "ports": { + Type: TypeList, + Optional: true, + Elem: &Schema{ + Type: TypeString, + }, + }, + }, + }, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "deny": &Schema{ + Type: TypeSet, + Optional: true, + Elem: &Resource{ + Schema: map[string]*Schema{ + "protocol": { + Type: TypeString, + Required: true, + }, + "ports": { + Type: TypeList, + Optional: true, + Elem: &Schema{ + Type: TypeString, + }, + }, + }, + }, + ExactlyOneOf: []string{"allow", "deny"}, + }, + }, + Config: map[string]interface{}{ + "allow": []interface{}{map[string]interface{}{ + "ports": []interface{}{"80"}, + "protocol": "TCP", + }}, + "deny": []interface{}{map[string]interface{}{ + "ports": []interface{}{"80"}, + "protocol": "TCP", + }}, + }, + Err: true, + }, + + "wholly unknown simple lists": { + Key: "allow", + Schema: map[string]*Schema{ + "allow": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Schema{Type: TypeString}, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "deny": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Schema{Type: TypeString}, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "purplelist": &Schema{ + Type: TypeString, + Optional: true, + }, + }, + Config: map[string]interface{}{ + "allow": []interface{}{ + hcl2shim.UnknownVariableValue, + hcl2shim.UnknownVariableValue, + }, + "deny": []interface{}{ + hcl2shim.UnknownVariableValue, + hcl2shim.UnknownVariableValue, + }, + "purplelist": "blah", + }, + Err: false, + }, + + // This should probably fail, but we let it pass and rely on 2nd + // validation phase when unknowns become known, which will then fail. + "partially known simple lists": { + Key: "allow", + Schema: map[string]*Schema{ + "allow": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Schema{Type: TypeString}, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "deny": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Schema{Type: TypeString}, + ExactlyOneOf: []string{"allow", "deny"}, + }, + }, + Config: map[string]interface{}{ + "allow": []interface{}{ + hcl2shim.UnknownVariableValue, + "known", + }, + "deny": []interface{}{ + hcl2shim.UnknownVariableValue, + "known", + }, + }, + Err: false, + }, + + "known simple lists": { + Key: "allow", + Schema: map[string]*Schema{ + "allow": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Schema{Type: TypeString}, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "deny": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Schema{Type: TypeString}, + ExactlyOneOf: []string{"allow", "deny"}, + }, + }, + Config: map[string]interface{}{ + "allow": []interface{}{ + "blah", + "known", + }, + "deny": []interface{}{ + "known", + }, + }, + Err: true, + }, + + "wholly unknown map keys and values": { + Key: "allow", + Schema: map[string]*Schema{ + "allow": &Schema{ + Type: TypeMap, + Optional: true, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "deny": &Schema{ + Type: TypeList, + Optional: true, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "purplelist": &Schema{ + Type: TypeString, + Optional: true, + }, + }, + Config: map[string]interface{}{ + "allow": map[string]interface{}{ + hcl2shim.UnknownVariableValue: hcl2shim.UnknownVariableValue, + }, + "deny": map[string]interface{}{ + hcl2shim.UnknownVariableValue: hcl2shim.UnknownVariableValue, + }, + "purplelist": "blah", + }, + Err: false, + }, + + // This should probably fail, but we let it pass and rely on 2nd + // validation phase when unknowns become known, which will then fail. + "wholly unknown map values": { + Key: "allow", + Schema: map[string]*Schema{ + "allow": &Schema{ + Type: TypeMap, + Optional: true, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "deny": &Schema{ + Type: TypeList, + Optional: true, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "purplelist": &Schema{ + Type: TypeString, + Optional: true, + }, + }, + Config: map[string]interface{}{ + "allow": map[string]interface{}{ + "key": hcl2shim.UnknownVariableValue, + }, + "deny": map[string]interface{}{ + "key": hcl2shim.UnknownVariableValue, + }, + "purplelist": "blah", + }, + Err: false, + }, + + // This should probably fail, but we let it pass and rely on 2nd + // validation phase when unknowns become known, which will then fail. + "partially known maps": { + Key: "allow", + Schema: map[string]*Schema{ + "allow": &Schema{ + Type: TypeMap, + Optional: true, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "deny": &Schema{ + Type: TypeList, + Optional: true, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "purplelist": &Schema{ + Type: TypeString, + Optional: true, + }, + }, + Config: map[string]interface{}{ + "allow": map[string]interface{}{ + "first": "value", + "second": hcl2shim.UnknownVariableValue, + }, + "deny": map[string]interface{}{ + "first": "value", + "second": hcl2shim.UnknownVariableValue, + }, + "purplelist": "blah", + }, + Err: false, + }, + + "known maps": { + Key: "allow", + Schema: map[string]*Schema{ + "allow": &Schema{ + Type: TypeMap, + Optional: true, + ExactlyOneOf: []string{"allow", "deny"}, + }, + "deny": &Schema{ + Type: TypeList, + Optional: true, + ExactlyOneOf: []string{"allow", "deny"}, + }, + }, + Config: map[string]interface{}{ + "allow": map[string]interface{}{ + "first": "value", + "second": "blah", + }, + "deny": map[string]interface{}{ + "first": "value", + "second": "boo", + }, + }, + Err: true, + }, } for tn, tc := range cases { From 4518ddca178b2d4e340a9b688e77d1194ac28214 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Tue, 7 Jan 2020 14:42:13 +0000 Subject: [PATCH 2/3] add test for ConflictsWith --- helper/schema/schema_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 78f7eae37a0..db6ba36dd08 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -4854,6 +4854,29 @@ func TestSchemaMap_Validate(t *testing.T) { Err: false, }, + "Conflicting list attributes okay when unknown 1": { + Schema: map[string]*Schema{ + "whitelist": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Schema{Type: TypeString}, + }, + "blacklist": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Schema{Type: TypeString}, + ConflictsWith: []string{"whitelist"}, + }, + }, + + Config: map[string]interface{}{ + "whitelist": []interface{}{"white-val"}, + "blacklist": []interface{}{hcl2shim.UnknownVariableValue}, + }, + + Err: false, + }, + "Conflicting attributes okay when unknown 2": { Schema: map[string]*Schema{ "whitelist": &Schema{ From f85f24ccd8cb83a292456ed149b34c6f27e31274 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Tue, 7 Jan 2020 14:45:37 +0000 Subject: [PATCH 3/3] add test for AtLeastOneOf --- helper/schema/schema_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index db6ba36dd08..5068e86ba65 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -6566,6 +6566,29 @@ func TestValidateAtLeastOneOfAttributes(t *testing.T) { Err: false, }, + "only unknown list value": { + Schema: map[string]*Schema{ + "whitelist": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Schema{Type: TypeString}, + AtLeastOneOf: []string{"whitelist", "blacklist"}, + }, + "blacklist": &Schema{ + Type: TypeList, + Optional: true, + Elem: &Schema{Type: TypeString}, + AtLeastOneOf: []string{"whitelist", "blacklist"}, + }, + }, + + Config: map[string]interface{}{ + "whitelist": []interface{}{hcl2shim.UnknownVariableValue}, + }, + + Err: false, + }, + "Unknown Variable Value and Known Value": { Schema: map[string]*Schema{ "whitelist": &Schema{