Skip to content

Commit

Permalink
Return error when remote indices are locally resolved
Browse files Browse the repository at this point in the history
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, but 8.x won't. We can then switch 8.0 to throw a more specific error in place of the index not found, to signal that remote indices have been requested in the context of an API that does not support cross cluster calls.

relates to elastic#26247
  • Loading branch information
javanna committed Jun 24, 2021
1 parent 3f44a5b commit f542fa7
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@
import org.elasticsearch.action.get.MultiGetResponse;
import org.elasticsearch.action.index.IndexResponse;
import org.elasticsearch.action.support.DefaultShardOperationFailedException;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.lucene.uid.Versions;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.index.IndexModule;
import org.elasticsearch.index.engine.EngineTestCase;
import org.elasticsearch.index.engine.VersionConflictEngineException;
Expand Down Expand Up @@ -781,6 +781,12 @@ void indexSingleDocumentWithStringFieldsGeneratedFromText(boolean stored, boolea
index("test", "1", doc);
}

public void testGetRemoteIndex() {
IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> client().prepareGet("cluster:index", "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 docId, String[] fields) {
assertGetFieldsAlwaysWorks(index, docId, fields, null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.IndexAbstraction.Type;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.logging.DeprecationCategory;
Expand All @@ -26,6 +25,7 @@
import org.elasticsearch.common.util.CollectionUtils;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.indices.IndexClosedException;
Expand Down Expand Up @@ -112,7 +112,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, getSystemIndexAccessPredicate());
Expand Down Expand Up @@ -163,7 +163,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);
Expand All @@ -185,7 +185,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,
Expand All @@ -203,11 +203,20 @@ 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) {
List<String> crossClusterIndices = Arrays.stream(indexExpressions)
.filter(index -> index.contains(":")).collect(Collectors.toList());
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.
Expand Down Expand Up @@ -386,7 +395,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) {
Expand Down Expand Up @@ -423,7 +432,8 @@ public Index concreteWriteIndex(ClusterState state, IndicesRequest request) {
* @param index index that can be resolved to alias or index name.
* @param allowNoIndices whether to allow resolve to no index
* @param includeDataStreams Whether data streams should be included in the evaluation.
* @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 or null if no index
*/
public Index concreteWriteIndex(ClusterState state, IndicesOptions options, String index, boolean allowNoIndices,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2394,6 +2394,27 @@ public void testMathExpressionSupportWithOlderDate() {

assertEquals(resolved, "older-date-2020-12");
}

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, 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(true, true, randomBoolean(), randomBoolean(), randomBoolean());
IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(state, options, NONE);
String[] indexNames = indexNameExpressionResolver.concreteIndexNames(context, "cluster:index", "local");
assertEquals(0, indexNames.length);
}
}

private ClusterState systemIndexTestClusterState() {
Settings settings = Settings.builder().build();
Metadata.Builder mdBuilder = Metadata.builder()
Expand Down

0 comments on commit f542fa7

Please sign in to comment.