SONAR-14039 replace 'sonar.search.transportPort' with 'sonar.es.port' on non-DCE

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<String, String> 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.