feat: Add Terraform pre-commit hooks (fmt --check & validate), skip examples, improve output, update README#20
Conversation
…xamples, improve output, update README
WalkthroughAdds a new Terraform validate pre-commit hook and script, updates the Terraform fmt hook and script to check formatting without writing, refines exclude patterns and serialization, and updates README usage instructions and examples accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant PC as pre-commit
participant FMT as terraform-fmt.sh
participant VAL as terraform-validate.sh
participant TF as terraform CLI
Dev->>PC: git commit
PC->>FMT: Run on staged .tf files
rect rgb(229,246,253)
note right of FMT: Check formatting (-check -diff)
FMT->>TF: terraform fmt -check -diff
TF-->>FMT: status/output
alt Any file not formatted or error
FMT-->>PC: Non-zero exit with diffs/errors
PC-->>Dev: Abort commit
else All files formatted
FMT-->>PC: Success
end
end
PC->>VAL: Run per directory with .tf files
rect rgb(235,252,232)
VAL->>TF: terraform init -backend=false -input=false
TF-->>VAL: status/output
alt Auth/registry errors detected
note right of VAL: Mark directory/files as skipped
VAL-->>PC: Continue (no failure)
else Other init failure
VAL-->>PC: Record failure (non-zero later)
else Init OK
VAL->>TF: terraform validate
TF-->>VAL: status/output
alt Validate OK
VAL-->>PC: Record success
else Validate failed
VAL-->>PC: Record failure
end
end
end
alt Any failures recorded
PC-->>Dev: Abort commit
else All hooks passed/skipped
PC-->>Dev: Commit succeeds
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
hooks/terraform-fmt.sh (2)
16-19: Guard directory navigation failures (pushd/popd).If pushd/popd fails (e.g., deleted/renamed path), the script continues with misleading output. Guard and propagate failure safely.
- pushd "$dir" >/dev/null + if ! pushd "$dir" >/dev/null; then + echo -e " ${RED}✗ cannot enter directory: ${dir}${RST}" + exit_code=1 + continue + fi out="$(terraform fmt -check -diff -no-color "$base" 2>&1)" status=$? - popd >/dev/null + popd >/dev/null || { echo -e " ${RED}✗ popd failed${RST}"; exit 1; }
5-6: Drop unused color or use it consistently; unify message language.YLW is unused; messages are in Spanish while README is in English. Prefer consistency (English) and remove the unused color.
-RED='\033[31m'; GRN='\033[32m'; YLW='\033[33m'; BLD='\033[1m'; RST='\033[0m' -if [[ "${PRE_COMMIT_COLOR:-}" == "never" ]]; then RED=''; GRN=''; YLW=''; BLD=''; RST=''; fi +RED='\033[31m'; GRN='\033[32m'; BLD='\033[1m'; RST='\033[0m' +if [[ "${PRE_COMMIT_COLOR:-}" == "never" ]]; then RED=''; GRN=''; BLD=''; RST=''; fi- echo -e "${BLD}→ ${f}${RST}" + echo -e "${BLD}→ ${f}${RST}" @@ - echo -e " ${GRN}✓ Formato correcto${RST}" + echo -e " ${GRN}✓ Already formatted${RST}" @@ - echo -e " ${RED}✗ Error de sintaxis (terraform fmt)${RST}" + echo -e " ${RED}✗ Syntax error (terraform fmt)${RST}" @@ - echo -e " ${RED}✗ No está formateado${RST} (aplicá 'terraform fmt')" + echo -e " ${RED}✗ Not formatted${RST} (run 'terraform fmt')"hooks/terraform-validate.sh (3)
24-24: Guard pushd/popd to avoid silent misbehavior on bad paths.Protect directory changes to prevent confusing output and to keep exit_code correct.
- pushd "${d}" >/dev/null + if ! pushd "${d}" >/dev/null; then + echo -e " ${RED}✗ cannot enter directory: ${d}${RST}" + exit_code=1 + echo + continue + fi @@ - popd >/dev/null + popd >/dev/null || { echo -e " ${RED}✗ popd failed${RST}"; exit 1; }Also applies to: 71-71
26-30: Remove set -e toggling; rely on captured statuses.The script isn’t using set -e globally, so toggling adds complexity without benefit. Explicit status capture is already in place.
- set +e out_init="$(terraform init -backend=false -input=false 2>&1)" st_init=$? - set -e @@ - # ---------- VALIDATE ---------- - set +e out_val="$(terraform validate "${VALIDATE_ARGS[@]}" 2>&1)" st_val=$? - set -eAlso applies to: 53-57
23-24: Unify messages in English for consistency with README.Consistent language improves DX and logs.
- echo -e "${BLD}>> Validando Terraform en: ${d}${RST}" + echo -e "${BLD}>> Validating Terraform in: ${d}${RST}" @@ - echo -e " ${YLW}! skipped${RST} (requires private registry auth)" + echo -e " ${YLW}! skipped${RST} (requires private registry auth)" @@ - echo -e " ${YLW}! skipped${RST}" + echo -e " ${YLW}! skipped${RST}" @@ - echo -e " ${GRN}✓ Válido${RST}" + echo -e " ${GRN}✓ Valid${RST}" @@ - echo -e " ${RED}✗ validate failed${RST}" + echo -e " ${RED}✗ validate failed${RST}"Also applies to: 31-39, 58-66
.pre-commit-hooks.yaml (1)
3-6: Align terraform-fmt metadata with check-only behavior.The script now runs terraform fmt --check -diff; the description (and optionally name) should reflect that to avoid confusion.
-- id: terraform-fmt - name: Terraform fmt - description: Rewrites all Terraform configuration files to a canonical format +- id: terraform-fmt + name: Terraform fmt (check) + description: Checks Terraform formatting (runs 'terraform fmt --check -diff')README.md (2)
24-24: Add a short prerequisites note (Terraform installed).Users need terraform on PATH for these hooks to run.
Next, have every developer: + +Note: Ensure Terraform is installed and available on PATH (tested with Terraform ≥ 1.0).
27-28: Unify list marker style to satisfy markdownlint (MD004).Switch dashes to asterisks to match the rest of the doc.
- - macOS: `brew install pre-commit` - - Linux: `pipx install pre-commit` (or `pip install --user pre-commit`) + * macOS: `brew install pre-commit` + * Linux: `pipx install pre-commit` (or `pip install --user pre-commit`)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.pre-commit-hooks.yaml(1 hunks)README.md(1 hunks)hooks/terraform-fmt.sh(1 hunks)hooks/terraform-validate.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
hooks/terraform-fmt.sh
[warning] 6-6: YLW appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 16-16: Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails.
(SC2164)
[warning] 19-19: Use 'popd ... || exit' or 'popd ... || return' in case popd fails.
(SC2164)
🪛 markdownlint-cli2 (0.17.2)
README.md
27-27: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
28-28: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
🔇 Additional comments (2)
.pre-commit-hooks.yaml (1)
9-10: Hook config looks solid.Good excludes for .terraform/ and examples/, and require_serial avoids concurrent terraform executions.
Also applies to: 12-19
README.md (1)
6-8: Docs align well with behavior (fmt check + validate + skip semantics).Clear, developer-friendly guidance. Nice.
| for file in "$@"; do | ||
| pushd "$(dirname "$file")" >/dev/null | ||
| terraform fmt -write=true "$(basename "$file")" | ||
| exit_code=0 |
There was a problem hiding this comment.
Fail fast when Terraform is not installed.
Without terraform, status=127 gets treated as “Not formatted,” which is confusing. Detect the binary early and exit.
exit_code=0
+
+# Ensure Terraform is available
+if ! command -v terraform >/dev/null 2>&1; then
+ echo -e "${RED}Error:${RST} terraform is not installed or not in PATH."
+ exit 127
+fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| exit_code=0 | |
| exit_code=0 | |
| # Ensure Terraform is available | |
| if ! command -v terraform >/dev/null 2>&1; then | |
| echo -e "${RED}Error:${RST} terraform is not installed or not in PATH." | |
| exit 127 | |
| fi |
🤖 Prompt for AI Agents
In hooks/terraform-fmt.sh around line 8, the script currently continues when
terraform is missing which causes a status 127 to be misinterpreted as “Not
formatted”; modify the script to fail fast by checking for the terraform binary
early (e.g., using command -v terraform or which terraform) and if not found,
print a clear error to stderr and exit with a non-zero status (127 or another
distinct code) before proceeding with formatting checks.
| VALIDATE_ARGS=() | ||
| [[ "${PRE_COMMIT_COLOR:-}" == "never" ]] && VALIDATE_ARGS+=("-no-color") | ||
|
|
||
| if [[ "$#" -gt 0 ]]; then | ||
| mapfile -t DIRS < <(for f in "$@"; do dirname "$f"; done | sort -u) | ||
| else | ||
| mapfile -t DIRS < <(git ls-files '*.tf' | xargs -n1 dirname | sort -u) | ||
| fi |
There was a problem hiding this comment.
Fail fast when Terraform (and optionally git) is missing.
Avoid misleading output when prerequisites aren’t available.
VALIDATE_ARGS=()
[[ "${PRE_COMMIT_COLOR:-}" == "never" ]] && VALIDATE_ARGS+=("-no-color")
+# Ensure Terraform is available
+if ! command -v terraform >/dev/null 2>&1; then
+ echo -e "${RED}Error:${RST} terraform is not installed or not in PATH."
+ exit 127
+fi
+
if [[ "$#" -gt 0 ]]; then
mapfile -t DIRS < <(for f in "$@"; do dirname "$f"; done | sort -u)
else
+ # git is required only for repo-wide discovery
+ if ! command -v git >/dev/null 2>&1; then
+ echo -e "${RED}Error:${RST} git is not installed or not in PATH."
+ exit 127
+ fi
mapfile -t DIRS < <(git ls-files '*.tf' | xargs -n1 dirname | sort -u)
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| VALIDATE_ARGS=() | |
| [[ "${PRE_COMMIT_COLOR:-}" == "never" ]] && VALIDATE_ARGS+=("-no-color") | |
| if [[ "$#" -gt 0 ]]; then | |
| mapfile -t DIRS < <(for f in "$@"; do dirname "$f"; done | sort -u) | |
| else | |
| mapfile -t DIRS < <(git ls-files '*.tf' | xargs -n1 dirname | sort -u) | |
| fi | |
| VALIDATE_ARGS=() | |
| [[ "${PRE_COMMIT_COLOR:-}" == "never" ]] && VALIDATE_ARGS+=("-no-color") | |
| # Ensure Terraform is available | |
| if ! command -v terraform >/dev/null 2>&1; then | |
| echo -e "${RED}Error:${RST} terraform is not installed or not in PATH." | |
| exit 127 | |
| fi | |
| if [[ "$#" -gt 0 ]]; then | |
| mapfile -t DIRS < <(for f in "$@"; do dirname "$f"; done | sort -u) | |
| else | |
| # git is required only for repo-wide discovery | |
| if ! command -v git >/dev/null 2>&1; then | |
| echo -e "${RED}Error:${RST} git is not installed or not in PATH." | |
| exit 127 | |
| fi | |
| mapfile -t DIRS < <(git ls-files '*.tf' | xargs -n1 dirname | sort -u) | |
| fi |
🤖 Prompt for AI Agents
In hooks/terraform-validate.sh around lines 8 to 15, the script proceeds to use
terraform and git without verifying they exist which can produce confusing
output; add upfront checks and fail fast: test for terraform with "command -v
terraform >/dev/null 2>&1 || { echo 'terraform is required but not installed'
>&2; exit 1; }" before any terraform usage, and when the no-args branch intends
to call git ls-files, ensure git exists with "command -v git >/dev/null 2>&1 ||
{ echo 'git is required for detecting .tf files' >&2; exit 1; }" so the script
exits with a clear error if prerequisites are missing.
Summary
Introduce internal pre-commit hooks for Terraform to standardize code quality checks across repositories:
terraform fmt --check -diffon*.tf.terraform init -backend=falsefollowed byterraform validate.Both hooks ignore
.terraform/andexamples/. Validation gracefully skips directories that require a private registry and are missing credentials (prints a clear warning, does not fail the commit).What’s included
terraform-fmt(check-only; shows diff on failure).terraform-validate(init without backend, validation with per-file output; skip auth-required registries).file://orrepo: local).Why
Summary by CodeRabbit