SONAR-8351 Configure HTTP proxy URL from bootstrap process

Proxy is configured in conf/sonar.properties. The related properties
are propagated to system properties by the first call  to DefaultHttpDownloader.
The correct approach is to directly configure the JVM properties when
the bootstrap process creates web server and CE processes.

5 files changed, 116 insertions, 109 deletions

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 0a8a46578de..197887d6eab 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
@@ -73,6 +73,13 @@ public class ProcessProperties {
 
   public static final String CE_ENFORCED_JVM_ARGS = "-Djava.awt.headless=true -Dfile.encoding=UTF-8";
 
+  public static final String HTTP_PROXY_HOST = "http.proxyHost";
+  public static final String HTTPS_PROXY_HOST = "https.proxyHost";
+  public static final String HTTP_PROXY_PORT = "http.proxyPort";
+  public static final String HTTPS_PROXY_PORT = "https.proxyPort";
+  public static final String HTTP_PROXY_USER = "http.proxyUser";
+  public static final String HTTP_PROXY_PASSWORD = "http.proxyPassword";
+
   private ProcessProperties() {
     // only static stuff
   }
diff --git a/sonar-application/src/main/java/org/sonar/application/App.java b/sonar-application/src/main/java/org/sonar/application/App.java
index 7ec6052e183..35276c6c71b 100644
--- a/sonar-application/src/main/java/org/sonar/application/App.java
+++ b/sonar-application/src/main/java/org/sonar/application/App.java
@@ -32,12 +32,29 @@ import org.sonar.process.monitor.JavaCommand;
 import org.sonar.process.monitor.Monitor;
 
 import static org.sonar.process.ProcessId.APP;
+import static org.sonar.process.ProcessProperties.HTTPS_PROXY_HOST;
+import static org.sonar.process.ProcessProperties.HTTPS_PROXY_PORT;
+import static org.sonar.process.ProcessProperties.HTTP_PROXY_HOST;
+import static org.sonar.process.ProcessProperties.HTTP_PROXY_PORT;
 
 /**
  * Entry-point of process that starts and monitors ElasticSearch, the Web Server and the Compute Engine.
  */
 public class App implements Stoppable {
 
+  /**
+   * Properties about proxy that must be set as system properties
+   */
+  private static final String[] PROXY_PROPERTY_KEYS = new String[]{
+    HTTP_PROXY_HOST,
+    HTTP_PROXY_PORT,
+    "http.nonProxyHosts",
+    HTTPS_PROXY_HOST,
+    HTTPS_PROXY_PORT,
+    "http.auth.ntlm.domain",
+    "socksProxyHost",
+    "socksProxyPort"};
+
   private final Monitor monitor;
 
   public App(AppFileSystem appFileSystem, boolean watchForHardStop) {
@@ -77,22 +94,17 @@ public class App implements Stoppable {
   }
 
   private static JavaCommand createESCommand(Props props, File homeDir) {
-    JavaCommand elasticsearch = new JavaCommand(ProcessId.ELASTICSEARCH);
-    elasticsearch
-      .setWorkDir(homeDir)
+    return newJavaCommand(ProcessId.ELASTICSEARCH, props, homeDir)
       .addJavaOptions("-Djava.awt.headless=true")
       .addJavaOptions(props.nonNullValue(ProcessProperties.SEARCH_JAVA_OPTS))
       .addJavaOptions(props.nonNullValue(ProcessProperties.SEARCH_JAVA_ADDITIONAL_OPTS))
       .setClassName("org.sonar.search.SearchServer")
-      .setArguments(props.rawProperties())
       .addClasspath("./lib/common/*")
       .addClasspath("./lib/search/*");
-    return elasticsearch;
   }
 
   private static JavaCommand createWebServerCommand(Props props, File homeDir) {
-    JavaCommand webServer = new JavaCommand(ProcessId.WEB_SERVER)
-      .setWorkDir(homeDir)
+    JavaCommand command = newJavaCommand(ProcessId.WEB_SERVER, props, homeDir)
       .addJavaOptions(ProcessProperties.WEB_ENFORCED_JVM_ARGS)
       .addJavaOptions(props.nonNullValue(ProcessProperties.WEB_JAVA_OPTS))
       .addJavaOptions(props.nonNullValue(ProcessProperties.WEB_JAVA_ADDITIONAL_OPTS))
@@ -101,32 +113,51 @@ public class App implements Stoppable {
       // ensure JRuby uses SQ's temp directory as temp directory (eg. for temp files used during HTTP uploads)
       .setEnvVariable("TMPDIR", props.nonNullValue(ProcessProperties.PATH_TEMP))
       .setClassName("org.sonar.server.app.WebServer")
-      .setArguments(props.rawProperties())
       .addClasspath("./lib/common/*")
       .addClasspath("./lib/server/*");
     String driverPath = props.value(ProcessProperties.JDBC_DRIVER_PATH);
     if (driverPath != null) {
-      webServer.addClasspath(driverPath);
+      command.addClasspath(driverPath);
     }
-    return webServer;
+    return command;
   }
 
   private static JavaCommand createCeServerCommand(Props props, File homeDir) {
-    JavaCommand webServer = new JavaCommand(ProcessId.COMPUTE_ENGINE)
-      .setWorkDir(homeDir)
+    JavaCommand command = newJavaCommand(ProcessId.COMPUTE_ENGINE, props, homeDir)
       .addJavaOptions(ProcessProperties.CE_ENFORCED_JVM_ARGS)
       .addJavaOptions(props.nonNullValue(ProcessProperties.CE_JAVA_OPTS))
       .addJavaOptions(props.nonNullValue(ProcessProperties.CE_JAVA_ADDITIONAL_OPTS))
       .setClassName("org.sonar.ce.app.CeServer")
-      .setArguments(props.rawProperties())
       .addClasspath("./lib/common/*")
       .addClasspath("./lib/server/*")
       .addClasspath("./lib/ce/*");
     String driverPath = props.value(ProcessProperties.JDBC_DRIVER_PATH);
     if (driverPath != null) {
-      webServer.addClasspath(driverPath);
+      command.addClasspath(driverPath);
+    }
+    return command;
+  }
+
+  private static JavaCommand newJavaCommand(ProcessId id, Props props, File homeDir) {
+    JavaCommand command = new JavaCommand(id)
+      .setWorkDir(homeDir)
+      .setArguments(props.rawProperties());
+
+    for (String key : PROXY_PROPERTY_KEYS) {
+      if (props.contains(key)) {
+        command.addJavaOption("-D" + key + "=" + props.value(key));
+      }
+    }
+    // defaults of HTTPS are the same than HTTP defaults
+    setSystemPropertyToDefaultIfNotSet(command, props, HTTPS_PROXY_HOST, HTTP_PROXY_HOST);
+    setSystemPropertyToDefaultIfNotSet(command, props, HTTPS_PROXY_PORT, HTTP_PROXY_PORT);
+    return command;
+  }
+
+  private static void setSystemPropertyToDefaultIfNotSet(JavaCommand command, Props props, String httpsProperty, String httpProperty) {
+    if (!props.contains(httpsProperty) && props.contains(httpProperty)) {
+      command.addJavaOption("-D" + httpsProperty + "=" + props.value(httpProperty));
     }
-    return webServer;
   }
 
   static String starPath(File homeDir, String relativePath) {
diff --git a/sonar-application/src/test/java/org/sonar/application/AppTest.java b/sonar-application/src/test/java/org/sonar/application/AppTest.java
index 9a0ae277836..1c1ac6ab110 100644
--- a/sonar-application/src/test/java/org/sonar/application/AppTest.java
+++ b/sonar-application/src/test/java/org/sonar/application/AppTest.java
@@ -124,6 +124,57 @@ public class AppTest {
     assertThat(commands.get(1).getEnvVariables()).contains(entry("TMPDIR", expectedTmpDir));
   }
 
+  @Test
+  public void configure_http_and_https_proxies_on_all_processes() throws Exception {
+    Props props = initDefaultProps();
+    // These properties can be defined in conf/sonar.properties.
+    // They must be propagated to JVM.
+    props.set("http.proxyHost", "1.2.3.4");
+    props.set("http.proxyPort", "80");
+    props.set("https.proxyHost", "5.6.7.8");
+    props.set("https.proxyPort", "443");
+
+    List<JavaCommand> commands = start(props);
+    assertThat(commands).isNotEmpty();
+
+    for (JavaCommand command : commands) {
+      assertThat(command.getJavaOptions()).contains("-Dhttp.proxyHost=1.2.3.4");
+      assertThat(command.getJavaOptions()).contains("-Dhttp.proxyPort=80");
+      assertThat(command.getJavaOptions()).contains("-Dhttps.proxyHost=5.6.7.8");
+      assertThat(command.getJavaOptions()).contains("-Dhttps.proxyPort=443");
+    }
+  }
+
+  @Test
+  public void https_proxy_defaults_are_http_proxy_properties() throws Exception {
+    Props props = initDefaultProps();
+    props.set("http.proxyHost", "1.2.3.4");
+    props.set("http.proxyPort", "80");
+
+    List<JavaCommand> commands = start(props);
+    assertThat(commands).isNotEmpty();
+
+    for (JavaCommand command : commands) {
+      assertThat(command.getJavaOptions()).contains("-Dhttp.proxyHost=1.2.3.4");
+      assertThat(command.getJavaOptions()).contains("-Dhttp.proxyPort=80");
+      assertThat(command.getJavaOptions()).contains("-Dhttps.proxyHost=1.2.3.4");
+      assertThat(command.getJavaOptions()).contains("-Dhttps.proxyPort=80");
+    }
+  }
+
+  @Test
+  public void no_http_proxy_settings_by_default() throws Exception {
+    List<JavaCommand> commands = start(initDefaultProps());
+
+    assertThat(commands).isNotEmpty();
+    for (JavaCommand command : commands) {
+      assertThat(command.getJavaOptions()).doesNotContain("http.proxyHost");
+      assertThat(command.getJavaOptions()).doesNotContain("https.proxyHost");
+      assertThat(command.getJavaOptions()).doesNotContain("http.proxyPort");
+      assertThat(command.getJavaOptions()).doesNotContain("https.proxyPort");
+    }
+  }
+
   private Props initDefaultProps() throws IOException {
     Props props = new Props(new Properties());
     ProcessProperties.completeDefaults(props);
diff --git a/sonar-core/src/main/java/org/sonar/core/util/DefaultHttpDownloader.java b/sonar-core/src/main/java/org/sonar/core/util/DefaultHttpDownloader.java
index ed174aff4c9..fe8e60475fc 100644
--- a/sonar-core/src/main/java/org/sonar/core/util/DefaultHttpDownloader.java
+++ b/sonar-core/src/main/java/org/sonar/core/util/DefaultHttpDownloader.java
@@ -22,7 +22,6 @@ package org.sonar.core.util;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import com.google.common.io.ByteStreams;
 import java.io.File;
@@ -75,7 +74,7 @@ public class DefaultHttpDownloader extends HttpDownloader {
   public DefaultHttpDownloader(Server server, Settings settings, @Nullable Integer connectTimeout, @Nullable Integer readTimeout) {
     this.readTimeout = readTimeout;
     this.connectTimeout = connectTimeout;
-    downloader = new BaseHttpDownloader(new SystemFacade(), settings, server.getVersion());
+    downloader = new BaseHttpDownloader(new AuthenticatorFacade(), settings, server.getVersion());
   }
 
   public DefaultHttpDownloader(Settings settings) {
@@ -89,7 +88,7 @@ public class DefaultHttpDownloader extends HttpDownloader {
   public DefaultHttpDownloader(Settings settings, @Nullable Integer connectTimeout, @Nullable Integer readTimeout) {
     this.readTimeout = readTimeout;
     this.connectTimeout = connectTimeout;
-    downloader = new BaseHttpDownloader(new SystemFacade(), settings, null);
+    downloader = new BaseHttpDownloader(new AuthenticatorFacade(), settings, null);
   }
 
   @Override
@@ -158,16 +157,10 @@ public class DefaultHttpDownloader extends HttpDownloader {
   }
 
   /**
-   * Facade to allow unit tests to verify calls to {@link System}.
-   * This class could be replaced by {@link org.sonar.api.utils.System2},
-   * but it sounds overkill to define {@link #setDefaultAuthenticator(Authenticator)}.
+   * Facade to allow unit tests to verify calls to {@link Authenticator#setDefault(Authenticator)}.
    */
-  static class SystemFacade {
-    public void setProperty(String key, String value) {
-      System.setProperty(key, value);
-    }
-
-    public void setDefaultAuthenticator(Authenticator authenticator) {
+  static class AuthenticatorFacade {
+    void setDefaultAuthenticator(Authenticator authenticator) {
       Authenticator.setDefault(authenticator);
     }
   }
@@ -177,34 +170,15 @@ public class DefaultHttpDownloader extends HttpDownloader {
     private static final String GET = "GET";
     private static final String HTTP_PROXY_USER = "http.proxyUser";
     private static final String HTTP_PROXY_PASSWORD = "http.proxyPassword";
-    private static final String HTTP_PROXY_HOST = "http.proxyHost";
-    private static final String HTTPS_PROXY_HOST = "https.proxyHost";
-    private static final String HTTP_PROXY_PORT = "http.proxyPort";
-    private static final String HTTPS_PROXY_PORT = "https.proxyPort";
-
-    private static final List<String> PROXY_SETTINGS = ImmutableList.of(
-      HTTP_PROXY_HOST, HTTP_PROXY_PORT, "http.nonProxyHosts",
-      HTTPS_PROXY_HOST, HTTPS_PROXY_PORT,
-      "http.auth.ntlm.domain", "socksProxyHost", "socksProxyPort");
 
     private String userAgent;
 
-    BaseHttpDownloader(SystemFacade system, Settings settings, @Nullable String userAgent) {
+    BaseHttpDownloader(AuthenticatorFacade system, Settings settings, @Nullable String userAgent) {
       initProxy(system, settings);
       initUserAgent(userAgent, settings);
     }
 
-    private void initProxy(SystemFacade system, Settings settings) {
-      // propagate system properties
-      for (String key : PROXY_SETTINGS) {
-        if (settings.hasKey(key)) {
-          system.setProperty(key, settings.getString(key));
-        }
-      }
-      // defaults of HTTPS properties are the values of HTTP properties
-      setSystemPropertyToDefaultIfNotSet(system, settings, HTTPS_PROXY_HOST, HTTP_PROXY_HOST);
-      setSystemPropertyToDefaultIfNotSet(system, settings, HTTPS_PROXY_PORT, HTTP_PROXY_PORT);
-
+    private void initProxy(AuthenticatorFacade system, Settings settings) {
       // register credentials
       String login = settings.getString(HTTP_PROXY_USER);
       if (isNotEmpty(login)) {
@@ -212,12 +186,6 @@ public class DefaultHttpDownloader extends HttpDownloader {
       }
     }
 
-    private static void setSystemPropertyToDefaultIfNotSet(SystemFacade system, Settings settings, String httpsProperty, String httpProperty) {
-      if (!settings.hasKey(httpsProperty) && settings.hasKey(httpProperty)) {
-        system.setProperty(httpsProperty, settings.getString(httpProperty));
-      }
-    }
-
     private void initUserAgent(@Nullable String sonarVersion, Settings settings) {
       String serverId = settings.getString(CoreProperties.SERVER_ID);
       userAgent = sonarVersion == null ? "SonarQube" : String.format("SonarQube %s # %s", sonarVersion, Optional.ofNullable(serverId).orElse(""));
diff --git a/sonar-core/src/test/java/org/sonar/core/util/DefaultHttpDownloaderTest.java b/sonar-core/src/test/java/org/sonar/core/util/DefaultHttpDownloaderTest.java
index fd90f8c6768..872094de53e 100644
--- a/sonar-core/src/test/java/org/sonar/core/util/DefaultHttpDownloaderTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/util/DefaultHttpDownloaderTest.java
@@ -61,11 +61,8 @@ import org.sonar.api.utils.SonarException;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.internal.matchers.ThrowableCauseMatcher.hasCause;
 import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.argThat;
-import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -289,41 +286,8 @@ public class DefaultHttpDownloaderTest {
   }
 
   @Test
-  public void configure_http_and_https_proxies() {
-    DefaultHttpDownloader.SystemFacade system = mock(DefaultHttpDownloader.SystemFacade.class);
-    Settings settings = new MapSettings();
-    settings.setProperty("http.proxyHost", "1.2.3.4");
-    settings.setProperty("http.proxyPort", "80");
-    settings.setProperty("https.proxyHost", "5.6.7.8");
-    settings.setProperty("https.proxyPort", "443");
-
-    new DefaultHttpDownloader.BaseHttpDownloader(system, settings, null);
-
-    verify(system).setProperty("http.proxyHost", "1.2.3.4");
-    verify(system).setProperty("http.proxyPort", "80");
-    verify(system).setProperty("https.proxyHost", "5.6.7.8");
-    verify(system).setProperty("https.proxyPort", "443");
-    verify(system, never()).setDefaultAuthenticator(any(Authenticator.class));
-  }
-
-  @Test
-  public void https_defaults_are_http_properties() {
-    DefaultHttpDownloader.SystemFacade system = mock(DefaultHttpDownloader.SystemFacade.class);
-    Settings settings = new MapSettings();
-    settings.setProperty("http.proxyHost", "1.2.3.4");
-    settings.setProperty("http.proxyPort", "80");
-
-    new DefaultHttpDownloader.BaseHttpDownloader(system, settings, null);
-
-    verify(system).setProperty("http.proxyHost", "1.2.3.4");
-    verify(system).setProperty("http.proxyPort", "80");
-    verify(system).setProperty("https.proxyHost", "1.2.3.4");
-    verify(system).setProperty("https.proxyPort", "80");
-  }
-
-  @Test
   public void configure_http_proxy_credentials() {
-    DefaultHttpDownloader.SystemFacade system = mock(DefaultHttpDownloader.SystemFacade.class);
+    DefaultHttpDownloader.AuthenticatorFacade system = mock(DefaultHttpDownloader.AuthenticatorFacade.class);
     Settings settings = new MapSettings();
     settings.setProperty("https.proxyHost", "1.2.3.4");
     settings.setProperty("http.proxyUser", "the_login");
@@ -346,23 +310,9 @@ public class DefaultHttpDownloaderTest {
     }));
   }
 
-  @Test
-  public void no_http_proxy_settings_by_default() {
-    DefaultHttpDownloader.SystemFacade system = mock(DefaultHttpDownloader.SystemFacade.class);
-    Settings settings = new MapSettings();
-    new DefaultHttpDownloader.BaseHttpDownloader(system, settings, null);
-
-    verify(system, never()).setProperty(eq("http.proxyHost"), anyString());
-    verify(system, never()).setProperty(eq("https.proxyHost"), anyString());
-    verify(system, never()).setProperty(eq("http.proxyPort"), anyString());
-    verify(system, never()).setProperty(eq("https.proxyPort"), anyString());
-    verify(system, never()).setDefaultAuthenticator(any(Authenticator.class));
-  }
-
-}
-
-class FakeProxy extends Proxy {
-  public FakeProxy() {
-    super(Type.HTTP, new InetSocketAddress("123.45.67.89", 4040));
+  private static class FakeProxy extends Proxy {
+    FakeProxy() {
+      super(Type.HTTP, new InetSocketAddress("123.45.67.89", 4040));
+    }
   }
 }