Adjust jdk-api-extractor output to be compatible with the public-callers-finder #135824

mosche · 2025-10-02T07:04:31Z

Adjust jdk-api-extractor output to be compatible with the public-callers-finder to easily find the transitive public surface of new additions that require entitlement instrumentation.

Relates to ES-11757

…ers-finder to easily find the transitive public surface of new additions that require entitlement instrumentation. Relates to ES-11757

elasticsearchmachine · 2025-10-02T07:04:56Z

Pinging @elastic/es-core-infra (Team:Core/Infra)

rjernst

Looks fine just a couple questions/asks for the future

rjernst · 2025-10-02T11:57:27Z

...titlement/tools/common/src/main/java/org/elasticsearch/entitlement/tools/ExternalAccess.java

            return EnumSet.of(ExternalAccess.PUBLIC_CLASS, ExternalAccess.PUBLIC_METHOD);
        }
+        if ("PROTECTED".equals(accessAsString)) {
+            return EnumSet.of(ExternalAccess.PUBLIC_CLASS, ExternalAccess.PROTECTED_METHOD);


Why public class and not protected class? What does this class represent (a short javadoc would be nice)?

PROTECTED refers to methods, that's only used by the jdk-api-extractor. The class had to be public in this case.
I can add a comment for that 👍

@ldematte maybe you could also chime in here, this part of the old public-callers-finder was a bit confusing.
fromPermissions is invoked as follows:

elasticsearch/libs/entitlement/tools/public-callers-finder/src/main/java/org/elasticsearch/entitlement/tools/publiccallersfinder/FindUsagesClassVisitor.java

Lines 123 to 128 in 7cbc236

EnumSet<ExternalAccess> externalAccess = ExternalAccess.fromPermissions(

moduleExports.contains(getPackageName(className)),

accessibleViaInterfaces || (classAccess & ACC_PUBLIC) != 0,

(methodAccess & ACC_PUBLIC) != 0,

(methodAccess & ACC_PROTECTED) != 0

);

so being exported is a requirement for PUBLIC_CLASS in addition to either extending a public interface or being public:

elasticsearch/libs/entitlement/tools/common/src/main/java/org/elasticsearch/entitlement/tools/ExternalAccess.java

Lines 27 to 48 in 7cbc236

public static EnumSet<ExternalAccess> fromPermissions(

boolean packageExported,

boolean publicClass,

boolean publicMethod,

boolean protectedMethod

) {

if (publicMethod && protectedMethod) {

throw new IllegalArgumentException();

}

EnumSet<ExternalAccess> externalAccesses = EnumSet.noneOf(ExternalAccess.class);

if (publicMethod) {

externalAccesses.add(ExternalAccess.PUBLIC_METHOD);

} else if (protectedMethod) {

externalAccesses.add(ExternalAccess.PROTECTED_METHOD);

}

if (packageExported && publicClass) {

externalAccesses.add(ExternalAccess.PUBLIC_CLASS);

}

return externalAccesses;

}

isExternallyAccessible seems to be to restrictive as it requires PUBLIC_CLASS. That practically excludes all classes that are not exported but which are externally accessible via exported public interfaces (or similarly super classes), doesn't it?

elasticsearch/libs/entitlement/tools/common/src/main/java/org/elasticsearch/entitlement/tools/ExternalAccess.java

Lines 50 to 53 in 7cbc236

public static boolean isExternallyAccessible(EnumSet<ExternalAccess> access) {

return access.contains(ExternalAccess.PUBLIC_CLASS)

&& (access.contains(ExternalAccess.PUBLIC_METHOD) || access.contains(ExternalAccess.PROTECTED_METHOD));

}

I think you are right: looking at the code you posted, it seems to me that PUBLIC_CLASS is accessibleViaInterfaces || (classAccess & ACC_PUBLIC) != 0, but also packageExported.
The fix here is maybe not too complicated: the boolean accessibleViaInterfaces should be changed to be true only if moduleExports.contains(getPackageName(interfaceName) (I think that is not the case currently?), and passed separately.
Then we can change PUBLIC_CLASS to be true if packageExported && (classAccess & ACC_PUBLIC) != 0 OR accessibleViaInterfaces.
Or something similar.

Thanks @ldematte , I'll follow up with a separate PR for this 👍

I wanted to give this a go, unfortunately it is more involved. Having a closer look, i'm seeing a couple if issues:

moduleExports.contains(getPackageName(interfaceName) is already checked, but not necessarily correct. the interface might belong to another module.

superName needs to be treated the same way as an interface

and last, accessibleViaInterfaces currently applies to all public methods. In case of interfaces like AutoClosable that could be very misleading, reporting far too much.

I'll create a separate issue for this.

moduleExports.contains(getPackageName(interfaceName) is already checked, but not necessarily correct. the interface might belong to another module.

Yep, that's why I think we need to do that when we compute and pass accessibleViaInterfaces (so at the call side, not here).
You might be right with superName; I acted under the assumption that were the JDK wants to expose something with an internal implementation it does so via interfaces, but it might be that they use e.g. abstract classes.

And yes, this need to be per-method, which I think is going to be the most complex part.
I want this to be correct, but not sure how urgent it is; probably we'd like to have it correct before JDK 26?
I wonder if this could be a good "first issue", e.g. something for onboarding? It's entitlements related, but this being a separate project it could be quite self-contained

yes, I agree... I'm also leaning towards postponing this ➕

regarding accessibleViaInterfaces : findAccessibility currently uses the exports of the current class rather than the exports of the module the interface originates from.

@Override public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) { ... if (interfaces.length > 0) { this.accessibleViaInterfaces = findAccessibility(interfaces, moduleExports); } } private static boolean findAccessibility(String[] interfaces, Set<String> moduleExports) { var accessibleViaInterfaces = false; for (var interfaceName : interfaces) { if (moduleExports.contains(getPackageName(interfaceName))) { var interfaceType = Type.getObjectType(interfaceName); try { var clazz = Class.forName(interfaceType.getClassName()); if (clazz.accessFlags().contains(AccessFlag.PUBLIC)) { accessibleViaInterfaces = true; } } catch (ClassNotFoundException ignored) {} } } return accessibleViaInterfaces; }

Yeah, I was suspecting this was the case :/

rjernst · 2025-10-02T12:00:24Z

libs/entitlement/tools/jdk-api-extractor/README.md

+# review new additions next for critical ones that should require entitlements
+# once done, remove all lines that are not considered critical and run the public-callers-finder to report
+# the transitive public surface for these additions
+./gradlew :libs:entitlement:tools:public-callers-finder:run -Druntime.java=25 --args="api-jdk25-additions.tsv true"


for followup: can we make this use the latest jdk (or maybe the bundled?) so we don't have to specify it? And why --args instead of specific args like --output-file?

for followup: can we make this use the latest jdk (or maybe the bundled?)

that's mostly for reference in the examples, I'll update the docs to mention it will default to the bundled JDK if not provided

And why --args instead of specific args like --output-file?

That's how gradle run passes args to the main class. We could use Gradle properties (-PoutputFile=api-jdk25-additions.tsv) and pass onto the args in the build script. But --output-file=api-jdk25-additions.tsv can't be done easily I think.

Ah I see, this is a task created by the gradle application plugin. You can have --output-file if you define the task yourself. @mark-vieira I assume there is no way to add -- inputs to a task as part of configuration rather than the definition of the task?

I'll follow up with a separate PR to simplify how to invoke these tools. @mark-vieira any advice?

Adjust jdk-api-extractor output to be compatible with the public-call…

81d903a

…ers-finder to easily find the transitive public surface of new additions that require entitlement instrumentation. Relates to ES-11757

mosche requested a review from a team as a code owner October 2, 2025 07:04

mosche added >refactoring :Core/Infra/Entitlements Entitlements infrastructure labels Oct 2, 2025

elasticsearchmachine added Team:Core/Infra Meta label for core/infra team v9.3.0 labels Oct 2, 2025

[CI] Auto commit changes from spotless

6050dbe

rjernst approved these changes Oct 2, 2025

View reviewed changes

mosche added 2 commits October 6, 2025 09:37

update comment

f296981

Merge branch 'main' into jdk/api-extractor-public-callers

15987f9

mosche merged commit 254e1f5 into elastic:main Oct 6, 2025
34 checks passed

mosche mentioned this pull request Oct 6, 2025

Extend public-callers-finder to check if methods are instrumented with entitlements. #136022

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Adjust jdk-api-extractor output to be compatible with the public-callers-finder #135824

Adjust jdk-api-extractor output to be compatible with the public-callers-finder #135824

mosche commented Oct 2, 2025

Uh oh!

elasticsearchmachine commented Oct 2, 2025

Uh oh!

rjernst left a comment

Uh oh!

rjernst Oct 2, 2025

Uh oh!

mosche Oct 2, 2025

Uh oh!

ldematte Oct 6, 2025

Uh oh!

mosche Oct 6, 2025

Uh oh!

mosche Oct 6, 2025

Uh oh!

ldematte Oct 6, 2025

Uh oh!

mosche Oct 6, 2025

Uh oh!

ldematte Oct 6, 2025

Uh oh!

rjernst Oct 2, 2025

Uh oh!

mosche Oct 2, 2025 •

edited

Loading

Uh oh!

mosche Oct 2, 2025

Uh oh!

rjernst Oct 2, 2025

Uh oh!

mosche Oct 6, 2025

Uh oh!

Uh oh!

Uh oh!

	EnumSet<ExternalAccess> externalAccess = ExternalAccess.fromPermissions(
	moduleExports.contains(getPackageName(className)),
	accessibleViaInterfaces \|\| (classAccess & ACC_PUBLIC) != 0,
	(methodAccess & ACC_PUBLIC) != 0,
	(methodAccess & ACC_PROTECTED) != 0
	);

	public static EnumSet<ExternalAccess> fromPermissions(
	boolean packageExported,
	boolean publicClass,
	boolean publicMethod,
	boolean protectedMethod
	) {
	if (publicMethod && protectedMethod) {
	throw new IllegalArgumentException();
	}

	EnumSet<ExternalAccess> externalAccesses = EnumSet.noneOf(ExternalAccess.class);
	if (publicMethod) {
	externalAccesses.add(ExternalAccess.PUBLIC_METHOD);
	} else if (protectedMethod) {
	externalAccesses.add(ExternalAccess.PROTECTED_METHOD);
	}

	if (packageExported && publicClass) {
	externalAccesses.add(ExternalAccess.PUBLIC_CLASS);
	}
	return externalAccesses;
	}

	public static boolean isExternallyAccessible(EnumSet<ExternalAccess> access) {
	return access.contains(ExternalAccess.PUBLIC_CLASS)
	&& (access.contains(ExternalAccess.PUBLIC_METHOD) \|\| access.contains(ExternalAccess.PROTECTED_METHOD));
	}

Adjust jdk-api-extractor output to be compatible with the public-callers-finder #135824

Adjust jdk-api-extractor output to be compatible with the public-callers-finder #135824

Conversation

mosche commented Oct 2, 2025

Uh oh!

elasticsearchmachine commented Oct 2, 2025

Uh oh!

rjernst left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mosche Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mosche Oct 2, 2025 •

edited

Loading