Skip to content

Commit ebd8be0

Browse files
prdoyleprdoyle
committed
Use child loggers broadly in PolicyManager
1 parent cb3c357 commit ebd8be0

File tree

2 files changed

+70
-45
lines changed

2 files changed

+70
-45
lines changed

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

Lines changed: 51 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,10 @@
5959
import static java.util.zip.ZipFile.OPEN_READ;
6060

6161
public class PolicyManager {
62-
private static final Logger logger = LogManager.getLogger(PolicyManager.class);
62+
/**
63+
* Use this if you don't have a {@link ModuleEntitlements} in hand.
64+
*/
65+
private static final Logger generalLogger = LogManager.getLogger(PolicyManager.class);
6366

6467
static final String UNKNOWN_COMPONENT_NAME = "(unknown)";
6568
static final String SERVER_COMPONENT_NAME = "(server)";
@@ -76,7 +79,8 @@ public class PolicyManager {
7679
record ModuleEntitlements(
7780
String componentName,
7881
Map<Class<? extends Entitlement>, List<Entitlement>> entitlementsByType,
79-
FileAccessTree fileAccess
82+
FileAccessTree fileAccess,
83+
Logger logger
8084
) {
8185

8286
ModuleEntitlements {
@@ -101,8 +105,13 @@ private FileAccessTree getDefaultFileAccess(String componentName, Path component
101105
}
102106

103107
// pkg private for testing
104-
ModuleEntitlements defaultEntitlements(String componentName, Path componentPath) {
105-
return new ModuleEntitlements(componentName, Map.of(), getDefaultFileAccess(componentName, componentPath));
108+
ModuleEntitlements defaultEntitlements(String componentName, Path componentPath, String moduleName) {
109+
return new ModuleEntitlements(
110+
componentName,
111+
Map.of(),
112+
getDefaultFileAccess(componentName, componentPath),
113+
getLogger(componentName, moduleName)
114+
);
106115
}
107116

108117
// pkg private for testing
@@ -116,7 +125,8 @@ ModuleEntitlements policyEntitlements(String componentName, Path componentPath,
116125
return new ModuleEntitlements(
117126
componentName,
118127
entitlements.stream().collect(groupingBy(Entitlement::getClass)),
119-
FileAccessTree.of(componentName, moduleName, filesEntitlement, pathLookup, componentPath, exclusivePaths)
128+
FileAccessTree.of(componentName, moduleName, filesEntitlement, pathLookup, componentPath, exclusivePaths),
129+
getLogger(componentName, moduleName)
120130
);
121131
}
122132

@@ -323,7 +333,7 @@ public void checkFileRead(Class<?> callerClass, File file) {
323333
private static boolean isPathOnDefaultFilesystem(Path path) {
324334
var pathFileSystemClass = path.getFileSystem().getClass();
325335
if (path.getFileSystem().getClass() != DEFAULT_FILESYSTEM_CLASS) {
326-
logger.trace(
336+
generalLogger.trace(
327337
() -> Strings.format(
328338
"File entitlement trivially allowed: path [%s] is for a different FileSystem class [%s], default is [%s]",
329339
path.toString(),
@@ -505,15 +515,16 @@ private void checkFlagEntitlement(
505515
classEntitlements.componentName()
506516
);
507517
}
508-
logger.debug(
509-
() -> Strings.format(
510-
"Entitled: component [%s], module [%s], class [%s], entitlement [%s]",
511-
classEntitlements.componentName(),
512-
requestingClass.getModule().getName(),
513-
requestingClass,
514-
PolicyParser.getEntitlementTypeName(entitlementClass)
515-
)
516-
);
518+
classEntitlements.logger()
519+
.debug(
520+
() -> Strings.format(
521+
"Entitled: component [%s], module [%s], class [%s], entitlement [%s]",
522+
classEntitlements.componentName(),
523+
requestingClass.getModule().getName(),
524+
requestingClass,
525+
PolicyParser.getEntitlementTypeName(entitlementClass)
526+
)
527+
);
517528
}
518529

519530
public void checkWriteProperty(Class<?> callerClass, String property) {
@@ -524,15 +535,16 @@ public void checkWriteProperty(Class<?> callerClass, String property) {
524535

525536
ModuleEntitlements entitlements = getEntitlements(requestingClass);
526537
if (entitlements.getEntitlements(WriteSystemPropertiesEntitlement.class).anyMatch(e -> e.properties().contains(property))) {
527-
logger.debug(
528-
() -> Strings.format(
529-
"Entitled: component [%s], module [%s], class [%s], entitlement [write_system_properties], property [%s]",
530-
entitlements.componentName(),
531-
requestingClass.getModule().getName(),
532-
requestingClass,
533-
property
534-
)
535-
);
538+
entitlements.logger()
539+
.debug(
540+
() -> Strings.format(
541+
"Entitled: component [%s], module [%s], class [%s], entitlement [write_system_properties], property [%s]",
542+
entitlements.componentName(),
543+
requestingClass.getModule().getName(),
544+
requestingClass,
545+
property
546+
)
547+
);
536548
return;
537549
}
538550
notEntitled(
@@ -553,8 +565,7 @@ private void notEntitled(String message, Class<?> callerClass, String componentN
553565
// Don't emit a log for muted classes, e.g. classes containing self tests
554566
if (mutedClasses.contains(callerClass) == false) {
555567
var moduleName = callerClass.getModule().getName();
556-
var loggerSuffix = "." + componentName + "." + ((moduleName == null) ? ALL_UNNAMED : moduleName);
557-
var notEntitledLogger = LogManager.getLogger(PolicyManager.class.getName() + loggerSuffix);
568+
var notEntitledLogger = getLogger(componentName, moduleName);
558569
String frameInfoSuffix = StackWalker.getInstance(RETAIN_CLASS_REFERENCE)
559570
.walk(this::findRequestingFrame)
560571
.map(frame -> "\n\tat " + frame)
@@ -564,6 +575,11 @@ private void notEntitled(String message, Class<?> callerClass, String componentN
564575
throw exception;
565576
}
566577

578+
private static Logger getLogger(String componentName, String moduleName) {
579+
var loggerSuffix = "." + componentName + "." + ((moduleName == null) ? ALL_UNNAMED : moduleName);
580+
return LogManager.getLogger(PolicyManager.class.getName() + loggerSuffix);
581+
}
582+
567583
public void checkManageThreadsEntitlement(Class<?> callerClass) {
568584
checkEntitlementPresent(callerClass, ManageThreadsEntitlement.class);
569585
}
@@ -596,7 +612,7 @@ private ModuleEntitlements computeEntitlements(Class<?> requestingClass) {
596612
if (pluginName != null) {
597613
var pluginEntitlements = pluginsEntitlements.get(pluginName);
598614
if (pluginEntitlements == null) {
599-
return defaultEntitlements(pluginName, sourcePaths.get(pluginName));
615+
return defaultEntitlements(pluginName, sourcePaths.get(pluginName), requestingModule.getName());
600616
} else {
601617
return getModuleScopeEntitlements(
602618
pluginEntitlements,
@@ -617,7 +633,7 @@ private ModuleEntitlements computeEntitlements(Class<?> requestingClass) {
617633
);
618634
}
619635

620-
return defaultEntitlements(UNKNOWN_COMPONENT_NAME, null);
636+
return defaultEntitlements(UNKNOWN_COMPONENT_NAME, null, requestingModule.getName());
621637
}
622638

623639
private static String getScopeName(Module requestingModule) {
@@ -638,7 +654,7 @@ static Path getComponentPathFromClass(Class<?> requestingClass) {
638654
return Paths.get(codeSource.getLocation().toURI());
639655
} catch (Exception e) {
640656
// If we get a URISyntaxException, or any other Exception due to an invalid URI, we return null to safely skip this location
641-
logger.info(
657+
generalLogger.info(
642658
"Cannot get component path for [{}]: [{}] cannot be converted to a valid Path",
643659
requestingClass.getName(),
644660
codeSource.getLocation().toString()
@@ -655,7 +671,7 @@ private ModuleEntitlements getModuleScopeEntitlements(
655671
) {
656672
var entitlements = scopeEntitlements.get(scopeName);
657673
if (entitlements == null) {
658-
return defaultEntitlements(componentName, componentPath);
674+
return defaultEntitlements(componentName, componentPath, scopeName);
659675
}
660676
return policyEntitlements(componentName, componentPath, scopeName, entitlements);
661677
}
@@ -698,18 +714,18 @@ Optional<StackFrame> findRequestingFrame(Stream<StackFrame> frames) {
698714
* @return true if permission is granted regardless of the entitlement
699715
*/
700716
private static boolean isTriviallyAllowed(Class<?> requestingClass) {
701-
if (logger.isTraceEnabled()) {
702-
logger.trace("Stack trace for upcoming trivially-allowed check", new Exception());
717+
if (generalLogger.isTraceEnabled()) {
718+
generalLogger.trace("Stack trace for upcoming trivially-allowed check", new Exception());
703719
}
704720
if (requestingClass == null) {
705-
logger.debug("Entitlement trivially allowed: no caller frames outside the entitlement library");
721+
generalLogger.debug("Entitlement trivially allowed: no caller frames outside the entitlement library");
706722
return true;
707723
}
708724
if (systemModules.contains(requestingClass.getModule())) {
709-
logger.debug("Entitlement trivially allowed from system module [{}]", requestingClass.getModule().getName());
725+
generalLogger.debug("Entitlement trivially allowed from system module [{}]", requestingClass.getModule().getName());
710726
return true;
711727
}
712-
logger.trace("Entitlement not trivially allowed");
728+
generalLogger.trace("Entitlement not trivially allowed");
713729
return false;
714730
}
715731

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

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,12 @@ public void testGetEntitlementsThrowsOnMissingPluginUnnamedModule() {
102102

103103
assertEquals(
104104
"No policy for the unnamed module",
105-
policyManager.defaultEntitlements("plugin1", plugin1SourcePath),
105+
policyManager.defaultEntitlements("plugin1", plugin1SourcePath, requestingModule.getName()),
106106
policyManager.getEntitlements(callerClass)
107107
);
108108

109109
assertEquals(
110-
Map.of(requestingModule, policyManager.defaultEntitlements("plugin1", plugin1SourcePath)),
110+
Map.of(requestingModule, policyManager.defaultEntitlements("plugin1", plugin1SourcePath, requestingModule.getName())),
111111
policyManager.moduleEntitlementsMap
112112
);
113113
}
@@ -132,12 +132,12 @@ public void testGetEntitlementsThrowsOnMissingPolicyForPlugin() {
132132

133133
assertEquals(
134134
"No policy for this plugin",
135-
policyManager.defaultEntitlements("plugin1", plugin1SourcePath),
135+
policyManager.defaultEntitlements("plugin1", plugin1SourcePath, requestingModule.getName()),
136136
policyManager.getEntitlements(callerClass)
137137
);
138138

139139
assertEquals(
140-
Map.of(requestingModule, policyManager.defaultEntitlements("plugin1", plugin1SourcePath)),
140+
Map.of(requestingModule, policyManager.defaultEntitlements("plugin1", plugin1SourcePath, requestingModule.getName())),
141141
policyManager.moduleEntitlementsMap
142142
);
143143
}
@@ -160,18 +160,24 @@ public void testGetEntitlementsFailureIsCached() {
160160
var callerClass = this.getClass();
161161
var requestingModule = callerClass.getModule();
162162

163-
assertEquals(policyManager.defaultEntitlements("plugin1", plugin1SourcePath), policyManager.getEntitlements(callerClass));
164163
assertEquals(
165-
Map.of(requestingModule, policyManager.defaultEntitlements("plugin1", plugin1SourcePath)),
164+
policyManager.defaultEntitlements("plugin1", plugin1SourcePath, requestingModule.getName()),
165+
policyManager.getEntitlements(callerClass)
166+
);
167+
assertEquals(
168+
Map.of(requestingModule, policyManager.defaultEntitlements("plugin1", plugin1SourcePath, requestingModule.getName())),
166169
policyManager.moduleEntitlementsMap
167170
);
168171

169172
// A second time
170-
assertEquals(policyManager.defaultEntitlements("plugin1", plugin1SourcePath), policyManager.getEntitlements(callerClass));
173+
assertEquals(
174+
policyManager.defaultEntitlements("plugin1", plugin1SourcePath, requestingModule.getName()),
175+
policyManager.getEntitlements(callerClass)
176+
);
171177

172178
// Nothing new in the map
173179
assertEquals(
174-
Map.of(requestingModule, policyManager.defaultEntitlements("plugin1", plugin1SourcePath)),
180+
Map.of(requestingModule, policyManager.defaultEntitlements("plugin1", plugin1SourcePath, requestingModule.getName())),
175181
policyManager.moduleEntitlementsMap
176182
);
177183
}
@@ -219,12 +225,15 @@ public void testGetEntitlementsThrowsOnMissingPolicyForServer() throws ClassNotF
219225

220226
assertEquals(
221227
"No policy for this module in server",
222-
policyManager.defaultEntitlements(SERVER_COMPONENT_NAME, mockServerSourcePath),
228+
policyManager.defaultEntitlements(SERVER_COMPONENT_NAME, mockServerSourcePath, requestingModule.getName()),
223229
policyManager.getEntitlements(mockServerClass)
224230
);
225231

226232
assertEquals(
227-
Map.of(requestingModule, policyManager.defaultEntitlements(SERVER_COMPONENT_NAME, mockServerSourcePath)),
233+
Map.of(
234+
requestingModule,
235+
policyManager.defaultEntitlements(SERVER_COMPONENT_NAME, mockServerSourcePath, requestingModule.getName())
236+
),
228237
policyManager.moduleEntitlementsMap
229238
);
230239
}

0 commit comments

Comments
 (0)