Skip to content

Commit 69249e3

Browse files
ldematteldematte
authored
Fix how we suppress logs for self-tests (#123361) (#123387)
1 parent 3c9a0c8 commit 69249e3

File tree

5 files changed

+74
-48
lines changed

5 files changed

+74
-48
lines changed

‎libs/entitlement/src/main/java/org/elasticsearch/entitlement/bootstrap/EntitlementBootstrap.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.nio.file.Files;
2525
import java.nio.file.Path;
2626
import java.util.Map;
27+
import java.util.Set;
2728
import java.util.function.Function;
2829
import java.util.stream.Stream;
2930

@@ -42,7 +43,8 @@ public record BootstrapArgs(
4243
Path libDir,
4344
Path logsDir,
4445
Path tempDir,
45-
Path pidFile
46+
Path pidFile,
47+
Set<Class<?>> suppressFailureLogClasses
4648
) {
4749
public BootstrapArgs {
4850
requireNonNull(pluginPolicies);
@@ -58,6 +60,7 @@ public record BootstrapArgs(
5860
requireNonNull(libDir);
5961
requireNonNull(logsDir);
6062
requireNonNull(tempDir);
63+
requireNonNull(suppressFailureLogClasses);
6164
}
6265
}
6366

@@ -82,6 +85,7 @@ public static BootstrapArgs bootstrapArgs() {
8285
* @param tempDir the temp directory for Elasticsearch
8386
* @param logsDir the log directory for Elasticsearch
8487
* @param pidFile path to a pid file for Elasticsearch, or {@code null} if one was not specified
88+
* @param suppressFailureLogClasses classes for which we do not need or want to log Entitlements failures
8589
*/
8690
public static void bootstrap(
8791
Map<String, Policy> pluginPolicies,
@@ -94,7 +98,8 @@ public static void bootstrap(
9498
Path libDir,
9599
Path logsDir,
96100
Path tempDir,
97-
Path pidFile
101+
Path pidFile,
102+
Set<Class<?>> suppressFailureLogClasses
98103
) {
99104
logger.debug("Loading entitlement agent");
100105
if (EntitlementBootstrap.bootstrapArgs != null) {
@@ -111,7 +116,8 @@ public static void bootstrap(
111116
libDir,
112117
logsDir,
113118
tempDir,
114-
pidFile
119+
pidFile,
120+
suppressFailureLogClasses
115121
);
116122
exportInitializationToAgent();
117123
loadAgent(findAgentJar());

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,8 @@ private static PolicyManager createPolicyManager() {
272272
resolver,
273273
AGENTS_PACKAGE_NAME,
274274
ENTITLEMENTS_MODULE,
275-
pathLookup
275+
pathLookup,
276+
bootstrapArgs.suppressFailureLogClasses()
276277
);
277278
}
278279

‎libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
import org.elasticsearch.core.Strings;
1313
import org.elasticsearch.core.SuppressForbidden;
14-
import org.elasticsearch.entitlement.bootstrap.EntitlementBootstrap;
1514
import org.elasticsearch.entitlement.instrumentation.InstrumentationService;
1615
import org.elasticsearch.entitlement.runtime.api.NotEntitledException;
1716
import org.elasticsearch.entitlement.runtime.policy.entitlements.CreateClassLoaderEntitlement;
@@ -114,6 +113,7 @@ ModuleEntitlements policyEntitlements(String componentName, List<Entitlement> en
114113
private final Function<Class<?>, String> pluginResolver;
115114
private final PathLookup pathLookup;
116115
private final FileAccessTree defaultFileAccess;
116+
private final Set<Class<?>> mutedClasses;
117117

118118
public static final String ALL_UNNAMED = "ALL-UNNAMED";
119119

@@ -150,7 +150,8 @@ public PolicyManager(
150150
Function<Class<?>, String> pluginResolver,
151151
String apmAgentPackageName,
152152
Module entitlementsModule,
153-
PathLookup pathLookup
153+
PathLookup pathLookup,
154+
Set<Class<?>> suppressFailureLogClasses
154155
) {
155156
this.serverEntitlements = buildScopeEntitlementsMap(requireNonNull(serverPolicy));
156157
this.apmAgentEntitlements = apmAgentEntitlements;
@@ -162,6 +163,7 @@ public PolicyManager(
162163
this.entitlementsModule = entitlementsModule;
163164
this.pathLookup = requireNonNull(pathLookup);
164165
this.defaultFileAccess = FileAccessTree.of(FilesEntitlement.EMPTY, pathLookup);
166+
this.mutedClasses = suppressFailureLogClasses;
165167

166168
for (var e : serverEntitlements.entrySet()) {
167169
validateEntitlementsPerModule(SERVER_COMPONENT_NAME, e.getKey(), e.getValue());
@@ -386,7 +388,7 @@ public void checkAllNetworkAccess(Class<?> callerClass) {
386388
checkFlagEntitlement(classEntitlements, OutboundNetworkEntitlement.class, requestingClass, callerClass);
387389
}
388390

389-
private static void checkFlagEntitlement(
391+
private void checkFlagEntitlement(
390392
ModuleEntitlements classEntitlements,
391393
Class<? extends Entitlement> entitlementClass,
392394
Class<?> requestingClass,
@@ -446,10 +448,10 @@ public void checkWriteProperty(Class<?> callerClass, String property) {
446448
);
447449
}
448450

449-
private static void notEntitled(String message, Class<?> callerClass) {
451+
private void notEntitled(String message, Class<?> callerClass) {
450452
var exception = new NotEntitledException(message);
451-
// don't log self tests in EntitlementBootstrap
452-
if (EntitlementBootstrap.class.equals(callerClass) == false) {
453+
// Don't emit a log for muted classes, e.g. classes containing self tests
454+
if (mutedClasses.contains(callerClass) == false) {
453455
logger.warn(message, exception);
454456
}
455457
throw exception;

‎libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java‎

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ public void testGetEntitlementsThrowsOnMissingPluginUnnamedModule() {
8787
c -> "plugin1",
8888
TEST_AGENTS_PACKAGE_NAME,
8989
NO_ENTITLEMENTS_MODULE,
90-
TEST_PATH_LOOKUP
90+
TEST_PATH_LOOKUP,
91+
Set.of()
9192
);
9293

9394
// Any class from the current module (unnamed) will do
@@ -111,7 +112,8 @@ public void testGetEntitlementsThrowsOnMissingPolicyForPlugin() {
111112
c -> "plugin1",
112113
TEST_AGENTS_PACKAGE_NAME,
113114
NO_ENTITLEMENTS_MODULE,
114-
TEST_PATH_LOOKUP
115+
TEST_PATH_LOOKUP,
116+
Set.of()
115117
);
116118

117119
// Any class from the current module (unnamed) will do
@@ -131,7 +133,8 @@ public void testGetEntitlementsFailureIsCached() {
131133
c -> "plugin1",
132134
TEST_AGENTS_PACKAGE_NAME,
133135
NO_ENTITLEMENTS_MODULE,
134-
TEST_PATH_LOOKUP
136+
TEST_PATH_LOOKUP,
137+
Set.of()
135138
);
136139

137140
// Any class from the current module (unnamed) will do
@@ -156,7 +159,8 @@ public void testGetEntitlementsReturnsEntitlementsForPluginUnnamedModule() {
156159
c -> "plugin2",
157160
TEST_AGENTS_PACKAGE_NAME,
158161
NO_ENTITLEMENTS_MODULE,
159-
TEST_PATH_LOOKUP
162+
TEST_PATH_LOOKUP,
163+
Set.of()
160164
);
161165

162166
// Any class from the current module (unnamed) will do
@@ -174,7 +178,8 @@ public void testGetEntitlementsThrowsOnMissingPolicyForServer() throws ClassNotF
174178
c -> null,
175179
TEST_AGENTS_PACKAGE_NAME,
176180
NO_ENTITLEMENTS_MODULE,
177-
TEST_PATH_LOOKUP
181+
TEST_PATH_LOOKUP,
182+
Set.of()
178183
);
179184

180185
// Tests do not run modular, so we cannot use a server class.
@@ -204,7 +209,8 @@ public void testGetEntitlementsReturnsEntitlementsForServerModule() throws Class
204209
c -> null,
205210
TEST_AGENTS_PACKAGE_NAME,
206211
NO_ENTITLEMENTS_MODULE,
207-
TEST_PATH_LOOKUP
212+
TEST_PATH_LOOKUP,
213+
Set.of()
208214
);
209215

210216
// Tests do not run modular, so we cannot use a server class.
@@ -230,7 +236,8 @@ public void testGetEntitlementsReturnsEntitlementsForPluginModule() throws IOExc
230236
c -> "mock-plugin",
231237
TEST_AGENTS_PACKAGE_NAME,
232238
NO_ENTITLEMENTS_MODULE,
233-
TEST_PATH_LOOKUP
239+
TEST_PATH_LOOKUP,
240+
Set.of()
234241
);
235242

236243
var layer = createLayerForJar(jar, "org.example.plugin");
@@ -249,7 +256,8 @@ public void testGetEntitlementsResultIsCached() {
249256
c -> "plugin2",
250257
TEST_AGENTS_PACKAGE_NAME,
251258
NO_ENTITLEMENTS_MODULE,
252-
TEST_PATH_LOOKUP
259+
TEST_PATH_LOOKUP,
260+
Set.of()
253261
);
254262

255263
// Any class from the current module (unnamed) will do
@@ -308,7 +316,8 @@ public void testAgentsEntitlements() throws IOException, ClassNotFoundException
308316
c -> c.getPackageName().startsWith(TEST_AGENTS_PACKAGE_NAME) ? null : "test",
309317
TEST_AGENTS_PACKAGE_NAME,
310318
NO_ENTITLEMENTS_MODULE,
311-
TEST_PATH_LOOKUP
319+
TEST_PATH_LOOKUP,
320+
Set.of()
312321
);
313322
ModuleEntitlements agentsEntitlements = policyManager.getEntitlements(TestAgent.class);
314323
assertThat(agentsEntitlements.hasEntitlement(CreateClassLoaderEntitlement.class), is(true));
@@ -336,7 +345,8 @@ public void testDuplicateEntitlements() {
336345
c -> "test",
337346
TEST_AGENTS_PACKAGE_NAME,
338347
NO_ENTITLEMENTS_MODULE,
339-
TEST_PATH_LOOKUP
348+
TEST_PATH_LOOKUP,
349+
Set.of()
340350
)
341351
);
342352
assertEquals(
@@ -353,7 +363,8 @@ public void testDuplicateEntitlements() {
353363
c -> "test",
354364
TEST_AGENTS_PACKAGE_NAME,
355365
NO_ENTITLEMENTS_MODULE,
356-
TEST_PATH_LOOKUP
366+
TEST_PATH_LOOKUP,
367+
Set.of()
357368
)
358369
);
359370
assertEquals(
@@ -387,7 +398,8 @@ public void testDuplicateEntitlements() {
387398
c -> "plugin1",
388399
TEST_AGENTS_PACKAGE_NAME,
389400
NO_ENTITLEMENTS_MODULE,
390-
TEST_PATH_LOOKUP
401+
TEST_PATH_LOOKUP,
402+
Set.of()
391403
)
392404
);
393405
assertEquals(
@@ -407,7 +419,8 @@ public void testPluginResolverOverridesAgents() {
407419
c -> "test", // Insist that the class is in a plugin
408420
TEST_AGENTS_PACKAGE_NAME,
409421
NO_ENTITLEMENTS_MODULE,
410-
TEST_PATH_LOOKUP
422+
TEST_PATH_LOOKUP,
423+
Set.of()
411424
);
412425
ModuleEntitlements notAgentsEntitlements = policyManager.getEntitlements(TestAgent.class);
413426
assertThat(notAgentsEntitlements.hasEntitlement(CreateClassLoaderEntitlement.class), is(false));
@@ -428,7 +441,8 @@ private static PolicyManager policyManager(String agentsPackageName, Module enti
428441
c -> "test",
429442
agentsPackageName,
430443
entitlementsModule,
431-
TEST_PATH_LOOKUP
444+
TEST_PATH_LOOKUP,
445+
Set.of()
432446
);
433447
}
434448

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

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -250,9 +250,10 @@ private static void initPhase2(Bootstrap bootstrap) throws IOException {
250250
nodeEnv.libDir(),
251251
nodeEnv.logsDir(),
252252
nodeEnv.tmpDir(),
253-
args.pidFile()
253+
args.pidFile(),
254+
Set.of(EntitlementSelfTester.class)
254255
);
255-
entitlementSelfTest();
256+
EntitlementSelfTester.entitlementSelfTest();
256257
} else {
257258
assert RuntimeVersionFeature.isSecurityManagerAvailable();
258259
// no need to explicitly enable native access for legacy code
@@ -269,31 +270,33 @@ private static void initPhase2(Bootstrap bootstrap) throws IOException {
269270
bootstrap.setPluginsLoader(pluginsLoader);
270271
}
271272

272-
// check entitlements were loaded correctly. note this must be outside the entitlements lib.
273-
private static void entitlementSelfTest() {
274-
ensureCannotStartProcess(ProcessBuilder::start);
275-
// Try again with reflection
276-
ensureCannotStartProcess(Elasticsearch::reflectiveStartProcess);
277-
}
273+
private static class EntitlementSelfTester {
274+
// check entitlements were loaded correctly. note this must be outside the entitlements lib.
275+
private static void entitlementSelfTest() {
276+
ensureCannotStartProcess(ProcessBuilder::start);
277+
// Try again with reflection
278+
ensureCannotStartProcess(EntitlementSelfTester::reflectiveStartProcess);
279+
}
278280

279-
private static void ensureCannotStartProcess(CheckedConsumer<ProcessBuilder, ?> startProcess) {
280-
try {
281-
// The command doesn't matter; it doesn't even need to exist
282-
startProcess.accept(new ProcessBuilder(""));
283-
} catch (NotEntitledException e) {
284-
return;
285-
} catch (Exception e) {
286-
throw new IllegalStateException("Failed entitlement protection self-test", e);
281+
private static void ensureCannotStartProcess(CheckedConsumer<ProcessBuilder, ?> startProcess) {
282+
try {
283+
// The command doesn't matter; it doesn't even need to exist
284+
startProcess.accept(new ProcessBuilder(""));
285+
} catch (NotEntitledException e) {
286+
return;
287+
} catch (Exception e) {
288+
throw new IllegalStateException("Failed entitlement protection self-test", e);
289+
}
290+
throw new IllegalStateException("Entitlement protection self-test was incorrectly permitted");
287291
}
288-
throw new IllegalStateException("Entitlement protection self-test was incorrectly permitted");
289-
}
290292

291-
private static void reflectiveStartProcess(ProcessBuilder pb) throws Exception {
292-
try {
293-
var start = ProcessBuilder.class.getMethod("start");
294-
start.invoke(pb);
295-
} catch (InvocationTargetException e) {
296-
throw (Exception) e.getCause();
293+
private static void reflectiveStartProcess(ProcessBuilder pb) throws Exception {
294+
try {
295+
var start = ProcessBuilder.class.getMethod("start");
296+
start.invoke(pb);
297+
} catch (InvocationTargetException e) {
298+
throw (Exception) e.getCause();
299+
}
297300
}
298301
}
299302

0 commit comments

Comments
 (0)