Terraform: Add `experimental-deploy` and wire-in dependency inference #19185

lilatomic · 2023-05-29T03:41:55Z

This was actually some prework for lockfiles. But Terraform has some ideas about differences between a "root" module (one you would deploy) and other modules (ones which would be included in a deployment). To model that, I creating the TerraformDeployment target type. Once you have that, might as well wire it in to the experimental-deploy goal (especially since that's most of the value of Terraform).
Also it looks like we weren't fetching dependencies (on other modules) when building up the files to run Terraform commands. Now that happens.

I feel like I'm not being efficient in this MR with the way I bundle the (pants-inferred) dependencies together with everything, since there's this ever-growing ball of files. LMK if we've got a better way.

lilatomic · 2023-05-29T03:47:03Z

resolves #18491

lilatomic · 2023-05-29T04:51:47Z

One complication is that this requires that the check goal is run against only terraform_deployment targets. This is technically in line with what Terraform tolerates, as it is possible for a "non-root" module to have incomplete provider blocks which would be specified in a root module (hashicorp/terraform#28490). OTOH, many non-root modules will not experience this, so maybe we should work around the need for a backend config. Is there a way to get a field only if it exists on the FieldSet?

alonsodomin

A few nits here and there. It would be great if we could align this with other usages of the experimental-deploy so end users find some familiarity

src/python/pants/backend/terraform/target_types.py

src/python/pants/backend/terraform/tool.py

alonsodomin · 2023-05-31T08:48:27Z

src/python/pants/backend/terraform/dependencies.py

+                description="Run `terraform init` to fetch dependencies",
+                chdir=directory,
+            ),
+        )


This looks like a good candidate to be part of the generate-lockfiles goal

I agree, I'm hoping to do that next.

tdyas · 2023-05-31T18:46:47Z

src/python/pants/backend/terraform/dependencies.py

+
+
+@dataclass(frozen=True)
+class GetTerraformDependenciesRequest:


Naming convention: In many backends, the more recent naming convention is FooRequest and FooResult. Although I see this code moved and I did not catch the naming issue before, :(

@Eric-Arellano: Thoughts here?

tdyas · 2023-05-31T18:48:53Z

src/python/pants/backend/terraform/dependencies.py

+
+
+@dataclass(frozen=True)
+class TerraformInitRequest:


Same comment re naming convention.

src/python/pants/backend/terraform/target_types.py

lilatomic · 2023-06-03T15:56:17Z

wait, seems terraform_modules are deployable. I'm not sure that's desireable, or really why that's happening. Probably too loose a fieldset?

smash together parts from Helm and Terraform check

…init logic

accommodates no Vars files and will not slurp in several conflicting ones

(request them and pass to processes)

This is technically more correct, since `terraform validate` may not be valid on all modules (since they don't have full provider blocks).

GetTerraformDependenciesRequest -> TerraformDependenciesRequest TerraformDependencies -> TerraformDependenciesResponse InitialisedTerraform -> TerraformInitResponse

lilatomic · 2023-06-19T00:30:25Z

Dependency listing doesn't work yet, since the root_module field is just a string field. I'm planning to do what the golang backend did for the GoBinaryMainPackageField, which is to have a separate dependency inference for go_binary which adds that.

alonsodomin · 2023-06-19T06:29:44Z

LGTM 👍

tdyas · 2023-06-19T06:41:54Z

src/python/pants/backend/terraform/dependencies.py

+    address_input = request.root_module.to_address_input()
+    module_address = await Get(Address, AddressInput, address_input)


I would recommend inferring root_module to point at the terraform_module in the same directory if it is not specified. This is what go_binary does for the main field. (There should only be one terraform_module per directory so this inference should always work.)

But this is fine to be done as a follow-up.

lilatomic added backend: Terraform Terraform backend-related issues category:new feature labels May 29, 2023

stuhood requested review from alonsodomin and tdyas May 30, 2023 22:09

alonsodomin reviewed May 31, 2023

View reviewed changes

tdyas reviewed May 31, 2023

View reviewed changes

alonsodomin mentioned this pull request Jun 3, 2023

Helm deployment chart field #19234

Merged

alonsodomin approved these changes Jun 3, 2023

View reviewed changes

lilatomic marked this pull request as draft June 3, 2023 15:54

lilatomic force-pushed the terraform/deploy branch from 2e549e0 to 4060959 Compare June 7, 2023 23:37

lilatomic marked this pull request as ready for review June 8, 2023 00:18

lilatomic force-pushed the terraform/deploy branch from 4060959 to 005ecd8 Compare June 14, 2023 18:06

lilatomic added 15 commits June 17, 2023 15:45

add TerraformDeploymentTarget

83784b2

wire into deploy

6bc3b76

smash together parts from Helm and Terraform check

pull terraform init call into separate rule

93e00c5

add support for backend configs to deploy

3d2b892

add support for var files to deploy

9d8e577

add support for extra args to deploy

b096b9d

commit utils

e2a98e7

allow forwarding env vars

20ac764

basic end-to-end test

f55b355

test for args passed to terraform process

3e539ff

split test for initialising terraform

15ce0f7

move fetching terraform dependencies out of inference and with other …

830cb0f

…init logic

do not automatically pull in tfvars

e33ae40

accommodates no Vars files and will not slurp in several conflicting ones

wire in dependencies (1/2)

b218b98

wire in dependencies (2/2)

99cfee8

(request them and pass to processes)

lilatomic added 7 commits June 17, 2023 15:45

use terraform_deployment target for check

ff21607

This is technically more correct, since `terraform validate` may not be valid on all modules (since they don't have full provider blocks).

remove unused function

3c5ef5a

rename TerraformVarFilesField -> TerraformVarFileSourcesField

f3e096e

simplify environment var passthrough

87cd71e

convert per-target extra-args to passthrough

e30b0cb

rename to follow convention

836a6b3

GetTerraformDependenciesRequest -> TerraformDependenciesRequest TerraformDependencies -> TerraformDependenciesResponse InitialisedTerraform -> TerraformInitResponse

rebuild terraform_deployment to target terraform_module as a root module

4c83a37

lilatomic force-pushed the terraform/deploy branch from 005ecd8 to 4c83a37 Compare June 19, 2023 00:14

lilatomic requested a review from alonsodomin June 19, 2023 00:30

alonsodomin approved these changes Jun 19, 2023

View reviewed changes

tdyas reviewed Jun 19, 2023

View reviewed changes

benjyw merged commit 2bfccd3 into pantsbuild:main Jun 25, 2023
24 checks passed

lilatomic mentioned this pull request Nov 22, 2023

Terraform validate not applied with check goal #20222

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Terraform: Add `experimental-deploy` and wire-in dependency inference #19185

Terraform: Add `experimental-deploy` and wire-in dependency inference #19185

lilatomic commented May 29, 2023

lilatomic commented May 29, 2023

lilatomic commented May 29, 2023

alonsodomin left a comment

alonsodomin May 31, 2023

lilatomic May 31, 2023

tdyas May 31, 2023

tdyas May 31, 2023

lilatomic commented Jun 3, 2023

lilatomic commented Jun 19, 2023

alonsodomin commented Jun 19, 2023

tdyas Jun 19, 2023

tdyas Jun 19, 2023



		@dataclass(frozen=True)
		class GetTerraformDependenciesRequest:

		address_input = request.root_module.to_address_input()
		module_address = await Get(Address, AddressInput, address_input)

Terraform: Add experimental-deploy and wire-in dependency inference #19185

Terraform: Add experimental-deploy and wire-in dependency inference #19185

Conversation

lilatomic commented May 29, 2023

lilatomic commented May 29, 2023

lilatomic commented May 29, 2023

alonsodomin left a comment

Choose a reason for hiding this comment

alonsodomin May 31, 2023

Choose a reason for hiding this comment

lilatomic May 31, 2023

Choose a reason for hiding this comment

tdyas May 31, 2023

Choose a reason for hiding this comment

tdyas May 31, 2023

Choose a reason for hiding this comment

lilatomic commented Jun 3, 2023

lilatomic commented Jun 19, 2023

alonsodomin commented Jun 19, 2023

tdyas Jun 19, 2023

Choose a reason for hiding this comment

tdyas Jun 19, 2023

Choose a reason for hiding this comment

Terraform: Add `experimental-deploy` and wire-in dependency inference #19185

Terraform: Add `experimental-deploy` and wire-in dependency inference #19185