Make remote cluster resolution stricter (#40419)

Remote cluster resolution is currently lenient, to support local
indices that may contain `:` in their name. From 8.0 on, there can no
longer be indices in the cluster that contain `:` in their name, hence
we can make remote cluster resolution stricter. Instead of treating
any index expression containing a `:` whenever there is no corresponding
matching remote cluster registered, we now throw a
`NoSuchRemoteClusterException`.

Closes #37863

Author: javanna

server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java

@@ -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) {

server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java

@@ -205,7 +205,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);

server/src/main/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolver.java

@@ -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;
@@ -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);
         }
     }
 

server/src/main/java/org/elasticsearch/transport/NoSuchRemoteClusterException.java

@@ -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 + "]");
     }

server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java

@@ -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;
@@ -244,36 +243,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);

server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java

@@ -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));

server/src/test/java/org/elasticsearch/cluster/metadata/ClusterNameExpressionResolverTests.java

@@ -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;
@@ -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() {

server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java

@@ -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:*"}));
                 }
             }
         }
@@ -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());
                     }

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java

@@ -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))