From 77587a21ecf779169c65062eae6c42ede3506d2e Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 30 May 2025 14:29:27 -0700 Subject: [PATCH 1/5] Fully initialize policy checker before instrumenting Entitlement instrumentation works by reflectively calling back into the entitlements lib to grab the checker. It must be fully in place before any classes are instrumented. This commit fixes a bug that was introduced by refactoring which caused the checker to not be set until after all classes were instrumented. In some situations this could lead the checker to being null when it is grab (and statically cached) by the entitlement bridge. --- .../initialization/EntitlementInitialization.java | 13 +++++++++---- muted-tests.yml | 3 --- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java index bbfd23ccd886a..902ce90bd9d9f 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java @@ -63,7 +63,8 @@ public static EntitlementChecker checker() { * @param inst the JVM instrumentation class instance */ public static void initialize(Instrumentation inst) throws Exception { - checker = initChecker(inst, createPolicyManager()); + checker = initChecker(createPolicyManager()); + initInstrumentation(inst); } private static PolicyCheckerImpl createPolicyChecker(PolicyManager policyManager) { @@ -115,7 +116,7 @@ private static void ensureClassesSensitiveToVerificationAreInitialized() { } } - static ElasticsearchEntitlementChecker initChecker(Instrumentation inst, PolicyManager policyManager) throws Exception { + static ElasticsearchEntitlementChecker initChecker(PolicyManager policyManager) { final PolicyChecker policyChecker = createPolicyChecker(policyManager); final Class clazz = EntitlementCheckerUtils.getVersionSpecificCheckerClass( ElasticsearchEntitlementChecker.class, @@ -136,17 +137,21 @@ static ElasticsearchEntitlementChecker initChecker(Instrumentation inst, PolicyM throw new AssertionError(e); } + + return checker; + } + + static void initInstrumentation(Instrumentation instrumentation) throws Exception { var verifyBytecode = Booleans.parseBoolean(System.getProperty("es.entitlements.verify_bytecode", "false")); if (verifyBytecode) { ensureClassesSensitiveToVerificationAreInitialized(); } DynamicInstrumentation.initialize( - inst, + instrumentation, EntitlementCheckerUtils.getVersionSpecificCheckerClass(EntitlementChecker.class, Runtime.version().feature()), verifyBytecode ); - return checker; } } diff --git a/muted-tests.yml b/muted-tests.yml index 85a8c6c5c27fe..e4306085608d5 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -501,9 +501,6 @@ tests: - class: org.elasticsearch.xpack.esql.expression.function.scalar.string.WildcardLikeTests method: testEvaluateInManyThreads {TestCase=100 random code points matches self case insensitive with text} issue: https://github.com/elastic/elasticsearch/issues/128677 -- class: org.elasticsearch.test.apmintegration.MetricsApmIT - method: testApmIntegration - issue: https://github.com/elastic/elasticsearch/issues/128678 # Examples: # From cebe21f73ad69d2d8e7ba497958fec7c61bea1ba Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 30 May 2025 14:38:26 -0700 Subject: [PATCH 2/5] add comment --- .../entitlement/initialization/EntitlementInitialization.java | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java index 902ce90bd9d9f..b1f4029ce9c2b 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java @@ -63,6 +63,7 @@ public static EntitlementChecker checker() { * @param inst the JVM instrumentation class instance */ public static void initialize(Instrumentation inst) throws Exception { + // the checker _MUST_ be set before _any_ instrumentation is done checker = initChecker(createPolicyManager()); initInstrumentation(inst); } From f482ea85056a48963bf1a15e928a67da12b8beee Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Fri, 30 May 2025 21:47:14 +0000 Subject: [PATCH 3/5] [CI] Auto commit changes from spotless --- .../entitlement/initialization/EntitlementInitialization.java | 1 - 1 file changed, 1 deletion(-) diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java index b1f4029ce9c2b..bd2cdfa49b151 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java @@ -138,7 +138,6 @@ static ElasticsearchEntitlementChecker initChecker(PolicyManager policyManager) throw new AssertionError(e); } - return checker; } From 8b8eece4a36101e8abbade575d6a45ba45be65bd Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 30 May 2025 14:58:50 -0700 Subject: [PATCH 4/5] fix test --- .../initialization/TestEntitlementInitialization.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/framework/src/main/java/org/elasticsearch/entitlement/initialization/TestEntitlementInitialization.java b/test/framework/src/main/java/org/elasticsearch/entitlement/initialization/TestEntitlementInitialization.java index 7e66d9e0343fd..4de24268c5a88 100644 --- a/test/framework/src/main/java/org/elasticsearch/entitlement/initialization/TestEntitlementInitialization.java +++ b/test/framework/src/main/java/org/elasticsearch/entitlement/initialization/TestEntitlementInitialization.java @@ -32,6 +32,8 @@ import java.util.List; import java.util.Map; +import static org.elasticsearch.entitlement.initialization.EntitlementInitialization.initInstrumentation; + /** * Test-specific version of {@code EntitlementInitialization} */ @@ -46,7 +48,8 @@ public static EntitlementChecker checker() { public static void initialize(Instrumentation inst) throws Exception { TestEntitlementBootstrap.BootstrapArgs bootstrapArgs = TestEntitlementBootstrap.bootstrapArgs(); - checker = EntitlementInitialization.initChecker(inst, createPolicyManager(bootstrapArgs.pathLookup())); + checker = EntitlementInitialization.initChecker(createPolicyManager(bootstrapArgs.pathLookup())); + initInstrumentation(inst); } private record TestPluginData(String pluginName, boolean isModular, boolean isExternalPlugin) {} From 198077a9a6de066833d761087c1ea756a71a6ea9 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 30 May 2025 15:01:11 -0700 Subject: [PATCH 5/5] unmute another test --- muted-tests.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/muted-tests.yml b/muted-tests.yml index e4306085608d5..7ac809b26b65e 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -492,9 +492,6 @@ tests: - class: org.elasticsearch.packaging.test.DockerTests method: test085EnvironmentVariablesAreRespectedUnderDockerExec issue: https://github.com/elastic/elasticsearch/issues/128115 -- class: org.elasticsearch.test.apmintegration.TracesApmIT - method: testApmIntegration - issue: https://github.com/elastic/elasticsearch/issues/128651 - class: org.elasticsearch.xpack.esql.expression.function.scalar.string.WildcardLikeTests method: testEvaluateInManyThreads {TestCase=100 random code points matches self case insensitive with keyword} issue: https://github.com/elastic/elasticsearch/issues/128676