Support ALPN in latest JDK 8 builds

[ignasi35]

References #6997

The Jetty ALPN agent added support for JVM provided ALPN (that is, JDK8u252 and later) in 2.0.10.

Netty did the necessary fixes in 4.1.49.Final but 4.1.50.Final is already out.

I've tested locally with both AdoptOpen JDK 8u242 and AdoptOpen JDK 8u252.

[ignasi35]

Oh, the build in Travis uses openjdk8 which currently translates into openjdk version "1.8.0_242".

Should I change the build to have an extra job run on 1.8.0_252?

[ignasi35]

Macos Expected — Waiting for status to be reported

IDK what's going on here, but that check has been in the waiting state for several days now.

[ignasi35]

I've turned this into a draft because of:

Should I change the build to have an extra job run on 1.8.0_252?

[ejona86]

IDK what's going on here, but that check has been in the waiting state for several days now.

Macos and many other CIs run on different infrastructure which require us to give an "okay" to run it. Don't worry about it, unless it is failing 😄

Should I change the build to have an extra job run on 1.8.0_252?

Simply upgrading to the newer version of OpenJDK would be fine. 252 is available on Xenial. And it was released sometime around April 15th. It looks like the Travis-CI images were built before that date:

Build image provisioning date and time
Tue Feb 18 10:44:28 UTC 2020

I think we just need to add this snippet to .travis.yml:

addons:
  apt:
    update: true
    packages:
    - openjdk-8-jre-headless

Although it should also include a comment saying we want to upgrade to u252.

Note that the upgrade of Netty isn't that simple. Netty can be fickle to upgrade. We do a lot of testing when upgrading it. (We will have to do that testing.)

If the failure was that Jetty_ALPN wasn't upgraded, that means we're probably preferring Jetty ALPN over the Java 9 ALPN API. As things are structured in Netty, I guess we can't easily test both Jetty ALPN and the Java 9 ALPN API within the same JVM.

I think we may just say "a manual test is fine." In which case you should disable Jetty ALPN locally and make sure things work. You can disable it by commenting out the javaagent argument to the test. In pre-u252 you should see TlsTest[JDK] skipped and in u252 you should see it successful. (You can see the results in netty/build/reports/tests/test/index.html)

[ignasi35]

Simply upgrading to the newer version of OpenJDK would be fine. [252 is available] ...

Done. It's running now, let's see...

If the failure was that Jetty_ALPN wasn't upgraded, that means we're probably preferring Jetty ALPN over the Java 9 ALPN API. As things are structured in Netty, I guess we can't easily test both Jetty ALPN and the Java 9 ALPN API within the same JVM.

I think the Jetty_ALPN is required since only in 2.0.10 there's support for jdk8u252 in the form of noop. Using a 2.0.9 of Jetty_ALPN in jdk8u252 will instrument an already valid code causing runtime failures.

I think setting up two jobs in TravisCI with JDK8u242 and JDK8u252 should do the trick. Both jobs should run the same code/build/settings and that should include Jetty_ALPN 2.0.10.

I think we may just say "a manual test is fine." In which case you should disable Jetty ALPN locally and make sure things work. You can disable it by commenting out the javaagent argument to the test. In pre-u252 you should see TlsTest[JDK] skipped and in u252 you should see it successful. (You can see the results in netty/build/reports/tests/test/index.html)

I've commented the use of the Jetty_ALPN agent and done manual testing (./gradlew :grpc-netty:test) with the outcome you predicted:

1. running with 8.0.242.hs-adpt (no Jetty) the TlsTest tests are ignored
2. running with 8.0.252.hs-adpt (no Jetty) the TlsTest tests succeed

Then I enabled the Jett-ALPN agent in tests back again and re-run both jobs (./gradlew :grpc-netty:test):

1. running with 8.0.242.hs-adpt (with Jetty) the TlsTest tests succeed
2. running with 8.0.252.hs-adpt (with Jetty) the TlsTest tests succeed

[ejona86]

@ignasi35, excellent! Thank you for the testing.

I think the Jetty_ALPN is required since only in 2.0.10 there's support for jdk8u252 in the form of noop. Using a 2.0.9 of Jetty_ALPN in jdk8u252 will instrument an already valid code causing runtime failures.

Yeah, the agent chooses the correct version for the particular JRE version. We should upgrade to version 2.0.10 of the agent, but it does look like it becomes a noop on newer JRE versions.

I think setting up two jobs in TravisCI with JDK8u242 and JDK8u252 should do the trick.

So we actually already test against the Java 9 ALPN API via the openjdk11 test. I'm okay with stopping active testing of Jetty ALPN. Once we upgrade to the newer Netty we'll need to update our SECURITY.md to mention that Jetty ALPN does nothing starting in 8u252. And we could merge this PR.

So we're temporarily blocked on the Netty upgrade.

[ignasi35]

Friendly nudge!

Hi @ejona86, how is this moving forward? Any plans to get the Netty upgrade completed?

Or did I miss something and you are waiting on me to complete anything on this PR?

[ejona86]

No, we weren't waiting for you on the PR. We were waiting on someone testing the new Netty version internally. I've tested v4.1.51 and it passed, so I'll be sending out an update of Netty.

[ejona86]

With #7358 in, I think that completes all the pieces this PR was hoping to achieve. Closing.

[ignasi35]

Thanks @ejona86