From 51041b398fa67efeb9cc193d6707b7ba20f43df3 Mon Sep 17 00:00:00 2001 From: Nikita_Glashchenko Date: Fri, 26 Apr 2019 16:51:07 +0400 Subject: [PATCH 1/8] Reject port ranges in `discovery.seed_hosts`. Fix wrong value of LOCAL_PORTS_COUNT --- .../azure/classic/AzureSeedHostsProvider.java | 3 +- .../ec2/AwsEc2SeedHostsProvider.java | 3 +- .../discovery/ec2/Ec2DiscoveryTests.java | 2 +- .../discovery/gce/GceSeedHostsProvider.java | 3 +- .../discovery/FileBasedSeedHostsProvider.java | 2 +- .../discovery/SeedHostsProvider.java | 5 +- .../discovery/SeedHostsResolver.java | 6 +- .../SettingsBasedSeedHostsProvider.java | 19 +++-- .../elasticsearch/transport/TcpTransport.java | 31 ++++---- .../elasticsearch/transport/Transport.java | 5 +- .../transport/TransportService.java | 8 ++- .../transport/FailAndRetryMockTransport.java | 2 +- .../TransportClientNodesServiceTests.java | 5 ++ .../cluster/NodeConnectionsServiceTests.java | 7 +- .../discovery/SeedHostsResolverTests.java | 54 +++----------- .../SettingsBasedSeedHostsProviderTests.java | 18 ++--- .../transport/TcpTransportTests.java | 71 +++---------------- .../test/transport/MockTransport.java | 7 +- .../test/transport/StubbableTransport.java | 9 ++- 19 files changed, 105 insertions(+), 155 deletions(-) diff --git a/plugins/discovery-azure-classic/src/main/java/org/elasticsearch/discovery/azure/classic/AzureSeedHostsProvider.java b/plugins/discovery-azure-classic/src/main/java/org/elasticsearch/discovery/azure/classic/AzureSeedHostsProvider.java index d6b5a85b51f14..4c527264e23fc 100644 --- a/plugins/discovery-azure-classic/src/main/java/org/elasticsearch/discovery/azure/classic/AzureSeedHostsProvider.java +++ b/plugins/discovery-azure-classic/src/main/java/org/elasticsearch/discovery/azure/classic/AzureSeedHostsProvider.java @@ -208,8 +208,7 @@ public List getSeedAddresses(HostsResolver hostsResolver) { } try { - // we only limit to 1 port per address, makes no sense to ping 100 ports - TransportAddress[] addresses = transportService.addressesFromString(networkAddress, 1); + TransportAddress[] addresses = transportService.addressesFromString(networkAddress); for (TransportAddress address : addresses) { logger.trace("adding {}, transport_address {}", networkAddress, address); dynamicHosts.add(address); diff --git a/plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2SeedHostsProvider.java b/plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2SeedHostsProvider.java index 97b7ade49f00c..515aef8408b01 100644 --- a/plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2SeedHostsProvider.java +++ b/plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2SeedHostsProvider.java @@ -174,8 +174,7 @@ && disjoint(securityGroupIds, groups)) { } if (address != null) { try { - // we only limit to 1 port per address, makes no sense to ping 100 ports - final TransportAddress[] addresses = transportService.addressesFromString(address, 1); + final TransportAddress[] addresses = transportService.addressesFromString(address); for (int i = 0; i < addresses.length; i++) { logger.trace("adding {}, address {}, transport_address {}", instance.getInstanceId(), address, addresses[i]); dynamicHosts.add(addresses[i]); diff --git a/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/Ec2DiscoveryTests.java b/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/Ec2DiscoveryTests.java index 9d7d7e0eb0677..6703812a4ec0c 100644 --- a/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/Ec2DiscoveryTests.java +++ b/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/Ec2DiscoveryTests.java @@ -77,7 +77,7 @@ public void createTransportService() { new NetworkService(Collections.emptyList()), PageCacheRecycler.NON_RECYCLING_INSTANCE, namedWriteableRegistry, new NoneCircuitBreakerService()) { @Override - public TransportAddress[] addressesFromString(String address, int perAddressLimit) throws UnknownHostException { + public TransportAddress[] addressesFromString(String address) throws UnknownHostException { // we just need to ensure we don't resolve DNS here return new TransportAddress[] {poorMansDNS.getOrDefault(address, buildNewFakeTransportAddress())}; } diff --git a/plugins/discovery-gce/src/main/java/org/elasticsearch/discovery/gce/GceSeedHostsProvider.java b/plugins/discovery-gce/src/main/java/org/elasticsearch/discovery/gce/GceSeedHostsProvider.java index fded7c2445d2a..d193cb25c6e31 100644 --- a/plugins/discovery-gce/src/main/java/org/elasticsearch/discovery/gce/GceSeedHostsProvider.java +++ b/plugins/discovery-gce/src/main/java/org/elasticsearch/discovery/gce/GceSeedHostsProvider.java @@ -233,8 +233,7 @@ public List getSeedAddresses(HostsResolver hostsResolver) { // ip_private is a single IP Address. We need to build a TransportAddress from it // If user has set `es_port` metadata, we don't need to ping all ports - // we only limit to 1 addresses, makes no sense to ping 100 ports - TransportAddress[] addresses = transportService.addressesFromString(address, 1); + TransportAddress[] addresses = transportService.addressesFromString(address); for (TransportAddress transportAddress : addresses) { logger.trace("adding {}, type {}, address {}, transport_address {}, status {}", name, type, diff --git a/server/src/main/java/org/elasticsearch/discovery/FileBasedSeedHostsProvider.java b/server/src/main/java/org/elasticsearch/discovery/FileBasedSeedHostsProvider.java index 3af83e36311eb..8e0192f58e720 100644 --- a/server/src/main/java/org/elasticsearch/discovery/FileBasedSeedHostsProvider.java +++ b/server/src/main/java/org/elasticsearch/discovery/FileBasedSeedHostsProvider.java @@ -75,7 +75,7 @@ private List getHostsList() { @Override public List getSeedAddresses(HostsResolver hostsResolver) { - final List transportAddresses = hostsResolver.resolveHosts(getHostsList(), 1); + final List transportAddresses = hostsResolver.resolveHosts(getHostsList()); logger.debug("seed addresses: {}", transportAddresses); return transportAddresses; } diff --git a/server/src/main/java/org/elasticsearch/discovery/SeedHostsProvider.java b/server/src/main/java/org/elasticsearch/discovery/SeedHostsProvider.java index 12eb11e368618..4811d13d2d970 100644 --- a/server/src/main/java/org/elasticsearch/discovery/SeedHostsProvider.java +++ b/server/src/main/java/org/elasticsearch/discovery/SeedHostsProvider.java @@ -36,10 +36,9 @@ public interface SeedHostsProvider { /** * Helper object that allows to resolve a list of hosts to a list of transport addresses. - * Each host is resolved into a transport address (or a collection of addresses if the - * number of ports is greater than one) + * Each host is resolved into a transport address */ interface HostsResolver { - List resolveHosts(List hosts, int limitPortCounts); + List resolveHosts(List hosts); } } diff --git a/server/src/main/java/org/elasticsearch/discovery/SeedHostsResolver.java b/server/src/main/java/org/elasticsearch/discovery/SeedHostsResolver.java index 5ba0402389aa6..aa4c39ad8276d 100644 --- a/server/src/main/java/org/elasticsearch/discovery/SeedHostsResolver.java +++ b/server/src/main/java/org/elasticsearch/discovery/SeedHostsResolver.java @@ -87,9 +87,7 @@ public static TimeValue getResolveTimeout(Settings settings) { } @Override - public List resolveHosts( - final List hosts, - final int limitPortCounts) { + public List resolveHosts(final List hosts) { Objects.requireNonNull(hosts); if (resolveTimeout.nanos() < 0) { throw new IllegalArgumentException("resolve timeout must be non-negative but was [" + resolveTimeout + "]"); @@ -98,7 +96,7 @@ public List resolveHosts( final List> callables = hosts .stream() - .map(hn -> (Callable) () -> transportService.addressesFromString(hn, limitPortCounts)) + .map(hn -> (Callable) () -> transportService.addressesFromString(hn)) .collect(Collectors.toList()); final List> futures; try { diff --git a/server/src/main/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProvider.java b/server/src/main/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProvider.java index b3b3ca27894a5..9a75a5d9577ab 100644 --- a/server/src/main/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProvider.java +++ b/server/src/main/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProvider.java @@ -27,8 +27,10 @@ import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.transport.TransportService; +import java.util.Arrays; import java.util.List; import java.util.function.Function; +import java.util.stream.Collectors; import static java.util.Collections.emptyList; @@ -48,21 +50,24 @@ public class SettingsBasedSeedHostsProvider implements SeedHostsProvider { Setting.listSetting("discovery.seed_hosts", emptyList(), Function.identity(), Property.NodeScope); // these limits are per-address - private static final int LIMIT_FOREIGN_PORTS_COUNT = 1; - private static final int LIMIT_LOCAL_PORTS_COUNT = 5; + private static final int LIMIT_LOCAL_PORTS_COUNT = 6; private final List configuredHosts; - private final int limitPortCounts; public SettingsBasedSeedHostsProvider(Settings settings, TransportService transportService) { if (DISCOVERY_SEED_HOSTS_SETTING.exists(settings)) { configuredHosts = DISCOVERY_SEED_HOSTS_SETTING.get(settings); // we only limit to 1 address, makes no sense to ping 100 ports - limitPortCounts = LIMIT_FOREIGN_PORTS_COUNT; } else { // if unicast hosts are not specified, fill with simple defaults on the local machine - configuredHosts = transportService.getLocalAddresses(); - limitPortCounts = LIMIT_LOCAL_PORTS_COUNT; + int[] ports = transportService.getDefaultPortRange(); + configuredHosts = transportService.getLocalAddresses().stream() + .flatMap( + address -> Arrays.stream(ports) + .limit(LIMIT_LOCAL_PORTS_COUNT) + .mapToObj(port -> address + ":" + port) + ) + .collect(Collectors.toList()); } logger.debug("using initial hosts {}", configuredHosts); @@ -70,6 +75,6 @@ public SettingsBasedSeedHostsProvider(Settings settings, TransportService transp @Override public List getSeedAddresses(HostsResolver hostsResolver) { - return hostsResolver.resolveHosts(configuredHosts, limitPortCounts); + return hostsResolver.resolveHosts(configuredHosts); } } diff --git a/server/src/main/java/org/elasticsearch/transport/TcpTransport.java b/server/src/main/java/org/elasticsearch/transport/TcpTransport.java index 42d613016352a..e5554db034da4 100644 --- a/server/src/main/java/org/elasticsearch/transport/TcpTransport.java +++ b/server/src/main/java/org/elasticsearch/transport/TcpTransport.java @@ -456,8 +456,13 @@ static int resolvePublishPort(ProfileSettings profileSettings, List 1) { + throw new IllegalArgumentException("Port ranges are not supported"); + } + port = ports[0]; } - // generate address for each port in the range Set addresses = new HashSet<>(Arrays.asList(InetAddress.getAllByName(host))); List transportAddresses = new ArrayList<>(); - int[] ports = new PortsRange(portString).ports(); - int limit = Math.min(ports.length, perAddressLimit); - for (int i = 0; i < limit; i++) { - for (InetAddress address : addresses) { - transportAddresses.add(new TransportAddress(address, ports[i])); - } + for (InetAddress address : addresses) { + transportAddresses.add(new TransportAddress(address, port)); } return transportAddresses.toArray(new TransportAddress[transportAddresses.size()]); } diff --git a/server/src/main/java/org/elasticsearch/transport/Transport.java b/server/src/main/java/org/elasticsearch/transport/Transport.java index eea8ce0f2ffe8..86abd8a4b16fc 100644 --- a/server/src/main/java/org/elasticsearch/transport/Transport.java +++ b/server/src/main/java/org/elasticsearch/transport/Transport.java @@ -68,7 +68,10 @@ public interface Transport extends LifecycleComponent { /** * Returns an address from its string representation. */ - TransportAddress[] addressesFromString(String address, int perAddressLimit) throws UnknownHostException; + TransportAddress[] addressesFromString(String address) throws UnknownHostException; + + int[] defaultPortRange(); + /** * Returns a list of all local adresses for this transport diff --git a/server/src/main/java/org/elasticsearch/transport/TransportService.java b/server/src/main/java/org/elasticsearch/transport/TransportService.java index c8493edc97983..b6240fe5fb1ba 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportService.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportService.java @@ -750,8 +750,12 @@ private boolean shouldTraceAction(String action) { return true; } - public TransportAddress[] addressesFromString(String address, int perAddressLimit) throws UnknownHostException { - return transport.addressesFromString(address, perAddressLimit); + public TransportAddress[] addressesFromString(String address) throws UnknownHostException { + return transport.addressesFromString(address); + } + + public int[] getDefaultPortRange() { + return transport.defaultPortRange(); } /** diff --git a/server/src/test/java/org/elasticsearch/client/transport/FailAndRetryMockTransport.java b/server/src/test/java/org/elasticsearch/client/transport/FailAndRetryMockTransport.java index 9a74282d51f87..a636dc8471d7d 100644 --- a/server/src/test/java/org/elasticsearch/client/transport/FailAndRetryMockTransport.java +++ b/server/src/test/java/org/elasticsearch/client/transport/FailAndRetryMockTransport.java @@ -170,7 +170,7 @@ public BoundTransportAddress boundAddress() { } @Override - public TransportAddress[] addressesFromString(String address, int perAddressLimit) throws UnknownHostException { + public TransportAddress[] addressesFromString(String address) throws UnknownHostException { throw new UnsupportedOperationException(); } diff --git a/server/src/test/java/org/elasticsearch/client/transport/TransportClientNodesServiceTests.java b/server/src/test/java/org/elasticsearch/client/transport/TransportClientNodesServiceTests.java index bdcaf80ee19e9..02ee502d1a215 100644 --- a/server/src/test/java/org/elasticsearch/client/transport/TransportClientNodesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/client/transport/TransportClientNodesServiceTests.java @@ -132,6 +132,11 @@ public List getLocalAddresses() { return Collections.emptyList(); } + @Override + public int[] defaultPortRange() { + return new int[0]; + } + @Override protected TestResponse newResponse() { return new TestResponse(); diff --git a/server/src/test/java/org/elasticsearch/cluster/NodeConnectionsServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/NodeConnectionsServiceTests.java index 25179427d863c..4ff57d369cc01 100644 --- a/server/src/test/java/org/elasticsearch/cluster/NodeConnectionsServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/NodeConnectionsServiceTests.java @@ -401,10 +401,15 @@ public Map profileBoundAddresses() { } @Override - public TransportAddress[] addressesFromString(String address, int perAddressLimit) { + public TransportAddress[] addressesFromString(String address) { return new TransportAddress[0]; } + @Override + public int[] defaultPortRange() { + return new int[0]; + } + @Override public Releasable openConnection(DiscoveryNode node, ConnectionProfile profile, ActionListener listener) { if (profile == null && randomConnectionExceptions && randomBoolean()) { diff --git a/server/src/test/java/org/elasticsearch/discovery/SeedHostsResolverTests.java b/server/src/test/java/org/elasticsearch/discovery/SeedHostsResolverTests.java index 3527d6de3da64..3d303ef47c462 100644 --- a/server/src/test/java/org/elasticsearch/discovery/SeedHostsResolverTests.java +++ b/server/src/test/java/org/elasticsearch/discovery/SeedHostsResolverTests.java @@ -152,43 +152,6 @@ public void testResolvesAddressesInBackgroundAndIgnoresConcurrentCalls() throws assertThat(resolvedAddressesRef.get(), equalTo(transportAddresses)); } - public void testPortLimit() { - final NetworkService networkService = new NetworkService(Collections.emptyList()); - final Transport transport = new MockNioTransport( - Settings.EMPTY, - Version.CURRENT, - threadPool, - networkService, - PageCacheRecycler.NON_RECYCLING_INSTANCE, - new NamedWriteableRegistry(Collections.emptyList()), - new NoneCircuitBreakerService()) { - - @Override - public BoundTransportAddress boundAddress() { - return new BoundTransportAddress( - new TransportAddress[]{new TransportAddress(InetAddress.getLoopbackAddress(), 9500)}, - new TransportAddress(InetAddress.getLoopbackAddress(), 9500) - ); - } - }; - closeables.push(transport); - final TransportService transportService = - new TransportService(Settings.EMPTY, transport, threadPool, TransportService.NOOP_TRANSPORT_INTERCEPTOR, x -> null, null, - Collections.emptySet()); - closeables.push(transportService); - recreateSeedHostsResolver(transportService); - final int limitPortCounts = randomIntBetween(1, 10); - final List transportAddresses = seedHostsResolver.resolveHosts(Collections.singletonList("127.0.0.1"), - limitPortCounts); - assertThat(transportAddresses, hasSize(limitPortCounts)); - final Set ports = new HashSet<>(); - for (final TransportAddress address : transportAddresses) { - assertTrue(address.address().getAddress().isLoopbackAddress()); - ports.add(address.getPort()); - } - assertThat(ports, equalTo(IntStream.range(9300, 9300 + limitPortCounts).boxed().collect(Collectors.toSet()))); - } - public void testRemovingLocalAddresses() { final NetworkService networkService = new NetworkService(Collections.emptyList()); final InetAddress loopbackAddress = InetAddress.getLoopbackAddress(); @@ -218,9 +181,10 @@ public BoundTransportAddress boundAddress() { Collections.emptySet()); closeables.push(transportService); recreateSeedHostsResolver(transportService); - final List transportAddresses = seedHostsResolver.resolveHosts( - Collections.singletonList(NetworkAddress.format(loopbackAddress)), - 10); + List hosts = IntStream.range(9300, 9310) + .mapToObj(port -> NetworkAddress.format(loopbackAddress) + ":" + port) + .collect(Collectors.toList()); + final List transportAddresses = seedHostsResolver.resolveHosts(hosts); assertThat(transportAddresses, hasSize(7)); final Set ports = new HashSet<>(); for (final TransportAddress address : transportAddresses) { @@ -252,7 +216,7 @@ public BoundTransportAddress boundAddress() { } @Override - public TransportAddress[] addressesFromString(String address, int perAddressLimit) throws UnknownHostException { + public TransportAddress[] addressesFromString(String address) throws UnknownHostException { throw unknownHostException; } @@ -279,7 +243,7 @@ public TransportAddress[] addressesFromString(String address, int perAddressLimi try { Loggers.addAppender(logger, appender); - final List transportAddresses = seedHostsResolver.resolveHosts(Collections.singletonList(hostname), 1); + final List transportAddresses = seedHostsResolver.resolveHosts(Collections.singletonList(hostname)); assertThat(transportAddresses, empty()); appender.assertAllExpectationsMatched(); @@ -310,7 +274,7 @@ public BoundTransportAddress boundAddress() { } @Override - public TransportAddress[] addressesFromString(String address, int perAddressLimit) throws UnknownHostException { + public TransportAddress[] addressesFromString(String address) throws UnknownHostException { if ("hostname1".equals(address)) { return new TransportAddress[]{new TransportAddress(TransportAddress.META_ADDRESS, 9300)}; } else if ("hostname2".equals(address)) { @@ -346,7 +310,7 @@ public TransportAddress[] addressesFromString(String address, int perAddressLimi try { Loggers.addAppender(logger, appender); - final List transportAddresses = seedHostsResolver.resolveHosts(Arrays.asList("hostname1", "hostname2"), 1); + final List transportAddresses = seedHostsResolver.resolveHosts(Arrays.asList("hostname1", "hostname2")); assertThat(transportAddresses, hasSize(1)); appender.assertAllExpectationsMatched(); @@ -396,7 +360,7 @@ public BoundTransportAddress boundAddress() { try { Loggers.addAppender(logger, appender); final List transportAddresses = seedHostsResolver.resolveHosts( - Arrays.asList("127.0.0.1:9300:9300", "127.0.0.1:9301"), 1); + Arrays.asList("127.0.0.1:9300:9300", "127.0.0.1:9301")); assertThat(transportAddresses, hasSize(1)); // only one of the two is valid and will be used assertThat(transportAddresses.get(0).getAddress(), equalTo("127.0.0.1")); assertThat(transportAddresses.get(0).getPort(), equalTo(9301)); diff --git a/server/src/test/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProviderTests.java b/server/src/test/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProviderTests.java index d98e152149382..118cbcbe0f84d 100644 --- a/server/src/test/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProviderTests.java +++ b/server/src/test/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProviderTests.java @@ -28,6 +28,7 @@ import java.util.Arrays; import java.util.List; import java.util.Set; +import java.util.stream.IntStream; import static java.util.Collections.emptyList; import static org.mockito.Mockito.mock; @@ -37,18 +38,15 @@ public class SettingsBasedSeedHostsProviderTests extends ESTestCase { private class AssertingHostsResolver implements HostsResolver { private final Set expectedHosts; - private final int expectedPortCount; private boolean resolvedHosts; - AssertingHostsResolver(int expectedPortCount, String... expectedHosts) { - this.expectedPortCount = expectedPortCount; + AssertingHostsResolver(String... expectedHosts) { this.expectedHosts = Sets.newHashSet(expectedHosts); } @Override - public List resolveHosts(List hosts, int limitPortCounts) { - assertEquals(expectedPortCount, limitPortCounts); + public List resolveHosts(List hosts) { assertEquals(expectedHosts, Sets.newHashSet(hosts)); resolvedHosts = true; return emptyList(); @@ -60,15 +58,19 @@ boolean getResolvedHosts() { } public void testScansPortsByDefault() { - final AssertingHostsResolver hostsResolver = new AssertingHostsResolver(5, "::1", "127.0.0.1"); + final AssertingHostsResolver hostsResolver = new AssertingHostsResolver( + "[::1]:9300", "[::1]:9301", "[::1]:9302", "[::1]:9303", "[::1]:9304", "[::1]:9305", + "127.0.0.1:9300", "127.0.0.1:9301", "127.0.0.1:9302", "127.0.0.1:9303", "127.0.0.1:9304", "127.0.0.1:9305" + ); final TransportService transportService = mock(TransportService.class); - when(transportService.getLocalAddresses()).thenReturn(Arrays.asList("::1", "127.0.0.1")); + when(transportService.getLocalAddresses()).thenReturn(Arrays.asList("[::1]", "127.0.0.1")); + when(transportService.getDefaultPortRange()).thenReturn(IntStream.range(9300, 9401).toArray()); new SettingsBasedSeedHostsProvider(Settings.EMPTY, transportService).getSeedAddresses(hostsResolver); assertTrue(hostsResolver.getResolvedHosts()); } public void testGetsHostsFromSetting() { - final AssertingHostsResolver hostsResolver = new AssertingHostsResolver(1, "bar", "foo"); + final AssertingHostsResolver hostsResolver = new AssertingHostsResolver("bar", "foo"); new SettingsBasedSeedHostsProvider(Settings.builder() .putList(SettingsBasedSeedHostsProvider.DISCOVERY_SEED_HOSTS_SETTING.getKey(), "foo", "bar") .build(), null).getSeedAddresses(hostsResolver); diff --git a/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java b/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java index 4519513db2812..b8d220704e41b 100644 --- a/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java +++ b/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java @@ -34,50 +34,26 @@ public class TcpTransportTests extends ESTestCase { /** Test ipv4 host with a default port works */ public void testParseV4DefaultPort() throws Exception { - TransportAddress[] addresses = TcpTransport.parse("127.0.0.1", "1234", Integer.MAX_VALUE); + TransportAddress[] addresses = TcpTransport.parse("127.0.0.1", 1234); assertEquals(1, addresses.length); assertEquals("127.0.0.1", addresses[0].getAddress()); assertEquals(1234, addresses[0].getPort()); } - /** Test ipv4 host with a default port range works */ - public void testParseV4DefaultRange() throws Exception { - TransportAddress[] addresses = TcpTransport.parse("127.0.0.1", "1234-1235", Integer.MAX_VALUE); - assertEquals(2, addresses.length); - - assertEquals("127.0.0.1", addresses[0].getAddress()); - assertEquals(1234, addresses[0].getPort()); - - assertEquals("127.0.0.1", addresses[1].getAddress()); - assertEquals(1235, addresses[1].getPort()); - } - /** Test ipv4 host with port works */ public void testParseV4WithPort() throws Exception { - TransportAddress[] addresses = TcpTransport.parse("127.0.0.1:2345", "1234", Integer.MAX_VALUE); + TransportAddress[] addresses = TcpTransport.parse("127.0.0.1:2345", 1234); assertEquals(1, addresses.length); assertEquals("127.0.0.1", addresses[0].getAddress()); assertEquals(2345, addresses[0].getPort()); } - /** Test ipv4 host with port range works */ - public void testParseV4WithPortRange() throws Exception { - TransportAddress[] addresses = TcpTransport.parse("127.0.0.1:2345-2346", "1234", Integer.MAX_VALUE); - assertEquals(2, addresses.length); - - assertEquals("127.0.0.1", addresses[0].getAddress()); - assertEquals(2345, addresses[0].getPort()); - - assertEquals("127.0.0.1", addresses[1].getAddress()); - assertEquals(2346, addresses[1].getPort()); - } - /** Test unbracketed ipv6 hosts in configuration fail. Leave no ambiguity */ public void testParseV6UnBracketed() throws Exception { try { - TcpTransport.parse("::1", "1234", Integer.MAX_VALUE); + TcpTransport.parse("::1", 1234); fail("should have gotten exception"); } catch (IllegalArgumentException expected) { assertTrue(expected.getMessage().contains("must be bracketed")); @@ -86,53 +62,28 @@ public void testParseV6UnBracketed() throws Exception { /** Test ipv6 host with a default port works */ public void testParseV6DefaultPort() throws Exception { - TransportAddress[] addresses = TcpTransport.parse("[::1]", "1234", Integer.MAX_VALUE); + TransportAddress[] addresses = TcpTransport.parse("[::1]", 1234); assertEquals(1, addresses.length); assertEquals("::1", addresses[0].getAddress()); assertEquals(1234, addresses[0].getPort()); } - /** Test ipv6 host with a default port range works */ - public void testParseV6DefaultRange() throws Exception { - TransportAddress[] addresses = TcpTransport.parse("[::1]", "1234-1235", Integer.MAX_VALUE); - assertEquals(2, addresses.length); - - assertEquals("::1", addresses[0].getAddress()); - assertEquals(1234, addresses[0].getPort()); - - assertEquals("::1", addresses[1].getAddress()); - assertEquals(1235, addresses[1].getPort()); - } - /** Test ipv6 host with port works */ public void testParseV6WithPort() throws Exception { - TransportAddress[] addresses = TcpTransport.parse("[::1]:2345", "1234", Integer.MAX_VALUE); + TransportAddress[] addresses = TcpTransport.parse("[::1]:2345", 1234); assertEquals(1, addresses.length); assertEquals("::1", addresses[0].getAddress()); assertEquals(2345, addresses[0].getPort()); } - /** Test ipv6 host with port range works */ - public void testParseV6WithPortRange() throws Exception { - TransportAddress[] addresses = TcpTransport.parse("[::1]:2345-2346", "1234", Integer.MAX_VALUE); - assertEquals(2, addresses.length); - - assertEquals("::1", addresses[0].getAddress()); - assertEquals(2345, addresses[0].getPort()); - - assertEquals("::1", addresses[1].getAddress()); - assertEquals(2346, addresses[1].getPort()); - } - - /** Test per-address limit */ - public void testAddressLimit() throws Exception { - TransportAddress[] addresses = TcpTransport.parse("[::1]:100-200", "1000", 3); - assertEquals(3, addresses.length); - assertEquals(100, addresses[0].getPort()); - assertEquals(101, addresses[1].getPort()); - assertEquals(102, addresses[2].getPort()); + public void testRejectsPortRanges() { + IllegalArgumentException expected = expectThrows( + IllegalArgumentException.class, + () -> TcpTransport.parse("[::1]:100-200", 1000) + ); + assertTrue(expected.getMessage().contains("Port ranges are not supported")); } public void testDecodeWithIncompleteHeader() throws IOException { diff --git a/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransport.java b/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransport.java index a9c70deaaeabd..0a1132edb67a9 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransport.java +++ b/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransport.java @@ -208,10 +208,15 @@ public Map profileBoundAddresses() { } @Override - public TransportAddress[] addressesFromString(String address, int perAddressLimit) { + public TransportAddress[] addressesFromString(String address) { return new TransportAddress[0]; } + @Override + public int[] defaultPortRange() { + return new int[0]; + } + @Override public Lifecycle.State lifecycleState() { return null; diff --git a/test/framework/src/main/java/org/elasticsearch/test/transport/StubbableTransport.java b/test/framework/src/main/java/org/elasticsearch/test/transport/StubbableTransport.java index 4ccc352158a73..f21a74fd2560e 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/transport/StubbableTransport.java +++ b/test/framework/src/main/java/org/elasticsearch/test/transport/StubbableTransport.java @@ -115,8 +115,13 @@ public BoundTransportAddress boundAddress() { } @Override - public TransportAddress[] addressesFromString(String address, int perAddressLimit) throws UnknownHostException { - return delegate.addressesFromString(address, perAddressLimit); + public TransportAddress[] addressesFromString(String address) throws UnknownHostException { + return delegate.addressesFromString(address); + } + + @Override + public int[] defaultPortRange() { + return delegate.defaultPortRange(); } @Override From 12b1338aa14f05cee2f3ff093d0267015c476eb1 Mon Sep 17 00:00:00 2001 From: Nikita_Glashchenko Date: Fri, 26 Apr 2019 17:04:20 +0400 Subject: [PATCH 2/8] remove redundant comment --- .../elasticsearch/discovery/SettingsBasedSeedHostsProvider.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProvider.java b/server/src/main/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProvider.java index 9a75a5d9577ab..c3a1f31d3749e 100644 --- a/server/src/main/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProvider.java +++ b/server/src/main/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProvider.java @@ -57,7 +57,6 @@ public class SettingsBasedSeedHostsProvider implements SeedHostsProvider { public SettingsBasedSeedHostsProvider(Settings settings, TransportService transportService) { if (DISCOVERY_SEED_HOSTS_SETTING.exists(settings)) { configuredHosts = DISCOVERY_SEED_HOSTS_SETTING.get(settings); - // we only limit to 1 address, makes no sense to ping 100 ports } else { // if unicast hosts are not specified, fill with simple defaults on the local machine int[] ports = transportService.getDefaultPortRange(); From a48c045cec50b716dcd66fa58d0faff4653a48a1 Mon Sep 17 00:00:00 2001 From: Nikita_Glashchenko Date: Mon, 29 Apr 2019 16:31:40 +0400 Subject: [PATCH 3/8] move default seed addresses computation to TcpTransport --- .../SettingsBasedSeedHostsProvider.java | 12 +------ .../elasticsearch/transport/TcpTransport.java | 31 +++++++++++++------ .../elasticsearch/transport/Transport.java | 7 ++--- .../transport/TransportService.java | 8 ++--- .../TransportClientNodesServiceTests.java | 7 +---- .../cluster/NodeConnectionsServiceTests.java | 7 +---- .../SettingsBasedSeedHostsProviderTests.java | 8 ++--- .../transport/TcpTransportTests.java | 14 +++++++++ .../test/transport/MockTransport.java | 7 +---- .../test/transport/StubbableTransport.java | 9 ++---- 10 files changed, 47 insertions(+), 63 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProvider.java b/server/src/main/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProvider.java index c3a1f31d3749e..b1e0b69b675d4 100644 --- a/server/src/main/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProvider.java +++ b/server/src/main/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProvider.java @@ -49,9 +49,6 @@ public class SettingsBasedSeedHostsProvider implements SeedHostsProvider { public static final Setting> DISCOVERY_SEED_HOSTS_SETTING = Setting.listSetting("discovery.seed_hosts", emptyList(), Function.identity(), Property.NodeScope); - // these limits are per-address - private static final int LIMIT_LOCAL_PORTS_COUNT = 6; - private final List configuredHosts; public SettingsBasedSeedHostsProvider(Settings settings, TransportService transportService) { @@ -59,14 +56,7 @@ public SettingsBasedSeedHostsProvider(Settings settings, TransportService transp configuredHosts = DISCOVERY_SEED_HOSTS_SETTING.get(settings); } else { // if unicast hosts are not specified, fill with simple defaults on the local machine - int[] ports = transportService.getDefaultPortRange(); - configuredHosts = transportService.getLocalAddresses().stream() - .flatMap( - address -> Arrays.stream(ports) - .limit(LIMIT_LOCAL_PORTS_COUNT) - .mapToObj(port -> address + ":" + port) - ) - .collect(Collectors.toList()); + configuredHosts = transportService.getDefaultSeedAddresses(); } logger.debug("using initial hosts {}", configuredHosts); diff --git a/server/src/main/java/org/elasticsearch/transport/TcpTransport.java b/server/src/main/java/org/elasticsearch/transport/TcpTransport.java index e5554db034da4..c3f0a5bcfa013 100644 --- a/server/src/main/java/org/elasticsearch/transport/TcpTransport.java +++ b/server/src/main/java/org/elasticsearch/transport/TcpTransport.java @@ -86,6 +86,7 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import static java.util.Collections.unmodifiableMap; import static org.elasticsearch.common.transport.NetworkExceptionHelper.isCloseConnectionException; @@ -102,6 +103,9 @@ public abstract class TcpTransport extends AbstractLifecycleComponent implements private static final long NINETY_PER_HEAP_SIZE = (long) (JvmInfo.jvmInfo().getMem().getHeapMax().getBytes() * 0.9); private static final BytesReference EMPTY_BYTES_REFERENCE = new BytesArray(new byte[0]); + // this limit is per-address + private static final int LIMIT_LOCAL_PORTS_COUNT = 6; + protected final Settings settings; protected final ThreadPool threadPool; protected final PageCacheRecycler pageCacheRecycler; @@ -311,14 +315,24 @@ public Map profileBoundAddresses() { } @Override - public List getLocalAddresses() { + public List getDefaultSeedAddresses() { List local = new ArrayList<>(); local.add("127.0.0.1"); // check if v6 is supported, if so, v4 will also work via mapped addresses. if (NetworkUtils.SUPPORTS_V6) { local.add("[::1]"); // may get ports appended! } - return local; + return combineHostsAndPorts(local, defaultPortRange()); + } + + static List combineHostsAndPorts(List hosts, int[] ports) { + return hosts.stream() + .flatMap( + address -> Arrays.stream(ports) + .limit(LIMIT_LOCAL_PORTS_COUNT) + .mapToObj(port -> address + ":" + port) + ) + .collect(Collectors.toList()); } protected void bindServer(ProfileSettings profileSettings) { @@ -460,8 +474,7 @@ public TransportAddress[] addressesFromString(String address) throws UnknownHost return parse(address, defaultPortRange()[0]); } - @Override - public int[] defaultPortRange() { + private int[] defaultPortRange() { return new PortsRange(settings.get("transport.profiles.default.port", TransportSettings.PORT.get(settings))).ports(); } @@ -515,12 +528,10 @@ static TransportAddress[] parse(String hostPortString, int defaultPort) throws U port = ports[0]; } - Set addresses = new HashSet<>(Arrays.asList(InetAddress.getAllByName(host))); - List transportAddresses = new ArrayList<>(); - for (InetAddress address : addresses) { - transportAddresses.add(new TransportAddress(address, port)); - } - return transportAddresses.toArray(new TransportAddress[transportAddresses.size()]); + return Arrays.stream(InetAddress.getAllByName(host)) + .distinct() + .map(address -> new TransportAddress(address, port)) + .toArray(TransportAddress[]::new); } @Override diff --git a/server/src/main/java/org/elasticsearch/transport/Transport.java b/server/src/main/java/org/elasticsearch/transport/Transport.java index 86abd8a4b16fc..0b79b6aecf093 100644 --- a/server/src/main/java/org/elasticsearch/transport/Transport.java +++ b/server/src/main/java/org/elasticsearch/transport/Transport.java @@ -70,13 +70,10 @@ public interface Transport extends LifecycleComponent { */ TransportAddress[] addressesFromString(String address) throws UnknownHostException; - int[] defaultPortRange(); - - /** - * Returns a list of all local adresses for this transport + * Returns a list of all local addresses for this transport */ - List getLocalAddresses(); + List getDefaultSeedAddresses(); default CircuitBreaker getInFlightRequestBreaker() { return new NoopCircuitBreaker("in-flight-noop"); diff --git a/server/src/main/java/org/elasticsearch/transport/TransportService.java b/server/src/main/java/org/elasticsearch/transport/TransportService.java index b6240fe5fb1ba..90fd7c1847baf 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportService.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportService.java @@ -313,8 +313,8 @@ public BoundTransportAddress boundAddress() { return transport.boundAddress(); } - public List getLocalAddresses() { - return transport.getLocalAddresses(); + public List getDefaultSeedAddresses() { + return transport.getDefaultSeedAddresses(); } /** @@ -754,10 +754,6 @@ public TransportAddress[] addressesFromString(String address) throws UnknownHost return transport.addressesFromString(address); } - public int[] getDefaultPortRange() { - return transport.defaultPortRange(); - } - /** * A set of all valid action prefixes. */ diff --git a/server/src/test/java/org/elasticsearch/client/transport/TransportClientNodesServiceTests.java b/server/src/test/java/org/elasticsearch/client/transport/TransportClientNodesServiceTests.java index 02ee502d1a215..9e13dbaa89b18 100644 --- a/server/src/test/java/org/elasticsearch/client/transport/TransportClientNodesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/client/transport/TransportClientNodesServiceTests.java @@ -128,15 +128,10 @@ private static class TestIteration implements Closeable { threadPool = new TestThreadPool("transport-client-nodes-service-tests"); transport = new FailAndRetryMockTransport(random(), clusterName) { @Override - public List getLocalAddresses() { + public List getDefaultSeedAddresses() { return Collections.emptyList(); } - @Override - public int[] defaultPortRange() { - return new int[0]; - } - @Override protected TestResponse newResponse() { return new TestResponse(); diff --git a/server/src/test/java/org/elasticsearch/cluster/NodeConnectionsServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/NodeConnectionsServiceTests.java index 4ff57d369cc01..193cde3180db8 100644 --- a/server/src/test/java/org/elasticsearch/cluster/NodeConnectionsServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/NodeConnectionsServiceTests.java @@ -405,11 +405,6 @@ public TransportAddress[] addressesFromString(String address) { return new TransportAddress[0]; } - @Override - public int[] defaultPortRange() { - return new int[0]; - } - @Override public Releasable openConnection(DiscoveryNode node, ConnectionProfile profile, ActionListener listener) { if (profile == null && randomConnectionExceptions && randomBoolean()) { @@ -445,7 +440,7 @@ public boolean isClosed() { } @Override - public List getLocalAddresses() { + public List getDefaultSeedAddresses() { return null; } diff --git a/server/src/test/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProviderTests.java b/server/src/test/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProviderTests.java index 118cbcbe0f84d..ab871614bfd1b 100644 --- a/server/src/test/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProviderTests.java +++ b/server/src/test/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProviderTests.java @@ -58,13 +58,9 @@ boolean getResolvedHosts() { } public void testScansPortsByDefault() { - final AssertingHostsResolver hostsResolver = new AssertingHostsResolver( - "[::1]:9300", "[::1]:9301", "[::1]:9302", "[::1]:9303", "[::1]:9304", "[::1]:9305", - "127.0.0.1:9300", "127.0.0.1:9301", "127.0.0.1:9302", "127.0.0.1:9303", "127.0.0.1:9304", "127.0.0.1:9305" - ); + final AssertingHostsResolver hostsResolver = new AssertingHostsResolver("[::1]:9300", "[::1]:9301", "127.0.0.1:9300", "127.0.0.1:9301"); final TransportService transportService = mock(TransportService.class); - when(transportService.getLocalAddresses()).thenReturn(Arrays.asList("[::1]", "127.0.0.1")); - when(transportService.getDefaultPortRange()).thenReturn(IntStream.range(9300, 9401).toArray()); + when(transportService.getDefaultSeedAddresses()).thenReturn(Arrays.asList("[::1]:9300", "[::1]:9301", "127.0.0.1:9300", "127.0.0.1:9301")); new SettingsBasedSeedHostsProvider(Settings.EMPTY, transportService).getSeedAddresses(hostsResolver); assertTrue(hostsResolver.getResolvedHosts()); } diff --git a/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java b/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java index b8d220704e41b..1a419e9ca7a15 100644 --- a/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java +++ b/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java @@ -26,6 +26,8 @@ import java.io.IOException; import java.io.StreamCorruptedException; +import java.util.List; +import java.util.stream.IntStream; import static org.hamcrest.core.IsInstanceOf.instanceOf; @@ -86,6 +88,18 @@ public void testRejectsPortRanges() { assertTrue(expected.getMessage().contains("Port ranges are not supported")); } + public void testCombinesHostsAndPorts() { + List actualAddresses = TcpTransport.combineHostsAndPorts( + List.of("[::1]", "127.0.0.1"), + IntStream.range(9300, 9401).toArray() + ); + List expectedAddresses = List.of( + "[::1]:9300", "[::1]:9301", "[::1]:9302", "[::1]:9303", "[::1]:9304", "[::1]:9305", + "127.0.0.1:9300", "127.0.0.1:9301", "127.0.0.1:9302", "127.0.0.1:9303", "127.0.0.1:9304", "127.0.0.1:9305" + ); + assertEquals(expectedAddresses, actualAddresses); + } + public void testDecodeWithIncompleteHeader() throws IOException { BytesStreamOutput streamOutput = new BytesStreamOutput(1 << 14); streamOutput.write('E'); diff --git a/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransport.java b/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransport.java index 0a1132edb67a9..28c4de9ee4bdc 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransport.java +++ b/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransport.java @@ -212,11 +212,6 @@ public TransportAddress[] addressesFromString(String address) { return new TransportAddress[0]; } - @Override - public int[] defaultPortRange() { - return new int[0]; - } - @Override public Lifecycle.State lifecycleState() { return null; @@ -243,7 +238,7 @@ public void close() { } @Override - public List getLocalAddresses() { + public List getDefaultSeedAddresses() { return Collections.emptyList(); } diff --git a/test/framework/src/main/java/org/elasticsearch/test/transport/StubbableTransport.java b/test/framework/src/main/java/org/elasticsearch/test/transport/StubbableTransport.java index f21a74fd2560e..6f0fed3cb4114 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/transport/StubbableTransport.java +++ b/test/framework/src/main/java/org/elasticsearch/test/transport/StubbableTransport.java @@ -120,13 +120,8 @@ public TransportAddress[] addressesFromString(String address) throws UnknownHost } @Override - public int[] defaultPortRange() { - return delegate.defaultPortRange(); - } - - @Override - public List getLocalAddresses() { - return delegate.getLocalAddresses(); + public List getDefaultSeedAddresses() { + return delegate.getDefaultSeedAddresses(); } @Override From 3262c3992d5490cd2ac0192c88675a650c8463b4 Mon Sep 17 00:00:00 2001 From: Nikita_Glashchenko Date: Mon, 29 Apr 2019 16:43:48 +0400 Subject: [PATCH 4/8] remove unused imports --- .../elasticsearch/discovery/SettingsBasedSeedHostsProvider.java | 2 -- .../discovery/SettingsBasedSeedHostsProviderTests.java | 1 - 2 files changed, 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProvider.java b/server/src/main/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProvider.java index b1e0b69b675d4..6a8cf3494ad37 100644 --- a/server/src/main/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProvider.java +++ b/server/src/main/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProvider.java @@ -27,10 +27,8 @@ import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.transport.TransportService; -import java.util.Arrays; import java.util.List; import java.util.function.Function; -import java.util.stream.Collectors; import static java.util.Collections.emptyList; diff --git a/server/src/test/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProviderTests.java b/server/src/test/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProviderTests.java index ab871614bfd1b..e4c32822003c7 100644 --- a/server/src/test/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProviderTests.java +++ b/server/src/test/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProviderTests.java @@ -28,7 +28,6 @@ import java.util.Arrays; import java.util.List; import java.util.Set; -import java.util.stream.IntStream; import static java.util.Collections.emptyList; import static org.mockito.Mockito.mock; From 58161e478f4e630d33e630f18fa13367d0ae71c9 Mon Sep 17 00:00:00 2001 From: Nikita_Glashchenko Date: Mon, 29 Apr 2019 16:53:57 +0400 Subject: [PATCH 5/8] avoid too long lines of code --- .../discovery/SettingsBasedSeedHostsProviderTests.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProviderTests.java b/server/src/test/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProviderTests.java index e4c32822003c7..226b61c002b5d 100644 --- a/server/src/test/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProviderTests.java +++ b/server/src/test/java/org/elasticsearch/discovery/SettingsBasedSeedHostsProviderTests.java @@ -57,9 +57,13 @@ boolean getResolvedHosts() { } public void testScansPortsByDefault() { - final AssertingHostsResolver hostsResolver = new AssertingHostsResolver("[::1]:9300", "[::1]:9301", "127.0.0.1:9300", "127.0.0.1:9301"); + final AssertingHostsResolver hostsResolver = new AssertingHostsResolver( + "[::1]:9300", "[::1]:9301", "127.0.0.1:9300", "127.0.0.1:9301" + ); final TransportService transportService = mock(TransportService.class); - when(transportService.getDefaultSeedAddresses()).thenReturn(Arrays.asList("[::1]:9300", "[::1]:9301", "127.0.0.1:9300", "127.0.0.1:9301")); + when(transportService.getDefaultSeedAddresses()).thenReturn( + Arrays.asList("[::1]:9300", "[::1]:9301", "127.0.0.1:9300", "127.0.0.1:9301") + ); new SettingsBasedSeedHostsProvider(Settings.EMPTY, transportService).getSeedAddresses(hostsResolver); assertTrue(hostsResolver.getResolvedHosts()); } From f90ca2cffde5d02c78b96f49c76de46b1cf937ec Mon Sep 17 00:00:00 2001 From: Nikita_Glashchenko Date: Mon, 6 May 2019 12:18:12 +0400 Subject: [PATCH 6/8] use `Integer.parseInt` instead of `new PortRange` --- .../main/java/org/elasticsearch/transport/TcpTransport.java | 6 +----- .../java/org/elasticsearch/transport/TcpTransportTests.java | 5 ++--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/transport/TcpTransport.java b/server/src/main/java/org/elasticsearch/transport/TcpTransport.java index c3f0a5bcfa013..d0b0d8bf0c33c 100644 --- a/server/src/main/java/org/elasticsearch/transport/TcpTransport.java +++ b/server/src/main/java/org/elasticsearch/transport/TcpTransport.java @@ -521,11 +521,7 @@ static TransportAddress[] parse(String hostPortString, int defaultPort) throws U if (portString == null || portString.isEmpty()) { port = defaultPort; } else { - int[] ports = new PortsRange(portString).ports(); - if (ports.length > 1) { - throw new IllegalArgumentException("Port ranges are not supported"); - } - port = ports[0]; + port = Integer.parseInt(portString); } return Arrays.stream(InetAddress.getAllByName(host)) diff --git a/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java b/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java index 1a419e9ca7a15..d10d7510a38d4 100644 --- a/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java +++ b/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java @@ -81,11 +81,10 @@ public void testParseV6WithPort() throws Exception { } public void testRejectsPortRanges() { - IllegalArgumentException expected = expectThrows( - IllegalArgumentException.class, + expectThrows( + NumberFormatException.class, () -> TcpTransport.parse("[::1]:100-200", 1000) ); - assertTrue(expected.getMessage().contains("Port ranges are not supported")); } public void testCombinesHostsAndPorts() { From 7f7cd3c4516d5777a4d02e18baad0d2d3dd92927 Mon Sep 17 00:00:00 2001 From: Nikita_Glashchenko Date: Mon, 6 May 2019 12:26:11 +0400 Subject: [PATCH 7/8] test `getDefaultSeedAddresses` properly --- .../elasticsearch/transport/TcpTransport.java | 8 +- .../transport/TcpTransportTests.java | 99 ++++++++++++++++--- 2 files changed, 90 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/transport/TcpTransport.java b/server/src/main/java/org/elasticsearch/transport/TcpTransport.java index d0b0d8bf0c33c..98e7e6bd3a7d2 100644 --- a/server/src/main/java/org/elasticsearch/transport/TcpTransport.java +++ b/server/src/main/java/org/elasticsearch/transport/TcpTransport.java @@ -322,13 +322,9 @@ public List getDefaultSeedAddresses() { if (NetworkUtils.SUPPORTS_V6) { local.add("[::1]"); // may get ports appended! } - return combineHostsAndPorts(local, defaultPortRange()); - } - - static List combineHostsAndPorts(List hosts, int[] ports) { - return hosts.stream() + return local.stream() .flatMap( - address -> Arrays.stream(ports) + address -> Arrays.stream(defaultPortRange()) .limit(LIMIT_LOCAL_PORTS_COUNT) .mapToObj(port -> address + ":" + port) ) diff --git a/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java b/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java index d10d7510a38d4..80d183e499e25 100644 --- a/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java +++ b/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java @@ -19,16 +19,25 @@ package org.elasticsearch.transport; +import org.elasticsearch.Version; +import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.common.network.NetworkService; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.TransportAddress; +import org.elasticsearch.common.util.MockPageCacheRecycler; +import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.TestThreadPool; +import org.hamcrest.Matcher; import java.io.IOException; import java.io.StreamCorruptedException; -import java.util.List; -import java.util.stream.IntStream; +import java.net.InetSocketAddress; +import java.util.Collections; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.core.IsInstanceOf.instanceOf; /** Unit tests for {@link TcpTransport} */ @@ -87,16 +96,84 @@ public void testRejectsPortRanges() { ); } - public void testCombinesHostsAndPorts() { - List actualAddresses = TcpTransport.combineHostsAndPorts( - List.of("[::1]", "127.0.0.1"), - IntStream.range(9300, 9401).toArray() - ); - List expectedAddresses = List.of( + public void testDefaultSeedAddressesWithDefaultPort() { + testDefaultSeedAddresses(Settings.EMPTY, containsInAnyOrder( "[::1]:9300", "[::1]:9301", "[::1]:9302", "[::1]:9303", "[::1]:9304", "[::1]:9305", - "127.0.0.1:9300", "127.0.0.1:9301", "127.0.0.1:9302", "127.0.0.1:9303", "127.0.0.1:9304", "127.0.0.1:9305" - ); - assertEquals(expectedAddresses, actualAddresses); + "127.0.0.1:9300", "127.0.0.1:9301", "127.0.0.1:9302", "127.0.0.1:9303", "127.0.0.1:9304", "127.0.0.1:9305")); + } + + public void testDefaultSeedAddressesWithNonstandardGlobalPortRange() { + testDefaultSeedAddresses(Settings.builder().put(TransportSettings.PORT.getKey(), "9500-9600").build(), containsInAnyOrder( + "[::1]:9500", "[::1]:9501", "[::1]:9502", "[::1]:9503", "[::1]:9504", "[::1]:9505", + "127.0.0.1:9500", "127.0.0.1:9501", "127.0.0.1:9502", "127.0.0.1:9503", "127.0.0.1:9504", "127.0.0.1:9505")); + } + + public void testDefaultSeedAddressesWithSmallGlobalPortRange() { + testDefaultSeedAddresses(Settings.builder().put(TransportSettings.PORT.getKey(), "9300-9302").build(), containsInAnyOrder( + "[::1]:9300", "[::1]:9301", "[::1]:9302", + "127.0.0.1:9300", "127.0.0.1:9301", "127.0.0.1:9302")); + } + + public void testDefaultSeedAddressesWithNonstandardProfilePortRange() { + testDefaultSeedAddresses(Settings.builder() + .put(TransportSettings.PORT_PROFILE.getConcreteSettingForNamespace(TransportSettings.DEFAULT_PROFILE).getKey(), "9500-9600") + .build(), + containsInAnyOrder( + "[::1]:9500", "[::1]:9501", "[::1]:9502", "[::1]:9503", "[::1]:9504", "[::1]:9505", + "127.0.0.1:9500", "127.0.0.1:9501", "127.0.0.1:9502", "127.0.0.1:9503", "127.0.0.1:9504", "127.0.0.1:9505")); + } + + public void testDefaultSeedAddressesWithSmallProfilePortRange() { + testDefaultSeedAddresses(Settings.builder() + .put(TransportSettings.PORT_PROFILE.getConcreteSettingForNamespace(TransportSettings.DEFAULT_PROFILE).getKey(), "9300-9302") + .build(), + containsInAnyOrder( + "[::1]:9300", "[::1]:9301", "[::1]:9302", + "127.0.0.1:9300", "127.0.0.1:9301", "127.0.0.1:9302")); + } + + public void testDefaultSeedAddressesPrefersProfileSettingToGlobalSetting() { + testDefaultSeedAddresses(Settings.builder() + .put(TransportSettings.PORT_PROFILE.getConcreteSettingForNamespace(TransportSettings.DEFAULT_PROFILE).getKey(), "9300-9302") + .put(TransportSettings.PORT.getKey(), "9500-9600") + .build(), + containsInAnyOrder( + "[::1]:9300", "[::1]:9301", "[::1]:9302", + "127.0.0.1:9300", "127.0.0.1:9301", "127.0.0.1:9302")); + } + + public void testDefaultSeedAddressesWithNonstandardSinglePort() { + testDefaultSeedAddresses(Settings.builder().put(TransportSettings.PORT.getKey(), "9500").build(), + containsInAnyOrder("[::1]:9500", "127.0.0.1:9500")); + } + + private void testDefaultSeedAddresses(final Settings settings, Matcher> seedAddressesMatcher) { + final TestThreadPool testThreadPool = new TestThreadPool("test"); + try { + final TcpTransport tcpTransport = new TcpTransport(settings, Version.CURRENT, testThreadPool, + new MockPageCacheRecycler(settings), + new NoneCircuitBreakerService(), writableRegistry(), new NetworkService(Collections.emptyList())) { + + @Override + protected TcpServerChannel bind(String name, InetSocketAddress address) { + throw new UnsupportedOperationException(); + } + + @Override + protected TcpChannel initiateChannel(DiscoveryNode node) { + throw new UnsupportedOperationException(); + } + + @Override + protected void stopInternal() { + throw new UnsupportedOperationException(); + } + }; + + assertThat(tcpTransport.getDefaultSeedAddresses(), seedAddressesMatcher); + } finally { + testThreadPool.shutdown(); + } } public void testDecodeWithIncompleteHeader() throws IOException { From a0d2917850726d511f1691f3cd99ba4d4c0d4577 Mon Sep 17 00:00:00 2001 From: Nikita_Glashchenko Date: Mon, 6 May 2019 12:36:34 +0400 Subject: [PATCH 8/8] avoid using hardcoded port setting name --- .../java/org/elasticsearch/transport/TcpTransport.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/transport/TcpTransport.java b/server/src/main/java/org/elasticsearch/transport/TcpTransport.java index 98e7e6bd3a7d2..eef9f4f42637c 100644 --- a/server/src/main/java/org/elasticsearch/transport/TcpTransport.java +++ b/server/src/main/java/org/elasticsearch/transport/TcpTransport.java @@ -471,7 +471,12 @@ public TransportAddress[] addressesFromString(String address) throws UnknownHost } private int[] defaultPortRange() { - return new PortsRange(settings.get("transport.profiles.default.port", TransportSettings.PORT.get(settings))).ports(); + return new PortsRange( + settings.get( + TransportSettings.PORT_PROFILE.getConcreteSettingForNamespace(TransportSettings.DEFAULT_PROFILE).getKey(), + TransportSettings.PORT.get(settings) + ) + ).ports(); } // this code is a take on guava's HostAndPort, like a HostAndPortRange