From 50f21e67b7d71945b3f3abfd16e1d31b53f15ded Mon Sep 17 00:00:00 2001 From: Richard North Date: Tue, 14 Apr 2020 10:12:43 +0100 Subject: [PATCH 1/3] Support ImageFromDockerfile authenticated image pulls --- .../images/builder/ImageFromDockerfile.java | 49 ++++++++++++------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java b/core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java index 1140247960a..48746cc8387 100644 --- a/core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java +++ b/core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java @@ -83,16 +83,6 @@ protected final String resolve() { DockerClient dockerClient = DockerClientFactory.instance().client(); - dependencyImageNames.forEach(imageName -> { - try { - log.info("Pre-emptively checking local images for '{}', referenced via a Dockerfile. If not available, it will be pulled.", imageName); - DockerClientFactory.instance().checkAndPullImage(dockerClient, imageName); - } catch (Exception e) { - log.warn("Unable to pre-fetch an image ({}) depended upon by Dockerfile - image build will continue but may fail. Exception message was: {}", imageName, e.getMessage()); - } - }); - - try { if (deleteOnExit) { ResourceReaper.instance().registerImageForCleanup(dockerImageName); @@ -118,6 +108,8 @@ public void onNext(BuildResponseItem item) { BuildImageCmd buildImageCmd = dockerClient.buildImageCmd(in); configure(buildImageCmd); + prePullDependencyImages(dependencyImageNames); + BuildImageResultCallback exec = buildImageCmd.exec(resultCallback); long bytesToDockerDaemon = 0; @@ -154,11 +146,30 @@ protected void configure(BuildImageCmd buildImageCmd) { this.dockerfile.ifPresent(p -> { buildImageCmd.withDockerfile(p.toFile()); dependencyImageNames = new ParsedDockerfile(p).getDependencyImageNames(); + + if (dependencyImageNames.size() > 0) { + // if we'l 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); } + private void prePullDependencyImages(Set imagesToPull) { + final DockerClient dockerClient = DockerClientFactory.instance().client(); + + imagesToPull.forEach(imageName -> { + try { + log.info("Pre-emptively checking local images for '{}', referenced via a Dockerfile. If not available, it will be pulled.", imageName); + DockerClientFactory.instance().checkAndPullImage(dockerClient, imageName); + } catch (Exception e) { + log.warn("Unable to pre-fetch an image ({}) depended upon by Dockerfile - image build will continue but may fail. Exception message was: {}", imageName, e.getMessage()); + } + }); + } + public ImageFromDockerfile withBuildArg(final String key, final String value) { this.buildArgs.put(key, value); return this; @@ -171,19 +182,23 @@ public ImageFromDockerfile withBuildArgs(final Map args) { /** * Sets the Dockerfile to be used for this image. - * @deprecated It is recommended to use {@link #withDockerfile} instead because it honors - * .dockerignore files and therefore will be more efficient - * @param relativePathFromBuildRoot + * + * @param relativePathFromBuildContextDirectory relative path to the Dockerfile, relative to the image build context directory + * @deprecated It is recommended to use {@link #withDockerfile} instead because it honors .dockerignore files and + * will therefore be more efficient. Additionally, using {@link #withDockerfile} supports Dockerfiles that depend + * upon images from authenticated private registries. */ @Deprecated - public ImageFromDockerfile withDockerfilePath(String relativePathFromBuildRoot) { - this.dockerFilePath = Optional.of(relativePathFromBuildRoot); + public ImageFromDockerfile withDockerfilePath(String relativePathFromBuildContextDirectory) { + this.dockerFilePath = Optional.of(relativePathFromBuildContextDirectory); return this; } /** - * Sets the Dockerfile to be used for this image. - * @param dockerfile + * Sets the Dockerfile to be used for this image. Honors .dockerignore files for efficiency. + * Additionally, supports Dockerfiles that depend upon images from authenticated private registries. + * + * @param dockerfile path to Dockerfile on the test host. */ public ImageFromDockerfile withDockerfile(Path dockerfile) { this.dockerfile = Optional.of(dockerfile); From 8f536def44edc1b776a0b6c344d21da61d943bc9 Mon Sep 17 00:00:00 2001 From: Richard North Date: Tue, 14 Apr 2020 21:03:44 +0100 Subject: [PATCH 2/3] Add integration tests for authenticated pulls for ImageFromDockerfile and Docker Compose --- .../utility/AuthenticatedImagePullTest.java | 103 ++++++++++++++---- 1 file changed, 80 insertions(+), 23 deletions(-) diff --git a/core/src/test/java/org/testcontainers/utility/AuthenticatedImagePullTest.java b/core/src/test/java/org/testcontainers/utility/AuthenticatedImagePullTest.java index 78b54798cd1..56945643a88 100644 --- a/core/src/test/java/org/testcontainers/utility/AuthenticatedImagePullTest.java +++ b/core/src/test/java/org/testcontainers/utility/AuthenticatedImagePullTest.java @@ -1,19 +1,27 @@ package org.testcontainers.utility; import com.github.dockerjava.api.DockerClient; +import com.github.dockerjava.api.async.ResultCallback; +import com.github.dockerjava.api.command.PullImageResultCallback; import com.github.dockerjava.api.model.AuthConfig; -import com.github.dockerjava.core.command.PullImageResultCallback; -import com.github.dockerjava.core.command.PushImageResultCallback; +import org.intellij.lang.annotations.Language; import org.junit.AfterClass; +import org.junit.Before; import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; import org.mockito.Mockito; import org.testcontainers.DockerClientFactory; +import org.testcontainers.containers.ContainerState; +import org.testcontainers.containers.DockerComposeContainer; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.wait.strategy.HttpWaitStrategy; import org.testcontainers.images.builder.ImageFromDockerfile; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.concurrent.TimeUnit; import static org.mockito.ArgumentMatchers.any; @@ -25,14 +33,17 @@ * This test checks the integration between Testcontainers and an authenticated registry, but uses * a mock instance of {@link RegistryAuthLocator} - the purpose of the test is solely to ensure that * the auth locator is utilised, and that the credentials it provides flow through to the registry. - * + *

* {@link RegistryAuthLocatorTest} covers actual credential scenarios at a lower level, which are * impractical to test end-to-end. */ public class AuthenticatedImagePullTest { + /** + * Containerised docker image registry, with simple hardcoded credentials + */ @ClassRule - public static GenericContainer authenticatedRegistry = new GenericContainer(new ImageFromDockerfile() + public static GenericContainer authenticatedRegistry = new GenericContainer<>(new ImageFromDockerfile() .withDockerfileFromBuilder(builder -> { builder.from("registry:2") .run("htpasswd -Bbn testuser notasecret > /htpasswd") @@ -46,28 +57,17 @@ public class AuthenticatedImagePullTest { private static RegistryAuthLocator originalAuthLocatorSingleton; private static DockerClient client; - private static String testRegistryAddress; private static String testImageName; private static String testImageNameWithTag; @BeforeClass - public static void setUp() { + public static void setUp() throws InterruptedException { originalAuthLocatorSingleton = RegistryAuthLocator.instance(); client = DockerClientFactory.instance().client(); - testRegistryAddress = authenticatedRegistry.getContainerIpAddress() + ":" + authenticatedRegistry.getFirstMappedPort(); + String testRegistryAddress = authenticatedRegistry.getContainerIpAddress() + ":" + authenticatedRegistry.getFirstMappedPort(); testImageName = testRegistryAddress + "/alpine"; testImageNameWithTag = testImageName + ":latest"; - } - - @AfterClass - public static void tearDown() { - RegistryAuthLocator.setInstance(originalAuthLocatorSingleton); - client.removeImageCmd(testImageNameWithTag).withForce(true).exec(); - } - - @Test - public void testThatAuthLocatorIsUsed() throws Exception { final DockerImageName expectedName = new DockerImageName(testImageNameWithTag); final AuthConfig authConfig = new AuthConfig() @@ -83,9 +83,23 @@ public void testThatAuthLocatorIsUsed() throws Exception { // a push will use the auth locator for authentication, although that isn't the goal of this test putImageInRegistry(); + } + + @Before + public void removeImageFromLocalDocker() { + // remove the image tag from local docker so that it must be pulled before use + client.removeImageCmd(testImageNameWithTag).withForce(true).exec(); + } + @AfterClass + public static void tearDown() { + RegistryAuthLocator.setInstance(originalAuthLocatorSingleton); + } + + @Test + public void testThatAuthLocatorIsUsedForContainerCreation() { // actually start a container, which will require an authenticated pull - try (final GenericContainer container = new GenericContainer<>(testImageNameWithTag) + try (final GenericContainer container = new GenericContainer<>(testImageNameWithTag) .withCommand("/bin/sh", "-c", "sleep 10")) { container.start(); @@ -93,7 +107,53 @@ public void testThatAuthLocatorIsUsed() throws Exception { } } - private void putImageInRegistry() throws InterruptedException { + @Test + public void testThatAuthLocatorIsUsedForDockerfileBuild() throws IOException { + // Prepare a simple temporary Dockerfile which requires our custom private image + Path tempContext = Files.createTempDirectory(Paths.get("."), this.getClass().getSimpleName() + "-test-"); + Path tempFile = Files.createTempFile(tempContext, "test", ".Dockerfile"); + String dockerFileContent = "FROM " + testImageNameWithTag; + Files.write(tempFile, dockerFileContent.getBytes()); + + // Start a container built from a derived image, which will require an authenticated pull + try (final GenericContainer container = new GenericContainer<>( + new ImageFromDockerfile() + .withDockerfile(tempFile) + ) + .withCommand("/bin/sh", "-c", "sleep 10")) { + container.start(); + + assertTrue("container started following an authenticated pull", container.isRunning()); + } + } + + @Test + public void testThatAuthLocatorIsUsedForDockerComposePull() throws IOException { + // Prepare a simple temporary Docker Compose manifest which requires our custom private image + Path tempContext = Files.createTempDirectory(Paths.get("."), this.getClass().getSimpleName() + "-test-"); + Path tempFile = Files.createTempFile(tempContext, "test", ".docker-compose.yml"); + @Language("yaml") String composeFileContent = + "version: '2.0'\n" + + "services:\n" + + " privateservice:\n" + + " command: /bin/sh -c 'sleep 60'\n" + + " image: " + testImageNameWithTag; + Files.write(tempFile, composeFileContent.getBytes()); + + // Start the docker compose project, which will require an authenticated pull + try (final DockerComposeContainer compose = new DockerComposeContainer<>(tempFile.toFile())) { + compose.start(); + + assertTrue("container started following an authenticated pull", + compose + .getContainerByServiceName("privateservice_1") + .map(ContainerState::isRunning) + .orElse(false) + ); + } + } + + private static void putImageInRegistry() throws InterruptedException { // It doesn't matter which image we use for this test, but use one that's likely to have been pulled already final String dummySourceImage = TestcontainersConfiguration.getInstance().getRyukImage(); @@ -109,10 +169,7 @@ private void putImageInRegistry() throws InterruptedException { client.tagImageCmd(id, testImageName, "latest").exec(); client.pushImageCmd(testImageNameWithTag) - .exec(new PushImageResultCallback()) + .exec(new ResultCallback.Adapter<>()) .awaitCompletion(1, TimeUnit.MINUTES); - - // remove the image tag from local docker so that it must be pulled before use - client.removeImageCmd(testImageNameWithTag).withForce(true).exec(); } } From b7fcf2236b4cf3fc695908396f23d8975dd70907 Mon Sep 17 00:00:00 2001 From: Richard North Date: Tue, 14 Apr 2020 21:05:44 +0100 Subject: [PATCH 3/3] Update core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java Co-Authored-By: David Byron --- .../org/testcontainers/images/builder/ImageFromDockerfile.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java b/core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java index 48746cc8387..11a7e2f4cb6 100644 --- a/core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java +++ b/core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java @@ -148,7 +148,7 @@ protected void configure(BuildImageCmd buildImageCmd) { dependencyImageNames = new ParsedDockerfile(p).getDependencyImageNames(); if (dependencyImageNames.size() > 0) { - // if we'l be pre-pulling images, disable the built-in pull as it is not necessary and will fail for + // 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); }