From 97bf5ef4aebb0c04b67238e034477b14e8cc2d95 Mon Sep 17 00:00:00 2001 From: Harald Albers Date: Mon, 27 Oct 2014 19:04:40 +0100 Subject: [PATCH 1/4] More intuitive way to add port bindings: new abstraction "PortBinding" The use of Ports for adding port bindings requires knowledge of the internal data structure used for storing port bindings in Docker API. This commit adds a more intuitive alternative that works like the --publish option of the Docker CLI and some builder methods like withBinds(Bind...) and withLinks(Link...). --- .../api/command/StartContainerCmd.java | 14 +++++ .../dockerjava/api/model/PortBinding.java | 36 ++++++++++++ .../github/dockerjava/api/model/Ports.java | 56 +++++++++++++++--- .../core/command/StartContainerCmdImpl.java | 11 ++++ ...tsTest.java => Ports_SerializingTest.java} | 2 +- .../api/model/Ports_addBindingsTest.java | 57 +++++++++++++++++++ 6 files changed, 168 insertions(+), 8 deletions(-) create mode 100644 src/main/java/com/github/dockerjava/api/model/PortBinding.java rename src/test/java/com/github/dockerjava/api/model/{PortsTest.java => Ports_SerializingTest.java} (94%) create mode 100644 src/test/java/com/github/dockerjava/api/model/Ports_addBindingsTest.java diff --git a/src/main/java/com/github/dockerjava/api/command/StartContainerCmd.java b/src/main/java/com/github/dockerjava/api/command/StartContainerCmd.java index ed16a727c..8535c8ed2 100644 --- a/src/main/java/com/github/dockerjava/api/command/StartContainerCmd.java +++ b/src/main/java/com/github/dockerjava/api/command/StartContainerCmd.java @@ -6,6 +6,7 @@ import com.github.dockerjava.api.model.Device; import com.github.dockerjava.api.model.Link; import com.github.dockerjava.api.model.LxcConf; +import com.github.dockerjava.api.model.PortBinding; import com.github.dockerjava.api.model.Ports; import com.github.dockerjava.api.model.RestartPolicy; @@ -53,8 +54,21 @@ public interface StartContainerCmd extends DockerCmd { public StartContainerCmd withLxcConf(LxcConf... lxcConf); + /** + * Add the port bindings that are contained in the given {@link Ports} + * object. + * + * @see #withPortBindings(PortBinding...) + */ public StartContainerCmd withPortBindings(Ports portBindings); + /** + * Add one or more {@link PortBinding}s. + * This corresponds to the --publish (-p) + * option of the docker run CLI command. + */ + public StartContainerCmd withPortBindings(PortBinding... portBindings); + public StartContainerCmd withPrivileged(boolean privileged); public StartContainerCmd withPublishAllPorts(boolean publishAllPorts); diff --git a/src/main/java/com/github/dockerjava/api/model/PortBinding.java b/src/main/java/com/github/dockerjava/api/model/PortBinding.java new file mode 100644 index 000000000..be4f5a291 --- /dev/null +++ b/src/main/java/com/github/dockerjava/api/model/PortBinding.java @@ -0,0 +1,36 @@ +package com.github.dockerjava.api.model; + +import com.github.dockerjava.api.command.InspectContainerResponse.HostConfig; +import com.github.dockerjava.api.command.InspectContainerResponse.NetworkSettings; +import com.github.dockerjava.api.model.Ports.Binding; + +/** + * In a {@link PortBinding}, a network socket on the Docker host, expressed + * as a {@link Binding}, is bound to an {@link ExposedPort} of a container. + * A {@link PortBinding} corresponds to the --publish + * (-p) option of the docker run (and similar) + * CLI command for adding port bindings to a container. + *

+ * Note: This is an abstraction used for creating new port bindings. + * It is not to be confused with the abstraction used for querying existing + * port bindings from a container configuration in + * {@link NetworkSettings#getPorts()} and {@link HostConfig#getPortBindings()}. + * In that context, a Map<ExposedPort, Binding[]> is used. + */ +public class PortBinding { + private final Binding binding; + private final ExposedPort exposedPort; + + public PortBinding(Binding binding, ExposedPort exposedPort) { + this.binding = binding; + this.exposedPort = exposedPort; + } + + public Binding getBinding() { + return binding; + } + + public ExposedPort getExposedPort() { + return exposedPort; + } +} diff --git a/src/main/java/com/github/dockerjava/api/model/Ports.java b/src/main/java/com/github/dockerjava/api/model/Ports.java index a14b31036..0aaf3b44f 100644 --- a/src/main/java/com/github/dockerjava/api/model/Ports.java +++ b/src/main/java/com/github/dockerjava/api/model/Ports.java @@ -28,6 +28,11 @@ /** * A container for port bindings, made available as a {@link Map} via its * {@link #getBindings()} method. + *

+ * Note: This is an abstraction used for querying existing port bindings from + * a container configuration. + * It is not to be confused with the {@link PortBinding} abstraction used for + * adding new port bindings to a container. * * @see HostConfig#getPortBindings() * @see NetworkSettings#getPorts() @@ -38,18 +43,42 @@ public class Ports { private final Map ports = new HashMap(); + /** + * Creates a {@link Ports} object with no {@link PortBinding}s. + * Use {@link #bind(ExposedPort, Binding)} or {@link #add(PortBinding...)} + * to add {@link PortBinding}s. + */ public Ports() { } + /** + * Creates a {@link Ports} object with an initial {@link PortBinding} for + * the specified {@link ExposedPort} and {@link Binding}. + * Use {@link #bind(ExposedPort, Binding)} or {@link #add(PortBinding...)} + * to add more {@link PortBinding}s. + */ public Ports(ExposedPort exposedPort, Binding host) { bind(exposedPort, host); } - public void bind(ExposedPort exposedPort, Binding host) { + /** + * Adds a new {@link PortBinding} for the specified {@link ExposedPort} and + * {@link Binding} to the current bindings. + */ + public void bind(ExposedPort exposedPort, Binding binding) { if (ports.containsKey(exposedPort)) { Binding[] bindings = ports.get(exposedPort); - ports.put(exposedPort, (Binding[]) ArrayUtils.add(bindings, host)); + ports.put(exposedPort, (Binding[]) ArrayUtils.add(bindings, binding)); } else { - ports.put(exposedPort, new Binding[]{host}); + ports.put(exposedPort, new Binding[]{binding}); + } + } + + /** + * Adds the specified {@link PortBinding}(s) to the list of {@link PortBinding}s. + */ + public void add(PortBinding... portBindings) { + for (PortBinding binding : portBindings) { + bind(binding.getExposedPort(), binding.getBinding()); } } @@ -59,6 +88,9 @@ public String toString(){ } /** + * Returns the port bindings in the format used by the Docker remote API, + * i.e. the {@link Binding}s grouped by {@link ExposedPort}. + * * @return the port bindings as a {@link Map} that contains one or more * {@link Binding}s per {@link ExposedPort}. */ @@ -75,10 +107,12 @@ public static Binding Binding(int hostPort) { /** - * The host part of a port binding. - * In a port binding a container port, expressed as an {@link ExposedPort}, - * is published as a port of the Docker host. + * A {@link Binding} represents a socket on the Docker host that is + * used in a {@link PortBinding}. + * It is characterized by an {@link #getHostIp() IP address} and a + * {@link #getHostPort() port number}. * + * @see Ports#bind(ExposedPort, Binding) * @see ExposedPort */ public static class Binding { @@ -88,7 +122,8 @@ public static class Binding { private final int hostPort; /** - * Creates the host part of a port binding. + * Creates a {@link Binding} for the given {@link #getHostIp() IP address} + * and {@link #getHostPort() port number}. * * @see Ports#bind(ExposedPort, Binding) * @see ExposedPort @@ -98,6 +133,13 @@ public Binding(String hostIp, int hostPort) { this.hostPort = hostPort; } + /** + * Creates a {@link Binding} for the given {@link #getHostPort() port number}, + * leaving the {@link #getHostIp() IP address} undefined. + * + * @see Ports#bind(ExposedPort, Binding) + * @see ExposedPort + */ public Binding(int hostPort) { this("", hostPort); } diff --git a/src/main/java/com/github/dockerjava/core/command/StartContainerCmdImpl.java b/src/main/java/com/github/dockerjava/core/command/StartContainerCmdImpl.java index 9cf830833..74fbd6790 100644 --- a/src/main/java/com/github/dockerjava/core/command/StartContainerCmdImpl.java +++ b/src/main/java/com/github/dockerjava/core/command/StartContainerCmdImpl.java @@ -13,6 +13,7 @@ import com.github.dockerjava.api.model.Link; import com.github.dockerjava.api.model.Links; import com.github.dockerjava.api.model.LxcConf; +import com.github.dockerjava.api.model.PortBinding; import com.github.dockerjava.api.model.Ports; import com.github.dockerjava.api.model.RestartPolicy; import com.google.common.base.Preconditions; @@ -179,6 +180,16 @@ public StartContainerCmd withPortBindings(Ports portBindings) { return this; } + @Override + public StartContainerCmd withPortBindings(PortBinding... portBindings) { + Preconditions.checkNotNull(portBindings, "portBindings was not specified"); + if (this.portBindings == null) { + this.portBindings = new Ports(); + } + this.portBindings.add(portBindings); + return this; + } + @Override public StartContainerCmd withPrivileged(boolean privileged) { this.privileged = privileged; diff --git a/src/test/java/com/github/dockerjava/api/model/PortsTest.java b/src/test/java/com/github/dockerjava/api/model/Ports_SerializingTest.java similarity index 94% rename from src/test/java/com/github/dockerjava/api/model/PortsTest.java rename to src/test/java/com/github/dockerjava/api/model/Ports_SerializingTest.java index 6a6c5e44e..d7ab23d2c 100644 --- a/src/test/java/com/github/dockerjava/api/model/PortsTest.java +++ b/src/test/java/com/github/dockerjava/api/model/Ports_SerializingTest.java @@ -9,7 +9,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.github.dockerjava.api.model.Ports.Binding; -public class PortsTest { +public class Ports_SerializingTest { private final ObjectMapper objectMapper = new ObjectMapper(); private final String jsonWithDoubleBindingForOnePort = "{\"80/tcp\":[{\"HostIp\":\"10.0.0.1\",\"HostPort\":\"80\"},{\"HostIp\":\"10.0.0.2\",\"HostPort\":\"80\"}]}"; diff --git a/src/test/java/com/github/dockerjava/api/model/Ports_addBindingsTest.java b/src/test/java/com/github/dockerjava/api/model/Ports_addBindingsTest.java new file mode 100644 index 000000000..23abbdd2c --- /dev/null +++ b/src/test/java/com/github/dockerjava/api/model/Ports_addBindingsTest.java @@ -0,0 +1,57 @@ +package com.github.dockerjava.api.model; + +import static org.testng.Assert.assertEquals; + +import java.util.Map; + +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import com.github.dockerjava.api.model.Ports.Binding; + +/** + * As there may be several {@link Binding}s per {@link ExposedPort}, + * it makes a difference if you add {@link PortBinding}s for the + * same or different {@link ExposedPort}s to {@link Ports}. + * This test verifies that the Map in {@link Ports} is populated + * correctly in both cases. + */ +public class Ports_addBindingsTest { + private static final ExposedPort TCP_80 = ExposedPort.tcp(80); + private static final ExposedPort TCP_90 = ExposedPort.tcp(90); + private static final Binding BINDING_8080 = Ports.Binding(8080); + private static final Binding BINDING_9090 = Ports.Binding(9090); + + private Ports ports; + + @BeforeMethod + public void setup() { + ports = new Ports(); + } + + @Test + public void addTwoBindingsForDifferentExposedPorts() { + ports.add( + new PortBinding(BINDING_8080, TCP_80), + new PortBinding(BINDING_9090, TCP_90)); + + Map bindings = ports.getBindings(); + // two keys with one value each + assertEquals(bindings.size(), 2); + assertEquals(bindings.get(TCP_80), new Binding[] { BINDING_8080 }); + assertEquals(bindings.get(TCP_90), new Binding[] { BINDING_9090 }); + } + + @Test + public void addTwoBindingsForSameExposedPort() { + ports.add( + new PortBinding(BINDING_8080, TCP_80), + new PortBinding(BINDING_9090, TCP_80)); + + Map bindings = ports.getBindings(); + // one key with two values + assertEquals(bindings.size(), 1); + assertEquals(bindings.get(TCP_80), new Binding[] { BINDING_8080, BINDING_9090 }); + } + +} From 3b14b1fa5cf35a72c150676c43ecceb196330875 Mon Sep 17 00:00:00 2001 From: Harald Albers Date: Fri, 31 Oct 2014 14:47:21 +0100 Subject: [PATCH 2/4] Allow port-only as a legal syntax for ExposedPort.parse(String) --- .../dockerjava/api/model/ExposedPort.java | 24 ++++++++++++++++--- .../dockerjava/api/model/ExposedPortTest.java | 12 ++++++++-- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/github/dockerjava/api/model/ExposedPort.java b/src/main/java/com/github/dockerjava/api/model/ExposedPort.java index d19bfbf05..30f7ce384 100644 --- a/src/main/java/com/github/dockerjava/api/model/ExposedPort.java +++ b/src/main/java/com/github/dockerjava/api/model/ExposedPort.java @@ -48,6 +48,16 @@ public ExposedPort(int port, InternetProtocol protocol) { this.protocol = protocol; } + /** + * Creates an {@link ExposedPort} for the given + * {@link #getPort() port number} and {@link InternetProtocol#DEFAULT}. + * + * @param port the {@link #getPort() port number} + */ + public ExposedPort(int port) { + this(port, InternetProtocol.DEFAULT); + } + /** * Creates an {@link ExposedPort} for the given parameters. * @@ -61,7 +71,8 @@ public ExposedPort(String scheme, int port) { this(port, InternetProtocol.valueOf(scheme)); } - /** @return the {@link InternetProtocol} */ + /** @return the {@link InternetProtocol} of the {@link #getPort() port} + * that the container exposes */ public InternetProtocol getProtocol() { return protocol; } @@ -75,7 +86,7 @@ public String getScheme() { return protocol.toString(); } - /** @return the port number */ + /** @return the port number that the container exposes */ public int getPort() { return port; } @@ -107,7 +118,14 @@ public static ExposedPort udp(int port) { public static ExposedPort parse(String serialized) throws IllegalArgumentException { try { String[] parts = serialized.split("/"); - return new ExposedPort(Integer.valueOf(parts[0]), InternetProtocol.parse(parts[1])); + switch (parts.length) { + case 1: + return new ExposedPort(Integer.valueOf(parts[0])); + case 2: + return new ExposedPort(Integer.valueOf(parts[0]), InternetProtocol.parse(parts[1])); + default: + throw new IllegalArgumentException(); + } } catch (Exception e) { throw new IllegalArgumentException("Error parsing ExposedPort '" + serialized + "'"); } diff --git a/src/test/java/com/github/dockerjava/api/model/ExposedPortTest.java b/src/test/java/com/github/dockerjava/api/model/ExposedPortTest.java index 052e44ff8..de1d23c21 100644 --- a/src/test/java/com/github/dockerjava/api/model/ExposedPortTest.java +++ b/src/test/java/com/github/dockerjava/api/model/ExposedPortTest.java @@ -1,5 +1,7 @@ package com.github.dockerjava.api.model; +import static com.github.dockerjava.api.model.InternetProtocol.DEFAULT; +import static com.github.dockerjava.api.model.InternetProtocol.TCP; import static org.testng.Assert.assertEquals; import org.testng.annotations.Test; @@ -7,9 +9,15 @@ public class ExposedPortTest { @Test - public void parse() { + public void parsePortAndProtocol() { ExposedPort exposedPort = ExposedPort.parse("80/tcp"); - assertEquals(exposedPort.getPort(), 80); + assertEquals(exposedPort, new ExposedPort(80, TCP)); + } + + @Test + public void parsePortOnly() { + ExposedPort exposedPort = ExposedPort.parse("80"); + assertEquals(exposedPort, new ExposedPort(80, DEFAULT)); } @Test(expectedExceptions = IllegalArgumentException.class, From 0ee67be7ac04bc50ce858e9525462e4a9bc5aa34 Mon Sep 17 00:00:00 2001 From: Harald Albers Date: Fri, 31 Oct 2014 14:44:52 +0100 Subject: [PATCH 3/4] Parser for PortBinding and Binding This allows you to use textual port binding specifications in the format used by the Docker CLI in docker-java: StartContainerCmd.withPortBindings(PortBinding.parse("80:8080/tcp")); --- .../dockerjava/api/model/PortBinding.java | 47 ++++++++++++++++++ .../github/dockerjava/api/model/Ports.java | 47 +++++++++++++++++- .../dockerjava/api/model/BindingTest.java | 43 ++++++++++++++++ .../dockerjava/api/model/PortBindingTest.java | 49 +++++++++++++++++++ 4 files changed, 184 insertions(+), 2 deletions(-) create mode 100644 src/test/java/com/github/dockerjava/api/model/BindingTest.java create mode 100644 src/test/java/com/github/dockerjava/api/model/PortBindingTest.java diff --git a/src/main/java/com/github/dockerjava/api/model/PortBinding.java b/src/main/java/com/github/dockerjava/api/model/PortBinding.java index be4f5a291..df9085597 100644 --- a/src/main/java/com/github/dockerjava/api/model/PortBinding.java +++ b/src/main/java/com/github/dockerjava/api/model/PortBinding.java @@ -1,5 +1,9 @@ package com.github.dockerjava.api.model; +import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang.builder.EqualsBuilder; +import org.apache.commons.lang.builder.HashCodeBuilder; + import com.github.dockerjava.api.command.InspectContainerResponse.HostConfig; import com.github.dockerjava.api.command.InspectContainerResponse.NetworkSettings; import com.github.dockerjava.api.model.Ports.Binding; @@ -33,4 +37,47 @@ public Binding getBinding() { public ExposedPort getExposedPort() { return exposedPort; } + + public static PortBinding parse(String serialized) throws IllegalArgumentException { + try { + String[] parts = StringUtils.splitByWholeSeparator(serialized, ":"); + switch (parts.length) { + case 3: + // 127.0.0.1:80:8080/tcp + return createFromSubstrings(parts[0] + ":" + parts[1], parts[2]); + case 2: + // 80:8080 // 127.0.0.1::8080 + return createFromSubstrings(parts[0], parts[1]); + case 1: + // 8080 + return createFromSubstrings("", parts[0]); + default: + throw new IllegalArgumentException(); + } + } catch (Exception e) { + throw new IllegalArgumentException("Error parsing PortBinding '" + + serialized + "'", e); + } + } + + private static PortBinding createFromSubstrings(String binding, String exposedPort) + throws IllegalArgumentException { + return new PortBinding(Binding.parse(binding), ExposedPort.parse(exposedPort)); + } + + @Override + public boolean equals(Object obj) { + if (obj instanceof PortBinding) { + PortBinding other = (PortBinding) obj; + return new EqualsBuilder().append(binding, other.getBinding()) + .append(exposedPort, other.getExposedPort()).isEquals(); + } else + return super.equals(obj); + } + + @Override + public int hashCode() { + return new HashCodeBuilder().append(binding).append(exposedPort).toHashCode(); + } + } diff --git a/src/main/java/com/github/dockerjava/api/model/Ports.java b/src/main/java/com/github/dockerjava/api/model/Ports.java index 0aaf3b44f..f483ba9d1 100644 --- a/src/main/java/com/github/dockerjava/api/model/Ports.java +++ b/src/main/java/com/github/dockerjava/api/model/Ports.java @@ -1,5 +1,7 @@ package com.github.dockerjava.api.model; +import static org.apache.commons.lang.StringUtils.isEmpty; + import java.io.IOException; import java.util.HashMap; import java.util.Iterator; @@ -8,7 +10,6 @@ import org.apache.commons.lang.ArrayUtils; import org.apache.commons.lang.builder.EqualsBuilder; -import org.apache.commons.lang.builder.ToStringBuilder; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonParser; @@ -152,9 +153,51 @@ public int getHostPort() { return hostPort; } + /** + * Parses a textual host and port specification (as used by the Docker CLI) + * to a {@link Binding}. + *

+ * Legal syntax: [IP:]Port + * + * @param serialized serialized the specification, e.g. + * 127.0.0.1:80 + * @return a {@link Binding} matching the specification + * @throws IllegalArgumentException if the specification cannot be parsed + */ + public static Binding parse(String serialized) throws IllegalArgumentException { + try { + String[] parts = serialized.split(":"); + switch (parts.length) { + case 2: { + return new Binding(parts[0], Integer.valueOf(parts[1])); + } + case 1: { + return new Binding(Integer.valueOf(parts[0])); + } + default: { + throw new IllegalArgumentException(); + } + } + } catch (Exception e) { + throw new IllegalArgumentException("Error parsing Binding '" + + serialized + "'"); + } + } + + /** + * Returns a string representation of this {@link Binding} suitable + * for inclusion in a JSON message. + * The format is [IP:]Port, like the argument in {@link #parse(String)}. + * + * @return a string representation of this {@link Binding} + */ @Override public String toString() { - return ToStringBuilder.reflectionToString(this); + if (isEmpty(hostIp)) { + return Integer.toString(hostPort); + } else { + return hostIp + ":" + hostPort; + } } @Override diff --git a/src/test/java/com/github/dockerjava/api/model/BindingTest.java b/src/test/java/com/github/dockerjava/api/model/BindingTest.java new file mode 100644 index 000000000..bfe581534 --- /dev/null +++ b/src/test/java/com/github/dockerjava/api/model/BindingTest.java @@ -0,0 +1,43 @@ +package com.github.dockerjava.api.model; + +import static org.testng.Assert.assertEquals; + +import org.testng.annotations.Test; + +import com.github.dockerjava.api.model.Ports.Binding; + +public class BindingTest { + + @Test + public void parseIpAndPort() { + assertEquals(Binding.parse("127.0.0.1:80"), Ports.Binding("127.0.0.1", 80)); + } + + @Test + public void parsePortOnly() { + assertEquals(Binding.parse("80"), Ports.Binding("", 80)); + } + + @Test(expectedExceptions = IllegalArgumentException.class, + expectedExceptionsMessageRegExp = "Error parsing Binding 'nonsense'") + public void parseInvalidInput() { + Binding.parse("nonsense"); + } + + @Test(expectedExceptions = IllegalArgumentException.class, + expectedExceptionsMessageRegExp = "Error parsing Binding 'null'") + public void parseNull() { + Binding.parse(null); + } + + @Test + public void toStringIpAndHost() { + assertEquals(Binding.parse("127.0.0.1:80").toString(), "127.0.0.1:80"); + } + + @Test + public void toStringPortOnly() { + assertEquals(Binding.parse("80").toString(), "80"); + } + +} diff --git a/src/test/java/com/github/dockerjava/api/model/PortBindingTest.java b/src/test/java/com/github/dockerjava/api/model/PortBindingTest.java new file mode 100644 index 000000000..21644b2c3 --- /dev/null +++ b/src/test/java/com/github/dockerjava/api/model/PortBindingTest.java @@ -0,0 +1,49 @@ +package com.github.dockerjava.api.model; + +import static org.testng.Assert.assertEquals; + +import org.testng.annotations.Test; + +import com.github.dockerjava.api.model.Ports.Binding; + +public class PortBindingTest { + + private static final ExposedPort TCP_8080 = ExposedPort.tcp(8080); + + @Test + public void fullDefinition() { + assertEquals(PortBinding.parse("127.0.0.1:80:8080/tcp"), + new PortBinding(new Binding("127.0.0.1", 80), TCP_8080)); + } + + @Test + public void noProtocol() { + assertEquals(PortBinding.parse("127.0.0.1:80:8080"), + new PortBinding(new Binding("127.0.0.1", 80), TCP_8080)); + } + + @Test + public void noHostIp() { + assertEquals(PortBinding.parse("80:8080/tcp"), + new PortBinding(new Binding(80), TCP_8080)); + } + + @Test + public void portsOnly() { + assertEquals(PortBinding.parse("80:8080"), + new PortBinding(new Binding(80), TCP_8080)); + } + + @Test(expectedExceptions = IllegalArgumentException.class, + expectedExceptionsMessageRegExp = "Error parsing PortBinding 'nonsense'") + public void parseInvalidInput() { + PortBinding.parse("nonsense"); + } + + @Test(expectedExceptions = IllegalArgumentException.class, + expectedExceptionsMessageRegExp = "Error parsing PortBinding 'null'") + public void parseNull() { + PortBinding.parse(null); + } + +} From 29643a46305a4f9308c469419fda39a516dc062d Mon Sep 17 00:00:00 2001 From: Harald Albers Date: Fri, 31 Oct 2014 13:29:36 +0100 Subject: [PATCH 4/4] Binding: Port and IP may be null In Docker API, an undefined IP address is expressed as "". This is not intuitive and should be hidden from the user. Likewise, docker-java uses 0 for an unspecified port number. This also is not intuitive, especially if you consider that 0 is a legal port number. This change makes the undefinedness explicit by using null in both cases. When serializing, "" is used instead of null. This implies changing the type of hostPort from int to Integer. --- .../github/dockerjava/api/model/Ports.java | 67 +++++++++--- .../dockerjava/api/model/BindingTest.java | 101 ++++++++++-------- .../dockerjava/api/model/PortBindingTest.java | 12 +++ .../api/model/Ports_SerializingTest.java | 5 + 4 files changed, 130 insertions(+), 55 deletions(-) diff --git a/src/main/java/com/github/dockerjava/api/model/Ports.java b/src/main/java/com/github/dockerjava/api/model/Ports.java index f483ba9d1..d68175a5a 100644 --- a/src/main/java/com/github/dockerjava/api/model/Ports.java +++ b/src/main/java/com/github/dockerjava/api/model/Ports.java @@ -99,10 +99,18 @@ public Map getBindings(){ return ports; } - public static Binding Binding(String hostIp, int hostPort) { + /** + * Creates a {@link Binding} for the given IP address and port number. + */ + public static Binding Binding(String hostIp, Integer hostPort) { return new Binding(hostIp, hostPort); } - public static Binding Binding(int hostPort) { + + /** + * Creates a {@link Binding} for the given port number, leaving the + * IP address undefined. + */ + public static Binding Binding(Integer hostPort) { return new Binding(hostPort); } @@ -112,6 +120,8 @@ public static Binding Binding(int hostPort) { * used in a {@link PortBinding}. * It is characterized by an {@link #getHostIp() IP address} and a * {@link #getHostPort() port number}. + * Both properties may be null in order to let Docker assign + * them dynamically/using defaults. * * @see Ports#bind(ExposedPort, Binding) * @see ExposedPort @@ -120,7 +130,7 @@ public static class Binding { private final String hostIp; - private final int hostPort; + private final Integer hostPort; /** * Creates a {@link Binding} for the given {@link #getHostIp() IP address} @@ -129,8 +139,8 @@ public static class Binding { * @see Ports#bind(ExposedPort, Binding) * @see ExposedPort */ - public Binding(String hostIp, int hostPort) { - this.hostIp = hostIp; + public Binding(String hostIp, Integer hostPort) { + this.hostIp = isEmpty(hostIp) ? null : hostIp; this.hostPort = hostPort; } @@ -141,15 +151,41 @@ public Binding(String hostIp, int hostPort) { * @see Ports#bind(ExposedPort, Binding) * @see ExposedPort */ - public Binding(int hostPort) { - this("", hostPort); + public Binding(Integer hostPort) { + this(null, hostPort); } + /** + * Creates a {@link Binding} for the given {@link #getHostIp() IP address}, + * leaving the {@link #getHostPort() port number} undefined. + */ + public Binding(String hostIp) { + this(hostIp, null); + } + + /** + * Creates a {@link Binding} with both {@link #getHostIp() IP address} and + * {@link #getHostPort() port number} undefined. + */ + public Binding() { + this(null, null); + } + + /** + * @return the IP address on the Docker host. + * May be null, in which case Docker will bind the + * port to all interfaces (0.0.0.0). + */ public String getHostIp() { return hostIp; } - public int getHostPort() { + /** + * @return the port number on the Docker host. + * May be null, in which case Docker will dynamically + * assign a port. + */ + public Integer getHostPort() { return hostPort; } @@ -157,7 +193,7 @@ public int getHostPort() { * Parses a textual host and port specification (as used by the Docker CLI) * to a {@link Binding}. *

- * Legal syntax: [IP:]Port + * Legal syntax: IP|IP:port|port * * @param serialized serialized the specification, e.g. * 127.0.0.1:80 @@ -166,13 +202,18 @@ public int getHostPort() { */ public static Binding parse(String serialized) throws IllegalArgumentException { try { + if (serialized.isEmpty()) { + return new Binding(); + } + String[] parts = serialized.split(":"); switch (parts.length) { case 2: { return new Binding(parts[0], Integer.valueOf(parts[1])); } case 1: { - return new Binding(Integer.valueOf(parts[0])); + return parts[0].contains(".") ? new Binding(parts[0]) + : new Binding(Integer.valueOf(parts[0])); } default: { throw new IllegalArgumentException(); @@ -195,6 +236,8 @@ public static Binding parse(String serialized) throws IllegalArgumentException { public String toString() { if (isEmpty(hostIp)) { return Integer.toString(hostPort); + } else if (hostPort == null) { + return hostIp; } else { return hostIp + ":" + hostPort; } @@ -249,8 +292,8 @@ public void serialize(Ports portBindings, JsonGenerator jsonGen, jsonGen.writeStartArray(); for (Binding binding : entry.getValue()) { jsonGen.writeStartObject(); - jsonGen.writeStringField("HostIp", binding.getHostIp()); - jsonGen.writeStringField("HostPort", "" + binding.getHostPort()); + jsonGen.writeStringField("HostIp", binding.getHostIp() == null ? "" : binding.getHostIp()); + jsonGen.writeStringField("HostPort", binding.getHostPort() == null ? "" : binding.getHostPort().toString()); jsonGen.writeEndObject(); } jsonGen.writeEndArray(); diff --git a/src/test/java/com/github/dockerjava/api/model/BindingTest.java b/src/test/java/com/github/dockerjava/api/model/BindingTest.java index bfe581534..bd8488660 100644 --- a/src/test/java/com/github/dockerjava/api/model/BindingTest.java +++ b/src/test/java/com/github/dockerjava/api/model/BindingTest.java @@ -1,43 +1,58 @@ -package com.github.dockerjava.api.model; - -import static org.testng.Assert.assertEquals; - -import org.testng.annotations.Test; - -import com.github.dockerjava.api.model.Ports.Binding; - -public class BindingTest { - - @Test - public void parseIpAndPort() { - assertEquals(Binding.parse("127.0.0.1:80"), Ports.Binding("127.0.0.1", 80)); - } - - @Test - public void parsePortOnly() { - assertEquals(Binding.parse("80"), Ports.Binding("", 80)); - } - - @Test(expectedExceptions = IllegalArgumentException.class, - expectedExceptionsMessageRegExp = "Error parsing Binding 'nonsense'") - public void parseInvalidInput() { - Binding.parse("nonsense"); - } - - @Test(expectedExceptions = IllegalArgumentException.class, - expectedExceptionsMessageRegExp = "Error parsing Binding 'null'") - public void parseNull() { - Binding.parse(null); - } - - @Test - public void toStringIpAndHost() { - assertEquals(Binding.parse("127.0.0.1:80").toString(), "127.0.0.1:80"); - } - - @Test - public void toStringPortOnly() { - assertEquals(Binding.parse("80").toString(), "80"); - } - -} +package com.github.dockerjava.api.model; + +import static org.testng.Assert.assertEquals; + +import org.testng.annotations.Test; + +import com.github.dockerjava.api.model.Ports.Binding; + +public class BindingTest { + + @Test + public void parseIpAndPort() { + assertEquals(Binding.parse("127.0.0.1:80"), Ports.Binding("127.0.0.1", 80)); + } + + @Test + public void parsePortOnly() { + assertEquals(Binding.parse("80"), Ports.Binding(null, 80)); + } + + @Test + public void parseIPOnly() { + assertEquals(Binding.parse("127.0.0.1"), Ports.Binding("127.0.0.1", null)); + } + + @Test + public void parseEmptyString() { + assertEquals(Binding.parse(""), Ports.Binding(null, null)); + } + + @Test(expectedExceptions = IllegalArgumentException.class, + expectedExceptionsMessageRegExp = "Error parsing Binding 'nonsense'") + public void parseInvalidInput() { + Binding.parse("nonsense"); + } + + @Test(expectedExceptions = IllegalArgumentException.class, + expectedExceptionsMessageRegExp = "Error parsing Binding 'null'") + public void parseNull() { + Binding.parse(null); + } + + @Test + public void toStringIpAndHost() { + assertEquals(Binding.parse("127.0.0.1:80").toString(), "127.0.0.1:80"); + } + + @Test + public void toStringPortOnly() { + assertEquals(Binding.parse("80").toString(), "80"); + } + + @Test + public void toStringIpOnly() { + assertEquals(Binding.parse("127.0.0.1").toString(), "127.0.0.1"); + } + +} diff --git a/src/test/java/com/github/dockerjava/api/model/PortBindingTest.java b/src/test/java/com/github/dockerjava/api/model/PortBindingTest.java index 21644b2c3..2aeb768f6 100644 --- a/src/test/java/com/github/dockerjava/api/model/PortBindingTest.java +++ b/src/test/java/com/github/dockerjava/api/model/PortBindingTest.java @@ -34,6 +34,18 @@ public void portsOnly() { new PortBinding(new Binding(80), TCP_8080)); } + @Test + public void exposedPortOnly() { + assertEquals(PortBinding.parse("8080"), + new PortBinding(new Binding(), TCP_8080)); + } + + @Test + public void dynamicHostPort() { + assertEquals(PortBinding.parse("127.0.0.1::8080"), + new PortBinding(new Binding("127.0.0.1"), TCP_8080)); + } + @Test(expectedExceptions = IllegalArgumentException.class, expectedExceptionsMessageRegExp = "Error parsing PortBinding 'nonsense'") public void parseInvalidInput() { diff --git a/src/test/java/com/github/dockerjava/api/model/Ports_SerializingTest.java b/src/test/java/com/github/dockerjava/api/model/Ports_SerializingTest.java index d7ab23d2c..d295c881e 100644 --- a/src/test/java/com/github/dockerjava/api/model/Ports_SerializingTest.java +++ b/src/test/java/com/github/dockerjava/api/model/Ports_SerializingTest.java @@ -34,4 +34,9 @@ public void serializingPortWithMultipleBindings() throws Exception { assertEquals(objectMapper.writeValueAsString(ports), jsonWithDoubleBindingForOnePort); } + @Test + public void serializingEmptyBinding() throws Exception { + Ports ports = new Ports(ExposedPort.tcp(80), new Binding(null, null)); + assertEquals(objectMapper.writeValueAsString(ports), "{\"80/tcp\":[{\"HostIp\":\"\",\"HostPort\":\"\"}]}"); + } }