[Entitlements] Fix "No SecurityManager when entitlements are enabled" #119742
Conversation
ldematte
added
the
test-entitlements
ldematte
changed the title
ldematte
added
>test
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
There was a problem hiding this comment.
We still won't be setting up the SecurityManager thanks to this else if, but assuming that's not necessary, LGTM.
| @@ -141,7 +141,7 @@ private static Stream<String> maybeWorkaroundG1Bug() { | |||
|
|
|||
| @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) | |||
| private static Stream<String> maybeAllowSecurityManager(boolean useEntitlements) { | |||
| if (useEntitlements == false && RuntimeVersionFeature.isSecurityManagerAvailable()) { | |||
| if (RuntimeVersionFeature.isSecurityManagerAvailable()) { | |||
| // Will become conditional on useEntitlements once entitlements can run without SM | |||
There was a problem hiding this comment.
Heh, a prescient comment here.
There was a problem hiding this comment.
Ehehe! I can remove/reword the comment :)
EDIT: it actually still make sense!
| @@ -36,7 +36,8 @@ | |||
| public class ExampleSecurityExtension implements SecurityExtension { | |||
|
|
|||
| static { | |||
| if (RuntimeVersionFeature.isSecurityManagerAvailable()) { | |||
| final boolean useEntitlements = Boolean.parseBoolean(System.getProperty("es.entitlements.enabled")); | |||
| if (useEntitlements == false && RuntimeVersionFeature.isSecurityManagerAvailable()) { | |||
There was a problem hiding this comment.
Why do we check for entitlements here?
There was a problem hiding this comment.
If we don't, and run with test-entitlement/ with entitlement enabled, there will be no Security Manager -- will be null and throw an exception.
Exactly, we still have no SM effectively, but we need it to be allowed for |
💔 Backport failed
You can use sqren/backport to manually backport by running |
Backport of #119742
Missing test still using SM even when disabled, plus revert change to SystemJvmOptions to keep allowing the SM. It seems that Hadoop needs that, as it does check if the SM is allowed via
Subject#getSubject. This is a temp fix, asSubject#getSubjectis effectively removed from JDK 24 (throws UnsupportedOperationException), but in the meantime allows our tests to run(in the long run we should update Hadoop after apache/hadoop#7081 has been merged)