Skip to content

Commit

Permalink
Prevent order being lost for _nodes API filters (#42045)
Browse files Browse the repository at this point in the history
* Switch to using a list instead of a Set for the filters, so that the
order of these filters is kept.
  • Loading branch information
astefan authored May 10, 2019
1 parent eb42fa8 commit 74a7438
Show file tree
Hide file tree
Showing 2 changed files with 156 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
import static org.elasticsearch.rest.RestRequest.Method.GET;

public class RestNodesInfoAction extends BaseRestHandler {
private static final Set<String> ALLOWED_METRICS = Sets.newHashSet(
static final Set<String> ALLOWED_METRICS = Sets.newHashSet(
"http",
"ingest",
"indices",
Expand Down Expand Up @@ -69,24 +69,32 @@ public String getName() {

@Override
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
final NodesInfoRequest nodesInfoRequest = prepareRequest(request);
settingsFilter.addFilterSettingParams(request);

return channel -> client.admin().cluster().nodesInfo(nodesInfoRequest, new NodesResponseRestListener<>(channel));
}

static NodesInfoRequest prepareRequest(final RestRequest request) {
String[] nodeIds;
Set<String> metrics;

// special case like /_nodes/os (in this case os are metrics and not the nodeId)
// still, /_nodes/_local (or any other node id) should work and be treated as usual
// this means one must differentiate between allowed metrics and arbitrary node ids in the same place
if (request.hasParam("nodeId") && !request.hasParam("metrics")) {
Set<String> metricsOrNodeIds = Strings.tokenizeByCommaToSet(request.param("nodeId", "_all"));
String nodeId = request.param("nodeId", "_all");
Set<String> metricsOrNodeIds = Strings.tokenizeByCommaToSet(nodeId);
boolean isMetricsOnly = ALLOWED_METRICS.containsAll(metricsOrNodeIds);
if (isMetricsOnly) {
nodeIds = new String[]{"_all"};
metrics = metricsOrNodeIds;
} else {
nodeIds = metricsOrNodeIds.toArray(new String[]{});
nodeIds = Strings.tokenizeToStringArray(nodeId, ",");
metrics = Sets.newHashSet("_all");
}
} else {
nodeIds = Strings.splitStringByCommaToArray(request.param("nodeId", "_all"));
nodeIds = Strings.tokenizeToStringArray(request.param("nodeId", "_all"), ",");
metrics = Strings.tokenizeByCommaToSet(request.param("metrics", "_all"));
}

Expand All @@ -108,10 +116,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
nodesInfoRequest.ingest(metrics.contains("ingest"));
nodesInfoRequest.indices(metrics.contains("indices"));
}

settingsFilter.addFilterSettingParams(request);

return channel -> client.admin().cluster().nodesInfo(nodesInfoRequest, new NodesResponseRestListener<>(channel));
return nodesInfoRequest;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.rest.action.admin.cluster;

import org.elasticsearch.action.admin.cluster.node.info.NodesInfoRequest;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.rest.FakeRestRequest;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static org.elasticsearch.rest.action.admin.cluster.RestNodesInfoAction.ALLOWED_METRICS;

public class RestNodesInfoActionTests extends ESTestCase {

public void testDuplicatedFiltersAreNotRemoved() {
Map<String, String> params = new HashMap<>();
params.put("nodeId", "_all,master:false,_all");

RestRequest restRequest = buildRestRequest(params);
NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest);
assertArrayEquals(new String[] { "_all", "master:false", "_all" }, actual.nodesIds());
}

public void testOnlyMetrics() {
Map<String, String> params = new HashMap<>();
int metricsCount = randomIntBetween(1, ALLOWED_METRICS.size());
List<String> metrics = new ArrayList<>();

for(int i = 0; i < metricsCount; i++) {
metrics.add(randomFrom(ALLOWED_METRICS));
}
params.put("nodeId", String.join(",", metrics));

RestRequest restRequest = buildRestRequest(params);
NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest);
assertArrayEquals(new String[] { "_all" }, actual.nodesIds());
assertMetrics(metrics, actual);
}

public void testAllMetricsSelectedWhenNodeAndMetricSpecified() {
Map<String, String> params = new HashMap<>();
String nodeId = randomValueOtherThanMany(ALLOWED_METRICS::contains, () -> randomAlphaOfLength(23));
String metric = randomFrom(ALLOWED_METRICS);

params.put("nodeId", nodeId + "," + metric);
RestRequest restRequest = buildRestRequest(params);

NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest);
assertArrayEquals(new String[] { nodeId, metric }, actual.nodesIds());
assertAllMetricsTrue(actual);
}

public void testSeparateNodeIdsAndMetrics() {
Map<String, String> params = new HashMap<>();
List<String> nodeIds = new ArrayList<>(5);
List<String> metrics = new ArrayList<>(5);

for(int i = 0; i < 5; i++) {
nodeIds.add(randomValueOtherThanMany(ALLOWED_METRICS::contains, () -> randomAlphaOfLength(23)));
metrics.add(randomFrom(ALLOWED_METRICS));
}

params.put("nodeId", String.join(",", nodeIds));
params.put("metrics", String.join(",", metrics));
RestRequest restRequest = buildRestRequest(params);

NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest);
assertArrayEquals(nodeIds.toArray(), actual.nodesIds());
assertMetrics(metrics, actual);
}

public void testExplicitAllMetrics() {
Map<String, String> params = new HashMap<>();
List<String> nodeIds = new ArrayList<>(5);

for(int i = 0; i < 5; i++) {
nodeIds.add(randomValueOtherThanMany(ALLOWED_METRICS::contains, () -> randomAlphaOfLength(23)));
}

params.put("nodeId", String.join(",", nodeIds));
params.put("metrics", "_all");
RestRequest restRequest = buildRestRequest(params);

NodesInfoRequest actual = RestNodesInfoAction.prepareRequest(restRequest);
assertArrayEquals(nodeIds.toArray(), actual.nodesIds());
assertAllMetricsTrue(actual);
}

private FakeRestRequest buildRestRequest(Map<String, String> params) {
return new FakeRestRequest.Builder(xContentRegistry())
.withMethod(RestRequest.Method.GET)
.withPath("/_nodes")
.withParams(params)
.build();
}

private void assertMetrics(List<String> metrics, NodesInfoRequest nodesInfoRequest) {
assertTrue((metrics.contains("http") && nodesInfoRequest.http()) || metrics.contains("http") == false);
assertTrue((metrics.contains("ingest") && nodesInfoRequest.ingest()) || metrics.contains("ingest") == false);
assertTrue((metrics.contains("indices") && nodesInfoRequest.indices()) || metrics.contains("indices") == false);
assertTrue((metrics.contains("jvm") && nodesInfoRequest.jvm()) || metrics.contains("jvm") == false);
assertTrue((metrics.contains("os") && nodesInfoRequest.os()) || metrics.contains("os") == false);
assertTrue((metrics.contains("plugins") && nodesInfoRequest.plugins()) || metrics.contains("plugins") == false);
assertTrue((metrics.contains("process") && nodesInfoRequest.process()) || metrics.contains("process") == false);
assertTrue((metrics.contains("settings") && nodesInfoRequest.settings()) || metrics.contains("settings") == false);
assertTrue((metrics.contains("thread_pool") && nodesInfoRequest.threadPool()) || metrics.contains("thread_pool") == false);
assertTrue((metrics.contains("transport") && nodesInfoRequest.transport()) || metrics.contains("transport") == false);
}

private void assertAllMetricsTrue(NodesInfoRequest nodesInfoRequest) {
assertTrue(nodesInfoRequest.http());
assertTrue(nodesInfoRequest.ingest());
assertTrue(nodesInfoRequest.indices());
assertTrue(nodesInfoRequest.jvm());
assertTrue(nodesInfoRequest.os());
assertTrue(nodesInfoRequest.plugins());
assertTrue(nodesInfoRequest.process());
assertTrue(nodesInfoRequest.settings());
assertTrue(nodesInfoRequest.threadPool());
assertTrue(nodesInfoRequest.transport());
}
}

0 comments on commit 74a7438

Please sign in to comment.