Return error when remote indices are locally resolved (#74762)

javanna · web-flow · commit 5837f9d8c1f0 · 2021-07-01T13:21:52.000+02:00
We support the cluster:index syntax in all the API that support cross-cluster calls. Those API will extract remote indices, properly resolve them, and resolve locally the local indices. API that don't support this syntax though end up attempting to resolve such indices locally, which in most cases leads to an index not found exception depending on how ignore_unavailable is configured for the API. The reason for treating these index names as local is that we used to support ':' in index names, but that is no longer supported since 7.x. That means that 7.x may still have indices with ':' in their names from 6.x though. Silently failing makes it hard for users to know that they are even relying on a feature that is not supported, hence we'd like to start throwing error also in 7.x, similarly to what we did in #74556. This commit introduces a check for remote indices that are locally resolved, which is an indication of cross cluster syntax used in API that don't support cross cluster calls. We then check if that index exists in the local cluster, and if so we proceed to resolve it as usual. If not, we throw a specific error that makes it clear to users that they are relying on cross cluster calls calling API that does not support them. relates to #26247
diff --git a/server/src/internalClusterTest/java/org/elasticsearch/get/GetActionIT.java b/server/src/internalClusterTest/java/org/elasticsearch/get/GetActionIT.java
@@ -783,6 +783,13 @@ void indexSingleDocumentWithStringFieldsGeneratedFromText(boolean stored, boolea
         index("test", "_doc", "1", doc);
     }
 
+    public void testGetRemoteIndex() {
+        IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
+            () -> client().prepareGet("cluster:index", "_doc", "id").get());
+        assertEquals("Cross-cluster calls are not supported in this context but remote indices were requested: [cluster:index]",
+            iae.getMessage());
+    }
+
     private void assertGetFieldsAlwaysWorks(String index, String type, String docId, String[] fields) {
         assertGetFieldsAlwaysWorks(index, type, docId, fields, null);
     }
diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java
@@ -113,7 +113,7 @@ public Index[] concreteIndices(ClusterState state, IndicesRequest request) {
      * provided indices options in the context don't allow such a case, or if the final result of the indices resolution
      * contains no indices and the indices options in the context don't allow such a case.
      * @throws IllegalArgumentException if one of the aliases resolve to multiple indices and the provided
-     * indices options in the context don't allow such a case.
+     * indices options in the context don't allow such a case; if a remote index is requested.
      */
     public String[] concreteIndexNames(ClusterState state, IndicesOptions options, String... indexExpressions) {
         Context context = new Context(state, options, getSystemIndexAccessLevel(),
@@ -168,7 +168,7 @@ public List<String> dataStreamNames(ClusterState state, IndicesOptions options,
      * provided indices options in the context don't allow such a case, or if the final result of the indices resolution
      * contains no indices and the indices options in the context don't allow such a case.
      * @throws IllegalArgumentException if one of the aliases resolve to multiple indices and the provided
-     * indices options in the context don't allow such a case.
+     * indices options in the context don't allow such a case; if a remote index is requested.
      */
     public Index[] concreteIndices(ClusterState state, IndicesOptions options, String... indexExpressions) {
         return concreteIndices(state, options, false, indexExpressions);
@@ -190,7 +190,7 @@ public Index[] concreteIndices(ClusterState state, IndicesOptions options, boole
      * provided indices options in the context don't allow such a case, or if the final result of the indices resolution
      * contains no indices and the indices options in the context don't allow such a case.
      * @throws IllegalArgumentException if one of the aliases resolve to multiple indices and the provided
-     * indices options in the context don't allow such a case.
+     * indices options in the context don't allow such a case; if a remote index is requested.
      */
     public Index[] concreteIndices(ClusterState state, IndicesRequest request, long startTime) {
         Context context = new Context(state, request.indicesOptions(), startTime, false, false, request.includeDataStreams(), false,
@@ -208,11 +208,45 @@ String[] concreteIndexNames(Context context, String... indexExpressions) {
     }
 
     Index[] concreteIndices(Context context, String... indexExpressions) {
+        Metadata metadata = context.getState().metadata();
+        IndicesOptions options = context.getOptions();
         if (indexExpressions == null || indexExpressions.length == 0) {
             indexExpressions = new String[]{Metadata.ALL};
+        } else {
+            if (options.ignoreUnavailable() == false) {
+                Set<String> crossClusterIndices = new HashSet<>();
+                for (String indexExpression : indexExpressions) {
+                    if (indexExpression.contains(":")) {
+                        List<String> resolved;
+                        try {
+                            resolved = wildcardExpressionResolver.resolve(context, Collections.singletonList(indexExpression));
+                        } catch(IndexNotFoundException e) {
+                            resolved = Collections.emptyList();
+                        }
+                        if (resolved.isEmpty()) {
+                            crossClusterIndices.add(indexExpression);
+                        } else {
+                            boolean found = false;
+                            for (String index : resolved) {
+                                if (metadata.getIndicesLookup().containsKey(index)) {
+                                    found = true;
+                                    break;
+                                }
+                            }
+                            if (found == false) {
+                                crossClusterIndices.add(indexExpression);
+                            }
+                        }
+
+                    }
+                }
+                if (crossClusterIndices.size() > 0) {
+                    throw new IllegalArgumentException("Cross-cluster calls are not supported in this context but remote indices " +
+                        "were requested: " + crossClusterIndices);
+                }
+            }
         }
-        Metadata metadata = context.getState().metadata();
-        IndicesOptions options = context.getOptions();
+
         // If only one index is specified then whether we fail a request if an index is missing depends on the allow_no_indices
         // option. At some point we should change this, because there shouldn't be a reason why whether a single index
         // or multiple indices are specified yield different behaviour.
@@ -397,7 +431,7 @@ private static IllegalArgumentException aliasesNotSupportedException(String expr
      * @param state             the cluster state containing all the data to resolve to expression to a concrete index
      * @param request           The request that defines how the an alias or an index need to be resolved to a concrete index
      *                          and the expression that can be resolved to an alias or an index name.
-     * @throws IllegalArgumentException if the index resolution lead to more than one index
+     * @throws IllegalArgumentException if the index resolution returns more than one index; if a remote index is requested.
      * @return the concrete index obtained as a result of the index resolution
      */
     public Index concreteSingleIndex(ClusterState state, IndicesRequest request) {
@@ -416,7 +450,8 @@ public Index concreteSingleIndex(ClusterState state, IndicesRequest request) {
      * @param state             the cluster state containing all the data to resolve to expression to a concrete index
      * @param request           The request that defines how the an alias or an index need to be resolved to a concrete index
      *                          and the expression that can be resolved to an alias or an index name.
-     * @throws IllegalArgumentException if the index resolution does not lead to an index, or leads to more than one index
+     * @throws IllegalArgumentException if the index resolution does not lead to an index, or leads to more than one index, as well as
+     * if a remote index is requested.
      * @return the write index obtained as a result of the index resolution
      */
     public Index concreteWriteIndex(ClusterState state, IndicesRequest request) {
diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java
@@ -2441,6 +2441,118 @@ private ClusterState systemIndexTestClusterState() {
         return ClusterState.builder(new ClusterName("_name")).metadata(mdBuilder).build();
     }
 
+    public void testRemoteIndex() {
+        Metadata.Builder mdBuilder = Metadata.builder();
+        ClusterState state = ClusterState.builder(new ClusterName("_name")).metadata(mdBuilder).build();
+
+        {
+            IndicesOptions options = IndicesOptions.fromOptions(false, randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean());
+            IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(
+                state, options, SystemIndexAccessLevel.NONE);
+            IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
+                () -> indexNameExpressionResolver.concreteIndexNames(context, "cluster:index", "local"));
+            assertEquals("Cross-cluster calls are not supported in this context but remote indices were requested: [cluster:index]",
+                iae.getMessage());
+        }
+        {
+            IndicesOptions options = IndicesOptions.fromOptions(false, randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean());
+            IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(
+                state, options, SystemIndexAccessLevel.NONE);
+            IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
+                () -> indexNameExpressionResolver.concreteIndexNames(context, "cluster:*", "local"));
+            assertEquals("Cross-cluster calls are not supported in this context but remote indices were requested: [cluster:*]",
+                iae.getMessage());
+        }
+        {
+            IndicesOptions options = IndicesOptions.fromOptions(false, randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean());
+            IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(
+                state, options, SystemIndexAccessLevel.NONE);
+            IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
+                () -> indexNameExpressionResolver.concreteIndexNames(context, "cluster:i*", "local"));
+            assertEquals("Cross-cluster calls are not supported in this context but remote indices were requested: [cluster:i*]",
+                iae.getMessage());
+        }
+        {
+            IndicesOptions options = IndicesOptions.fromOptions(true, true, randomBoolean(), randomBoolean(), randomBoolean());
+            IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(
+                state, options, SystemIndexAccessLevel.NONE);
+            String[] indexNames = indexNameExpressionResolver.concreteIndexNames(context, "cluster:index", "local");
+            assertEquals(0, indexNames.length);
+        }
+        {
+            IndicesOptions options = IndicesOptions.fromOptions(true, true, randomBoolean(), randomBoolean(), randomBoolean());
+            IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(
+                state, options, SystemIndexAccessLevel.NONE);
+            String[] indexNames = indexNameExpressionResolver.concreteIndexNames(context, "cluster:i*", "local");
+            assertEquals(0, indexNames.length);
+        }
+    }
+
+    public void testColonWithinIndexName() {
+        Settings settings = Settings.builder().build();
+        Metadata.Builder mdBuilder = Metadata.builder()
+            .put(indexBuilder("cluster:index", settings).state(State.OPEN));
+        ClusterState state = ClusterState.builder(new ClusterName("_name")).metadata(mdBuilder).build();
+
+        {
+            IndicesOptions options = IndicesOptions.fromOptions(randomBoolean(), randomBoolean(),
+                randomBoolean(), randomBoolean(), randomBoolean());
+            IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(
+                state, options, SystemIndexAccessLevel.NONE);
+            String[] indexNames = indexNameExpressionResolver.concreteIndexNames(context, "cluster:index");
+            assertThat(indexNames, arrayContaining("cluster:index"));
+        }
+        //Using wildcards, expand wildcards to open indices: true -> index locally resolved
+        {
+            IndicesOptions options = IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), true, randomBoolean(), randomBoolean());
+            IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(
+                state, options, SystemIndexAccessLevel.NONE);
+            String[] indexNames = indexNameExpressionResolver.concreteIndexNames(context, "cluster:*");
+            assertThat(indexNames, arrayContaining("cluster:index"));
+        }
+        {
+            IndicesOptions options = IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), true, randomBoolean(), randomBoolean());
+            IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(
+                state, options, SystemIndexAccessLevel.NONE);
+            String[] indexNames = indexNameExpressionResolver.concreteIndexNames(context, "cluster:in*");
+            assertThat(indexNames, arrayContaining("cluster:index"));
+        }
+        //With wildcards, ignore_unavailable: false, expand wildcards to open indices: false -> error about cross cluster indices
+        {
+            IndicesOptions options = IndicesOptions.fromOptions(false, randomBoolean(), false, randomBoolean(), randomBoolean());
+            IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(
+                state, options, SystemIndexAccessLevel.NONE);
+            IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
+                () -> indexNameExpressionResolver.concreteIndexNames(context, "cluster:*"));
+            assertEquals("Cross-cluster calls are not supported in this context but remote indices were requested: [cluster:*]",
+                iae.getMessage());
+        }
+        {
+            IndicesOptions options = IndicesOptions.fromOptions(false, randomBoolean(), false, randomBoolean(), randomBoolean());
+            IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(
+                state, options, SystemIndexAccessLevel.NONE);
+            IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
+                () -> indexNameExpressionResolver.concreteIndexNames(context, "cluster:in*"));
+            assertEquals("Cross-cluster calls are not supported in this context but remote indices were requested: [cluster:in*]",
+                iae.getMessage());
+        }
+        //With wildcards: ignore_unavailable: true, allow_no_indices: true, expand wildcards to open indices: false -> empty list of indices
+        {
+            IndicesOptions options = IndicesOptions.fromOptions(true, true, false, randomBoolean(), randomBoolean());
+            IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(
+                state, options, SystemIndexAccessLevel.NONE);
+            String[] indexNames = indexNameExpressionResolver.concreteIndexNames(context, "cluster:*");
+            assertEquals(0, indexNames.length);
+        }
+        {
+            IndicesOptions options = IndicesOptions.fromOptions(true, true, false, randomBoolean(), randomBoolean());
+            IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(
+                state, options, SystemIndexAccessLevel.NONE);
+            String[] indexNames = indexNameExpressionResolver.concreteIndexNames(context, "cluster:in*");
+            assertEquals(0, indexNames.length);
+        }
+    }
+
     private List<String> resolveConcreteIndexNameList(ClusterState state, SearchRequest request) {
         return Arrays
             .stream(indexNameExpressionResolver.concreteIndices(state, request))

Original file line number	Diff line number	Diff line change
`@@ -783,6 +783,13 @@ void indexSingleDocumentWithStringFieldsGeneratedFromText(boolean stored, boolea`
`783`	`783`	`index("test", "_doc", "1", doc);`
`784`	`784`	`}`
`785`	`785`
	`786`	`+ public void testGetRemoteIndex() {`
	`787`	`+ IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,`
	`788`	`+ () -> client().prepareGet("cluster:index", "_doc", "id").get());`
	`789`	`+ assertEquals("Cross-cluster calls are not supported in this context but remote indices were requested: [cluster:index]",`
	`790`	`+ iae.getMessage());`
	`791`	`+ }`
	`792`	`+`
`786`	`793`	`private void assertGetFieldsAlwaysWorks(String index, String type, String docId, String[] fields) {`
`787`	`794`	`assertGetFieldsAlwaysWorks(index, type, docId, fields, null);`
`788`	`795`	`}`