Make JPMS parameters work with and without enabling Jetty JPMS mode

[jnehlmeier]

This is a two split issue from end user perspective. Some need to be fixed in the docker image and some in jetty itself. The main goal is to make JPMS useable.

Consider a web application that has some old libraries (before JPMS) and newer ones that may or may not have JPMS metadata. Yet the application needs some --add-opens parameter to make some libraries work properly.

Migrating to the official jetty docker image has the following issues:

1. Nearly every java based docker image has a JAVA_OPTIONS env variable that is passed to the final JVM being executed in the container as is. A Dockerfile like

   FROM jetty:10.0.15-jdk17-eclipse-temurin
   ENV JAVA_OPTIONS --add-opens jdk.management/com.sun.management.internal=ALL-UNNAMED \
     --add-opens java.base/sun.nio.ch=ALL-UNNAMED \
     --add-opens java.base/jdk.internal.misc=ALL-UNNAMED \
     --add-opens java.base/java.nio=ALL-UNNAMED \
     -Dio.grpc.netty.shaded.io.netty.tryReflectionSetAccessible=true

   should just work without issues. However the above results in a jetty.start file that contains (spaces replaced with newline for readability)

   exec 
   /opt/java/openjdk/bin/java
   -Djetty.home=/usr/local/jetty
   -Djetty.base=/var/lib/jetty
   --add-opens
   -Dio.grpc.netty.shaded.io.netty.tryReflectionSetAccessible=true
   --class-path
   

   This command is broken because the single --add-opens does not have a parameter value and the remaining --add-opens are swallowed completely.

2. Using a custom jetty module to use JPMS as envisioned by jetty would look like

   [description]
   App JVM Parameter
   
   [tags]
   jvm
   
   [ini]
   --jpms
   
   [exec]
   -Dio.grpc.netty.shaded.io.netty.tryReflectionSetAccessible=true
   
   [jpms]
   add-opens: jdk.management/com.sun.management.internal=ALL-UNNAMED
   add-opens: java.base/sun.nio.ch=ALL-UNNAMED
   add-opens: java.base/jdk.internal.misc=ALL-UNNAMED
   add-opens: java.base/java.nio=ALL-UNNAMED
   

   However after activating such a module the --dry-run command emits a command line that contains --module org.eclipse.jetty.xml/org.eclipse.jetty.xml.XmlConfiguration . The generate-jetty-start.sh script executes a regex on the --dry-run output and that regex filters the emitted command line resulting in jetty.start being empty and the container fails to start. The regex always searches for org.eclipse.jetty.xml.XmlConfiguration surrounded by a single space character which is not given in JPMS mode. See regex at: https://github.com/eclipse/jetty.docker/blob/992eabbc38b2694207852286b15623dc18e67978/eclipse-temurin/10.0/jdk17/generate-jetty-start.sh#L8C3-L10C56

3. In my opinion a custom jetty module should also allow defining JPMS parameters in the [exec] block because there are situations you do not want to fully enable JPMS by using module-path but still want to open some packages for reflection. This could be done using JAVA_OPTIONS if 1. is fixed but I think it is still valid to have a custom module that should work the same.

   [exec]
   --add-opens
   jdk.management/com.sun.management.internal=ALL-UNNAMED
   --add-opens
   java.base/sun.nio.ch=ALL-UNNAMED
   --add-opens
   java.base/jdk.internal.misc=ALL-UNNAMED
   --add-opens
   java.base/java.nio=ALL-UNNAMED
   -Dio.grpc.netty.shaded.io.netty.tryReflectionSetAccessible=true
   

   However the above fails because all --add-opens parameters will be deduplicated resulting in a broken command line again (spaces replaced with newline for readability):

   exec
   /opt/java/openjdk/bin/java
   -Djetty.home=/usr/local/jetty
   -Djetty.base=/var/lib/jetty
   --add-opens
   jdk.management/com.sun.management.internal=ALL-UNNAMED
   java.base/sun.nio.ch=ALL-UNNAMED
   java.base/jdk.internal.misc=ALL-UNNAMED
   java.base/java.nio=ALL-UNNAMED
   -Dio.grpc.netty.shaded.io.netty.tryReflectionSetAccessible=true
   --class-path
   

   This is also tracked in --dry-run generates broken command line when using multiple --add-opens in an [exec] module jetty.project#8348 and there was a workaround for it before jetty introduced quotes on parameters. Now with quotes this workaround does not work anymore. I repeated that issue here to have a full view of the usability problems.

[lachlan-roberts]

I think points 1 and 3 are issues for jetty.project, but number 2 can be fixed here.

For point 1 we have to first take docker out of the equation. If you can't use start.jar with --dry-run to produce a valid java command, then its probably an issue with the Jetty --dry-run.

java -jar $JETTY_HOME/start.jar "--add-opens java.base/java.nio=ALL-UNNAMED" --dry-run

This produces a command where the --add-opens is quoted

'--add-opens java.base/java.nio=ALL-UNNAMED'

And doing this without quotes:

java -jar $JETTY_HOME/start.jar --add-opens java.base/java.nio=ALL-UNNAMED --dry-run

Treats java.base/java.nio=ALL-UNNAMED as a jetty property, and leaves a blank --add-opens.

[jnehlmeier]

@lachlan-roberts Thanks for your reply. I think for point 1. the docker image could also implement a workaround until start.jar works properly.

Currently the --dry-run command is executed using

echo "exec " $JAVA $JAVA_OPTIONS "$@" $JAVA_OPTIONS $JETTY_PROPERTIES > $JETTY_BASE/jetty.exec

First I don't see the point why JAVA_OPTIONS is used for the start.jar JVM given that it most likely contains options that should be used for the real JVM later running inside the container. So something like an additional JAVA_OPTIONS_STARTJAR would be more appropriate I guess.

Anways, the entry point could be changed to

echo "exec " $JAVA $JAVA_OPTIONS "$@" $JETTY_PROPERTIES > $JETTY_BASE/jetty.exec

to avoid passing JAVA_OPTIONS through start.jar. Finally sed is used to do an in-place replace in jetty.exec. It could replace exec $JAVA (.*) with exec $JAVA $JAVA_OPTIONS $1 (pseudo code). Then JAVA_OPTIONS would be used as-is. When using sed you would need to escape the sed delimiter inside variable content.

Later once start.jar handles java options correctly this could be reverted back. But personally I think start.jar should not deal with JVM parameters at all and require them to be in an env variable, read that variable and put its content inside the generated --dry-run command. Otherwise start.jar must always be updated if JVM provides some other parameter syntax in the future.