Skip to content

Split PolicyChecker from PolicyManager #128004

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

prdoyle
Copy link
Contributor

@prdoyle prdoyle commented May 9, 2025

PolicyManager has a number of responsibilities, including at least:

  • Determining the entitlements granted to a given requesting class
  • Two dozen check methods that determine whether some required entitlement is present

This PR splits those into two objects, with the immediate consequence being that we can now swap in a PolicyManagerForTesting that modifies the behaviour.

With this change, the design looks as follows:

  • The EntitlementChecker interface has one method per sensitive JDK method. Its implementation (called ElasticsearchEntitlementChecker) determines which kind of check we should do for each method. Its methods are all one-liners that call PolicyChecker.
  • The PolicyChecker interface has one method per distinct kind of check (about 27 methods in total). Its implementation (PolicyCheckerImpl) has the job of determining the caller class, querying the PolicyManager to determine what entitlements are granted to that class, and then checking whether the required entitlements are present.
  • The PolicyManager class determines, for a given caller class, what entitlements that class has been granted, and whether it is trivially allowed.

It's PolicyManager that implements getEntitlements and isTriviallyAllowed. The idea is that we'd create a PolicyManagerForTesting that would override these to provide the required semantics when running in junit.

@prdoyle prdoyle added >refactoring auto-backport Automatically create backport pull requests when merged v8.19.0 :Core/Infra/Entitlements Entitlements infrastructure v8.18.2 v9.0.2 labels May 9, 2025
@prdoyle prdoyle force-pushed the externalized-cache branch from 5d95452 to 60de9dc Compare May 12, 2025 13:38
@prdoyle prdoyle marked this pull request as ready for review May 12, 2025 13:42
@prdoyle prdoyle requested a review from a team as a code owner May 12, 2025 13:42
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label May 12, 2025
@prdoyle prdoyle force-pushed the externalized-cache branch 2 times, most recently from 6b023dc to cba405d Compare May 12, 2025 14:15
@prdoyle prdoyle force-pushed the externalized-cache branch from caef5bc to 6d17a3a Compare May 12, 2025 15:22
@prdoyle prdoyle force-pushed the externalized-cache branch from 6d17a3a to 8530cc1 Compare May 12, 2025 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged :Core/Infra/Entitlements Entitlements infrastructure >refactoring Team:Core/Infra Meta label for core/infra team v8.18.2 v8.19.0 v9.0.2 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants