Skip to content

Conversation

@jannis-a
Copy link
Contributor

Put an x into the box if that apply:

  • This PR introduces breaking change.
  • This PR fixes a bug.
  • This PR adds new functionality.
  • This PR enhances existing functionality.

Description of your changes

Terragrunt's run/run-all command requires a -- before commands that get passed to Terraform to proper handle passed arguments. Without the -- any arguments passed via args: [--arg=-foo] would get interpreted as Terragrunt argument rather than a Terraform argument.

The currently used way without -- is just a shortcut:

❯ terragrunt run --help Usage: terragrunt run [options] -- <tofu/terraform command> ... Examples: # Run a plan terragrunt run -- plan # Shortcut: # terragrunt plan 

How can we test changes

Try e.g. this configuration:

repos: - repo: https://github.com/antonbabenko/pre-commit-terraform hooks: - id: terragrunt_providers_lock args: - --args=-platform=linux_amd64 - --args=-platform=darwin_arm64

Without this PR, the above would fail with:

Terragrunt providers lock................................................Failed - hook id: terragrunt_providers_lock - exit code: 1 ERRO[0000] flag provided but not defined: -platform 
@coderabbitai
Copy link

coderabbitai bot commented Sep 13, 2025

📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • Improved compatibility with newer Terragrunt versions when running multi-module actions.
  • Bug Fixes
    • Resolved misinterpretation of options during multi-module “providers lock” and “validate” runs, ensuring arguments are passed to the underlying commands correctly.
    • Reduces unexpected failures and improves reliability for runs that include additional arguments.
  • Chores
    • Maintains backward compatibility with older Terragrunt versions without changing user workflows.

Walkthrough

Insert an explicit end-of-options marker (--) into RUN_ALL_SUBCOMMAND in two Terragrunt hook scripts so top-level Terragrunt options stop consuming arguments intended for the underlying subcommand.

Changes

Cohort / File(s) Summary
Terragrunt hooks: add end-of-options marker
hooks/terragrunt_providers_lock.sh, hooks/terragrunt_validate.sh
For Terragrunt >= 0.78 branch, change RUN_ALL_SUBCOMMAND from run --all <subcmd> to run --all -- <subcmd> so final invocations become terragrunt run --all -- .... The older run-all <subcmd> path is unchanged. No other logic alterations.

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 
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

hook/terragrunt_providers_lock, hook/terragrunt_validate

Suggested reviewers

  • antonbabenko
  • yermulnik
  • MaxymVlasov
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed The PR description clearly explains the bug and root cause (missing -- separator for terragrunt run/run-all), includes a concrete reproduction example with the exact failing error message, and identifies the change as a bug fix; this directly relates to the changes in the hooks and provides sufficient context for reviewers to verify the fix. It also provides a test configuration and expected behavior which aids validation. Overall the description is relevant and actionable.
Title Check ✅ Passed The title clearly identifies this as a bug fix affecting the terragrunt_providers_lock and terragrunt_validate hooks and concisely communicates the intent to fix argument handling, so it aligns with the PR objectives and changes. It does include a trailing "x" that appears to be a typo and slightly reduces clarity.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) fi
hooks/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

📥 Commits

Reviewing files that changed from the base of the PR and between 48b3a29 and 57235d9.

📒 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 yermulnik changed the title fix: proper argument handling for terragrunt hooks fix: Proper argument handling for terragrunt hooks Sep 14, 2025
@yermulnik
Copy link
Collaborator

yermulnik commented Sep 14, 2025

@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 pull_request should be pull_request_target? (but I'm not entirely sure)
Refs:

@jannis-a jannis-a changed the title fix: Proper argument handling for terragrunt hooks fix(terragrunt_providers_lock, terragrunt_validate): Proper argument handling Sep 15, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 TG

Adding -- after run --all is right for forwarding Terraform flags; retaining run-all providers lock for < 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) fi

Please sanity‑check both paths with TG v0.77.x and v0.78+ to confirm that hook‑provided -platform args are forwarded in “whole repo” runs.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 57235d9 and aeb18a0.

📒 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 quoted

Array + "${args[@]}" preserves argument boundaries; good for flags like -platform=linux_amd64.

@jannis-a jannis-a changed the title fix(terragrunt_providers_lock, terragrunt_validate): Proper argument handling fix(terragrunt_providers_lock, terragrunt_validate): Proper argument handlingx Sep 15, 2025
@jannis-a jannis-a changed the title fix(terragrunt_providers_lock, terragrunt_validate): Proper argument handlingx fix(terragrunt_providers_lock, terragrunt_validate): Proper argument handling Sep 15, 2025
@MaxymVlasov MaxymVlasov merged commit ea46354 into antonbabenko:master Sep 27, 2025
42 of 45 checks passed
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))
@antonbabenko
Copy link
Owner

This PR is included in version 1.100.1 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants