SONAR-9773 ignore JAVA_TOOL_OPTIONS is defined and log warning

diff --git a/server/sonar-process/src/main/java/org/sonar/process/command/CommandFactoryImpl.java b/server/sonar-process/src/main/java/org/sonar/process/command/CommandFactoryImpl.java
index ae0ed14b06432521355e07cd58bec91889b6a34f..abeabc778196db30308c262d862050598ce7492c 100644 (file)
--- a/server/sonar-process/src/main/java/org/sonar/process/command/CommandFactoryImpl.java
+++ b/server/sonar-process/src/main/java/org/sonar/process/command/CommandFactoryImpl.java
@@ -22,9 +22,11 @@ package org.sonar.process.command;
 import java.io.File;
 import java.util.Map;
 import java.util.Optional;
+import org.slf4j.LoggerFactory;
 import org.sonar.process.ProcessId;
 import org.sonar.process.ProcessProperties;
 import org.sonar.process.Props;
+import org.sonar.process.System2;
 import org.sonar.process.es.EsFileSystem;
 import org.sonar.process.es.EsLogging;
 import org.sonar.process.es.EsSettings;
@@ -40,6 +42,7 @@ import static org.sonar.process.ProcessProperties.HTTP_PROXY_HOST;
 import static org.sonar.process.ProcessProperties.HTTP_PROXY_PORT;
 
 public class CommandFactoryImpl implements CommandFactory {
+  private static final String ENV_VAR_JAVA_TOOL_OPTIONS = "JAVA_TOOL_OPTIONS";
   /**
    * Properties about proxy that must be set as system properties
    */
@@ -56,9 +59,15 @@ public class CommandFactoryImpl implements CommandFactory {
   private final Props props;
   private final File tempDir;
 
-  public CommandFactoryImpl(Props props, File tempDir) {
+  public CommandFactoryImpl(Props props, File tempDir, System2 system2) {
     this.props = props;
     this.tempDir = tempDir;
+    String javaToolOptions = system2.getenv(ENV_VAR_JAVA_TOOL_OPTIONS);
+    if (javaToolOptions != null && !javaToolOptions.trim().isEmpty()) {
+      LoggerFactory.getLogger(CommandFactoryImpl.class)
+        .warn("JAVA_TOOL_OPTIONS is defined but will be ignored. " +
+          "Use properties sonar.*.javaOpts and/or sonar.*.javaAdditionalOpts in sonar.properties to change SQ JVM processes options");
+    }
   }
 
   @Override
@@ -76,13 +85,14 @@ public class CommandFactoryImpl implements CommandFactory {
       .setClusterName(settingsMap.get("cluster.name"))
       .setHost(settingsMap.get("network.host"))
       .setPort(Integer.valueOf(settingsMap.get("transport.tcp.port")))
-      .addEsOption("-Epath.conf="+esFileSystem.getConfDirectory().getAbsolutePath())
+      .addEsOption("-Epath.conf=" + esFileSystem.getConfDirectory().getAbsolutePath())
       .setEsJvmOptions(new EsJvmOptions()
         .addFromMandatoryProperty(props, ProcessProperties.SEARCH_JAVA_OPTS)
         .addFromMandatoryProperty(props, ProcessProperties.SEARCH_JAVA_ADDITIONAL_OPTS))
       .setEsYmlSettings(new EsYmlSettings(settingsMap))
       .setEnvVariable("ES_JVM_OPTIONS", esFileSystem.getJvmOptions().getAbsolutePath())
-      .setEnvVariable("JAVA_HOME", System.getProperties().getProperty("java.home"));
+      .setEnvVariable("JAVA_HOME", System.getProperties().getProperty("java.home"))
+      .suppressEnvVariable(ENV_VAR_JAVA_TOOL_OPTIONS);
   }
 
   @Override
@@ -107,6 +117,7 @@ public class CommandFactoryImpl implements CommandFactory {
     if (driverPath != null) {
       command.addClasspath(driverPath);
     }
+    command.suppressEnvVariable(ENV_VAR_JAVA_TOOL_OPTIONS);
     return command;
   }
 
@@ -130,6 +141,7 @@ public class CommandFactoryImpl implements CommandFactory {
     if (driverPath != null) {
       command.addClasspath(driverPath);
     }
+    command.suppressEnvVariable(ENV_VAR_JAVA_TOOL_OPTIONS);
     return command;
   }
 

diff --git a/server/sonar-process/src/test/java/org/sonar/process/command/CommandFactoryImplTest.java b/server/sonar-process/src/test/java/org/sonar/process/command/CommandFactoryImplTest.java
index f2bc87961d9eb9c0202296edbc6b29a9bb2ad583..5c87e00c6b42fd45021e0ff888c2ffe86beb392c 100644 (file)
--- a/server/sonar-process/src/test/java/org/sonar/process/command/CommandFactoryImplTest.java
+++ b/server/sonar-process/src/test/java/org/sonar/process/command/CommandFactoryImplTest.java
@@ -19,23 +19,35 @@
  */
 package org.sonar.process.command;
 
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.AppenderBase;
 import java.io.File;
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Properties;
 import org.apache.commons.io.FileUtils;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;
+import org.mockito.Mockito;
 import org.sonar.process.ProcessId;
 import org.sonar.process.ProcessProperties;
 import org.sonar.process.Props;
+import org.sonar.process.System2;
+import org.sonar.process.logging.LogbackHelper;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.entry;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.when;
 
 public class CommandFactoryImplTest {
+
+  private static final String MEMORY_APPENDER_NAME = "MEMORY_APPENDER";
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
   @Rule
@@ -44,6 +56,7 @@ public class CommandFactoryImplTest {
   private File homeDir;
   private File tempDir;
   private File logsDir;
+  private ListAppender listAppender;
 
   @Before
   public void setUp() throws Exception {
@@ -52,6 +65,41 @@ public class CommandFactoryImplTest {
     logsDir = temp.newFolder();
   }
 
+  @After
+  public void tearDown() throws Exception {
+    if (listAppender != null) {
+      listAppender.stop();
+      new LogbackHelper().getRootContext().getLogger(CommandFactoryImpl.class)
+          .detachAppender(listAppender);
+    }
+  }
+
+  @Test
+  public void constructor_logs_no_warning_if_env_variable_JAVA_TOOL_OPTIONS_is_not_set() {
+    System2 system2 = Mockito.mock(System2.class);
+    when(system2.getenv(anyString())).thenReturn(null);
+    attachMemoryAppenderToLoggerOf(CommandFactoryImpl.class);
+
+    new CommandFactoryImpl(new Props(new Properties()), tempDir, system2);
+
+    assertThat(listAppender.getLogs()).isEmpty();
+  }
+
+  @Test
+  public void constructor_logs_warning_if_env_variable_JAVA_TOOL_OPTIONS_is_set() {
+    System2 system2 = Mockito.mock(System2.class);
+    when(system2.getenv("JAVA_TOOL_OPTIONS")).thenReturn("sds");
+    attachMemoryAppenderToLoggerOf(CommandFactoryImpl.class);
+
+    new CommandFactoryImpl(new Props(new Properties()), tempDir, system2);
+
+    assertThat(listAppender.getLogs())
+      .extracting(ILoggingEvent::getMessage)
+      .containsOnly(
+        "JAVA_TOOL_OPTIONS is defined but will be ignored. " +
+          "Use properties sonar.*.javaOpts and/or sonar.*.javaAdditionalOpts in sonar.properties to change SQ JVM processes options");
+  }
+
   @Test
   public void createEsCommand_throws_ISE_if_es_binary_is_not_found() throws Exception {
     expectedException.expect(IllegalStateException.class);
@@ -86,6 +134,8 @@ public class CommandFactoryImplTest {
 
     assertThat(esCommand.getLog4j2Properties())
       .contains(entry("appender.file_es.fileName", new File(logsDir, "es.log").getAbsolutePath()));
+
+    assertThat(esCommand.getSuppressedEnvVariables()).containsOnly("JAVA_TOOL_OPTIONS");
   }
 
   @Test
@@ -132,6 +182,8 @@ public class CommandFactoryImplTest {
       // default settings
       .contains(entry("sonar.web.javaOpts", "-Xmx512m -Xms128m -XX:+HeapDumpOnOutOfMemoryError"))
       .contains(entry("sonar.cluster.enabled", "false"));
+
+    assertThat(command.getSuppressedEnvVariables()).containsOnly("JAVA_TOOL_OPTIONS");
   }
 
   @Test
@@ -153,6 +205,8 @@ public class CommandFactoryImplTest {
       // default settings
       .contains(entry("sonar.web.javaOpts", "-Xmx10G"))
       .contains(entry("sonar.cluster.enabled", "false"));
+
+    assertThat(command.getSuppressedEnvVariables()).containsOnly("JAVA_TOOL_OPTIONS");
   }
 
   @Test
@@ -181,6 +235,31 @@ public class CommandFactoryImplTest {
 
     Props props = new Props(p);
     ProcessProperties.completeDefaults(props);
-    return new CommandFactoryImpl(props, tempDir);
+    return new CommandFactoryImpl(props, tempDir, System2.INSTANCE);
+  }
+
+  private <T> void attachMemoryAppenderToLoggerOf(Class<T> loggerClass) {
+    this.listAppender = new ListAppender();
+    new LogbackHelper().getRootContext().getLogger(loggerClass)
+      .addAppender(listAppender);
+    listAppender.start();
+  }
+
+  private static final class ListAppender extends AppenderBase<ILoggingEvent> {
+    private final List<ILoggingEvent> logs = new ArrayList<>();
+
+    @Override
+    public String getName() {
+      return MEMORY_APPENDER_NAME;
+    }
+
+    @Override
+    protected void append(ILoggingEvent eventObject) {
+      logs.add(eventObject);
+    }
+
+    public List<ILoggingEvent> getLogs() {
+      return logs;
+    }
   }
 }

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 4e25774bc5ea7f3c34e7b0aef22126e94b45561c..54d233f5947ed7710f626e6dd53eaec2cdadeb5e 100644 (file)
--- a/sonar-application/src/main/java/org/sonar/application/App.java
+++ b/sonar-application/src/main/java/org/sonar/application/App.java
@@ -27,6 +27,7 @@ import org.sonar.application.process.ProcessLauncher;
 import org.sonar.application.process.ProcessLauncherImpl;
 import org.sonar.application.process.StopRequestWatcher;
 import org.sonar.application.process.StopRequestWatcherImpl;
+import org.sonar.process.System2;
 import org.sonar.process.SystemExit;
 import org.sonar.process.command.CommandFactory;
 import org.sonar.process.command.CommandFactoryImpl;
@@ -52,7 +53,7 @@ public class App {
       appState.registerClusterName(settings.getProps().value(CLUSTER_NAME, "sonarqube"));
       AppReloader appReloader = new AppReloaderImpl(settingsLoader, fileSystem, appState, logging);
       fileSystem.reset();
-      CommandFactory commandFactory = new CommandFactoryImpl(settings.getProps(), fileSystem.getTempDir());
+      CommandFactory commandFactory = new CommandFactoryImpl(settings.getProps(), fileSystem.getTempDir(), System2.INSTANCE);
 
       try (ProcessLauncher processLauncher = new ProcessLauncherImpl(fileSystem.getTempDir())) {
         Scheduler scheduler = new SchedulerImpl(settings, appReloader, commandFactory, processLauncher, appState);