feat: Add terraform_provider_version_consistency hook #960
Conversation
Add new hook that checks provider version constraints are consistent across all versions.tf files in the repository. This helps catch version drift when updating provider versions in multi-module repositories where each module has its own versions.tf. The hook: - Finds all versions.tf files (excluding .terraform/) - Extracts provider version constraints - Fails if different version constraints are found - Reports which files have inconsistent versions Configurable via hook-config: - --version-file-pattern: customize filename pattern (default: versions.tf) - --exclude-pattern: customize exclusion pattern (default: .terraform/) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Peter Nied <peternied@hotmail.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new pre-commit hook that checks Terraform provider version constraints for consistency across repository Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@hooks/terraform_provider_version_consistency.sh`:
- Around line 52-97: The grep patterns and looping can fail: change grep -E
'^\s*version\s*=' to grep -hE '^[[:space:]]*version\s*=' so indentation is
matched, ensure grep failures don't trigger set -e by appending "|| true" to the
grep pipeline when building all_versions (e.g. all_versions=$(grep -hE
'^[[:space:]]*version\s*=' $version_files 2>/dev/null || true | sed ...)), and
stop word-splitting of version_files by using find -print0 and reading with
while IFS= read -r -d '' file (or use an array) when iterating files (replace
the for file in $version_files loop with a null-delimited read loop) so paths
with spaces are handled correctly; keep the existing sed/sort -u logic and
shellcheck disables as appropriate (referenced symbols: version_files,
all_versions, unique_count, the grep invocations and the for file loop).
| # Find all version files, excluding specified pattern | ||
| local version_files | ||
| version_files=$(find . -name "$version_file_pattern" -type f ! -path "*${exclude_pattern}*" 2>/dev/null | sort) | ||
|
|
||
| if [[ -z "$version_files" ]]; then | ||
| common::colorify "yellow" "No $version_file_pattern files found" | ||
| return 0 | ||
| fi | ||
|
|
||
| local file_count | ||
| file_count=$(echo "$version_files" | wc -l | tr -d ' ') | ||
|
|
||
| if [[ "$file_count" -eq 1 ]]; then | ||
| common::colorify "green" "Only one $version_file_pattern file found, skipping consistency check" | ||
| return 0 | ||
| fi | ||
|
|
||
| # Extract all unique provider version constraint lines | ||
| local all_versions | ||
| # shellcheck disable=SC2086 # Word splitting is intentional | ||
| all_versions=$(grep -hE '^\s*version\s*=' $version_files 2>/dev/null | \ | ||
| sed 's/^[[:space:]]*//' | sort -u) | ||
|
|
||
| # Handle case where no provider version constraints found | ||
| if [[ -z "$all_versions" ]]; then | ||
| common::colorify "yellow" "No provider version constraints found in $version_file_pattern files" | ||
| return 0 | ||
| fi | ||
|
|
||
| local unique_count | ||
| unique_count=$(echo "$all_versions" | wc -l | tr -d ' ') | ||
|
|
||
| if [[ "$unique_count" -eq 1 ]]; then | ||
| common::colorify "green" "All provider versions are consistent across $file_count files" | ||
| return 0 | ||
| fi | ||
|
|
||
| # Versions are inconsistent - report details | ||
| common::colorify "red" "Inconsistent provider versions found across $file_count files:" | ||
| echo "" | ||
|
|
||
| local file | ||
| # shellcheck disable=SC2086 # Word splitting is intentional | ||
| for file in $version_files; do | ||
| echo "--- $file" | ||
| grep -E '^\s*version\s*=' "$file" 2>/dev/null | sed 's/^/ /' |
There was a problem hiding this comment.
Fix grep matching + errexit handling + filename splitting (current logic can miss constraints or exit early).
- Line 72 uses
\swithgrep -E, which won’t match indentedversion =lines. - With
set -e+pipefail, a no‑matchgrepexits the script before the “no constraints” branch. - Word-splitting
$version_filesbreaks paths with spaces.
✅ Suggested fix (Bash 3.2‑compatible)
- local version_files
- version_files=$(find . -name "$version_file_pattern" -type f ! -path "*${exclude_pattern}*" 2>/dev/null | sort)
+ local -a version_files=()
+ while IFS= read -r file; do
+ version_files+=("$file")
+ done < <(find . -name "$version_file_pattern" -type f ! -path "*${exclude_pattern}*" 2>/dev/null | sort)
- if [[ -z "$version_files" ]]; then
+ if ((${`#version_files`[@]} == 0)); then
common::colorify "yellow" "No $version_file_pattern files found"
return 0
fi
local file_count
- file_count=$(echo "$version_files" | wc -l | tr -d ' ')
+ file_count=${`#version_files`[@]}
# Extract all unique provider version constraint lines
local all_versions
- # shellcheck disable=SC2086 # Word splitting is intentional
- all_versions=$(grep -hE '^\s*version\s*=' $version_files 2>/dev/null | \
- sed 's/^[[:space:]]*//' | sort -u)
+ all_versions=$(
+ { grep -hE '^[[:space:]]*version[[:space:]]*=' "${version_files[@]}" 2>/dev/null || true; } \
+ | sed 's/^[[:space:]]*//' | sort -u
+ )
# Handle case where no provider version constraints found
if [[ -z "$all_versions" ]]; then
common::colorify "yellow" "No provider version constraints found in $version_file_pattern files"
return 0
fi
local file
- # shellcheck disable=SC2086 # Word splitting is intentional
- for file in $version_files; do
+ for file in "${version_files[@]}"; do
echo "--- $file"
- grep -E '^\s*version\s*=' "$file" 2>/dev/null | sed 's/^/ /'
+ { grep -E '^[[:space:]]*version[[:space:]]*=' "$file" 2>/dev/null || true; } | sed 's/^/ /'
done📝 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.
| # Find all version files, excluding specified pattern | |
| local version_files | |
| version_files=$(find . -name "$version_file_pattern" -type f ! -path "*${exclude_pattern}*" 2>/dev/null | sort) | |
| if [[ -z "$version_files" ]]; then | |
| common::colorify "yellow" "No $version_file_pattern files found" | |
| return 0 | |
| fi | |
| local file_count | |
| file_count=$(echo "$version_files" | wc -l | tr -d ' ') | |
| if [[ "$file_count" -eq 1 ]]; then | |
| common::colorify "green" "Only one $version_file_pattern file found, skipping consistency check" | |
| return 0 | |
| fi | |
| # Extract all unique provider version constraint lines | |
| local all_versions | |
| # shellcheck disable=SC2086 # Word splitting is intentional | |
| all_versions=$(grep -hE '^\s*version\s*=' $version_files 2>/dev/null | \ | |
| sed 's/^[[:space:]]*//' | sort -u) | |
| # Handle case where no provider version constraints found | |
| if [[ -z "$all_versions" ]]; then | |
| common::colorify "yellow" "No provider version constraints found in $version_file_pattern files" | |
| return 0 | |
| fi | |
| local unique_count | |
| unique_count=$(echo "$all_versions" | wc -l | tr -d ' ') | |
| if [[ "$unique_count" -eq 1 ]]; then | |
| common::colorify "green" "All provider versions are consistent across $file_count files" | |
| return 0 | |
| fi | |
| # Versions are inconsistent - report details | |
| common::colorify "red" "Inconsistent provider versions found across $file_count files:" | |
| echo "" | |
| local file | |
| # shellcheck disable=SC2086 # Word splitting is intentional | |
| for file in $version_files; do | |
| echo "--- $file" | |
| grep -E '^\s*version\s*=' "$file" 2>/dev/null | sed 's/^/ /' | |
| # Find all version files, excluding specified pattern | |
| local -a version_files=() | |
| while IFS= read -r file; do | |
| version_files+=("$file") | |
| done < <(find . -name "$version_file_pattern" -type f ! -path "*${exclude_pattern}*" 2>/dev/null | sort) | |
| if ((${`#version_files`[@]} == 0)); then | |
| common::colorify "yellow" "No $version_file_pattern files found" | |
| return 0 | |
| fi | |
| local file_count | |
| file_count=${`#version_files`[@]} | |
| if [[ "$file_count" -eq 1 ]]; then | |
| common::colorify "green" "Only one $version_file_pattern file found, skipping consistency check" | |
| return 0 | |
| fi | |
| # Extract all unique provider version constraint lines | |
| local all_versions | |
| all_versions=$( | |
| { grep -hE '^[[:space:]]*version[[:space:]]*=' "${version_files[@]}" 2>/dev/null || true; } \ | |
| | sed 's/^[[:space:]]*//' | sort -u | |
| ) | |
| # Handle case where no provider version constraints found | |
| if [[ -z "$all_versions" ]]; then | |
| common::colorify "yellow" "No provider version constraints found in $version_file_pattern files" | |
| return 0 | |
| fi | |
| local unique_count | |
| unique_count=$(echo "$all_versions" | wc -l | tr -d ' ') | |
| if [[ "$unique_count" -eq 1 ]]; then | |
| common::colorify "green" "All provider versions are consistent across $file_count files" | |
| return 0 | |
| fi | |
| # Versions are inconsistent - report details | |
| common::colorify "red" "Inconsistent provider versions found across $file_count files:" | |
| echo "" | |
| local file | |
| for file in "${version_files[@]}"; do | |
| echo "--- $file" | |
| { grep -E '^[[:space:]]*version[[:space:]]*=' "$file" 2>/dev/null || true; } | sed 's/^/ /' | |
| done |
🤖 Prompt for AI Agents
In `@hooks/terraform_provider_version_consistency.sh` around lines 52 - 97, The
grep patterns and looping can fail: change grep -E '^\s*version\s*=' to grep -hE
'^[[:space:]]*version\s*=' so indentation is matched, ensure grep failures don't
trigger set -e by appending "|| true" to the grep pipeline when building
all_versions (e.g. all_versions=$(grep -hE '^[[:space:]]*version\s*='
$version_files 2>/dev/null || true | sed ...)), and stop word-splitting of
version_files by using find -print0 and reading with while IFS= read -r -d ''
file (or use an array) when iterating files (replace the for file in
$version_files loop with a null-delimited read loop) so paths with spaces are
handled correctly; keep the existing sed/sort -u logic and shellcheck disables
as appropriate (referenced symbols: version_files, all_versions, unique_count,
the grep invocations and the for file loop).
There was a problem hiding this comment.
Pull request overview
This PR adds a new pre-commit hook terraform_provider_version_consistency that checks provider version constraints are consistent across all versions.tf files in a repository. This is useful for multi-module Terraform repositories to prevent version drift.
Changes:
- Adds new hook script
terraform_provider_version_consistency.shthat finds and compares version constraints across files - Updates README.md with hook documentation, usage examples, and configuration options
- Registers the new hook in
.pre-commit-hooks.yamlwith appropriate file patterns
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| hooks/terraform_provider_version_consistency.sh | New bash script implementing the consistency check logic with configurable file patterns |
| README.md | Adds documentation for the new hook including usage examples and configuration options |
| .pre-commit-hooks.yaml | Registers the new hook with appropriate metadata and file filtering rules |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # shellcheck disable=SC2086 # Word splitting is intentional | ||
| all_versions=$(grep -hE '^\s*version\s*=' $version_files 2>/dev/null | \ |
There was a problem hiding this comment.
Similar to the issue in the loop below, the unquoted variable expansion in grep can break if file paths contain spaces or special characters. While the shellcheck disable indicates this is intentional, it creates a fragile implementation. A safer approach would be to use an array and proper quoting, or use a while loop with null-delimited input.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| - --hook-config=--version-file-pattern=versions.tf | ||
| - --hook-config=--exclude-pattern=.terraform/ |
There was a problem hiding this comment.
The example configuration shows setting the hook config parameters to their default values (versions.tf and .terraform/), which is redundant and could be confusing for users. This example doesn't demonstrate the utility of the configuration options. Consider either removing this example entirely (since the basic example above already shows the default behavior), or showing a more meaningful example with non-default values to illustrate when customization would be useful.
| - --hook-config=--version-file-pattern=versions.tf | |
| - --hook-config=--exclude-pattern=.terraform/ | |
| # Look for provider versions in providers.tf instead of the default versions.tf | |
| - --hook-config=--version-file-pattern=providers.tf | |
| # Exclude both the .terraform directory and a custom generated/ directory | |
| - --hook-config=--exclude-pattern=.terraform/,generated/ |
There was a problem hiding this comment.
I'll rewrite this part of the readme to be better tailored for readers
| key="${config[0]## }" | ||
| value=${config[1]} |
There was a problem hiding this comment.
The variable key should be declared as local to avoid polluting the global namespace and potential conflicts with variables in other functions. All other local variables in the function are properly scoped.
| key="${config[0]## }" | |
| value=${config[1]} | |
| local key="${config[0]## }" | |
| local value=${config[1]} |
| function main { | ||
| common::initialize "$SCRIPT_DIR" | ||
| common::parse_cmdline "$@" | ||
| common::export_provided_env_vars "${ENV_VARS[@]}" |
There was a problem hiding this comment.
Missing call to common::parse_and_export_env_vars that is present in all other hooks in the codebase. This function is responsible for expanding environment variables in the ARGS array. Without this call, users cannot use environment variable substitution in hook arguments, which is a feature documented in the README and consistently implemented across all other hooks (see terraform_docs.sh:22, terraform_validate.sh:17, terraform_wrapper_module_for_each.sh:14, etc.). Add this call after common::export_provided_env_vars for consistency.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| all_versions=$(grep -hE '^\s*version\s*=' $version_files 2>/dev/null | \ | ||
| sed 's/^[[:space:]]*//' | sort -u) |
There was a problem hiding this comment.
The regex pattern '^\s*version\s*=' matches any line that starts with whitespace followed by "version =", which could match version constraints in different contexts within a versions.tf file (e.g., terraform version requirements, provider version constraints, and potentially module version constraints). This lack of specificity could lead to false positives when comparing version constraints that are actually in different contexts (terraform vs. provider versions). Consider making the pattern more specific to only match provider version constraints within provider blocks, or document this limitation clearly.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| # shellcheck disable=SC2086 # Word splitting is intentional | ||
| for file in $version_files; do |
There was a problem hiding this comment.
While the shellcheck disable comment indicates word splitting is intentional, this approach can break if any file paths contain spaces or special characters. The current implementation relies on newline-separated output from find, but doesn't properly handle edge cases where filenames might contain spaces. Consider using a safer approach with arrays or null-delimited output (find with -print0 and read with -d ''). This is particularly important since the hook operates on user-provided file patterns and exclusion patterns.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| require_serial: true | ||
|
|
||
| - id: terraform_provider_version_consistency | ||
| name: Terraform provider version consistency |
There was a problem hiding this comment.
Is there any reason why you cant use https://github.com/antonbabenko/pre-commit-terraform#tfupdate ?
There was a problem hiding this comment.
@peternied Any update on this please?
The quality level of this AI slop is a bit disputable and requires some work and effort to make it decent and acceptable. Whereas if you threw this at us to take care further, it doesn't look good, you know.
If you think tfupdate hook doesn't meet your needs (and you can reason that) and you feel you'd need a hand getting the code in this PR improved, we're here to help. Thanks.
3 participants
Put an
xinto the box if that apply:Description of your changes
Add a new hook that checks provider version constraints are consistent across all
versions.tffiles in the repository.This helps catch version drift when updating provider versions in multi-module repositories where each module has its own
versions.tffile.What the hook does:
versions.tffiles (excluding.terraform/directories)version = "..."lines)Configuration options via
--hook-config:--version-file-pattern: customize filename pattern (default:versions.tf)--exclude-pattern: customize exclusion pattern (default:.terraform/)Example output (failure case):
How can we test changes
🤖 Generated with Claude Code