Adjust status on bad allocation explain requests

When a user requests a cluster allocation explain in a situation where
it does not make sense (for example, there are no unassigned shards), we
should consider this a bad request instead of a server error. Yet, today
by throwing an illegal state exception, these are treated as server
errors. This commit adjusts these so that they throw illegal argument
exceptions and are treated as bad requests.

Relates #25503

3 files changed, 17 insertions, 11 deletions

diff --git a/core/src/main/java/org/elasticsearch/action/admin/cluster/allocation/TransportClusterAllocationExplainAction.java b/core/src/main/java/org/elasticsearch/action/admin/cluster/allocation/TransportClusterAllocationExplainAction.java
index 4d4796aaf3..77d4b24d8c 100644
--- a/core/src/main/java/org/elasticsearch/action/admin/cluster/allocation/TransportClusterAllocationExplainAction.java
+++ b/core/src/main/java/org/elasticsearch/action/admin/cluster/allocation/TransportClusterAllocationExplainAction.java
@@ -139,7 +139,7 @@ public class TransportClusterAllocationExplainAction
                 foundShard = ui.next();
             }
             if (foundShard == null) {
-                throw new IllegalStateException("unable to find any unassigned shards to explain [" + request + "]");
+                throw new IllegalArgumentException("unable to find any unassigned shards to explain [" + request + "]");
             }
         } else {
             String index = request.getIndex();
@@ -151,7 +151,8 @@ public class TransportClusterAllocationExplainAction
                     DiscoveryNode primaryNode = allocation.nodes().resolveNode(request.getCurrentNode());
                     // the primary is assigned to a node other than the node specified in the request
                     if (primaryNode.getId().equals(foundShard.currentNodeId()) == false) {
-                        throw new IllegalStateException("unable to find primary shard assigned to node [" + request.getCurrentNode() + "]");
+                        throw new IllegalArgumentException(
+                                "unable to find primary shard assigned to node [" + request.getCurrentNode() + "]");
                     }
                 }
             } else {
@@ -168,7 +169,7 @@ public class TransportClusterAllocationExplainAction
                         }
                     }
                     if (foundShard == null) {
-                        throw new IllegalStateException("unable to find a replica shard assigned to node [" +
+                        throw new IllegalArgumentException("unable to find a replica shard assigned to node [" +
                                                             request.getCurrentNode() + "]");
                     }
                 } else {
@@ -193,7 +194,7 @@ public class TransportClusterAllocationExplainAction
         }
 
         if (foundShard == null) {
-            throw new IllegalStateException("unable to find any shards to explain [" + request + "] in the routing table");
+            throw new IllegalArgumentException("unable to find any shards to explain [" + request + "] in the routing table");
         }
         return foundShard;
     }
diff --git a/core/src/test/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainActionTests.java b/core/src/test/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainActionTests.java
index 0f8f71a3af..f9785df649 100644
--- a/core/src/test/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainActionTests.java
+++ b/core/src/test/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainActionTests.java
@@ -108,7 +108,7 @@ public class ClusterAllocationExplainActionTests extends ESTestCase {
         final ClusterState allStartedClusterState = ClusterStateCreationUtils.state("idx", randomBoolean(),
             ShardRoutingState.STARTED, ShardRoutingState.STARTED);
         final ClusterAllocationExplainRequest anyUnassignedShardsRequest = new ClusterAllocationExplainRequest();
-        expectThrows(IllegalStateException.class, () ->
+        expectThrows(IllegalArgumentException.class, () ->
             findShardToExplain(anyUnassignedShardsRequest, routingAllocation(allStartedClusterState)));
     }
 
@@ -161,7 +161,7 @@ public class ClusterAllocationExplainActionTests extends ESTestCase {
             }
         }
         final ClusterAllocationExplainRequest failingRequest = new ClusterAllocationExplainRequest("idx", 0, primary, explainNode);
-        expectThrows(IllegalStateException.class, () -> findShardToExplain(failingRequest, allocation));
+        expectThrows(IllegalArgumentException.class, () -> findShardToExplain(failingRequest, allocation));
     }
 
     private static RoutingAllocation routingAllocation(ClusterState clusterState) {
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.allocation_explain/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.allocation_explain/10_basic.yml
index 6ec8651fe8..5e1bc42ec4 100644
--- a/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.allocation_explain/10_basic.yml
+++ b/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.allocation_explain/10_basic.yml
@@ -1,14 +1,19 @@
----
-"cluster shard allocation explanation test":
+"bad cluster shard allocation explanation request":
   - skip:
-      version: " - 5.1.99"
-      reason: explain API response output was changed starting in 5.2.0
+      version: " - 5.99.99"
+      reason: response status on bad request was changed starting in 5.6.0
 
   - do:
       # there aren't any unassigned shards to explain
-      catch: /illegal_state_exception/
+      catch: /illegal_argument_exception/
       cluster.allocation_explain: {}
 
+---
+"cluster shard allocation explanation test":
+  - skip:
+      version: " - 5.1.99"
+      reason: explain API response output was changed starting in 5.2.0
+
   - do:
       indices.create:
         index: test