Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,16 @@ public Optional<String> reservedStateHandlerName() {
public Set<String> 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()
);
}
Comment on lines +89 to +99
Copy link
Member Author

Choose a reason for hiding this comment

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

This is copied from the same check from other other reserved project-state actions such as this one. There is room for de-duplication as suggested by the comment, but I'll leave it out from this PR.

}
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@
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;
Expand All @@ -36,7 +35,7 @@
* 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<List<PutRepositoryRequest>> {
public class ReservedRepositoryAction implements ReservedProjectStateHandler<List<PutRepositoryRequest>> {
Copy link
Member

Choose a reason for hiding this comment

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

Basic question. In a non-MP stateless cluster, this is still sufficient because now the cluster back up repo falls under the "default" project?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Additionally, we have ProjectClusterStateHandlerAdapter which reads the reserved state from the main settings.json file in a non-MP setup.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is correct, no need to explicitly handle the non-MP case.

public static final String NAME = "snapshot_repositories";

private final RepositoriesService repositoriesService;
Expand All @@ -56,28 +55,24 @@ public String name() {
}

@SuppressWarnings("unchecked")
public Collection<PutRepositoryRequest> prepare(Object input) {
public Collection<PutRepositoryRequest> prepare(ProjectId projectId, Object input) {
List<PutRepositoryRequest> repositories = (List<PutRepositoryRequest>) 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);
}

return repositories;
}

@Override
public TransformState transform(List<PutRepositoryRequest> source, TransformState prevState) throws Exception {
var requests = prepare(source);
public TransformState transform(ProjectId projectId, List<PutRepositoryRequest> 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1125,7 +1125,7 @@ public Map<String, String> queryFields() {
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
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;
Expand All @@ -34,7 +35,6 @@

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;
Expand All @@ -44,9 +44,10 @@
*/
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);
}
}

Expand All @@ -69,20 +70,24 @@ public void testValidation() throws Exception {

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());

Expand All @@ -103,14 +108,17 @@ public void testAddRepo() throws Exception {
}""";

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);

Expand All @@ -120,7 +128,7 @@ public void testRemoveRepo() {
// 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()
);
}

Expand Down Expand Up @@ -153,7 +161,7 @@ public Repository create(ProjectId projectId, RepositoryMetadata metadata) {
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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,8 @@ public void testOperatorControllerFromJSONContent() throws IOException {
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 = """
Expand Down Expand Up @@ -362,12 +362,8 @@ public void testOperatorControllerFromJSONContent() throws IOException {
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)) {
Expand Down