terraform_deprecated_interpolation: deeply inspect map/list attributes

[morremeyer]

terraform_deprecated_interpolation does not detect interpolation everywhere in a configuration. Take the following two examples:

data "terraform_remote_state" "remote_state" {
  backend = "remote"

  config = {
    organization = "Organization"
    workspaces = {
      name = "${var.environment}"
    }
  }
}

and

provider "aws" {
  region = var.region
  default_tags {
    tags = {
      Owner          = "owner"
      Application    = "application"
      Environment    = "${var.environment}"
      Provisioned-by = "terraform"
    }
  }
}

In both, the "${var.environment}" is not detected as deprecated interpolation-only variable.

Interestingly, the same applies to terraform fmt, which does also not detect this. This might be related as tflint uses terraform built-in as far as I’m aware.

Version

❯ tflint -v
TFLint version 0.33.0
+ ruleset.aws (0.8.0)
❯ terraform -v
Terraform v1.0.11
on darwin_amd64

[bendrucker]

There is nothing special about either of these two block types so I have a hard time seeing why this would be. That rule attempts to walk all expressions:

tflint/tflint/runner_walk.go

Lines 143 to 186 in a18d094

	// 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.
	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
	}

That certainly includes walking provider and data blocks. It's conceivable that Terraform parses them differently I guess.

[bendrucker]

Ok, did a bit of debugging here. The key characteristic of the examples is not their block types, it's the fact that it's an object expression.

The rule is looking for expressions of type *hclsyntax.TemplateWrapExpr. That applies when an attribute is set to an unnecessarily quoted expression directly. But if the attribute is assigned to an object or another collection type that contains the expression, there's no match.

I seem to recall the logic for that rule was lifted from Terraform. terraform fmt could do better here and making the TFLint rule more comprehensive than fmt is also ok, as long as the rule is faithful to the written guidance in the Terraform docs.

In this case, the expression is actually a *hclsyntax.ObjectConsExpr. And expr.(*hclsyntax.ObjectConsExpr).Items[0].ValueExpr is a *hclsyntax.TemplateWrapExpr. Don't have time to take this any further at the moment, but in theory it would be doable to traverse the data in known collection expression types.

[bendrucker]

Similar to WalkExpressions we'd probably want to implement the complex expression walking in the runner and then call it in the rule.

[morremeyer]

Thanks for analyzing it! I’ll see if we can contribute that change to terraform fmt directly and adapt it for tflint.

[bendrucker]

It's a real tricky one for sure. There are presumably a fair number of collection types, e.g. a distinction between objects (known keys) and maps (unknown). Then there's sets/lists/tuples. It's almost certain that this is technically possible already, but I can imagine that implementing this elegantly could involve enhancing the hcl packages too.

[wata727]

For deeper walking, we can use hclsyntax.Walk.
https://github.com/hashicorp/hcl/blob/v2.13.0/hclsyntax/walk.go

However, since json.Walk does not exist, it is difficult to fully meet the requirements.