Skip to content

Use child loggers broadly in PolicyManager #124896

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Mar 15, 2025
Merged

Use child loggers broadly in PolicyManager #124896

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ebd8be0
Use child loggers broadly in PolicyManager
prdoyle Mar 14, 2025
6a4292a
Pass ModuleEntitlements to notEntitled
prdoyle Mar 14, 2025
eec5e1e
Store logger name instead of object.
prdoyle Mar 14, 2025
536fd4f
Alright, let's cache 'em
prdoyle Mar 14, 2025
ef1f0ef
Memoize at point of creation, not point of use
prdoyle Mar 14, 2025
2a87032
Explanatory comments
prdoyle Mar 14, 2025
be7f2b2
Merge branch 'main' into fine-grained-loggers
prdoyle Mar 14, 2025
55c54dc
Merge branch 'main' into fine-grained-loggers
prdoyle Mar 14, 2025
ff1f676
Merge branch 'main' into fine-grained-loggers
prdoyle Mar 14, 2025
6f31ee5
Merge branch 'main' into fine-grained-loggers
prdoyle Mar 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 51 additions & 35 deletions ...entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@
import static java.util.zip.ZipFile.OPEN_READ;

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

static final String UNKNOWN_COMPONENT_NAME = "(unknown)";
static final String SERVER_COMPONENT_NAME = "(server)";
Expand All @@ -76,7 +79,8 @@ public class PolicyManager {
record ModuleEntitlements(
String componentName,
Map<Class<? extends Entitlement>, List<Entitlement>> entitlementsByType,
FileAccessTree fileAccess
FileAccessTree fileAccess,
Logger logger
) {

ModuleEntitlements {
Expand All @@ -101,8 +105,13 @@ private FileAccessTree getDefaultFileAccess(String componentName, Path component
}

// pkg private for testing
ModuleEntitlements defaultEntitlements(String componentName, Path componentPath) {
return new ModuleEntitlements(componentName, Map.of(), getDefaultFileAccess(componentName, componentPath));
ModuleEntitlements defaultEntitlements(String componentName, Path componentPath, String moduleName) {
return new ModuleEntitlements(
componentName,
Map.of(),
getDefaultFileAccess(componentName, componentPath),
getLogger(componentName, moduleName)
);
}

// pkg private for testing
Expand All @@ -116,7 +125,8 @@ ModuleEntitlements policyEntitlements(String componentName, Path componentPath,
return new ModuleEntitlements(
componentName,
entitlements.stream().collect(groupingBy(Entitlement::getClass)),
FileAccessTree.of(componentName, moduleName, filesEntitlement, pathLookup, componentPath, exclusivePaths)
FileAccessTree.of(componentName, moduleName, filesEntitlement, pathLookup, componentPath, exclusivePaths),
getLogger(componentName, moduleName)
);
}

Expand Down Expand Up @@ -323,7 +333,7 @@ public void checkFileRead(Class<?> callerClass, File file) {
private static boolean isPathOnDefaultFilesystem(Path path) {
var pathFileSystemClass = path.getFileSystem().getClass();
if (path.getFileSystem().getClass() != DEFAULT_FILESYSTEM_CLASS) {
logger.trace(
generalLogger.trace(
() -> Strings.format(
"File entitlement trivially allowed: path [%s] is for a different FileSystem class [%s], default is [%s]",
path.toString(),
Expand Down Expand Up @@ -505,15 +515,16 @@ private void checkFlagEntitlement(
classEntitlements.componentName()
);
}
logger.debug(
() -> Strings.format(
"Entitled: component [%s], module [%s], class [%s], entitlement [%s]",
classEntitlements.componentName(),
requestingClass.getModule().getName(),
requestingClass,
PolicyParser.getEntitlementTypeName(entitlementClass)
)
);
classEntitlements.logger()
.debug(
() -> Strings.format(
"Entitled: component [%s], module [%s], class [%s], entitlement [%s]",
classEntitlements.componentName(),
requestingClass.getModule().getName(),
requestingClass,
PolicyParser.getEntitlementTypeName(entitlementClass)
)
);
}

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

ModuleEntitlements entitlements = getEntitlements(requestingClass);
if (entitlements.getEntitlements(WriteSystemPropertiesEntitlement.class).anyMatch(e -> e.properties().contains(property))) {
logger.debug(
() -> Strings.format(
"Entitled: component [%s], module [%s], class [%s], entitlement [write_system_properties], property [%s]",
entitlements.componentName(),
requestingClass.getModule().getName(),
requestingClass,
property
)
);
entitlements.logger()
.debug(
() -> Strings.format(
"Entitled: component [%s], module [%s], class [%s], entitlement [write_system_properties], property [%s]",
entitlements.componentName(),
requestingClass.getModule().getName(),
requestingClass,
property
)
);
return;
}
notEntitled(
Expand All @@ -553,8 +565,7 @@ private void notEntitled(String message, Class<?> callerClass, String componentN
// Don't emit a log for muted classes, e.g. classes containing self tests
if (mutedClasses.contains(callerClass) == false) {
var moduleName = callerClass.getModule().getName();
var loggerSuffix = "." + componentName + "." + ((moduleName == null) ? ALL_UNNAMED : moduleName);
var notEntitledLogger = LogManager.getLogger(PolicyManager.class.getName() + loggerSuffix);
var notEntitledLogger = getLogger(componentName, moduleName);
String frameInfoSuffix = StackWalker.getInstance(RETAIN_CLASS_REFERENCE)
.walk(this::findRequestingFrame)
.map(frame -> "\n\tat " + frame)
Expand All @@ -564,6 +575,11 @@ private void notEntitled(String message, Class<?> callerClass, String componentN
throw exception;
}

private static Logger getLogger(String componentName, String moduleName) {
var loggerSuffix = "." + componentName + "." + ((moduleName == null) ? ALL_UNNAMED : moduleName);
return LogManager.getLogger(PolicyManager.class.getName() + loggerSuffix);
}

public void checkManageThreadsEntitlement(Class<?> callerClass) {
checkEntitlementPresent(callerClass, ManageThreadsEntitlement.class);
}
Expand Down Expand Up @@ -596,7 +612,7 @@ private ModuleEntitlements computeEntitlements(Class<?> requestingClass) {
if (pluginName != null) {
var pluginEntitlements = pluginsEntitlements.get(pluginName);
if (pluginEntitlements == null) {
return defaultEntitlements(pluginName, sourcePaths.get(pluginName));
return defaultEntitlements(pluginName, sourcePaths.get(pluginName), requestingModule.getName());
} else {
return getModuleScopeEntitlements(
pluginEntitlements,
Expand All @@ -617,7 +633,7 @@ private ModuleEntitlements computeEntitlements(Class<?> requestingClass) {
);
}

return defaultEntitlements(UNKNOWN_COMPONENT_NAME, null);
return defaultEntitlements(UNKNOWN_COMPONENT_NAME, null, requestingModule.getName());
}

private static String getScopeName(Module requestingModule) {
Expand All @@ -638,7 +654,7 @@ static Path getComponentPathFromClass(Class<?> requestingClass) {
return Paths.get(codeSource.getLocation().toURI());
} catch (Exception e) {
// If we get a URISyntaxException, or any other Exception due to an invalid URI, we return null to safely skip this location
logger.info(
generalLogger.info(
"Cannot get component path for [{}]: [{}] cannot be converted to a valid Path",
requestingClass.getName(),
codeSource.getLocation().toString()
Expand All @@ -655,7 +671,7 @@ private ModuleEntitlements getModuleScopeEntitlements(
) {
var entitlements = scopeEntitlements.get(scopeName);
if (entitlements == null) {
return defaultEntitlements(componentName, componentPath);
return defaultEntitlements(componentName, componentPath, scopeName);
}
return policyEntitlements(componentName, componentPath, scopeName, entitlements);
}
Expand Down Expand Up @@ -698,18 +714,18 @@ Optional<StackFrame> findRequestingFrame(Stream<StackFrame> frames) {
* @return true if permission is granted regardless of the entitlement
*/
private static boolean isTriviallyAllowed(Class<?> requestingClass) {
if (logger.isTraceEnabled()) {
logger.trace("Stack trace for upcoming trivially-allowed check", new Exception());
if (generalLogger.isTraceEnabled()) {
generalLogger.trace("Stack trace for upcoming trivially-allowed check", new Exception());
}
if (requestingClass == null) {
logger.debug("Entitlement trivially allowed: no caller frames outside the entitlement library");
generalLogger.debug("Entitlement trivially allowed: no caller frames outside the entitlement library");
return true;
}
if (systemModules.contains(requestingClass.getModule())) {
logger.debug("Entitlement trivially allowed from system module [{}]", requestingClass.getModule().getName());
generalLogger.debug("Entitlement trivially allowed from system module [{}]", requestingClass.getModule().getName());
return true;
}
logger.trace("Entitlement not trivially allowed");
generalLogger.trace("Entitlement not trivially allowed");
return false;
}

Expand Down
29 changes: 19 additions & 10 deletions ...lement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,12 @@ public void testGetEntitlementsThrowsOnMissingPluginUnnamedModule() {

assertEquals(
"No policy for the unnamed module",
policyManager.defaultEntitlements("plugin1", plugin1SourcePath),
policyManager.defaultEntitlements("plugin1", plugin1SourcePath, requestingModule.getName()),
policyManager.getEntitlements(callerClass)
);

assertEquals(
Map.of(requestingModule, policyManager.defaultEntitlements("plugin1", plugin1SourcePath)),
Map.of(requestingModule, policyManager.defaultEntitlements("plugin1", plugin1SourcePath, requestingModule.getName())),
policyManager.moduleEntitlementsMap
);
}
Expand All @@ -132,12 +132,12 @@ public void testGetEntitlementsThrowsOnMissingPolicyForPlugin() {

assertEquals(
"No policy for this plugin",
policyManager.defaultEntitlements("plugin1", plugin1SourcePath),
policyManager.defaultEntitlements("plugin1", plugin1SourcePath, requestingModule.getName()),
policyManager.getEntitlements(callerClass)
);

assertEquals(
Map.of(requestingModule, policyManager.defaultEntitlements("plugin1", plugin1SourcePath)),
Map.of(requestingModule, policyManager.defaultEntitlements("plugin1", plugin1SourcePath, requestingModule.getName())),
policyManager.moduleEntitlementsMap
);
}
Expand All @@ -160,18 +160,24 @@ public void testGetEntitlementsFailureIsCached() {
var callerClass = this.getClass();
var requestingModule = callerClass.getModule();

assertEquals(policyManager.defaultEntitlements("plugin1", plugin1SourcePath), policyManager.getEntitlements(callerClass));
assertEquals(
Map.of(requestingModule, policyManager.defaultEntitlements("plugin1", plugin1SourcePath)),
policyManager.defaultEntitlements("plugin1", plugin1SourcePath, requestingModule.getName()),
policyManager.getEntitlements(callerClass)
);
assertEquals(
Map.of(requestingModule, policyManager.defaultEntitlements("plugin1", plugin1SourcePath, requestingModule.getName())),
policyManager.moduleEntitlementsMap
);

// A second time
assertEquals(policyManager.defaultEntitlements("plugin1", plugin1SourcePath), policyManager.getEntitlements(callerClass));
assertEquals(
policyManager.defaultEntitlements("plugin1", plugin1SourcePath, requestingModule.getName()),
policyManager.getEntitlements(callerClass)
);

// Nothing new in the map
assertEquals(
Map.of(requestingModule, policyManager.defaultEntitlements("plugin1", plugin1SourcePath)),
Map.of(requestingModule, policyManager.defaultEntitlements("plugin1", plugin1SourcePath, requestingModule.getName())),
policyManager.moduleEntitlementsMap
);
}
Expand Down Expand Up @@ -219,12 +225,15 @@ public void testGetEntitlementsThrowsOnMissingPolicyForServer() throws ClassNotF

assertEquals(
"No policy for this module in server",
policyManager.defaultEntitlements(SERVER_COMPONENT_NAME, mockServerSourcePath),
policyManager.defaultEntitlements(SERVER_COMPONENT_NAME, mockServerSourcePath, requestingModule.getName()),
policyManager.getEntitlements(mockServerClass)
);

assertEquals(
Map.of(requestingModule, policyManager.defaultEntitlements(SERVER_COMPONENT_NAME, mockServerSourcePath)),
Map.of(
requestingModule,
policyManager.defaultEntitlements(SERVER_COMPONENT_NAME, mockServerSourcePath, requestingModule.getName())
),
policyManager.moduleEntitlementsMap
);
}
Expand Down