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

TFLint skips expressions that reference "each" #1139

Closed
bendrucker opened this issue Jun 22, 2021 · 1 comment · Fixed by #1537
Closed

TFLint skips expressions that reference "each" #1139

bendrucker opened this issue Jun 22, 2021 · 1 comment · Fixed by #1537

Comments

@bendrucker
Copy link
Member

Similar to #571, TFLint will not evaluate any attribute that references each:

resource "foo" "bar" {
  for_each = toset(["a", "b"])

  attr = each.key
}

In theory, TFLint could attempt to evaluate the expression and determine whether its value is wholly "known" or not. If it is known, any expression containing each could be evaluated for all the for_each keys. This is a fair amount of effort to reproduce low level behavior of Terraform and should only be considered once #937 is released/stabilized.

TFLint already does have some smarts around for_each and will skip evaluation entirely for resources where count or for_each are either unevaluable or empty:

https://github.com/terraform-linters/tflint/search?q=for_each

This prevents false positives for conditional resources. Avoiding false positives is more important (and usually easier) than maximizing expression coverage for complex (but state-less) expressions.

@wata727
Copy link
Member

wata727 commented Sep 24, 2022

The challenge in supporting each.* is that the evaluated value has a union type. The EvaluateExpr resolves to a cty.Value, but cty.Value doesn't support union type.

The first approach is to change the interface of EvaluateExpr. As mentioned in terraform-linters/tflint-plugin-sdk#196, change to a design that registers a callback. The callback can be invoked multiple times for each.* expressions. The problem with this approach is that it will cause incorrect results if passed a callback that is not meant to be called multiple times. The behavior that a callback may be called multiple times may not be intuitive.

The second approach is to expand resources, similar to Terraform. The GetModuleContent returns multiple blocks depending on the count and for_each. This approach requires evaluating each.* during the GetModuleContent stage to assign different values to each block. The evaluated value can be returned as an expression that satisfies hcl.Expression, but we need to consider how to support encoding/decoding.

I think the latter is the better design, but needs a little more research.

EDIT: We can solve the serialization issue by adding an expr_value and decoding expr as a pre-evaluated expression if it has this value.
https://github.com/terraform-linters/tflint-plugin-sdk/blob/v0.13.0/plugin/proto/tflint.proto#L200-L201
https://github.com/terraform-linters/tflint-plugin-sdk/blob/v0.13.0/plugin/proto/tflint.proto#L151-L152

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants