From a8bd57b93cf6c3ca4ba21cc2f5c4c6e9a1678fd9 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Fri, 13 Jan 2017 12:22:52 +0100 Subject: [PATCH] Fail request if there is a local index that matches the both a remote and local index --- .../action/search/RemoteClusterService.java | 13 +++++++++++-- .../action/search/TransportSearchAction.java | 12 +++++++----- .../action/search/RemoteClusterServiceTests.java | 6 +++++- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java b/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java index 107f910597c..3a91430b6d9 100644 --- a/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java +++ b/core/src/main/java/org/elasticsearch/action/search/RemoteClusterService.java @@ -88,7 +88,7 @@ public final class RemoteClusterService extends AbstractComponent implements Clo /** * The name of a node attribute to select nodes that should be connected to in the remote cluster. - * For instance a node can be configured with node.node_attr.gateway: true in order to be eligible as a gateway node between + * For instance a node can be configured with node.attr.gateway: true in order to be eligible as a gateway node between * clusters. In that case search.remote.node_attribute: gateway can be used to filter out other nodes in the remote cluster. * The value of the setting is expected to be a boolean, true for nodes that can become gateways, false otherwise. */ @@ -177,15 +177,24 @@ public final class RemoteClusterService extends AbstractComponent implements Clo * * @param perClusterIndices a map to fill with remote cluster indices from the given request indices * @param requestIndices the indices in the search request to filter + * @param indexExists a predicate that can test if a certain index or alias exists + * * @return all indices in the requestIndices array that are not remote cluster indices */ - public String[] filterIndices(Map> perClusterIndices, String[] requestIndices) { + public String[] filterIndices(Map> perClusterIndices, String[] requestIndices, Predicate indexExists) { List localIndicesList = new ArrayList<>(); for (String index : requestIndices) { int i = index.indexOf(REMOTE_CLUSTER_INDEX_SEPARATOR); if (i >= 0) { String remoteCluster = index.substring(0, i); if (isRemoteClusterRegistered(remoteCluster)) { + if (indexExists.test(index)) { + // we use : as a separator for remote clusters. might conflict if there is an index that is actually named + // remote_cluster_alias:index_name - for this case we fail the request. the user can easily change the cluster alias + // if that happens + throw new IllegalArgumentException("Index " + index + " exists but there is also a remote cluster named: " + + remoteCluster + " can't filter indices"); + } String remoteIndex = index.substring(i + 1); List indices = perClusterIndices.get(remoteCluster); if (indices == null) { diff --git a/core/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java b/core/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java index ffe5f07c543..d7441484644 100644 --- a/core/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java +++ b/core/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java @@ -122,9 +122,11 @@ public class TransportSearchAction extends HandledTransportAction> remoteIndicesByCluster; + final ClusterState clusterState = clusterService.state(); if (remoteClusterService.isCrossClusterSearchEnabled()) { remoteIndicesByCluster = new HashMap<>(); - localIndices = remoteClusterService.filterIndices(remoteIndicesByCluster, searchRequest.indices()); + localIndices = remoteClusterService.filterIndices(remoteIndicesByCluster, searchRequest.indices(), + idx -> indexNameExpressionResolver.hasIndexOrAlias(idx, clusterState)); } else { remoteIndicesByCluster = Collections.emptyMap(); localIndices = searchRequest.indices(); @@ -132,7 +134,7 @@ public class TransportSearchAction extends HandledTransportAction null, Collections.emptyMap(), listener); + (nodeId) -> null, clusterState, Collections.emptyMap(), listener); } else { remoteClusterService.collectSearchShards(searchRequest, remoteIndicesByCluster, ActionListener.wrap((searchShardsResponses) -> { @@ -141,16 +143,16 @@ public class TransportSearchAction extends HandledTransportAction connectionFunction = remoteClusterService.processRemoteShards( searchShardsResponses, remoteShardIterators, remoteAliasFilters); executeSearch((SearchTask)task, startTimeInMillis, searchRequest, localIndices, remoteShardIterators, - connectionFunction, remoteAliasFilters, listener); + connectionFunction, clusterState, remoteAliasFilters, listener); }, listener::onFailure)); } } private void executeSearch(SearchTask task, long startTimeInMillis, SearchRequest searchRequest, String[] localIndices, List remoteShardIterators, Function remoteConnections, - Map remoteAliasMap, ActionListener listener) { + ClusterState clusterState, Map remoteAliasMap, + ActionListener listener) { - ClusterState clusterState = clusterService.state(); clusterState.blocks().globalBlockedRaiseException(ClusterBlockLevel.READ); // TODO: I think startTime() should become part of ActionRequest and that should be used both for index name // date math expressions and $now in scripts. This way all apis will deal with now in the same way instead diff --git a/core/src/test/java/org/elasticsearch/action/search/RemoteClusterServiceTests.java b/core/src/test/java/org/elasticsearch/action/search/RemoteClusterServiceTests.java index 9650b0821dc..6c3752d7104 100644 --- a/core/src/test/java/org/elasticsearch/action/search/RemoteClusterServiceTests.java +++ b/core/src/test/java/org/elasticsearch/action/search/RemoteClusterServiceTests.java @@ -144,11 +144,15 @@ public class RemoteClusterServiceTests extends ESTestCase { assertFalse(service.isRemoteClusterRegistered("foo")); Map> perClusterIndices = new HashMap<>(); String[] localIndices = service.filterIndices(perClusterIndices, new String[]{"foo:bar", "cluster_1:bar", - "cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo"}); + "cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo"}, i -> false); assertArrayEquals(new String[]{"foo:bar", "foo"}, localIndices); assertEquals(2, perClusterIndices.size()); assertEquals(Arrays.asList("bar", "test"), perClusterIndices.get("cluster_1")); assertEquals(Arrays.asList("foo:bar", "foo*"), perClusterIndices.get("cluster_2")); + + expectThrows(IllegalArgumentException.class, () -> + service.filterIndices(perClusterIndices, new String[]{"foo:bar", "cluster_1:bar", + "cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo"}, i -> "cluster_1:bar".equals(i))); } } }