Skip to content

Log NotEntitledExceptions using logger with <component>.<package> suffix #131031

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 4 commits into from
Jul 15, 2025
Merged

Log NotEntitledExceptions using logger with <component>.<package> suffix #131031

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2428489
Log NotEntitledExceptions using logger with <component>.<package> suf…
mosche Jul 10, 2025
e06a127
Merge branch 'main' into entitlements/component_class_logger
mosche Jul 11, 2025
3f2225a
backwards compatible logger name
mosche Jul 14, 2025
6298160
Merge branch 'main' into entitlements/component_class_logger
mosche Jul 14, 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
63 changes: 30 additions & 33 deletions ...tlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyCheckerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ private void neverEntitled(Class<?> callerClass, Supplier<String> operationDescr
requestingClass,
operationDescription.get()
),
callerClass,
requestingClass,
entitlements
);
}
Expand Down Expand Up @@ -251,7 +251,7 @@ public void checkFileRead(Class<?> callerClass, Path path, boolean followLinks)
requestingClass,
realPath == null ? path : Strings.format("%s -> %s", path, realPath)
),
callerClass,
requestingClass,
entitlements
);
}
Expand Down Expand Up @@ -283,7 +283,7 @@ public void checkFileWrite(Class<?> callerClass, Path path) {
requestingClass,
path
),
callerClass,
requestingClass,
entitlements
);
}
Expand Down Expand Up @@ -360,8 +360,8 @@ public void checkAllNetworkAccess(Class<?> callerClass) {
}

var classEntitlements = policyManager.getEntitlements(requestingClass);
checkFlagEntitlement(classEntitlements, InboundNetworkEntitlement.class, requestingClass, callerClass);
checkFlagEntitlement(classEntitlements, OutboundNetworkEntitlement.class, requestingClass, callerClass);
checkFlagEntitlement(classEntitlements, InboundNetworkEntitlement.class, requestingClass);
checkFlagEntitlement(classEntitlements, OutboundNetworkEntitlement.class, requestingClass);
}

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

ModuleEntitlements entitlements = policyManager.getEntitlements(requestingClass);
if (entitlements.getEntitlements(WriteSystemPropertiesEntitlement.class).anyMatch(e -> e.properties().contains(property))) {
entitlements.logger()
.debug(
() -> Strings.format(
"Entitled: component [%s], module [%s], class [%s], entitlement [write_system_properties], property [%s]",
entitlements.componentName(),
entitlements.moduleName(),
requestingClass,
property
)
);
PolicyManager.generalLogger.debug(
() -> Strings.format(
"Entitled: component [%s], module [%s], class [%s], entitlement [write_system_properties], property [%s]",
entitlements.componentName(),
entitlements.moduleName(),
requestingClass,
property
)
);
return;
}
notEntitled(
Expand All @@ -398,7 +397,7 @@ public void checkWriteProperty(Class<?> callerClass, String property) {
requestingClass,
property
),
callerClass,
requestingClass,
entitlements
);
}
Expand Down Expand Up @@ -439,8 +438,7 @@ Optional<StackWalker.StackFrame> findRequestingFrame(Stream<StackWalker.StackFra
private void checkFlagEntitlement(
ModuleEntitlements classEntitlements,
Class<? extends Entitlement> entitlementClass,
Class<?> requestingClass,
Class<?> callerClass
Class<?> requestingClass
) {
if (classEntitlements.hasEntitlement(entitlementClass) == false) {
notEntitled(
Expand All @@ -451,27 +449,26 @@ private void checkFlagEntitlement(
requestingClass,
PolicyParser.buildEntitlementNameFromClass(entitlementClass)
),
callerClass,
requestingClass,
classEntitlements
);
}
classEntitlements.logger()
.debug(
() -> Strings.format(
"Entitled: component [%s], module [%s], class [%s], entitlement [%s]",
classEntitlements.componentName(),
classEntitlements.moduleName(),
requestingClass,
PolicyParser.buildEntitlementNameFromClass(entitlementClass)
)
);
PolicyManager.generalLogger.debug(
() -> Strings.format(
"Entitled: component [%s], module [%s], class [%s], entitlement [%s]",
classEntitlements.componentName(),
classEntitlements.moduleName(),
requestingClass,
PolicyParser.buildEntitlementNameFromClass(entitlementClass)
)
);
}

private void notEntitled(String message, Class<?> callerClass, ModuleEntitlements entitlements) {
private void notEntitled(String message, Class<?> requestingClass, ModuleEntitlements entitlements) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fixes a potential NPE: if callerClass != null, requestingClass is callerClass

var exception = new NotEntitledException(message);
// Don't emit a log for suppressed packages, e.g. packages containing self tests
if (suppressFailureLogPackages.contains(callerClass.getPackage()) == false) {
entitlements.logger().warn("Not entitled: {}", message, exception);
if (suppressFailureLogPackages.contains(requestingClass.getPackage()) == false) {
entitlements.logger(requestingClass).warn("Not entitled: {}", message, exception);
}
throw exception;
}
Expand All @@ -482,7 +479,7 @@ public void checkEntitlementPresent(Class<?> callerClass, Class<? extends Entitl
if (policyManager.isTriviallyAllowed(requestingClass)) {
return;
}
checkFlagEntitlement(policyManager.getEntitlements(requestingClass), entitlementClass, requestingClass, callerClass);
checkFlagEntitlement(policyManager.getEntitlements(requestingClass), entitlementClass, requestingClass);
}

@Override
Expand Down
34 changes: 8 additions & 26 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 @@ -122,8 +122,7 @@ protected record ModuleEntitlements(
String componentName,
String moduleName,
Map<Class<? extends Entitlement>, List<Entitlement>> entitlementsByType,
FileAccessTree fileAccess,
Logger logger
FileAccessTree fileAccess
) {

public ModuleEntitlements {
Expand All @@ -141,6 +140,11 @@ public <E extends Entitlement> Stream<E> getEntitlements(Class<E> entitlementCla
}
return entitlements.stream().map(entitlementClass::cast);
}

Logger logger(Class<?> requestingClass) {
var loggerSuffix = "." + componentName + "." + requestingClass.getPackageName();
return LogManager.getLogger(PolicyManager.class.getName() + loggerSuffix);
}
}

private FileAccessTree getDefaultFileAccess(Collection<Path> componentPaths) {
Expand All @@ -149,13 +153,7 @@ private FileAccessTree getDefaultFileAccess(Collection<Path> componentPaths) {

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

// pkg private for testing
Expand All @@ -175,8 +173,7 @@ ModuleEntitlements policyEntitlements(
componentName,
moduleName,
entitlements.stream().collect(groupingBy(Entitlement::getClass)),
FileAccessTree.of(componentName, moduleName, filesEntitlement, pathLookup, componentPaths, exclusivePaths),
getLogger(componentName, moduleName)
FileAccessTree.of(componentName, moduleName, filesEntitlement, pathLookup, componentPaths, exclusivePaths)
);
}

Expand Down Expand Up @@ -286,21 +283,6 @@ private static void validateEntitlementsPerModule(
}
}

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

/**
* We want to use the same {@link Logger} object for a given name, because we want {@link ModuleEntitlements}
* {@code equals} and {@code hashCode} to work.
* <p>
* This would not be required if LogManager
* <a href="https://github.com/elastic/elasticsearch/issues/87511">memoized the loggers</a>,
* but here we are.
*/
private static final ConcurrentHashMap<String, Logger> MODULE_LOGGERS = new ConcurrentHashMap<>();

protected ModuleEntitlements getEntitlements(Class<?> requestingClass) {
return moduleEntitlementsMap.computeIfAbsent(requestingClass.getModule(), m -> computeEntitlements(requestingClass));
}
Expand Down