Bootstrap entitlements for testing #129268

prdoyle · 2025-06-11T15:22:38Z

Add entitlement enforcement during ordinary unit tests.

This does not yet cover tests that run ES nodes; only ordinary unit tests.

See ES-11597.

…source. Using getResource makes this sensitive to unrelated classpath entries, such as the entitlement bridge library, that get prepended to the classpath.

Using the root logger makes this sensitive to unrelated logging, such as from the entitlement library.

…titlements. Taking the actual module name from the class doesn't work in tests, where those classes are loaded from the classpath and so their module info is misleading.

elasticsearchmachine · 2025-06-17T17:43:36Z

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

ldematte

I think this looks good; I've skipped configureEntitlements as I have far from the necessary gradle knowledge.
My main concern is with TestPolicyManager/PolicyManager (see comments)

build-tools/src/main/java/org/elasticsearch/gradle/test/TestBuildInfoPlugin.java

libs/build.gradle

server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java

server/src/test/java/org/elasticsearch/bootstrap/WithoutEntitlementsMetaTests.java

...ramework/src/main/java/org/elasticsearch/entitlement/bootstrap/TestEntitlementBootstrap.java

ldematte · 2025-06-18T07:55:08Z

.../framework/src/main/java/org/elasticsearch/entitlement/runtime/policy/TestPolicyManager.java

+    boolean isTriviallyAllowingTestCode;
+
+    /**
+     * We don't have modules in tests, so we can't use the inherited map of entitlements per module.


This is a thing that might be confusing, in the future and/or while debugging.
In general, there are a lot of other things in PolicyManager that do not make sense in TestPolicyManager, e.g. everything related to modules like SYSTEM_LAYER_MODULES, SERVER_LAYER_MODULES, etc.)
Can we split this better? E.g. make PolicyManager abstract and move everything that is not common to TestPolicyManager to another subclass ProductionPolicyManager (or ElasticsearchPolicyManager)?

Or any other thing really; you know I usually prefer to avoid inheritance, and inject strategies into a common class. I suggested another subclass here as it seems to be the easier approach.

Composition > inheritance of abstract classes > inheritance of concrete classes. 😂

That's a great suggestion.

Thinking back... I think the reason I didn't do this in the first place was that my goal was to avoid adding complexity to production code solely for the sake of testing. From the POV of the production code, the refactoring we've already done can be justified as a way to separate concerns and have different responsibilities in different classes, which is a desirable property even without testing; but I felt like there's no reason to have an AbstractPolicyManager superclass to PolicyManager besides testing, so I shied away from it.

I don't feel strongly about it though.

Technically we would just shuffle code around, moving some things from PolicyManager to AbstractPolicyManager, but...

avoid adding complexity to production code solely for the sake of testing

Yes, that's true and it's always a good principle; however we do this all the time in ES :) not a justification, just a point. It's true that we would add some complexity to production code, but we would separate a concern; the concrete PolicyManager would be effectively a ModuleBasedPolicyManager, if you think about it: I believe the things that will be moved there that we don't want/need to share with the TestPolicyManager are those related to modules.
And I'd weigh in the "clarity" (or confusion) of having unused stuff in TestPolicyManager, especially if someone is debugging a entitlement exception in a unit test.

We can maybe try it in a separate PR (even a follow-up if we are tight in time to close this)

Ok that all makes sense. Let's do this in another PR.

BTW I love the name ModuleBasedPolicyManager.

.../framework/src/main/java/org/elasticsearch/entitlement/runtime/policy/TestPolicyManager.java

ldematte · 2025-06-18T08:32:23Z

x-pack/plugin/core/src/main/plugin-metadata/entitlement-policy.yaml

@@ -18,6 +18,7 @@ org.apache.httpcomponents.httpasyncclient:
  - manage_threads
 unboundid.ldapsdk:
  - set_https_connection_properties # TODO: review if we need this once we have proper test coverage
+  - inbound_network # For com.unboundid.ldap.listener.LDAPListener


Really? Is this something we use? Looks like an unnecessary security hazard...

I'm not sure. It was definitely exercised by the unit test.

I wonder who would know. Does our team own x-pack-core?

It's kind of unrelated, so if a test hits it it's worth adding it. But I think this is an anomaly, and needs to be investigate (later -- I'm completely fine adding this now to make progress!)
By whoever "owns" x-pack-code, which is going to be really tricky. I guess since this is related to authN/authZ this should be security?

These are alread set in ElasticsearchJavaBasePlugin.

For serverless, those project dependencies don't exist, and we'll need to add the dependencies differently, using Maven coordinates.

It's typically a very very long string, which made Windows angry.

We already get the "needle" in the form of a URI, so this skips a step, and has the benefit of also working on Windows.

prdoyle self-assigned this Jun 11, 2025

prdoyle added >test Issues or PRs that are addressing/adding tests test-windows Trigger CI checks on Windows auto-backport Automatically create backport pull requests when merged v8.19.0 v9.1.0 :Core/Infra/Entitlements Entitlements infrastructure labels Jun 11, 2025

prdoyle force-pushed the bootstrap-entitlements-for-testing branch 4 times, most recently from e1166dc to d331569 Compare June 17, 2025 12:34

prdoyle added 4 commits June 17, 2025 09:15

Fix ExceptionSerializationTests to use getCodeSource instead of getRe…

2daa627

…source. Using getResource makes this sensitive to unrelated classpath entries, such as the entitlement bridge library, that get prepended to the classpath.

FIx logging tests to use org.elasticsearch.index instead of root logger.

87b58f6

Using the root logger makes this sensitive to unrelated logging, such as from the entitlement library.

Fix entitlement error message by stashing the module name in ModuleEn…

3248fd2

…titlements. Taking the actual module name from the class doesn't work in tests, where those classes are loaded from the classpath and so their module info is misleading.

Ignore server locations whose representative class isn't loaded

afaaf3d

prdoyle force-pushed the bootstrap-entitlements-for-testing branch 2 times, most recently from 7bcefba to ba15751 Compare June 17, 2025 14:37

prdoyle added 13 commits June 17, 2025 10:41

Partial initial implementation

c363a24

System properties: testOnlyClasspath and enableForTests

ee60773

Trivially allow some packages

ad955a5

DEBUG: use TreeMap in TestScopeResolver for readability

3dd3962

Special case bouncycastle for security plugin

caa6a70

Add CONFIG to TestPathLookup

08b16bb

Add the classpath to the source path list for every plugin

c3b6388

Add @WithoutEntitlements to tests that run ES nodes

687b904

Set es.entitlement.enableForTests for all libs

c8f1af6

Use @WithoutEntitlements on ingest plugin tests

38c3f9c

Substitute ALL-UNNAMED for module name in non-modular plugins

f5a7c86

Add missing entitlements found by unit tests

93878f9

Comment in TestScopeResolver

bda4331

ldematte reviewed Jun 18, 2025

View reviewed changes

prdoyle added 3 commits June 18, 2025 09:26

Call out nonServerLibs

d039ba4

Don't build two TestPathLookups

c53e10a

More comments for meta-tests

6c1b7fd

prdoyle force-pushed the bootstrap-entitlements-for-testing branch 2 times, most recently from b7e4315 to 6c1b7fd Compare June 18, 2025 14:42

prdoyle and others added 5 commits June 18, 2025 11:32

Remove redundant dependencies for bridgeJarConfig.

8776617

These are alread set in ElasticsearchJavaBasePlugin.

Merge branch 'main' into bootstrap-entitlements-for-testing

3737d0a

Add bridge+agent dependencies only if those exist.

d9517f7

For serverless, those project dependencies don't exist, and we'll need to add the dependencies differently, using Maven coordinates.

[CI] Auto commit changes from spotless

2381c88

Pass testOnlyPath in environment instead of command line.

26baece

It's typically a very very long string, which made Windows angry.

prdoyle force-pushed the bootstrap-entitlements-for-testing branch from 6c1abaf to 26baece Compare June 18, 2025 17:37

elasticsearchmachine and others added 3 commits June 18, 2025 17:47

[CI] Auto commit changes from spotless

a846c50

Split testOnlyPathString at File.pathSeparator

e9093d3

Use doFirst to delay setting testOnlyPath env var

b2bbf94

prdoyle force-pushed the bootstrap-entitlements-for-testing branch from c8b1f17 to b2bbf94 Compare June 18, 2025 19:19

prdoyle and others added 5 commits June 18, 2025 15:54

Trivially allow jimfs (??)

3c760da

Merge branch 'main' into bootstrap-entitlements-for-testing

65a7678

Don't enforce entitlements on internalClusterTest for now

5ef8233

Merge branch 'main' into bootstrap-entitlements-for-testing

0b4273a

Replace forbidden APIs

4c9acbb

prdoyle force-pushed the bootstrap-entitlements-for-testing branch from a81059a to 4c9acbb Compare June 20, 2025 12:36

prdoyle and others added 4 commits June 20, 2025 10:21

Match testOnlyClasspath using URI instead of String.

a2fcb3a

We already get the "needle" in the form of a URI, so this skips a step, and has the benefit of also working on Windows.

[CI] Auto commit changes from spotless

4b36d55

Merge branch 'main' into bootstrap-entitlements-for-testing

48fe7cc

More forbidden APIs

a2d8eb8

prdoyle enabled auto-merge (squash) June 20, 2025 16:19

prdoyle added 2 commits June 20, 2025 13:00

Merge branch 'main' into bootstrap-entitlements-for-testing

eea9cca

Merge branch 'main' into bootstrap-entitlements-for-testing

846c8b7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Bootstrap entitlements for testing #129268

Bootstrap entitlements for testing #129268

Uh oh!

prdoyle commented Jun 11, 2025 •

edited

Loading

Uh oh!

elasticsearchmachine commented Jun 17, 2025

Uh oh!

ldematte left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ldematte Jun 18, 2025

Uh oh!

ldematte Jun 18, 2025

Uh oh!

prdoyle Jun 18, 2025

Uh oh!

prdoyle Jun 18, 2025

Uh oh!

ldematte Jun 18, 2025

Uh oh!

prdoyle Jun 18, 2025

Uh oh!

prdoyle Jun 18, 2025

Uh oh!

Uh oh!

ldematte Jun 18, 2025

Uh oh!

prdoyle Jun 18, 2025

Uh oh!

ldematte Jun 18, 2025

Uh oh!

Uh oh!

Bootstrap entitlements for testing #129268

Are you sure you want to change the base?

Bootstrap entitlements for testing #129268

Uh oh!

Conversation

prdoyle commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Jun 17, 2025

Uh oh!

ldematte left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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!

prdoyle commented Jun 11, 2025 •

edited

Loading