Fix: attach lockfiles to terraform modules (#21220)

#21099 made synthetic targets for lockfiles. `terraform_deployments`
would then pull these in as source files. But `terraform validate` can
be run against modules, and these were missing their lockfiles.

This MR attaches the lockfiles to the module. `terraform_deployment`
targets still pull in the lockfile, but now transitively through the
module.

Author: lilatomic

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

@@ -2,6 +2,7 @@
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 from __future__ import annotations
 
+import os.path
 from dataclasses import dataclass
 from typing import Optional
 
@@ -15,7 +16,9 @@
 )
 from pants.backend.terraform.tool import TerraformProcess
 from pants.backend.terraform.utils import terraform_arg, terraform_relpath
+from pants.base.glob_match_error_behavior import GlobMatchErrorBehavior
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
+from pants.engine.fs import DigestSubset, PathGlobs
 from pants.engine.internals.native_engine import Address, AddressInput, Digest, MergeDigests
 from pants.engine.internals.selectors import Get, MultiGet
 from pants.engine.process import FallibleProcessResult, ProcessExecutionFailure
@@ -103,7 +106,6 @@ class TerraformInitRequest:
 
     # Not initialising the backend means we won't access remote state. Useful for `validate`
     initialise_backend: bool = False
-    upgrade: bool = False
 
 
 @dataclass(frozen=True)
@@ -113,8 +115,14 @@ class TerraformInitResponse:
     chdir: str
 
 
-@rule
-async def init_terraform(request: TerraformInitRequest) -> TerraformInitResponse:
+@dataclass(frozen=True)
+class TerraformUpgradeResponse:
+    lockfile: Digest
+    chdir: str
+
+
+async def run_terraform_init(request: TerraformInitRequest, upgrade: bool):
+    """Just run `terraform init`"""
     this_targets_dependencies = await Get(
         TransitiveTargets, TransitiveTargetsRequest((request.dependencies.address,))
     )
@@ -180,25 +188,38 @@ async def init_terraform(request: TerraformInitRequest) -> TerraformInitResponse
     )
 
     has_lockfile = invocation_files.lockfile is not None
-    third_party_deps = await Get(
+    init_response = await Get(
         TerraformDependenciesResponse,
         TerraformDependenciesRequest(
             chdir,
             backend_config,
             has_lockfile,
             source_for_validate,
             initialise_backend=request.initialise_backend,
-            upgrade=request.upgrade,
+            upgrade=upgrade,
         ),
     )
 
+    return source_files, dependencies_files, init_response, chdir
+
+
+@rule
+async def terraform_init(request: TerraformInitRequest) -> TerraformInitResponse:
+    """Run `terraform init`.
+
+    Returns all the initialised files, ready for execution of subsequent tasks
+    """
+    source_files, dependencies_files, init_response, chdir = await run_terraform_init(
+        request, upgrade=False
+    )
+
     all_terraform_files = await Get(
         Digest,
         MergeDigests(
             [
                 source_files.snapshot.digest,
                 dependencies_files.snapshot.digest,
-                third_party_deps.digest,
+                init_response.digest,
             ]
         ),
     )
@@ -208,5 +229,29 @@ async def init_terraform(request: TerraformInitRequest) -> TerraformInitResponse
     )
 
 
+@rule
+async def terraform_upgrade_lockfile(request: TerraformInitRequest) -> TerraformUpgradeResponse:
+    """Run `terraform init -upgrade`. Returns just the lockfile.
+
+    This split exists because the new and old lockfile will conflict if merging digests
+    """
+
+    _, _, init_response, chdir = await run_terraform_init(request, upgrade=True)
+
+    lockfile = await Get(
+        Digest,
+        DigestSubset(
+            init_response.digest,
+            PathGlobs(
+                (os.path.join(chdir, ".terraform.lock.hcl"),),
+                glob_match_error_behavior=GlobMatchErrorBehavior.error,
+                description_of_origin="upgrade terraform lockfile with `terraform init`",
+            ),
+        ),
+    )
+
+    return TerraformUpgradeResponse(lockfile, chdir)
+
+
 def rules():
     return collect_rules()

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

@@ -110,50 +110,16 @@ async def setup_process_for_parse_terraform_module_sources(
 
 @dataclass(frozen=True)
 class TerraformModuleDependenciesInferenceFieldSet(FieldSet):
-    required_fields = (TerraformModuleSourcesField,)
+    required_fields = (TerraformModuleSourcesField, TerraformDependenciesField)
 
     sources: TerraformModuleSourcesField
+    dependencies: TerraformDependenciesField
 
 
 class InferTerraformModuleDependenciesRequest(InferDependenciesRequest):
     infer_from = TerraformModuleDependenciesInferenceFieldSet
 
 
-@rule
-async def infer_terraform_module_dependencies(
-    request: InferTerraformModuleDependenciesRequest,
-) -> InferredDependencies:
-    hydrated_sources = await Get(HydratedSources, HydrateSourcesRequest(request.field_set.sources))
-
-    paths = OrderedSet(
-        filename for filename in hydrated_sources.snapshot.files if filename.endswith(".tf")
-    )
-    result = await Get(
-        ProcessResult,
-        ParseTerraformModuleSources(
-            sources_digest=hydrated_sources.snapshot.digest,
-            paths=tuple(paths),
-        ),
-    )
-    candidate_spec_paths = [line for line in result.stdout.decode("utf-8").split("\n") if line]
-
-    # For each path, see if there is a `terraform_module` target at the specified spec_path.
-    candidate_targets = await Get(
-        Targets,
-        RawSpecs(
-            dir_globs=tuple(DirGlobSpec(path) for path in candidate_spec_paths),
-            unmatched_glob_behavior=GlobMatchErrorBehavior.ignore,
-            description_of_origin="the `terraform_module` dependency inference rule",
-        ),
-    )
-    # TODO: Need to either implement the standard ambiguous dependency logic or ban >1 terraform_module
-    # per directory.
-    terraform_module_addresses = [
-        tgt.address for tgt in candidate_targets if tgt.has_field(TerraformModuleSourcesField)
-    ]
-    return InferredDependencies(terraform_module_addresses)
-
-
 @dataclass(frozen=True)
 class TerraformDeploymentDependenciesInferenceFieldSet(TerraformDeploymentFieldSet):
     pass
@@ -260,6 +226,66 @@ def identify_terraform_backend_and_vars(
     return TerraformDeploymentInvocationFiles(backend_targets, vars_targets, lockfile)
 
 
+async def _infer_dependencies_from_sources(
+    request: InferTerraformModuleDependenciesRequest,
+) -> list[Address]:
+    """Parse the source code for references to other modules."""
+    hydrated_sources = await Get(HydratedSources, HydrateSourcesRequest(request.field_set.sources))
+    paths = OrderedSet(
+        filename for filename in hydrated_sources.snapshot.files if filename.endswith(".tf")
+    )
+    result = await Get(
+        ProcessResult,
+        ParseTerraformModuleSources(
+            sources_digest=hydrated_sources.snapshot.digest,
+            paths=tuple(paths),
+        ),
+    )
+    candidate_spec_paths = [line for line in result.stdout.decode("utf-8").split("\n") if line]
+    # For each path, see if there is a `terraform_module` target at the specified spec_path.
+    candidate_targets = await Get(
+        Targets,
+        RawSpecs(
+            dir_globs=tuple(DirGlobSpec(path) for path in candidate_spec_paths),
+            unmatched_glob_behavior=GlobMatchErrorBehavior.ignore,
+            description_of_origin="the `terraform_module` dependency inference rule",
+        ),
+    )
+    # TODO: Need to either implement the standard ambiguous dependency logic or ban >1 terraform_module
+    # per directory.
+    terraform_module_addresses = [
+        tgt.address for tgt in candidate_targets if tgt.has_field(TerraformModuleSourcesField)
+    ]
+    return terraform_module_addresses
+
+
+async def _infer_lockfile(request: InferTerraformModuleDependenciesRequest) -> list[Address]:
+    """Pull in the lockfile for a Terraform module.
+
+    This is necessary for `terraform validate`.
+    """
+    invocation_files = await Get(
+        TerraformDeploymentInvocationFiles,
+        TerraformDeploymentInvocationFilesRequest(
+            request.field_set.address, request.field_set.dependencies
+        ),
+    )
+    if invocation_files.lockfile:
+        return [invocation_files.lockfile.address]
+    else:
+        return []
+
+
+@rule
+async def infer_terraform_module_dependencies(
+    request: InferTerraformModuleDependenciesRequest,
+) -> InferredDependencies:
+    terraform_module_addresses = await _infer_dependencies_from_sources(request)
+    lockfile_address = await _infer_lockfile(request)
+
+    return InferredDependencies([*terraform_module_addresses, *lockfile_address])
+
+
 @rule
 async def infer_terraform_deployment_dependencies(
     request: InferTerraformDeploymentDependenciesRequest,
@@ -276,8 +302,7 @@ async def infer_terraform_deployment_dependencies(
     )
     deps.extend(e.address for e in invocation_files.backend_configs)
     deps.extend(e.address for e in invocation_files.vars_files)
-    if invocation_files.lockfile:
-        deps.append(invocation_files.lockfile.address)
+    # lockfile is attached to the module itself
 
     return InferredDependencies(deps)
 

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

@@ -13,9 +13,11 @@
     TerraformHcl2Parser,
     TerraformModuleDependenciesInferenceFieldSet,
 )
+from pants.backend.terraform.goals.lockfiles import rules as terraform_lockfile_rules
 from pants.backend.terraform.target_types import (
     TerraformBackendTarget,
     TerraformDeploymentTarget,
+    TerraformLockfileTarget,
     TerraformModuleTarget,
     TerraformVarFileTarget,
 )
@@ -43,10 +45,12 @@ def rule_runner() -> RuleRunner:
             TerraformDeploymentTarget,
             TerraformBackendTarget,
             TerraformVarFileTarget,
+            TerraformLockfileTarget,
         ],
         rules=[
             *external_tool.rules(),
             *source_files.rules(),
+            *terraform_lockfile_rules(),
             *dependency_inference.rules(),
             QueryRule(InferredDependencies, [InferTerraformModuleDependenciesRequest]),
             QueryRule(InferredDependencies, [InferTerraformDeploymentDependenciesRequest]),
@@ -68,6 +72,7 @@ def test_dependency_inference_module(rule_runner: RuleRunner) -> None:
             "src/tf/modules/foo/versions.tf": "",
             "src/tf/modules/foo/bar/BUILD": "terraform_module()\n",
             "src/tf/modules/foo/bar/versions.tf": "",
+            "src/tf/resources/grok/.terraform.lock.hcl": "",
             "src/tf/resources/grok/subdir/BUILD": "terraform_module()\n",
             "src/tf/resources/grok/subdir/versions.tf": "",
             "src/tf/resources/grok/BUILD": "terraform_module()\n",
@@ -107,6 +112,7 @@ def test_dependency_inference_module(rule_runner: RuleRunner) -> None:
                 Address("src/tf/modules/foo"),
                 Address("src/tf/modules/foo/bar"),
                 Address("src/tf/resources/grok/subdir"),
+                Address("src/tf/resources/grok", target_name=".terraform.lock.hcl"),
             ]
         ),
     )
@@ -142,6 +148,7 @@ def test_dependency_inference_autoinfered_files(rule_runner: RuleRunner) -> None
             "src/tf/main.tf": "",
             "src/tf/main.tfvars": "",
             "src/tf/main.tfbackend": "",
+            "src/tf/.terraform.lock.hcl": "",
         }
     )
     target = rule_runner.get_target(Address("src/tf", target_name="deployment"))

src/python/pants/backend/terraform/goals/lockfiles.py

@@ -4,15 +4,14 @@
 from dataclasses import dataclass
 from pathlib import Path
 
-from pants.backend.terraform.dependencies import TerraformInitRequest, TerraformInitResponse
+from pants.backend.terraform.dependencies import TerraformInitRequest, TerraformUpgradeResponse
 from pants.backend.terraform.target_types import (
     TerraformDependenciesField,
     TerraformLockfileTarget,
     TerraformModuleSourcesField,
     TerraformModuleTarget,
     TerraformRootModuleField,
 )
-from pants.backend.terraform.tool import TerraformProcess
 from pants.core.goals.generate_lockfiles import (
     GenerateLockfile,
     GenerateLockfileResult,
@@ -27,7 +26,6 @@
 from pants.engine.internals.selectors import Get, MultiGet
 from pants.engine.internals.synthetic_targets import SyntheticAddressMaps, SyntheticTargetsRequest
 from pants.engine.internals.target_adaptor import TargetAdaptor
-from pants.engine.process import ProcessResult
 from pants.engine.rules import collect_rules, rule
 from pants.engine.target import AllTargets, Targets
 from pants.engine.unions import UnionRule
@@ -101,32 +99,20 @@ async def generate_lockfile_from_sources(
 ) -> GenerateLockfileResult:
     """Generate a Terraform lockfile by running `terraform providers lock` on the sources."""
     initialised_terraform = await Get(
-        TerraformInitResponse,
+        TerraformUpgradeResponse,
         TerraformInitRequest(
             TerraformRootModuleField(
                 lockfile_request.target.address.spec, lockfile_request.target.address
             ),
             lockfile_request.target[TerraformDependenciesField],
             initialise_backend=False,
-            upgrade=True,
-        ),
-    )
-    result = await Get(
-        ProcessResult,
-        TerraformProcess(
-            args=(
-                "providers",
-                "lock",
-            ),
-            input_digest=initialised_terraform.sources_and_deps,
-            output_files=(".terraform.lock.hcl",),
-            description=f"Update terraform lockfile for {lockfile_request.resolve_name}",
-            chdir=initialised_terraform.chdir,
         ),
     )
 
     return GenerateLockfileResult(
-        result.output_digest, lockfile_request.resolve_name, lockfile_request.lockfile_dest
+        initialised_terraform.lockfile,
+        lockfile_request.resolve_name,
+        lockfile_request.lockfile_dest,
     )