helper/schema: Skip validation of unknown values

With the introduction of explicit "null" in 0.12 it's possible for a value
that is unknown during plan to become a known null during apply, so we
need to slightly weaken our validation rules to accommodate that, in
particular skipping the validation of conflicting attributes if the result
could potentially be valid after the unknown values become known.

This change is in the codepath that is common to both 0.12 and 0.11
callers, but that's safe because 0.11 re-runs validation during the apply
step and so will still catch problems here, albeit in the apply step
rather than in the plan step, thus matching the 0.12 behavior. This new
behavior is a superset of the old in the sense that everything that was
valid before is still valid.

The implementation here also causes us to skip all other validation for
an attribute whose value is unknown. Most of the downstream validation
functions handle this directly anyway, but again this doesn't add any new
failure cases, and should clean up some of the rough edges we've seen with
unknown values in 0.11 once people upgrade to 0.12-compatible providers.
Any issues we now short-circuit during planning will still be caught
during apply.

While working on this I found that the existing "Not a list" test was not
actually testing the correct behavior, so this also includes a tweak to
that to ensure that it really is checking the "should be a list" path
rather than the "cannot be set" codepath it was inadvertently testing
before.
This commit is contained in:
Martin Atkins 2019-01-04 13:44:03 -08:00
parent 148f6be244
commit 06acc3f6c8
2 changed files with 134 additions and 5 deletions

View File

@ -1266,6 +1266,13 @@ func (m schemaMap) validate(
"%q: this field cannot be set", k)}
}
if raw == config.UnknownVariableValue {
// 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.
return nil, nil
}
err := m.validateConflictingAttributes(k, schema, c)
if err != nil {
return nil, []error{err}
@ -1283,10 +1290,15 @@ func (m schemaMap) validateConflictingAttributes(
return nil
}
for _, conflicting_key := range schema.ConflictsWith {
if _, ok := c.Get(conflicting_key); ok {
for _, conflictingKey := range schema.ConflictsWith {
if raw, ok := c.Get(conflictingKey); ok {
if raw == config.UnknownVariableValue {
// An unknown value might become unset (null) once known, so
// we must defer validation until it's known.
continue
}
return fmt.Errorf(
"%q: conflicts with %s", k, conflicting_key)
"%q: conflicts with %s", k, conflictingKey)
}
}

View File

@ -4356,10 +4356,11 @@ func TestSchemaMap_Validate(t *testing.T) {
Err: true,
},
"Not a list": {
"Not a list nested block": {
Schema: map[string]*Schema{
"ingress": &Schema{
Type: TypeList,
Optional: true,
Elem: &Resource{
Schema: map[string]*Schema{
"from": &Schema{
@ -4376,6 +4377,48 @@ func TestSchemaMap_Validate(t *testing.T) {
},
Err: true,
Errors: []error{
fmt.Errorf(`ingress: should be a list`),
},
},
"Not a list primitive": {
Schema: map[string]*Schema{
"strings": &Schema{
Type: TypeList,
Optional: true,
Elem: &Schema{
Type: TypeString,
},
},
},
Config: map[string]interface{}{
"strings": "foo",
},
Err: true,
Errors: []error{
fmt.Errorf(`strings: should be a list`),
},
},
"Unknown list": {
Schema: map[string]*Schema{
"strings": &Schema{
Type: TypeList,
Optional: true,
Elem: &Schema{
Type: TypeString,
},
},
},
Config: map[string]interface{}{
"strings": config.UnknownVariableValue,
},
Err: false,
},
"Required sub-resource field": {
@ -4870,6 +4913,80 @@ func TestSchemaMap_Validate(t *testing.T) {
},
},
"Conflicting attributes okay when unknown 1": {
Schema: map[string]*Schema{
"whitelist": &Schema{
Type: TypeString,
Optional: true,
},
"blacklist": &Schema{
Type: TypeString,
Optional: true,
ConflictsWith: []string{"whitelist"},
},
},
Config: map[string]interface{}{
"whitelist": "white-val",
"blacklist": config.UnknownVariableValue,
},
Err: false,
},
"Conflicting attributes okay when unknown 2": {
Schema: map[string]*Schema{
"whitelist": &Schema{
Type: TypeString,
Optional: true,
},
"blacklist": &Schema{
Type: TypeString,
Optional: true,
ConflictsWith: []string{"whitelist"},
},
},
Config: map[string]interface{}{
"whitelist": config.UnknownVariableValue,
"blacklist": "black-val",
},
Err: false,
},
"Conflicting attributes generate error even if one is unknown": {
Schema: map[string]*Schema{
"whitelist": &Schema{
Type: TypeString,
Optional: true,
ConflictsWith: []string{"blacklist", "greenlist"},
},
"blacklist": &Schema{
Type: TypeString,
Optional: true,
ConflictsWith: []string{"whitelist", "greenlist"},
},
"greenlist": &Schema{
Type: TypeString,
Optional: true,
ConflictsWith: []string{"whitelist", "blacklist"},
},
},
Config: map[string]interface{}{
"whitelist": config.UnknownVariableValue,
"blacklist": "black-val",
"greenlist": "green-val",
},
Err: true,
Errors: []error{
fmt.Errorf("\"blacklist\": conflicts with greenlist"),
fmt.Errorf("\"greenlist\": conflicts with blacklist"),
},
},
"Required attribute & undefined conflicting optional are good": {
Schema: map[string]*Schema{
"required_att": &Schema{