From c17bfcbfc219f1a0746baaf3b9501b2e479488a4 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 3 Jul 2025 10:25:27 +1000 Subject: [PATCH] Migrate the reserved repository action to be per-project (#130155) Resolves: ES-10479 --- .../TransportDeleteRepositoryAction.java | 12 ++++++++ .../ReservedRepositoryAction.java | 15 ++++------ .../elasticsearch/node/NodeConstruction.java | 2 +- .../ReservedRepositoryActionTests.java | 28 ++++++++++++------- ...vedSnapshotLifecycleStateServiceTests.java | 12 +++----- 5 files changed, 40 insertions(+), 29 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/TransportDeleteRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/TransportDeleteRepositoryAction.java index d52e7e91d4f4..34d03bb37a92 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/TransportDeleteRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/TransportDeleteRepositoryAction.java @@ -85,4 +85,16 @@ public class TransportDeleteRepositoryAction extends AcknowledgedTransportMaster public Set modifiedKeys(DeleteRepositoryRequest request) { return Set.of(request.name()); } + + @Override + protected void validateForReservedState(DeleteRepositoryRequest request, ClusterState state) { + super.validateForReservedState(request, state); + + validateForReservedState( + projectResolver.getProjectMetadata(state).reservedStateMetadata().values(), + reservedStateHandlerName().get(), + modifiedKeys(request), + request.toString() + ); + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/reservedstate/ReservedRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/reservedstate/ReservedRepositoryAction.java index 35ee66834716..1014c1729881 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/reservedstate/ReservedRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/reservedstate/ReservedRepositoryAction.java @@ -12,9 +12,8 @@ package org.elasticsearch.action.admin.cluster.repositories.reservedstate; import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryRequest; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.ProjectId; -import org.elasticsearch.core.FixForMultiProject; import org.elasticsearch.repositories.RepositoriesService; -import org.elasticsearch.reservedstate.ReservedClusterStateHandler; +import org.elasticsearch.reservedstate.ReservedProjectStateHandler; import org.elasticsearch.reservedstate.TransformState; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.XContentParserConfiguration; @@ -36,7 +35,7 @@ import static org.elasticsearch.common.xcontent.XContentHelper.mapToXContentPars * It is used by the ReservedClusterStateService to add/update or remove snapshot repositories. Typical usage * for this action is in the context of file based settings. */ -public class ReservedRepositoryAction implements ReservedClusterStateHandler> { +public class ReservedRepositoryAction implements ReservedProjectStateHandler> { public static final String NAME = "snapshot_repositories"; private final RepositoriesService repositoriesService; @@ -56,14 +55,12 @@ public class ReservedRepositoryAction implements ReservedClusterStateHandler prepare(Object input) { + public Collection prepare(ProjectId projectId, Object input) { List repositories = (List) input; for (var repositoryRequest : repositories) { validate(repositoryRequest); RepositoriesService.validateRepositoryName(repositoryRequest.name()); - @FixForMultiProject(description = "resolve the actual projectId, ES-10479") - final var projectId = ProjectId.DEFAULT; repositoriesService.validateRepositoryCanBeCreated(projectId, repositoryRequest); } @@ -71,13 +68,11 @@ public class ReservedRepositoryAction implements ReservedClusterStateHandler source, TransformState prevState) throws Exception { - var requests = prepare(source); + public TransformState transform(ProjectId projectId, List source, TransformState prevState) throws Exception { + var requests = prepare(projectId, source); ClusterState state = prevState.state(); - @FixForMultiProject(description = "resolve the actual projectId, ES-10479") - final var projectId = ProjectId.DEFAULT; for (var request : requests) { RepositoriesService.RegisterRepositoryTask task = new RepositoriesService.RegisterRepositoryTask( repositoriesService, diff --git a/server/src/main/java/org/elasticsearch/node/NodeConstruction.java b/server/src/main/java/org/elasticsearch/node/NodeConstruction.java index 73a2efabb9e8..dd134549ab01 100644 --- a/server/src/main/java/org/elasticsearch/node/NodeConstruction.java +++ b/server/src/main/java/org/elasticsearch/node/NodeConstruction.java @@ -1125,7 +1125,7 @@ class NodeConstruction { indicesService ); - actionModule.getReservedClusterStateService().installClusterStateHandler(new ReservedRepositoryAction(repositoriesService)); + actionModule.getReservedClusterStateService().installProjectStateHandler(new ReservedRepositoryAction(repositoriesService)); actionModule.getReservedClusterStateService().installProjectStateHandler(new ReservedPipelineAction()); var fileSettingsHealthIndicatorPublisher = new FileSettingsService.FileSettingsHealthIndicatorPublisherImpl(clusterService, client); diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/repositories/reservedstate/ReservedRepositoryActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/repositories/reservedstate/ReservedRepositoryActionTests.java index cee2c5fd8d41..bc45f6b7b3c7 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/repositories/reservedstate/ReservedRepositoryActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/repositories/reservedstate/ReservedRepositoryActionTests.java @@ -14,6 +14,7 @@ import org.elasticsearch.client.internal.node.NodeClient; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.ProjectId; +import org.elasticsearch.cluster.metadata.ProjectMetadata; import org.elasticsearch.cluster.metadata.RepositoryMetadata; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; @@ -34,7 +35,6 @@ import java.util.Set; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; @@ -44,9 +44,10 @@ import static org.mockito.Mockito.spy; */ public class ReservedRepositoryActionTests extends ESTestCase { - private TransformState processJSON(ReservedRepositoryAction action, TransformState prevState, String json) throws Exception { + private TransformState processJSON(ProjectId projectId, ReservedRepositoryAction action, TransformState prevState, String json) + throws Exception { try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, json)) { - return action.transform(action.fromXContent(parser), prevState); + return action.transform(projectId, action.fromXContent(parser), prevState); } } @@ -69,20 +70,24 @@ public class ReservedRepositoryActionTests extends ESTestCase { assertEquals( "[repo] repository type [inter_planetary] does not exist", - expectThrows(RepositoryException.class, () -> processJSON(action, prevState, badPolicyJSON)).getMessage() + expectThrows(RepositoryException.class, () -> processJSON(randomProjectIdOrDefault(), action, prevState, badPolicyJSON)) + .getMessage() ); } public void testAddRepo() throws Exception { var repositoriesService = mockRepositoriesService(); + final var projectId = randomProjectIdOrDefault(); - ClusterState state = ClusterState.builder(new ClusterName("elasticsearch")).build(); + ClusterState state = ClusterState.builder(new ClusterName("elasticsearch")) + .putProjectMetadata(ProjectMetadata.builder(projectId)) + .build(); TransformState prevState = new TransformState(state, Collections.emptySet()); ReservedRepositoryAction action = new ReservedRepositoryAction(repositoriesService); String emptyJSON = ""; - TransformState updatedState = processJSON(action, prevState, emptyJSON); + TransformState updatedState = processJSON(projectId, action, prevState, emptyJSON); assertEquals(0, updatedState.keys().size()); assertEquals(prevState.state(), updatedState.state()); @@ -103,14 +108,17 @@ public class ReservedRepositoryActionTests extends ESTestCase { }"""; prevState = updatedState; - updatedState = processJSON(action, prevState, settingsJSON); + updatedState = processJSON(projectId, action, prevState, settingsJSON); assertThat(updatedState.keys(), containsInAnyOrder("repo", "repo1")); } public void testRemoveRepo() { var repositoriesService = mockRepositoriesService(); + final var projectId = randomProjectIdOrDefault(); - ClusterState state = ClusterState.builder(new ClusterName("elasticsearch")).build(); + ClusterState state = ClusterState.builder(new ClusterName("elasticsearch")) + .putProjectMetadata(ProjectMetadata.builder(projectId)) + .build(); TransformState prevState = new TransformState(state, Set.of("repo1")); ReservedRepositoryAction action = new ReservedRepositoryAction(repositoriesService); @@ -120,7 +128,7 @@ public class ReservedRepositoryActionTests extends ESTestCase { // missing is sufficient to tell that we attempted to delete that repo assertEquals( "[repo1] missing", - expectThrows(RepositoryMissingException.class, () -> processJSON(action, prevState, emptyJSON)).getMessage() + expectThrows(RepositoryMissingException.class, () -> processJSON(projectId, action, prevState, emptyJSON)).getMessage() ); } @@ -153,7 +161,7 @@ public class ReservedRepositoryActionTests extends ESTestCase { throw new RepositoryException(request.name(), "repository type [" + request.type() + "] does not exist"); } return null; - }).when(repositoriesService).validateRepositoryCanBeCreated(eq(ProjectId.DEFAULT), any()); + }).when(repositoriesService).validateRepositoryCanBeCreated(any(ProjectId.class), any()); return repositoriesService; } diff --git a/x-pack/plugin/slm/src/test/java/org/elasticsearch/xpack/slm/action/ReservedSnapshotLifecycleStateServiceTests.java b/x-pack/plugin/slm/src/test/java/org/elasticsearch/xpack/slm/action/ReservedSnapshotLifecycleStateServiceTests.java index 044cc8d1d159..3be6e80ace20 100644 --- a/x-pack/plugin/slm/src/test/java/org/elasticsearch/xpack/slm/action/ReservedSnapshotLifecycleStateServiceTests.java +++ b/x-pack/plugin/slm/src/test/java/org/elasticsearch/xpack/slm/action/ReservedSnapshotLifecycleStateServiceTests.java @@ -290,8 +290,8 @@ public class ReservedSnapshotLifecycleStateServiceTests extends ESTestCase { ReservedClusterStateService controller = new ReservedClusterStateService( clusterService, null, - List.of(new ReservedClusterSettingsAction(clusterSettings), new ReservedRepositoryAction(repositoriesService)), - List.of() + List.of(new ReservedClusterSettingsAction(clusterSettings)), + List.of(new ReservedRepositoryAction(repositoriesService)) ); String testJSON = """ @@ -362,12 +362,8 @@ public class ReservedSnapshotLifecycleStateServiceTests extends ESTestCase { controller = new ReservedClusterStateService( clusterService, null, - List.of( - new ReservedClusterSettingsAction(clusterSettings), - new ReservedSnapshotAction(), - new ReservedRepositoryAction(repositoriesService) - ), - List.of() + List.of(new ReservedClusterSettingsAction(clusterSettings), new ReservedSnapshotAction()), + List.of(new ReservedRepositoryAction(repositoriesService)) ); try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, testJSON)) {