Only publish exposed ports (#4122)

testcontainers · May 26, 2021 · 96b2065 · 96b2065
1 parent 3167adc
commit 96b2065
Show file tree

Hide file tree

Showing 7 changed files with 147 additions and 59 deletions.
diff --git a/core/src/main/java/org/testcontainers/containers/GenericContainer.java b/core/src/main/java/org/testcontainers/containers/GenericContainer.java
@@ -1,8 +1,5 @@
 package org.testcontainers.containers;
 
-import static com.google.common.collect.Lists.newArrayList;
-import static org.testcontainers.utility.CommandLine.runShellCommand;
-
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.MapperFeature;
 import com.fasterxml.jackson.databind.SerializationFeature;
@@ -16,46 +13,14 @@
 import com.github.dockerjava.api.model.HostConfig;
 import com.github.dockerjava.api.model.Link;
 import com.github.dockerjava.api.model.PortBinding;
+import com.github.dockerjava.api.model.Ports;
 import com.github.dockerjava.api.model.Volume;
 import com.github.dockerjava.api.model.VolumesFrom;
 import com.github.dockerjava.core.DefaultDockerClientConfig;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.hash.Hashing;
-import java.io.File;
-import java.io.IOException;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
-import java.lang.reflect.UndeclaredThrowableException;
-import java.nio.charset.Charset;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.time.Duration;
-import java.time.Instant;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
-import java.util.Optional;
-import java.util.Set;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Future;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicInteger;
-import java.util.function.Consumer;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
-import java.util.zip.Adler32;
-import java.util.zip.Checksum;
 import lombok.AccessLevel;
 import lombok.Data;
 import lombok.NonNull;
@@ -97,6 +62,43 @@
 import org.testcontainers.utility.ResourceReaper;
 import org.testcontainers.utility.TestcontainersConfiguration;
 
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.UndeclaredThrowableException;
+import java.nio.charset.Charset;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Consumer;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import java.util.zip.Adler32;
+import java.util.zip.Checksum;
+
+import static com.google.common.collect.Lists.newArrayList;
+import static org.testcontainers.utility.CommandLine.runShellCommand;
+
 /**
  * Base class for that allows a container to be launched and controlled.
  */
@@ -718,21 +720,28 @@ public Set<Integer> getLivenessCheckPortNumbers() {
 
     private void applyConfiguration(CreateContainerCmd createCommand) {
         HostConfig hostConfig = buildHostConfig();
-        createCommand.withHostConfig(hostConfig);
-
-        // Set up exposed ports (where there are no host port bindings defined)
-        ExposedPort[] portArray = exposedPorts.stream()
-                .map(ExposedPort::new)
-                .toArray(ExposedPort[]::new);
 
-        createCommand.withExposedPorts(portArray);
+        // PortBindings must contain:
+        //  * all exposed ports with a randomized host port (equivalent to -p CONTAINER_PORT)
+        //  * all exposed ports with a fixed host port (equivalent to -p HOST_PORT:CONTAINER_PORT)
+        Map<ExposedPort, PortBinding> allPortBindings = new HashMap<>();
+        // First collect all the randomized host ports from our 'exposedPorts' field
+        for (final Integer tcpPort : exposedPorts) {
+            ExposedPort exposedPort = ExposedPort.tcp(tcpPort);
+            allPortBindings.put(exposedPort, new PortBinding(Ports.Binding.empty(), exposedPort));
+        }
+        // Next collect all the fixed host ports from our 'portBindings' field, overwriting any randomized ports so that
+        // we don't create two bindings for the same container port.
+        for (final String portBinding : portBindings) {
+            PortBinding parsedBinding = PortBinding.parse(portBinding);
+            allPortBindings.put(parsedBinding.getExposedPort(), parsedBinding);
+        }
+        hostConfig.withPortBindings(new ArrayList<>(allPortBindings.values()));
 
-        // Set up exposed ports that need host port bindings
-        PortBinding[] portBindingArray = portBindings.stream()
-                .map(PortBinding::parse)
-                .toArray(PortBinding[]::new);
+        // Next, ExposedPorts must be set up to publish all of the above ports, both randomized and fixed.
+        createCommand.withExposedPorts(new ArrayList<>(allPortBindings.keySet()));
 
-        createCommand.withPortBindings(portBindingArray);
+        createCommand.withHostConfig(hostConfig);
 
         if (commandParts != null) {
             createCommand.withCmd(commandParts);
@@ -798,8 +807,6 @@ private void applyConfiguration(CreateContainerCmd createCommand) {
             createCommand.withNetworkMode(networkForLinks.get());
         }
 
-        createCommand.withPublishAllPorts(true);
-
         PortForwardingContainer.INSTANCE.getNetwork().ifPresent(it -> {
             withExtraHost(INTERNAL_HOST_HOSTNAME, it.getIpAddress());
         });

diff --git a/core/src/test/java/org/testcontainers/containers/GenericContainerTest.java b/core/src/test/java/org/testcontainers/containers/GenericContainerTest.java
@@ -1,8 +1,12 @@
 package org.testcontainers.containers;
 
 import com.github.dockerjava.api.DockerClient;
+import com.github.dockerjava.api.command.InspectContainerResponse;
 import com.github.dockerjava.api.command.InspectContainerResponse.ContainerState;
+import com.github.dockerjava.api.model.ExposedPort;
 import com.github.dockerjava.api.model.Info;
+import com.github.dockerjava.api.model.Ports;
+import com.google.common.primitives.Ints;
 import lombok.RequiredArgsConstructor;
 import lombok.SneakyThrows;
 import lombok.experimental.FieldDefaults;
@@ -11,15 +15,23 @@
 import org.assertj.core.api.Assumptions;
 import org.junit.Test;
 import org.rnorth.ducttape.unreliables.Unreliables;
-import org.testcontainers.TestImages;
 import org.testcontainers.DockerClientFactory;
+import org.testcontainers.TestImages;
 import org.testcontainers.containers.startupcheck.StartupCheckStrategy;
 import org.testcontainers.containers.wait.strategy.AbstractWaitStrategy;
+import org.testcontainers.images.builder.ImageFromDockerfile;
 
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals;
+import static org.rnorth.visibleassertions.VisibleAssertions.assertThrows;
+import static org.rnorth.visibleassertions.VisibleAssertions.assertTrue;
 
 public class GenericContainerTest {
 
@@ -60,6 +72,53 @@ public void shouldReportErrorAfterWait() {
         }
     }
 
+    @Test
+    public void shouldOnlyPublishExposedPorts() {
+        ImageFromDockerfile image = new ImageFromDockerfile("publish-multiple")
+            .withDockerfileFromBuilder(builder ->
+                builder
+                    .from("testcontainers/helloworld:1.1.0")
+                    .expose(8080, 8081)
+                    .build()
+            );
+        try (GenericContainer<?> container = new GenericContainer<>(image).withExposedPorts(8080)) {
+            container.start();
+
+            InspectContainerResponse inspectedContainer = container.getContainerInfo();
+
+            List<Integer> exposedPorts = Arrays.stream(inspectedContainer.getConfig().getExposedPorts())
+                .map(ExposedPort::getPort)
+                .collect(Collectors.toList());
+
+            assertEquals(
+                "the exposed ports are all of those EXPOSEd by the image",
+                Ints.asList(8080, 8081),
+                exposedPorts
+            );
+
+            Map<ExposedPort, Ports.Binding[]> hostBindings = inspectedContainer.getHostConfig().getPortBindings().getBindings();
+            assertEquals(
+                "only 1 port is bound on the host (published)",
+                1,
+                hostBindings.size()
+            );
+
+            Integer mappedPort = container.getMappedPort(8080);
+            assertTrue(
+                "port 8080 is bound to a different port on the host",
+                mappedPort != 8080
+            );
+
+            assertThrows(
+                "trying to get a non-bound port mapping fails",
+                IllegalArgumentException.class,
+                () -> {
+                    container.getMappedPort(8081);
+                }
+            );
+        }
+    }
+
     static class NoopStartupCheckStrategy extends StartupCheckStrategy {
 
         @Override

diff --git a/modules/clickhouse/src/main/java/org/testcontainers/containers/ClickHouseContainer.java b/modules/clickhouse/src/main/java/org/testcontainers/containers/ClickHouseContainer.java
@@ -1,9 +1,11 @@
 package org.testcontainers.containers;
 
+import com.google.common.collect.Sets;
 import org.testcontainers.containers.wait.strategy.HttpWaitStrategy;
 import org.testcontainers.utility.DockerImageName;
 
 import java.time.Duration;
+import java.util.Set;
 
 public class ClickHouseContainer extends JdbcDatabaseContainer {
     public static final String NAME = "clickhouse";
@@ -54,8 +56,8 @@ public ClickHouseContainer(final DockerImageName dockerImageName) {
     }
 
     @Override
-    protected Integer getLivenessCheckPort() {
-        return getMappedPort(HTTP_PORT);
+    public Set<Integer> getLivenessCheckPortNumbers() {
+        return Sets.newHashSet(HTTP_PORT);
     }
 
     @Override

diff --git a/modules/couchbase/src/main/java/org/testcontainers/couchbase/CouchbaseContainer.java b/modules/couchbase/src/main/java/org/testcontainers/couchbase/CouchbaseContainer.java
@@ -166,6 +166,19 @@ public String getConnectionString() {
     protected void configure() {
         super.configure();
 
+        addExposedPorts(
+            MGMT_PORT,
+            MGMT_SSL_PORT,
+            VIEW_PORT,
+            VIEW_SSL_PORT,
+            QUERY_PORT,
+            QUERY_SSL_PORT,
+            SEARCH_PORT,
+            SEARCH_SSL_PORT,
+            KV_PORT,
+            KV_SSL_PORT
+        );
+
         WaitAllStrategy waitStrategy = new WaitAllStrategy();
 
         // Makes sure that all nodes in the cluster are healthy.

diff --git a/modules/mariadb/src/main/java/org/testcontainers/containers/MariaDBContainer.java b/modules/mariadb/src/main/java/org/testcontainers/containers/MariaDBContainer.java
@@ -1,7 +1,10 @@
 package org.testcontainers.containers;
 
+import com.google.common.collect.Sets;
 import org.testcontainers.utility.DockerImageName;
 
+import java.util.Set;
+
 /**
  * Container implementation for the MariaDB project.
  *
@@ -51,8 +54,8 @@ public MariaDBContainer(final DockerImageName dockerImageName) {
     }
 
     @Override
-    protected Integer getLivenessCheckPort() {
-        return getMappedPort(MARIADB_PORT);
+    public Set<Integer> getLivenessCheckPortNumbers() {
+        return Sets.newHashSet(MARIADB_PORT);
     }
 
     @Override

diff --git a/modules/mssqlserver/src/main/java/org/testcontainers/containers/MSSQLServerContainer.java b/modules/mssqlserver/src/main/java/org/testcontainers/containers/MSSQLServerContainer.java
@@ -1,8 +1,10 @@
 package org.testcontainers.containers;
 
+import com.google.common.collect.Sets;
 import org.testcontainers.utility.DockerImageName;
 import org.testcontainers.utility.LicenseAcceptance;
 
+import java.util.Set;
 import java.util.regex.Pattern;
 import java.util.stream.Stream;
 
@@ -60,8 +62,8 @@ public MSSQLServerContainer(final DockerImageName dockerImageName) {
     }
 
     @Override
-    protected Integer getLivenessCheckPort() {
-        return getMappedPort(MS_SQL_SERVER_PORT);
+    public Set<Integer> getLivenessCheckPortNumbers() {
+        return Sets.newHashSet(MS_SQL_SERVER_PORT);
     }
 
     @Override

diff --git a/modules/oracle-xe/src/main/java/org/testcontainers/containers/OracleContainer.java b/modules/oracle-xe/src/main/java/org/testcontainers/containers/OracleContainer.java
@@ -1,8 +1,10 @@
 package org.testcontainers.containers;
 
+import com.google.common.collect.Sets;
 import org.testcontainers.utility.DockerImageName;
 import org.testcontainers.utility.TestcontainersConfiguration;
 
+import java.util.Set;
 import java.util.concurrent.Future;
 
 /**
@@ -61,8 +63,8 @@ private void preconfigure() {
     }
 
     @Override
-    protected Integer getLivenessCheckPort() {
-        return getMappedPort(ORACLE_PORT);
+    public Set<Integer> getLivenessCheckPortNumbers() {
+        return Sets.newHashSet(ORACLE_PORT);
     }
 
     @Override