Deprecate ambiguous constructors (#2839)

Our constructors for `GenericContainer` and all the classes that inherit from it tend to be a bit inconsistent. In some places, a no-args constructor is favoured, in other places a String parameter is favoured - which can be either a full image name or a tag.

This raises confusion, and in the case of an image tag being the parameter, it restricts which images people can easily use.

Another problem this raises is when the container class specifies a default tag version: this frequently falls out of date. We're stuck for options: some users (and image maintainers) would like to see newer image versions being used, but this could be a breaking change for existing users. This is especially risky when a major version bump is proposed.

This change aims to:

- Deprecate 'ambiguous' constructors for containers - any no-args or String-arg constructor gets this treatment.
- Introduce a new constructor which accepts a `DockerImageName` object. This class is preferred to a String because its meaning is unambiguous.
- Enhance `DockerImageName` with a static factory method and a convenience `withTag` method so that users can easily create and derive new instances.
- Update all modules to follow suit

Author: rnorth

core/src/main/java/org/testcontainers/DockerClientFactory.java

@@ -60,7 +60,7 @@ public class DockerClientFactory {
             TESTCONTAINERS_SESSION_ID_LABEL, SESSION_ID
     );
 
-    private static final String TINY_IMAGE = TestcontainersConfiguration.getInstance().getTinyImage();
+    private static final String TINY_IMAGE = TestcontainersConfiguration.getInstance().getTinyDockerImageName().asCanonicalNameString();
     private static DockerClientFactory instance;
 
     // Cached client configuration

core/src/main/java/org/testcontainers/containers/DockerComposeContainer.java

@@ -586,7 +586,7 @@ class ContainerisedDockerCompose extends GenericContainer<ContainerisedDockerCom
 
     public ContainerisedDockerCompose(List<File> composeFiles, String identifier) {
 
-        super(TestcontainersConfiguration.getInstance().getDockerComposeContainerImage());
+        super(TestcontainersConfiguration.getInstance().getDockerComposeDockerImageName());
         addEnv(ENV_PROJECT_NAME, identifier);
 
         // Map the docker compose file into the container

core/src/main/java/org/testcontainers/containers/FixedHostPortGenericContainer.java

@@ -12,6 +12,11 @@
  * not available - which could manifest as flaky or unstable tests.</p>
  */
 public class FixedHostPortGenericContainer<SELF extends FixedHostPortGenericContainer<SELF>> extends GenericContainer<SELF> {
+
+    /**
+     * @deprecated it is highly recommended that {@link FixedHostPortGenericContainer} not be used, as it risks port conflicts.
+     */
+    @Deprecated
     public FixedHostPortGenericContainer(@NotNull String dockerImageName) {
         super(dockerImageName);
     }

core/src/main/java/org/testcontainers/containers/GenericContainer.java

@@ -12,7 +12,6 @@
 import com.github.dockerjava.api.model.ContainerNetwork;
 import com.github.dockerjava.api.model.ExposedPort;
 import com.github.dockerjava.api.model.HostConfig;
-import com.github.dockerjava.api.model.Info;
 import com.github.dockerjava.api.model.Link;
 import com.github.dockerjava.api.model.PortBinding;
 import com.github.dockerjava.api.model.Volume;
@@ -55,6 +54,7 @@
 import org.testcontainers.lifecycle.TestDescription;
 import org.testcontainers.lifecycle.TestLifecycleAware;
 import org.testcontainers.utility.Base58;
+import org.testcontainers.utility.DockerImageName;
 import org.testcontainers.utility.DockerLoggerFactory;
 import org.testcontainers.utility.DockerMachineClient;
 import org.testcontainers.utility.MountableFile;
@@ -227,10 +227,27 @@ public class GenericContainer<SELF extends GenericContainer<SELF>>
     @Setter(AccessLevel.NONE)
     private boolean shouldBeReused = false;
 
+
+    public GenericContainer(@NonNull final DockerImageName dockerImageName) {
+        this.image = new RemoteDockerImage(dockerImageName);
+    }
+
+    public GenericContainer(@NonNull final RemoteDockerImage image) {
+        this.image = image;
+    }
+
+    /**
+     * @deprecated use {@link GenericContainer(DockerImageName)} instead
+     */
+    @Deprecated
     public GenericContainer() {
         this(TestcontainersConfiguration.getInstance().getTinyImage());
     }
 
+    /**
+     * @deprecated use {@link GenericContainer(DockerImageName)} instead
+     */
+    @Deprecated
     public GenericContainer(@NonNull final String dockerImageName) {
         this.setDockerImageName(dockerImageName);
     }

core/src/main/java/org/testcontainers/containers/PortForwardingContainer.java

@@ -19,7 +19,7 @@
 public enum PortForwardingContainer {
     INSTANCE;
 
-    private GenericContainer container;
+    private GenericContainer<?> container;
 
     private final Set<Entry<Integer, Integer>> exposedPorts = Collections.newSetFromMap(new ConcurrentHashMap<>());
 
@@ -29,7 +29,7 @@ public enum PortForwardingContainer {
     @SneakyThrows
     private Connection createSSHSession() {
         String password = UUID.randomUUID().toString();
-        container = new GenericContainer<>(TestcontainersConfiguration.getInstance().getSSHdImage())
+        container = new GenericContainer<>(TestcontainersConfiguration.getInstance().getSSHdDockerImageName())
             .withExposedPorts(22)
             .withEnv("PASSWORD", password)
             .withCommand(

core/src/main/java/org/testcontainers/containers/SocatContainer.java

@@ -1,6 +1,7 @@
 package org.testcontainers.containers;
 
 import org.testcontainers.utility.Base58;
+import org.testcontainers.utility.DockerImageName;
 import org.testcontainers.utility.TestcontainersConfiguration;
 
 import java.util.HashMap;
@@ -16,7 +17,11 @@ public class SocatContainer extends GenericContainer<SocatContainer> {
     private final Map<Integer, String> targets = new HashMap<>();
 
     public SocatContainer() {
-        super(TestcontainersConfiguration.getInstance().getSocatContainerImage());
+        this(TestcontainersConfiguration.getInstance().getSocatDockerImageName());
+    }
+
+    public SocatContainer(final DockerImageName dockerImageName) {
+        super(dockerImageName);
         withCreateContainerCmdModifier(it -> it.withEntrypoint("/bin/sh"));
         withCreateContainerCmdModifier(it -> it.withName("testcontainers-socat-" + Base58.randomString(8)));
     }
@@ -39,4 +44,4 @@ protected void configure() {
                         .collect(Collectors.joining(" & "))
         );
     }
-}
+}

core/src/main/java/org/testcontainers/containers/VncRecordingContainer.java

@@ -52,7 +52,7 @@ public VncRecordingContainer(@NonNull GenericContainer<?> targetContainer) {
      * Create a sidekick container and attach it to another container. The VNC output of that container will be recorded.
      */
     public VncRecordingContainer(@NonNull Network network, @NonNull String targetNetworkAlias) throws IllegalStateException {
-        super(TestcontainersConfiguration.getInstance().getVncRecordedContainerImage());
+        super(TestcontainersConfiguration.getInstance().getVncDockerImageName());
 
         this.targetNetworkAlias = targetNetworkAlias;
         withNetwork(network);

core/src/main/java/org/testcontainers/dockerclient/AuthDelegatingDockerClientConfig.java

@@ -40,7 +40,7 @@ public AuthConfig effectiveAuthConfig(String imageName) {
         }
 
         // try and obtain more accurate auth config using our resolution
-        final DockerImageName parsed = new DockerImageName(imageName);
+        final DockerImageName parsed = DockerImageName.parse(imageName);
         final AuthConfig effectiveAuthConfig = RegistryAuthLocator.instance()
             .lookupAuthConfig(parsed, fallbackAuthConfig);
 

core/src/main/java/org/testcontainers/images/AbstractImagePullPolicy.java

@@ -12,7 +12,7 @@ public abstract class AbstractImagePullPolicy implements ImagePullPolicy {
 
     @Override
     public boolean shouldPull(DockerImageName imageName) {
-        Logger logger = DockerLoggerFactory.getLogger(imageName.toString());
+        Logger logger = DockerLoggerFactory.getLogger(imageName.asCanonicalNameString());
 
         // Does our cache already know the image?
         ImageData cachedImageData = LOCAL_IMAGES_CACHE.get(imageName);

core/src/main/java/org/testcontainers/images/LocalImagesCache.java

@@ -39,7 +39,7 @@ public Optional<ImageData> refreshCache(DockerImageName imageName) {
 
             InspectImageResponse response = null;
             try {
-                response = dockerClient.inspectImageCmd(imageName.toString()).exec();
+                response = dockerClient.inspectImageCmd(imageName.asCanonicalNameString()).exec();
             } catch (NotFoundException e) {
                 log.trace("Image {} not found", imageName, e);
             }

core/src/main/java/org/testcontainers/images/RemoteDockerImage.java

@@ -38,12 +38,18 @@ public class RemoteDockerImage extends LazyFuture<String> {
     @ToString.Exclude
     private DockerClient dockerClient = DockerClientFactory.lazyClient();
 
+    public RemoteDockerImage(DockerImageName dockerImageName) {
+        this.imageNameFuture = CompletableFuture.completedFuture(dockerImageName);
+    }
+
+    @Deprecated
     public RemoteDockerImage(String dockerImageName) {
-        this.imageNameFuture = CompletableFuture.completedFuture(new DockerImageName(dockerImageName));
+        this.imageNameFuture = CompletableFuture.completedFuture(DockerImageName.parse(dockerImageName));
     }
 
+    @Deprecated
     public RemoteDockerImage(@NonNull String repository, @NonNull String tag) {
-        this.imageNameFuture = CompletableFuture.completedFuture(new DockerImageName(repository, tag));
+        this.imageNameFuture = CompletableFuture.completedFuture(DockerImageName.parse(repository).withTag(tag));
     }
 
     public RemoteDockerImage(@NonNull Future<String> imageFuture) {
@@ -57,7 +63,7 @@ protected final String resolve() {
         Logger logger = DockerLoggerFactory.getLogger(imageName.toString());
         try {
             if (!imagePullPolicy.shouldPull(imageName)) {
-                return imageName.toString();
+                return imageName.asCanonicalNameString();
             }
 
             // The image is not available locally - pull it
@@ -76,7 +82,7 @@ protected final String resolve() {
 
                     LocalImagesCache.INSTANCE.refreshCache(imageName);
 
-                    return imageName.toString();
+                    return imageName.asCanonicalNameString();
                 } catch (InterruptedException | InternalServerErrorException e) {
                     // these classes of exception often relate to timeout/connection errors so should be retried
                     lastFailure = e;
@@ -104,7 +110,7 @@ private String imageNameToString() {
         }
 
         try {
-            return getImageName().toString();
+            return getImageName().asCanonicalNameString();
         } catch (InterruptedException | ExecutionException e) {
             return e.getMessage();
         }

core/src/main/java/org/testcontainers/images/builder/dockerfile/traits/FromStatementTrait.java

@@ -6,7 +6,7 @@
 public interface FromStatementTrait<SELF extends FromStatementTrait<SELF> & DockerfileBuilderTrait<SELF>> {
 
     default SELF from(String dockerImageName) {
-        new DockerImageName(dockerImageName).assertValid();
+        DockerImageName.parse(dockerImageName).assertValid();
 
         return ((SELF) this).withStatement(new SingleArgumentStatement("FROM", dockerImageName));
     }

core/src/main/java/org/testcontainers/utility/DockerImageName.java

@@ -2,12 +2,16 @@
 
 
 import com.google.common.net.HostAndPort;
+import lombok.AccessLevel;
+import lombok.AllArgsConstructor;
 import lombok.Data;
 import lombok.EqualsAndHashCode;
+import org.jetbrains.annotations.NotNull;
 
 import java.util.regex.Pattern;
 
 @EqualsAndHashCode(exclude = "rawName")
+@AllArgsConstructor(access = AccessLevel.PRIVATE)
 public final class DockerImageName {
 
     /* Regex patterns used for validation */
@@ -19,22 +23,42 @@ public final class DockerImageName {
     private final String rawName;
     private final String registry;
     private final String repo;
-    private final Versioning versioning;
+    @NotNull private final Versioning versioning;
 
-    public DockerImageName(String name) {
-        this.rawName = name;
-        final int slashIndex = name.indexOf('/');
+    /**
+     * Parses a docker image name from a provided string.
+     *
+     * @param fullImageName in standard Docker format, e.g. <code>name:tag</code>,
+     *                      <code>some.registry/path/name:tag</code>,
+     *                      <code>some.registry/path/name@sha256:abcdef...</code>, etc.
+     */
+    public static DockerImageName parse(String fullImageName) {
+        return new DockerImageName(fullImageName);
+    }
+
+    /**
+     * Parses a docker image name from a provided string.
+     *
+     * @param fullImageName in standard Docker format, e.g. <code>name:tag</code>,
+     *                      <code>some.registry/path/name:tag</code>,
+     *                      <code>some.registry/path/name@sha256:abcdef...</code>, etc.
+     * @deprecated use {@link DockerImageName#parse(String)} instead
+     */
+    @Deprecated
+    public DockerImageName(String fullImageName) {
+        this.rawName = fullImageName;
+        final int slashIndex = fullImageName.indexOf('/');
 
         String remoteName;
         if (slashIndex == -1 ||
-            (!name.substring(0, slashIndex).contains(".") &&
-                !name.substring(0, slashIndex).contains(":") &&
-                !name.substring(0, slashIndex).equals("localhost"))) {
+            (!fullImageName.substring(0, slashIndex).contains(".") &&
+             !fullImageName.substring(0, slashIndex).contains(":") &&
+             !fullImageName.substring(0, slashIndex).equals("localhost"))) {
             registry = "";
-            remoteName = name;
+            remoteName = fullImageName;
         } else {
-            registry = name.substring(0, slashIndex);
-            remoteName = name.substring(slashIndex + 1);
+            registry = fullImageName.substring(0, slashIndex);
+            remoteName = fullImageName.substring(slashIndex + 1);
         }
 
         if (remoteName.contains("@sha256:")) {
@@ -49,28 +73,40 @@ public DockerImageName(String name) {
         }
     }
 
-    public DockerImageName(String name, String tag) {
-        this.rawName = name;
-        final int slashIndex = name.indexOf('/');
+    /**
+     * Parses a docker image name from a provided string, and uses a separate provided version.
+     *
+     * @param nameWithoutTag in standard Docker format, e.g. <code>name</code>,
+     *                       <code>some.registry/path/name</code>,
+     *                       <code>some.registry/path/name</code>, etc.
+     * @param version        a docker image version identifier, either as a tag or sha256 checksum, e.g.
+     *                       <code>tag</code>,
+     *                       <code>sha256:abcdef...</code>.
+     * @deprecated use {@link DockerImageName#parse(String)}.{@link DockerImageName#withTag(String)} instead
+     */
+    @Deprecated
+    public DockerImageName(String nameWithoutTag, @NotNull String version) {
+        this.rawName = nameWithoutTag;
+        final int slashIndex = nameWithoutTag.indexOf('/');
 
         String remoteName;
         if (slashIndex == -1 ||
-            (!name.substring(0, slashIndex).contains(".") &&
-                !name.substring(0, slashIndex).contains(":") &&
-                !name.substring(0, slashIndex).equals("localhost"))) {
+            (!nameWithoutTag.substring(0, slashIndex).contains(".") &&
+             !nameWithoutTag.substring(0, slashIndex).contains(":") &&
+             !nameWithoutTag.substring(0, slashIndex).equals("localhost"))) {
             registry = "";
-            remoteName = name;
+            remoteName = nameWithoutTag;
         } else {
-            registry = name.substring(0, slashIndex);
-            remoteName = name.substring(slashIndex + 1);
+            registry = nameWithoutTag.substring(0, slashIndex);
+            remoteName = nameWithoutTag.substring(slashIndex + 1);
         }
 
-        if (tag.startsWith("sha256:")) {
+        if (version.startsWith("sha256:")) {
             repo = remoteName;
-            versioning = new Sha256Versioning(tag.replace("sha256:", ""));
+            versioning = new Sha256Versioning(version.replace("sha256:", ""));
         } else {
             repo = remoteName;
-            versioning = new TagVersioning(tag);
+            versioning = new TagVersioning(version);
         }
     }
 
@@ -92,13 +128,16 @@ public String getVersionPart() {
         return versioning.toString();
     }
 
+    /**
+     * @return canonical name for the image
+     */
+    public String asCanonicalNameString() {
+        return getUnversionedPart() + versioning.getSeparator() + versioning.toString();
+    }
+
     @Override
     public String toString() {
-        if (versioning == null) {
-            return getUnversionedPart();
-        } else {
-            return getUnversionedPart() + versioning.getSeparator() + versioning.toString();
-        }
+        return asCanonicalNameString();
     }
 
     /**
@@ -111,10 +150,6 @@ public void assertValid() {
         if (!REPO_NAME.matcher(repo).matches()) {
             throw new IllegalArgumentException(repo + " is not a valid Docker image name (in " + rawName + ")");
         }
-        if (versioning == null) {
-            throw new IllegalArgumentException("No image tag was specified in docker image name " +
-                "(" + rawName + "). Please provide a tag; this may be 'latest' or a specific version");
-        }
         if (!versioning.isValid()) {
             throw new IllegalArgumentException(versioning + " is not a valid image versioning identifier (in " + rawName + ")");
         }
@@ -124,8 +159,13 @@ public String getRegistry() {
         return registry;
     }
 
+    public DockerImageName withTag(final String newTag) {
+        return new DockerImageName(rawName, registry, repo, new TagVersioning(newTag));
+    }
+
     private interface Versioning {
         boolean isValid();
+
         String getSeparator();
     }
 
@@ -155,7 +195,7 @@ public String toString() {
     }
 
     @Data
-    private class Sha256Versioning implements Versioning {
+    private static class Sha256Versioning implements Versioning {
         public static final String HASH_REGEX = "[0-9a-fA-F]{32,}";
         private final String hash;
 

core/src/main/java/org/testcontainers/utility/DockerMachineClient.java

@@ -1,7 +1,6 @@
 package org.testcontainers.utility;
 
 import lombok.NonNull;
-
 import org.apache.commons.lang.SystemUtils;
 import org.slf4j.Logger;