Move ES_TMPDIR substitution into jvm options parser (#47189)

This commit moves the ES_TMPDIR substitution that we do for JVM options
into the JVM options parser itself. This solves a problem where the fact
that the we do not make the substitution before ergonomics parsing can
lead to the JVM that we start for computing the ergonomic values failing
to start. Additionally, moving this substitution here enables us to
simplify the shell scripts since we do not need to implement this there,
and twice for Bash and Windows.

Author: jasontedor

distribution/src/bin/elasticsearch

@@ -21,8 +21,7 @@ if [ -z "$ES_TMPDIR" ]; then
 fi
 
 ES_JVM_OPTIONS="$ES_PATH_CONF"/jvm.options
-JVM_OPTIONS=`"$JAVA" -cp "$ES_CLASSPATH" org.elasticsearch.tools.launchers.JvmOptionsParser "$ES_JVM_OPTIONS"`
-ES_JAVA_OPTS="${JVM_OPTIONS//\$\{ES_TMPDIR\}/$ES_TMPDIR}"
+ES_JAVA_OPTS=`export ES_TMPDIR; "$JAVA" -cp "$ES_CLASSPATH" org.elasticsearch.tools.launchers.JvmOptionsParser "$ES_JVM_OPTIONS"`
 
 # manual parsing to find out, if process should be detached
 if ! echo $* | grep -E '(^-d |-d$| -d |--daemonize$|--daemonize )' > /dev/null; then

distribution/src/bin/elasticsearch-service.bat

@@ -115,8 +115,8 @@ set ES_JVM_OPTIONS=%ES_PATH_CONF%\jvm.options
 if not "%ES_JAVA_OPTS%" == "" set ES_JAVA_OPTS=%ES_JAVA_OPTS: =;%
 
 @setlocal
-for /F "usebackq delims=" %%a in (`"%JAVA% -cp "!ES_CLASSPATH!" "org.elasticsearch.tools.launchers.JvmOptionsParser" "!ES_JVM_OPTIONS!" || echo jvm_options_parser_failed"`) do set JVM_OPTIONS=%%a
-@endlocal & set "MAYBE_JVM_OPTIONS_PARSER_FAILED=%JVM_OPTIONS%" & set ES_JAVA_OPTS=%JVM_OPTIONS:${ES_TMPDIR}=!ES_TMPDIR!%
+for /F "usebackq delims=" %%a in (`"%JAVA% -cp "!ES_CLASSPATH!" "org.elasticsearch.tools.launchers.JvmOptionsParser" "!ES_JVM_OPTIONS!" || echo jvm_options_parser_failed"`) do set ES_JAVA_OPTS=%%a
+@endlocal & set "MAYBE_JVM_OPTIONS_PARSER_FAILED=%ES_JAVA_OPTS%" & set ES_JAVA_OPTS=%ES_JAVA_OPTS%
 
 if "%MAYBE_JVM_OPTIONS_PARSER_FAILED%" == "jvm_options_parser_failed" (
   exit /b 1

distribution/src/bin/elasticsearch.bat

@@ -47,8 +47,8 @@ if not defined ES_TMPDIR (
 
 set ES_JVM_OPTIONS=%ES_PATH_CONF%\jvm.options
 @setlocal
-for /F "usebackq delims=" %%a in (`CALL %JAVA% -cp "!ES_CLASSPATH!" "org.elasticsearch.tools.launchers.JvmOptionsParser" "!ES_JVM_OPTIONS!" ^|^| echo jvm_options_parser_failed`) do set JVM_OPTIONS=%%a
-@endlocal & set "MAYBE_JVM_OPTIONS_PARSER_FAILED=%JVM_OPTIONS%" & set ES_JAVA_OPTS=%JVM_OPTIONS:${ES_TMPDIR}=!ES_TMPDIR!%
+for /F "usebackq delims=" %%a in (`CALL %JAVA% -cp "!ES_CLASSPATH!" "org.elasticsearch.tools.launchers.JvmOptionsParser" "!ES_JVM_OPTIONS!" ^|^| echo jvm_options_parser_failed`) do set ES_JAVA_OPTS=%%a
+@endlocal & set "MAYBE_JVM_OPTIONS_PARSER_FAILED=%ES_JAVA_OPTS%" & set ES_JAVA_OPTS=%ES_JAVA_OPTS%
 
 if "%MAYBE_JVM_OPTIONS_PARSER_FAILED%" == "jvm_options_parser_failed" (
   exit /b 1

distribution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOptionsParser.java

@@ -86,9 +86,11 @@ public void accept(final int lineNumber, final String line) {
                         .filter(s -> s.trim().isEmpty() == false)
                         .collect(Collectors.toList()));
             }
-            final List<String> ergonomicJvmOptions = JvmErgonomics.choose(jvmOptions);
-            jvmOptions.addAll(ergonomicJvmOptions);
-            final String spaceDelimitedJvmOptions = spaceDelimitJvmOptions(jvmOptions);
+            final List<String> substitutedJvmOptions =
+                substitutePlaceholders(jvmOptions, Map.of("ES_TMPDIR", System.getenv("ES_TMPDIR")));
+            final List<String> ergonomicJvmOptions = JvmErgonomics.choose(substitutedJvmOptions);
+            substitutedJvmOptions.addAll(ergonomicJvmOptions);
+            final String spaceDelimitedJvmOptions = spaceDelimitJvmOptions(substitutedJvmOptions);
             Launchers.outPrintln(spaceDelimitedJvmOptions);
             Launchers.exit(0);
         } else {
@@ -114,6 +116,24 @@ public void accept(final int lineNumber, final String line) {
         }
     }
 
+    static List<String> substitutePlaceholders(final List<String> jvmOptions, final Map<String, String> substitutions) {
+        final Map<String, String> placeholderSubstitutions =
+            substitutions.entrySet().stream().collect(Collectors.toMap(e -> "${" + e.getKey() + "}", Map.Entry::getValue));
+        return jvmOptions.stream()
+            .map(
+                jvmOption -> {
+                    String actualJvmOption = jvmOption;
+                    int start = jvmOption.indexOf("${");
+                    if (start >= 0 && jvmOption.indexOf('}', start) > 0) {
+                        for (final Map.Entry<String, String> placeholderSubstitution : placeholderSubstitutions.entrySet()) {
+                            actualJvmOption = actualJvmOption.replace(placeholderSubstitution.getKey(), placeholderSubstitution.getValue());
+                        }
+                    }
+                    return actualJvmOption;
+                })
+            .collect(Collectors.toList());
+    }
+
     /**
      * Callback for valid JVM options.
      */

distribution/tools/launchers/src/test/java/org/elasticsearch/tools/launchers/JvmOptionsParserTests.java

@@ -30,6 +30,7 @@
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
@@ -39,6 +40,12 @@
 
 public class JvmOptionsParserTests extends LaunchersTestCase {
 
+    public void testSubstitution() {
+        final List<String> jvmOptions =
+            JvmOptionsParser.substitutePlaceholders(List.of("-Djava.io.tmpdir=${ES_TMPDIR}"), Map.of("ES_TMPDIR", "/tmp/elasticsearch"));
+        assertThat(jvmOptions, contains("-Djava.io.tmpdir=/tmp/elasticsearch"));
+    }
+
     public void testUnversionedOptions() throws IOException {
         try (StringReader sr = new StringReader("-Xms1g\n-Xmx1g");
              BufferedReader br = new BufferedReader(sr)) {