Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add terrafmt hook #313

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.0.1
rev: v4.1.0
hooks:
# Git style
- id: check-added-large-files
Expand Down Expand Up @@ -33,3 +33,10 @@ repos:
hooks:
- id: shfmt
args: ['-l', '-i', '2', '-ci', '-sr', '-w']

- repo: https://github.com/rahulmlokurte/pre-commit-terraform
rev: 90bd2a5727d2bbd5d5a27ed5390073f11c359a0c
hooks:
- id: terrafmt
args:
- --args=fmt
rahulmlokurte marked this conversation as resolved.
Show resolved Hide resolved
9 changes: 9 additions & 0 deletions .pre-commit-hooks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,12 @@
files: \.tf$
exclude: \.terraform\/.*$
require_serial: true

- id: terrafmt
name: terrafmt
description: Runs terrafmt on Markdown files.
language: script
entry: terrafmt.sh
files: \.md$
exclude: \.md\/.*$
require_serial: true
13 changes: 11 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ RUN apk add --no-cache \

ARG PRE_COMMIT_VERSION=${PRE_COMMIT_VERSION:-latest}
ARG TERRAFORM_VERSION=${TERRAFORM_VERSION:-latest}

# Install pre-commit
RUN [ ${PRE_COMMIT_VERSION} = "latest" ] && pip3 install --no-cache-dir pre-commit \
|| pip3 install --no-cache-dir pre-commit==${PRE_COMMIT_VERSION}
Expand All @@ -34,6 +33,7 @@ ARG TERRAGRUNT_VERSION=${TERRAGRUNT_VERSION:-false}
ARG TERRASCAN_VERSION=${TERRASCAN_VERSION:-false}
ARG TFLINT_VERSION=${TFLINT_VERSION:-false}
ARG TFSEC_VERSION=${TFSEC_VERSION:-false}
ARG TERRAFMT_VERSION=${TERRAFMT_VERSION:-false}


# Tricky thing to install all tools by set only one arg.
Expand All @@ -47,7 +47,8 @@ RUN if [ "$INSTALL_ALL" != "false" ]; then \
echo "export TERRAGRUNT_VERSION=latest" >> /.env && \
echo "export TERRASCAN_VERSION=latest" >> /.env && \
echo "export TFLINT_VERSION=latest" >> /.env && \
echo "export TFSEC_VERSION=latest" >> /.env \
echo "export TFSEC_VERSION=latest" >> /.env && \
echo "export TERRAFMT_VERSION=latest" >> /.env \
; else \
touch /.env \
; fi
Expand Down Expand Up @@ -126,6 +127,13 @@ RUN . /.env && \
) && chmod +x tfsec \
; fi

RUN . /.env && \
if [ "TERRAFMT_VERSION" != "false" ]; then \
( \
curl -L "https://github.com/katbyte/terrafmt/archive/refs/tags/v0.3.0.zip" > terrafmt.zip \
) && unzip terrafmt.zip && rm terrafmt.zip \
; fi
Comment on lines +130 to +135
Copy link
Collaborator

Choose a reason for hiding this comment

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

This block must follow common for this file convention/standard for this kind of blocks:

  1. Have a comment line right before the block with the name of the tool.
  2. Have a logic to distinguish which version to download.

Moreover the katbyte/terrafmt repo does not distribute pre-compiled binaries, which means the {zip,tar}ball contains source code rather than the executable file. From what I see in katbyte/terrafmt README, the only available option to install the tool at the moment is to build it locally or use Golang builtin mechanism to build and install. Which doesn't seem to be an acceptable approach for pre-commit-terraform at the moment (@antonbabenko @MaxymVlasov ?).


# Checking binaries versions and write it to debug file
RUN . /.env && \
F=tools_versions_info && \
Expand All @@ -138,6 +146,7 @@ RUN . /.env && \
(if [ "$TERRASCAN_VERSION" != "false" ]; then echo "terrascan $(./terrascan version)" >> $F; else echo "terrascan SKIPPED" >> $F ; fi) && \
(if [ "$TFLINT_VERSION" != "false" ]; then ./tflint --version >> $F; else echo "tflint SKIPPED" >> $F ; fi) && \
(if [ "$TFSEC_VERSION" != "false" ]; then echo "tfsec $(./tfsec --version)" >> $F; else echo "tfsec SKIPPED" >> $F ; fi) && \
(if [ "$TERRAFMT_VERSION" != "false" ]; then echo "terrafmt $(terrafmt version)" >> $F; else echo "terrafmt SKIPPED" >> $F ; fi) && \
echo -e "\n\n" && cat $F && echo -e "\n\n"


Expand Down
20 changes: 20 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ If you are using `pre-commit-terraform` already or want to support its developme
* [`TFSec`](https://github.com/liamg/tfsec) required for `terraform_tfsec` hook.
* [`infracost`](https://github.com/infracost/infracost) required for `infracost_breakdown` hook.
* [`jq`](https://github.com/stedolan/jq) required for `infracost_breakdown` hook.
* [`terrafmt`](https://github.com/katbyte/terrafmt) required for `terraform_fmt` hook.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add needed installation instructions both for macOS and Ubuntu in the sections below.


<details><summary><b>Docker</b></summary><br>

Expand Down Expand Up @@ -225,6 +226,7 @@ There are several [pre-commit](https://pre-commit.com/) hooks to keep Terraform
| `terragrunt_fmt` | Reformat all [Terragrunt](https://github.com/gruntwork-io/terragrunt) configuration files (`*.hcl`) to a canonical format. | `terragrunt` |
| `terragrunt_validate` | Validates all [Terragrunt](https://github.com/gruntwork-io/terragrunt) configuration files (`*.hcl`) | `terragrunt` |
| `terrascan` | [terrascan](https://github.com/accurics/terrascan) Detect compliance and security violations. [Hook notes](#terrascan) | `terrascan` |
| `terrafmt` | [terrafmt](https://github.com/katbyte/terrafmt) Format terraform blocks embedded in files. | `terrafmt` |
<!-- markdownlint-enable no-inline-html -->

Check the [source file](https://github.com/antonbabenko/pre-commit-terraform/blob/master/.pre-commit-hooks.yaml) to know arguments used for each hook.
Expand Down Expand Up @@ -568,6 +570,24 @@ Example:
3. Use `--skip-rules="ruleID1,ruleID2"` parameter to skip one or more rules globally while scanning (e.g.: `--args=--skip-rules="ruleID1,ruleID2"`).
4. Use the syntax `#ts:skip=RuleID optional_comment` inside a resource to skip the rule for that resource.

### terrafmt

1. `terrafmt` supports custom arguments so you can pass [supported flags](https://github.com/katbyte/terrafmt). Eg:
rahulmlokurte marked this conversation as resolved.
Show resolved Hide resolved

```tf
resource "aws_lambda_function" "pass" {
function_name = "test-env"
role = ""
runtime = "python3.8"

environment {
variables = {
AWS_DEFAULT_REGION = "us-west-2"
}
}
}
```
rahulmlokurte marked this conversation as resolved.
Show resolved Hide resolved

## Authors

This repository is managed by [Anton Babenko](https://github.com/antonbabenko) with help from these awesome contributors:
Expand Down
59 changes: 59 additions & 0 deletions terrafmt.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
#!/usr/bin/env bash
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might worth to pick up what @MaxymVlasov works on in #310 to modernize and standardize common shell coding conventions and approaches across hook scripts in this project.

set -eo pipefail

main() {
initialize_
parse_cmdline_ "$@"
terrafmt_
}

initialize_() {
# get directory containing this script
local dir
local source
source="${BASH_SOURCE[0]}"
while [[ -L $source ]]; do # resolve $source until the file is no longer a symlink
dir="$(cd -P "$(dirname "$source")" > /dev/null && pwd)"
source="$(readlink "$source")"
# if $source was a relative symlink, we need to resolve it relative to the path where the symlink file was located
[[ $source != /* ]] && source="$dir/$source"
done
_SCRIPT_DIR="$(dirname "$source")"

# source getopt function
# shellcheck source=lib_getopt
. "$_SCRIPT_DIR/lib_getopt"
}

parse_cmdline_() {
declare argv
argv=$(getopt -o a: --long args: -- "$@") || return
eval "set -- $argv"

for argv; do
case $argv in
-a | --args)
shift
ARGS+=("$1")
shift
;;
--)
shift
break
;;
esac
done
}

terrafmt_() {
find . | grep -E "README.md" | sort | while read -r f; do
Copy link
Collaborator

@yermulnik yermulnik Jan 5, 2022

Choose a reason for hiding this comment

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

  1. find has an option to lookup files with particular names (or by a pattern), hence grep is extraneous and redundant here (as a side note, -E means grep should interpret pattern as an extended regex, which is redundant in this code construction).
  2. Hook config is configured in this PR to pass all markdown files to the hook (files: \.md$), so why would you need to pick only README.md files then?

echo "Processing $f"
echo "terrafmt ${ARGS[*]} $f"
rahulmlokurte marked this conversation as resolved.
Show resolved Hide resolved
terrafmt "${ARGS[@]}" $f
rahulmlokurte marked this conversation as resolved.
Show resolved Hide resolved
done
}

# global arrays
declare -a ARGS=()

[[ ${BASH_SOURCE[0]} != "$0" ]] || main "$@"