Skip to content

Commit

Permalink
Walk map/list expressions deeply (#1432)
Browse files Browse the repository at this point in the history
  • Loading branch information
@wata727
wata727 authored Jul 10, 2022
1 parent aed2eaf commit 8e61771
Show file tree
Hide file tree
Showing 5 changed files with 199 additions and 255 deletions.
24 changes: 18 additions & 6 deletions rules/terraformrules/terraform_deprecated_interpolation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,17 @@ resource "null_resource" "a" {
resource "null_resource" "a" {
triggers = ["${var.triggers}"]
}`,
Expected: tflint.Issues{},
Expected: tflint.Issues{
{
Rule: NewTerraformDeprecatedInterpolationRule(),
Message: "Interpolation-only expressions are deprecated in Terraform v0.12.14",
Range: hcl.Range{
Filename: "config.tf",
Start: hcl.Pos{Line: 3, Column: 14},
End: hcl.Pos{Line: 3, Column: 31},
},
},
},
},
{
Name: "new interpolation syntax",
Expand All @@ -116,12 +126,14 @@ resource "null_resource" "a" {
rule := NewTerraformDeprecatedInterpolationRule()

for _, tc := range cases {
runner := tflint.TestRunner(t, map[string]string{"config.tf": tc.Content})
t.Run(tc.Name, func(t *testing.T) {
runner := tflint.TestRunner(t, map[string]string{"config.tf": tc.Content})

if err := rule.Check(runner); err != nil {
t.Fatalf("Unexpected error occurred: %s", err)
}
if err := rule.Check(runner); err != nil {
t.Fatalf("Unexpected error occurred: %s", err)
}

tflint.AssertIssues(t, tc.Expected, runner.Issues)
tflint.AssertIssues(t, tc.Expected, runner.Issues)
})
}
}
7 changes: 7 additions & 0 deletions rules/terraformrules/terraform_workspace_remote.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"log"

"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hclsyntax"
"github.com/hashicorp/hcl/v2/json"
"github.com/terraform-linters/tflint/terraform/addrs"
"github.com/terraform-linters/tflint/terraform/lang"
"github.com/terraform-linters/tflint/tflint"
Expand Down Expand Up @@ -58,6 +60,11 @@ func (r *TerraformWorkspaceRemoteRule) Check(runner *tflint.Runner) error {
}

func (r *TerraformWorkspaceRemoteRule) checkForTerraformWorkspaceInExpr(runner *tflint.Runner, expr hcl.Expression) error {
_, isScopeTraversalExpr := expr.(*hclsyntax.ScopeTraversalExpr)
if !isScopeTraversalExpr && !json.IsJSONExpression(expr) {
return nil
}

refs, diags := lang.ReferencesInExpr(expr)
if diags.HasErrors() {
log.Printf("[DEBUG] Cannot find references in expression, ignoring: %v", diags.Err())
Expand Down
47 changes: 42 additions & 5 deletions rules/terraformrules/terraform_workspace_remote_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
func Test_TerraformWorkspaceRemoteRule(t *testing.T) {
cases := []struct {
Name string
JSON bool
Content string
Expected tflint.Issues
}{
Expand All @@ -30,8 +31,8 @@ resource "null_resource" "a" {
Message: "terraform.workspace should not be used with a 'remote' backend",
Range: hcl.Range{
Filename: "config.tf",
Start: hcl.Pos{Line: 6, Column: 13},
End: hcl.Pos{Line: 8, Column: 3},
Start: hcl.Pos{Line: 7, Column: 7},
End: hcl.Pos{Line: 7, Column: 26},
},
},
},
Expand Down Expand Up @@ -77,8 +78,8 @@ data "null_data_source" "a" {
Message: "terraform.workspace should not be used with a 'remote' backend",
Range: hcl.Range{
Filename: "config.tf",
Start: hcl.Pos{Line: 6, Column: 11},
End: hcl.Pos{Line: 8, Column: 3},
Start: hcl.Pos{Line: 7, Column: 7},
End: hcl.Pos{Line: 7, Column: 26},
},
},
},
Expand Down Expand Up @@ -203,13 +204,49 @@ resource "aws_instance" "foo" {
}`,
Expected: tflint.Issues{},
},
{
Name: "terraform.workspace in JSON syntax",
JSON: true,
Content: `
{
"terraform": {
"backend": {
"remote": {}
}
},
"resource": {
"null_resource": {
"a": {
"triggers": {
"w": "${terraform.workspace}"
}
}
}
}
}`,
Expected: tflint.Issues{
{
Rule: NewTerraformWorkspaceRemoteRule(),
Message: "terraform.workspace should not be used with a 'remote' backend",
Range: hcl.Range{
Filename: "config.tf.json",
Start: hcl.Pos{Line: 8, Column: 15},
End: hcl.Pos{Line: 16, Column: 4},
},
},
},
},
}

rule := NewTerraformWorkspaceRemoteRule()

for _, tc := range cases {
t.Run(tc.Name, func(t *testing.T) {
runner := tflint.TestRunner(t, map[string]string{"config.tf": tc.Content})
filename := "config.tf"
if tc.JSON {
filename = "config.tf.json"
}
runner := tflint.TestRunner(t, map[string]string{filename: tc.Content})

if err := rule.Check(runner); err != nil {
t.Fatalf("Unexpected error occurred: %s", err)
Expand Down
128 changes: 40 additions & 88 deletions tflint/runner_walk.go
View file
Original file line number Diff line number Diff line change
@@ -1,99 +1,51 @@
package tflint

import (
"log"

hcl "github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hclsyntax"
)

// WalkExpressions visits all blocks that can contain expressions:
// resource, data, module, provider, locals, and output. It calls the walker
// function with every expression it encounters and halts if the walker
// returns an error.
// WalkExpressions visits all expressions, including those in the file before merging.
// Note that it behaves differently in native HCL syntax and JSON syntax.
// In the HCL syntax, expressions in expressions, such as list and object are passed to
// the walker function. The walker should check the type of the expression.
// In the JSON syntax, only an expression of an attribute seen from the top level of the file
// is passed, not expressions in expressions to the walker. This is an API limitation of JSON syntax.
func (r *Runner) WalkExpressions(walker func(hcl.Expression) error) error {
visit := func(expr hcl.Expression) error {
return r.WithExpressionContext(expr, func() error {
return walker(expr)
})
}

for _, resource := range r.TFConfig.Module.ManagedResources {
if err := r.walkBody(resource.Config, visit); err != nil {
return err
}
}
for _, resource := range r.TFConfig.Module.DataResources {
if err := r.walkBody(resource.Config, visit); err != nil {
return err
}
}
for _, module := range r.TFConfig.Module.ModuleCalls {
if err := r.walkBody(module.Config, visit); err != nil {
return err
}
}
for _, provider := range r.TFConfig.Module.ProviderConfigs {
if err := r.walkBody(provider.Config, visit); err != nil {
return err
}
}
for _, local := range r.TFConfig.Module.Locals {
if err := visit(local.Expr); err != nil {
return err
}
}
for _, output := range r.TFConfig.Module.Outputs {
if err := visit(output.Expr); err != nil {
return err
}
}

return nil
}

// walkBody visits all attributes and passes their expressions to the walker function.
// It recurses on nested blocks.
func (r *Runner) walkBody(b hcl.Body, walker func(hcl.Expression) error) error {
body, ok := b.(*hclsyntax.Body)
if !ok {
// HACK: Other than hclsyntax.Body, there are json.body and configs.mergeBody structs that satisfy hcl.Body,
// but since both are private structs, there is no reliable way to determine them.
// Here, it is judged by whether it can process `body.JustAttributes`. See also `walkAttributes`.
if _, diags := b.JustAttributes(); diags.HasErrors() {
log.Printf("[WARN] Ignore attributes of `%T` because we can only handle hclsyntax.Body or json.body", b)
return nil
}
return r.walkAttributes(b, walker)
}

for _, attr := range body.Attributes {
if err := walker(attr.Expr); err != nil {
return err
}
}

for _, block := range body.Blocks {
if err := r.walkBody(block.Body, walker); err != nil {
return err
}
}

return nil
}

// walkAttributes visits all attributes and passes their expressions to the walker function.
// It should be used only for non-HCL bodies (JSON) when distinguishing a block from an attribute
// is not possible without a schema.
func (r *Runner) walkAttributes(b hcl.Body, walker func(hcl.Expression) error) error {
attrs, diags := b.JustAttributes()
if diags.HasErrors() {
return diags
}

for _, attr := range attrs {
if err := walker(attr.Expr); err != nil {
return err
visit := func(node hclsyntax.Node) hcl.Diagnostics {
if expr, ok := node.(hcl.Expression); ok {
if err := walker(expr); err != nil {
// FIXME: walker should returns hcl.Diagnostics directly
return hcl.Diagnostics{
{
Severity: hcl.DiagError,
Summary: err.Error(),
},
}
}
}
return hcl.Diagnostics{}
}

for _, file := range r.Files() {
if body, ok := file.Body.(*hclsyntax.Body); ok {
diags := hclsyntax.VisitAll(body, visit)
if diags.HasErrors() {
return diags
}
continue
}

// In JSON syntax, everything can be walked as an attribute.
attrs, diags := file.Body.JustAttributes()
if diags.HasErrors() {
return diags
}

for _, attr := range attrs {
if err := walker(attr.Expr); err != nil {
return err
}
}
}

Expand Down
Loading

0 comments on commit 8e61771

Please sign in to comment.