fix(terraform_providers_lock): Logging in which folder hook failed
#950
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds runtime error handling to the Terraform providers lock hook: when Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
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. Comment |
MaxymVlasov
changed the title
terraform_providers_lock): Log in which folder hook failedterraform_providers_lock): Logging in which folder hook failed
There was a problem hiding this comment.
Pull request overview
This PR enhances error logging for the terraform_providers_lock hook by displaying which folder the hook failed in, making it easier to debug issues when working with multiple directories.
Key Changes
- Added error message with directory path when
terraform providers lockcommand fails - Included helpful guidance pointing users to common resolution (running
terraform init)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hooks/terraform_providers_lock.sh (1)
149-153: Critical: Core issue #949 is not addressed—early-exit logic still allows fallthrough to terraform providers lock.The conditional at lines 149–153 remains unchanged and still has the fundamental flaw described in issue #949. When
mode == "only-check-is-current-lockfile-cross-platform"andlockfile_contains_all_needed_shareturns non-zero (invalid/incomplete lockfile), the condition evaluates to false, execution continues to line 158, and the hook runsterraform providers lockanyway—triggering unwantedterraform init.Per issue #949, this block should be restructured to always exit when in that mode, using the exit code from the lockfile check:
- if [ "$mode" == "only-check-is-current-lockfile-cross-platform" ] && - lockfile_contains_all_needed_sha "$platforms_count"; then - - exit 0 - fi + if [ "$mode" == "only-check-is-current-lockfile-cross-platform" ]; then + lockfile_contains_all_needed_sha "$platforms_count" + exit_code=$? + return $exit_code + fiThis ensures the hook does not run
terraform providers lock(and avoid cascadingterraform init) when only checking cross-platform lockfile status in check-only mode.
🧹 Nitpick comments (1)
hooks/terraform_providers_lock.sh (1)
161-165: Error message text has grammar and clarity issues.Minor improvements to user-facing messaging:
- Line 162–163: "you didn't run requiring 'terraform init'" → awkward phrasing
- Suggested revision: "you likely skipped 'terraform init'" or "you need to run 'terraform init' first"
- common::colorify "red" "$dir_path run failed. Detailed error above. - Most common issue is that you didn't run requiring 'terraform init' before running this hook. It can be run by 'terraform_validate' hook - https://github.com/antonbabenko/pre-commit-terraform#terraform_validate - " + common::colorify "red" "$dir_path run failed. See details above. + The most common cause is missing 'terraform init'. You can run it via the terraform_validate hook: https://github.com/antonbabenko/pre-commit-terraform#terraform_validate + "
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
hooks/terraform_providers_lock.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: jannis-a
Repo: antonbabenko/pre-commit-terraform PR: 933
File: hooks/terragrunt_providers_lock.sh:18-21
Timestamp: 2025-09-13T19:34:00.317Z
Learning: In terragrunt hooks, the `--` separator should only be added before Terraform commands that are invoked through Terragrunt (like `providers lock`, `validate`), but not before native Terragrunt commands (like `hcl validate`). The separator tells Terragrunt to pass subsequent arguments to the underlying Terraform command, which is not applicable for Terragrunt's own commands.
📚 Learning: 2025-09-13T19:34:00.317Z
Learnt from: jannis-a
Repo: antonbabenko/pre-commit-terraform PR: 933
File: hooks/terragrunt_providers_lock.sh:18-21
Timestamp: 2025-09-13T19:34:00.317Z
Learning: In terragrunt hooks, the `--` separator should only be added before Terraform commands that are invoked through Terragrunt (like `providers lock`, `validate`), but not before native Terragrunt commands (like `hcl validate`). The separator tells Terragrunt to pass subsequent arguments to the underlying Terraform command, which is not applicable for Terragrunt's own commands.
Applied to files:
hooks/terraform_providers_lock.sh
📚 Learning: 2025-08-12T19:49:13.257Z
Learnt from: actuarysailor
Repo: antonbabenko/pre-commit-terraform PR: 925
File: .pre-commit-hooks.yaml:216-227
Timestamp: 2025-08-12T19:49:13.257Z
Learning: In Terraform projects, most module folders are downloaded dependencies (similar to GitHub Actions) rather than locally maintained code. Users typically want to document only the root module to avoid commit noise from modules they consume but don't maintain. The terraform_docs_docker hook's current design with pass_filenames: false and args targeting the current directory (.) is appropriate for this common single-module repository pattern.
Applied to files:
hooks/terraform_providers_lock.sh
🧬 Code graph analysis (1)
hooks/terraform_providers_lock.sh (1)
hooks/_common.sh (1)
common::colorify(443-464)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: 🧪 Tests / pytest@🐍3.14@windows-2025
- GitHub Check: 🧪 Tests / pytest@🐍3.14@macos-15
- GitHub Check: 🧪 Tests / pytest@🐍3.12@macos-15
- GitHub Check: 🧪 Tests / pytest@🐍3.13@macos-15
- GitHub Check: 🧪 Tests / pytest@🐍3.11@windows-2025
- GitHub Check: 🧪 Tests / pytest@🐍3.11@macos-15
- GitHub Check: 🧪 Tests / pytest@🐍3.10@macos-15
- GitHub Check: pre-commit
🔇 Additional comments (1)
hooks/terraform_providers_lock.sh (1)
160-168: Error propagation and return flow are correctly implemented.The exit code is properly captured, conditionally logged with helpful context, and returned. This ensures failures bubble up correctly and users see actionable guidance.
Co-authored-by: George Yermulnik (Georgii Iermulnik) <[email protected]>
Put an
xinto the box if that apply:Description of your changes
Fixes #949
Related #54
How can we test changes