Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Method implementation and logic isOnGAEStandard7() is incorrect for java21 jetty which does not define the old jetty logging class. #1376

Open
ludoch opened this issue Mar 13, 2024 · 2 comments
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@ludoch
Copy link

ludoch commented Mar 13, 2024

An Appengine customer recently filed this bug:

We recently upgraded to Java 21 and noticed a change in behavior when calling GoogleCredentials.getApplicationDefault()

Before Java 21, it was returning us a ComputeEngineCredentials, which was expected.But now it is returning us an AppEngineCredentials so we have to set a environment to revert to the old behavior.

We suspect maybe one of the conditions changed in isOnGaeStandard7() on Java 21 runtime.

See

return GAE_RUNTIME_VERSION != null
&& (SPECIFICATION_VERSION.equals("1.7") || RUNTIME_JETTY_LOGGER == null);

While it might have been true for GAE Java7 (now completely removed), for the GAE runtimes java8,11 and 17, RUNTIME_JETTY_LOGGER was never null, and customers expect anyway ComputeEngineCredentials, not AppEngineCredentials by default!

But the new GAE environment supporting Java21 GAE, RUNTIME_JETTY_LOGGER is now null, as Jetty12 entirely rewrote their logging logic and doe not define this Jetty internal property.

Currently, App Engine Java21 code had to defined again this property to avoid app regression, but we need to fix this auth-library bug for newer releases so we can avoid doing this ugly workaround for GAE customers.
Maybe remove entirely the special case for GAE to avoid regression for existing GAE customers.

@ludoch
Copy link
Author

ludoch commented Mar 13, 2024

Google internal bug is b/328772995

Related temp PR GoogleCloudPlatform/appengine-java-standard#100

@ludoch ludoch changed the title Method implementation and logic isOnGAEStandard7() was Method implementation and logic isOnGAEStandard7() is incorrect for java21 jetty whicj does not define the old jetty logging class. Mar 13, 2024
@ludoch ludoch changed the title Method implementation and logic isOnGAEStandard7() is incorrect for java21 jetty whicj does not define the old jetty logging class. Method implementation and logic isOnGAEStandard7() is incorrect for java21 jetty which does not define the old jetty logging class. Mar 13, 2024
@TimurSadykov TimurSadykov self-assigned this Mar 21, 2024
@TimurSadykov TimurSadykov added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Mar 21, 2024
@TimurSadykov
Copy link
Member

thanks for the detailed explnation, looking :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

2 participants