GoogleCloudPlatform / appengine-plugins

A client Java library to manage App Engine Java applications for any project that performs App Engine Java application management. For example, the Maven, Gradle and Eclipse App Engine plugins, custom user tools, etc.
Apache License 2.0
40 stars 26 forks source link

java17 runtime support for launcher when using a jdk17 build and run env #893

Closed ludoch closed 2 years ago

ludoch commented 2 years ago

The core plugin still refers to isSandboxEnforced() which can be deleted entirely for java7 which is not supported (everything with java7 string can be deleted including tests But when using a JDK17 for the java17 runtime (same logic as java8), the devappserver launcher does not pass a needed flag for the devappserver process very similar to the .sh and .cmd launcher, as seen as in https://github.com/GoogleCloudPlatform/appengine-java-standard/issues/13 I will fix this one by adding the 3:

--add-opens java.base/java.net=ALL-UNNAMED --add-opens java.base/java.base/sun.net.www.protocol.http=ALL-UNNAMED --add-opens java.base/java.base/sun.net.www.protocol.https=ALL-UNNAMED

flags in the scripts but we also need to add the flag in com.google.cloud.tools.appengine.operations.DevServer.java which is also a process launcher written in java which does not create the good command line when running on jdk17.

Thanks!

ludoch commented 2 years ago

To repro, use a JDK17 for a mvn appengine:run command on a GAE project and you will see:

--- appengine-maven-plugin:2.4.3:run (default-cli) @ courteline --- Aug 05, 2022 3:10:29 PM com.google.cloud.tools.appengine.operations.DevAppServerRunner run INFO: submitting command: /Users/ludo/.sdkman/candidates/java/17.0.3-tem/bin/java -Duse_jetty9_runtime=true -D--enable_all_permissions=true -Dappengine.sdk.root=/Users/ludo/Library/Application Support/google-cloud-tools-java/managed-cloud-sdk/LATEST/google-cloud-sdk/platform/google_appengine/google/appengine/tools/java -cp /Users/ludo/Library/Application Support/google-cloud-tools-java/managed-cloud-sdk/LATEST/google-cloud-sdk/platform/google_appengine/google/appengine/tools/java/lib/appengine-tools-api.jar com.google.appengine.tools.development.DevAppServerMain --application=courteline-nantes --allow_remote_shutdown --disable_update_check --no_java_agent /Users/ludo/a/courteline/target/courteline-1.0-SNAPSHOT GCLOUD: java.lang.RuntimeException: Unable to create a DevAppServer GCLOUD: at com.google.appengine.tools.development.DevAppServerFactory.doCreateDevAppServer(DevAppServerFactory.java:378) GCLOUD: at com.google.appengine.tools.development.DevAppServerFactory.createDevAppServer(DevAppServerFactory.java:310) GCLOUD: at com.google.appengine.tools.development.DevAppServerMain$StartAction.apply(DevAppServerMain.java:384) GCLOUD: at com.google.appengine.tools.util.Parser$ParseResult.applyArgs(Parser.java:58) GCLOUD: at com.google.appengine.tools.development.DevAppServerMain.run(DevAppServerMain.java:258) GCLOUD: at com.google.appengine.tools.development.DevAppServerMain.main(DevAppServerMain.java:249) GCLOUD: Caused by: java.lang.ExceptionInInitializerError GCLOUD: at com.google.appengine.tools.development.DevAppServerImpl.(DevAppServerImpl.java:135) GCLOUD: at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) GCLOUD: at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77) GCLOUD: at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) GCLOUD: at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499) GCLOUD: at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480) GCLOUD: at com.google.appengine.tools.development.DevAppServerFactory.doCreateDevAppServer(DevAppServerFactory.java:363) GCLOUD: ... 5 more GCLOUD: Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make static java.net.URLStreamHandler java.net.URL.getURLStreamHandler(java.lang.String) accessible: module java.base does not "opens java.net" to unnamed module @531be3c5 GCLOUD: at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354) GCLOUD: at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297) GCLOUD: at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:199) GCLOUD: at java.base/java.lang.reflect.Method.setAccessible(Method.java:193) GCLOUD: at com.google.appengine.tools.development.StreamHandlerFactory.(StreamHandlerFactory.java:52) GCLOUD: ... 12 more

ludoch commented 2 years ago

As a side effect, these flags can be removed:

-Duse_jetty9_runtime=true -D--enable_all_permissions=true

as we are only on jetty9 now, and we do not have a security manager for java8, 11 and 17.

ludoch commented 2 years ago

Confirmed that for command line mode, adding --add-opens java.base/java.net=ALL-UNNAMED --add-opens java.base/java.base/sun.net.www.protocol.http=ALL-UNNAMED --add-opens java.base/java.base/sun.net.www.protocol.https=ALL-UNNAMED allows the local devappserver to boot with JDK17

elefeint commented 2 years ago

Ah, I see. I've added documentation for users to add the extra flags in GoogleCloudPlatform/app-maven-plugin#455, but we can definitely make the changes ourselves in core.

Long-term, it would be best if devserver did not modify JDK classes visibility...