From 08e4a358399d5d931666fed8471ac00bcdec8a61 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 13 Apr 2022 11:13:37 -0700 Subject: [PATCH] Add request parameter 'cluster_manager_timeout' and deprecate 'master_timeout' - in Ingest APIs and Script APIs (#2682) - Deprecate the request parameter `master_timeout` that used in Ingest APIs and Script APIs which have got the parameter. - Add alternative new request parameter `cluster_manager_timeout`. - Add unit tests. Signed-off-by: Tianli Feng --- .../rest-api-spec/api/delete_script.json | 10 +++- .../rest-api-spec/api/get_script.json | 10 +++- .../api/ingest.delete_pipeline.json | 10 +++- .../api/ingest.get_pipeline.json | 10 +++- .../api/ingest.put_pipeline.json | 10 +++- .../rest-api-spec/api/put_script.json | 10 +++- .../cluster/RestDeleteStoredScriptAction.java | 8 ++- .../cluster/RestGetStoredScriptAction.java | 6 +- .../cluster/RestPutStoredScriptAction.java | 6 +- .../ingest/RestDeletePipelineAction.java | 6 +- .../action/ingest/RestGetPipelineAction.java | 6 +- .../action/ingest/RestPutPipelineAction.java | 6 +- .../RenamedTimeoutRequestParameterTests.java | 59 +++++++++++++++++++ 13 files changed, 145 insertions(+), 12 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/delete_script.json b/rest-api-spec/src/main/resources/rest-api-spec/api/delete_script.json index b38b97ae57c2e..acaa389738606 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/delete_script.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/delete_script.json @@ -28,7 +28,15 @@ }, "master_timeout":{ "type":"time", - "description":"Specify timeout for connection to master" + "description":"Specify timeout for connection to master", + "deprecated":{ + "version":"2.0.0", + "description":"To support inclusive language, use 'cluster_manager_timeout' instead." + } + }, + "cluster_manager_timeout":{ + "type":"time", + "description":"Specify timeout for connection to cluster-manager node" } } } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/get_script.json b/rest-api-spec/src/main/resources/rest-api-spec/api/get_script.json index 14307bea2ef0b..9cdac886b1b27 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/get_script.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/get_script.json @@ -24,7 +24,15 @@ "params":{ "master_timeout":{ "type":"time", - "description":"Specify timeout for connection to master" + "description":"Specify timeout for connection to master", + "deprecated":{ + "version":"2.0.0", + "description":"To support inclusive language, use 'cluster_manager_timeout' instead." + } + }, + "cluster_manager_timeout":{ + "type":"time", + "description":"Specify timeout for connection to cluster-manager node" } } } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/ingest.delete_pipeline.json b/rest-api-spec/src/main/resources/rest-api-spec/api/ingest.delete_pipeline.json index 29b4219038cd2..3e40136f556fa 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/ingest.delete_pipeline.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/ingest.delete_pipeline.json @@ -24,7 +24,15 @@ "params":{ "master_timeout":{ "type":"time", - "description":"Explicit operation timeout for connection to master node" + "description":"Explicit operation timeout for connection to master node", + "deprecated":{ + "version":"2.0.0", + "description":"To support inclusive language, use 'cluster_manager_timeout' instead." + } + }, + "cluster_manager_timeout":{ + "type":"time", + "description":"Explicit operation timeout for connection to cluster-manager node" }, "timeout":{ "type":"time", diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/ingest.get_pipeline.json b/rest-api-spec/src/main/resources/rest-api-spec/api/ingest.get_pipeline.json index 65fc4f91b2b42..cde980e67c8c9 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/ingest.get_pipeline.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/ingest.get_pipeline.json @@ -30,7 +30,15 @@ "params":{ "master_timeout":{ "type":"time", - "description":"Explicit operation timeout for connection to master node" + "description":"Explicit operation timeout for connection to master node", + "deprecated":{ + "version":"2.0.0", + "description":"To support inclusive language, use 'cluster_manager_timeout' instead." + } + }, + "cluster_manager_timeout":{ + "type":"time", + "description":"Explicit operation timeout for connection to cluster-manager node" } } } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/ingest.put_pipeline.json b/rest-api-spec/src/main/resources/rest-api-spec/api/ingest.put_pipeline.json index 4d2105866791c..5475905e7b99f 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/ingest.put_pipeline.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/ingest.put_pipeline.json @@ -24,7 +24,15 @@ "params":{ "master_timeout":{ "type":"time", - "description":"Explicit operation timeout for connection to master node" + "description":"Explicit operation timeout for connection to master node", + "deprecated":{ + "version":"2.0.0", + "description":"To support inclusive language, use 'cluster_manager_timeout' instead." + } + }, + "cluster_manager_timeout":{ + "type":"time", + "description":"Explicit operation timeout for connection to cluster-manager node" }, "timeout":{ "type":"time", diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/put_script.json b/rest-api-spec/src/main/resources/rest-api-spec/api/put_script.json index 750f7fdf4eb62..c8413d1476402 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/put_script.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/put_script.json @@ -46,7 +46,15 @@ }, "master_timeout":{ "type":"time", - "description":"Specify timeout for connection to master" + "description":"Specify timeout for connection to master", + "deprecated":{ + "version":"2.0.0", + "description":"To support inclusive language, use 'cluster_manager_timeout' instead." + } + }, + "cluster_manager_timeout":{ + "type":"time", + "description":"Specify timeout for connection to cluster-manager node" }, "context":{ "type":"string", diff --git a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestDeleteStoredScriptAction.java b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestDeleteStoredScriptAction.java index 8703899d5ed14..b303f769d216b 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestDeleteStoredScriptAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestDeleteStoredScriptAction.java @@ -33,6 +33,7 @@ import org.opensearch.action.admin.cluster.storedscripts.DeleteStoredScriptRequest; import org.opensearch.client.node.NodeClient; +import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.RestRequest; import org.opensearch.rest.action.RestToXContentListener; @@ -45,6 +46,8 @@ public class RestDeleteStoredScriptAction extends BaseRestHandler { + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestDeleteStoredScriptAction.class); + @Override public List routes() { return singletonList(new Route(DELETE, "/_scripts/{id}")); @@ -60,7 +63,10 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client String id = request.param("id"); DeleteStoredScriptRequest deleteStoredScriptRequest = new DeleteStoredScriptRequest(id); deleteStoredScriptRequest.timeout(request.paramAsTime("timeout", deleteStoredScriptRequest.timeout())); - deleteStoredScriptRequest.masterNodeTimeout(request.paramAsTime("master_timeout", deleteStoredScriptRequest.masterNodeTimeout())); + deleteStoredScriptRequest.masterNodeTimeout( + request.paramAsTime("cluster_manager_timeout", deleteStoredScriptRequest.masterNodeTimeout()) + ); + parseDeprecatedMasterTimeoutParameter(deleteStoredScriptRequest, request, deprecationLogger, getName()); return channel -> client.admin().cluster().deleteStoredScript(deleteStoredScriptRequest, new RestToXContentListener<>(channel)); } diff --git a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestGetStoredScriptAction.java b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestGetStoredScriptAction.java index b75fb7693f865..5a904b99be469 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestGetStoredScriptAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestGetStoredScriptAction.java @@ -33,6 +33,7 @@ import org.opensearch.action.admin.cluster.storedscripts.GetStoredScriptRequest; import org.opensearch.client.node.NodeClient; +import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.RestRequest; import org.opensearch.rest.action.RestStatusToXContentListener; @@ -45,6 +46,8 @@ public class RestGetStoredScriptAction extends BaseRestHandler { + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestGetStoredScriptAction.class); + @Override public List routes() { return singletonList(new Route(GET, "/_scripts/{id}")); @@ -59,7 +62,8 @@ public String getName() { public RestChannelConsumer prepareRequest(final RestRequest request, NodeClient client) throws IOException { String id = request.param("id"); GetStoredScriptRequest getRequest = new GetStoredScriptRequest(id); - getRequest.masterNodeTimeout(request.paramAsTime("master_timeout", getRequest.masterNodeTimeout())); + getRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", getRequest.masterNodeTimeout())); + parseDeprecatedMasterTimeoutParameter(getRequest, request, deprecationLogger, getName()); return channel -> client.admin().cluster().getStoredScript(getRequest, new RestStatusToXContentListener<>(channel)); } } diff --git a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestPutStoredScriptAction.java b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestPutStoredScriptAction.java index f4fe21b8adbe0..1568a80278bb9 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestPutStoredScriptAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestPutStoredScriptAction.java @@ -34,6 +34,7 @@ import org.opensearch.action.admin.cluster.storedscripts.PutStoredScriptRequest; import org.opensearch.client.node.NodeClient; import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.common.xcontent.XContentType; import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.RestRequest; @@ -50,6 +51,8 @@ public class RestPutStoredScriptAction extends BaseRestHandler { + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestPutStoredScriptAction.class); + @Override public List routes() { return unmodifiableList( @@ -76,7 +79,8 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client StoredScriptSource source = StoredScriptSource.parse(content, xContentType); PutStoredScriptRequest putRequest = new PutStoredScriptRequest(id, context, content, request.getXContentType(), source); - putRequest.masterNodeTimeout(request.paramAsTime("master_timeout", putRequest.masterNodeTimeout())); + putRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", putRequest.masterNodeTimeout())); + parseDeprecatedMasterTimeoutParameter(putRequest, request, deprecationLogger, getName()); putRequest.timeout(request.paramAsTime("timeout", putRequest.timeout())); return channel -> client.admin().cluster().putStoredScript(putRequest, new RestToXContentListener<>(channel)); } diff --git a/server/src/main/java/org/opensearch/rest/action/ingest/RestDeletePipelineAction.java b/server/src/main/java/org/opensearch/rest/action/ingest/RestDeletePipelineAction.java index 179736b4b1816..69f9316bc3d9c 100644 --- a/server/src/main/java/org/opensearch/rest/action/ingest/RestDeletePipelineAction.java +++ b/server/src/main/java/org/opensearch/rest/action/ingest/RestDeletePipelineAction.java @@ -34,6 +34,7 @@ import org.opensearch.action.ingest.DeletePipelineRequest; import org.opensearch.client.node.NodeClient; +import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.RestRequest; import org.opensearch.rest.action.RestToXContentListener; @@ -45,6 +46,8 @@ import static org.opensearch.rest.RestRequest.Method.DELETE; public class RestDeletePipelineAction extends BaseRestHandler { + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestDeletePipelineAction.class); + @Override public List routes() { return singletonList(new Route(DELETE, "/_ingest/pipeline/{id}")); @@ -58,7 +61,8 @@ public String getName() { @Override public RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { DeletePipelineRequest request = new DeletePipelineRequest(restRequest.param("id")); - request.masterNodeTimeout(restRequest.paramAsTime("master_timeout", request.masterNodeTimeout())); + request.masterNodeTimeout(restRequest.paramAsTime("cluster_manager_timeout", request.masterNodeTimeout())); + parseDeprecatedMasterTimeoutParameter(request, restRequest, deprecationLogger, getName()); request.timeout(restRequest.paramAsTime("timeout", request.timeout())); return channel -> client.admin().cluster().deletePipeline(request, new RestToXContentListener<>(channel)); } diff --git a/server/src/main/java/org/opensearch/rest/action/ingest/RestGetPipelineAction.java b/server/src/main/java/org/opensearch/rest/action/ingest/RestGetPipelineAction.java index cf86541ca8cd9..5555bf53a5ee9 100644 --- a/server/src/main/java/org/opensearch/rest/action/ingest/RestGetPipelineAction.java +++ b/server/src/main/java/org/opensearch/rest/action/ingest/RestGetPipelineAction.java @@ -35,6 +35,7 @@ import org.opensearch.action.ingest.GetPipelineRequest; import org.opensearch.client.node.NodeClient; import org.opensearch.common.Strings; +import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.RestRequest; import org.opensearch.rest.action.RestStatusToXContentListener; @@ -48,6 +49,8 @@ public class RestGetPipelineAction extends BaseRestHandler { + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestGetPipelineAction.class); + @Override public List routes() { return unmodifiableList(asList(new Route(GET, "/_ingest/pipeline"), new Route(GET, "/_ingest/pipeline/{id}"))); @@ -61,7 +64,8 @@ public String getName() { @Override public RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { GetPipelineRequest request = new GetPipelineRequest(Strings.splitStringByCommaToArray(restRequest.param("id"))); - request.masterNodeTimeout(restRequest.paramAsTime("master_timeout", request.masterNodeTimeout())); + request.masterNodeTimeout(restRequest.paramAsTime("cluster_manager_timeout", request.masterNodeTimeout())); + parseDeprecatedMasterTimeoutParameter(request, restRequest, deprecationLogger, getName()); return channel -> client.admin().cluster().getPipeline(request, new RestStatusToXContentListener<>(channel)); } } diff --git a/server/src/main/java/org/opensearch/rest/action/ingest/RestPutPipelineAction.java b/server/src/main/java/org/opensearch/rest/action/ingest/RestPutPipelineAction.java index 09f40c962dda7..8a9abc860fbc9 100644 --- a/server/src/main/java/org/opensearch/rest/action/ingest/RestPutPipelineAction.java +++ b/server/src/main/java/org/opensearch/rest/action/ingest/RestPutPipelineAction.java @@ -36,6 +36,7 @@ import org.opensearch.client.node.NodeClient; import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.collect.Tuple; +import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.common.xcontent.XContentType; import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.RestRequest; @@ -49,6 +50,8 @@ public class RestPutPipelineAction extends BaseRestHandler { + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestPutPipelineAction.class); + @Override public List routes() { return singletonList(new Route(PUT, "/_ingest/pipeline/{id}")); @@ -63,7 +66,8 @@ public String getName() { public RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { Tuple sourceTuple = restRequest.contentOrSourceParam(); PutPipelineRequest request = new PutPipelineRequest(restRequest.param("id"), sourceTuple.v2(), sourceTuple.v1()); - request.masterNodeTimeout(restRequest.paramAsTime("master_timeout", request.masterNodeTimeout())); + request.masterNodeTimeout(restRequest.paramAsTime("cluster_manager_timeout", request.masterNodeTimeout())); + parseDeprecatedMasterTimeoutParameter(request, restRequest, deprecationLogger, getName()); request.timeout(restRequest.paramAsTime("timeout", request.timeout())); return channel -> client.admin().cluster().putPipeline(request, new RestToXContentListener<>(channel)); } diff --git a/server/src/test/java/org/opensearch/action/RenamedTimeoutRequestParameterTests.java b/server/src/test/java/org/opensearch/action/RenamedTimeoutRequestParameterTests.java index 86529d96573f8..648766681a377 100644 --- a/server/src/test/java/org/opensearch/action/RenamedTimeoutRequestParameterTests.java +++ b/server/src/test/java/org/opensearch/action/RenamedTimeoutRequestParameterTests.java @@ -63,6 +63,9 @@ import org.opensearch.rest.action.admin.cluster.RestRestoreSnapshotAction; import org.opensearch.rest.action.admin.cluster.RestSnapshotsStatusAction; import org.opensearch.rest.action.admin.cluster.RestVerifyRepositoryAction; +import org.opensearch.rest.action.admin.cluster.RestDeleteStoredScriptAction; +import org.opensearch.rest.action.admin.cluster.RestGetStoredScriptAction; +import org.opensearch.rest.action.admin.cluster.RestPutStoredScriptAction; import org.opensearch.rest.action.cat.RestAllocationAction; import org.opensearch.rest.action.cat.RestRepositoriesAction; import org.opensearch.rest.action.cat.RestThreadPoolAction; @@ -76,6 +79,9 @@ import org.opensearch.rest.action.cat.RestPendingClusterTasksAction; import org.opensearch.rest.action.cat.RestSegmentsAction; import org.opensearch.rest.action.cat.RestSnapshotAction; +import org.opensearch.rest.action.ingest.RestDeletePipelineAction; +import org.opensearch.rest.action.ingest.RestGetPipelineAction; +import org.opensearch.rest.action.ingest.RestPutPipelineAction; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.rest.FakeRestRequest; import org.opensearch.threadpool.TestThreadPool; @@ -612,6 +618,59 @@ public void testVerifyRepository() { assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE); } + public void testDeletePipeline() { + FakeRestRequest request = new FakeRestRequest(); + request.params().put("cluster_manager_timeout", "1h"); + request.params().put("master_timeout", "3s"); + request.params().put("id", "test"); + RestDeletePipelineAction action = new RestDeletePipelineAction(); + Exception e = assertThrows(OpenSearchParseException.class, () -> action.prepareRequest(request, client)); + assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE)); + assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE); + } + + public void testGetPipeline() { + RestGetPipelineAction action = new RestGetPipelineAction(); + Exception e = assertThrows(OpenSearchParseException.class, () -> action.prepareRequest(getRestRequestWithBothParams(), client)); + assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE)); + assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE); + } + + public void testPutPipeline() { + FakeRestRequest request = getFakeRestRequestWithBody(); + request.params().put("cluster_manager_timeout", "2m"); + request.params().put("master_timeout", "3s"); + request.params().put("id", "test"); + RestPutPipelineAction action = new RestPutPipelineAction(); + Exception e = assertThrows(OpenSearchParseException.class, () -> action.prepareRequest(request, client)); + assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE)); + assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE); + } + + public void testDeleteStoredScript() { + RestDeleteStoredScriptAction action = new RestDeleteStoredScriptAction(); + Exception e = assertThrows(OpenSearchParseException.class, () -> action.prepareRequest(getRestRequestWithBothParams(), client)); + assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE)); + assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE); + } + + public void testGetStoredScript() { + RestGetStoredScriptAction action = new RestGetStoredScriptAction(); + Exception e = assertThrows(OpenSearchParseException.class, () -> action.prepareRequest(getRestRequestWithBothParams(), client)); + assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE)); + assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE); + } + + public void testPutStoredScript() { + RestPutStoredScriptAction action = new RestPutStoredScriptAction(); + Exception e = assertThrows( + OpenSearchParseException.class, + () -> action.prepareRequest(getRestRequestWithBodyWithBothParams(), client) + ); + assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE)); + assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE, "empty templates should no longer be used"); + } + private MasterNodeRequest getMasterNodeRequest() { return new MasterNodeRequest() { @Override