[tfenv] terraform_fmt fails with tfenv and "min-required" version

[jcarlson]

Describe the bug

tfenv allows for the use of min-required as the version specified in a project's .terraform-version file, which then parses the Terraform required_version from the configuration files.

The terraform_fmt hook runs terraform fmt using the directory of the changed file as the working directory, rather than the repository root as the working directory.

In a configuration that includes sub-modules as sub-directories, where that sub-module requires a different, less-restrictive version of Terraform, the terraform_fmt hook then attempts to install the wrong version of Terraform by installing the minimum version for that sub-module, rather than the version for that project.

For example, I have a root configuration that uses Terraform 1.4.4, and a sub-module which requires Terraform >= 0.14, which 1.4.4 satisfies. However, when files in the sub-module change, terraform_fmt failed because tfenv is attempting to install Terraform 0.14, and on my M1 Apple Silicon Mac, this fails, since there is no ARM build of Terraform 0.14.

How can we reproduce it?

Observe the following sample configuration:

main.tf

terraform {
  required_version = "1.4.4"
}

module "sub_module" {
  source = "./modules/sub"
}

output "lorem_ipsum" {
  value = join([module.sub_module.foo, module.sub_module.bar], " ")
}

modules/sub/main.tf

terraform {
  required_version = ">= 0.14"
}

output "foo" {
  value = "Foo is fun."
}

output "bar" {
  value = "Bar is bad."
}

The repository root also includes the following files:

.terraform-version

min-required

.pre-commit-config.yaml

---
repos:
  - repo: https://github.com/antonbabenko/pre-commit-terraform
    rev: v1.77.2
    hooks:
      - id: terraform_fmt

With the above configuration in a clean git working tree:

cat <<-EOF | tee -a modules/sub/main.tf

output "baz" {
  value = "Baz is not used"
}
EOF

git add modules/sub/main.tf
git commit

The following error is produced:

[INFO] Initializing environment for https://github.com/antonbabenko/pre-commit-terraform.
Terraform fmt............................................................Failed
- hook id: terraform_fmt
- exit code: 127

version '0.14.0' is not installed (set by /Users/jcarlson/example/.terraform-version). Installing now as TFENV_AUTO_INSTALL==true
Installing Terraform v0.14.0
Downloading release tarball from https://releases.hashicorp.com/terraform/0.14.0/terraform_0.14.0_darwin_arm64.zip
curl: (22) The requested URL returned error: 404

Tarball download failed
/opt/homebrew/Cellar/tfenv/3.0.0/lib/tfenv-exec.sh: line 43: /opt/homebrew/Cellar/tfenv/3.0.0/versions/0.14.0/terraform: No such file or directory

Environment information

• OS: MacOS 13.3.1
• uname -a output:

$ uname -a
Darwin Thor 22.4.0 Darwin Kernel Version 22.4.0: Mon Mar  6 20:59:28 PST 2023; root:xnu-8796.101.5~3/RELEASE_ARM64_T6000 arm64

• Tools availability and versions:

GNU bash, version 5.2.15(1)-release (aarch64-apple-darwin22.1.0)
pre-commit 3.2.2
Terraform v1.4.4
python SKIPPED
Python 3.11.3
checkov 2.3.165
terraform-docs version v0.16.0 darwin/arm64
terragrunt version v0.42.8
terrascan terrascan SKIPPED
TFLint version 0.44.1
+ ruleset.terraform (0.2.2-bundled)
tfsec tfsec SKIPPED
tfenv 3.0.0
tfupdate tfupdate SKIPPED
hcledit hcledit SKIPPED

• .pre-commit-config.yaml

---
repos:
  - repo: https://github.com/antonbabenko/pre-commit-terraform
    rev: v1.77.2
    hooks:
      - id: terraform_fmt

Proposed Solution

The terraform_fmt hook appears to receive an array of files which have changed and should be formatted. Instead of dropping into each containing directory to run terraform fmt, run terraform fmt from the repository root and pass the relative path of each file as an argument to terraform fmt.

For example, do this:

terraform fmt main.tf modules/sub/main.tf

And not:

terraform fmt
cd modules/sub
terraform fmt

This will allow tfenv to detect the Terraform required_version specified at the root instead of in the modules/sub module.

[MaxymVlasov]

That's a https://github.com/tfutils/tfenv bug. Please open the issue to them.

tfenv should check the version recursively starting from dir when it found .terraform-version.
But not higher than closest .git dir exist, I suppose

[jcarlson]

Respectfully, I do not believe this to be a bug in tfenv.

When this plugin runs, it changes the current working directory successively to each sub-directory of the repository with changed files before running terraform fmt. From the context of a sub-directory, tfenv has no way of knowing what the correct version of Terraform to use should be, because the Terraform version is specified in the configuration root.

Even if tfenv were able to successfully install 0.14.0 (and I can resolve that issue), the next issue is that this plugin is then formatting each sub-directory with a different version of Terraform than the root. In my example above, the root gets formatted with Terraform 1.4.4, while the modules/sub directory would get formatted with Terraform 0.14.0.

At the very least, this plugin should use a consistent version of Terraform throughout.

In my opinion, this plugin should not change working directory into each sub-directory containing changed files, but instead pass as arguments to terraform fmt the name of each file that has changed and run using the repository's root as the working directory.

[MaxymVlasov]

Even if tfenv were able to successfully install 0.14.0 (and I can resolve that issue), the next issue is that this plugin is then formatting each sub-directory with a different version of Terraform than the root. In my example above, the root gets formatted with Terraform 1.4.4, while the modules/sub directory would get formatted with Terraform 0.14.0.

As I said above, tfenv should try find .terraform-version by next algorithm:

1. Current dir
2. If git repo - Check every parent dir to git repo root until it finds .terraform-version
   2.1. If .terraform-version is founded - check for TF version recursively from dir where .terraform-version founded
   2.2 If after checking parent dirs in git repo .terraform-version is still not found - go to step 3.
3. If you have global .terraform-version - tfenv should use that config and recursively apply it from git repo root
4. If no configs at all - show error

[jcarlson]

Yes, tfenv does all those things as described. In my example configuration above, tfenv finds the .terraform-version at the repo root, but the .terraform-version is specified as min-required.

When tfenv is instructed to resolve min-required, it looks at the required_version inside the terraform block of the current working directory, which tfenv would reasonably assume to be the Terraform configuration root.

In my example configuration, the terraform block in the project root specifies 1.4.4, and this is the version we want to use. This works correctly when you run terraform fmt from the configuration root.

The problem occurs when you change directories into the included sub-module. This is no longer the configuration root, and this included module does not specify an exact version of Terraform, but rather, a minimum viable version of Terraform that the current module is compatible with.

So from the repo root, tfenv sees min-required, which means 1.4.4.

But if you cd modules/sub and then run terraform version, tfenv now sees min-required and from the modules/sub directory, that is >= 0.14, which is why tfenv is trying to use 0.14.

So from that point of view, this issue doesn’t really have anything to do with terraform_fmt at all, since we can reproduce this issue without using pre-commit at all.

What I am suggesting here is that perhaps the terraform_fmt script should keep the working directory as the repository root, or at least expose a configuration option to make it so, so that tfenv version resolution works based off the Terraform root configuration, and not an arbitrary sub-directory in the repository.

[MaxymVlasov]

When tfenv is instructed to resolve min-required, it looks at the required_version inside the terraform block of the current working directory, which tfenv would reasonably assume to be the Terraform configuration root.

Step 2.1.

[jcarlson]

What are you suggesting needs to be changed about my sample configuration above in order to work properly?

[MaxymVlasov]

What I am suggesting here is that perhaps the terraform_fmt script should keep the working directory as the repository root, or at least expose a configuration option to make it so, so that tfenv version resolution works based off the Terraform root configuration, and not an arbitrary sub-directory in the repository.

Okay, let's imagine that tfenv works correctly (which is not), and there is some problem here in pre-commit-terraform hooks logic.

Firstly, did you see the hooks code? You propose or change how works mostly all hooks here:

pre-commit-terraform/hooks/_common.sh

Lines 229 to 238 in e1d78ae

	pushd "$dir_path" > /dev/null || continue
	per_dir_hook_unique_part "$dir_path" "${args[@]}"
	local exit_code=$?
	if [ $exit_code -ne 0 ]; then
	final_exit_code=$exit_code
	fi
	popd > /dev/null

Or make terraform_fmt maintained completely separate with different logic.

Also, tfenv could affect mostly all terraform hooks, which somehow use terraform, IE, these two, which return us to option 1 (which will not work correctly out of the box, because some hooks required to be inside dir)

What are you suggesting needs to be changed about my sample configuration above in order to work properly?

Open bug report to tflint and pin version in another way:

main.tf

terraform {
  required_version = "~> 1.0"
}

.terraform-version

1.4.4

[MaxymVlasov]

Hmm, that could not work too, please check. No idea, don't use tfenv, I prefer asdf, which has no problems at all