Repository navigation
Add ImageFromDockerfile.withBuildContext(Path) #12116
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,6 +63,8 @@ public class ImageFromDockerfile | |
|
|
||
| private Optional<Path> dockerfile = Optional.empty(); | ||
|
|
||
| private Optional<Path> buildContext = Optional.empty(); | ||
|
|
||
| private Optional<String> target = Optional.empty(); | ||
|
|
||
| private final Set<Consumer<BuildImageCmd>> buildImageCmdModifiers = new LinkedHashSet<>(); | ||
|
|
@@ -173,16 +175,36 @@ public void onNext(BuildResponseItem item) { | |
| protected void configure(BuildImageCmd buildImageCmd) { | ||
| buildImageCmd.withTags(Collections.singleton(getDockerImageName())); | ||
| this.dockerFilePath.ifPresent(buildImageCmd::withDockerfilePath); | ||
| this.dockerfile.ifPresent(p -> { | ||
| buildImageCmd.withDockerfile(p.toFile()); | ||
| dependencyImageNames = new ParsedDockerfile(p).getDependencyImageNames(); | ||
|
|
||
| if (dependencyImageNames.size() > 0) { | ||
| // if we'll be pre-pulling images, disable the built-in pull as it is not necessary and will fail for | ||
| // authenticated registries | ||
| buildImageCmd.withPull(false); | ||
| } | ||
| }); | ||
| Optional<Path> effectiveDockerfile = this.dockerfile; | ||
| if (this.buildContext.isPresent()) { | ||
| Path context = this.buildContext.get().toAbsolutePath().normalize(); | ||
| if (!this.transferables.isEmpty()) { | ||
| throw new IllegalStateException( | ||
| "withBuildContext cannot be combined with withFile*/withDockerfileFromBuilder, " + | ||
| "the build context directory is sent as-is" | ||
| ); | ||
| } | ||
| if (!effectiveDockerfile.isPresent()) { | ||
| effectiveDockerfile = Optional.of(context.resolve("Dockerfile")); | ||
| } | ||
| if (!effectiveDockerfile.get().toAbsolutePath().normalize().startsWith(context)) { | ||
| throw new IllegalArgumentException( | ||
| "Dockerfile " + effectiveDockerfile.get() + " must be located inside the build context " + context | ||
| ); | ||
| } | ||
|
Comment on lines
+190
to
+194
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Resolve symlinks before accepting the Dockerfile path. If the Dockerfile path is a symlink to a file outside the context, 🤖 Prompt for AI AgentsSource: Learnings |
||
| // must be set before withDockerfile, which otherwise uses the Dockerfile's parent directory as context | ||
| buildImageCmd.withBaseDirectory(context.toFile()); | ||
| } | ||
| effectiveDockerfile.ifPresent(p -> { | ||
| buildImageCmd.withDockerfile(p.toFile()); | ||
| dependencyImageNames = new ParsedDockerfile(p).getDependencyImageNames(); | ||
|
|
||
| if (dependencyImageNames.size() > 0) { | ||
| // if we'll be pre-pulling images, disable the built-in pull as it is not necessary and will fail for | ||
| // authenticated registries | ||
| buildImageCmd.withPull(false); | ||
| } | ||
| }); | ||
|
|
||
| this.buildArgs.forEach(buildImageCmd::withBuildArg); | ||
| this.target.ifPresent(buildImageCmd::withTarget); | ||
|
|
@@ -269,6 +291,21 @@ public ImageFromDockerfile withDockerfile(Path dockerfile) { | |
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the directory on the test host to be sent as build context, like the context argument of {@code docker build}. | ||
| * A {@code .dockerignore} file in its root is honored. | ||
| * <p> | ||
| * The Dockerfile defaults to {@code Dockerfile} in the root of the build context. Use {@link #withDockerfile(Path)} | ||
| * to select a different one, which must be located inside the build context. | ||
| * Cannot be combined with the {@code withFile*} methods. | ||
| * | ||
| * @param buildContext path to the build context directory on the test host. | ||
| */ | ||
| public ImageFromDockerfile withBuildContext(Path buildContext) { | ||
| this.buildContext = Optional.of(buildContext); | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Allow low level modifications of {@link BuildImageCmd}. | ||
| * Warning: this does expose the underlying docker-java API so might change outside of our control. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| package org.testcontainers.images.builder; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.io.TempDir; | ||
| import org.testcontainers.containers.GenericContainer; | ||
| import org.testcontainers.containers.startupcheck.OneShotStartupCheckStrategy; | ||
| import org.testcontainers.utility.DockerImageName; | ||
|
|
||
| import java.io.IOException; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
|
|
||
| class BuildContextTest { | ||
|
|
||
| @TempDir | ||
| Path context; | ||
|
|
||
| /** | ||
| * A Gradle cache used as build context, with the Dockerfile in a subdirectory and the cache's lock files excluded. | ||
| */ | ||
| @Test | ||
| void shouldSendBuildContextDirectoryHonoringDockerignore() throws IOException { | ||
| write("caches/modules-2/files-2.1/org.example/lib/1.0/lib-1.0.jar", "jar"); | ||
| write("caches/modules-2/modules-2.lock", "lock"); | ||
| write("caches/journal-1/journal-1.lock", "lock"); | ||
| write("caches/journal-1/file-access.bin", "bin"); | ||
| write(".dockerignore", "**/*.lock\n"); | ||
| write("docker/Dockerfile", "FROM alpine:3.16\nCOPY caches /gradle/caches\n"); | ||
|
|
||
| ImageFromDockerfile image = new ImageFromDockerfile() | ||
| .withBuildContext(context) | ||
| .withDockerfile(context.resolve("docker/Dockerfile")); | ||
|
|
||
| assertThat(listFiles(image, "/gradle")) | ||
| .contains("/gradle/caches/modules-2/files-2.1/org.example/lib/1.0/lib-1.0.jar") | ||
| .contains("/gradle/caches/journal-1/file-access.bin") | ||
| .doesNotContain(".lock"); | ||
| } | ||
|
|
||
| @Test | ||
| void shouldDefaultToDockerfileInBuildContextRoot() throws IOException { | ||
| write("Dockerfile", "FROM alpine:3.16\nCOPY . /context\n"); | ||
| write("included.txt", "included"); | ||
| write("excluded.txt", "excluded"); | ||
| write(".dockerignore", "excluded.txt\n"); | ||
|
|
||
| ImageFromDockerfile image = new ImageFromDockerfile().withBuildContext(context); | ||
|
|
||
| assertThat(listFiles(image, "/context")).contains("/context/included.txt").doesNotContain("excluded.txt"); | ||
| } | ||
|
|
||
| @Test | ||
| void shouldRejectDockerfileOutsideBuildContext(@TempDir Path other) throws IOException { | ||
| Path dockerfile = Files.write(other.resolve("Dockerfile"), "FROM alpine:3.16\n".getBytes()); | ||
|
|
||
| ImageFromDockerfile image = new ImageFromDockerfile().withBuildContext(context).withDockerfile(dockerfile); | ||
|
|
||
| assertThatThrownBy(image::get).isInstanceOf(IllegalArgumentException.class); | ||
| } | ||
|
|
||
| @Test | ||
| void shouldRejectBuildContextCombinedWithFiles() throws IOException { | ||
| write("Dockerfile", "FROM alpine:3.16\n"); | ||
|
|
||
| ImageFromDockerfile image = new ImageFromDockerfile() | ||
| .withBuildContext(context) | ||
| .withFileFromString("extra.txt", "extra"); | ||
|
|
||
| assertThatThrownBy(image::get).isInstanceOf(IllegalStateException.class); | ||
| } | ||
|
|
||
| private void write(String relativePath, String content) throws IOException { | ||
| Path file = context.resolve(relativePath); | ||
| Files.createDirectories(file.getParent()); | ||
| Files.write(file, content.getBytes()); | ||
| } | ||
|
|
||
| @SuppressWarnings("resource") | ||
| private static String listFiles(ImageFromDockerfile image, String directory) { | ||
| try ( | ||
| GenericContainer<?> container = new GenericContainer<>(DockerImageName.parse(image.get())) | ||
| .withStartupCheckStrategy(new OneShotStartupCheckStrategy()) | ||
| .withCommand("find", directory, "-type", "f") | ||
| ) { | ||
| container.start(); | ||
| return container.getLogs(); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Honor an existing
withDockerfilePathselection.If a caller sets
withBuildContext(context).withDockerfilePath("docker/Dockerfile")and the context has no root Dockerfile, this branch still selectscontext/Dockerfile. docker-java rejects that missing file before it can use the configured relative path. Select the effective Dockerfile fromdockerFilePathwhen nowithDockerfile(Path)was supplied, or explicitly reject this combination. (raw.githubusercontent.com)🤖 Prompt for AI Agents