Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Checks: semantics validation on attribute reference #163

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions helper/analysisutils/schema_analyzers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@ package analysisutils

import (
"fmt"
"github.com/bflad/tfproviderlint/passes/helper/schema/resourceinfo"

"github.com/bflad/tfproviderlint/passes/commentignore"
"github.com/bflad/tfproviderlint/passes/helper/schema/schemainfo"
"golang.org/x/tools/go/analysis"
)

// SchemaAttributeReferencesAnalyzer returns an Analyzer for fields that use schema attribute references
func SchemaAttributeReferencesAnalyzer(analyzerName string, fieldName string) *analysis.Analyzer {
doc := fmt.Sprintf(`check for Schema with invalid %[2]s references
// SchemaAttributeReferencesSyntaxAnalyzer returns an Analyzer for fields that use schema attribute references
func SchemaAttributeReferencesSyntaxAnalyzer(analyzerName string, fieldName string) *analysis.Analyzer {
doc := fmt.Sprintf(`check for Schema %[2]s references with invalid syntax

The %[1]s analyzer ensures schema attribute references in the Schema %[2]s
field use valid syntax. The Terraform Plugin SDK can unit test attribute
Expand All @@ -24,6 +25,25 @@ references to verify the references against the full schema.
commentignore.Analyzer,
schemainfo.Analyzer,
},
Run: SchemaAttributeReferencesRunner(analyzerName, fieldName),
Run: SchemaAttributeReferencesSyntaxRunner(analyzerName, fieldName),
}
}

// SchemaAttributeReferencesSemanticsAnalyzer returns an Analyzer for fields that use schema attribute references
func SchemaAttributeReferencesSemanticsAnalyzer(analyzerName string, fieldName string) *analysis.Analyzer {
doc := fmt.Sprintf(`check for Schema %[2]s references with invalid semantics

The %[1]s analyzer ensures schema attribute references in the Schema %[2]s
field refers to valid attribute.
`, analyzerName, fieldName)

return &analysis.Analyzer{
Name: analyzerName,
Doc: doc,
Requires: []*analysis.Analyzer{
commentignore.Analyzer,
resourceinfo.Analyzer,
},
Run: SchemaAttributeReferencesSemanticsRunner(analyzerName, fieldName),
}
}
74 changes: 71 additions & 3 deletions helper/analysisutils/schema_runners.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package analysisutils

import (
"github.com/bflad/tfproviderlint/passes/helper/schema/resourceinfo"
"go/ast"

"github.com/bflad/tfproviderlint/helper/astutils"
Expand All @@ -10,8 +11,8 @@ import (
"golang.org/x/tools/go/analysis"
)

// SchemaAttributeReferencesRunner returns an Analyzer runner for fields that use schema attribute references
func SchemaAttributeReferencesRunner(analyzerName string, fieldName string) func(*analysis.Pass) (interface{}, error) {
// SchemaAttributeReferencesSyntaxRunner returns an Analyzer runner for fields that use schema attribute references
func SchemaAttributeReferencesSyntaxRunner(analyzerName string, fieldName string) func(*analysis.Pass) (interface{}, error) {
return func(pass *analysis.Pass) (interface{}, error) {
ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer)
schemaInfos := pass.ResultOf[schemainfo.Analyzer].([]*schema.SchemaInfo)
Expand Down Expand Up @@ -39,7 +40,74 @@ func SchemaAttributeReferencesRunner(analyzerName string, fieldName string) func
}

if _, err := schema.ParseAttributeReference(*attributeReference); err != nil {
pass.Reportf(elt.Pos(), "%s: invalid %s attribute reference: %s", analyzerName, fieldName, err)
pass.Reportf(elt.Pos(), "%s: invalid %s attribute reference syntax: %s", analyzerName, fieldName, err)
}
}
}
}

return nil, nil
}
}

// SchemaAttributeReferencesSemanticsRunner returns an Analyzer runner for fields that use schema attribute references
func SchemaAttributeReferencesSemanticsRunner(analyzerName string, fieldName string) func(*analysis.Pass) (interface{}, error) {
return func(pass *analysis.Pass) (interface{}, error) {
ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer)
resourceInfos := pass.ResultOf[resourceinfo.Analyzer].([]*schema.ResourceInfo)

for _, resourceInfo := range resourceInfos {
// Handle "root" schema.Resource only since we will use the resourceInfo of that to validate attribute reference.
if !resourceInfo.IsDataSource() && !resourceInfo.IsResource() {
continue
}

// Collection schemaInfo under current resourceInfo
var schemaInfos []*schema.SchemaInfo
ast.Inspect(resourceInfo.AstCompositeLit, func(n ast.Node) bool {
compositeLit, ok := n.(*ast.CompositeLit)

if !ok {
return true
}

if schema.IsMapStringSchema(compositeLit, pass.TypesInfo) {
for _, mapSchema := range schema.GetSchemaMapSchemas(compositeLit) {
schemaInfos = append(schemaInfos, schema.NewSchemaInfo(mapSchema, pass.TypesInfo))
}
} else if schema.IsTypeSchema(pass.TypesInfo.TypeOf(compositeLit.Type)) {
schemaInfos = append(schemaInfos, schema.NewSchemaInfo(compositeLit, pass.TypesInfo))
}

return true
})

// Validate each schemaInfo under current resourceInfo
for _, schemaInfo := range schemaInfos {
if ignorer.ShouldIgnore(analyzerName, schemaInfo.AstCompositeLit) {
continue
}

if !schemaInfo.DeclaresField(fieldName) {
continue
}

switch value := schemaInfo.Fields[fieldName].Value.(type) {
case *ast.CompositeLit:
if !astutils.IsExprTypeArrayString(value.Type) {
continue
}

for _, elt := range value.Elts {
attributeReference := astutils.ExprStringValue(elt)

if attributeReference == nil {
continue
}

if err := schema.ValidateAttributeReference(resourceInfo.Resource, *attributeReference); err != nil {
pass.Reportf(elt.Pos(), "%s: invalid %s attribute reference semantics: %s", analyzerName, fieldName, err)
}
}
}
}
Expand Down
52 changes: 52 additions & 0 deletions helper/terraformtype/helper/schema/attributes.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package schema

import (
"fmt"
tfschema "github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"math"
"regexp"
"strconv"
Expand Down Expand Up @@ -59,3 +60,54 @@ func ParseAttributeReference(reference string) ([]string, error) {

return attributeReferenceParts, nil
}

// ValidateAttributeReference validates schema attribute reference semantically.
// Attribute references are used in Schema fields such as AtLeastOneOf, ConflictsWith, and ExactlyOneOf.
func ValidateAttributeReference(tfresource *tfschema.Resource, reference string) error {
var curSchemaOrResource interface{} = tfresource
attributeReferenceParts := strings.Split(reference, ".")
for idx, attributeReferencePart := range attributeReferenceParts {
attributeReference := strings.Join(attributeReferenceParts[:idx+1], ".")
if math.Mod(float64(idx), 2) == 1 {
// For odd part, ensure it is a 0 and the containing schema has `MaxItems` set to `1`. This is because
// reference among multiple instances of the same nested block is not supported in current plugin SDK.
attributeReferencePartInt, err := strconv.Atoi(attributeReferencePart)

configurationBlockReferenceErr := fmt.Errorf("%q configuration block attribute references must be separated by .0", attributeReference)
if err != nil {
return configurationBlockReferenceErr
}
if attributeReferencePartInt != 0 {
return configurationBlockReferenceErr
}

switch curSchema := curSchemaOrResource.(type) {
case *tfschema.Schema:
if curSchema.MaxItems != 1 || curSchema.Type != tfschema.TypeList {
return fmt.Errorf("%q configuration block attribute references are only valid for TypeList and MaxItems: 1 attributes", attributeReference)
}
curSchemaOrResource = curSchema.Elem
default:
return nil
}
} else {
// For even part, ensure it references to defined attribute
switch curResource := curSchemaOrResource.(type) {
case *tfschema.Resource:
if curResource.Schema == nil {
return fmt.Errorf("%q references resource attribute without schema", attributeReference)
}
schema := curResource.Schema[attributeReferencePart]
if schema == nil {
return fmt.Errorf("%q references to unknown attribute", attributeReference)
}
curSchemaOrResource = schema
default:
return nil
}
}
}

return nil
}

2 changes: 1 addition & 1 deletion passes/S035/S035.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ import (
"github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema"
)

var Analyzer = analysisutils.SchemaAttributeReferencesAnalyzer("S035", schema.SchemaFieldAtLeastOneOf)
var Analyzer = analysisutils.SchemaAttributeReferencesSyntaxAnalyzer("S035", schema.SchemaFieldAtLeastOneOf)
2 changes: 1 addition & 1 deletion passes/S036/S036.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ import (
"github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema"
)

var Analyzer = analysisutils.SchemaAttributeReferencesAnalyzer("S036", schema.SchemaFieldConflictsWith)
var Analyzer = analysisutils.SchemaAttributeReferencesSyntaxAnalyzer("S036", schema.SchemaFieldConflictsWith)
2 changes: 1 addition & 1 deletion passes/S037/S037.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ import (
"github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema"
)

var Analyzer = analysisutils.SchemaAttributeReferencesAnalyzer("S037", schema.SchemaFieldExactlyOneOf)
var Analyzer = analysisutils.SchemaAttributeReferencesSyntaxAnalyzer("S037", schema.SchemaFieldExactlyOneOf)
86 changes: 86 additions & 0 deletions passes/S038/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# S038

The S038 analyzer reports cases of Schemas which include `AtLeastOneOf` and have invalid schema attribute references.

NOTE: This pass only works with `schema.Resource` that are wholly declared.

## Flagged Code

```go
// Attribute reference in configuration block with potentially more than one element is not supported
&schema.Resource{
Schema: map[string]*schema.Schema{
"x": {
Type: schema.TypeList,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"foo": {
AtLeastOneOf: []string{"x.0.bar"},
},
"bar": {
AtLeastOneOf: []string{"x.0.foo"},
},
},
},
},
},
}
```

or

```go
// Non-existed attribute reference
&schema.Resource{
Schema: map[string]*schema.Schema{
"x": {
Type: schema.TypeList,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"foo": {
AtLeastOneOf: []string{"x.1.bar"},
},
"bar": {
AtLeastOneOf: []string{"x.1.foo"},
},
},
},
},
},
}
```

## Passing Code

```go
&schema.Resource{
Schema: map[string]*schema.Schema{
"x": {
Type: schema.TypeList,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"foo": {
AtLeastOneOf: []string{"x.0.bar"},
},
"bar": {
AtLeastOneOf: []string{"x.0.foo"},
},
},
},
},
},
}
```

## Ignoring Reports

Singular reports can be ignored by adding the a `//lintignore:S038` Go code comment at the end of the offending line or on the line immediately proceding, e.g.

```go
//lintignore:S038
&schema.Resource{
// ...
}
```
8 changes: 8 additions & 0 deletions passes/S038/S038.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package S038

import (
"github.com/bflad/tfproviderlint/helper/analysisutils"
"github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema"
)

var Analyzer = analysisutils.SchemaAttributeReferencesSemanticsAnalyzer("S038", schema.SchemaFieldAtLeastOneOf)
12 changes: 12 additions & 0 deletions passes/S038/S038_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package S038

import (
"testing"

"golang.org/x/tools/go/analysis/analysistest"
)

func TestS038(t *testing.T) {
testdata := analysistest.TestData()
analysistest.Run(t, testdata, Analyzer, "a")
}
26 changes: 26 additions & 0 deletions passes/S038/testdata/src/a/alias.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package a

import (
s "github.com/hashicorp/terraform-plugin-sdk/helper/schema"
)

func falias() {
_ = &s.Resource{
Read: func(*s.ResourceData, interface{}) error { return nil },
Schema: map[string]*s.Schema{
"x": {
Type: s.TypeList,
Elem: &s.Resource{
Schema: map[string]*s.Schema{
"foo": {
AtLeastOneOf: []string{"x.0.bar"}, // want `S038: invalid AtLeastOneOf attribute reference semantics: "x.0" configuration block attribute references are only valid for TypeList and MaxItems: 1 attributes`
},
"bar": {
AtLeastOneOf: []string{"x.0.foo"}, // want `S038: invalid AtLeastOneOf attribute reference semantics: "x.0" configuration block attribute references are only valid for TypeList and MaxItems: 1 attributes`
},
},
},
},
},
}
}
Loading