fix: Support custom TF paths which contains spaces

[robinbowes]

Put an x into the box if that apply:

• This PR introduces breaking change.
• This PR fixes a bug.
• This PR adds new functionality.
• This PR enhances existing functionality.

Description of your changes

Add quotes around variable containing path to terraform so it works when the path contains a space.

Fixes #713

How can we test changes

[yermulnik]

@robinbowes Thanks for the contribution. It looks like we'd better quote all instances of $tf_path. Would you be up to update your PR accordingly? See #713 (comment) just in case to avoid race condition =)

> sift -n '[^"]\$tf_path'
hooks/terraform_validate.sh:121:  $tf_path validate "${args[@]}" &> /dev/null && {
hooks/terraform_validate.sh:135:    validate_output=$($tf_path validate "${args[@]}" 2>&1)
hooks/terraform_validate.sh:139:    validate_output=$($tf_path validate -json "${args[@]}" 2>&1)
hooks/terraform_validate.sh:162:      validate_output=$($tf_path validate "${args[@]}" 2>&1)
hooks/terraform_providers_lock.sh:158:  $tf_path providers lock "${args[@]}"
hooks/_common.sh:539:      init_output=$($tf_path init -backend=false "${TF_INIT_ARGS[@]}" 2>&1)
hooks/_common.sh:545:        init_output=$($tf_path init -backend=false "${TF_INIT_ARGS[@]}" 2>&1)
hooks/terraform_fmt.sh:52:  $tf_path fmt "${args[@]}"

[robinbowes]

@robinbowes Thanks for the contribution. It looks like we'd better quote all instances of $tf_path. Would you be up to update your PR accordingly? See #713 (comment) just in case to avoid race condition =)

> sift -n '[^"]\$tf_path'
hooks/terraform_validate.sh:121:  $tf_path validate "${args[@]}" &> /dev/null && {
hooks/terraform_validate.sh:135:    validate_output=$($tf_path validate "${args[@]}" 2>&1)
hooks/terraform_validate.sh:139:    validate_output=$($tf_path validate -json "${args[@]}" 2>&1)
hooks/terraform_validate.sh:162:      validate_output=$($tf_path validate "${args[@]}" 2>&1)
hooks/terraform_providers_lock.sh:158:  $tf_path providers lock "${args[@]}"
hooks/_common.sh:539:      init_output=$($tf_path init -backend=false "${TF_INIT_ARGS[@]}" 2>&1)
hooks/_common.sh:545:        init_output=$($tf_path init -backend=false "${TF_INIT_ARGS[@]}" 2>&1)
hooks/terraform_fmt.sh:52:  $tf_path fmt "${args[@]}"

Heh, I fixed all the occurrences you listed before I saw your comment.

I made one additional change in terraform_providers_lock.sh to use $tf_path instead of the text "terraform" to match the style used in other files.

Should be good to go now.

[yermulnik]

LGTM
Let's wait for @MaxymVlasov to take a look into this change so that we don't miss something non-obvious

[antonbabenko]

This PR is included in version 1.94.2 🎉

[pcarn]

This caused a breaking change, FYI.

My CI job calls pre-commit run --all-files, and it started failing with 1.94.2.

I found that terraform was not installed on the CI environment. Before 1.94.2, the pre-commit job would pass anyway. Now it fails for terraform being missing.

It did expose that terraform wasn't actually installed, so it was helpful in that way. But I wanted to call out the breaking change.

[MaxymVlasov]

1. I don't see how changes in this PR could affect your CI

2. Improving checks which prevents unexpected behavior (from your description, previously it just bypassed checks) is fix by definition, not breaking change.

3. Also, it should be checked here -

   pre-commit-terraform/hooks/_common.sh

   Lines 450 to 500 in 89f4561

   	#######################################################################
   	# Get Terraform/OpenTofu binary path
   	# Allows user to set the path to custom Terraform or OpenTofu binary
   	# Globals (init and populate):
   	# HOOK_CONFIG (array) arguments that configure hook behavior
   	# PCT_TFPATH (string) user defined env var with path to Terraform/OpenTofu binary
   	# TERRAGRUNT_TFPATH (string) user defined env var with path to Terraform/OpenTofu binary
   	# Outputs:
   	# If failed - exit 1 with error message about missing Terraform/OpenTofu binary
   	#######################################################################
   	function common::get_tf_binary_path {
   	local hook_config_tf_path
   	for config in "${HOOK_CONFIG[@]}"; do
   	if [[ $config == --tf-path=* ]]; then
   	hook_config_tf_path=${config#*=}
   	hook_config_tf_path=${hook_config_tf_path%;}
   	break
   	fi
   	done
   	# direct hook config, has the highest precedence
   	if [[ $hook_config_tf_path ]]; then
   	echo "$hook_config_tf_path"
   	return
   	# environment variable
   	elif [[ $PCT_TFPATH ]]; then
   	echo "$PCT_TFPATH"
   	return
   	# Maybe there is a similar setting for Terragrunt already
   	elif [[ $TERRAGRUNT_TFPATH ]]; then
   	echo "$TERRAGRUNT_TFPATH"
   	return
   	# check if Terraform binary is available
   	elif command -v terraform &> /dev/null; then
   	command -v terraform
   	return
   	# finally, check if Tofu binary is available
   	elif command -v tofu &> /dev/null; then
   	command -v tofu
   	return
   	else
   	common::colorify "red" "Neither Terraform nor OpenTofu binary could be found. Please either set the \"--tf-path\" hook configuration argument, or set the \"PCT_TFPATH\" environment variable, or set the \"TERRAGRUNT_TFPATH\" environment variable, or install Terraform or OpenTofu globally."
   	exit 1
   	fi
   	}

   @pcarn Please open an issue if you can provide minimum reproduction example via GH workflows etc, as I not sure how it was even possible to out from function above without error, if there is no binary, and you didn't specify path in configuration explicitly

[pcarn]

To be clear, I’m glad this change was made. When terraform was not installed, it was not actually doing any checks or doing any formatting. It succeeded silently.

I’m mentioning it because when it started failing with no changes of my own, I looked at the changelog and nothing looked related. I went version by version to find that it was this that changed it.

minimal steps to reproduce:

• set up a repo with terraform files that are unformatted.
• do not have terraform installed.
• use this hook

Before this commit, it would pass, and make no formatting changes.

After this commit, it fails, and says terraform is not installed. (Which is probably the better behavior)

[MaxymVlasov]

Okay, it's actually asked for terraform binary before < 1.90.0.
But somehow check start bypass in 1.90.0 (#670)
And this regression fixed in some magical way in 1.94.2

I'll add this to release notes

And we need tests here -_-

@pcarn thank you, btw