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.

Sign up for GitHub

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

Add terraform and terragrunt testing #581

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add terraform and terragrunt testing #581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
da09736
Add terraform and terragrunt testing
peter-halliday Oct 29, 2023
d119e33
Add terraform and terragrunt testing
peter-halliday Oct 29, 2023
42d470d
update after PR comments
peter-halliday Nov 3, 2023
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
8 changes: 8 additions & 0 deletions .pre-commit-hooks.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,14 @@
files: (\.hcl)$
exclude: \.terraform\/.*$

- id: terraform_test
name: Terraform test
description: Runs all terraform tests
entry: hooks/terraform_test.sh
language: script
files: (\.tf|\.tfvars|\.terraform\.lock\.hcl||\.terraform\.lock\.hcl||\.terraform\.lock\.json)$
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
files: (\.tf|\.tfvars|\.terraform\.lock\.hcl||\.terraform\.lock\.hcl||\.terraform\.lock\.json)$
files: \.(tf(vars|test\.(hcl|json))?|terraform\.lock\.hcl)$

Should

exclude: \.terraform\/.*$

- id: terragrunt_validate
name: Terragrunt validate
description: Validates all Terragrunt configuration files.
Expand Down
22 changes: 21 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ If you are using `pre-commit-terraform` already or want to support its developme
* [terraform\_docs\_replace (deprecated)](#terraform_docs_replace-deprecated)
* [terraform\_fmt](#terraform_fmt)
* [terraform\_providers\_lock](#terraform_providers_lock)
* [terraform\_test](#terraform_test)
* [terraform\_tflint](#terraform_tflint)
* [terraform\_tfsec](#terraform_tfsec)
* [terraform\_validate](#terraform_validate)
Expand Down Expand Up @@ -271,9 +272,10 @@ There are several [pre-commit](https://pre-commit.com/) hooks to keep Terraform
| `terraform_docs_without_`<br>`aggregate_type_defaults` | Inserts input and output documentation into `README.md` without aggregate type defaults. Hook notes same as for [terraform_docs](#terraform_docs) | `terraform-docs` |
| `terraform_fmt` | Reformat all Terraform configuration files to a canonical format. [Hook notes](#terraform_fmt) | - |
| `terraform_providers_lock` | Updates provider signatures in [dependency lock files](https://www.terraform.io/docs/cli/commands/providers/lock.html). [Hook notes](#terraform_providers_lock) | - |
| `terraform_test` | Runs any Terraform Tests, which requires Terraform 1.6 and above. [Hook notes](#terraform_test) |
| `terraform_tflint` | Validates all Terraform configuration files with [TFLint](https://github.com/terraform-linters/tflint). [Available TFLint rules](https://github.com/terraform-linters/tflint/tree/master/docs/rules#rules). [Hook notes](#terraform_tflint). | `tflint` |
| `terraform_tfsec` | [TFSec](https://github.com/aquasecurity/tfsec) static analysis of terraform templates to spot potential security issues. [Hook notes](#terraform_tfsec) | `tfsec` |
| `terraform_validate` | Validates all Terraform configuration files. [Hook notes](#terraform_validate) | `jq`, only for `--retry-once-with-cleanup` flag |
| `terraform_validate` sd | Validates all Terraform configuration files. [Hook notes](#terraform_validate) | `jq`, only for `--retry-once-with-cleanup` flag |
Copy link
Collaborator

Choose a reason for hiding this comment

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

sd?

| `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` |
| `terraform_wrapper_module_for_each` | Generates Terraform wrappers with `for_each` in module. [Hook notes](#terraform_wrapper_module_for_each) | `hcledit` |
Expand Down Expand Up @@ -645,6 +647,24 @@ To replicate functionality in `terraform_docs` hook:
- --tf-init-args=-upgrade
```

### terraform_test

1. `terraform_test` supports custom arguments so you can set the test directory, set a variable file, change output to JSON and change the verbosity, filtering which tests runs, and set individual variables.

Example:

```yaml
- id: terraform_test
args:
- --args=-test-directory=path
- --arg=-filter=testfile
- --args=-json
- --arg=-verbose
- --arg=-var-file=filename
- --arg=-var=variable
```

2. `terraform_test` only runs per repository.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I beg your pardon once again, though is it not possible to run terraform test per directory? 🤔

Copy link
Author

Choose a reason for hiding this comment

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

I beg your pardon once again, though is it not possible to run terraform test per directory? 🤔

not sure what that would mean. Is it possible that people litter tests in directories throughout? Yes. I can put the per directory back if you want to support that. Although they’ll have to use the same convention for test-directories for it to work. That makes sense although I can imagine why someone would do that but I’m not a mono repo fan so that’s one use case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

What I mean is that we should deliberately and explicitly support both monorepo and multirepo consumers of pre-commit-terraform instead of making assumptions on how others should better Terraform their infrastructure.

Copy link
Author

Choose a reason for hiding this comment

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

What I mean is that we should deliberately and explicitly support both monorepo and multirepo consumers of pre-commit-terraform instead of making assumptions on how others should better Terraform their infrastructure.

I understand why you’d want to do that as your project can enforce opinions. I’m trying to balance the little time to contribute and wanted to contribute and not fork. But in a
Big organization money repo is very unwieldy so we disallow it. I’ll try to find some time to build in support that make sense per dir.

Copy link
Collaborator

@MaxymVlasov MaxymVlasov Nov 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

But in a Big organization money repo is very unwieldy so we disallow it.

Well, I have ~320 unique components (modules not included), which are used by about ~2000 stacks. In one monorepo. We switched to monorepo, because it is much better manageable than hundreds of mini-repos.

I previously had "luck" to work with about ~50 repos which were set up on a per-account basis, and that was hell on Earth.

Probably, you'll start to see some issues when achieving thousands of unique components, mostly from a git perspective, which will need some split-up or additional tuning as it is done in big companies like Google or Microsoft, but it is still better than trying to maintain thousands of repos at once.

In any case, the spirit of pre-commit hooks by definition - runs only on changed files a.k.a minimum subset to check that all is OK.
Otherwise, you'll just waste time and resources.
Also, function run_hook_on_whole_repo { is just an optimization option basically for pre-commit run -a, because usually, you don't change every file when you run git commit (and invoking pre-commit run)

pre-commit-terraform/hooks/_common.sh

Lines 201 to 207 in 95fc56f

# check is (optional) function defined
if [ "$(type -t run_hook_on_whole_repo)" == function ] &&
# check is hook run via `pre-commit run --all`
common::is_hook_run_on_whole_repo "$hook_id" "${files[@]}"; then
run_hook_on_whole_repo "${args[@]}"
exit 0
fi

At the same time, not providing the per_dir_hook_unique_part function means a broken hook.


### terraform_tflint

Expand Down
42 changes: 42 additions & 0 deletions hooks/terraform_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#!/usr/bin/env bash
set -eo pipefail

# globals variables
# shellcheck disable=SC2155 # No way to assign to readonly variable in separate lines
readonly SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -P)"
# shellcheck source=_common.sh
. "$SCRIPT_DIR/_common.sh"

function main {
common::initialize "$SCRIPT_DIR"
common::parse_cmdline "$@"
common::export_provided_env_vars "${ENV_VARS[@]}"
common::parse_and_export_env_vars

# Suppress terraform test color
if [ "$PRE_COMMIT_COLOR" = "never" ]; then
ARGS+=("-no-color")
fi

# shellcheck disable=SC2153 # False positive
common::per_dir_hook "$HOOK_ID" "${#ARGS[@]}" "${ARGS[@]}" "${FILES[@]}"
}

#######################################################################
# Unique part of `common::per_dir_hook`. The function is executed one time
# in the root git repo
# Arguments:
# args (array) arguments that configure wrapped tool behavior
#######################################################################
function run_hook_on_whole_repo {
local -a -r args=("$@")

# pass the arguments to hook
terraform test "${args[@]}"

# return exit code to common::per_dir_hook
local exit_code=$?
return $exit_code
}

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