terraform-linters / tflint

A Pluggable Terraform Linter
Mozilla Public License 2.0
4.98k stars 357 forks source link

Error when var file includes undeclared variables #2072

Closed rpdelaney closed 4 months ago

rpdelaney commented 4 months ago

Introduction

Terraform gives appropriate warnings for values found in tfvars without a corresponding variable in the configuration:

2024-06-28_09-19-19

Proposal

tflint should also raise errors for the same problem.

References

N/A

bendrucker commented 4 months ago

Duplicate of #1877 Resolved #1941

tflint --var 'foo=bar'
Failed to parse variables; <value for var.foo>:1,1-1: Value for undeclared variable; A variable named "foo" was assigned, but the root module does not declare a variable of that name.:

Error: Value for undeclared variable

  on <value for var.foo> line 1:
  (source code not available)

A variable named "foo" was assigned, but the root module does not declare a variable of that name.
rpdelaney commented 4 months ago

@bendrucker I'm sorry, but I'm not sure I understand how this is a solution for me. As a user I want to use tflint in CI to scan for tfvars settings that do not have a corresponding variable. Adding extra command-line switches does not help because that would require me to do the analysis myself, wouldn't it?

rpdelaney commented 4 months ago

Reading the tickets you mentioned, they seem to describe a very different issue than the new feature I'm proposing.

bendrucker commented 4 months ago

Hmm, I looked at --var as a shortcut but it turns out --var-file behaves differently and does not error.

echo 'foo = "bar"' > terraform.tfvars
tflint --var-file terraform.tfvars
# nothing...

1941 says:

Note that the environment variables TFVAR* are not subject to this check. This is the same behavior as Terraform.

Given that the goal is to error in the cases where Terraform prints a warning, this is looking more like a bug/inconsistency.

wata727 commented 4 months ago

See https://github.com/hashicorp/terraform/blob/v1.9.0/internal/backend/backendrun/unparsed_value.go#L58-L61

Reading the above comment, it seems like Terraform intentionally makes this a warning rather than an error. There is room for discussion on whether TFLint should actively raise an error or conservatively keep the same behavior as Terraform.

Personally, I think it might be better to provide it as a rule that can be enabled when needed.

rpdelaney commented 4 months ago

If tflint returns a message with the same log level for this as e.g. when a local variable is declared but unused, that would feel internally consistent, no?

wata727 commented 4 months ago

It's consistent, but I'm not sure if it helps. Unlike Terraform warnings, TFLint logs are intended for debugging, so they may not be useful for finding typos or other problems.

In your use case, does this help?