diff --git a/client/java-armeria/src/test/java/com/linecorp/centraldogma/client/armeria/ArmeriaCentralDogmaBuilderTest.java b/client/java-armeria/src/test/java/com/linecorp/centraldogma/client/armeria/ArmeriaCentralDogmaBuilderTest.java index 62dea5c0c..94c4523dd 100644 --- a/client/java-armeria/src/test/java/com/linecorp/centraldogma/client/armeria/ArmeriaCentralDogmaBuilderTest.java +++ b/client/java-armeria/src/test/java/com/linecorp/centraldogma/client/armeria/ArmeriaCentralDogmaBuilderTest.java @@ -100,6 +100,15 @@ void buildingWithSingleResolvedHost() throws Exception { assertThat(b.endpointGroup()).isEqualTo(Endpoint.of("1.2.3.4", 36462)); } + @Test + void buildingWithSingleResolvedHostWithTls() throws Exception { + final ArmeriaCentralDogmaBuilder b = new ArmeriaCentralDogmaBuilder(); + b.healthCheckIntervalMillis(0); + b.useTls(); + b.host("1.2.3.4"); + assertThat(b.endpointGroup()).isEqualTo(Endpoint.of("1.2.3.4", 443)); + } + @Test void buildingSingleResolvedHostWithHealthCheck() throws Exception { final ArmeriaCentralDogmaBuilder b = new ArmeriaCentralDogmaBuilder(); diff --git a/client/java/src/main/java/com/linecorp/centraldogma/client/AbstractCentralDogmaBuilder.java b/client/java/src/main/java/com/linecorp/centraldogma/client/AbstractCentralDogmaBuilder.java index 7489ca195..80bf4ee2e 100644 --- a/client/java/src/main/java/com/linecorp/centraldogma/client/AbstractCentralDogmaBuilder.java +++ b/client/java/src/main/java/com/linecorp/centraldogma/client/AbstractCentralDogmaBuilder.java @@ -59,6 +59,10 @@ public abstract class AbstractCentralDogmaBuilder= 1 && port < 65536, "port: %s (expected: 1 .. 65535)", port); + return host0(host, port); + } + + private B host0(String host, int port) { + requireNonNull(host, "host"); + checkArgument(!host.startsWith("group:"), "host: %s (must not start with 'group:')", host); final InetSocketAddress addr = newEndpoint(host, port); checkState(selectedProfile == null, "profile() and host() cannot be used together."); @@ -133,14 +143,16 @@ public final B host(String host, int port) { } /** - * Sets the client to use TLS. + * Sets the client to use TLS. A host added via {@link #host(String)} without a port number will use + * the default port number of {@value #DEFAULT_TLS_PORT}. */ public final B useTls() { return useTls(true); } /** - * Sets whether the client uses TLS or not. + * Sets whether the client uses TLS or not. If TLS is enabled, a host added via {@link #host(String)} + * without a port number will use the default port number of {@value #DEFAULT_TLS_PORT}. */ public final B useTls(boolean useTls) { checkState(selectedProfile == null, "useTls() cannot be called once a profile is selected."); @@ -337,9 +349,21 @@ protected final String selectedProfile() { /** * Returns the hosts added via {@link #host(String, int)} or {@link #profile(String...)}. + * A host added via {@link #host(String)} without a port number gets the default port number of + * {@value #DEFAULT_PORT}, or {@value #DEFAULT_TLS_PORT} if TLS is enabled with {@link #useTls()}. */ protected final Set hosts() { - return hosts; + final int defaultPort = useTls ? DEFAULT_TLS_PORT : DEFAULT_PORT; + final ImmutableSet.Builder builder = + ImmutableSet.builderWithExpectedSize(hosts.size()); + for (InetSocketAddress addr : hosts) { + if (addr.getPort() == UNSPECIFIED_PORT) { + builder.add(newEndpoint(addr.getHostString(), defaultPort)); + } else { + builder.add(addr); + } + } + return builder.build(); } /** diff --git a/client/java/src/test/java/com/linecorp/centraldogma/client/CentralDogmaBuilderTest.java b/client/java/src/test/java/com/linecorp/centraldogma/client/CentralDogmaBuilderTest.java index 8ae7d697f..db995d57e 100644 --- a/client/java/src/test/java/com/linecorp/centraldogma/client/CentralDogmaBuilderTest.java +++ b/client/java/src/test/java/com/linecorp/centraldogma/client/CentralDogmaBuilderTest.java @@ -125,6 +125,69 @@ void singleHost() { assertThat(b.hosts()).containsExactly(InetSocketAddress.createUnresolved("foo", 36462)); } + @Test + void tlsHostWithoutPort() { + // useTls() before host() + final CentralDogmaBuilder b1 = new CentralDogmaBuilder(); + b1.useTls(); + b1.host("foo"); + assertThat(b1.hosts()).containsExactly(InetSocketAddress.createUnresolved("foo", 443)); + + // useTls() after host() + final CentralDogmaBuilder b2 = new CentralDogmaBuilder(); + b2.host("foo"); + b2.useTls(); + assertThat(b2.hosts()).containsExactly(InetSocketAddress.createUnresolved("foo", 443)); + + // An IP address without a port number + final CentralDogmaBuilder b3 = new CentralDogmaBuilder(); + b3.host("192.168.0.1"); + b3.useTls(); + assertThat(b3.hosts()).containsExactly(new InetSocketAddress("192.168.0.1", 443)); + + // An IPv6 address without a port number + final CentralDogmaBuilder b4 = new CentralDogmaBuilder(); + b4.host("::1"); + b4.useTls(); + assertThat(b4.hosts()).containsExactly(new InetSocketAddress("::1", 443)); + + // useTls(false) keeps the default cleartext port. + final CentralDogmaBuilder b5 = new CentralDogmaBuilder(); + b5.host("foo"); + b5.useTls(false); + assertThat(b5.hosts()).containsExactly(InetSocketAddress.createUnresolved("foo", 36462)); + } + + @Test + void tlsHostWithExplicitPort() { + final CentralDogmaBuilder b = new CentralDogmaBuilder(); + b.host("foo", 36462); + b.useTls(); + assertThat(b.hosts()).containsExactly(InetSocketAddress.createUnresolved("foo", 36462)); + } + + @Test + void tlsHostDeduplication() { + // A host added with and without an explicit port collapses into one entry once resolved. + final CentralDogmaBuilder b = new CentralDogmaBuilder(); + b.host("foo"); + b.host("foo", 443); + b.useTls(); + assertThat(b.hosts()).containsExactly(InetSocketAddress.createUnresolved("foo", 443)); + } + + @Test + void uriWithoutPort() { + final CentralDogmaBuilder b1 = new CentralDogmaBuilder(); + b1.uri("tbinary+https://foo/cd/thrift/v1"); + b1.useTls(); + assertThat(b1.hosts()).containsExactly(InetSocketAddress.createUnresolved("foo", 443)); + + final CentralDogmaBuilder b2 = new CentralDogmaBuilder(); + b2.uri("tbinary+http://foo/cd/thrift/v1"); + assertThat(b2.hosts()).containsExactly(InetSocketAddress.createUnresolved("foo", 36462)); + } + @Test void multipleHosts() { final CentralDogmaBuilder b = new CentralDogmaBuilder(); diff --git a/site/src/sphinx/client-java.rst b/site/src/sphinx/client-java.rst index 1a1858e4b..6832b2ece 100644 --- a/site/src/sphinx/client-java.rst +++ b/site/src/sphinx/client-java.rst @@ -49,7 +49,8 @@ First, we should create a new instance of :api:`com.linecorp.centraldogma.client CentralDogma dogma = new ArmeriaCentralDogmaBuilder() .host("127.0.0.1") .build(); - // You can specify an alternative port or enable TLS as well: + // You can specify an alternative port or enable TLS as well. + // When TLS is enabled, the default port 443 is used if unspecified. CentralDogma dogma2 = new ArmeriaCentralDogmaBuilder() .useTls() // Enable TLS. .host("example.com", 8443) // Use port 8443.