Merge pull request #19382 from hashicorp/b-tfdiags-enhancements

tfdiags: Detect more complex cfgs in ElaborateFromConfigBody
This commit is contained in:
Radek Simko 2018-11-17 14:27:24 +00:00 committed by GitHub
commit da5ae6e382
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 612 additions and 152 deletions

View File

@ -131,11 +131,70 @@ func (d *attributeDiagnostic) ElaborateFromConfigBody(body hcl.Body) Diagnostic
// propagated to every place in Terraform, and this happens only in the // propagated to every place in Terraform, and this happens only in the
// presence of errors where performance isn't a concern. // presence of errors where performance isn't a concern.
traverse := d.attrPath[:len(d.attrPath)-1] traverse := d.attrPath[:]
final := d.attrPath[len(d.attrPath)-1] final := d.attrPath[len(d.attrPath)-1]
// If we have more than one step then we'll first try to traverse to // Index should never be the first step
// a child body corresponding to the requested path. // as indexing of top blocks (such as resources & data sources)
// is handled elsewhere
if _, isIdxStep := traverse[0].(cty.IndexStep); isIdxStep {
subject := SourceRangeFromHCL(body.MissingItemRange())
ret.subject = &subject
return &ret
}
// Process index separately
idxStep, hasIdx := final.(cty.IndexStep)
if hasIdx {
final = d.attrPath[len(d.attrPath)-2]
traverse = d.attrPath[:len(d.attrPath)-1]
}
// If we have more than one step after removing index
// then we'll first try to traverse to a child body
// corresponding to the requested path.
if len(traverse) > 1 {
body = traversePathSteps(traverse, body)
}
// Default is to indicate a missing item in the deepest body we reached
// while traversing.
subject := SourceRangeFromHCL(body.MissingItemRange())
ret.subject = &subject
// Once we get here, "final" should be a GetAttr step that maps to an
// attribute in our current body.
finalStep, isAttr := final.(cty.GetAttrStep)
if !isAttr {
return &ret
}
content, _, contentDiags := body.PartialContent(&hcl.BodySchema{
Attributes: []hcl.AttributeSchema{
{
Name: finalStep.Name,
Required: true,
},
},
})
if contentDiags.HasErrors() {
return &ret
}
if attr, ok := content.Attributes[finalStep.Name]; ok {
hclRange := attr.Expr.Range()
if hasIdx {
// Try to be more precise by finding index range
hclRange = hclRangeFromIndexStepAndAttribute(idxStep, attr)
}
subject = SourceRangeFromHCL(hclRange)
ret.subject = &subject
}
return &ret
}
func traversePathSteps(traverse []cty.PathStep, body hcl.Body) hcl.Body {
for i := 0; i < len(traverse); i++ { for i := 0; i < len(traverse); i++ {
step := traverse[i] step := traverse[i]
@ -173,9 +232,7 @@ func (d *attributeDiagnostic) ElaborateFromConfigBody(body hcl.Body) Diagnostic
}, },
}) })
if contentDiags.HasErrors() { if contentDiags.HasErrors() {
subject := SourceRangeFromHCL(body.MissingItemRange()) return body
ret.subject = &subject
return &ret
} }
filtered := make([]*hcl.Block, 0, len(content.Blocks)) filtered := make([]*hcl.Block, 0, len(content.Blocks))
for _, block := range content.Blocks { for _, block := range content.Blocks {
@ -184,23 +241,21 @@ func (d *attributeDiagnostic) ElaborateFromConfigBody(body hcl.Body) Diagnostic
} }
} }
if len(filtered) == 0 { if len(filtered) == 0 {
// Step doesn't refer to a block
continue
} }
switch indexType { switch indexType {
case cty.NilType: // no index at all case cty.NilType: // no index at all
if len(filtered) != 1 { if len(filtered) != 1 {
subject := SourceRangeFromHCL(body.MissingItemRange()) return body
ret.subject = &subject
return &ret
} }
body = filtered[0].Body body = filtered[0].Body
case cty.Number: case cty.Number:
var idx int var idx int
err := gocty.FromCtyValue(indexVal, &idx) err := gocty.FromCtyValue(indexVal, &idx)
if err != nil || idx >= len(filtered) { if err != nil || idx >= len(filtered) {
subject := SourceRangeFromHCL(body.MissingItemRange()) return body
ret.subject = &subject
return &ret
} }
body = filtered[idx].Body body = filtered[idx].Body
case cty.String: case cty.String:
@ -215,58 +270,56 @@ func (d *attributeDiagnostic) ElaborateFromConfigBody(body hcl.Body) Diagnostic
if block == nil { if block == nil {
// No block with this key, so we'll just indicate a // No block with this key, so we'll just indicate a
// missing item in the containing block. // missing item in the containing block.
subject := SourceRangeFromHCL(body.MissingItemRange()) return body
ret.subject = &subject
return &ret
} }
body = block.Body body = block.Body
default: default:
// Should never happen, because only string and numeric indices // Should never happen, because only string and numeric indices
// are supported by cty collections. // are supported by cty collections.
subject := SourceRangeFromHCL(body.MissingItemRange()) return body
ret.subject = &subject
return &ret
} }
default: default:
// For any other kind of step, we'll just return our current body // For any other kind of step, we'll just return our current body
// as the subject and accept that this is a little inaccurate. // as the subject and accept that this is a little inaccurate.
subject := SourceRangeFromHCL(body.MissingItemRange()) return body
ret.subject = &subject
return &ret
} }
} }
return body
}
// Default is to indicate a missing item in the deepest body we reached func hclRangeFromIndexStepAndAttribute(idxStep cty.IndexStep, attr *hcl.Attribute) hcl.Range {
// while traversing. switch idxStep.Key.Type() {
subject := SourceRangeFromHCL(body.MissingItemRange()) case cty.Number:
ret.subject = &subject var idx int
err := gocty.FromCtyValue(idxStep.Key, &idx)
// Once we get here, "final" should be a GetAttr step that maps to an items, diags := hcl.ExprList(attr.Expr)
// attribute in our current body. if diags.HasErrors() {
finalStep, isAttr := final.(cty.GetAttrStep) return attr.Expr.Range()
if !isAttr { }
return &ret if err != nil || idx >= len(items) {
return attr.NameRange
}
return items[idx].Range()
case cty.String:
pairs, diags := hcl.ExprMap(attr.Expr)
if diags.HasErrors() {
return attr.Expr.Range()
}
stepKey := idxStep.Key.AsString()
for _, kvPair := range pairs {
key, err := kvPair.Key.Value(nil)
if err != nil {
return attr.Expr.Range()
}
if key.AsString() == stepKey {
startRng := kvPair.Value.StartRange()
return startRng
}
}
return attr.NameRange
} }
return attr.Expr.Range()
content, _, contentDiags := body.PartialContent(&hcl.BodySchema{
Attributes: []hcl.AttributeSchema{
{
Name: finalStep.Name,
Required: true,
},
},
})
if contentDiags.HasErrors() {
return &ret
}
if attr, ok := content.Attributes[finalStep.Name]; ok {
subject = SourceRangeFromHCL(attr.Expr.Range())
ret.subject = &subject
}
return &ret
} }
func (d *attributeDiagnostic) Source() Source { func (d *attributeDiagnostic) Source() Source {

View File

@ -1,6 +1,7 @@
package tfdiags package tfdiags
import ( import (
"fmt"
"reflect" "reflect"
"testing" "testing"
@ -27,115 +28,521 @@ baz "a" {
baz "b" { baz "b" {
bar = "boop" bar = "boop"
} }
parent {
nested_str = "hello"
nested_str_tuple = ["aa", "bbb", "cccc"]
nested_num_tuple = [1, 9863, 22]
nested_map = {
first_key = "first_value"
second_key = "2nd value"
}
}
tuple_of_one = ["one"]
tuple_of_two = ["first", "22222"]
root_map = {
first = "1st"
second = "2nd"
}
simple_attr = "val"
` `
// TODO: Test ConditionalExpr
// TODO: Test ForExpr
// TODO: Test FunctionCallExpr
// TODO: Test IndexExpr
// TODO: Test interpolation
// TODO: Test SplatExpr
f, parseDiags := hclsyntax.ParseConfig([]byte(testConfig), "test.tf", hcl.Pos{Line: 1, Column: 1}) f, parseDiags := hclsyntax.ParseConfig([]byte(testConfig), "test.tf", hcl.Pos{Line: 1, Column: 1})
if len(parseDiags) != 0 { if len(parseDiags) != 0 {
t.Fatal(parseDiags) t.Fatal(parseDiags)
} }
emptySrcRng := &SourceRange{
body := f.Body Filename: "test.tf",
var diags Diagnostics Start: SourcePos{Line: 33, Column: 1, Byte: 440},
diags = diags.Append(AttributeValue( End: SourcePos{Line: 33, Column: 1, Byte: 440},
Error,
"foo[0].bar",
"detail",
cty.Path{
cty.GetAttrStep{Name: "foo"},
cty.IndexStep{Key: cty.NumberIntVal(0)},
cty.GetAttrStep{Name: "bar"},
},
))
diags = diags.Append(AttributeValue(
Error,
"foo[1].bar",
"detail",
cty.Path{
cty.GetAttrStep{Name: "foo"},
cty.IndexStep{Key: cty.NumberIntVal(1)},
cty.GetAttrStep{Name: "bar"},
},
))
diags = diags.Append(AttributeValue(
Error,
"bar.bar",
"detail",
cty.Path{
cty.GetAttrStep{Name: "bar"},
cty.GetAttrStep{Name: "bar"},
},
))
diags = diags.Append(AttributeValue(
Error,
`baz["a"].bar`,
"detail",
cty.Path{
cty.GetAttrStep{Name: "baz"},
cty.IndexStep{Key: cty.StringVal("a")},
cty.GetAttrStep{Name: "bar"},
},
))
diags = diags.Append(AttributeValue(
Error,
`baz["b"].bar`,
"detail",
cty.Path{
cty.GetAttrStep{Name: "baz"},
cty.IndexStep{Key: cty.StringVal("b")},
cty.GetAttrStep{Name: "bar"},
},
))
// Attribute value with subject already populated should not be disturbed.
// (in a real case, this might've been passed through from a deeper function
// in the call stack, for example.)
diags = diags.Append(&attributeDiagnostic{
diagnosticBase: diagnosticBase{
summary: "preexisting",
detail: "detail",
},
subject: &SourceRange{
Filename: "somewhere_else.tf",
},
})
gotDiags := diags.InConfigBody(body)
wantRanges := map[string]*SourceRange{
`foo[0].bar`: {
Filename: "test.tf",
Start: SourcePos{Line: 3, Column: 9, Byte: 15},
End: SourcePos{Line: 3, Column: 13, Byte: 19},
},
`foo[1].bar`: {
Filename: "test.tf",
Start: SourcePos{Line: 6, Column: 9, Byte: 36},
End: SourcePos{Line: 6, Column: 14, Byte: 41},
},
`bar.bar`: {
Filename: "test.tf",
Start: SourcePos{Line: 9, Column: 9, Byte: 58},
End: SourcePos{Line: 9, Column: 15, Byte: 64},
},
`baz["a"].bar`: {
Filename: "test.tf",
Start: SourcePos{Line: 12, Column: 9, Byte: 85},
End: SourcePos{Line: 12, Column: 15, Byte: 91},
},
`baz["b"].bar`: {
Filename: "test.tf",
Start: SourcePos{Line: 15, Column: 9, Byte: 112},
End: SourcePos{Line: 15, Column: 15, Byte: 118},
},
`preexisting`: {
Filename: "somewhere_else.tf",
},
}
gotRanges := make(map[string]*SourceRange)
for _, diag := range gotDiags {
gotRanges[diag.Description().Summary] = diag.Source().Subject
} }
for _, problem := range deep.Equal(gotRanges, wantRanges) { testCases := []struct {
t.Error(problem) Diag Diagnostic
ExpectedRange *SourceRange
}{
{
AttributeValue(
Error,
"foo[0].bar",
"detail",
cty.Path{
cty.GetAttrStep{Name: "foo"},
cty.IndexStep{Key: cty.NumberIntVal(0)},
cty.GetAttrStep{Name: "bar"},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 3, Column: 9, Byte: 15},
End: SourcePos{Line: 3, Column: 13, Byte: 19},
},
},
{
AttributeValue(
Error,
"foo[1].bar",
"detail",
cty.Path{
cty.GetAttrStep{Name: "foo"},
cty.IndexStep{Key: cty.NumberIntVal(1)},
cty.GetAttrStep{Name: "bar"},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 6, Column: 9, Byte: 36},
End: SourcePos{Line: 6, Column: 14, Byte: 41},
},
},
{
AttributeValue(
Error,
"foo[99].bar",
"detail",
cty.Path{
cty.GetAttrStep{Name: "foo"},
cty.IndexStep{Key: cty.NumberIntVal(99)},
cty.GetAttrStep{Name: "bar"},
},
),
emptySrcRng,
},
{
AttributeValue(
Error,
"bar.bar",
"detail",
cty.Path{
cty.GetAttrStep{Name: "bar"},
cty.GetAttrStep{Name: "bar"},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 9, Column: 9, Byte: 58},
End: SourcePos{Line: 9, Column: 15, Byte: 64},
},
},
{
AttributeValue(
Error,
`baz["a"].bar`,
"detail",
cty.Path{
cty.GetAttrStep{Name: "baz"},
cty.IndexStep{Key: cty.StringVal("a")},
cty.GetAttrStep{Name: "bar"},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 12, Column: 9, Byte: 85},
End: SourcePos{Line: 12, Column: 15, Byte: 91},
},
},
{
AttributeValue(
Error,
`baz["b"].bar`,
"detail",
cty.Path{
cty.GetAttrStep{Name: "baz"},
cty.IndexStep{Key: cty.StringVal("b")},
cty.GetAttrStep{Name: "bar"},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 15, Column: 9, Byte: 112},
End: SourcePos{Line: 15, Column: 15, Byte: 118},
},
},
{
AttributeValue(
Error,
`baz["not_exists"].bar`,
"detail",
cty.Path{
cty.GetAttrStep{Name: "baz"},
cty.IndexStep{Key: cty.StringVal("not_exists")},
cty.GetAttrStep{Name: "bar"},
},
),
emptySrcRng,
},
{
// Attribute value with subject already populated should not be disturbed.
// (in a real case, this might've been passed through from a deeper function
// in the call stack, for example.)
&attributeDiagnostic{
attrPath: cty.Path{cty.GetAttrStep{Name: "foo"}},
diagnosticBase: diagnosticBase{
summary: "preexisting",
detail: "detail",
},
subject: &SourceRange{
Filename: "somewhere_else.tf",
},
},
&SourceRange{
Filename: "somewhere_else.tf",
},
},
{
// Missing path
&attributeDiagnostic{
diagnosticBase: diagnosticBase{
summary: "missing path",
},
},
nil,
},
// Nested attributes
{
AttributeValue(
Error,
"parent.nested_str",
"detail",
cty.Path{
cty.GetAttrStep{Name: "parent"},
cty.GetAttrStep{Name: "nested_str"},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 18, Column: 16, Byte: 145},
End: SourcePos{Line: 18, Column: 23, Byte: 152},
},
},
{
AttributeValue(
Error,
"parent.nested_str_tuple[99]",
"detail",
cty.Path{
cty.GetAttrStep{Name: "parent"},
cty.GetAttrStep{Name: "nested_str_tuple"},
cty.IndexStep{Key: cty.NumberIntVal(99)},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 19, Column: 3, Byte: 155},
End: SourcePos{Line: 19, Column: 19, Byte: 171},
},
},
{
AttributeValue(
Error,
"parent.nested_str_tuple[0]",
"detail",
cty.Path{
cty.GetAttrStep{Name: "parent"},
cty.GetAttrStep{Name: "nested_str_tuple"},
cty.IndexStep{Key: cty.NumberIntVal(0)},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 19, Column: 23, Byte: 175},
End: SourcePos{Line: 19, Column: 27, Byte: 179},
},
},
{
AttributeValue(
Error,
"parent.nested_str_tuple[2]",
"detail",
cty.Path{
cty.GetAttrStep{Name: "parent"},
cty.GetAttrStep{Name: "nested_str_tuple"},
cty.IndexStep{Key: cty.NumberIntVal(2)},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 19, Column: 36, Byte: 188},
End: SourcePos{Line: 19, Column: 42, Byte: 194},
},
},
{
AttributeValue(
Error,
"parent.nested_num_tuple[0]",
"detail",
cty.Path{
cty.GetAttrStep{Name: "parent"},
cty.GetAttrStep{Name: "nested_num_tuple"},
cty.IndexStep{Key: cty.NumberIntVal(0)},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 20, Column: 23, Byte: 218},
End: SourcePos{Line: 20, Column: 24, Byte: 219},
},
},
{
AttributeValue(
Error,
"parent.nested_num_tuple[1]",
"detail",
cty.Path{
cty.GetAttrStep{Name: "parent"},
cty.GetAttrStep{Name: "nested_num_tuple"},
cty.IndexStep{Key: cty.NumberIntVal(1)},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 20, Column: 26, Byte: 221},
End: SourcePos{Line: 20, Column: 30, Byte: 225},
},
},
{
AttributeValue(
Error,
"parent.nested_map.first_key",
"detail",
cty.Path{
cty.GetAttrStep{Name: "parent"},
cty.GetAttrStep{Name: "nested_map"},
cty.IndexStep{Key: cty.StringVal("first_key")},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 22, Column: 19, Byte: 266},
End: SourcePos{Line: 22, Column: 30, Byte: 277},
},
},
{
AttributeValue(
Error,
"parent.nested_map.second_key",
"detail",
cty.Path{
cty.GetAttrStep{Name: "parent"},
cty.GetAttrStep{Name: "nested_map"},
cty.IndexStep{Key: cty.StringVal("second_key")},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 23, Column: 19, Byte: 297},
End: SourcePos{Line: 23, Column: 28, Byte: 306},
},
},
{
AttributeValue(
Error,
"parent.nested_map.undefined_key",
"detail",
cty.Path{
cty.GetAttrStep{Name: "parent"},
cty.GetAttrStep{Name: "nested_map"},
cty.IndexStep{Key: cty.StringVal("undefined_key")},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 21, Column: 3, Byte: 233},
End: SourcePos{Line: 21, Column: 13, Byte: 243},
},
},
// Root attributes of complex types
{
AttributeValue(
Error,
"tuple_of_one[0]",
"detail",
cty.Path{
cty.GetAttrStep{Name: "tuple_of_one"},
cty.IndexStep{Key: cty.NumberIntVal(0)},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 26, Column: 17, Byte: 330},
End: SourcePos{Line: 26, Column: 22, Byte: 335},
},
},
{
AttributeValue(
Error,
"tuple_of_two[0]",
"detail",
cty.Path{
cty.GetAttrStep{Name: "tuple_of_two"},
cty.IndexStep{Key: cty.NumberIntVal(0)},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 27, Column: 17, Byte: 353},
End: SourcePos{Line: 27, Column: 24, Byte: 360},
},
},
{
AttributeValue(
Error,
"tuple_of_two[1]",
"detail",
cty.Path{
cty.GetAttrStep{Name: "tuple_of_two"},
cty.IndexStep{Key: cty.NumberIntVal(1)},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 27, Column: 26, Byte: 362},
End: SourcePos{Line: 27, Column: 33, Byte: 369},
},
},
{
AttributeValue(
Error,
"tuple_of_one[null]",
"detail",
cty.Path{
cty.GetAttrStep{Name: "tuple_of_one"},
cty.IndexStep{Key: cty.NullVal(cty.Number)},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 26, Column: 1, Byte: 314},
End: SourcePos{Line: 26, Column: 13, Byte: 326},
},
},
{
// index out of range
AttributeValue(
Error,
"tuple_of_two[99]",
"detail",
cty.Path{
cty.GetAttrStep{Name: "tuple_of_two"},
cty.IndexStep{Key: cty.NumberIntVal(99)},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 27, Column: 1, Byte: 337},
End: SourcePos{Line: 27, Column: 13, Byte: 349},
},
},
{
AttributeValue(
Error,
"root_map.first",
"detail",
cty.Path{
cty.GetAttrStep{Name: "root_map"},
cty.IndexStep{Key: cty.StringVal("first")},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 29, Column: 13, Byte: 396},
End: SourcePos{Line: 29, Column: 16, Byte: 399},
},
},
{
AttributeValue(
Error,
"root_map.second",
"detail",
cty.Path{
cty.GetAttrStep{Name: "root_map"},
cty.IndexStep{Key: cty.StringVal("second")},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 30, Column: 13, Byte: 413},
End: SourcePos{Line: 30, Column: 16, Byte: 416},
},
},
{
AttributeValue(
Error,
"root_map.undefined_key",
"detail",
cty.Path{
cty.GetAttrStep{Name: "root_map"},
cty.IndexStep{Key: cty.StringVal("undefined_key")},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 28, Column: 1, Byte: 371},
End: SourcePos{Line: 28, Column: 9, Byte: 379},
},
},
{
AttributeValue(
Error,
"simple_attr",
"detail",
cty.Path{
cty.GetAttrStep{Name: "simple_attr"},
},
),
&SourceRange{
Filename: "test.tf",
Start: SourcePos{Line: 32, Column: 15, Byte: 434},
End: SourcePos{Line: 32, Column: 20, Byte: 439},
},
},
{
// This should never happen as error should always point to an attribute
// or index of an attribute, but we should not crash if it does
AttributeValue(
Error,
"key",
"index_step",
cty.Path{
cty.IndexStep{Key: cty.StringVal("key")},
},
),
emptySrcRng,
},
{
// This should never happen as error should always point to an attribute
// or index of an attribute, but we should not crash if it does
AttributeValue(
Error,
"key.another",
"index_step",
cty.Path{
cty.IndexStep{Key: cty.StringVal("key")},
cty.IndexStep{Key: cty.StringVal("another")},
},
),
emptySrcRng,
},
}
for i, tc := range testCases {
t.Run(fmt.Sprintf("%d:%s", i, tc.Diag.Description()), func(t *testing.T) {
var diags Diagnostics
diags = diags.Append(tc.Diag)
gotDiags := diags.InConfigBody(f.Body)
gotRange := gotDiags[0].Source().Subject
for _, problem := range deep.Equal(gotRange, tc.ExpectedRange) {
t.Error(problem)
}
})
} }
} }