From c1721c6d7945fa6019b587c31c77d0d1aff36c6c Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Tue, 18 Oct 2016 09:24:22 -0600 Subject: [PATCH] Only negate index expression on all indices with preceding wildcard (#20898) * Only negate index expression on all indices with preceding wildcard There is currently a very confusing behavior in Elasticsearch for the following: Given the indices: `[test1, test2, -foo1, -foo2]` ``` DELETE /-foo* ``` Will cause the `test1` and `test2` indices to be deleted, when what is usually intended is to delete the `-foo1` and `-foo2` indices. Previously we added a change in #20033 to disallow creating indices starting with `-` or `+`, which will help with this situation. However, users may have existing indices starting with these characters. This changes the negation to only take effect in a wildcard (`*`) has been seen somewhere in the expression, so in order to delete `-foo1` and `-foo2` the following now works: ``` DELETE /-foo* ``` As well as: ``` DELETE /-foo1,-foo2 ``` so in order to actually delete everything except for the "foo" indices (ie, `test1` and `test2`) a user would now issue: ``` DELETE /*,--foo* ``` Relates to #19800 --- .../metadata/IndexNameExpressionResolver.java | 18 ++++-- .../IndexNameExpressionResolverTests.java | 60 ++++++++++++++++++- .../WildcardExpressionResolverTests.java | 6 +- 3 files changed, 74 insertions(+), 10 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 198d37971c6..377409dd86b 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -579,6 +579,7 @@ public class IndexNameExpressionResolver extends AbstractComponent { private Set innerResolve(Context context, List expressions, IndicesOptions options, MetaData metaData) { Set result = null; + boolean wildcardSeen = false; for (int i = 0; i < expressions.size(); i++) { String expression = expressions.get(i); if (aliasOrIndexExists(metaData, expression)) { @@ -598,13 +599,14 @@ public class IndexNameExpressionResolver extends AbstractComponent { } expression = expression.substring(1); } else if (expression.charAt(0) == '-') { - // if its the first, fill it with all the indices... - if (i == 0) { - List concreteIndices = resolveEmptyOrTrivialWildcard(options, metaData, false); - result = new HashSet<>(concreteIndices); + // if there is a negation without a wildcard being previously seen, add it verbatim, + // otherwise return the expression + if (wildcardSeen) { + add = false; + expression = expression.substring(1); + } else { + add = true; } - add = false; - expression = expression.substring(1); } if (result == null) { // add all the previous ones... @@ -634,6 +636,10 @@ public class IndexNameExpressionResolver extends AbstractComponent { if (!noIndicesAllowedOrMatches(options, matches)) { throw infe(expression); } + + if (Regex.isSimpleMatchPattern(expression)) { + wildcardSeen = true; + } } return result; } 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 63b9dc05e7a..b68f3735c0a 100644 --- a/core/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java +++ b/core/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java @@ -305,7 +305,7 @@ public class IndexNameExpressionResolverTests extends ESTestCase { assertEquals(1, results.length); assertEquals("bar", results[0]); - results = indexNameExpressionResolver.concreteIndexNames(context, "-foo*"); + results = indexNameExpressionResolver.concreteIndexNames(context, "*", "-foo*"); assertEquals(1, results.length); assertEquals("bar", results[0]); @@ -585,6 +585,64 @@ public class IndexNameExpressionResolverTests extends ESTestCase { assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "testX*")), equalTo(newHashSet("testXXX", "testXXY", "testXYY"))); } + public void testConcreteIndicesWildcardWithNegation() { + MetaData.Builder mdBuilder = MetaData.builder() + .put(indexBuilder("testXXX").state(State.OPEN)) + .put(indexBuilder("testXXY").state(State.OPEN)) + .put(indexBuilder("testXYY").state(State.OPEN)) + .put(indexBuilder("-testXYZ").state(State.OPEN)) + .put(indexBuilder("-testXZZ").state(State.OPEN)) + .put(indexBuilder("-testYYY").state(State.OPEN)) + .put(indexBuilder("testYYY").state(State.OPEN)) + .put(indexBuilder("testYYX").state(State.OPEN)); + ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build(); + + IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(state, + IndicesOptions.fromOptions(true, true, true, true)); + assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "testX*")), + equalTo(newHashSet("testXXX", "testXXY", "testXYY"))); + + assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "test*", "-testX*")), + equalTo(newHashSet("testYYY", "testYYX"))); + + assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "-testX*")), + equalTo(newHashSet("-testXYZ", "-testXZZ"))); + + assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "testXXY", "-testX*")), + equalTo(newHashSet("testXXY", "-testXYZ", "-testXZZ"))); + + assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "*", "--testX*")), + equalTo(newHashSet("testXXX", "testXXY", "testXYY", "testYYX", "testYYY", "-testYYY"))); + + assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "-testXXX", "test*")), + equalTo(newHashSet("testYYX", "testXXX", "testXYY", "testYYY", "testXXY"))); + + assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "test*", "-testXXX")), + equalTo(newHashSet("testYYX", "testXYY", "testYYY", "testXXY"))); + + assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "+testXXX", "+testXXY", "+testYYY", "-testYYY")), + equalTo(newHashSet("testXXX", "testXXY", "testYYY", "-testYYY"))); + + assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "testYYY", "testYYX", "testX*", "-testXXX")), + equalTo(newHashSet("testYYY", "testYYX", "testXXY", "testXYY"))); + + assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "-testXXX", "*testY*", "-testYYY")), + equalTo(newHashSet("testYYX", "testYYY", "-testYYY"))); + + String[] indexNames = indexNameExpressionResolver.concreteIndexNames(state, IndicesOptions.lenientExpandOpen(), "-doesnotexist"); + assertEquals(0, indexNames.length); + + assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(state, IndicesOptions.lenientExpandOpen(), "-*")), + equalTo(newHashSet("-testXYZ", "-testXZZ", "-testYYY"))); + + assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(state, IndicesOptions.lenientExpandOpen(), + "+testXXX", "+testXXY", "+testXYY", "-testXXY")), + equalTo(newHashSet("testXXX", "testXYY", "testXXY"))); + + indexNames = indexNameExpressionResolver.concreteIndexNames(state, IndicesOptions.lenientExpandOpen(), "*", "-*"); + assertEquals(0, indexNames.length); + } + /** * test resolving _all pattern (null, empty array or "_all") for random IndicesOptions */ 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 01110e796e8..bac9a681341 100644 --- a/core/src/test/java/org/elasticsearch/cluster/metadata/WildcardExpressionResolverTests.java +++ b/core/src/test/java/org/elasticsearch/cluster/metadata/WildcardExpressionResolverTests.java @@ -50,9 +50,9 @@ public class WildcardExpressionResolverTests extends ESTestCase { assertThat(newHashSet(resolver.resolve(context, Arrays.asList("*"))), equalTo(newHashSet("testXXX", "testXYY", "testYYY", "kuku"))); assertThat(newHashSet(resolver.resolve(context, Arrays.asList("*", "-kuku"))), equalTo(newHashSet("testXXX", "testXYY", "testYYY"))); assertThat(newHashSet(resolver.resolve(context, Arrays.asList("testXXX", "+testYYY"))), equalTo(newHashSet("testXXX", "testYYY"))); - assertThat(newHashSet(resolver.resolve(context, Arrays.asList("testXXX", "-testXXX"))).size(), equalTo(0)); + assertThat(newHashSet(resolver.resolve(context, Arrays.asList("testXXX", "-testXXX"))), equalTo(newHashSet("testXXX", "-testXXX"))); assertThat(newHashSet(resolver.resolve(context, Arrays.asList("testXXX", "+testY*"))), equalTo(newHashSet("testXXX", "testYYY"))); - assertThat(newHashSet(resolver.resolve(context, Arrays.asList("testXXX", "-testX*"))).size(), equalTo(0)); + assertThat(newHashSet(resolver.resolve(context, Arrays.asList("testXXX", "-testX*"))), equalTo(newHashSet("testXXX"))); } public void testConvertWildcardsTests() { @@ -66,7 +66,7 @@ public class WildcardExpressionResolverTests extends ESTestCase { IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(state, IndicesOptions.lenientExpandOpen()); 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("-kuku"))), equalTo(newHashSet("-kuku"))); 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"))); assertThat(newHashSet(resolver.resolve(context, Arrays.asList("+testYYY", "+testX*"))), equalTo(newHashSet("testXXX", "testXYY", "testYYY")));