diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index 1c82a541ec4..1131a4a99ec 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -33,6 +33,7 @@ import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.logging.LogConfigurator; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.transport.BoundTransportAddress; import org.elasticsearch.env.Environment; import org.elasticsearch.monitor.jvm.JvmInfo; import org.elasticsearch.monitor.os.OsProbe; @@ -184,9 +185,12 @@ final class Bootstrap { .put(InternalSettingsPreparer.IGNORE_SYSTEM_PROPERTIES_SETTING.getKey(), true) .build(); - BootstrapCheck.check(nodeSettings); - - node = new Node(nodeSettings); + node = new Node(nodeSettings) { + @Override + protected void validateNodeBeforeAcceptingRequests(Settings settings, BoundTransportAddress boundTransportAddress) { + BootstrapCheck.check(settings, boundTransportAddress); + } + }; } private static Environment initialSettings(boolean foreground, String pidFile) { diff --git a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCheck.java b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCheck.java index dcb88dca848..719a968faa2 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCheck.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCheck.java @@ -22,20 +22,18 @@ package org.elasticsearch.bootstrap; import org.apache.lucene.util.Constants; import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.logging.Loggers; -import org.elasticsearch.common.network.NetworkService; -import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.transport.BoundTransportAddress; +import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.discovery.zen.elect.ElectMasterService; import org.elasticsearch.monitor.process.ProcessProbe; -import org.elasticsearch.transport.TransportSettings; +import org.elasticsearch.node.Node; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Locale; -import java.util.Set; /** * We enforce limits once any network host is configured. In this case we assume the node is running in production @@ -54,10 +52,11 @@ final class BootstrapCheck { * checks the current limits against the snapshot or release build * checks * - * @param settings the current node settings + * @param settings the current node settings + * @param boundTransportAddress the node network bindings */ - public static void check(final Settings settings) { - check(enforceLimits(settings), checks(settings)); + static void check(final Settings settings, final BoundTransportAddress boundTransportAddress) { + check(enforceLimits(boundTransportAddress), checks(settings), Node.NODE_NAME_SETTING.get(settings)); } /** @@ -67,10 +66,11 @@ final class BootstrapCheck { * @param enforceLimits true if the checks should be enforced or * warned * @param checks the checks to execute + * @param nodeName the node name to be used as a logging prefix */ // visible for testing - static void check(final boolean enforceLimits, final List checks) { - final ESLogger logger = Loggers.getLogger(BootstrapCheck.class); + static void check(final boolean enforceLimits, final List checks, final String nodeName) { + final ESLogger logger = Loggers.getLogger(BootstrapCheck.class, nodeName); for (final Check check : checks) { final boolean fail = check.check(); @@ -84,33 +84,16 @@ final class BootstrapCheck { } } - /** - * The set of settings such that if any are set for the node, then - * the checks are enforced - * - * @return the enforcement settings - */ - // visible for testing - static Set enforceSettings() { - return Collections.unmodifiableSet(new HashSet<>(Arrays.asList( - TransportSettings.BIND_HOST, - TransportSettings.HOST, - TransportSettings.PUBLISH_HOST, - NetworkService.GLOBAL_NETWORK_HOST_SETTING, - NetworkService.GLOBAL_NETWORK_BINDHOST_SETTING, - NetworkService.GLOBAL_NETWORK_PUBLISHHOST_SETTING - ))); - } - /** * Tests if the checks should be enforced * - * @param settings the current node settings + * @param boundTransportAddress the node network bindings * @return true if the checks should be enforced */ // visible for testing - static boolean enforceLimits(final Settings settings) { - return enforceSettings().stream().anyMatch(s -> s.exists(settings)); + static boolean enforceLimits(BoundTransportAddress boundTransportAddress) { + return !(Arrays.stream(boundTransportAddress.boundAddresses()).allMatch(TransportAddress::isLoopbackOrLinkLocalAddress) && + boundTransportAddress.publishAddress().isLoopbackOrLinkLocalAddress()); } // the list of checks to execute diff --git a/core/src/main/java/org/elasticsearch/common/transport/DummyTransportAddress.java b/core/src/main/java/org/elasticsearch/common/transport/DummyTransportAddress.java index 74bcfecdc69..a5912250154 100644 --- a/core/src/main/java/org/elasticsearch/common/transport/DummyTransportAddress.java +++ b/core/src/main/java/org/elasticsearch/common/transport/DummyTransportAddress.java @@ -44,6 +44,11 @@ public class DummyTransportAddress implements TransportAddress { return other == INSTANCE; } + @Override + public boolean isLoopbackOrLinkLocalAddress() { + return false; + } + @Override public String getHost() { return "dummy"; diff --git a/core/src/main/java/org/elasticsearch/common/transport/InetSocketTransportAddress.java b/core/src/main/java/org/elasticsearch/common/transport/InetSocketTransportAddress.java index 2f1284922a1..d6db17e7e24 100644 --- a/core/src/main/java/org/elasticsearch/common/transport/InetSocketTransportAddress.java +++ b/core/src/main/java/org/elasticsearch/common/transport/InetSocketTransportAddress.java @@ -74,6 +74,11 @@ public final class InetSocketTransportAddress implements TransportAddress { address.getAddress().equals(((InetSocketTransportAddress) other).address.getAddress()); } + @Override + public boolean isLoopbackOrLinkLocalAddress() { + return address.getAddress().isLinkLocalAddress() || address.getAddress().isLoopbackAddress(); + } + @Override public String getHost() { return getAddress(); // just delegate no resolving diff --git a/core/src/main/java/org/elasticsearch/common/transport/LocalTransportAddress.java b/core/src/main/java/org/elasticsearch/common/transport/LocalTransportAddress.java index a5c6171392c..c78cf62988a 100644 --- a/core/src/main/java/org/elasticsearch/common/transport/LocalTransportAddress.java +++ b/core/src/main/java/org/elasticsearch/common/transport/LocalTransportAddress.java @@ -55,6 +55,11 @@ public final class LocalTransportAddress implements TransportAddress { return other instanceof LocalTransportAddress && id.equals(((LocalTransportAddress) other).id); } + @Override + public boolean isLoopbackOrLinkLocalAddress() { + return false; + } + @Override public String getHost() { return "local"; diff --git a/core/src/main/java/org/elasticsearch/common/transport/TransportAddress.java b/core/src/main/java/org/elasticsearch/common/transport/TransportAddress.java index b5ccda41d15..f68bbfa0dd8 100644 --- a/core/src/main/java/org/elasticsearch/common/transport/TransportAddress.java +++ b/core/src/main/java/org/elasticsearch/common/transport/TransportAddress.java @@ -46,5 +46,7 @@ public interface TransportAddress extends Writeable { boolean sameHost(TransportAddress other); + boolean isLoopbackOrLinkLocalAddress(); + public String toString(); } diff --git a/core/src/main/java/org/elasticsearch/node/Node.java b/core/src/main/java/org/elasticsearch/node/Node.java index f58b2676cf5..38da5eff95a 100644 --- a/core/src/main/java/org/elasticsearch/node/Node.java +++ b/core/src/main/java/org/elasticsearch/node/Node.java @@ -266,10 +266,6 @@ public class Node implements Closeable { * Start the node. If the node is already started, this method is no-op. */ public Node start() { - if (!lifecycle.moveToStarted()) { - return this; - } - ESLogger logger = Loggers.getLogger(Node.class, NODE_NAME_SETTING.get(settings)); logger.info("starting ..."); // hack around dependency injection problem (for now...) @@ -308,10 +304,12 @@ public class Node implements Closeable { final TribeService tribeService = injector.getInstance(TribeService.class); tribeService.start(); - // Start the transport service now so the publish address will be added to the local disco node in ClusterService TransportService transportService = injector.getInstance(TransportService.class); transportService.start(); + + validateNodeBeforeAcceptingRequests(settings, transportService.boundAddress()); + DiscoveryNode localNode = injector.getInstance(DiscoveryNodeService.class) .buildLocalNode(transportService.boundAddress().publishAddress()); @@ -381,6 +379,9 @@ public class Node implements Closeable { } private Node stop() { + if (lifecycle.moveToStarted()) { + return this; + } if (!lifecycle.moveToStopped()) { return this; } @@ -521,6 +522,20 @@ public class Node implements Closeable { return this.injector; } + /** + * Hook for validating the node after network + * services are started but before the cluster service is started + * and before the network service starts accepting incoming network + * requests. + * + * @param settings the fully-resolved settings + * @param boundTransportAddress the network addresses the node is + * bound and publishing to + */ + @SuppressWarnings("unused") + protected void validateNodeBeforeAcceptingRequests(Settings settings, BoundTransportAddress boundTransportAddress) { + } + /** Writes a file to the logs dir containing the ports for the given transport type */ private void writePortsFile(String type, BoundTransportAddress boundAddress) { Path tmpPortsFile = environment.logsFile().resolve(type + ".ports.tmp"); diff --git a/core/src/test/java/org/elasticsearch/bootstrap/BootstrapCheckTests.java b/core/src/test/java/org/elasticsearch/bootstrap/BootstrapCheckTests.java index 27f26e7c8c2..28ff6b17d7e 100644 --- a/core/src/test/java/org/elasticsearch/bootstrap/BootstrapCheckTests.java +++ b/core/src/test/java/org/elasticsearch/bootstrap/BootstrapCheckTests.java @@ -20,26 +20,81 @@ package org.elasticsearch.bootstrap; import org.apache.lucene.util.Constants; -import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.transport.BoundTransportAddress; +import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.test.ESTestCase; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Set; import java.util.concurrent.atomic.AtomicLong; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.Matchers.not; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class BootstrapCheckTests extends ESTestCase { public void testNonProductionMode() { // nothing should happen since we are in non-production mode - BootstrapCheck.check(Settings.EMPTY); + final List transportAddresses = new ArrayList<>(); + for (int i = 0; i < randomIntBetween(1, 8); i++) { + TransportAddress localTransportAddress = mock(TransportAddress.class); + when(localTransportAddress.isLoopbackOrLinkLocalAddress()).thenReturn(true); + transportAddresses.add(localTransportAddress); + } + + TransportAddress publishAddress = mock(TransportAddress.class); + when(publishAddress.isLoopbackOrLinkLocalAddress()).thenReturn(true); + BoundTransportAddress boundTransportAddress = mock(BoundTransportAddress.class); + when(boundTransportAddress.boundAddresses()).thenReturn(transportAddresses.toArray(new TransportAddress[0])); + when(boundTransportAddress.publishAddress()).thenReturn(publishAddress); + BootstrapCheck.check(Settings.EMPTY, boundTransportAddress); + } + + public void testEnforceLimitsWhenBoundToNonLocalAddress() { + final List transportAddresses = new ArrayList<>(); + final TransportAddress nonLocalTransportAddress = mock(TransportAddress.class); + when(nonLocalTransportAddress.isLoopbackOrLinkLocalAddress()).thenReturn(false); + transportAddresses.add(nonLocalTransportAddress); + + for (int i = 0; i < randomIntBetween(0, 7); i++) { + final TransportAddress randomTransportAddress = mock(TransportAddress.class); + when(randomTransportAddress.isLoopbackOrLinkLocalAddress()).thenReturn(randomBoolean()); + transportAddresses.add(randomTransportAddress); + } + + final TransportAddress publishAddress = mock(TransportAddress.class); + when(publishAddress.isLoopbackOrLinkLocalAddress()).thenReturn(randomBoolean()); + + final BoundTransportAddress boundTransportAddress = mock(BoundTransportAddress.class); + Collections.shuffle(transportAddresses, random()); + when(boundTransportAddress.boundAddresses()).thenReturn(transportAddresses.toArray(new TransportAddress[0])); + when(boundTransportAddress.publishAddress()).thenReturn(publishAddress); + + assertTrue(BootstrapCheck.enforceLimits(boundTransportAddress)); + } + + public void testEnforceLimitsWhenPublishingToNonLocalAddress() { + final List transportAddresses = new ArrayList<>(); + + for (int i = 0; i < randomIntBetween(1, 8); i++) { + final TransportAddress randomTransportAddress = mock(TransportAddress.class); + when(randomTransportAddress.isLoopbackOrLinkLocalAddress()).thenReturn(false); + transportAddresses.add(randomTransportAddress); + } + + final TransportAddress publishAddress = mock(TransportAddress.class); + when(publishAddress.isLoopbackOrLinkLocalAddress()).thenReturn(true); + + final BoundTransportAddress boundTransportAddress = mock(BoundTransportAddress.class); + when(boundTransportAddress.boundAddresses()).thenReturn(transportAddresses.toArray(new TransportAddress[0])); + when(boundTransportAddress.publishAddress()).thenReturn(publishAddress); + + assertTrue(BootstrapCheck.enforceLimits(boundTransportAddress)); } public void testFileDescriptorLimits() { @@ -64,7 +119,7 @@ public class BootstrapCheckTests extends ESTestCase { } try { - BootstrapCheck.check(true, Collections.singletonList(check)); + BootstrapCheck.check(true, Collections.singletonList(check), "testFileDescriptorLimits"); fail("should have failed due to max file descriptors too low"); } catch (final RuntimeException e) { assertThat(e.getMessage(), containsString("max file descriptors")); @@ -72,12 +127,12 @@ public class BootstrapCheckTests extends ESTestCase { maxFileDescriptorCount.set(randomIntBetween(limit + 1, Integer.MAX_VALUE)); - BootstrapCheck.check(true, Collections.singletonList(check)); + BootstrapCheck.check(true, Collections.singletonList(check), "testFileDescriptorLimits"); // nothing should happen if current file descriptor count is // not available maxFileDescriptorCount.set(-1); - BootstrapCheck.check(true, Collections.singletonList(check)); + BootstrapCheck.check(true, Collections.singletonList(check), "testFileDescriptorLimits"); } public void testFileDescriptorLimitsThrowsOnInvalidLimit() { @@ -119,7 +174,7 @@ public class BootstrapCheckTests extends ESTestCase { if (testCase.shouldFail) { try { - BootstrapCheck.check(true, Collections.singletonList(check)); + BootstrapCheck.check(true, Collections.singletonList(check), "testFileDescriptorLimitsThrowsOnInvalidLimit"); fail("should have failed due to memory not being locked"); } catch (final RuntimeException e) { assertThat( @@ -128,7 +183,7 @@ public class BootstrapCheckTests extends ESTestCase { } } else { // nothing should happen - BootstrapCheck.check(true, Collections.singletonList(check)); + BootstrapCheck.check(true, Collections.singletonList(check), "testFileDescriptorLimitsThrowsOnInvalidLimit"); } } } @@ -144,7 +199,7 @@ public class BootstrapCheckTests extends ESTestCase { }; try { - BootstrapCheck.check(true, Collections.singletonList(check)); + BootstrapCheck.check(true, Collections.singletonList(check), "testMaxNumberOfThreadsCheck"); fail("should have failed due to max number of threads too low"); } catch (final RuntimeException e) { assertThat(e.getMessage(), containsString("max number of threads")); @@ -152,12 +207,12 @@ public class BootstrapCheckTests extends ESTestCase { maxNumberOfThreads.set(randomIntBetween(limit + 1, Integer.MAX_VALUE)); - BootstrapCheck.check(true, Collections.singletonList(check)); + BootstrapCheck.check(true, Collections.singletonList(check), "testMaxNumberOfThreadsCheck"); // nothing should happen if current max number of threads is // not available maxNumberOfThreads.set(-1); - BootstrapCheck.check(true, Collections.singletonList(check)); + BootstrapCheck.check(true, Collections.singletonList(check), "testMaxNumberOfThreadsCheck"); } public void testMaxSizeVirtualMemory() { @@ -176,7 +231,7 @@ public class BootstrapCheckTests extends ESTestCase { }; try { - BootstrapCheck.check(true, Collections.singletonList(check)); + BootstrapCheck.check(true, Collections.singletonList(check), "testMaxSizeVirtualMemory"); fail("should have failed due to max size virtual memory too low"); } catch (final RuntimeException e) { assertThat(e.getMessage(), containsString("max size virtual memory")); @@ -184,19 +239,12 @@ public class BootstrapCheckTests extends ESTestCase { maxSizeVirtualMemory.set(rlimInfinity); - BootstrapCheck.check(true, Collections.singletonList(check)); + BootstrapCheck.check(true, Collections.singletonList(check), "testMaxSizeVirtualMemory"); // nothing should happen if max size virtual memory is not // available maxSizeVirtualMemory.set(Long.MIN_VALUE); - BootstrapCheck.check(true, Collections.singletonList(check)); - } - - public void testEnforceLimits() { - final Set enforceSettings = BootstrapCheck.enforceSettings(); - final Setting setting = randomFrom(Arrays.asList(enforceSettings.toArray(new Setting[enforceSettings.size()]))); - final Settings settings = Settings.builder().put(setting.getKey(), randomAsciiOfLength(8)).build(); - assertTrue(BootstrapCheck.enforceLimits(settings)); + BootstrapCheck.check(true, Collections.singletonList(check), "testMaxSizeVirtualMemory"); } public void testMinMasterNodes() { @@ -205,6 +253,7 @@ public class BootstrapCheckTests extends ESTestCase { assertThat(check.check(), not(equalTo(isSet))); List defaultChecks = BootstrapCheck.checks(Settings.EMPTY); - expectThrows(RuntimeException.class, () -> BootstrapCheck.check(true, defaultChecks)); + expectThrows(RuntimeException.class, () -> BootstrapCheck.check(true, defaultChecks, "testMinMasterNodes")); } + }