Skip to content

Commit 4cb555a

Browse files
rnorthrnorth
committed
Revert "Support ImageFromDockerfile authenticated image pulls (#2573)"
f631023
1 parent 2c2e207 commit 4cb555a

File tree

2 files changed

+40
-112
lines changed

2 files changed

+40
-112
lines changed

‎core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java‎

Lines changed: 17 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,16 @@ protected final String resolve() {
8383

8484
DockerClient dockerClient = DockerClientFactory.instance().client();
8585

86+
dependencyImageNames.forEach(imageName -> {
87+
try {
88+
log.info("Pre-emptively checking local images for '{}', referenced via a Dockerfile. If not available, it will be pulled.", imageName);
89+
DockerClientFactory.instance().checkAndPullImage(dockerClient, imageName);
90+
} catch (Exception e) {
91+
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());
92+
}
93+
});
94+
95+
8696
try {
8797
if (deleteOnExit) {
8898
ResourceReaper.instance().registerImageForCleanup(dockerImageName);
@@ -108,8 +118,6 @@ public void onNext(BuildResponseItem item) {
108118
BuildImageCmd buildImageCmd = dockerClient.buildImageCmd(in);
109119
configure(buildImageCmd);
110120

111-
prePullDependencyImages(dependencyImageNames);
112-
113121
BuildImageResultCallback exec = buildImageCmd.exec(resultCallback);
114122

115123
long bytesToDockerDaemon = 0;
@@ -146,30 +154,11 @@ protected void configure(BuildImageCmd buildImageCmd) {
146154
this.dockerfile.ifPresent(p -> {
147155
buildImageCmd.withDockerfile(p.toFile());
148156
dependencyImageNames = new ParsedDockerfile(p).getDependencyImageNames();
149-
150-
if (dependencyImageNames.size() > 0) {
151-
// if we'll be pre-pulling images, disable the built-in pull as it is not necessary and will fail for
152-
// authenticated registries
153-
buildImageCmd.withPull(false);
154-
}
155157
});
156158

157159
this.buildArgs.forEach(buildImageCmd::withBuildArg);
158160
}
159161

160-
private void prePullDependencyImages(Set<String> imagesToPull) {
161-
final DockerClient dockerClient = DockerClientFactory.instance().client();
162-
163-
imagesToPull.forEach(imageName -> {
164-
try {
165-
log.info("Pre-emptively checking local images for '{}', referenced via a Dockerfile. If not available, it will be pulled.", imageName);
166-
DockerClientFactory.instance().checkAndPullImage(dockerClient, imageName);
167-
} catch (Exception e) {
168-
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());
169-
}
170-
});
171-
}
172-
173162
public ImageFromDockerfile withBuildArg(final String key, final String value) {
174163
this.buildArgs.put(key, value);
175164
return this;
@@ -182,23 +171,19 @@ public ImageFromDockerfile withBuildArgs(final Map<String, String> args) {
182171

183172
/**
184173
* Sets the Dockerfile to be used for this image.
185-
*
186-
* @param relativePathFromBuildContextDirectory relative path to the Dockerfile, relative to the image build context directory
187-
* @deprecated It is recommended to use {@link #withDockerfile} instead because it honors .dockerignore files and
188-
* will therefore be more efficient. Additionally, using {@link #withDockerfile} supports Dockerfiles that depend
189-
* upon images from authenticated private registries.
174+
* @deprecated It is recommended to use {@link #withDockerfile} instead because it honors
175+
* .dockerignore files and therefore will be more efficient
176+
* @param relativePathFromBuildRoot
190177
*/
191178
@Deprecated
192-
public ImageFromDockerfile withDockerfilePath(String relativePathFromBuildContextDirectory) {
193-
this.dockerFilePath = Optional.of(relativePathFromBuildContextDirectory);
179+
public ImageFromDockerfile withDockerfilePath(String relativePathFromBuildRoot) {
180+
this.dockerFilePath = Optional.of(relativePathFromBuildRoot);
194181
return this;
195182
}
196183

197184
/**
198-
* Sets the Dockerfile to be used for this image. Honors .dockerignore files for efficiency.
199-
* Additionally, supports Dockerfiles that depend upon images from authenticated private registries.
200-
*
201-
* @param dockerfile path to Dockerfile on the test host.
185+
* Sets the Dockerfile to be used for this image.
186+
* @param dockerfile
202187
*/
203188
public ImageFromDockerfile withDockerfile(Path dockerfile) {
204189
this.dockerfile = Optional.of(dockerfile);

‎core/src/test/java/org/testcontainers/utility/AuthenticatedImagePullTest.java‎

Lines changed: 23 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,19 @@
11
package org.testcontainers.utility;
22

33
import com.github.dockerjava.api.DockerClient;
4-
import com.github.dockerjava.api.async.ResultCallback;
5-
import com.github.dockerjava.api.command.PullImageResultCallback;
64
import com.github.dockerjava.api.model.AuthConfig;
7-
import org.intellij.lang.annotations.Language;
5+
import com.github.dockerjava.core.command.PullImageResultCallback;
6+
import com.github.dockerjava.core.command.PushImageResultCallback;
87
import org.junit.AfterClass;
9-
import org.junit.Before;
108
import org.junit.BeforeClass;
119
import org.junit.ClassRule;
1210
import org.junit.Test;
1311
import org.mockito.Mockito;
1412
import org.testcontainers.DockerClientFactory;
15-
import org.testcontainers.containers.ContainerState;
16-
import org.testcontainers.containers.DockerComposeContainer;
1713
import org.testcontainers.containers.GenericContainer;
1814
import org.testcontainers.containers.wait.strategy.HttpWaitStrategy;
1915
import org.testcontainers.images.builder.ImageFromDockerfile;
2016

21-
import java.io.IOException;
22-
import java.nio.file.Files;
23-
import java.nio.file.Path;
24-
import java.nio.file.Paths;
2517
import java.util.concurrent.TimeUnit;
2618

2719
import static org.mockito.ArgumentMatchers.any;
@@ -33,17 +25,14 @@
3325
* This test checks the integration between Testcontainers and an authenticated registry, but uses
3426
* a mock instance of {@link RegistryAuthLocator} - the purpose of the test is solely to ensure that
3527
* the auth locator is utilised, and that the credentials it provides flow through to the registry.
36-
* <p>
28+
*
3729
* {@link RegistryAuthLocatorTest} covers actual credential scenarios at a lower level, which are
3830
* impractical to test end-to-end.
3931
*/
4032
public class AuthenticatedImagePullTest {
4133

42-
/**
43-
* Containerised docker image registry, with simple hardcoded credentials
44-
*/
4534
@ClassRule
46-
public static GenericContainer<?> authenticatedRegistry = new GenericContainer<>(new ImageFromDockerfile()
35+
public static GenericContainer authenticatedRegistry = new GenericContainer(new ImageFromDockerfile()
4736
.withDockerfileFromBuilder(builder -> {
4837
builder.from("registry:2")
4938
.run("htpasswd -Bbn testuser notasecret > /htpasswd")
@@ -57,17 +46,28 @@ public class AuthenticatedImagePullTest {
5746
private static RegistryAuthLocator originalAuthLocatorSingleton;
5847
private static DockerClient client;
5948

49+
private static String testRegistryAddress;
6050
private static String testImageName;
6151
private static String testImageNameWithTag;
6252

6353
@BeforeClass
64-
public static void setUp() throws InterruptedException {
54+
public static void setUp() {
6555
originalAuthLocatorSingleton = RegistryAuthLocator.instance();
6656
client = DockerClientFactory.instance().client();
6757

68-
String testRegistryAddress = authenticatedRegistry.getContainerIpAddress() + ":" + authenticatedRegistry.getFirstMappedPort();
58+
testRegistryAddress = authenticatedRegistry.getContainerIpAddress() + ":" + authenticatedRegistry.getFirstMappedPort();
6959
testImageName = testRegistryAddress + "/alpine";
7060
testImageNameWithTag = testImageName + ":latest";
61+
}
62+
63+
@AfterClass
64+
public static void tearDown() {
65+
RegistryAuthLocator.setInstance(originalAuthLocatorSingleton);
66+
client.removeImageCmd(testImageNameWithTag).withForce(true).exec();
67+
}
68+
69+
@Test
70+
public void testThatAuthLocatorIsUsed() throws Exception {
7171

7272
final DockerImageName expectedName = new DockerImageName(testImageNameWithTag);
7373
final AuthConfig authConfig = new AuthConfig()
@@ -83,77 +83,17 @@ public static void setUp() throws InterruptedException {
8383

8484
// a push will use the auth locator for authentication, although that isn't the goal of this test
8585
putImageInRegistry();
86-
}
87-
88-
@Before
89-
public void removeImageFromLocalDocker() {
90-
// remove the image tag from local docker so that it must be pulled before use
91-
client.removeImageCmd(testImageNameWithTag).withForce(true).exec();
92-
}
9386

94-
@AfterClass
95-
public static void tearDown() {
96-
RegistryAuthLocator.setInstance(originalAuthLocatorSingleton);
97-
}
98-
99-
@Test
100-
public void testThatAuthLocatorIsUsedForContainerCreation() {
10187
// actually start a container, which will require an authenticated pull
102-
try (final GenericContainer<?> container = new GenericContainer<>(testImageNameWithTag)
88+
try (final GenericContainer container = new GenericContainer<>(testImageNameWithTag)
10389
.withCommand("/bin/sh", "-c", "sleep 10")) {
10490
container.start();
10591

10692
assertTrue("container started following an authenticated pull", container.isRunning());
10793
}
10894
}
10995

110-
@Test
111-
public void testThatAuthLocatorIsUsedForDockerfileBuild() throws IOException {
112-
// Prepare a simple temporary Dockerfile which requires our custom private image
113-
Path tempContext = Files.createTempDirectory(Paths.get("."), this.getClass().getSimpleName() + "-test-");
114-
Path tempFile = Files.createTempFile(tempContext, "test", ".Dockerfile");
115-
String dockerFileContent = "FROM " + testImageNameWithTag;
116-
Files.write(tempFile, dockerFileContent.getBytes());
117-
118-
// Start a container built from a derived image, which will require an authenticated pull
119-
try (final GenericContainer<?> container = new GenericContainer<>(
120-
new ImageFromDockerfile()
121-
.withDockerfile(tempFile)
122-
)
123-
.withCommand("/bin/sh", "-c", "sleep 10")) {
124-
container.start();
125-
126-
assertTrue("container started following an authenticated pull", container.isRunning());
127-
}
128-
}
129-
130-
@Test
131-
public void testThatAuthLocatorIsUsedForDockerComposePull() throws IOException {
132-
// Prepare a simple temporary Docker Compose manifest which requires our custom private image
133-
Path tempContext = Files.createTempDirectory(Paths.get("."), this.getClass().getSimpleName() + "-test-");
134-
Path tempFile = Files.createTempFile(tempContext, "test", ".docker-compose.yml");
135-
@Language("yaml") String composeFileContent =
136-
"version: '2.0'\n" +
137-
"services:\n" +
138-
" privateservice:\n" +
139-
" command: /bin/sh -c 'sleep 60'\n" +
140-
" image: " + testImageNameWithTag;
141-
Files.write(tempFile, composeFileContent.getBytes());
142-
143-
// Start the docker compose project, which will require an authenticated pull
144-
try (final DockerComposeContainer<?> compose = new DockerComposeContainer<>(tempFile.toFile())) {
145-
compose.start();
146-
147-
assertTrue("container started following an authenticated pull",
148-
compose
149-
.getContainerByServiceName("privateservice_1")
150-
.map(ContainerState::isRunning)
151-
.orElse(false)
152-
);
153-
}
154-
}
155-
156-
private static void putImageInRegistry() throws InterruptedException {
96+
private void putImageInRegistry() throws InterruptedException {
15797
// It doesn't matter which image we use for this test, but use one that's likely to have been pulled already
15898
final String dummySourceImage = TestcontainersConfiguration.getInstance().getRyukImage();
15999

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

171111
client.pushImageCmd(testImageNameWithTag)
172-
.exec(new ResultCallback.Adapter<>())
112+
.exec(new PushImageResultCallback())
173113
.awaitCompletion(1, TimeUnit.MINUTES);
114+
115+
// remove the image tag from local docker so that it must be pulled before use
116+
client.removeImageCmd(testImageNameWithTag).withForce(true).exec();
174117
}
175118
}

0 commit comments

Comments
 (0)