From d32d16f1171f800b484de008d25019672e6f6e45 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Mon, 11 Jan 2016 10:27:37 +0100 Subject: [PATCH] Check lenient_expand_open after aliases have been resolved We fail today with ClusterBlockExceptions if an alias expands to a closed index during search since we miss to check the index option down the road after we expanded aliases. Closes #13278 --- .../metadata/IndexNameExpressionResolver.java | 51 ++++++++++++++----- .../IndexNameExpressionResolverTests.java | 35 ++++++++++++- .../WildcardExpressionResolverTests.java | 2 +- .../routing/AliasResolveRoutingIT.java | 20 ++++++++ 4 files changed, 92 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 4b1514d13e72..d2f3a47b7541 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -237,7 +237,7 @@ public String resolveDateMathExpression(String dateExpression) { public String[] filteringAliases(ClusterState state, String index, String... expressions) { // expand the aliases wildcard List resolvedExpressions = expressions != null ? Arrays.asList(expressions) : Collections.emptyList(); - Context context = new Context(state, IndicesOptions.lenientExpandOpen()); + Context context = new Context(state, IndicesOptions.lenientExpandOpen(), true); for (ExpressionResolver expressionResolver : expressionResolvers) { resolvedExpressions = expressionResolver.resolve(context, resolvedExpressions); } @@ -459,17 +459,25 @@ final static class Context { private final ClusterState state; private final IndicesOptions options; private final long startTime; + private final boolean preserveAliases; Context(ClusterState state, IndicesOptions options) { - this.state = state; - this.options = options; - startTime = System.currentTimeMillis(); + this(state, options, System.currentTimeMillis()); + } + + Context(ClusterState state, IndicesOptions options, boolean preserveAliases) { + this(state, options, System.currentTimeMillis(), preserveAliases); } public Context(ClusterState state, IndicesOptions options, long startTime) { + this(state, options, startTime, false); + } + + public Context(ClusterState state, IndicesOptions options, long startTime, boolean preserveAliases) { this.state = state; this.options = options; this.startTime = startTime; + this.preserveAliases = preserveAliases; } public ClusterState getState() { @@ -483,6 +491,15 @@ public IndicesOptions getOptions() { public long getStartTime() { return startTime; } + + /** + * This is used to prevent resolving aliases to concrete indices but this also means + * that we might return aliases that point to a closed index. This is currently only used + * by {@link #filteringAliases(ClusterState, String, String...)} since it's the only one that needs aliases + */ + boolean isPreserveAliases() { + return preserveAliases; + } } private interface ExpressionResolver { @@ -531,6 +548,9 @@ public List resolve(Context context, List expressions) { } continue; } + if (Strings.isEmpty(expression)) { + throw new IndexNotFoundException(expression); + } boolean add = true; if (expression.charAt(0) == '+') { // if its the first, add empty result set @@ -612,21 +632,24 @@ public List resolve(Context context, List expressions) { .filter(e -> Regex.simpleMatch(pattern, e.getKey())) .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); } + Set expand = new HashSet<>(); for (Map.Entry entry : matches.entrySet()) { AliasOrIndex aliasOrIndex = entry.getValue(); - if (aliasOrIndex.isAlias() == false) { - AliasOrIndex.Index index = (AliasOrIndex.Index) aliasOrIndex; - if (excludeState != null && index.getIndex().getState() == excludeState) { - continue; - } - } - - if (add) { - result.add(entry.getKey()); + if (context.isPreserveAliases() && aliasOrIndex.isAlias()) { + expand.add(entry.getKey()); } else { - result.remove(entry.getKey()); + for (IndexMetaData meta : aliasOrIndex.getIndices()) { + if (excludeState == null || meta.getState() != excludeState) { + expand.add(meta.getIndex()); + } + } } } + if (add) { + result.addAll(expand); + } else { + result.removeAll(expand); + } if (matches.isEmpty() && options.allowNoIndices() == false) { IndexNotFoundException infe = new IndexNotFoundException(expression); diff --git a/core/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java b/core/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java index 7b8eb2ebc51e..d3b31221b67e 100644 --- a/core/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java +++ b/core/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java @@ -192,7 +192,7 @@ public void testIndexOptionsLenient() { context = new IndexNameExpressionResolver.Context(state, lenientExpand); results = indexNameExpressionResolver.concreteIndices(context, Strings.EMPTY_ARRAY); - assertEquals(4, results.length); + assertEquals(Arrays.toString(results), 4, results.length); context = new IndexNameExpressionResolver.Context(state, IndicesOptions.lenientExpandOpen()); results = indexNameExpressionResolver.concreteIndices(context, "foofoo*"); @@ -867,4 +867,37 @@ private MetaData metaDataBuilder(String... indices) { } return mdBuilder.build(); } + + public void testFilterClosedIndicesOnAliases() { + MetaData.Builder mdBuilder = MetaData.builder() + .put(indexBuilder("test-0").state(State.OPEN).putAlias(AliasMetaData.builder("alias-0"))) + .put(indexBuilder("test-1").state(IndexMetaData.State.CLOSE).putAlias(AliasMetaData.builder("alias-1"))); + ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); + + IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(state, IndicesOptions.lenientExpandOpen()); + String[] strings = indexNameExpressionResolver.concreteIndices(context, "alias-*"); + assertArrayEquals(new String[] {"test-0"}, strings); + + context = new IndexNameExpressionResolver.Context(state, IndicesOptions.strictExpandOpen()); + strings = indexNameExpressionResolver.concreteIndices(context, "alias-*"); + + assertArrayEquals(new String[] {"test-0"}, strings); + } + + public void testFilteringAliases() { + MetaData.Builder mdBuilder = MetaData.builder() + .put(indexBuilder("test-0").state(State.OPEN).putAlias(AliasMetaData.builder("alias-0").filter("{ \"term\": \"foo\"}"))) + .put(indexBuilder("test-1").state(State.OPEN).putAlias(AliasMetaData.builder("alias-1"))); + ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); + + String[] strings = indexNameExpressionResolver.filteringAliases(state, "test-0", "alias-*"); + assertArrayEquals(new String[] {"alias-0"}, strings); + + // concrete index supersedes filtering alias + strings = indexNameExpressionResolver.filteringAliases(state, "test-0", "test-0,alias-*"); + assertNull(strings); + + strings = indexNameExpressionResolver.filteringAliases(state, "test-0", "test-*,alias-*"); + assertNull(strings); + } } diff --git a/core/src/test/java/org/elasticsearch/cluster/metadata/WildcardExpressionResolverTests.java b/core/src/test/java/org/elasticsearch/cluster/metadata/WildcardExpressionResolverTests.java index 324086a04487..d9cf9f0d7909 100644 --- a/core/src/test/java/org/elasticsearch/cluster/metadata/WildcardExpressionResolverTests.java +++ b/core/src/test/java/org/elasticsearch/cluster/metadata/WildcardExpressionResolverTests.java @@ -59,7 +59,7 @@ public void testConvertWildcardsTests() { IndexNameExpressionResolver.WildcardExpressionResolver resolver = new IndexNameExpressionResolver.WildcardExpressionResolver(); IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(state, IndicesOptions.lenientExpandOpen()); - assertThat(newHashSet(resolver.resolve(context, Arrays.asList("testYY*", "alias*"))), equalTo(newHashSet("alias1", "alias2", "alias3", "testYYY"))); + assertThat(newHashSet(resolver.resolve(context, Arrays.asList("testYY*", "alias*"))), equalTo(newHashSet("testXXX", "testXYY", "testYYY"))); assertThat(newHashSet(resolver.resolve(context, Arrays.asList("-kuku"))), equalTo(newHashSet("testXXX", "testXYY", "testYYY"))); assertThat(newHashSet(resolver.resolve(context, Arrays.asList("+test*", "-testYYY"))), equalTo(newHashSet("testXXX", "testXYY"))); assertThat(newHashSet(resolver.resolve(context, Arrays.asList("+testX*", "+testYYY"))), equalTo(newHashSet("testXXX", "testXYY", "testYYY"))); diff --git a/core/src/test/java/org/elasticsearch/routing/AliasResolveRoutingIT.java b/core/src/test/java/org/elasticsearch/routing/AliasResolveRoutingIT.java index db21fef69307..e73f7e510fc3 100644 --- a/core/src/test/java/org/elasticsearch/routing/AliasResolveRoutingIT.java +++ b/core/src/test/java/org/elasticsearch/routing/AliasResolveRoutingIT.java @@ -19,6 +19,7 @@ package org.elasticsearch.routing; +import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.common.Priority; @@ -27,9 +28,12 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; +import java.util.concurrent.ExecutionException; import static org.elasticsearch.cluster.metadata.AliasAction.newAddAliasAction; import static org.elasticsearch.common.util.set.Sets.newHashSet; +import static org.elasticsearch.index.query.QueryBuilders.matchQuery; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.nullValue; @@ -37,6 +41,22 @@ * */ public class AliasResolveRoutingIT extends ESIntegTestCase { + + + // see https://github.com/elastic/elasticsearch/issues/13278 + public void testSearchClosedWildcardIndex() throws ExecutionException, InterruptedException { + createIndex("test-0"); + createIndex("test-1"); + ensureGreen(); + client().admin().indices().prepareAliases().addAlias("test-0", "alias-0").addAlias("test-1", "alias-1").get(); + client().admin().indices().prepareClose("test-1").get(); + indexRandom(true, client().prepareIndex("test-0", "type1", "1").setSource("field1", "the quick brown fox jumps"), + client().prepareIndex("test-0", "type1", "2").setSource("field1", "quick brown"), + client().prepareIndex("test-0", "type1", "3").setSource("field1", "quick")); + refresh("test-*"); + assertHitCount(client().prepareSearch().setIndices("alias-*").setIndicesOptions(IndicesOptions.lenientExpandOpen()).setQuery(matchQuery("_all", "quick")).get(), 3l); + } + public void testResolveIndexRouting() throws Exception { createIndex("test1"); createIndex("test2");