Skip to content

Commit

Permalink
helper/schema: Adjusted validation error messages
Browse files Browse the repository at this point in the history 
Some of the validation error messages were written with the provider
developer audience rather than the end-user audience in mind, and so in
helping various folks in the community forum, etc I've occasionally seen
folks be confused as to what these messages mean and what to do about
them.

Since I was here anyway I also did some general review of the terminology
used and adjusted some things that were not necessarily confusing but that
were using different terminology than Terraform Core would use for a
similar problem. However, for many of them I didn't change them heavily
because in many cases these validation errors are masked by equivalent
schema-based validation in Terraform Core itself anyway, and so these can
often be seen only by Terraform v0.10/v0.11 users.
  • Loading branch information
@apparentlymart
apparentlymart committed Apr 27, 2021
1 parent e512e37 commit 576f351
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 32 deletions.
34 changes: 17 additions & 17 deletions helper/schema/schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1439,7 +1439,7 @@ func (m schemaMap) validate(
if err != nil {
return append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "Loading Default",
Summary: "Failed to determine default value",
Detail: err.Error(),
AttributePath: path,
})
Expand All @@ -1453,7 +1453,7 @@ func (m schemaMap) validate(
if err != nil {
return append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "ExactlyOne",
Summary: "Invalid combination of arguments",
Detail: err.Error(),
AttributePath: path,
})
Expand All @@ -1463,7 +1463,7 @@ func (m schemaMap) validate(
if err != nil {
return append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "AtLeastOne",
Summary: "Missing required argument",
Detail: err.Error(),
AttributePath: path,
})
Expand All @@ -1485,8 +1485,8 @@ func (m schemaMap) validate(
// This is a computed-only field
return append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "Computed attributes cannot be set",
Detail: fmt.Sprintf("Computed attributes cannot be set, but a value was set for %q.", k),
Summary: "Value for unconfigurable attribute",
Detail: fmt.Sprintf("Can't configure a value for %q: its value will be decided automatically based on the result of applying this configuration.", k),
AttributePath: path,
})
}
Expand All @@ -1495,7 +1495,7 @@ func (m schemaMap) validate(
if err != nil {
return append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "RequiredWith",
Summary: "Missing required argument",
Detail: err.Error(),
AttributePath: path,
})
Expand All @@ -1510,7 +1510,7 @@ func (m schemaMap) validate(
if schema.Deprecated != "" {
return append(diags, diag.Diagnostic{
Severity: diag.Warning,
Summary: "Attribute is deprecated",
Summary: "Argument is deprecated",
Detail: schema.Deprecated,
AttributePath: path,
})
Expand All @@ -1522,7 +1522,7 @@ func (m schemaMap) validate(
if err != nil {
return append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "ConflictsWith",
Summary: "Conflicting configuration arguments",
Detail: err.Error(),
AttributePath: path,
})
Expand Down Expand Up @@ -1699,7 +1699,7 @@ func (m schemaMap) validateList(
if rawV.Kind() != reflect.Slice {
return append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "Attribute should be a list",
Summary: "Attribute must be a list",
AttributePath: path,
})
}
Expand All @@ -1717,17 +1717,17 @@ func (m schemaMap) validateList(
if schema.MaxItems > 0 && rawV.Len() > schema.MaxItems {
return append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "List longer than MaxItems",
Detail: fmt.Sprintf("Attribute supports %d item maximum, config has %d declared", schema.MaxItems, rawV.Len()),
Summary: "Too many list items",
Detail: fmt.Sprintf("Attribute supports %d item maximum, but config has %d declared.", schema.MaxItems, rawV.Len()),
AttributePath: path,
})
}

if schema.MinItems > 0 && rawV.Len() < schema.MinItems {
return append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "List shorter than MinItems",
Detail: fmt.Sprintf("Attribute supports %d item minimum, config has %d declared", schema.MinItems, rawV.Len()),
Summary: "Not enough list items",
Detail: fmt.Sprintf("Attribute requires %d item minimum, but config has only %d declared.", schema.MinItems, rawV.Len()),
AttributePath: path,
})
}
Expand Down Expand Up @@ -1794,7 +1794,7 @@ func (m schemaMap) validateMap(
if reifiedOk && raw == reified && !c.IsComputed(k) {
return append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "Attribute should be a map",
Summary: "Attribute must be a map",
AttributePath: path,
})
}
Expand All @@ -1805,7 +1805,7 @@ func (m schemaMap) validateMap(
default:
return append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "Attribute should be a map",
Summary: "Attribute must be a map",
AttributePath: path,
})
}
Expand All @@ -1832,7 +1832,7 @@ func (m schemaMap) validateMap(
if v.Kind() != reflect.Map {
return append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "Attribute should be a map",
Summary: "Attribute must be a map",
AttributePath: path,
})
}
Expand Down Expand Up @@ -2111,7 +2111,7 @@ func (m schemaMap) validateType(
if schema.Deprecated != "" {
diags = append(diags, diag.Diagnostic{
Severity: diag.Warning,
Summary: "Deprecated Attribute",
Summary: "Argument is deprecated",
Detail: schema.Deprecated,
AttributePath: path,
})
Expand Down
30 changes: 15 additions & 15 deletions helper/schema/schema_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5506,7 +5506,7 @@ func TestSchemaMap_Validate(t *testing.T) {

Err: true,
Errors: []error{
fmt.Errorf("Error: Attribute should be a list"),
fmt.Errorf("Error: Attribute must be a list"),
},
},

Expand All @@ -5527,7 +5527,7 @@ func TestSchemaMap_Validate(t *testing.T) {

Err: true,
Errors: []error{
fmt.Errorf("Error: Attribute should be a list"),
fmt.Errorf("Error: Attribute must be a list"),
},
},

Expand Down Expand Up @@ -5563,7 +5563,7 @@ func TestSchemaMap_Validate(t *testing.T) {
},

Warnings: []string{
"Warning: Attribute is deprecated: please use 'new_news' instead",
"Warning: Argument is deprecated: please use 'new_news' instead",
},
},

Expand Down Expand Up @@ -5953,7 +5953,7 @@ func TestSchemaMap_Validate(t *testing.T) {
Err: false,

Warnings: []string{
"Warning: Deprecated Attribute: please use 'new_news' instead",
"Warning: Argument is deprecated: please use 'new_news' instead",
},
},

Expand Down Expand Up @@ -5991,7 +5991,7 @@ func TestSchemaMap_Validate(t *testing.T) {

Err: true,
Errors: []error{
fmt.Errorf(`Error: ConflictsWith: "blacklist": conflicts with whitelist`),
fmt.Errorf(`Error: Conflicting configuration arguments: "blacklist": conflicts with whitelist`),
},
},

Expand Down Expand Up @@ -6087,8 +6087,8 @@ func TestSchemaMap_Validate(t *testing.T) {

Err: true,
Errors: []error{
fmt.Errorf(`Error: ConflictsWith: "blacklist": conflicts with greenlist`),
fmt.Errorf(`Error: ConflictsWith: "greenlist": conflicts with blacklist`),
fmt.Errorf(`Error: Conflicting configuration arguments: "blacklist": conflicts with greenlist`),
fmt.Errorf(`Error: Conflicting configuration arguments: "greenlist": conflicts with blacklist`),
},
},

Expand Down Expand Up @@ -6132,7 +6132,7 @@ func TestSchemaMap_Validate(t *testing.T) {

Err: true,
Errors: []error{
fmt.Errorf(`Error: ConflictsWith: "optional_att": conflicts with required_att`),
fmt.Errorf(`Error: Conflicting configuration arguments: "optional_att": conflicts with required_att`),
},
},

Expand All @@ -6159,8 +6159,8 @@ func TestSchemaMap_Validate(t *testing.T) {

Err: true,
Errors: []error{
fmt.Errorf(`Error: ConflictsWith: "bar_att": conflicts with foo_att`),
fmt.Errorf(`Error: ConflictsWith: "foo_att": conflicts with bar_att`),
fmt.Errorf(`Error: Conflicting configuration arguments: "bar_att": conflicts with foo_att`),
fmt.Errorf(`Error: Conflicting configuration arguments: "foo_att": conflicts with bar_att`),
},
},

Expand Down Expand Up @@ -6528,7 +6528,7 @@ func TestSchemaMap_Validate(t *testing.T) {

ws := diagutils.WarningDiags(diags).Warnings()
if !reflect.DeepEqual(ws, tc.Warnings) {
t.Fatalf("%q: warnings:\n\nexpected: %#v\ngot:%#v", tn, tc.Warnings, ws)
t.Fatalf("%q: warnings:\n\ngot: %#v\nwant: %#v", tn, ws, tc.Warnings)
}

es := diagutils.ErrorDiags(diags).Errors()
Expand All @@ -6537,7 +6537,7 @@ func TestSchemaMap_Validate(t *testing.T) {
sort.Sort(errorSort(tc.Errors))

if !errorEquals(es, tc.Errors) {
t.Fatalf("%q: errors:\n\nexpected: %q\ngot: %q", tn, tc.Errors, es)
t.Fatalf("%q: errors:\n\ngot: %q\nwant: %q", tn, es, tc.Errors)
}
}
})
Expand Down Expand Up @@ -6583,7 +6583,7 @@ func TestSchemaSet_ValidateMaxItems(t *testing.T) {
Diff: nil,
Err: true,
Errors: []error{
fmt.Errorf("Error: List longer than MaxItems: Attribute supports 1 item maximum, config has 2 declared"),
fmt.Errorf("Error: Too many list items: Attribute supports 1 item maximum, but config has 2 declared."),
},
},
"#1": {
Expand Down Expand Up @@ -6705,7 +6705,7 @@ func TestSchemaSet_ValidateMinItems(t *testing.T) {
Diff: nil,
Err: true,
Errors: []error{
fmt.Errorf("Error: List shorter than MinItems: Attribute supports 2 item minimum, config has 1 declared"),
fmt.Errorf("Error: Not enough list items: Attribute requires 2 item minimum, but config has only 1 declared."),
},
},
}
Expand All @@ -6729,7 +6729,7 @@ func TestSchemaSet_ValidateMinItems(t *testing.T) {
es := diagutils.ErrorDiags(diags).Errors()
if tc.Errors != nil {
if !errorEquals(es, tc.Errors) {
t.Fatalf("%q: expected: %q\ngot: %q", tn, tc.Errors, es)
t.Fatalf("%q: wrong errors\ngot: %q\nwant: %q", tn, es, tc.Errors)
}
}
}
Expand Down

0 comments on commit 576f351

Please sign in to comment.