From 874973829ca3a7ba8e39709d478695216357cf97 Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 27 Oct 2020 11:20:27 +0100 Subject: [PATCH] SONAR-14039 replace 'sonar.search.transportPort' with 'sonar.es.port' on non-DCE --- .../org/sonar/application/es/EsSettings.java | 13 ++-- .../sonar/application/es/EsSettingsTest.java | 26 ++++---- .../org/sonar/process/ProcessProperties.java | 16 +++-- .../sonar/process/ProcessPropertiesTest.java | 64 ++++++++++++------- .../sonar/server/es/EsClientProviderTest.java | 4 +- .../src/main/assembly/conf/sonar.properties | 4 ++ 6 files changed, 81 insertions(+), 46 deletions(-) diff --git a/server/sonar-main/src/main/java/org/sonar/application/es/EsSettings.java b/server/sonar-main/src/main/java/org/sonar/application/es/EsSettings.java index ff3765e1639..51323964cae 100644 --- a/server/sonar-main/src/main/java/org/sonar/application/es/EsSettings.java +++ b/server/sonar-main/src/main/java/org/sonar/application/es/EsSettings.java @@ -41,10 +41,10 @@ import static org.sonar.process.ProcessProperties.Property.CLUSTER_NODE_ES_PORT; import static org.sonar.process.ProcessProperties.Property.CLUSTER_NODE_NAME; import static org.sonar.process.ProcessProperties.Property.CLUSTER_NODE_SEARCH_HOST; import static org.sonar.process.ProcessProperties.Property.CLUSTER_NODE_SEARCH_PORT; +import static org.sonar.process.ProcessProperties.Property.ES_PORT; import static org.sonar.process.ProcessProperties.Property.SEARCH_HOST; import static org.sonar.process.ProcessProperties.Property.SEARCH_INITIAL_STATE_TIMEOUT; import static org.sonar.process.ProcessProperties.Property.SEARCH_PORT; -import static org.sonar.process.ProcessProperties.Property.SEARCH_TRANSPORT_PORT; public class EsSettings { private static final String ES_HTTP_HOST_KEY = "http.host"; @@ -91,7 +91,9 @@ public class EsSettings { configureNetwork(builder); configureCluster(builder); configureOthers(builder); - LOGGER.info("Elasticsearch listening on {}:{}", builder.get(ES_HTTP_HOST_KEY), builder.get(ES_HTTP_PORT_KEY)); + LOGGER.info("Elasticsearch listening on [HTTP: {}:{}, TCP: {}:{}]", + builder.get(ES_HTTP_HOST_KEY), builder.get(ES_HTTP_PORT_KEY), + builder.get(ES_TRANSPORT_HOST_KEY), builder.get(ES_TRANSPORT_PORT_KEY)); return builder; } @@ -108,9 +110,10 @@ public class EsSettings { builder.put(ES_HTTP_PORT_KEY, valueOf(searchPort)); builder.put(ES_NETWORK_HOST_KEY, valueOf(searchHost.getHostAddress())); - // FIXME remove definition of transport properties when Web and CE have moved to ES Rest client - int transportPort = props.valueAsInt(SEARCH_TRANSPORT_PORT.getKey(), 9002); - builder.put(ES_TRANSPORT_HOST_KEY, searchHost.getHostAddress()); + int transportPort = Integer.parseInt(props.nonNullValue(ES_PORT.getKey())); + + //we have no use of transport port in non-DCE editions + builder.put(ES_TRANSPORT_HOST_KEY, this.loopbackAddress.getHostAddress()); builder.put(ES_TRANSPORT_PORT_KEY, valueOf(transportPort)); } diff --git a/server/sonar-main/src/test/java/org/sonar/application/es/EsSettingsTest.java b/server/sonar-main/src/test/java/org/sonar/application/es/EsSettingsTest.java index 765feb6718e..f3722d1332a 100644 --- a/server/sonar-main/src/test/java/org/sonar/application/es/EsSettingsTest.java +++ b/server/sonar-main/src/test/java/org/sonar/application/es/EsSettingsTest.java @@ -56,6 +56,7 @@ import static org.sonar.process.ProcessProperties.Property.CLUSTER_NODE_ES_PORT; import static org.sonar.process.ProcessProperties.Property.CLUSTER_NODE_NAME; import static org.sonar.process.ProcessProperties.Property.CLUSTER_NODE_SEARCH_HOST; import static org.sonar.process.ProcessProperties.Property.CLUSTER_NODE_SEARCH_PORT; +import static org.sonar.process.ProcessProperties.Property.ES_PORT; import static org.sonar.process.ProcessProperties.Property.PATH_DATA; import static org.sonar.process.ProcessProperties.Property.PATH_HOME; import static org.sonar.process.ProcessProperties.Property.PATH_LOGS; @@ -131,6 +132,7 @@ public class EsSettingsTest { File homeDir = temp.newFolder(); Props props = new Props(new Properties()); props.set(SEARCH_PORT.getKey(), "1234"); + props.set(ES_PORT.getKey(), "5678"); props.set(SEARCH_HOST.getKey(), "127.0.0.1"); props.set(PATH_HOME.getKey(), homeDir.getAbsolutePath()); props.set(PATH_DATA.getKey(), temp.newFolder().getAbsolutePath()); @@ -142,16 +144,13 @@ public class EsSettingsTest { Map generated = esSettings.build(); - // FIXME transport.port and transport.host should not be set in standalone - assertThat(generated.get("transport.port")).isEqualTo("9002"); - assertThat(generated.get("transport.host")).isEqualTo("127.0.0.1"); - - assertThat(generated.get("http.port")).isEqualTo("1234"); - assertThat(generated.get("http.host")).isEqualTo("127.0.0.1"); - - // no cluster, but cluster and node names are set though - assertThat(generated.get("cluster.name")).isEqualTo("sonarqube"); - assertThat(generated.get("node.name")).isEqualTo("sonarqube"); + assertThat(generated).containsEntry("transport.port", "5678") + .containsEntry("transport.host", "127.0.0.1") + .containsEntry("http.port", "1234") + .containsEntry("http.host", "127.0.0.1") + // no cluster, but cluster and node names are set though + .containsEntry("cluster.name", "sonarqube") + .containsEntry("node.name", "sonarqube"); assertThat(generated.get("path.data")).isNotNull(); assertThat(generated.get("path.logs")).isNotNull(); @@ -159,9 +158,9 @@ public class EsSettingsTest { assertThat(generated.get("path.conf")).isNull(); assertThat(generated.get("discovery.seed_hosts")).isNull(); - assertThat(generated.get("discovery.initial_state_timeout")).isEqualTo("30s"); - - assertThat(generated.get("action.auto_create_index")).isEqualTo("false"); + assertThat(generated) + .containsEntry("discovery.initial_state_timeout", "30s") + .containsEntry("action.auto_create_index", "false"); } @Test @@ -214,6 +213,7 @@ public class EsSettingsTest { props.set(CLUSTER_NAME.getKey(), "sonarqube"); props.set(Property.CLUSTER_ENABLED.getKey(), "false"); props.set(SEARCH_PORT.getKey(), "1234"); + props.set(ES_PORT.getKey(), "5678"); props.set(SEARCH_HOST.getKey(), "127.0.0.1"); props.set(PATH_HOME.getKey(), homeDir.getAbsolutePath()); props.set(PATH_DATA.getKey(), temp.newFolder().getAbsolutePath()); diff --git a/server/sonar-process/src/main/java/org/sonar/process/ProcessProperties.java b/server/sonar-process/src/main/java/org/sonar/process/ProcessProperties.java index a1e7089900f..7038fc4952c 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/ProcessProperties.java +++ b/server/sonar-process/src/main/java/org/sonar/process/ProcessProperties.java @@ -36,6 +36,7 @@ import org.sonar.core.extension.ServiceLoaderWrapper; import static com.google.common.base.Preconditions.checkState; import static java.lang.String.format; import static org.sonar.process.ProcessProperties.Property.CLUSTER_ENABLED; +import static org.sonar.process.ProcessProperties.Property.ES_PORT; import static org.sonar.process.ProcessProperties.Property.SEARCH_HOST; import static org.sonar.process.ProcessProperties.Property.SEARCH_PORT; @@ -77,8 +78,7 @@ public class ProcessProperties { SEARCH_HOST("sonar.search.host"), SEARCH_PORT("sonar.search.port"), - // FIXME default is 0 until we move out of usage of TransportClient and we can put the expected default: 9002 - SEARCH_TRANSPORT_PORT("sonar.search.transportPort", "0"), + ES_PORT("sonar.es.port"), SEARCH_JAVA_OPTS("sonar.search.javaOpts", "-Xmx512m -Xms512m -XX:MaxDirectMemorySize=256m -XX:+HeapDumpOnOutOfMemoryError"), SEARCH_JAVA_ADDITIONAL_OPTS("sonar.search.javaAdditionalOpts", ""), SEARCH_REPLICAS("sonar.search.replicas"), @@ -239,8 +239,7 @@ public class ProcessProperties { props.setDefault(SEARCH_HOST.getKey(), InetAddress.getLoopbackAddress().getHostAddress()); props.setDefault(SEARCH_PORT.getKey(), "9001"); fixPortIfZero(props, Property.SEARCH_HOST.getKey(), SEARCH_PORT.getKey()); - // FIXME remove when transport is not used anymore in non-DCE editions: sonar.search.transportPort must not support port 0 - fixPortIfZero(props, Property.SEARCH_HOST.getKey(), Property.SEARCH_TRANSPORT_PORT.getKey()); + fixEsTransportPortIfNull(props); } } @@ -278,6 +277,15 @@ public class ProcessProperties { } } + private static void fixEsTransportPortIfNull(Props props) { + String port = props.value(ES_PORT.getKey()); + if (port == null) { + int allocatedPort = NetworkUtilsImpl.INSTANCE.getNextAvailablePort(InetAddress.getLoopbackAddress().getHostAddress()) + .orElseThrow(() -> new IllegalStateException("Cannot resolve address for Elasticsearch TCP transport port")); + props.set(ES_PORT.getKey(), String.valueOf(allocatedPort)); + } + } + public static long parseTimeoutMs(Property property, String value) { long l = Long.parseLong(value); checkState(l >= 1, "value of %s must be >= 1", property); diff --git a/server/sonar-process/src/test/java/org/sonar/process/ProcessPropertiesTest.java b/server/sonar-process/src/test/java/org/sonar/process/ProcessPropertiesTest.java index 9141871fb42..5e894790d0a 100644 --- a/server/sonar-process/src/test/java/org/sonar/process/ProcessPropertiesTest.java +++ b/server/sonar-process/src/test/java/org/sonar/process/ProcessPropertiesTest.java @@ -25,9 +25,7 @@ import java.net.InetAddress; import java.util.Map; import java.util.Properties; import java.util.Random; -import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.sonar.core.extension.CoreExtension; import org.sonar.core.extension.ServiceLoaderWrapper; @@ -38,8 +36,6 @@ import static org.mockito.Mockito.when; import static org.sonar.process.ProcessProperties.parseTimeoutMs; public class ProcessPropertiesTest { - @Rule - public ExpectedException expectedException = ExpectedException.none(); private final ServiceLoaderWrapper serviceLoaderWrapper = mock(ServiceLoaderWrapper.class); private final ProcessProperties processProperties = new ProcessProperties(serviceLoaderWrapper); @@ -52,8 +48,8 @@ public class ProcessPropertiesTest { assertThat(props.value("sonar.search.javaOpts")).contains("-Xmx"); assertThat(props.valueAsInt("sonar.jdbc.maxActive")).isEqualTo(60); - assertThat(props.valueAsBoolean("sonar.sonarcloud.enabled")).isEqualTo(false); - assertThat(props.valueAsBoolean("sonar.updatecenter.activate")).isEqualTo(true); + assertThat(props.valueAsBoolean("sonar.sonarcloud.enabled")).isFalse(); + assertThat(props.valueAsBoolean("sonar.updatecenter.activate")).isTrue(); } @Test @@ -68,7 +64,7 @@ public class ProcessPropertiesTest { } @Test - public void completeDefaults_sets_default_values_for_sonar_search_host_and_sonar_search_port_in_non_cluster_mode() throws Exception { + public void completeDefaults_sets_default_values_for_sonar_search_host_and_sonar_search_port_and_random_port_for_sonar_es_port_in_non_cluster_mode() throws Exception { Properties p = new Properties(); p.setProperty("sonar.cluster.enabled", "false"); Props props = new Props(p); @@ -79,10 +75,11 @@ public class ProcessPropertiesTest { assertThat(address).isNotEmpty(); assertThat(InetAddress.getByName(address).isLoopbackAddress()).isTrue(); assertThat(props.valueAsInt("sonar.search.port")).isEqualTo(9001); + assertThat(props.valueAsInt("sonar.es.port")).isPositive(); } @Test - public void completeDefaults_does_not_set_default_values_for_sonar_search_host_and_sonar_search_port_in_cluster_mode() { + public void completeDefaults_does_not_set_default_values_for_sonar_search_host_and_sonar_search_port_and_sonar_es_port_in_cluster_mode() { Properties p = new Properties(); p.setProperty("sonar.cluster.enabled", "true"); Props props = new Props(p); @@ -91,10 +88,11 @@ public class ProcessPropertiesTest { assertThat(props.contains("sonar.search.port")).isFalse(); assertThat(props.contains("sonar.search.port")).isFalse(); + assertThat(props.contains("sonar.es.port")).isFalse(); } @Test - public void completeDefaults_sets_the_transport_port_of_elasticsearch_if_value_is_zero_in_cluster_mode() { + public void completeDefaults_sets_the_http_port_of_elasticsearch_if_value_is_zero_in_standalone_mode() { Properties p = new Properties(); p.setProperty("sonar.search.port", "0"); Props props = new Props(p); @@ -105,14 +103,38 @@ public class ProcessPropertiesTest { } @Test - public void completeDefaults_sets_the_search_port_of_elasticsearch_if_value_is_zero_in_search_node_in_cluster() { + public void completeDefaults_does_not_fall_back_to_default_if_transport_port_of_elasticsearch_set_in_standalone_mode() { Properties p = new Properties(); + p.setProperty("sonar.es.port", "9002"); + Props props = new Props(p); + + processProperties.completeDefaults(props); + + assertThat(props.valueAsInt("sonar.es.port")).isEqualTo(9002); + } + + @Test + public void completeDefaults_does_not_set_the_http_port_of_elasticsearch_if_value_is_zero_in_search_node_in_cluster() { + Properties p = new Properties(); + p.setProperty("sonar.cluster.enabled", "true"); p.setProperty("sonar.search.port", "0"); Props props = new Props(p); processProperties.completeDefaults(props); - assertThat(props.valueAsInt("sonar.search.port")).isPositive(); + assertThat(props.valueAsInt("sonar.search.port")).isZero(); + } + + @Test + public void completeDefaults_does_not_set_the_transport_port_of_elasticsearch_if_value_is_zero_in_search_node_in_cluster() { + Properties p = new Properties(); + p.setProperty("sonar.cluster.enabled", "true"); + p.setProperty("sonar.es.port", "0"); + Props props = new Props(p); + + processProperties.completeDefaults(props); + + assertThat(props.valueAsInt("sonar.es.port")).isZero(); } @Test @@ -202,9 +224,8 @@ public class ProcessPropertiesTest { @Test public void parseTimeoutMs_throws_NumberFormat_exception_if_value_is_not_long() { - expectedException.expect(NumberFormatException.class); - - parseTimeoutMs(ProcessProperties.Property.WEB_GRACEFUL_STOP_TIMEOUT, "tru"); + assertThatThrownBy(() -> parseTimeoutMs(ProcessProperties.Property.WEB_GRACEFUL_STOP_TIMEOUT, "tru")) + .isInstanceOf(NumberFormatException.class); } @Test @@ -218,17 +239,16 @@ public class ProcessPropertiesTest { @Test public void parseTimeoutMs_throws_ISE_if_value_is_0() { - expectedException.expect(IllegalStateException.class); - expectedException.expectMessage("value of WEB_GRACEFUL_STOP_TIMEOUT must be >= 1"); - - parseTimeoutMs(ProcessProperties.Property.WEB_GRACEFUL_STOP_TIMEOUT, 0 + ""); + assertThatThrownBy(() -> parseTimeoutMs(ProcessProperties.Property.WEB_GRACEFUL_STOP_TIMEOUT, 0 + "")) + .isInstanceOf(IllegalStateException.class) + .hasMessage("value of WEB_GRACEFUL_STOP_TIMEOUT must be >= 1"); } @Test public void parseTimeoutMs_throws_ISE_if_value_is_less_than_0() { - expectedException.expect(IllegalStateException.class); - expectedException.expectMessage("value of WEB_GRACEFUL_STOP_TIMEOUT must be >= 1"); - - parseTimeoutMs(ProcessProperties.Property.WEB_GRACEFUL_STOP_TIMEOUT, -(1 + new Random().nextInt(5_999_663)) + ""); + int timeoutValue = -(1 + new Random().nextInt(5_999_663)); + assertThatThrownBy(() -> parseTimeoutMs(ProcessProperties.Property.WEB_GRACEFUL_STOP_TIMEOUT, timeoutValue + "")) + .isInstanceOf(IllegalStateException.class) + .hasMessage("value of WEB_GRACEFUL_STOP_TIMEOUT must be >= 1"); } } diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/es/EsClientProviderTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/es/EsClientProviderTest.java index 3ec60eb6237..1a681191a58 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/es/EsClientProviderTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/es/EsClientProviderTest.java @@ -37,9 +37,9 @@ import static org.sonar.process.ProcessProperties.Property.CLUSTER_ENABLED; import static org.sonar.process.ProcessProperties.Property.CLUSTER_NAME; import static org.sonar.process.ProcessProperties.Property.CLUSTER_NODE_TYPE; import static org.sonar.process.ProcessProperties.Property.CLUSTER_SEARCH_HOSTS; +import static org.sonar.process.ProcessProperties.Property.ES_PORT; import static org.sonar.process.ProcessProperties.Property.SEARCH_HOST; import static org.sonar.process.ProcessProperties.Property.SEARCH_PORT; -import static org.sonar.process.ProcessProperties.Property.SEARCH_TRANSPORT_PORT; public class EsClientProviderTest { @@ -66,7 +66,7 @@ public class EsClientProviderTest { settings.setProperty(CLUSTER_ENABLED.getKey(), false); settings.setProperty(SEARCH_HOST.getKey(), localhostHostname); settings.setProperty(SEARCH_PORT.getKey(), 9000); - settings.setProperty(SEARCH_TRANSPORT_PORT.getKey(), 8080); + settings.setProperty(ES_PORT.getKey(), 8080); EsClient client = underTest.provide(settings.asConfig()); RestHighLevelClient nativeClient = client.nativeClient(); diff --git a/sonar-application/src/main/assembly/conf/sonar.properties b/sonar-application/src/main/assembly/conf/sonar.properties index 6ca7452a06e..109cafd5a12 100644 --- a/sonar-application/src/main/assembly/conf/sonar.properties +++ b/sonar-application/src/main/assembly/conf/sonar.properties @@ -275,6 +275,10 @@ # As a security precaution, should be blocked by a firewall and not exposed to the Internet. #sonar.search.port=9001 +# Elasticsearch TCP transport port that is bound to loopback address. When nothing is set, a random port will be chosen. +# As a security precaution, your OS configuration should not expose this port for external access. +#sonar.es.port= + # Elasticsearch host. The search server will bind this address and the search client will connect to it. # Default is loopback address. # As a security precaution, should NOT be set to a publicly available address. -- 2.39.5