Refactor: ScopeResolver

[prdoyle]

Currently, the responsibilities are split:

• Mapping a Class to a plugin name is done by PluginsResolver
• Mapping a Class to a module name is done by PolicyManager

Conceptually speaking, these are part of one process of determining the scope from the Class.

Practically speaking, we're going to want to override both of these together for a testing environment, where we can't rely on modules and module layers, but instead need to go spelunking in jar files.

This PR moves the scope resolution responsibility into one component called ScopeResolver.

• ScopeResolver is responsible for mapping a Class to a component and a module
• PolicyManager is responsible for looking up the appropriate entitlements for a given component and module

Not included in this PR

The PolicyManager still references the Module class. More work is required to make this unnecessary, including (but not limited to) allowing moduleEntitlementsMap to be managed externally.

A note about unit tests

The unit tests, while they cover all the functionality, don't make a ton of sense anymore. As a follow-on item, I'd like to clean up the tests to reflect the new split in responsibilities.

However, given that:

• the team is waiting for this change,
• rejiggering the tests will take some time (a day?), and
• the test coverage is already sufficient

... I think it makes sense to clean up the tests as a follow-on item.

[elasticsearchmachine]

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

[jdconrad]

LGTM!

[jdconrad]

This is unfortunate.

[prdoyle]

It could go inside ScopeResolver I guess? I think I made it a constructor arg because unit tests passed in a different value.

[rjernst]

Drive by comments, but nothing blocking necessarily.

[rjernst]

If we move ScopeResolver to bootstrap (or in an exported package) can we use that here instead of a Function?

[prdoyle]

I can change it if you want. Function has the benefit that everyone knows exactly what it means.

[rjernst]

Instead of exposing these strings, can we have constants defined in ScopeInfo for each of these?

[prdoyle]

Oh like an enum? Yeah I'd much prefer that actually. I was considering it but didn't want to hold up the PR.

I was thinking of:

enum ComponentKind { UNKNOWN, SERVER, PLUGIN, AGENT }

Is that what you were picturing?

[prdoyle]

If we didn't have to support JDK 17, we could go all the way to algebraic datatypes:

sealed interface PolicyScope {
  String component();
  String module();

  record Unknown() implements PolicyScope { ... };
  record Server(String module) implements PolicyScope { ... };
  record ApmAgent() implements PolicyScope { ... };
  record Plugin(String component, String module) {}
}

[rjernst]

what about Java 17 prohibits that? Still seems doable, even if with static methods?

[prdoyle]

Consuming it is harder than the enum because you can't use switch patterns until Java 21.

[rjernst]

Where do we consume it in a switch? Don't we just use the two strings right now? I view the helpers as making construction if the scope more robust, and allowing us to keep some eg logging strings for server internal to entitlements.

[prdoyle]

Here.

[prdoyle]

I've gone with an enum plus some factory method helpers.

[elasticsearchmachine]

💔 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	Commit could not be cherrypicked due to conflicts

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

[prdoyle]

Backport is conflicting with #127040 which has not yet been backported.

[prdoyle]

Backporting in #127174.