feat: Include .tfbackend files in hooks #822
Conversation
As per the official docs https://developer.hashicorp.com/terraform/language/backend#file: > *.backendname.tfbackend (e.g. config.consul.tfbackend) is the recommended naming pattern. Terraform will not prevent you from using other names but following this convention will help your editor understand the content and likely provide better editing experience as a result.
|
Warning Rate limit exceeded@thiagowfx has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 58 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis change updates the Changes
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
|
thiagowfx
changed the title
thiagowfx
changed the title
![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 (1)
.pre-commit-hooks.yaml (1)
1-9: Removal of the Infracost Breakdown HookIt appears that the
infracost_breakdownhook has been removed. Please confirm that this hook is no longer required in your workflow. If it was used elsewhere or its removal might affect downstream processes, provide migration guidance or update documentation accordingly.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.pre-commit-hooks.yaml(1 hunks)
🔇 Additional comments (18)
.pre-commit-hooks.yaml (18)
10-18: Update to Terraform fmt File PatternThe
terraform_fmthook’s file pattern is now updated to include the.tfbackendfile extension:files: (\.tf|\.tfbackend|\.tfvars)$This correctly matches files ending with
.tf,.tfbackend, or.tfvars. Please double-check that this regex meets the expected naming convention and that there isn’t any unintended overlap with similar file extensions.
19-29: Removal of Terraform Docs HookThe
terraform_docshook has been removed. Ensure that removing documentation generation via terraform-docs is intentional. If users relied on this functionality, consider providing a notice or migration path.
30-40: Removal of the Terraform Docs (Without Aggregate Defaults) HookSimilarly, the
terraform_docs_without_aggregate_type_defaultshook has been removed. This helps in reducing redundancy if it was duplicative ofterraform_docs. Please verify that this removal does not impact users who might have depended on the slight differences in behavior.
41-49: Removal of the Terraform Docs Replace HookThe hook
terraform_docs_replace(which overwrites README.md with terraform-docs output) has been removed. Confirm that this functionality is no longer needed or has been handled elsewhere.
50-58: Removal of the Terraform Validate HookThe
terraform_validatehook has been removed. Verify that any necessary validation for Terraform configuration files is now performed by another tool or hook to ensure configuration integrity.
59-67: Removal of the Terraform Providers Lock HookThe hook for locking Terraform provider versions (
terraform_providers_lock) is removed. Make sure that provider version management is handled appropriately elsewhere or that its removal is a deliberate decision.
68-76: Removal of the Terraform TFLint HookThe removal of the
terraform_tflinthook implies that static analysis with TFLint will no longer be available via this pre-commit configuration. Confirm that this deprecation aligns with your newer static analysis choices.
77-85: Removal of the Terragrunt fmt HookThe
terragrunt_fmthook has been removed. Please ensure that any format standardization for Terragrunt files is either no longer necessary or is handled by a different mechanism.
86-93: Removal of the Terragrunt Validate HookSimilarly, the
terragrunt_validatehook is removed. If Terragrunt configuration validation is still required, confirm it is provided by another hook or tool.
94-101: Removal of the Terragrunt Validate Inputs HookThe removal of the
terragrunt_validate_inputshook suggests that validating inputs for Terragrunt is no longer performed in this configuration. Please verify that this is an intended change.
102-110: Removal of the Terragrunt Providers Lock HookThis segment shows the removal of the
terragrunt_providers_lockhook. Ensure that dependency lock management for Terragrunt is either deprecated or superseded by other processes.
111-119: Deprecation of the Terraform tfsec HookThe
terraform_tfsechook is being deprecated (and removed in this diff) in favor of the newterraform_trivyhook. It’s good practice to remove deprecated hooks once users have been given sufficient notice. Verify that any documentation or upgrade guides have been updated to reflect this change.
120-128: Addition of the Terraform Trivy HookA new
terraform_trivyhook has been introduced:id: terraform_trivy name: Terraform validate with trivy ... files: \.tf(vars)?$This looks like an appropriate replacement for tfsec. Please ensure that the regex and configuration match your requirements and that the environment has Trivy installed.
129-139: Deprecation and Removal of the Checkov HookThe original
checkovhook has been removed, which is in line with the newterraform_checkovhook introduction. Verify that users are aware of this migration.
140-149: Addition of the Terraform Checkov HookThe new
terraform_checkovhook has been added to run Checkov on Terraform templates:id: terraform_checkov name: Checkov entry: hooks/terraform_checkov.sh ... files: \.tf$This update should provide equivalent functionality as the deprecated hook. Ensure that its integration has been tested and documented.
150-160: Removal of the Terraform Wrapper Module for Each HookThe hook
terraform_wrapper_module_for_eachhas been removed. Confirm that this functionality is either obsolete or has been integrated into another process.
161-169: Removal of the Terrascan HookThe
terrascanhook has been removed. Please verify that this static analysis tool is no longer part of your workflow or has been replaced.
170-179: Removal of the tfupdate HookFinally, the
tfupdatehook has also been removed. Double-check that automatic updates for Terraform templates are handled appropriately if needed.
|
Hi, so it is a new possibility in terraform to define backend. Interesting. There is a chance that the same changes need to be included in these hooks:
But need to test it. Would much appreciate it if you'll handle it. If not - I'll do it later by myself. |
MaxymVlasov
added
feature
|
My understanding is that ...etc. Formatting them makes sense ( I am not completely sure whether the other hooks make sense, would need to look deeper. I would be happy to send more PRs later on if it turns out they make sense. What do you think? |
Do the other hooks take TF backend configuration into account at all? 🤔 If yes, then, yep, we'd better off adding it to those that are aware of this type of files 👍🏻 |
Co-authored-by: George L. Yermulnik <[email protected]>
|
I was thinking about this. Perhaps it makes sense to add it to all hooks that have |
thiagowfx
changed the title
|
Updated the file + PR title + PR description. |
Underlying tools infer values for variables from tfvars, whereas I can't say for sure how they will behave encountering "variables" in tfbackend files as these "variables" ain't declared by means of As @MaxymVlasov already inquired: would you be able to add change to |
yermulnik
dismissed
their stale review
Take approval back as it was for TF fmt only and we need to be sure that other hooks don't get confused with the tfbackend vars.
|
I am only currently using For the other 4 hooks, are there existing samples / tests somewhere I could use for testing? Footnotes
|
|
Oh wait, I take it back for It should be possible to do so. Setting this back to a Draft for now. |
|
Upstream FR / bug: hashicorp/terraform#36564 |
It should be as simple as adding each of those hooks to your repos:
- repo: https://github.com/antonbabenko/pre-commit-terraform
rev: 837b9ae2b24688c9cc3727fcf717c8494c421ee6 # commit that you want to test against
hooks:
- id: terraform_fmt
- id: terraform_validate
- id: terraform_tflint
- id: terraform_docs
- id: terraform_tfsec
- id: terraform_trivy
#- id: infracost_breakdown # I don't have a API key :shrugging:So for me none fails (and even # running with `0dad06983ea30273c52be9c3c443ad5970de0a90`
> pre-commit run -a
[INFO] Initializing environment for https://github.com/antonbabenko/pre-commit-terraform.
Terraform fmt...........................................................................Passed
Terraform validate......................................................................Passed
Terraform validate with tflint..........................................................Passed
Terraform docs......................................................(no files to check)Skipped
Terraform validate with tfsec (deprecated, use "terraform_trivy").......................Passed
Terraform validate with trivy...........................................................PassedI'm not sure whether we can enforce each hook to run over specific file(s) instead. @MaxymVlasov can assist perceiving this. Else we should probably be safe with this PR (including TF fmt hook). |
|
About hook examples and how to test it. Good news that we can do it right now (and I'll probably do it anyway, glad you asked, as I tired to test changes somehow locally with hope that I cover everything - that's far from ideal) I can't say ETA when I'll do that, but it will not take more than 1 workday, just need to find that time |
Put an
xinto the box if that apply:Description of your changes
As per the official docs https://developer.hashicorp.com/terraform/language/backend#file:
How can we test changes
Add
foo.tfbackend(a simple HCL file, without blocks, similar to.tfvars), run e.g.pre-commit run --all-files terraform_fmt.