Skip to content

Commit 497db4b

Browse files
authored
Fail PutProject request for existing project (#127490)
Recreating an existing project breaks cluster state consistency. This PR prevents it from happening.
1 parent a64598d commit 497db4b

File tree

3 files changed

+112
-41
lines changed

3 files changed

+112
-41
lines changed

test/external-modules/multi-project/src/javaRestTest/java/org/elasticsearch/multiproject/action/DeleteProjectActionIT.java

Lines changed: 0 additions & 40 deletions
This file was deleted.
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.multiproject.action;
11+
12+
import org.elasticsearch.client.Request;
13+
import org.elasticsearch.client.Response;
14+
import org.elasticsearch.client.ResponseException;
15+
import org.elasticsearch.test.cluster.ElasticsearchCluster;
16+
import org.elasticsearch.test.cluster.local.distribution.DistributionType;
17+
import org.elasticsearch.test.rest.ESRestTestCase;
18+
import org.elasticsearch.test.rest.ObjectPath;
19+
import org.junit.ClassRule;
20+
21+
import java.io.IOException;
22+
import java.io.UncheckedIOException;
23+
import java.util.List;
24+
import java.util.Set;
25+
import java.util.concurrent.atomic.AtomicInteger;
26+
import java.util.stream.Collectors;
27+
import java.util.stream.IntStream;
28+
29+
import static org.hamcrest.CoreMatchers.containsString;
30+
import static org.hamcrest.CoreMatchers.equalTo;
31+
import static org.hamcrest.CoreMatchers.hasItem;
32+
import static org.hamcrest.CoreMatchers.not;
33+
34+
public class ProjectCrudActionIT extends ESRestTestCase {
35+
36+
@ClassRule
37+
public static ElasticsearchCluster CLUSTER = ElasticsearchCluster.local()
38+
.distribution(DistributionType.DEFAULT)
39+
.setting("test.multi_project.enabled", "true")
40+
.setting("xpack.security.enabled", "false")
41+
.build();
42+
43+
@Override
44+
protected String getTestRestCluster() {
45+
return CLUSTER.getHttpAddresses();
46+
}
47+
48+
public void testCreateAndDeleteProject() throws IOException {
49+
final var projectId = randomUniqueProjectId();
50+
var request = new Request("PUT", "/_project/" + projectId);
51+
52+
final int numberOfRequests = between(1, 8);
53+
final var successCount = new AtomicInteger();
54+
final var errorCount = new AtomicInteger();
55+
56+
runInParallel(numberOfRequests, ignore -> {
57+
try {
58+
var response = client().performRequest(request);
59+
assertAcknowledged(response);
60+
successCount.incrementAndGet();
61+
} catch (IOException e) {
62+
if (e instanceof ResponseException responseException) {
63+
assertThat(responseException.getMessage(), containsString("project [" + projectId + "] already exists"));
64+
errorCount.incrementAndGet();
65+
return;
66+
}
67+
fail(e, "unexpected exception");
68+
}
69+
});
70+
71+
assertThat(successCount.get(), equalTo(1));
72+
assertThat(errorCount.get(), equalTo(numberOfRequests - 1));
73+
assertThat(getProjectIdsFromClusterState(), hasItem(projectId.id()));
74+
75+
final Response response = client().performRequest(new Request("DELETE", "/_project/" + projectId));
76+
assertAcknowledged(response);
77+
assertThat(getProjectIdsFromClusterState(), not(hasItem(projectId.id())));
78+
}
79+
80+
private Set<String> getProjectIdsFromClusterState() throws IOException {
81+
final Response response = client().performRequest(new Request("GET", "/_cluster/state?multi_project=true"));
82+
final ObjectPath clusterState = assertOKAndCreateObjectPath(response);
83+
84+
final Set<String> projectIdsFromMetadata = extractProjectIds(clusterState, "metadata.projects");
85+
final Set<String> projectIdsFromRoutingTable = extractProjectIds(clusterState, "routing_table.projects");
86+
87+
assertThat(projectIdsFromMetadata, equalTo(projectIdsFromRoutingTable));
88+
return projectIdsFromMetadata;
89+
}
90+
91+
@SuppressWarnings("unchecked")
92+
private Set<String> extractProjectIds(ObjectPath clusterState, String path) throws IOException {
93+
final int numberProjects = ((List<Object>) clusterState.evaluate(path)).size();
94+
return IntStream.range(0, numberProjects).mapToObj(i -> {
95+
try {
96+
return (String) clusterState.evaluate(path + "." + i + ".id");
97+
} catch (IOException e) {
98+
throw new UncheckedIOException(e);
99+
}
100+
}).collect(Collectors.toUnmodifiableSet());
101+
}
102+
}

test/external-modules/multi-project/src/main/java/org/elasticsearch/multiproject/action/PutProjectAction.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
package org.elasticsearch.multiproject.action;
1111

12+
import org.elasticsearch.ResourceAlreadyExistsException;
1213
import org.elasticsearch.action.ActionListener;
1314
import org.elasticsearch.action.ActionRequestValidationException;
1415
import org.elasticsearch.action.ActionType;
@@ -37,6 +38,8 @@
3738
import org.elasticsearch.transport.TransportService;
3839

3940
import java.io.IOException;
41+
import java.util.HashSet;
42+
import java.util.Set;
4043
import java.util.regex.Pattern;
4144

4245
public class PutProjectAction extends ActionType<AcknowledgedResponse> {
@@ -105,11 +108,17 @@ static class PutProjectExecutor implements ClusterStateTaskExecutor<PutProjectTa
105108

106109
@Override
107110
public ClusterState execute(BatchExecutionContext<PutProjectTask> batchExecutionContext) throws Exception {
108-
var stateBuilder = ClusterState.builder(batchExecutionContext.initialState());
111+
final ClusterState initialState = batchExecutionContext.initialState();
112+
final Set<ProjectId> knownProjectIds = new HashSet<>(initialState.metadata().projects().keySet());
113+
var stateBuilder = ClusterState.builder(initialState);
109114
for (TaskContext<PutProjectTask> taskContext : batchExecutionContext.taskContexts()) {
110115
try {
111116
Request request = taskContext.getTask().request();
117+
if (knownProjectIds.contains(request.projectId)) {
118+
throw new ResourceAlreadyExistsException("project [{}] already exists", request.projectId);
119+
}
112120
stateBuilder.putProjectMetadata(ProjectMetadata.builder(request.projectId));
121+
knownProjectIds.add(request.projectId);
113122
taskContext.success(() -> taskContext.getTask().listener.onResponse(AcknowledgedResponse.TRUE));
114123
} catch (Exception e) {
115124
taskContext.onFailure(e);

0 commit comments

Comments
 (0)