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

Java Info Contributor #28136

Conversation

jonatan-ivanov
Copy link
Member

This InfoContributor add Java-related details to actuator's info endpoint as part of the Enhanced Observability effort: #25476
When configured, it looks like this:

  "java": {
    "vendor": "Eclipse Adoptium",
    "version": "17",
    "runtime": {
      "name": "OpenJDK Runtime Environment",
      "version": "17+35"
    },
    "vm": {
      "name": "OpenJDK 64-Bit Server VM",
      "vendor": "Eclipse Adoptium",
      "version": "17+35"
    }
  }

If you are curious what would this look like on other OpenJDK distributions or using native-image, I tested 30+ of the most popular OpenJDK images (latest and current LTS), you can see the results in this public gist: java-info.md. You can see where the values above are different (e.g.: different java and vm vendors or different jre and vm versions, etc.)
If you want to run your own tests, please take a look at this repo: https://github.com/jonatan-ivanov/app-info

A couple of notes regarding these changes:

  • The JavaInfoContributor is auto-configured by default
  • I haven't written docs, if these changes seems initially ok, I will add them
  • I used real classes instead of Map<String,Object> in the InfoContributor
  • I'm not sure if serialized output is tested anywhere, I did not write any tests to cover this but I would be happy to, I just need some guidance about where to put them
  • JMX is intentionally not used to keep the implementation simpler and native-image still has some challenges with JMX

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 26, 2021
@jonatan-ivanov jonatan-ivanov mentioned this pull request Sep 26, 2021
15 tasks
@wilkinsona
Copy link
Member

Thanks for the PR, @jonatan-ivanov.

Note to the Boot team's future selves: I've labelled this for merging with amendments as we might rename some of the DTOs.

@wilkinsona wilkinsona added for: merge-with-amendments Needs some changes when we merge and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 30, 2021
@wilkinsona wilkinsona modified the milestones: 2.6.0-RC1, 2.6.x Sep 30, 2021
@wilkinsona wilkinsona added the type: enhancement A general enhancement label Sep 30, 2021
@snicoll snicoll self-assigned this Oct 1, 2021
snicoll pushed a commit to snicoll/spring-boot that referenced this pull request Oct 1, 2021
snicoll added a commit to snicoll/spring-boot that referenced this pull request Oct 1, 2021
snicoll added a commit to snicoll/spring-boot that referenced this pull request Oct 1, 2021
@snicoll snicoll modified the milestones: 2.6.x, 2.6.0-RC1 Oct 5, 2021
snicoll pushed a commit that referenced this pull request Oct 5, 2021
snicoll added a commit that referenced this pull request Oct 5, 2021
@snicoll snicoll closed this in f1c1a4f Oct 5, 2021
@vpavic
Copy link
Contributor

vpavic commented Oct 5, 2021

What's the reason this kind of info gets exposed by default? Similar like with #25134, I see this as a potential source of information leakage that can potentially lead to exploits.

@wilkinsona
Copy link
Member

Thanks for taking a look at this, @vpavic. We went back and forth on that one quite a bit. Since 2.5, the info endpoint is not exposed by default. When you expose it over HTTP and you're using Spring Security, it is secured by default. In the end, we concluded that should be sufficient.

@jonatan-ivanov jonatan-ivanov deleted the java-info-contributor branch October 5, 2021 17:33
@vpavic
Copy link
Contributor

vpavic commented Oct 8, 2021

Yes, I'm aware of info endpoint now being treated as secured by default. I personally welcome that change.

However, I wouldn't conflate the secure setting with enabling individual contributors that might be source of information leakage. IMO anything that can be categorized as such should be an opt-in feature with a dedicated configuration property. There are no such assumptions with, for example, error attributes (#7872) or favicon (#17925). Or even health endpoint - even if I secure it, I still need to opt-in to expose health contributor's details.

It's also important to take into consideration that many people expose their info endpoints to the public. Even Spring does that itself. So many have likely made adjustments upgrading to 2.5 (where info endpoint became secure) will now get this implicitly when they pick up 2.6.

Finally, but admittedly very much my personal take that many others might not share depending on environments they run their applications in, I really don't see value in this being enabled by default. If I have at least some control over the runtime infrastructure, or am running containers, I pretty much control the JVM runtime so I don't need an application endpoint to tell me details about it as it was a conscious choice. So putting it out there can only be a risk of some kind.

@wilkinsona wilkinsona added the for: team-meeting An issue we'd like to discuss as a team to make progress label Oct 8, 2021
@jonatan-ivanov
Copy link
Member Author

jonatan-ivanov commented Oct 8, 2021

I think I share some of your concerns this being enabled by default vs. opt-in but I disagree with this:

If I have at least some control over the runtime infrastructure, or am running containers, I pretty much control the JVM runtime so I don't need an application endpoint to tell me details about it as it was a conscious choice. So putting it out there can only be a risk of some kind.

The whole idea behind this feature (at least from my side) is making applications more observable so that we can detect things that should never ever happen but somehow they do, e.g.: you think you use a certain Java version because you have control over it and set it explicitly but somehow you end-up with a different version. Let me give you a few examples, I saw every single one of them in production:

  • The deployment solution copied the JRE from versioned folders, somehow there was Java 5 in a folder named java6uXYZ
  • Somebody changed JAVA_HOME manually
  • Somebody set the wrong value in a config file -> wrong version (Spring did something similar too)
  • The wrong config file was referenced/linked during the deployment
  • Deployment config was updated to update the java version (app version did not change) but during the deployment there was a silent rollback (the verification process only checked the app version)
  • People use latest or not specifying the full version (8, 11, etc.) for container images
  • Also, docker used an image from cache and the app did not get the new version
  • Somebody mistagged a docker base image and fixed the issue in a minute; in that minute there were multiple deployments with the wrong Java version
  • Somehow the app was deployed to a box that should have been decommissioned (with an old installation of Java)

From my perspective: if I have at least some control over the runtime infrastructure, or am running containers, I should control the JVM runtime but in fact I don't control it 100% and I never will so I need something (like an application endpoint) to tell me details about it so that I can verify.

@philwebb
Copy link
Member

We're going to flip the default. See #28310 (we couldn't reopen this PR because the branch is gone)

@philwebb philwebb removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Oct 13, 2021
@vpavic
Copy link
Contributor

vpavic commented Oct 13, 2021

Thanks @philwebb.

@jonatan-ivanov I acknowledge that things you listed can happen but most of those are IMO simply negligence of core (software) engineering principles like reproducibility. Builds should be reproducible and the same goes for infra setup/scripting. So I believe that education on those principles is what addresses the listed issues, not another putting another potential problem (information leakage) on top of it.

@jonatan-ivanov
Copy link
Member Author

I agree with you on this wholeheartedly except the very end: no matter what you do, software is insanely complex and we are all humans, we all make mistakes, you can't really avoid it (sometimes software and also hardware make mistakes too).

This is why we have logs, metrics, distributed tracing, this is why write tests, and also this is why we have Spring Boot Actuator. You can call it information leakage but I would call it "observability" instead.
Again, I agree on disabling this by default but I disagree on not having it at all.

@vpavic
Copy link
Contributor

vpavic commented Oct 13, 2021

I don't think anyone argued in the direction of not having it at all - this can certainly be helpful in some cases.

@dmengelt
Copy link

@jonatan-ivanov how do you see it regarding java.vendor.version? With this new JavaInfoContributor I now get the following:

{
  "java": {
    "vendor": "Amazon.com Inc.",
    "version": "11.0.13",
    "runtime": {
      "name": "OpenJDK Runtime Environment",
      "version": "11.0.13+8-LTS"
    },
    "jvm": {
      "name": "OpenJDK 64-Bit Server VM",
      "vendor": "Amazon.com Inc.",
      "version": "11.0.13+8-LTS"
    }
  }
}

However, the Java version I actually use is: Corretto-11.0.13.8.1 (11.0.13.8.1)
Currently I can get the full version string with: info.java-version=${java.vendor.version}

Do you think that exposing ${java.vendor.version} would also make sense?

@jonatan-ivanov
Copy link
Member Author

jonatan-ivanov commented Nov 23, 2021

@dmengelt I don't quite remember why didn't I include java.vendor.version in the original PR.
I guess I did not because in general, it was non-deterministic/very valuable: I tested the properties with 40+ different Java distributions (+1 GraalVM) and sometimes the value was missing, sometimes it did not add new information.

Let me re-run the tests tonight and get back to you (you can find them here if you are curious, see test.sh).

@jonatan-ivanov
Copy link
Member Author

I re-ran the tests against 35 images, TL;DR: it seems only the Corretto 11 and 17 images has some value that provide extra information (maybe the Zulu 11 and 17 images too but I'm not sure, see below).

Based on this I can see why past me did not include it but, since it can make sense in some cases, I think at least it worth to ask the Boot Team in an issue (should be fairly easy to implement but it could potentially change the response: vendor.name, vendor.version).

Details

  • 17 of the images have this property 18 of them do not: see java-vendor-versions.md
  • Out of the 17 that have this property
    • some of them do not make sense to me
      • openjdk:11-jre-slim has java.vendor.version: 18.9 (while java.version: 11.0.13)
      • sapmachine:11 has java.vendor.version: SapMachine
      • sapmachine:17 has java.vendor.version: SapMachine
    • or do not add new info
      • adoptopenjdk:11-jre-openj9
        • java.vendor.version: AdoptOpenJDK-11.0.11+9
        • java.runtime.version: 11.0.11+9
      • adoptopenjdk:16-jre-openj9
        • java.vendor.version: AdoptOpenJDK-16.0.1+9
        • java.runtime.version: 16.0.1+9
    • eclipse-temurin:11-jre
      • java.vendor.version: Temurin-11.0.13+8
      • java.runtime.version: 11.0.13+8
    • eclipse-temurin:11-jre-alpine
      • java.vendor.version: Temurin-11.0.13+8
      • java.runtime.version: 11.0.13+8
    • eclipse-temurin:17-alpine
      • java.vendor.version: Temurin-17.0.1+12
      • java.runtime.version: 17.0.1+12
    • eclipse-temurin:17-jdk
      • java.vendor.version: Temurin-17.0.1+12
      • java.runtime.version: 17.0.1+12
    • or makes me confused (I guess it's the internal build version so should make sense but we can track this in java.runtime.version)
      • azul_zulu-openjdk-alpine:11-jre
        • java.vendor.version: Zulu11.52+13-CA
        • java.version: 11.0.13
        • java.runtime.version: 11.0.13+8-LTS (java.vm.version is the same)
      • azul_zulu-openjdk-alpine:17-jre
        • java.vendor.version: Zulu17.30+15-CA
        • java.version: 17.0.1
        • java.runtime.version: 17.0.1+12-LTS (java.vm.version is the same)
      • azul_zulu-openjdk:11
        • java.vendor.version: Zulu11.52+13-CA
        • java.version: 11.0.13
        • java.runtime.version: 11.0.13+8-LTS (java.vm.version is the same)
      • azul_zulu-openjdk:17
        • java.vendor.version: Zulu17.30+15-CA
        • java.version: 17.0.1
        • java.runtime.version: 17.0.1+12-LTS(java.vm.version is the same)
  • The Corretto 11 and 17 images make sense though (Corretto 8 images do not have this property)

@jonatan-ivanov
Copy link
Member Author

jonatan-ivanov commented Nov 23, 2021

One more thing, I found: it seems this was introduced in JEP-322 (Java 10) but they wanted to remove it though the community found it useful so they kept it. With the comments on that issue I would say that the property makes sense for the Zulu and the Corretto images and might make sense for more distributions in the future.

@dmengelt
Copy link

thanks @jonatan-ivanov

@wilkinsona
Copy link
Member

Thanks very much for the extensive research, @jonatan-ivanov.

@Axinet
Copy link

Axinet commented Oct 17, 2022

Are there any particular reasons why java.home and java.vendor.url are not included in the JavaInfo class?

@wilkinsona
Copy link
Member

wilkinsona commented Oct 18, 2022

I can't see much benefit in knowingjava.home. What would be the benefit of knowing where the JVM is installed? I'm struggling to think of a strong need for java.vendor.url either. If you have a need, feel free to open a new issue that makes a case for them and we can consider it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: merge-with-amendments Needs some changes when we merge type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants