Skip to content

Commit

Permalink
Revert "Support ImageFromDockerfile authenticated image pulls (#2573)"
Browse files Browse the repository at this point in the history 
f631023
  • Loading branch information
@rnorth
rnorth committed Apr 22, 2020
1 parent 2c2e207 commit 4cb555a
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 112 deletions.
49 changes: 17 additions & 32 deletions core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java
View file
Expand Up @@ -83,6 +83,16 @@ 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);
Expand All @@ -108,8 +118,6 @@ public void onNext(BuildResponseItem item) {
BuildImageCmd buildImageCmd = dockerClient.buildImageCmd(in);
configure(buildImageCmd);

prePullDependencyImages(dependencyImageNames);

BuildImageResultCallback exec = buildImageCmd.exec(resultCallback);

long bytesToDockerDaemon = 0;
Expand Down Expand Up @@ -146,30 +154,11 @@ 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;
Expand All @@ -182,23 +171,19 @@ public ImageFromDockerfile withBuildArgs(final Map<String, String> args) {

/**
* Sets the Dockerfile to be used for this image.
*
* @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 It is recommended to use {@link #withDockerfile} instead because it honors
* .dockerignore files and therefore will be more efficient
* @param relativePathFromBuildRoot
*/
@Deprecated
public ImageFromDockerfile withDockerfilePath(String relativePathFromBuildContextDirectory) {
this.dockerFilePath = Optional.of(relativePathFromBuildContextDirectory);
public ImageFromDockerfile withDockerfilePath(String relativePathFromBuildRoot) {
this.dockerFilePath = Optional.of(relativePathFromBuildRoot);
return this;
}

/**
* 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.
* Sets the Dockerfile to be used for this image.
* @param dockerfile
*/
public ImageFromDockerfile withDockerfile(Path dockerfile) {
this.dockerfile = Optional.of(dockerfile);
Expand Down
103 changes: 23 additions & 80 deletions core/src/test/java/org/testcontainers/utility/AuthenticatedImagePullTest.java
View file
@@ -1,27 +1,19 @@
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 org.intellij.lang.annotations.Language;
import com.github.dockerjava.core.command.PullImageResultCallback;
import com.github.dockerjava.core.command.PushImageResultCallback;
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;
Expand All @@ -33,17 +25,14 @@
* 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")
Expand All @@ -57,17 +46,28 @@ 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() throws InterruptedException {
public static void setUp() {
originalAuthLocatorSingleton = RegistryAuthLocator.instance();
client = DockerClientFactory.instance().client();

String testRegistryAddress = authenticatedRegistry.getContainerIpAddress() + ":" + authenticatedRegistry.getFirstMappedPort();
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()
Expand All @@ -83,77 +83,17 @@ public static void setUp() throws InterruptedException {

// 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());
}
}

@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 {
private 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();

Expand All @@ -169,7 +109,10 @@ private static void putImageInRegistry() throws InterruptedException {
client.tagImageCmd(id, testImageName, "latest").exec();

client.pushImageCmd(testImageNameWithTag)
.exec(new ResultCallback.Adapter<>())
.exec(new PushImageResultCallback())
.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();
}
}

0 comments on commit 4cb555a

Please sign in to comment.