Skip to content

Commit 23204e3

Browse files
committed
[CCR] Fixed follow and unfollow api url path according to design.
The TODOs in the rest actions was incorrect. The problem was that these rest actions used `follow_index` as first named variable in the path under which the rest actions were registered. Other candidate rest actions that also have a named variable as first element in the path (but with a different name) get resolved as rest parameters too and passed down to the rest action that actually ends up getting executed. In the case of the follow index api, a `index` parameter got passed down to `RestFollowExistingAction`, but that param was never used. This caused the follow index api call to fail, because of unused http parameters. This change doesn't fixes that problem, but works around it by using `index` as named variable for the follow index (instead of `follow_index`). Relates to #30102
1 parent 64b9731 commit 23204e3

File tree

9 files changed

+26
-19
lines changed

9 files changed

+26
-19
lines changed

x-pack/plugin/ccr/qa/multi-cluster-with-security/src/test/java/org/elasticsearch/xpack/ccr/FollowIndexSecurityIT.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public void testFollowIndex() throws Exception {
8585
followIndex("leader_cluster:" + indexName1, indexName1);
8686
assertBusy(() -> verifyDocuments(client(), indexName1, numDocs));
8787
assertThat(countCcrNodeTasks(), equalTo(1));
88-
assertOK(client().performRequest("POST", "/_xpack/ccr/" + indexName1 + "/_unfollow"));
88+
assertOK(client().performRequest("POST", "/" + indexName1 + "/_xpack/ccr/_unfollow"));
8989
// Make sure that there are no other ccr relates operations running:
9090
assertBusy(() -> {
9191
Map<String, Object> clusterState = toMap(adminClient().performRequest("GET", "/_cluster/state"));
@@ -140,7 +140,7 @@ private static void refresh(String index) throws IOException {
140140

141141
private static void followIndex(String leaderIndex, String followIndex) throws IOException {
142142
Map<String, String> params = Collections.singletonMap("leader_index", leaderIndex);
143-
assertOK(client().performRequest("POST", "/_xpack/ccr/" + followIndex + "/_follow", params));
143+
assertOK(client().performRequest("POST", "/" + followIndex + "/_xpack/ccr/_follow", params));
144144
}
145145

146146
void verifyDocuments(RestClient client, String index, int expectedNumDocs) throws IOException {

x-pack/plugin/ccr/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ccr/FollowIndexIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ private static void refresh(String index) throws IOException {
9494

9595
private static void followIndex(String leaderIndex, String followIndex) throws IOException {
9696
Map<String, String> params = Collections.singletonMap("leader_index", leaderIndex);
97-
assertOK(client().performRequest("POST", "/_xpack/ccr/" + followIndex + "/_follow", params));
97+
assertOK(client().performRequest("POST", "/" + followIndex + "/_xpack/ccr/_follow", params));
9898
}
9999

100100
private static void verifyDocuments(String index, int expectedNumDocs) throws IOException {

x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/UnfollowIndexAction.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,11 @@ public TransportAction(Settings settings, ThreadPool threadPool, TransportServic
130130
protected void doExecute(Request request, ActionListener<Response> listener) {
131131
client.admin().cluster().state(new ClusterStateRequest(), ActionListener.wrap(r -> {
132132
IndexMetaData followIndexMetadata = r.getState().getMetaData().index(request.followIndex);
133+
if (followIndexMetadata == null) {
134+
listener.onFailure(new IllegalArgumentException("follow index [" + request.followIndex + "] does not exist"));
135+
return;
136+
}
137+
133138
final int numShards = followIndexMetadata.getNumberOfShards();
134139
final AtomicInteger counter = new AtomicInteger(numShards);
135140
final AtomicReferenceArray<Object> responses = new AtomicReferenceArray<>(followIndexMetadata.getNumberOfShards());

x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/rest/RestFollowExistingIndexAction.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,11 @@
1818
import static org.elasticsearch.xpack.ccr.action.FollowExistingIndexAction.INSTANCE;
1919
import static org.elasticsearch.xpack.ccr.action.FollowExistingIndexAction.Request;
2020

21-
// TODO: change to confirm with API design
2221
public class RestFollowExistingIndexAction extends BaseRestHandler {
2322

2423
public RestFollowExistingIndexAction(Settings settings, RestController controller) {
2524
super(settings);
26-
// TODO: figure out why: '/{follow_index}/_xpack/ccr/_follow' path clashes with create index api.
27-
controller.registerHandler(RestRequest.Method.POST, "/_xpack/ccr/{follow_index}/_follow", this);
25+
controller.registerHandler(RestRequest.Method.POST, "/{index}/_xpack/ccr/_follow", this);
2826
}
2927

3028
@Override
@@ -36,7 +34,7 @@ public String getName() {
3634
protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
3735
Request request = new Request();
3836
request.setLeaderIndex(restRequest.param("leader_index"));
39-
request.setFollowIndex(restRequest.param("follow_index"));
37+
request.setFollowIndex(restRequest.param("index"));
4038
if (restRequest.hasParam(ShardFollowTask.MAX_CHUNK_SIZE.getPreferredName())) {
4139
request.setBatchSize(Long.valueOf(restRequest.param(ShardFollowTask.MAX_CHUNK_SIZE.getPreferredName())));
4240
}

x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/rest/RestUnfollowIndexAction.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,11 @@
1717
import static org.elasticsearch.xpack.ccr.action.UnfollowIndexAction.INSTANCE;
1818
import static org.elasticsearch.xpack.ccr.action.UnfollowIndexAction.Request;
1919

20-
// TODO: change to confirm with API design
2120
public class RestUnfollowIndexAction extends BaseRestHandler {
2221

2322
public RestUnfollowIndexAction(Settings settings, RestController controller) {
2423
super(settings);
25-
// TODO: figure out why: '/{follow_index}/_xpack/ccr/_unfollow' path clashes with create index api.
26-
controller.registerHandler(RestRequest.Method.POST, "/_xpack/ccr/{follow_index}/_unfollow", this);
24+
controller.registerHandler(RestRequest.Method.POST, "/{index}/_xpack/ccr/_unfollow", this);
2725
}
2826

2927
@Override
@@ -34,7 +32,7 @@ public String getName() {
3432
@Override
3533
protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
3634
Request request = new Request();
37-
request.setFollowIndex(restRequest.param("follow_index"));
35+
request.setFollowIndex(restRequest.param("index"));
3836
return channel -> client.execute(INSTANCE, request, new RestToXContentListener<>(channel));
3937
}
4038
}

x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/ShardChangesIT.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,12 @@ public void testFollowIndexWithNestedField() throws Exception {
276276
});
277277
}
278278

279+
public void testUnfollowNonExistingIndex() {
280+
UnfollowIndexAction.Request unfollowRequest = new UnfollowIndexAction.Request();
281+
unfollowRequest.setFollowIndex("non-existing-index");
282+
expectThrows(IllegalArgumentException.class, () -> client().execute(UnfollowIndexAction.INSTANCE, unfollowRequest).actionGet());
283+
}
284+
279285
public void testFollowNonExistentIndex() throws Exception {
280286
assertAcked(client().admin().indices().prepareCreate("test-leader").get());
281287
assertAcked(client().admin().indices().prepareCreate("test-follower").get());

x-pack/plugin/src/test/resources/rest-api-spec/api/xpack.ccr.follow_existing_index.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
"documentation": "http://www.elastic.co/guide/en/elasticsearch/reference/current",
44
"methods": [ "POST" ],
55
"url": {
6-
"path": "/_xpack/ccr/{follow_index}/_follow",
7-
"paths": [ "/_xpack/ccr/{follow_index}/_follow" ],
6+
"path": "/{index}/_xpack/ccr/_follow",
7+
"paths": [ "/{index}/_xpack/ccr/_follow" ],
88
"parts": {
9-
"follow_index": {
9+
"index": {
1010
"type": "string",
1111
"required": true,
1212
"description": "The name of the index that follows to leader index."

x-pack/plugin/src/test/resources/rest-api-spec/api/xpack.ccr.unfollow_index.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
"documentation": "http://www.elastic.co/guide/en/elasticsearch/reference/current",
44
"methods": [ "POST" ],
55
"url": {
6-
"path": "/_xpack/ccr/{follow_index}/_unfollow",
7-
"paths": [ "/_xpack/ccr/{follow_index}/_unfollow" ],
6+
"path": "/{index}/_xpack/ccr/_unfollow",
7+
"paths": [ "/{index}/_xpack/ccr/_unfollow" ],
88
"parts": {
9-
"follow_index": {
9+
"index": {
1010
"type": "string",
1111
"required": true,
1212
"description": "The name of the follow index that should stop following its leader index."

x-pack/plugin/src/test/resources/rest-api-spec/test/ccr/follow_and_unfollow.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@
2525
- do:
2626
xpack.ccr.follow_existing_index:
2727
leader_index: foo
28-
follow_index: bar
28+
index: bar
2929
- is_true: acknowledged
3030

3131
- do:
3232
xpack.ccr.unfollow_index:
33-
follow_index: bar
33+
index: bar
3434
- is_true: acknowledged

0 commit comments

Comments
 (0)