fix(terragrunt_providers_lock, terragrunt_validate): Proper argument handling#933
Merged
MaxymVlasov merged 1 commit intoantonbabenko:masterfrom Sep 27, 2025
Merged
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughInsert an explicit end-of-options marker ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Hook as Hook Script
participant TG as Terragrunt
participant Sub as Subcommand (providers lock / validate)
User->>Hook: Run hook
Hook->>TG: terragrunt run --all -- <subcommand> [args]
note right of TG #DDEBF7: `--` prevents TG from parsing further options
TG->>Sub: Invoke <subcommand> with [args]
Sub-->>TG: Exit code / output
TG-->>Hook: Propagate result
Hook-->>User: Return status
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
hooks/terragrunt_validate.sh (1)
17-21: Add a brief comment to prevent regressions.Document why “--” is required here (Terragrunt parses flags before the separator).
if common::terragrunt_version_ge_0.78; then - local -ra RUN_ALL_SUBCOMMAND=(run --all -- validate) + # NOTE: keep the `--` to ensure flags after it go to Terraform, not Terragrunt. + local -ra RUN_ALL_SUBCOMMAND=(run --all -- validate) else - local -ra RUN_ALL_SUBCOMMAND=(run-all -- validate) + # NOTE: legacy run-all also requires `--` before the underlying command. + local -ra RUN_ALL_SUBCOMMAND=(run-all -- validate) fihooks/terragrunt_providers_lock.sh (1)
17-21: Minor DRY refactor: compose from RUN_PREFIX and SUBCOMMAND.Keeps scripts consistent and reduces duplication across hooks.
- if common::terragrunt_version_ge_0.78; then - local -ra RUN_ALL_SUBCOMMAND=(run --all -- providers lock) - else - local -ra RUN_ALL_SUBCOMMAND=(run-all -- providers lock) - fi + local -ra SUBCOMMAND=(providers lock) + local -a RUN_PREFIX=() + if common::terragrunt_version_ge_0.78; then + RUN_PREFIX=(run --all) + else + RUN_PREFIX=(run-all) + fi + # `--` ensures subsequent flags are forwarded to Terraform. + local -ra RUN_ALL_SUBCOMMAND=("${RUN_PREFIX[@]}" -- "${SUBCOMMAND[@]}")
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hooks/terragrunt_providers_lock.sh(1 hunks)hooks/terragrunt_validate.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
hooks/terragrunt_providers_lock.sh (1)
hooks/terragrunt_validate_inputs.sh (2)
run_hook_on_whole_repo(70-79)main(10-27)
hooks/terragrunt_validate.sh (1)
hooks/terragrunt_validate_inputs.sh (3)
run_hook_on_whole_repo(70-79)main(10-27)per_dir_hook_unique_part(44-62)
🔇 Additional comments (2)
hooks/terragrunt_validate.sh (1)
18-21: LGTM: correct use of “--” to forward args to Terraform under run/run-all.This fixes the -platform parsing issue and keeps behavior consistent across >=0.78 and legacy paths.
hooks/terragrunt_providers_lock.sh (1)
18-21: LGTM: “--” correctly inserted before providers lock.This ensures args like -platform=… reach Terraform in both version branches.
yermulnik
reviewed
Sep 14, 2025
Collaborator
|
@MaxymVlasov JFYI: the Validate PR title is failing and the error is a way too cryptic to me — will you have a chance to look into it please? UPD: Maybe |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
hooks/terragrunt_providers_lock.sh (1)
17-21: Correct use of end-of-options marker for TG ≥ 0.78; keep run-all unchanged for older TGAdding
--afterrun --allis right for forwarding Terraform flags; retainingrun-all providers lockfor < 0.78 avoids deviating from older UX/docs. Consider documenting this inline to prevent regressions.Apply this tiny doc diff:
if common::terragrunt_version_ge_0.78; then - local -ra RUN_ALL_SUBCOMMAND=(run --all -- providers lock) + # TG >= 0.78: use `run --all` and `--` to forward flags to Terraform/OpenTofu. + local -ra RUN_ALL_SUBCOMMAND=(run --all -- providers lock) else - local -ra RUN_ALL_SUBCOMMAND=(run-all providers lock) + # TG < 0.78: keep `run-all` (no standalone `--`) for compatibility with legacy behavior/docs. + local -ra RUN_ALL_SUBCOMMAND=(run-all providers lock) fiPlease sanity‑check both paths with TG v0.77.x and v0.78+ to confirm that hook‑provided
-platformargs are forwarded in “whole repo” runs.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hooks/terragrunt_providers_lock.sh(1 hunks)hooks/terragrunt_validate.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- hooks/terragrunt_validate.sh
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: jannis-a
PR: antonbabenko/pre-commit-terraform#933
File: hooks/terragrunt_providers_lock.sh:18-21
Timestamp: 2025-09-13T19:34:00.281Z
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.281Z
Learnt from: jannis-a
PR: antonbabenko/pre-commit-terraform#933
File: hooks/terragrunt_providers_lock.sh:18-21
Timestamp: 2025-09-13T19:34:00.281Z
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/terragrunt_providers_lock.sh
🔇 Additional comments (1)
hooks/terragrunt_providers_lock.sh (1)
71-73: Invocation is safely quotedArray +
"${args[@]}"preserves argument boundaries; good for flags like-platform=linux_amd64.
yermulnik
approved these changes
Sep 15, 2025
MaxymVlasov
approved these changes
Sep 27, 2025
antonbabenko
pushed a commit
that referenced
this pull request
Sep 27, 2025
## [1.100.1](v1.100.0...v1.100.1) (2025-09-27) ### Bug Fixes * **`terragrunt_providers_lock`, `terragrunt_validate`:** Properly handle arguments passed from terragrunt to TF ([#933](#933)) ([ea46354](ea46354))
Owner
|
This PR is included in version 1.100.1 🎉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Put an
xinto the box if that apply:Description of your changes
Terragrunt's
run/run-allcommand requires a--before commands that get passed to Terraform to proper handle passed arguments. Without the--any arguments passed viaargs: [--arg=-foo]would get interpreted as Terragrunt argument rather than a Terraform argument.The currently used way without
--is just a shortcut:How can we test changes
Try e.g. this configuration:
Without this PR, the above would fail with: