feat(INT-83): dial in configs for tflint #39
Conversation
westonplatter
changed the title
WalkthroughThis set of changes introduces a new Possibly related PRs
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
examples/complete/variables.tf (1)
2-4: Enforce Terraform CLI version constraint
Good addition ofrequired_version = ">= 1.0.0"to ensure compatibility. Consider also specifyingrequired_providerswithin this block for completeness if not declared elsewhere.README.md (3)
11-14: Clarify repository purpose
Clear description added. Consider minor rephrasing or punctuation for readability and consistent capitalization (e.g., "Terraform modules").
16-17: Improve grammar and capitalization
The phrase "while when writing terraform + open tofu" is awkward. Suggest:-It comes pre-packaged with Masterpoint's standard Open Source tool set that our -team uses to operate effectively while when writing terraform + open tofu. +It comes pre-packaged with Masterpoint's standard open source tool set that our +team uses when writing Terraform and OpenTofu, helping us operate effectively.
19-30: Ensure consistent tool name casing
Tool names should follow official branding. For example, capitalize acronyms and product names:- trunk: Trunk CLI for managing code quality and automation (linters and pre-commit actions) - tflint: Terraform linter for best practices and errors - renovate: Automated dependency updates - tofu: OpenTofu linter for Terraform alternative - actionlint: Linter for GitHub Actions workflows - checkov: Infrastructure as Code (IaC) security scanner - git-diff-check: Checks for issues in git diffs - markdownlint: Linter for Markdown files - prettier: Code formatter for consistent style - trivy: Vulnerability scanner for containers and other artifacts - trufflehog: Secret and sensitive data scanner - yamllint: Linter for YAML files + Trunk: Trunk CLI for managing code quality and automation (linters and pre-commit actions) + TFLint: Terraform linter for best practices and errors + Renovate: Automated dependency updates + OpenTofu: OpenTofu linter for Terraform alternative + ActionLint: Linter for GitHub Actions workflows + Checkov: Infrastructure as Code (IaC) security scanner + git-diff-check: Checks for issues in git diffs + markdownlint: Linter for Markdown files + Prettier: Code formatter for consistent style + Trivy: Vulnerability scanner for containers and other artifacts + TruffleHog: Secret and sensitive data scanner + yamllint: Linter for YAML files.trunk/configs/.tflint.hcl (5)
1-4: Consider refining the Terraform plugin preset
Usingpreset = "all"enables every rule (including deprecated ones) and can generate noise. If you want a more focused set of checks, consider using therecommendedpreset and then explicitly enabling additional rules as needed.
6-11: Remove redundant default settings inconfigblock
The fieldsforce = falseanddisabled_by_default = falseare the defaults in TFLint; you can omit them for brevity. Keeping only non-default configurations (e.g.,formatandcall_module_type) improves readability.
13-26: Clarify AWS plugin installation instructions and consider enabling deep checks
The comment correctly explains the need forGITHUB_TOKENto fetch the AWS ruleset. You may also want to enabledeep_check = truefor more thorough linting of nested AWS module configurations, at the cost of performance.
Diff suggestion:plugin "aws" { enabled = true version = "0.39.0" source = "github.com/terraform-linters/tflint-ruleset-aws" - deep_check = false + deep_check = true }
29-71: Consolidate and organize Terraform core rule definitions
Since you havepreset = "all", many of these rules are already enabled by default. To reduce maintenance overhead, consider removing redundantenabled = trueentries for rules covered by the preset, or switch topreset = "recommended". Additionally, grouping rules alphabetically or by category (core vs. AWS) and standardizing indentation (e.g., 2 spaces vs. 4 spaces) will improve readability.
82-98: Standardize grouping and indentation of additional rules
These rules (terraform_typed_variables,terraform_unused_declarations, etc.) enhance code quality. For consistency, align their indentation with earlier blocks and consider grouping them under a clear header (e.g., “Core Rules” vs. “AWS Rules”) to make future updates more discoverable.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/lint.yaml(1 hunks).gitignore(2 hunks).trunk/configs/.tflint.hcl(1 hunks).trunk/trunk.yaml(1 hunks)README.md(3 hunks)examples/complete/variables.tf(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.tf`: You're a Terraform expert who has thoroughly studied all the documentation from Hashicorp https://developer.hashicorp.com/terraform/docs and OpenTofu https://opentofu.or...
**/*.tf: You're a Terraform expert who has thoroughly studied all the documentation from Hashicorp https://developer.hashicorp.com/terraform/docs and OpenTofu https://opentofu.org/docs/.
You have a strong grasp of Terraform syntax and prioritize providing accurate and insightful code suggestions.
As a fan of the Cloud Posse / SweetOps ecosystem, you incorporate many of their best practices https://docs.cloudposse.com/best-practices/terraform/ while balancing them with general Terraform guidelines.
examples/complete/variables.tf
🔇 Additional comments (5)
.gitignore (1)
19-19: Exclude.cursor/artifacts
Adding.cursor/is helpful to ignore editor-specific cursor cache files. Ensure this aligns with other IDE/Editor sections..github/workflows/lint.yaml (1)
23-27: InjectGITHUB_TOKENfor Trunk-managed TFLint
Nice update to passGITHUB_TOKENand avoid rate limiting for TFLint plugin installation. The existing permissions (checks: write, etc.) support this usage..trunk/trunk.yaml (2)
30-30: Upgrade TFLint to v0.57.0
Upgrading to 0.57.0 ensures the latest linter rules and fixes. Confirm compatibility with existing.tflint.hclconfigurations.
34-38: DefineTFLINT_CONFIG_FILEenvironment variable
Centralizing the TFLint config path viaTFLINT_CONFIG_FILEaligns with Trunk conventions. Ensure${workspace}resolves correctly in CI context..trunk/configs/.tflint.hcl (1)
78-80: Disablingterraform_standard_module_structureis valid for contextual variable files
Given your module layout use cases, disabling this rule makes sense. Ensure that team members are aware of this exception to avoid surprises.
|
Note to self while out and about:
|
| - name: tflint | ||
| environment: | ||
| - name: TFLINT_CONFIG_FILE | ||
| value: ${workspace}/.trunk/configs/.tflint.hcl |
There was a problem hiding this comment.
Instead of doing this, do we just want to include it at the root of the project so that it can be picked up like other configs and isn't hidden in the trunk folder?
There was a problem hiding this comment.
I wondered about that.
I looked at a other linter configs in the .trunk/configs folder, and thought bending the configurations to keep the convention of storing the configs in the same place would be valuable for downstream consumers.
.trunk/configs/.tflint.hcl
Outdated
| call_module_type = "local" | ||
| force = false |
There was a problem hiding this comment.
call_module_type = "local" - ignore values passed into modules. I think we actually want to lint the values in module blocks; changing this to all. More details here.
force = false - If set to true, tflint process will "return zero exit status even if issues found". My guess is that this setting could be used to keep a CI/CD pipeline running regardless of lint issues. I think we do want the pipeline to fail if linting issues exist?
disabled_by_default = false - false means all rules in the rule sets are active. More info.
what
Summary by CodeRabbit
New Features
Chores