-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Encapsulate entitlements #128637
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.
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
Encapsulate entitlements #128637
Conversation
It's an implementation detail that doesn't need to be exposed to the rest of the system.
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
|
||
| requires static org.elasticsearch.entitlement.bridge; // At runtime, this will be in java.base | ||
|
|
||
| exports org.elasticsearch.entitlement.runtime.api; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Should this drop to the lines with the other runtime exports?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the one we proudly export. The others are not supposed to be public, but they currently are of necessity, so I pulled them down into their own section with a TODO.
|
|
||
| private static final Module ENTITLEMENTS_MODULE = PolicyManager.class.getModule(); | ||
|
|
||
| public static InitializeArgs initializeArgs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InitializationArgs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figured they are the arguments for the initialize method. Not the best name I ever invented.
| checker = EntitlementInitialization.initChecker(inst, createPolicyManager(initializeArgs.pathLookup())); | ||
| } | ||
|
|
||
| public record InitializeArgs(PathLookup pathLookup) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TestInitializationArgs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto. The problem this solves is that we can't pass arguments directly to the initialize method.
* Rename and encapsulate InitializeArgs * Move ElasticsearchEntitlementChecker out of api package. It's an implementation detail that doesn't need to be exposed to the rest of the system. * Stub TestPathLookup (not yet implemented)
💚 Backport successful
|
* Encapsulate entitlements (#128637) * Rename and encapsulate InitializeArgs * Move ElasticsearchEntitlementChecker out of api package. It's an implementation detail that doesn't need to be exposed to the rest of the system. * Stub TestPathLookup (not yet implemented) * Move entitlement checkers to policy package. Maintain parity with main branch to ease backporting.
* Rename and encapsulate InitializeArgs * Move ElasticsearchEntitlementChecker out of api package. It's an implementation detail that doesn't need to be exposed to the rest of the system. * Stub TestPathLookup (not yet implemented)
* Rename and encapsulate InitializeArgs * Move ElasticsearchEntitlementChecker out of api package. It's an implementation detail that doesn't need to be exposed to the rest of the system. * Stub TestPathLookup (not yet implemented)
* Rename and encapsulate InitializeArgs * Move ElasticsearchEntitlementChecker out of api package. It's an implementation detail that doesn't need to be exposed to the rest of the system. * Stub TestPathLookup (not yet implemented)
* Rename and encapsulate InitializeArgs * Move ElasticsearchEntitlementChecker out of api package. It's an implementation detail that doesn't need to be exposed to the rest of the system. * Stub TestPathLookup (not yet implemented)
A few refactorings to hide some more implementation details of entitlements.