[Java17] Add --add-export settings to restore JDK17 compatibility (#13825)

* [Java17] Add `--add-export` settings to restore JDK17 compatibility

After #13700 updated google-java-format dependency, it is now required to add a number
of `--add-export` flags in order to run on JDK17. This commit adds these flags to the
jvm options for a running logstash, and to the tests running on gradle to enable tests
to still work

* Move mandatory JVM options out of `jvm.options` and into JvmOptionsParser

Certain values for the JVM are mandatory for Logstash to function correctly. Rather than
leave them in config/jvm.options where they can be updated, and can cause upgrade issues
for users when we add new mandatory options, move them into code where they cannot be
changed

Author: robbavey

build.gradle

@@ -81,9 +81,17 @@ allprojects {
       delete "${projectDir}/out/"
   }
 
-  //https://stackoverflow.com/questions/3963708/gradle-how-to-display-test-results-in-the-console-in-real-time
   tasks.withType(Test) {
-    testLogging {
+    // Add Exports to enable tests to run in JDK17
+    jvmArgs = [
+      "--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
+      "--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
+      "--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
+      "--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
+      "--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED"
+    ]
+      //https://stackoverflow.com/questions/3963708/gradle-how-to-display-test-results-in-the-console-in-real-time
+      testLogging {
       // set options for log level LIFECYCLE
       events "passed", "skipped", "failed", "standardOut"
       showExceptions true
@@ -893,4 +901,4 @@ if (System.getenv('OSS') != 'true') {
  tasks.register("runXPackIntegrationTests"){
      dependsOn copyPluginTestAlias
      dependsOn ":logstash-xpack:rubyIntegrationTests"
- }
+ }

config/jvm.options

@@ -49,8 +49,6 @@
 -Djruby.compile.invokedynamic=true
 # Force Compilation
 -Djruby.jit.threshold=0
-# Make sure joni regexp interruptability is enabled
--Djruby.regexp.interruptible=true
 
 ## heap dumps
 
@@ -73,10 +71,4 @@
 -Djava.security.egd=file:/dev/urandom
 
 # Copy the logging context from parent threads to children
--Dlog4j2.isThreadContextMapInheritable=true
-
-11-:--add-opens=java.base/java.security=ALL-UNNAMED
-11-:--add-opens=java.base/java.io=ALL-UNNAMED
-11-:--add-opens=java.base/java.nio.channels=ALL-UNNAMED
-11-:--add-opens=java.base/sun.nio.ch=ALL-UNNAMED
-11-:--add-opens=java.management/sun.management=ALL-UNNAMED
+-Dlog4j2.isThreadContextMapInheritable=true

logstash-core/src/main/java/org/logstash/launchers/JvmOptionsParser.java

@@ -13,22 +13,40 @@
 import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 import java.util.SortedMap;
 import java.util.TreeMap;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
 
 /**
  * Parse jvm.options file applying version conditional logic. Heavily inspired by same functionality in Elasticsearch.
  * */
 public class JvmOptionsParser {
 
+    private static final String[] MANDATORY_JVM_OPTIONS = new String[]{
+            "-Djruby.regexp.interruptible=true",
+            "16-:--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
+            "16-:--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
+            "16-:--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
+            "16-:--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
+            "16-:--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
+            "11-:--add-opens=java.base/java.security=ALL-UNNAMED",
+            "11-:--add-opens=java.base/java.io=ALL-UNNAMED",
+            "11-:--add-opens=java.base/java.nio.channels=ALL-UNNAMED",
+            "11-:--add-opens=java.base/sun.nio.ch=ALL-UNNAMED",
+            "11-:--add-opens=java.management/sun.management=ALL-UNNAMED"
+    };
+
     static class JvmOptionsFileParserException extends Exception {
 
         private static final long serialVersionUID = 2446165130736962758L;
@@ -71,8 +89,7 @@ public static void main(final String[] args) throws InterruptedException, IOExce
             );
         }
         bailOnOldJava();
-        final String lsJavaOpts = System.getenv("LS_JAVA_OPTS");
-        handleJvmOptions(args, lsJavaOpts);
+        handleJvmOptions(args, System.getenv("LS_JAVA_OPTS"));
     }
 
     static void bailOnOldJava(){
@@ -93,7 +110,7 @@ static void handleJvmOptions(String[] args, String lsJavaOpts) {
         final String jvmOpts = args.length == 2 ? args[1] : null;
         try {
             Optional<Path> jvmOptions = parser.lookupJvmOptionsFile(jvmOpts);
-            parser.parseAndInjectEnvironment(jvmOptions, lsJavaOpts);
+            parser.handleJvmOptions(jvmOptions, lsJavaOpts);
         } catch (JvmOptionsFileParserException pex) {
             System.err.printf(Locale.ROOT,
                     "encountered [%d] error%s parsing [%s]",
@@ -128,20 +145,38 @@ private Optional<Path> lookupJvmOptionsFile(String jvmOpts) {
                 .findFirst();
     }
 
-    private void parseAndInjectEnvironment(Optional<Path> jvmOptionsFile, String lsJavaOpts) throws IOException, JvmOptionsFileParserException {
-        final List<String> jvmOptionsContent = new ArrayList<>(parseJvmOptions(jvmOptionsFile));
+    private void handleJvmOptions(Optional<Path> jvmOptionsFile, String lsJavaOpts) throws IOException, JvmOptionsFileParserException {
+        int javaMajorVersion = javaMajorVersion();
+
+        // Add JVM Options from config/jvm.options
+        final Set<String> jvmOptionsContent = new LinkedHashSet<>(getJvmOptionsFromFile(jvmOptionsFile, javaMajorVersion));
 
+        // Add JVM Options from LS_JAVA_OPTS
         if (lsJavaOpts != null && !lsJavaOpts.isEmpty()) {
             if (isDebugEnabled()) {
                 System.err.println("Appending jvm options from environment LS_JAVA_OPTS");
             }
             jvmOptionsContent.add(lsJavaOpts);
         }
+        // Set mandatory JVM options
+        jvmOptionsContent.addAll(getMandatoryJvmOptions(javaMajorVersion));
 
         System.out.println(String.join(" ", jvmOptionsContent));
     }
 
-    private List<String> parseJvmOptions(Optional<Path> jvmOptionsFile) throws IOException, JvmOptionsFileParserException {
+    /**
+     * Returns the list of mandatory JVM options for the given version of Java.
+     * @param javaMajorVersion
+     * @return Collection of mandatory options
+     */
+     static Collection<String> getMandatoryJvmOptions(int javaMajorVersion){
+          return Arrays.stream(MANDATORY_JVM_OPTIONS)
+                  .map(option -> jvmOptionFromLine(javaMajorVersion, option))
+                  .flatMap(Optional::stream)
+                  .collect(Collectors.toUnmodifiableList());
+    }
+
+    private List<String> getJvmOptionsFromFile(final Optional<Path> jvmOptionsFile, final int javaMajorVersion) throws IOException, JvmOptionsFileParserException {
         if (!jvmOptionsFile.isPresent()) {
             System.err.println("Warning: no jvm.options file found.");
             return Collections.emptyList();
@@ -155,13 +190,11 @@ private List<String> parseJvmOptions(Optional<Path> jvmOptionsFile) throws IOExc
         if (isDebugEnabled()) {
             System.err.format("Processing jvm.options file at `%s`\n", optionsFilePath);
         }
-        final int majorJavaVersion = javaMajorVersion();
-
         try (InputStream is = Files.newInputStream(optionsFilePath);
              Reader reader = new InputStreamReader(is, StandardCharsets.UTF_8);
              BufferedReader br = new BufferedReader(reader)
         ) {
-            final ParseResult parseResults = parse(majorJavaVersion, br);
+            final ParseResult parseResults = parse(javaMajorVersion, br);
             if (parseResults.hasErrors()) {
                 throw new JvmOptionsFileParserException(optionsFilePath, parseResults.getInvalidLines());
             }
@@ -209,7 +242,36 @@ public List<String> getJvmOptions() {
     private static final Pattern OPTION_DEFINITION = Pattern.compile("((?<start>\\d+)(?<range>-)?(?<end>\\d+)?:)?(?<option>-.*)$");
 
     /**
-     * Parse the line-delimited JVM options from the specified buffered reader for the specified Java major version.
+     *
+     * If the version syntax specified on a line matches the specified JVM options, the JVM option callback will be invoked with the JVM
+     * option. If the line does not match the specified syntax for the JVM options, the invalid line callback will be invoked with the
+     * contents of the entire line.
+     *
+     * @param javaMajorVersion the Java major version to match JVM options against
+     * @param br the buffered reader to read line-delimited JVM options from
+     * @return the admitted options lines respecting the javaMajorVersion and the error lines
+     * @throws IOException if an I/O exception occurs reading from the buffered reader
+     */
+    static ParseResult parse(final int javaMajorVersion, final BufferedReader br) throws IOException {
+        final ParseResult result = new ParseResult();
+        int lineNumber = 0;
+        while (true) {
+            final String line = br.readLine();
+            lineNumber++;
+            if (line == null) {
+                break;
+            }
+            try{
+                jvmOptionFromLine(javaMajorVersion, line).ifPresent(result::appendOption);
+            } catch (IllegalArgumentException e){
+                result.appendError(lineNumber, line);
+            };
+        }
+        return result;
+    }
+
+    /**
+     * Parse the line-delimited JVM options from the specified string for the specified Java major version.
      * Valid JVM options are:
      * <ul>
      *     <li>
@@ -256,82 +318,52 @@ public List<String> getJvmOptions() {
      *         {@code 9-10:-Xlog:age*=trace,gc*,safepoint:file=logs/gc.log:utctime,pid,tags:filecount=32,filesize=64m}
      *     </li>
      * </ul>
-     *
-     * If the version syntax specified on a line matches the specified JVM options, the JVM option callback will be invoked with the JVM
-     * option. If the line does not match the specified syntax for the JVM options, the invalid line callback will be invoked with the
-     * contents of the entire line.
-     *
-     * @param javaMajorVersion the Java major version to match JVM options against
-     * @param br the buffered reader to read line-delimited JVM options from
-     * @return the admitted options lines respecting the javaMajorVersion and the error lines
-     * @throws IOException if an I/O exception occurs reading from the buffered reader
+
+     * @param javaMajorVersion
+     * @param line
+     * @return Returns an Optional containing a string if the line contains a valid option for the specified Java
+     *         version and empty otherwise
      */
-    static ParseResult parse(final int javaMajorVersion, final BufferedReader br) throws IOException {
-        final ParseResult result = new ParseResult();
-        int lineNumber = 0;
-        while (true) {
-            final String line = br.readLine();
-            lineNumber++;
-            if (line == null) {
-                break;
-            }
-            if (line.startsWith("#")) {
-                // lines beginning with "#" are treated as comments
-                continue;
-            }
-            if (line.matches("\\s*")) {
-                // skip blank lines
-                continue;
-            }
-            final Matcher matcher = OPTION_DEFINITION.matcher(line);
-            if (matcher.matches()) {
-                final String start = matcher.group("start");
-                final String end = matcher.group("end");
-                if (start == null) {
-                    // no range present, unconditionally apply the JVM option
-                    result.appendOption(line);
+    private static Optional<String> jvmOptionFromLine(final int javaMajorVersion, final String line){
+        if (line.startsWith("#") || line.matches("\\s*")) {
+            // Skip comments and blank lines
+            return Optional.empty();
+        }
+        final Matcher matcher = OPTION_DEFINITION.matcher(line);
+        if (matcher.matches()) {
+            final String start = matcher.group("start");
+            final String end = matcher.group("end");
+            if (start == null) {
+                // no range present, unconditionally apply the JVM option
+                return Optional.of(line);
+            } else {
+                final int lower = Integer.parseInt(start);
+                final int upper;
+                if (matcher.group("range") == null) {
+                    // no range is present, apply the JVM option to the specified major version only
+                    upper = lower;
+                } else if (end == null) {
+                    // a range of the form \\d+- is present, apply the JVM option to all major versions larger than the specified one
+                    upper = Integer.MAX_VALUE;
                 } else {
-                    final int lower;
-                    try {
-                        lower = Integer.parseInt(start);
-                    } catch (final NumberFormatException e) {
-                        result.appendError(lineNumber, line);
-                        continue;
-                    }
-                    final int upper;
-                    if (matcher.group("range") == null) {
-                        // no range is present, apply the JVM option to the specified major version only
-                        upper = lower;
-                    } else if (end == null) {
-                        // a range of the form \\d+- is present, apply the JVM option to all major versions larger than the specified one
-                        upper = Integer.MAX_VALUE;
-                    } else {
-                        // a range of the form \\d+-\\d+ is present, apply the JVM option to the specified range of major versions
-                        try {
-                            upper = Integer.parseInt(end);
-                        } catch (final NumberFormatException e) {
-                            result.appendError(lineNumber, line);
-                            continue;
-                        }
-                        if (upper < lower) {
-                            result.appendError(lineNumber, line);
-                            continue;
-                        }
-                    }
-                    if (lower <= javaMajorVersion && javaMajorVersion <= upper) {
-                        result.appendOption(matcher.group("option"));
+                        upper = Integer.parseInt(end);
+                    if (upper < lower) {
+                        throw new IllegalArgumentException("Upper bound must be greater than lower bound");
                     }
                 }
-            } else {
-                result.appendError(lineNumber, line);
+                if (lower <= javaMajorVersion && javaMajorVersion <= upper) {
+                    return Optional.of(matcher.group("option"));
+                }
             }
+        } else {
+            throw new IllegalArgumentException("Illegal JVM Option");
         }
-        return result;
+        return Optional.empty();
     }
 
     private static final Pattern JAVA_VERSION = Pattern.compile("^(?:1\\.)?(?<javaMajorVersion>\\d+)(?:\\.\\d+)?$");
 
-    private int javaMajorVersion() {
+    private static int javaMajorVersion() {
         final String specVersion = System.getProperty("java.specification.version");
         final Matcher specVersionMatcher = JAVA_VERSION.matcher(specVersion);
         if (!specVersionMatcher.matches()) {

logstash-core/src/test/java/org/logstash/launchers/JvmOptionsParserTest.java

@@ -40,7 +40,7 @@ public void test_LS_JAVA_OPTS_isUsedWhenNoJvmOptionsIsAvailable() throws IOExcep
 
         // Verify
         final String output = outputStreamCaptor.toString();
-        assertEquals("Output MUST contains the options present in LS_JAVA_OPTS", "-Xblabla" + System.lineSeparator(), output);
+        assertTrue("Output MUST contains the options present in LS_JAVA_OPTS", output.contains("-Xblabla"));
     }
 
     @SuppressWarnings({ "unchecked" })
@@ -89,6 +89,27 @@ public void testParseOptionVersionRange() throws IOException {
         assertTrue("No option match outside the range [10-11]", res.getJvmOptions().isEmpty());
     }
 
+    @Test
+    public void testMandatoryJvmOptionApplicableJvmPresent() throws IOException{
+        assertTrue("Contains add-exports value for Java 17",
+                JvmOptionsParser.getMandatoryJvmOptions(17).contains("--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED"));
+    }
+
+    @Test
+    public void testMandatoryJvmOptionNonApplicableJvmNotPresent() throws IOException{
+        assertFalse("Does not contains add-exports value for Java 11",
+                JvmOptionsParser.getMandatoryJvmOptions(11).contains("--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED"));
+    }
+
+    @Test
+    public void testAlwaysMandatoryJvmPresent() {
+        assertTrue("Contains regexp interruptible for Java 11",
+                JvmOptionsParser.getMandatoryJvmOptions(11).contains("-Djruby.regexp.interruptible=true"));
+        assertTrue("Contains regexp interruptible for Java 17",
+                JvmOptionsParser.getMandatoryJvmOptions(17).contains("-Djruby.regexp.interruptible=true"));
+
+    }
+
     @Test
     public void testErrorLinesAreReportedCorrectly() throws IOException {
         final String jvmOptionsContent = "10-11:-XX:+UseConcMarkSweepGC" + System.lineSeparator() +