Skip to content

Commit 16a8367

Browse files
rjernstSamiul-TheSoccerFan
authored andcommitted
Fully initialize policy checker before instrumenting (elastic#128703)
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.
1 parent 53614cf commit 16a8367

File tree

3 files changed

+13
-11
lines changed

3 files changed

+13
-11
lines changed

libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,9 @@ public static EntitlementChecker checker() {
6868
* @param inst the JVM instrumentation class instance
6969
*/
7070
public static void initialize(Instrumentation inst) throws Exception {
71-
checker = initChecker(inst, createPolicyManager());
71+
// the checker _MUST_ be set before _any_ instrumentation is done
72+
checker = initChecker(createPolicyManager());
73+
initInstrumentation(inst);
7274
}
7375

7476
/**
@@ -146,7 +148,7 @@ private static void ensureClassesSensitiveToVerificationAreInitialized() {
146148
}
147149
}
148150

149-
static ElasticsearchEntitlementChecker initChecker(Instrumentation inst, PolicyManager policyManager) throws Exception {
151+
static ElasticsearchEntitlementChecker initChecker(PolicyManager policyManager) {
150152
final PolicyChecker policyChecker = createPolicyChecker(policyManager);
151153
final Class<?> clazz = EntitlementCheckerUtils.getVersionSpecificCheckerClass(
152154
ElasticsearchEntitlementChecker.class,
@@ -167,17 +169,20 @@ static ElasticsearchEntitlementChecker initChecker(Instrumentation inst, PolicyM
167169
throw new AssertionError(e);
168170
}
169171

172+
return checker;
173+
}
174+
175+
static void initInstrumentation(Instrumentation instrumentation) throws Exception {
170176
var verifyBytecode = Booleans.parseBoolean(System.getProperty("es.entitlements.verify_bytecode", "false"));
171177
if (verifyBytecode) {
172178
ensureClassesSensitiveToVerificationAreInitialized();
173179
}
174180

175181
DynamicInstrumentation.initialize(
176-
inst,
182+
instrumentation,
177183
EntitlementCheckerUtils.getVersionSpecificCheckerClass(EntitlementChecker.class, Runtime.version().feature()),
178184
verifyBytecode
179185
);
180186

181-
return checker;
182187
}
183188
}

muted-tests.yml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -492,18 +492,12 @@ tests:
492492
- class: org.elasticsearch.packaging.test.DockerTests
493493
method: test085EnvironmentVariablesAreRespectedUnderDockerExec
494494
issue: https://github.com/elastic/elasticsearch/issues/128115
495-
- class: org.elasticsearch.test.apmintegration.TracesApmIT
496-
method: testApmIntegration
497-
issue: https://github.com/elastic/elasticsearch/issues/128651
498495
- class: org.elasticsearch.xpack.esql.expression.function.scalar.string.WildcardLikeTests
499496
method: testEvaluateInManyThreads {TestCase=100 random code points matches self case insensitive with keyword}
500497
issue: https://github.com/elastic/elasticsearch/issues/128676
501498
- class: org.elasticsearch.xpack.esql.expression.function.scalar.string.WildcardLikeTests
502499
method: testEvaluateInManyThreads {TestCase=100 random code points matches self case insensitive with text}
503500
issue: https://github.com/elastic/elasticsearch/issues/128677
504-
- class: org.elasticsearch.test.apmintegration.MetricsApmIT
505-
method: testApmIntegration
506-
issue: https://github.com/elastic/elasticsearch/issues/128678
507501

508502
# Examples:
509503
#

test/framework/src/main/java/org/elasticsearch/entitlement/initialization/TestEntitlementInitialization.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
import java.util.List;
3232
import java.util.Map;
3333

34+
import static org.elasticsearch.entitlement.initialization.EntitlementInitialization.initInstrumentation;
35+
3436
/**
3537
* Test-specific version of {@code EntitlementInitialization}
3638
*/
@@ -45,7 +47,8 @@ public static EntitlementChecker checker() {
4547
}
4648

4749
public static void initialize(Instrumentation inst) throws Exception {
48-
checker = EntitlementInitialization.initChecker(inst, createPolicyManager(initializeArgs.pathLookup()));
50+
checker = EntitlementInitialization.initChecker(createPolicyManager(initializeArgs.pathLookup()));
51+
initInstrumentation(inst);
4952
}
5053

5154
public record InitializeArgs(PathLookup pathLookup) {}

0 commit comments

Comments
 (0)