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.

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

Run terraform plan on package #18490

Closed
ndellosa95 opened this issue Mar 13, 2023 · 4 comments · Fixed by #20488
Closed

Run terraform plan on package #18490

ndellosa95 opened this issue Mar 13, 2023 · 4 comments · Fixed by #20488
Labels
backend: Terraform Terraform backend-related issues enhancement

Comments

@ndellosa95
Copy link
Contributor

ndellosa95 commented Mar 13, 2023

Is your feature request related to a problem? Please describe.
I created a bug issue here related to pants check not running terraform init before terraform validate. IMO, check should also run terraform plan.

Describe the solution you'd like
pants check runs terraform plan after running init and validate.

Describe alternatives you've considered
Check feels like the right goal to me but I'm fairly ambivalent about what the actual goal is called so long as some mechanism to call terraform plan is in place.

@cognifloyd cognifloyd added the backend: Terraform Terraform backend-related issues label Mar 13, 2023
@ndellosa95 ndellosa95 changed the title Run terraform plan on check after validate Run terraform plan on package May 10, 2023
@lilatomic
Copy link
Contributor

I think that the check goal is the wrong place for terraform plan. check is described as "Run type checking or the lightest variant of compilation available for a language." In that way, terraform validate is the command that fits under that goal. terraform plan needs to interact with state, which is a reason against including it under check.
I agree that we should have a way of running terraform plan that isn't just "type 'no' at the prompt for apply". Pants wasn't designed with managing infrastructure in mind, so it doesn't have a gamut of goals to map 1:1 to every Terraform command. (at the extreme, Pants having a workspace goal wouldn't really fit). So there isn't really a goal that terraform plan maps to. Given that plan is essentially a dry-run, we could make a "dry run" flag which will toggle between plan and apply (invocation like pants experimental-deploy --dry-run src/tf/my_deployment:stg). That would let you set it as an environment variable as well.
An alternative would be to use the export goal (not yet implemented for Terraform) to do the equivalent of terraform init so you can work out of there with all the commands (that's probably what you'd need to do to run the Terraform-specific commands like import). But given that plan is just a dry-run, I think it would be a bit of a shame to have to break out.
For comparison, Helm allows this by allowing --dry-run to be passed through to the underlying processes.

@agoblet
Copy link
Contributor

agoblet commented Feb 5, 2024

Hello there, I would like to implement this feature. I added a draft PR highlighting my approach so far. I'm still figuring out the test suite, so it's not done yet. But I wanted to get in touch already for some early feedback. How do you feel about this change?

@agoblet
Copy link
Contributor

agoblet commented Feb 5, 2024

added a unit test as well and removed the Draft from the PR, looking forward to your feedback.

@agoblet
Copy link
Contributor

agoblet commented Feb 13, 2024

ping @alonsodomin @lilatomic

alonsodomin pushed a commit that referenced this issue Feb 16, 2024
…eploy` goal (#20488)

Changed the following things to achieve this
- Add a `--dry-run` flag to the `experimental-deploy` goal to handle dry
runs such as `terraform plan`
- Run `plan` rather than `apply` when setting the `--dry-run` flag while
deploying a `terraform_deployment`
- Changed Helm deployments to use the new `--dry-run` flag instead of a
passthrough arg for dry-running

Tested 
- the terraform change via a unit test and also tried it out manually in
my own project.
- the helm change via a unit test.

Closes #18490
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Terraform Terraform backend-related issues enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants