Skip to content
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

Support ImageFromDockerfile authenticated image pulls #2573

Merged
merged 7 commits into from Apr 19, 2020
Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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 -> {
bsideup marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -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);
Expand Down
@@ -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;
Expand All @@ -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")
Expand All @@ -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()
Expand All @@ -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;
Comment on lines +135 to +140
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish we had text blocks!

I would have preferred to just point to a static file on disk, but unfortunately our dockerized private registry has a port number that can vary, and thus the image name changes.

Hence, constructing a tiny YAML file in a string seemed like a tolerable evil.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Expand All @@ -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<>())
Comment on lines -112 to +172
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deprecation duty.

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