Skip to content

Conversation

@ldematte
Copy link
Contributor

@ldematte ldematte commented Feb 11, 2025

This PR adds entitlement instrumentation for FileSystemProvider; we instrument the concrete class obtained at runtime from FileSystem.default().

Relates to ES-10793

@ldematte ldematte added >non-issue :Core/Infra/Core Core issues without another label auto-backport Automatically create backport pull requests when merged v8.18.1 v8.19.0 v9.0.1 test-entitlements labels Feb 11, 2025
@ldematte ldematte marked this pull request as ready for review February 11, 2025 10:20
@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 Feb 11, 2025
@ldematte ldematte requested a review from a team February 11, 2025 15:24
FileAttribute<?>... attrs
);

void checkNewDirectoryStream(Class<?> callerClass, FileSystemProvider that, Path dir, DirectoryStream.Filter<? super Path> filter);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, this is a large number of special-case check methods. If we had check inheritance, would that make these unnecessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, we will have these anyway! We would just not need the 25 lines or so in EntitlementInitialization that wire them up to the correct class, so not such a big difference.

@ldematte ldematte requested a review from a team February 12, 2025 10:22
Copy link
Contributor

@mosche mosche left a comment

Choose a reason for hiding this comment

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

lgtm, @prdoyle any more comments from your side?


private static ElasticsearchEntitlementChecker manager;

interface InstrumentationInfoFunction {
Copy link
Contributor

Choose a reason for hiding this comment

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

optional nit: InstrumentationInfoFactory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I always appreciate a good name suggestion :)

@ldematte ldematte merged commit bd242cc into elastic:main Feb 12, 2025
22 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.18 Commit could not be cherrypicked due to conflicts
8.x Commit could not be cherrypicked due to conflicts
9.0

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 122232

ldematte added a commit to ldematte/elasticsearch that referenced this pull request Feb 12, 2025
ldematte added a commit to ldematte/elasticsearch that referenced this pull request Feb 13, 2025
@ldematte ldematte deleted the entitlements/file-system-provider branch February 13, 2025 08:21
elasticsearchmachine pushed a commit that referenced this pull request Feb 13, 2025
…122471)

* [Entitlements] Instrumentation for FileSystemProvider (#122232)

* Move some check function and tests to version specific checker classes

* Refactor/fix: lookupImplementationMethod looks up the class hierarchy too

* Spotless
ldematte added a commit to ldematte/elasticsearch that referenced this pull request Feb 13, 2025
…22232) (elastic#122471)

* [Entitlements] Instrumentation for FileSystemProvider (elastic#122232)

* Move some check function and tests to version specific checker classes

* Refactor/fix: lookupImplementationMethod looks up the class hierarchy too

* Spotless
elasticsearchmachine pushed a commit that referenced this pull request Feb 13, 2025
…122471) (#122492)

* [Entitlements] Instrumentation for FileSystemProvider (#122232)

* Move some check function and tests to version specific checker classes

* Refactor/fix: lookupImplementationMethod looks up the class hierarchy too

* Spotless
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/Core Core issues without another label >non-issue Team:Core/Infra Meta label for core/infra team v8.18.1 v8.19.0 v9.0.1 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants