Remove special handling for _all in nodes info

Today when requesting _all we return all nodes regardless of what other
node qualifiers are in the request. This is contrary to how the
remainder of the API behaves which acts as additive and subtractive
based on the qualifiers and their ordering. It is also contrary to how
the wildcard * behaves. This commit removes the special handling for
_all so that it behaves identical to the wildcard *.

Relates elastic#28971

Author: jasontedor

server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodes.java

@@ -40,6 +40,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.stream.StreamSupport;
 
 /**
  * This class holds all {@link DiscoveryNode} in the cluster and provides convenience methods to
@@ -232,10 +233,6 @@ public DiscoveryNode findByAddress(TransportAddress address) {
         return null;
     }
 
-    public boolean isAllNodes(String... nodesIds) {
-        return nodesIds == null || nodesIds.length == 0 || (nodesIds.length == 1 && nodesIds[0].equals("_all"));
-    }
-
     /**
      * Returns the version of the node with the oldest version in the cluster that is not a client node
      *
@@ -304,13 +301,8 @@ public DiscoveryNode resolveNode(String node) {
      *   or a generic node attribute name in which case value will be treated as a wildcard and matched against the node attribute values.
      */
     public String[] resolveNodes(String... nodes) {
-        if (isAllNodes(nodes)) {
-            int index = 0;
-            nodes = new String[this.nodes.size()];
-            for (DiscoveryNode node : this) {
-                nodes[index++] = node.getId();
-            }
-            return nodes;
+        if (nodes == null || nodes.length == 0) {
+            return StreamSupport.stream(this.spliterator(), false).map(DiscoveryNode::getId).toArray(String[]::new);
         } else {
             ObjectHashSet<String> resolvedNodesIds = new ObjectHashSet<>(nodes.length);
             for (String nodeId : nodes) {
@@ -327,16 +319,11 @@ public String[] resolveNodes(String... nodes) {
                 } else if (nodeExists(nodeId)) {
                     resolvedNodesIds.add(nodeId);
                 } else {
-                    // not a node id, try and search by name
-                    for (DiscoveryNode node : this) {
-                        if (Regex.simpleMatch(nodeId, node.getName())) {
-                            resolvedNodesIds.add(node.getId());
-                        }
-                    }
                     for (DiscoveryNode node : this) {
-                        if (Regex.simpleMatch(nodeId, node.getHostAddress())) {
-                            resolvedNodesIds.add(node.getId());
-                        } else if (Regex.simpleMatch(nodeId, node.getHostName())) {
+                        if ("_all".equals(nodeId)
+                                || Regex.simpleMatch(nodeId, node.getName())
+                                || Regex.simpleMatch(nodeId, node.getHostAddress())
+                                || Regex.simpleMatch(nodeId, node.getHostName())) {
                             resolvedNodesIds.add(node.getId());
                         }
                     }

server/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodesTests.java

@@ -35,9 +35,11 @@
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;
 
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.nullValue;
 
@@ -70,6 +72,41 @@ public void testResolveNodeByAttribute() {
         }
     }
 
+    public void testAll() {
+        final DiscoveryNodes discoveryNodes = buildDiscoveryNodes();
+
+        final String[] allNodes =
+                StreamSupport.stream(discoveryNodes.spliterator(), false).map(DiscoveryNode::getId).toArray(String[]::new);
+        assertThat(discoveryNodes.resolveNodes(), arrayContainingInAnyOrder(allNodes));
+        assertThat(discoveryNodes.resolveNodes(new String[0]), arrayContainingInAnyOrder(allNodes));
+        assertThat(discoveryNodes.resolveNodes("_all"), arrayContainingInAnyOrder(allNodes));
+
+        final String[] nonMasterNodes =
+                StreamSupport.stream(discoveryNodes.getNodes().values().spliterator(), false)
+                        .map(n -> n.value)
+                        .filter(n -> n.isMasterNode() == false)
+                        .map(DiscoveryNode::getId)
+                        .toArray(String[]::new);
+        assertThat(discoveryNodes.resolveNodes("_all", "master:false"), arrayContainingInAnyOrder(nonMasterNodes));
+
+        assertThat(discoveryNodes.resolveNodes("master:false", "_all"), arrayContainingInAnyOrder(allNodes));
+    }
+
+    public void testCoordinatorOnlyNodes() {
+        final DiscoveryNodes discoveryNodes = buildDiscoveryNodes();
+
+        final String[] coordinatorOnlyNodes =
+                StreamSupport.stream(discoveryNodes.getNodes().values().spliterator(), false)
+                    .map(n -> n.value)
+                    .filter(n -> n.isDataNode() == false && n.isIngestNode() == false && n.isMasterNode() == false)
+                    .map(DiscoveryNode::getId)
+                    .toArray(String[]::new);
+
+        assertThat(
+                discoveryNodes.resolveNodes("_all", "data:false", "ingest:false", "master:false"),
+                arrayContainingInAnyOrder(coordinatorOnlyNodes));
+    }
+
     public void testResolveNodesIds() {
         DiscoveryNodes discoveryNodes = buildDiscoveryNodes();