terraform fmt being run in .terraform dir

[sysadmiral]

Not sure why yet but this hook runs in the .terraform dir for me but from the code it looks like that dir should be ignored?

I will investigate further and post an update.

[antonbabenko]

Make sure that you are using correct version of the hook. Also, you can try to clean ~/.cache/pre-commit and run pre-commit install to see if it resolves the issue.

[sysadmiral]

I am using the latest version of the hook (1.4.0).

But I do still get this behaviour. If I have an existing tf repo with a .terraform dir and I run pre-commit run -a it will run terraform fmt on files in the .terraform dir too (downloaded modules) - I'm trying to determine if it is only happening on internal modules or any modules.

I'll check if there is a verbose/debug mode for pre-commit to see if it's apparent why this happens for me.

[antonbabenko]

Do you have .terraform in your .gitignore? If not, you should.

In fact it should not be related to the issue you specify, because those files should be ignored by pre-commit hook.

[sysadmiral]

Yeah I always include .terraform/ in my repo gitignores.

I wonder if this is perhaps a pre-commit issue rather than an issue with this hook where (for me) it is not ignoring the files it is meant to...?

Still testing (in between real job! 😁)

[samgd]

pre-commit passes the correct set of files to terraform_fmt.sh.

However, terraform_fmt.sh calls dirname on each file to create a set of paths:

paths[index]=$(dirname "$file_with_path")

These directories are then passed to terraform fmt.

If there exists a .terraform directory within any of those directories then they too will be processed by terraform fmt.

Why is terraform fmt ran on the directory containing each file rather than just the file itself?

[samgd]

This can be replicated by creating a directory with the following structure:

.
├── .pre-commit-config.yaml
├── .terraform
│   └── invalid.tf
└── valid.tf

where invalid.tf is a syntactically incorrect Terraform file such as:

invalid

The standard pre-commit requirements will need to be performed (install pre-commit, terraform, this hook, git stuff, etc).

Running pre-commit run produces:

$ pre-commit run
Terraform fmt............................................................Failed
hookid: terraform_fmt

Error running fmt: In .terraform/invalid.tf: key 'invalid' expected start of object ('{') or assignment ('=')

[moxli]

I can confirm the behaviour reported by @samgd .

[sysadmiral]

terraform fmt does indeed include the capability to run against a single file (even though it isn't documented) so continuing from what @samgd has added I think the hook could be improved by:

• only running against files and not by directory - we only want to act on files included in the commit and should ignore the rest of the directory
• ignoring the .terraform dir (although doing the above should omit .terraform as long as it's in the committers .gitignore i.e. .terraform will never be included in a commit)

If that sounds acceptable I would be happy to start a PR?

edit: I've just realised that unless pre-commit sends a list of changed files we can't act on that subset of files. I haven't looked into the internals of pre-commit yet so I'm not sure if it's possible to act only on the files changed in a commit. It could be a case of all or nothing... 🤔

[antonbabenko]

@sysadmiral @samgd thanks for keeping this issue alive :)

pre-commit sends a list of changed files to process to the hook, and then the hook (terraform_fmt.sh) creates a list of unique directories where changed files are and runs terraform fmt in each of these directories. It is much faster than running terraform fmt on files one by one.

I wonder whether pre-commit tracks changes in .terraform directory even when it should be ignored by .gitignore?

Does exclude work as expected in config - https://github.com/antonbabenko/pre-commit-terraform/blob/master/.pre-commit-hooks.yaml#L7 ? Maybe regexp is bad.

If exclude from above is ok, terraform_fmt.sh should force exclude the content of .terraform (and .terragrunt-cache for the same reason).

[pixie79]

Are there any updates on this issue? I see the same issue with tfsec not ignoring the .terraform directory.

[MaxymVlasov]

Not reproduced for terraform_fmt.

22:17 test-8 git:(main +) 
➜ ls -lah
total 140K
drwxr-xr-x  4 vm   vm   4.0K Sep  9 22:15 .
drwxrwxrwt 62 root root 120K Sep  9 22:15 ..
drwxr-xr-x  5 vm   vm   4.0K Sep  9 22:17 .git
-rw-r--r--  1 vm   vm    112 Sep  9 22:14 .pre-commit-config.yaml
drwxr-xr-x  2 vm   vm   4.0K Sep  9 22:14 .terraform
-rw-r--r--  1 vm   vm      0 Sep  9 22:14 valid.tf

22:17 test-8 git:(main +) 
➜ ls -lah .terraform                        

total 12K
drwxr-xr-x 2 vm vm 4.0K Sep  9 22:14 .
drwxr-xr-x 4 vm vm 4.0K Sep  9 22:15 ..
-rw-r--r-- 1 vm vm    9 Sep  9 22:14 invalid.tf

22:17 test-8 git:(main +) 
➜ find . -maxdepth 1 -type f -exec cat {} \;                                                 
repos:
- repo: git://github.com/antonbabenko/pre-commit-terraform
  rev: v1.50.0
  hooks:
  - id: terraform_fmt

22:18 test-8 git:(main +) 
➜ find ./.terraform -maxdepth 1 -type f -exec cat {} \; 
invalid


22:18 test-8 git:(main +) 
➜ git status        

On branch main

No commits yet

Changes to be committed:
  (use "git rm --cached <file>..." to unstage)
        new file:   .pre-commit-config.yaml
        new file:   .terraform/invalid.tf
        new file:   valid.tf


22:19 test-8 git:(main +) 
➜ pre-commit run -a 

Terraform fmt............................................................Passed

22:19 test-8 git:(main +) 
➜ pre-commit --version
pre-commit 2.14.0

22:19 test-8 git:(main +) 
➜ terraform --version | head -n 1                                 
Terraform v1.0.6

22:19 test-8 git:(main +) 
➜ 

[MaxymVlasov]

And not reproduced for terrafrom_tfsec too.

22:25 test-8 git:(main +) 
➜ pre-commit run -a 

Terraform validate with tfsec............................................Passed

Feel free to reopen the issue with details about your environment and reproduction steps if the issue is still present.