Add request parameter 'cluster_manager_timeout' as the alternative for 'master_timeout', and deprecate 'master_timeout' - in CAT Nodes API

rest-api-spec/src/main/resources/rest-api-spec/api/cat.nodes.json

@@ -51,7 +51,15 @@
       },
       "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 promote inclusive language, use 'cluster_manager_timeout' instead."
+        }
+      },
+      "cluster_manager_timeout":{
+        "type":"time",
+        "description":"Explicit operation timeout for connection to cluster-manager node"
       },
       "h":{
         "type":"list",

server/src/main/java/org/opensearch/rest/RestRequest.java

@@ -54,6 +54,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -578,6 +579,31 @@ public static XContentType parseContentType(List<String> header) {
         throw new IllegalArgumentException("empty Content-Type header");
     }
 
+    /**
+     * The method is only used to validate whether the values of the 2 request parameters "master_timeout" and "cluster_manager_timeout" is the same value or not.
+     * If the 2 values are not the same, throw an {@link OpenSearchParseException}.
+     * @param keys Keys of the request parameters.
+     * @deprecated The method will be removed along with the request parameter "master_timeout".
+     */
+    @Deprecated
+    public void validateParamValuesAreEqual(String... keys) {
+        // Filter the non-empty values of the parameters with the key, and get the distinct values.
+        Set<String> set = new HashSet<>();
+        for (String key : keys) {
+            String value = param(key);
+            if (!Strings.isNullOrEmpty(value)) {
+                set.add(value);
+            }
+        }
+        // If there are more than 1 distinct value of the request parameters, throw an exception.
+        if (set.size() > 1) {
+            throw new OpenSearchParseException(
+                "The values of the request parameters: {} are required to be equal, otherwise please only assign value to one of the request parameters.",
+                Arrays.toString(keys)
+            );
+        }
+    }
+
     public static class ContentTypeHeaderException extends RuntimeException {
 
         ContentTypeHeaderException(final IllegalArgumentException cause) {

server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java

@@ -86,6 +86,8 @@ public class RestNodesAction extends AbstractCatAction {
     private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestNodesAction.class);
     static final String LOCAL_DEPRECATED_MESSAGE = "Deprecated parameter [local] used. This parameter does not cause this API to act "
         + "locally, and should not be used. It will be unsupported in version 8.0.";
+    static final String MASTER_TIMEOUT_DEPRECATED_MESSAGE =
+        "Deprecated parameter [master_timeout] used. To promote inclusive language, please use [cluster_manager_timeout] instead. It will be unsupported in a future major version.";
 
     @Override
     public List<Route> routes() {
@@ -110,7 +112,8 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli
             deprecationLogger.deprecate("cat_nodes_local_parameter", LOCAL_DEPRECATED_MESSAGE);
         }
         clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
-        clusterStateRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterStateRequest.masterNodeTimeout()));
+        clusterStateRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterStateRequest.masterNodeTimeout()));
+        parseDeprecatedMasterTimeoutParameter(clusterStateRequest, request);
         final boolean fullId = request.paramAsBoolean("full_id", false);
         return channel -> client.admin().cluster().state(clusterStateRequest, new RestActionListener<ClusterStateResponse>(channel) {
             @Override
@@ -525,4 +528,21 @@ Table buildTable(
     private short calculatePercentage(long used, long max) {
         return max <= 0 ? 0 : (short) ((100d * used) / max);
     }
+
+    /**
+     * Parse the deprecated request parameter 'master_timeout', and add deprecated log if the parameter is used.
+     * It also validates whether the value of 'master_timeout' is the same with 'cluster_manager_timeout'.
+     * Remove the method along with MASTER_ROLE.
+     * @deprecated As of 2.0, because promoting inclusive language.
+     */
+    @Deprecated
+    private void parseDeprecatedMasterTimeoutParameter(ClusterStateRequest clusterStateRequest, RestRequest request) {
+        final String deprecatedTimeoutParam = "master_timeout";
+        final String clusterManagerTimeoutParam = "cluster_manager_timeout";
+        clusterStateRequest.masterNodeTimeout(request.paramAsTime(deprecatedTimeoutParam, clusterStateRequest.masterNodeTimeout()));
+        if (request.hasParam(deprecatedTimeoutParam)) {
+            deprecationLogger.deprecate("cat_nodes_master_timeout_parameter", MASTER_TIMEOUT_DEPRECATED_MESSAGE);
+            request.validateParamValuesAreEqual(deprecatedTimeoutParam, clusterManagerTimeoutParam);
+        }
+    }
 }

server/src/test/java/org/opensearch/rest/RestRequestTests.java

@@ -34,6 +34,7 @@
 
 import org.opensearch.OpenSearchParseException;
 import org.opensearch.common.CheckedConsumer;
+import org.opensearch.common.Strings;
 import org.opensearch.common.bytes.BytesArray;
 import org.opensearch.common.bytes.BytesReference;
 import org.opensearch.common.collect.MapBuilder;
@@ -47,6 +48,7 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -280,6 +282,37 @@ public void testRequiredContent() {
         assertEquals("unknown content type", e.getMessage());
     }
 
+    /*
+     * The test is added in 2.0 when the request parameter "cluster_manager_timeout" is introduced.
+     * Remove the test along with the removal of the non-inclusive terminology "master_timeout".
+     */
+    public void testValidateParamValuesAreEqual() {
+        FakeRestRequest request = new FakeRestRequest();
+        List<String> valueList = new ArrayList<>(Arrays.asList(null, "", "value1", "value2"));
+        String valueForKey1 = randomFrom(valueList);
+        String valueForKey2 = randomFrom(valueList);
+        request.params().put("key1", valueForKey1);
+        request.params().put("key2", valueForKey2);
+        try {
+            request.validateParamValuesAreEqual("key1", "key2");
+            assertTrue(
+                "Values of the 2 keys should be equal, or having a least 1 null value or empty String. Value of key1: "
+                    + valueForKey1
+                    + ". Value of key2: "
+                    + valueForKey2,
+                Strings.isNullOrEmpty(valueForKey1) || Strings.isNullOrEmpty(valueForKey2) || valueForKey1.equals(valueForKey2)
+            );
+        } catch (OpenSearchParseException e) {
+            assertEquals(
+                "The values of the request parameters: [key1, key2] are required to be equal, otherwise please only assign value to one of the request parameters.",
+                e.getMessage()
+            );
+            assertNotEquals(valueForKey1, valueForKey2);
+            assertFalse(Strings.isNullOrEmpty(valueForKey1));
+            assertFalse(Strings.isNullOrEmpty(valueForKey2));
+        }
+    }
+
     private static RestRequest contentRestRequest(String content, Map<String, String> params) {
         Map<String, List<String>> headers = new HashMap<>();
         headers.put("Content-Type", Collections.singletonList("application/json"));

server/src/test/java/org/opensearch/rest/action/cat/RestNodesActionTests.java

@@ -32,6 +32,7 @@
 
 package org.opensearch.rest.action.cat;
 
+import org.opensearch.OpenSearchParseException;
 import org.opensearch.Version;
 import org.opensearch.action.admin.cluster.node.info.NodesInfoResponse;
 import org.opensearch.action.admin.cluster.node.stats.NodesStatsResponse;
@@ -51,6 +52,7 @@
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.emptySet;
+import static org.hamcrest.CoreMatchers.containsString;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -89,4 +91,23 @@ public void testCatNodesWithLocalDeprecationWarning() {
 
         terminate(threadPool);
     }
+
+    /**
+     * Validate both cluster_manager_timeout and its predecessor can be parsed correctly.
+     * Remove the test along with MASTER_ROLE. It's added in version 2.0.0.
+     */
+    public void testCatNodesWithClusterManagerTimeout() {
+        TestThreadPool threadPool = new TestThreadPool(RestNodesActionTests.class.getName());
+        NodeClient client = new NodeClient(Settings.EMPTY, threadPool);
+        FakeRestRequest request = new FakeRestRequest();
+        request.params().put("cluster_manager_timeout", randomFrom("1h", "2m"));
+        request.params().put("master_timeout", "3s");
+        try {
+            action.doCatRequest(request, client);
+        } catch (OpenSearchParseException e) {
+            assertThat(e.getMessage(), containsString("cluster_manager_timeout"));
+        }
+        assertWarnings(RestNodesAction.MASTER_TIMEOUT_DEPRECATED_MESSAGE);
+        terminate(threadPool);
+    }
 }