From 59b7af6efa02b83dcb2beb8984b145916f3b6fa3 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Wed, 30 Jun 2021 16:51:43 +0200 Subject: [PATCH] Return error when remote indices are locally resolved 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 --- .../org/elasticsearch/get/GetActionIT.java | 7 ++ .../metadata/IndexNameExpressionResolver.java | 49 ++++++-- .../IndexNameExpressionResolverTests.java | 112 ++++++++++++++++++ 3 files changed, 161 insertions(+), 7 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/get/GetActionIT.java b/server/src/internalClusterTest/java/org/elasticsearch/get/GetActionIT.java index 6f99df24eaf95..a5d159423771e 100644 --- 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 index 64eb7c3820164..9f5bb657cf9e9 100644 --- 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 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 crossClusterIndices = new HashSet<>(); + for (String indexExpression : indexExpressions) { + if (indexExpression.contains(":")) { + List 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 index 67fab9db471a7..9e2bb0fcfa4f0 100644 --- 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 resolveConcreteIndexNameList(ClusterState state, SearchRequest request) { return Arrays .stream(indexNameExpressionResolver.concreteIndices(state, request))