Properly include file dependencies in docker build context (cherrypic…

…k of #12758) (#12823)

Turns out that there were quite a few corner cases uncovered wrgt targets, transitive targets and dependencies that I hadn't really come to terms with.

[ci skip-rust]
[ci skip-build-wheels]

Co-authored-by: Andreas Stenius <andreas.stenius@svenskaspel.se>

src/python/pants/backend/docker/docker_binary.py

@@ -23,13 +23,13 @@ class DockerBinary(BinaryPath):
 
     DEFAULT_SEARCH_PATH = SearchPath(("/usr/bin", "/bin", "/usr/local/bin"))
 
-    def build_image(
-        self, tag: str, digest: Digest, context_root: str, dockerfile: Optional[str] = None
-    ) -> Process:
+    def build_image(self, tag: str, digest: Digest, dockerfile: Optional[str] = None) -> Process:
         args = [self.path, "build", "-t", tag]
         if dockerfile:
             args.extend(["-f", dockerfile])
-        args.append(context_root)
+
+        # Add build context root.
+        args.append(".")
 
         return Process(
             argv=tuple(args),

src/python/pants/backend/docker/docker_binary_test.py

@@ -2,24 +2,22 @@
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
 from hashlib import sha256
-from os import path
 
 from pants.backend.docker.docker_binary import DockerBinary
 from pants.engine.fs import Digest
 from pants.engine.process import Process
 
 
 def test_docker_binary_build_image():
-    source_path = "src/test/repo"
     docker_path = "/bin/docker"
-    dockerfile = path.join(source_path, "Dockerfile")
+    dockerfile = "src/test/repo/Dockerfile"
     docker = DockerBinary(docker_path)
     digest = Digest(sha256().hexdigest(), 123)
     tag = "test:latest"
-    build_request = docker.build_image(tag, digest, source_path, dockerfile)
+    build_request = docker.build_image(tag, digest, dockerfile)
 
     assert build_request == Process(
-        argv=(docker_path, "build", "-t", tag, "-f", dockerfile, source_path),
+        argv=(docker_path, "build", "-t", tag, "-f", dockerfile, "."),
         input_digest=digest,
         description=f"Building docker image {tag}",
     )

src/python/pants/backend/docker/docker_build.py

@@ -7,11 +7,7 @@
 
 from pants.backend.docker.docker_binary import DockerBinary, DockerBinaryRequest
 from pants.backend.docker.docker_build_context import DockerBuildContext, DockerBuildContextRequest
-from pants.backend.docker.target_types import (
-    DockerContextRoot,
-    DockerImageSources,
-    DockerImageVersion,
-)
+from pants.backend.docker.target_types import DockerImageSources, DockerImageVersion
 from pants.core.goals.package import BuiltPackage, BuiltPackageArtifact, PackageFieldSet
 from pants.engine.process import Process, ProcessResult
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
@@ -24,7 +20,6 @@
 class DockerFieldSet(PackageFieldSet):
     required_fields = (DockerImageSources,)
 
-    context_root_field: DockerContextRoot
     image_version: DockerImageVersion
     sources: DockerImageSources
 
@@ -38,17 +33,6 @@ def dockerfile_relpath(self) -> str:
     def dockerfile_path(self) -> str:
         return os.path.join(self.address.spec_path, self.dockerfile_relpath)
 
-    @property
-    def context_root(self) -> str:
-        """Translates context_root from a users perspective in the BUILD file, to Dockers
-        perspective of the file system."""
-        path = self.context_root_field.value or "."
-        if os.path.isabs(path):
-            path = os.path.relpath(path, "/")
-        else:
-            path = os.path.join(self.address.spec_path, path)
-        return path
-
     @property
     def image_tag(self) -> str:
         return ":".join(s for s in [self.address.target_name, self.image_version.value] if s)
@@ -61,18 +45,21 @@ async def build_docker_image(
     docker, context = await MultiGet(
         Get(DockerBinary, DockerBinaryRequest()),
         Get(
-            DockerBuildContext, DockerBuildContextRequest(field_set.address, field_set.context_root)
+            DockerBuildContext,
+            DockerBuildContextRequest(
+                address=field_set.address,
+                build_upstream_images=True,
+            ),
         ),
     )
 
     result = await Get(
         ProcessResult,
         Process,
         docker.build_image(
-            field_set.image_tag,
-            context.digest,
-            field_set.context_root,
-            field_set.dockerfile_path,
+            tag=field_set.image_tag,
+            digest=context.digest,
+            dockerfile=field_set.dockerfile_path,
         ),
     )
 

src/python/pants/backend/docker/docker_build_context.py

@@ -3,18 +3,22 @@
 
 import logging
 from dataclasses import dataclass
+from itertools import chain
 
 from pants.backend.docker.target_types import DockerImageSources
 from pants.core.goals.package import BuiltPackage, PackageFieldSet
-from pants.core.target_types import FilesSources, ResourcesSources
+from pants.core.target_types import FilesSources
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
 from pants.engine.addresses import Address
-from pants.engine.fs import AddPrefix, Digest, MergeDigests
+from pants.engine.fs import Digest, MergeDigests
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
 from pants.engine.target import (
+    Dependencies,
+    DependenciesRequest,
     FieldSetsPerTarget,
     FieldSetsPerTargetRequest,
     Sources,
+    Targets,
     TransitiveTargets,
     TransitiveTargetsRequest,
 )
@@ -30,19 +34,35 @@ class DockerBuildContext:
 @dataclass(frozen=True)
 class DockerBuildContextRequest:
     address: Address
-    context_root: str
+    build_upstream_images: bool = False
 
 
 @rule
 async def create_docker_build_context(request: DockerBuildContextRequest) -> DockerBuildContext:
+    # Get all targets to include in context.
     transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest([request.address]))
 
-    # Get all sources, going into the build context.
+    # Get the Dockerfile from the root target.
+    dockerfiles_request = Get(
+        SourceFiles,
+        SourceFilesRequest(
+            sources_fields=[t.get(Sources) for t in transitive_targets.roots],
+            for_sources_types=(DockerImageSources,),
+        ),
+    )
+
+    # Get all dependencies for the root target.
+    root_dependencies = await MultiGet(
+        Get(Targets, DependenciesRequest(target.get(Dependencies)))
+        for target in transitive_targets.roots
+    )
+
+    # Get all sources from the root dependencies (i.e. files).
     sources_request = Get(
         SourceFiles,
         SourceFilesRequest(
-            sources_fields=[t.get(Sources) for t in transitive_targets.closure],
-            for_sources_types=(DockerImageSources, FilesSources, ResourcesSources),
+            sources_fields=[t.get(Sources) for t in chain(*root_dependencies)],
+            for_sources_types=(FilesSources,),
         ),
     )
 
@@ -51,37 +71,31 @@ async def create_docker_build_context(request: DockerBuildContextRequest) -> Doc
         FieldSetsPerTargetRequest(PackageFieldSet, transitive_targets.dependencies),
     )
 
-    sources, embedded_pkgs_per_target = await MultiGet(
-        sources_request, embedded_pkgs_per_target_request
+    dockerfiles, sources, embedded_pkgs_per_target = await MultiGet(
+        dockerfiles_request,
+        sources_request,
+        embedded_pkgs_per_target_request,
     )
 
     # Package binary dependencies for build context.
     embedded_pkgs = await MultiGet(
         Get(BuiltPackage, PackageFieldSet, field_set)
         for field_set in embedded_pkgs_per_target.field_sets
+        # Exclude docker images, unless build_upstream_images is true.
+        if request.build_upstream_images
+        or not isinstance(getattr(field_set, "sources", None), DockerImageSources)
     )
 
     packages_str = ", ".join(a.relpath for p in embedded_pkgs for a in p.artifacts if a.relpath)
     logger.debug(f"Packages for Docker image: {packages_str}")
 
-    embedded_pkgs_digests = tuple(built_package.digest for built_package in embedded_pkgs)
-    if request.context_root != ".":
-        # Copy packages to context root tree, unless the context root is at the project root.
-        embedded_pkgs_digests = await MultiGet(
-            Get(Digest, AddPrefix(digest, request.context_root)) for digest in embedded_pkgs_digests
-        )
+    embedded_pkgs_digest = [built_package.digest for built_package in embedded_pkgs]
+    all_digests = (dockerfiles.snapshot.digest, sources.snapshot.digest, *embedded_pkgs_digest)
 
-    # Merge build context.
+    # Merge all digests to get the final docker build context.
     context = await Get(
         Digest,
-        MergeDigests(
-            d
-            for d in (
-                sources.snapshot.digest,
-                *embedded_pkgs_digests,
-            )
-            if d
-        ),
+        MergeDigests(d for d in all_digests if d),
     )
 
     return DockerBuildContext(context)