Skip to content

Conversation

andrross
Copy link
Member

Update the forbidden APIs plugin to the latest release and also fix all the usages of APIs that are newly deprecated in JDK 25.

Check List

  • Functionality includes testing.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@andrross andrross requested a review from a team as a code owner October 13, 2025 22:48
@andrross
Copy link
Member Author

@cwperks I'd appreciate a review whenever you get a few minutes. I've removed some code, all of which involved using permissions that the java agent does not enforce. Please let me know if this makes sense.

I've also removed some code that used "UnresolvedPermission". As far as I know, our policy file parser will not produce this permission so it is safe to remove.

Copy link
Contributor

❌ Gradle check result for 95fcccf: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@reta
Copy link
Contributor

reta commented Oct 14, 2025

@cwperks I'd appreciate a review whenever you get a few minutes. I've removed some code, all of which involved using permissions that the java agent does not enforce. Please let me know if this makes sense.

@andrross the agent does support bare minimum (more or less) set of permissions, but we could certainly work it through to add more and more. If we remove these now, we won't be able to cover these flow anymore. If we could keep them and than decide - which ones we will not support, we could clean up not only some classes but policies as well, holistically. What do you think?

@cwperks
Copy link
Member

cwperks commented Oct 14, 2025

I think its reasonable to remove UnresolvedPermission based on the javadoc here: https://github.com/openjdk/jdk17/blob/master/src/java.base/share/classes/java/security/UnresolvedPermission.java#L40-L43

The UnresolvedPermission class is used to hold Permissions that
were "unresolved" when the Policy was initialized.
An unresolved permission is one whose actual Permission class
does not yet exist at the time the Policy is initialized (see below).

My understanding is that the java agent has permissions resolved at the time of initialization including permissions that require env variable or system prop replacement.

What is the purpose of UnresolvedPermissions? Could that be a permission with system prop substitution where it assumes the system prop changes at runtime after the policy has been initialized?

Edit: I think my understanding is wrong about this class of permission.

Other permission classes may not yet exist during Policy initialization.

Do we plan to support custom permissions classes in the future? If not then I think its reasonable to remove.

@andrross
Copy link
Member Author

@cwperks I'd appreciate a review whenever you get a few minutes. I've removed some code, all of which involved using permissions that the java agent does not enforce. Please let me know if this makes sense.

@andrross the agent does support bare minimum (more or less) set of permissions, but we could certainly work it through to add more and more. If we remove these now, we won't be able to cover these flow anymore. If we could keep them and than decide - which ones we will not support, we could clean up not only some classes but policies as well, holistically. What do you think?

That's fair. Probably better to suppress the warnings in this PR and figure out the path forward for the deprecated classes separately. I'll push an update.

Update the forbidden APIs plugin to the latest release and also fix or
suppress all the usages of APIs that are newly deprecated in JDK 25.

Signed-off-by: Andrew Ross <[email protected]>
Copy link
Contributor

✅ Gradle check result for 97099d5: SUCCESS

Copy link

codecov bot commented Oct 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.09%. Comparing base (252cff8) to head (97099d5).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #19623      +/-   ##
============================================
- Coverage     73.11%   73.09%   -0.02%     
- Complexity    70661    70680      +19     
============================================
  Files          5724     5725       +1     
  Lines        323498   323733     +235     
  Branches      46852    46880      +28     
============================================
+ Hits         236518   236636     +118     
- Misses        67846    68000     +154     
+ Partials      19134    19097      -37     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@andrross andrross merged commit 4ab0ae9 into opensearch-project:main Oct 14, 2025
33 checks passed
@andrross andrross deleted the deprecations-jdk25 branch October 14, 2025 23:45
peteralfonsi pushed a commit to peteralfonsi/OpenSearch that referenced this pull request Oct 15, 2025
Update the forbidden APIs plugin to the latest release and also fix or
suppress all the usages of APIs that are newly deprecated in JDK 25.

Signed-off-by: Andrew Ross <[email protected]>
rgsriram pushed a commit to rgsriram/OpenSearch that referenced this pull request Oct 18, 2025
Update the forbidden APIs plugin to the latest release and also fix or
suppress all the usages of APIs that are newly deprecated in JDK 25.

Signed-off-by: Andrew Ross <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants