Terraform: Add experimental-deploy and wire-in dependency inference#19185
Terraform: Add experimental-deploy and wire-in dependency inference#19185benjyw merged 22 commits intopantsbuild:mainfrom
experimental-deploy and wire-in dependency inference#19185Conversation
|
resolves #18491 |
|
One complication is that this requires that the |
alonsodomin
left a comment
There was a problem hiding this comment.
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
| description="Run `terraform init` to fetch dependencies", | ||
| chdir=directory, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
This looks like a good candidate to be part of the generate-lockfiles goal
There was a problem hiding this comment.
I agree, I'm hoping to do that next.
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class GetTerraformDependenciesRequest: |
There was a problem hiding this comment.
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?
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class TerraformInitRequest: |
There was a problem hiding this comment.
Same comment re naming convention.
|
wait, seems terraform_modules are deployable. I'm not sure that's desireable, or really why that's happening. Probably too loose a fieldset? |
2e549e0 to
4060959
Compare
4060959 to
005ecd8
Compare
smash together parts from Helm and Terraform check
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
005ecd8 to
4c83a37
Compare
|
Dependency listing doesn't work yet, since the |
|
LGTM 👍 |
| address_input = request.root_module.to_address_input() | ||
| module_address = await Get(Address, AddressInput, address_input) |
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
But this is fine to be done as a follow-up.
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
TerraformDeploymenttarget type. Once you have that, might as well wire it in to theexperimental-deploygoal (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.