Support ImageFromDockerfile authenticated image pulls (#2573)

rnorth · dbyron0 · rnorth · commit 54307ab0d27e · 2020-04-22T21:01:55.000+01:00
Co-Authored-By: David Byron &lt;dbyron@dbyron.com&gt;
diff --git 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'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);
     }
 
+    private void prePullDependencyImages(Set<String> 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<String, String> 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);
diff --git 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.
- *
+ * <p>
  * {@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,17 +83,77 @@ 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();
 
             assertTrue("container started following an authenticated pull", container.isRunning());
         }
     }
 
-    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();
     }
 }
