Skip to content

Make Watch transport actions project-aware #129612

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 5 commits into
base: main
Choose a base branch
from

Conversation

samxbr
Copy link
Contributor

@samxbr samxbr commented Jun 18, 2025

Make Watcher transport actions project-aware.

@samxbr samxbr changed the title Make TransportGetWatcherSettingsAction project-aware Make Watch transport actions project-aware Jun 18, 2025
@samxbr samxbr marked this pull request as ready for review June 18, 2025 16:53
@samxbr samxbr requested a review from nielsbauman June 18, 2025 16:54
@elasticsearchmachine elasticsearchmachine added the Team:Data Management Meta label for data/management team label Jun 18, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

Copy link
Contributor

@nielsbauman nielsbauman left a comment

Choose a reason for hiding this comment

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

LGTM with some minor comments

@@ -37,6 +38,7 @@ public class TransportGetWatcherSettingsAction extends TransportLocalClusterStat
GetWatcherSettingsAction.Response> {

private final IndexNameExpressionResolver indexNameExpressionResolver;
private final ProjectResolver projectResolver;
Copy link
Contributor

Choose a reason for hiding this comment

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

You can make this class extend TransportLocalProjectMetadataAction instead of storing the field here.

@@ -73,7 +77,7 @@ public ClusterState execute(ClusterState clusterState) {
XPackPlugin.checkReadyForXPackCustomMetadata(clusterState);

WatcherMetadata newWatcherMetadata = new WatcherMetadata(manuallyStopped);
final var project = clusterState.metadata().getProject();
final var project = clusterState.metadata().getProject(projectResolver.getProjectId());
Copy link
Contributor

Choose a reason for hiding this comment

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

Super small nit:

Suggested change
final var project = clusterState.metadata().getProject(projectResolver.getProjectId());
final var project = projectResolver.getProjectMetadata(clusterState);

Comment on lines 67 to +69
when(clusterState.getMetadata()).thenReturn(Metadata.EMPTY_METADATA);
when(clusterService.state()).thenReturn(clusterState);
when(clusterState.metadata()).thenReturn(Metadata.builder().put(ProjectMetadata.builder(projectId).build()).build());
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we avoid returning two different metadata objects to avoid confusion? Ideally, we'd align the production code to only use one of the two getter methods (metadata() being the preference), but if that results in a very large number of changes, I'm also ok with leaving that part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants