feat(lint): Lint TF with tflint#828
Conversation
stevendee-recon
requested review from
JesseTatasciore and
jbedard
as code owners
|
|
|
This enables linting Terraform projects with [tflint]. The approach taken here is agnostic to the specific TF (Terraform / OpenTofu) ruleset used; plain filegroups work, as does anything that generates `tf_module` and/or `tf_environment` rule kinds (or any configurable rule kind.) `tflint` plugins are supported via a repository rule that vendors them at specific hashes to account for terraform-linters/tflint#1634. Lint runs are done hermetically out of a staging directory, with sources and plugins symlinked in as needed. I have left the Starzelle plugin and HCL tree-sitter work out of this PR, to be done elsewhere. This code was done heavily AI-assisted, but I have attempted to review it and comprehend it, and am using it elsewhere. Part of aspect-build#325. [tflint]: https://github.com/terraform-linters/tflint Co-authored-by: Claude <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3fd0ac36b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "_plugins": attr.label_list( | ||
| default = plugins, | ||
| ), |
There was a problem hiding this comment.
The _plugins attribute is declared without cfg = "exec", so plugin labels are analyzed in the target configuration even though their binaries are loaded at execution time by tflint. With cross-platform builds/executors, this can select the wrong plugin artifact from @tflint_plugin_*//:plugin and cause plugin load failures (for example exec format error) when the lint action runs.
Useful? React with 👍 / 👎.
| copy_cmds = ["ln -s $PWD/{src} {dir}/{basename}".format( | ||
| src = src.path, | ||
| dir = staging_dir, | ||
| basename = src.basename, | ||
| ) for src in srcs] |
There was a problem hiding this comment.
Each Terraform source is symlinked into the staging directory by src.basename only, which drops directory structure. If a target includes files from different directories with the same filename (for example module_a/main.tf and module_b/main.tf), one symlink conflicts and that file is not staged, so lint coverage becomes incomplete or incorrect for tagged filegroups/multi-module targets.
Useful? React with 👍 / 👎.
|
|
||
| # Determine the common package prefix for the source files so we can | ||
| # rewrite tflint's output paths back to workspace-relative locations. | ||
| src_prefix = srcs[0].short_path.rsplit("/", 1)[0] + "/" if srcs and "/" in srcs[0].short_path else "" |
There was a problem hiding this comment.
Did you copy this pattern from somewhere? Isn't it possible that srcs[0] doesn't have the "common package prefix" and srcs[1] has something else?
| # Symlink .tf sources into the scratch directory (flat — tflint lints one | ||
| # module directory at a time). Absolute paths ensure symlinks resolve after | ||
| # --chdir into the staging directory. | ||
| copy_cmds = ["ln -s $PWD/{src} {dir}/{basename}".format( |
There was a problem hiding this comment.
Is the heavy use of symlinks and creating the .tflint.d/ really the best way of doing this? Why doesn't this directory already exist in the correct format? Why do we have to create it only at lint-time?
|
Thank you for the review.
Wish I could tell you. Claude came up with it; I was insufficiently skeptical. FWIW I think what broke in my mind, as reviewer, was something like "aspect lint must not respect Bazel's sandboxing so extra effort is needed to make a lint run hermetic." Will take a pass, removing the unnecessary copies and staging the plugin repository rule appropriately. |
3 participants
This enables linting Terraform projects with tflint. The approach taken here is agnostic to the specific TF (Terraform / OpenTofu) ruleset used; plain filegroups work, as does anything that generates
tf_moduleand/ortf_environmentrule kinds (or any configurable rule kind.)tflintplugins are supported via a repository rule that vendors them at specific hashes to account for terraform-linters/tflint#1634.Lint runs are done hermetically out of a staging directory, with sources and plugins symlinked in as needed.
I have left the Starzelle plugin and HCL tree-sitter work out of this PR, to be done elsewhere.
This code was done heavily AI-assisted, but I have attempted to review it and comprehend it, and am using it elsewhere.
Part of #325.
Changes are visible to end-users: yes
Add support for linting TF (Terraform/OpenTofu) modules
Test plan