Skip to content

Make remote cluster resolution stricter #40419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public TransportFieldCapabilitiesAction(TransportService transportService,
protected void doExecute(Task task, FieldCapabilitiesRequest request, final ActionListener<FieldCapabilitiesResponse> listener) {
final ClusterState clusterState = clusterService.state();
final Map<String, OriginalIndices> remoteClusterIndices = remoteClusterService.groupIndices(request.indicesOptions(),
request.indices(), idx -> indexNameExpressionResolver.hasIndexOrAlias(idx, clusterState));
request.indices());
final OriginalIndices localIndices = remoteClusterIndices.remove(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY);
final String[] concreteIndices;
if (localIndices == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ protected void doExecute(Task task, SearchRequest searchRequest, ActionListener<
}
final ClusterState clusterState = clusterService.state();
final Map<String, OriginalIndices> remoteClusterIndices = remoteClusterService.groupIndices(searchRequest.indicesOptions(),
searchRequest.indices(), idx -> indexNameExpressionResolver.hasIndexOrAlias(idx, clusterState));
searchRequest.indices());
OriginalIndices localIndices = remoteClusterIndices.remove(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY);
if (remoteClusterIndices.isEmpty()) {
executeLocalSearch(task, timeProvider, searchRequest, localIndices, clusterState, listener);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.cluster.metadata;

import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.transport.NoSuchRemoteClusterException;

import java.util.ArrayList;
import java.util.Collections;
Expand All @@ -36,20 +37,21 @@ public final class ClusterNameExpressionResolver {
private final WildcardExpressionResolver wildcardResolver = new WildcardExpressionResolver();

/**
* Resolves the provided cluster expression to matching cluster names. This method only
* supports exact or wildcard matches.
* Resolves the provided cluster expression to matching cluster names. Supports exact or wildcard matches.
* Throws {@link NoSuchRemoteClusterException} in case there are no registered remote clusters matching the provided expression.
*
* @param remoteClusters the aliases for remote clusters
* @param clusterExpression the expressions that can be resolved to cluster names.
* @return the resolved cluster aliases.
* @throws NoSuchRemoteClusterException if there are no remote clusters matching the provided expression
*/
public List<String> resolveClusterNames(Set<String> remoteClusters, String clusterExpression) {
if (remoteClusters.contains(clusterExpression)) {
return Collections.singletonList(clusterExpression);
} else if (Regex.isSimpleMatchPattern(clusterExpression)) {
return wildcardResolver.resolve(remoteClusters, clusterExpression);
} else {
return Collections.emptyList();
throw new NoSuchRemoteClusterException(clusterExpression);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
*/
public final class NoSuchRemoteClusterException extends ResourceNotFoundException {

NoSuchRemoteClusterException(String clusterName) {
public NoSuchRemoteClusterException(String clusterName) {
//No node available for cluster
super("no such remote cluster: [" + clusterName + "]");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
import java.util.NavigableSet;
import java.util.Set;
import java.util.TreeSet;
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand Down Expand Up @@ -246,36 +245,19 @@ static DiscoveryNode buildSeedNode(String clusterName, String address, boolean p
*
* @param remoteClusterNames the remote cluster names
* @param requestIndices the indices in the search request to filter
* @param indexExists a predicate that can test if a certain index or alias exists in the local cluster
*
* @return a map of grouped remote and local indices
*/
protected Map<String, List<String>> groupClusterIndices(Set<String> remoteClusterNames, String[] requestIndices,
Predicate<String> indexExists) {
protected Map<String, List<String>> groupClusterIndices(Set<String> remoteClusterNames, String[] requestIndices) {
Map<String, List<String>> perClusterIndices = new HashMap<>();
for (String index : requestIndices) {
int i = index.indexOf(RemoteClusterService.REMOTE_CLUSTER_INDEX_SEPARATOR);
if (i >= 0) {
String remoteClusterName = index.substring(0, i);
List<String> clusters = clusterNameResolver.resolveClusterNames(remoteClusterNames, remoteClusterName);
if (clusters.isEmpty() == false) {
if (indexExists.test(index)) {
//We use ":" as a separator for remote clusters. There may be a conflict if there is an index that is named
//remote_cluster_alias:index_name - for this case we fail the request. The user can easily change the cluster alias
//if that happens. Note that indices and aliases can be created with ":" in their names names up to 6.last, which
//means such names need to be supported until 7.last. It will be possible to remove this check from 8.0 on.
throw new IllegalArgumentException("Can not filter indices; index " + index +
" exists but there is also a remote cluster named: " + remoteClusterName);
}
String indexName = index.substring(i + 1);
for (String clusterName : clusters) {
perClusterIndices.computeIfAbsent(clusterName, k -> new ArrayList<>()).add(indexName);
}
} else {
//Indices and aliases can be created with ":" in their names up to 6.last (although deprecated), and still be
//around in 7.x. That's why we need to be lenient here and treat the index as local although it contains ":".
//It will be possible to remove such leniency and assume that no local indices contain ":" only from 8.0 on.
perClusterIndices.computeIfAbsent(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY, k -> new ArrayList<>()).add(index);
String indexName = index.substring(i + 1);
for (String clusterName : clusters) {
perClusterIndices.computeIfAbsent(clusterName, k -> new ArrayList<>()).add(indexName);
}
} else {
perClusterIndices.computeIfAbsent(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY, k -> new ArrayList<>()).add(index);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,10 +288,10 @@ boolean isRemoteNodeConnected(final String remoteCluster, final DiscoveryNode no
return remoteClusters.get(remoteCluster).isNodeConnected(node);
}

public Map<String, OriginalIndices> groupIndices(IndicesOptions indicesOptions, String[] indices, Predicate<String> indexExists) {
public Map<String, OriginalIndices> groupIndices(IndicesOptions indicesOptions, String[] indices) {
Map<String, OriginalIndices> originalIndicesMap = new HashMap<>();
if (isCrossClusterSearchEnabled()) {
final Map<String, List<String>> groupedIndices = groupClusterIndices(getRemoteClusterNames(), indices, indexExists);
final Map<String, List<String>> groupedIndices = groupClusterIndices(getRemoteClusterNames(), indices);
if (groupedIndices.isEmpty()) {
//search on _all in the local cluster if neither local indices nor remote indices were specified
originalIndicesMap.put(LOCAL_CLUSTER_GROUP_KEY, new OriginalIndices(Strings.EMPTY_ARRAY, indicesOptions));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.cluster.metadata;

import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.transport.NoSuchRemoteClusterException;

import java.util.Arrays;
import java.util.HashSet;
Expand All @@ -43,8 +44,8 @@ public void testExactMatch() {
}

public void testNoWildCardNoMatch() {
List<String> clusters = clusterNameResolver.resolveClusterNames(remoteClusters, "totallyDifferent2");
assertTrue(clusters.isEmpty());
expectThrows(NoSuchRemoteClusterException.class,
() -> clusterNameResolver.resolveClusterNames(remoteClusters, "totallyDifferent2"));
}

public void testWildCardNoMatch() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,22 +217,21 @@ public void testGroupClusterIndices() throws IOException {
assertTrue(service.isRemoteClusterRegistered("cluster_2"));
assertFalse(service.isRemoteClusterRegistered("foo"));
Map<String, List<String>> perClusterIndices = service.groupClusterIndices(service.getRemoteClusterNames(),
new String[]{"foo:bar", "cluster_1:bar", "cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo",
"cluster*:baz", "*:boo", "no*match:boo"},
i -> false);
new String[]{"cluster_1:bar", "cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo", "cluster*:baz",
"*:boo"});
List<String> localIndices = perClusterIndices.remove(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY);
assertNotNull(localIndices);
assertEquals(Arrays.asList("foo:bar", "foo", "no*match:boo"), localIndices);
assertEquals("foo", localIndices.get(0));
assertEquals(2, perClusterIndices.size());
assertEquals(Arrays.asList("bar", "test", "baz", "boo"), perClusterIndices.get("cluster_1"));
assertEquals(Arrays.asList("foo:bar", "foo*", "baz", "boo"), perClusterIndices.get("cluster_2"));

IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () ->
service.groupClusterIndices(service.getRemoteClusterNames(), new String[]{"foo:bar", "cluster_1:bar",
"cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo"}, "cluster_1:bar"::equals));
expectThrows(NoSuchRemoteClusterException.class, () -> service.groupClusterIndices(service.getRemoteClusterNames(),
new String[]{"foo:bar", "cluster_1:bar", "cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo"}));

assertEquals("Can not filter indices; index cluster_1:bar exists but there is also a remote cluster named:" +
" cluster_1", iae.getMessage());
expectThrows(NoSuchRemoteClusterException.class, () ->
service.groupClusterIndices(service.getRemoteClusterNames(), new String[]{"cluster_1:bar",
"cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "does_not_exist:*"}));
}
}
}
Expand Down Expand Up @@ -264,34 +263,28 @@ public void testGroupIndices() throws IOException {
assertFalse(service.isRemoteClusterRegistered("foo"));
{
Map<String, OriginalIndices> perClusterIndices = service.groupIndices(IndicesOptions.LENIENT_EXPAND_OPEN,
new String[]{"foo:bar", "cluster_1:bar", "cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo",
"cluster*:baz", "*:boo", "no*match:boo"},
i -> false);
new String[]{"cluster_1:bar", "cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo", "cluster*:baz",
"*:boo"});
assertEquals(3, perClusterIndices.size());
assertArrayEquals(new String[]{"foo:bar", "foo", "no*match:boo"},
assertArrayEquals(new String[]{"foo"},
perClusterIndices.get(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY).indices());
assertArrayEquals(new String[]{"bar", "test", "baz", "boo"}, perClusterIndices.get("cluster_1").indices());
assertArrayEquals(new String[]{"foo:bar", "foo*", "baz", "boo"}, perClusterIndices.get("cluster_2").indices());
}
{
IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () ->
service.groupClusterIndices(service.getRemoteClusterNames(), new String[]{"foo:bar", "cluster_1:bar",
"cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo"}, "cluster_1:bar"::equals));
assertEquals("Can not filter indices; index cluster_1:bar exists but there is also a remote cluster named:" +
" cluster_1", iae.getMessage());
expectThrows(NoSuchRemoteClusterException.class, () -> service.groupClusterIndices(service.getRemoteClusterNames(),
new String[]{"foo:bar", "cluster_1:bar", "cluster_2:foo:bar", "cluster_1:test", "cluster_2:foo*", "foo"}));
}
{
Map<String, OriginalIndices> perClusterIndices = service.groupIndices(IndicesOptions.LENIENT_EXPAND_OPEN,
new String[]{"cluster_1:bar", "cluster_2:foo*"},
i -> false);
new String[]{"cluster_1:bar", "cluster_2:foo*"});
assertEquals(2, perClusterIndices.size());
assertArrayEquals(new String[]{"bar"}, perClusterIndices.get("cluster_1").indices());
assertArrayEquals(new String[]{"foo*"}, perClusterIndices.get("cluster_2").indices());
}
{
Map<String, OriginalIndices> perClusterIndices = service.groupIndices(IndicesOptions.LENIENT_EXPAND_OPEN,
Strings.EMPTY_ARRAY,
i -> false);
Strings.EMPTY_ARRAY);
assertEquals(1, perClusterIndices.size());
assertArrayEquals(Strings.EMPTY_ARRAY, perClusterIndices.get(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY).indices());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ protected void updateRemoteCluster(String clusterAlias, List<String> addresses,
}

ResolvedIndices splitLocalAndRemoteIndexNames(String... indices) {
final Map<String, List<String>> map = super.groupClusterIndices(clusters, indices, exists -> false);
final Map<String, List<String>> map = super.groupClusterIndices(clusters, indices);
final List<String> local = map.remove(LOCAL_CLUSTER_GROUP_KEY);
final List<String> remote = map.entrySet().stream()
.flatMap(e -> e.getValue().stream().map(v -> e.getKey() + REMOTE_CLUSTER_INDEX_SEPARATOR + v))
Expand Down