feat: Add terraform_graph hook
#589
Conversation
MaxymVlasov
added this to the Features implementing and cleanup without breaking changes milestone
There was a problem hiding this comment.
Please add hook and it deps to Dockerfile
And double-check https://github.com/antonbabenko/pre-commit-terraform/blob/master/.github/CONTRIBUTING.md#add-new-hook - you could miss something else
| local -a -r args=("$@") | ||
|
|
||
| if [[ ! $(command -v dot) ]]; then | ||
| echo "ERROR: dot is required by terraform_graph pre-commit hook but is not installed or in the system's PATH." |
There was a problem hiding this comment.
Let's add some colors
| echo "ERROR: dot is required by terraform_graph pre-commit hook but is not installed or in the system's PATH." | |
| common::colorify "red" "ERROR: dot is required by terraform_graph pre-commit hook but is not installed or in the system's PATH." |
| @@ -926,6 +930,16 @@ If the generated name is incorrect, set them by providing the `module-repo-short | |||
| Check [`tfupdate` usage instructions](https://github.com/minamijoyo/tfupdate#usage) for other available options and usage examples. | |||
| No need to pass `--recursive .` as it is added automatically. | |||
|
|
|||
| ### terraform_graph | |||
| `terraform_graph` utilizes Terraform's built-in [`graph` command](https://developer.hashicorp.com/terraform/cli/commands/graph) and `dot` from [GraphViz](https://www.graphviz.org/) to generate charts of your configuration. | |||
|
|
|||
There was a problem hiding this comment.
Would be nice to have an example image of resulting graph here
You can populate it inside assets/ dir
|
@MaxymVlasov, awesome, I'll work on this. Thanks for your help. |
|
@jrdnbradford howdy. Do you plan to continue working on this PR? Do you need any help from our side? |
|
I’m really crossing my fingers that @jrdnbradford pulls off this new feature! . I'm looking forward to using it. |
I was originally out of my depth here and now I don't have the extra time to get back up to speed and finish this up. Sorry about that. If someone else wants to pick up this work that would be great. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughA new pre-commit hook called Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PreCommit
participant terraform_graph.sh
participant Terraform
participant GraphvizDot
User->>PreCommit: Runs pre-commit with terraform_graph hook
PreCommit->>terraform_graph.sh: Executes hook script
terraform_graph.sh->>terraform_graph.sh: Parse args, check for dot
terraform_graph.sh->>Terraform: Run terraform graph
Terraform-->>terraform_graph.sh: Outputs dependency graph (dot format)
terraform_graph.sh->>GraphvizDot: Pipe dot format to dot (SVG output)
GraphvizDot-->>terraform_graph.sh: Receives SVG
terraform_graph.sh->>terraform_graph.sh: Compare with previous SVG
terraform_graph.sh-->>PreCommit: Exit with status (based on diff)
Suggested reviewers
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
README.md (1)
1194-1196: Include a sample output imageIt would be helpful to add a sample SVG (e.g., under
assets/) to visually illustrate theterraform_graphoutput, as previously suggested.
🧹 Nitpick comments (7)
.pre-commit-hooks.yaml (1)
180-186: Anchor patterns, quote regex, and disable filename passingTo ensure precise matching, quote and anchor the
filesandexcluderegex, and consider settingpass_filenames: falsesince the hook generates a single graph per directory:- id: terraform_graph name: Terraform graph entry: hooks/terraform_graph.sh language: script files: '^.*\.tf$' exclude: '^\.terraform/.*$' pass_filenames: falseREADME.md (5)
106-106: Maintain alphabetical order in dependencies listThe new
graphvizdependency is inserted at the end of the list. For readability, consider placing it alphabetically (e.g., aftergitand beforehcledit).
180-181: Improve readability of MacOS install commandThe
brew installline is very long. Consider splitting out thegraphvizinstallation or grouping dependencies on separate lines:brew install graphviz
204-204: Consolidate Ubuntu 18.04 Graphviz installationThe
sudo apt install -y graphvizcommand is separated from the main install block. To maintain consistency, merge it with the earlierapt installinvocation.
227-227: Consolidate Ubuntu 20.04+ Graphviz installationSimilarly, include
graphvizalongside other packages in theapt installblock for Ubuntu 20.04+.
1197-1202: Refine the usage snippet formattingFor consistency with other hook examples, indent the
argsitems and situate this under an explicithooks:section. For example:- id: terraform_graph args: - --args=-type=apply # Default is `plan` - --hook-config=--path-to-file=test-config.svg # Default is tf-graph.svghooks/terraform_graph.sh (1)
2-2: Consider enabling strict mode (set -euo pipefail)Add
set -uto treat unset variables as errors for improved robustness:-set -eo pipefail +set -euo pipefail
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.pre-commit-hooks.yaml(1 hunks)README.md(7 hunks)hooks/terraform_graph.sh(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
hooks/terraform_graph.sh (1)
hooks/_common.sh (5)
common::initialize(31-36)common::parse_cmdline(52-115)common::export_provided_env_vars(590-608)common::parse_and_export_env_vars(123-152)common::per_dir_hook(285-429)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: 🧪 Tests / pytest@🐍3.9@windows-2025
- GitHub Check: 🧪 Tests / pytest@🐍3.9@macos-14
- GitHub Check: 🧪 Tests / pytest@🐍3.9@macos-13
- GitHub Check: 🧪 Tests / pytest@🐍3.13@windows-2025
- GitHub Check: 🧪 Tests / pytest@🐍3.10@windows-2025
🔇 Additional comments (1)
README.md (1)
329-329: Table entry forterraform_graphlooks goodThe hook name, description, and dependency (
dotfrom GraphViz) are clearly documented. Nice addition!
| if [[ ! $(command -v dot) ]]; then | ||
| echo "ERROR: dot is required by terraform_graph pre-commit hook but is not installed or in the system's PATH." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Use a robust check for GraphViz's dot executable
The current test [[ ! $(command -v dot) ]] can misfire. Prefer:
if ! command -v dot >/dev/null 2>&1; then
common::colorify "red" "ERROR: \`dot\` is required but not found in PATH."
exit 1
fiThis reliably detects dot and applies consistent error styling.
| temp_file=$(mktemp) | ||
| # pass the arguments to hook | ||
| echo "${args[@]}" >> per_dir_hook_unique_part |
There was a problem hiding this comment.
Remove unintended debug output
Left-over debug lines append args to files (>> per_dir_hook_unique_part), polluting the working directory. These should be removed.
| temp_file=$(mktemp) | ||
| # pass the arguments to hook | ||
| echo "${args[@]}" >> per_dir_hook_unique_part | ||
| terraform graph "${args[@]}" | dot -Tsvg > "$temp_file" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Respect the detected Terraform binary path
Rather than hardcoding terraform, invoke the binary discovered by common::get_tf_binary_path. For example:
- terraform graph "${args[@]}" | dot -Tsvg > "$temp_file"
+ "$tf_path" graph "${args[@]}" | dot -Tsvg > "$temp_file"This ensures compatibility with custom Terraform/OpenTofu installations.
📝 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.
| terraform graph "${args[@]}" | dot -Tsvg > "$temp_file" | |
| - terraform graph "${args[@]}" | dot -Tsvg > "$temp_file" | |
| + "$tf_path" graph "${args[@]}" | dot -Tsvg > "$temp_file" |
| echo "${args[@]}" >> run_hook_on_whole_repo | ||
| terraform graph "$(pwd)" "${args[@]}" | dot -Tsvg > "$text_file" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clean up debug echo and use custom Terraform path
In run_hook_on_whole_repo, remove the debug echo and switch to the detected binary:
- echo "${args[@]}" >> run_hook_on_whole_repo
- terraform graph "$(pwd)" "${args[@]}" | dot -Tsvg > "$text_file"
+ "$tf_path" graph "$(pwd)" "${args[@]}" | dot -Tsvg > "$text_file"This prevents stray files and honors custom binaries.
| cmp -s "$temp_file" "$text_file" | ||
|
|
||
| # return exit code to common::per_dir_hook | ||
| local exit_code=$? | ||
| mv "$temp_file" "$text_file" | ||
| return $exit_code |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Only update SVG on content changes
The current logic always moves the temp file, updating the SVG even when identical. This can trigger false diffs. Instead:
- cmp -s "$temp_file" "$text_file"
- local exit_code=$?
- mv "$temp_file" "$text_file"
- return $exit_code
+ if cmp -s "$temp_file" "$text_file"; then
+ rm "$temp_file"
+ return 0
+ else
+ mv "$temp_file" "$text_file"
+ return 1
+ fiThis preserves modification timestamps when no content change has occurred.
📝 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.
| cmp -s "$temp_file" "$text_file" | |
| # return exit code to common::per_dir_hook | |
| local exit_code=$? | |
| mv "$temp_file" "$text_file" | |
| return $exit_code | |
| # return exit code to common::per_dir_hook | |
| if cmp -s "$temp_file" "$text_file"; then | |
| rm "$temp_file" | |
| return 0 | |
| else | |
| mv "$temp_file" "$text_file" | |
| return 1 | |
| fi |
Put an
xinto the box if that apply:Description of your changes
This adds a
terraform_graphhook that generates charts usingterraform graphand GraphViz. My org finds something like this helpful to upkeep documentation.Test results
200 runs '
terraform_graphv0.0.1:'Run details
TEST_NUM200TEST_COMMANDpre-commit try-repo -a /home/$user/Desktop/pre-commit-terraform terraform_graphTEST_DIR/home/$user/$repoTEST_DESCRIPTION200 runs 'terraform_graphv0.0.1:'RAW_TEST_RESULTS_FILE_NAMEterraform_graph_resultsMemory info (
head -n 6 /proc/meminfo):CPU info:
How can we test changes
Testing can be performed by running against a repo with
*.tffiles and ensuring that the correct*.svgfiles are output in each directory:The most important configs for the hook are
-typeand a hook config--path-to-filethat sets the name of the output*.svg.I cannot say that I'm a particularly good shell scripter, so if there's interest in adding this please feel free to edit.