Skip to content

Commit 83d0e25

Browse files
Cluster Allocation Explain API Throws IAE
Throws an Illegal Argument Exception when a non-integer value is passed as the `shard` parameter. The legacy behaviour of allowing strings to be passed, providing they are strings containing integers, is maintained.
1 parent 1e6473a commit 83d0e25

File tree

3 files changed

+137
-16
lines changed

3 files changed

+137
-16
lines changed

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cluster.allocation_explain/10_basic.yml

Lines changed: 103 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
body: { "index": "test", "shard": 2147483647, "primary": true }
4040

4141
---
42-
"cluster shard allocation explanation test with long shard value":
42+
"cluster shard allocation explanation test with an invalid, string shard parameter in the body":
4343
- do:
4444
indices.create:
4545
index: test
@@ -49,33 +49,107 @@
4949
- do:
5050
catch: /x_content_parse_exception/
5151
cluster.allocation_explain:
52-
body: { "index": "test", "shard": 214748364777, "primary": true }
52+
body: { "index": "test", "shard": "wrong", "primary": true }
5353

5454
---
55-
"cluster shard allocation explanation test with float shard value":
55+
"cluster shard allocation explanation test with a valid, string shard parameter in the body":
5656
- do:
5757
indices.create:
5858
index: test
59-
body: { "settings": { "index.number_of_shards": 2, "index.number_of_replicas": 0 } }
6059

6160
- match: { acknowledged: true }
6261

6362
- do:
6463
cluster.allocation_explain:
65-
body: { "index": "test", "shard": 1.0, "primary": true }
64+
body: { "index": "test", "shard": "0", "primary": true }
6665

6766
- match: { current_state: "started" }
6867
- is_true: current_node.id
6968
- match: { index: "test" }
70-
- match: { shard: 1 }
69+
- match: { shard: 0 }
7170
- match: { primary: true }
7271
- is_true: can_remain_on_current_node
7372
- is_true: can_rebalance_cluster
7473
- is_true: can_rebalance_to_other_node
7574
- is_true: rebalance_explanation
7675

76+
---
77+
"cluster shard allocation explanation test with long shard value":
78+
- requires:
79+
capabilities:
80+
- method: GET
81+
path: /_cluster/allocation/explain
82+
capabilities: [shard_parameter_non_integer_validation]
83+
test_runner_features: [capabilities]
84+
reason: Additional validation for the shard parameter was added in 9.2.0
85+
86+
- do:
87+
indices.create:
88+
index: test
89+
90+
- match: { acknowledged: true }
91+
92+
- do:
93+
catch: /x_content_parse_exception/
94+
cluster.allocation_explain:
95+
body: { "index": "test", "shard": 214748364777, "primary": true }
96+
97+
---
98+
"cluster shard allocation explanation test with float shard value":
99+
- requires:
100+
capabilities:
101+
- method: GET
102+
path: /_cluster/allocation/explain
103+
capabilities: [ shard_parameter_non_integer_validation ]
104+
test_runner_features: [ capabilities ]
105+
reason: Additional validation for the shard parameter was added in 9.2.0
106+
107+
- do:
108+
indices.create:
109+
index: test
110+
body: { "settings": { "index.number_of_shards": 2, "index.number_of_replicas": 0 } }
111+
112+
- match: { acknowledged: true }
113+
114+
- do:
115+
catch: /illegal_argument_exception/
116+
cluster.allocation_explain:
117+
body: { "index": "test", "shard": 1.0, "primary": true }
118+
119+
---
120+
"cluster shard allocation explanation test with stringified float shard value":
121+
- requires:
122+
capabilities:
123+
- method: GET
124+
path: /_cluster/allocation/explain
125+
capabilities: [ shard_parameter_non_integer_validation ]
126+
test_runner_features: [ capabilities ]
127+
reason: Additional validation for the shard parameter was added in 9.2.0
128+
129+
130+
- do:
131+
indices.create:
132+
index: test
133+
body: { "settings": { "index.number_of_shards": 2, "index.number_of_replicas": 0 } }
134+
135+
- match: { acknowledged: true }
136+
137+
- do:
138+
catch: /illegal_argument_exception/
139+
cluster.allocation_explain:
140+
body: { "index": "test", "shard": "1.0", "primary": true }
141+
142+
77143
---
78144
"cluster shard allocation explanation test with double shard value":
145+
- requires:
146+
capabilities:
147+
- method: GET
148+
path: /_cluster/allocation/explain
149+
capabilities: [ shard_parameter_non_integer_validation ]
150+
test_runner_features: [ capabilities ]
151+
reason: Additional validation for the shard parameter was added in 9.2.0
152+
79153
- do:
80154
indices.create:
81155
index: test
@@ -84,18 +158,32 @@
84158
- match: { acknowledged: true }
85159

86160
- do:
161+
catch: /x_content_parse_exception/
87162
cluster.allocation_explain:
88163
body: { "index": "test", "shard": 1.1234567891234567, "primary": true }
89164

90-
- match: { current_state: "started" }
91-
- is_true: current_node.id
92-
- match: { index: "test" }
93-
- match: { shard: 1 }
94-
- match: { primary: true }
95-
- is_true: can_remain_on_current_node
96-
- is_true: can_rebalance_cluster
97-
- is_true: can_rebalance_to_other_node
98-
- is_true: rebalance_explanation
165+
---
166+
"cluster shard allocation explanation test with stringified double shard value":
167+
- requires:
168+
capabilities:
169+
- method: GET
170+
path: /_cluster/allocation/explain
171+
capabilities: [ shard_parameter_non_integer_validation ]
172+
test_runner_features: [ capabilities ]
173+
reason: Additional validation for the shard parameter was added in 9.2.0
174+
175+
- do:
176+
indices.create:
177+
index: test
178+
body: { "settings": { "index.number_of_shards": 2, "index.number_of_replicas": 0 } }
179+
180+
- match: { acknowledged: true }
181+
182+
- do:
183+
catch: /x_content_parse_exception/
184+
cluster.allocation_explain:
185+
body: { "index": "test", "shard": "1.1234567891234567", "primary": true }
186+
99187

100188
---
101189
"cluster shard allocation explanation test with three valid body parameters":

server/src/main/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainRequest.java

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,34 @@ public class ClusterAllocationExplainRequest extends MasterNodeRequest<ClusterAl
3131
private static final ObjectParser<ClusterAllocationExplainRequest, Void> PARSER = new ObjectParser<>("cluster/allocation/explain");
3232
static {
3333
PARSER.declareString(ClusterAllocationExplainRequest::setIndex, new ParseField("index"));
34-
PARSER.declareInt(ClusterAllocationExplainRequest::setShard, new ParseField("shard"));
34+
PARSER.declareField(
35+
ClusterAllocationExplainRequest::setShard,
36+
(xContentParser) -> {
37+
if (xContentParser.currentToken() == XContentParser.Token.VALUE_NUMBER) {
38+
if (xContentParser.numberType() == XContentParser.NumberType.INT) {
39+
return xContentParser.intValue();
40+
} else {
41+
throw new IllegalArgumentException("Expected an integer value for [shard]");
42+
}
43+
} else if (xContentParser.currentToken() == XContentParser.Token.VALUE_STRING) {
44+
String text = xContentParser.text();
45+
try {
46+
// Only accept if the string is a valid integer representation
47+
if (text.matches("[-+]?\\d+")) {
48+
return Integer.parseInt(text);
49+
} else {
50+
throw new IllegalArgumentException("String value for [shard] must be an integer, but was: " + text);
51+
}
52+
} catch (NumberFormatException e) {
53+
throw new IllegalArgumentException("Invalid integer value for [shard]: " + text, e);
54+
}
55+
} else {
56+
throw new IllegalArgumentException("Expected an integer for [shard]");
57+
}
58+
},
59+
new ParseField("shard"),
60+
ObjectParser.ValueType.INT
61+
);
3562
PARSER.declareBoolean(ClusterAllocationExplainRequest::setPrimary, new ParseField("primary"));
3663
PARSER.declareString(ClusterAllocationExplainRequest::setCurrentNode, new ParseField("current_node"));
3764
}

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterAllocationExplainAction.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import java.io.IOException;
2424
import java.util.List;
25+
import java.util.Set;
2526

2627
import static org.elasticsearch.rest.RestRequest.Method.GET;
2728
import static org.elasticsearch.rest.RestRequest.Method.POST;
@@ -47,6 +48,11 @@ public boolean allowSystemIndexAccessByDefault() {
4748
return true;
4849
}
4950

51+
@Override
52+
public Set<String> supportedCapabilities() {
53+
return Set.of("shard_parameter_non_integer_validation");
54+
}
55+
5056
@Override
5157
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
5258
final var req = new ClusterAllocationExplainRequest(RestUtils.getMasterNodeTimeout(request));

0 commit comments

Comments
 (0)