busser / tfautomv

Generate Terraform moved blocks automatically for painless refactoring
https://github.com/busser/tfautomv
Apache License 2.0
693 stars 24 forks source link

Support configuring Terraform Binary name #27

Closed randywallace closed 1 year ago

randywallace commented 1 year ago

https://github.com/padok-team/tfautomv/blob/bbb53aabe7ed7bca9d26b5d62e08530b76bb5e75/internal/terraform/runner.go#L73

It would be very useful to be able to override the command used to run terraform. My usecase is that I would like to use terragrunt to "wrap" the terraform binary. I'm sure there are others.

busser commented 1 year ago

Hi @randywallace, thanks for opening this issue!

If I understand you correctly, the problem is that you don't use Terraform directly to run your Terraform code, but rather Terragrunt, so you need tfautomv to run Terragrunt instead of Terraform.

Could you tell me more about the workflow you would like to achieve with tfautomv? If you would provide an example, with a directory structure and commands you would run, and the result you would expect, that would really help!

spoukke commented 1 year ago

Hi!

Maybe we could have a switch between terraform and terragrunt. And we could select which one to use through a --command flag. Indeed, terragrunt works thourgh a terragrunt plan.

However, this will not take into account the option of running terragrunt plan-all, so you would still have to run tfautomv in each layer.

How does that sound @busser @randywallace ?

busser commented 1 year ago

I don't have much experience with Terragrunt, so correct me if I'm wrong about something.

Currently, tfautomv runs the following terraform commands:

If I understand you correctly, replacing terraform with terragrunt in all of these commands yields the same result, because terragrunt calls terraform in a passthrough manner.

Assuming all the above is correct, here are my thoughts:

From a UX standpoint, it doesn't seem like we need a flag. We could simply check if the current directory has a terragrunt.hcl file and adjust automatically. That raises the question of whether users may want to override this automatic behaviour. What do you think?

From an implementation standpoint, should we start thinking about broader Terragrunt support before adding this feature in?

Looking forward to reading your thoughts on the matter.

spoukke commented 1 year ago

I don't have much experience with terragrunt either. Maybe @Alan-pad can give use more information based on his experience.

Alan-pad commented 1 year ago

@busser Detecting a terragrunt.hcl in the folder should do the trick however WDYT of the option to override the binary command launched in your runner. That way you would support :

I don't see the benefits of implementing special cases for terragrunt though, the only thing I see would be the ability to run tfautomv in multiple stacks at the same time using the terragrunt run-all command but even then I don't think it would add a lot of value to the tool.

busser commented 1 year ago

If we want to support any Terraform wrapper, then allowing for a command override seems necessary. For Terragrunt that should be simple enough, but what about multi-argument wrappers? For exemple:

terragrunt plan # simple
terraform-plan # complex
my-wrapper terraform plan # complex

In order to implement a generic solution we would need to list the use-cases we want to support.

busser commented 1 year ago

I opened a draft PR (#29) to see what a quick and dirty implementation might look like. Let me know what you think.

Alan-pad commented 1 year ago

@busser Looks good to me, for the E2E test though, don't you think we need to setup a real terragrunt setup using a local module ? I think it's ok the way you did it, I don't know if there could be a special case to handle though

busser commented 1 year ago

I agree that the E2E test is insufficient. I would like the test to fail if tfautomv uses terraform and not terragrunt. I don't have enough experience with terragrunt to know how to do that simply. You think a local module would do the trick?

busser commented 1 year ago

I've updated the E2E test in PR #29 to use a more realistic Terragrunt setup. The test now fails because our E2E test suite uses terraform to set up, where it should use terragrunt instead. Another issue is that the test suite moves Terraform's state file (terraform.tfstate). With Terragrunt, this file is in the .terragrunt-cache directory. Moving this directory is not enough, because Terragrunt does not reuse the cache (I'm assuming because the original code and the refactored code have different paths).

We need to update the test suite so that we have separate logic for testing with Terraform or with Terragrunt.

busser commented 1 year ago

PR #34 adds a -terraform-bin flag that exactly what you need @randywallace. I'll close this issue once I've released version 0.5.