Skip to content

Commit c1721c6

Browse files
dakronejavanna
dakrone
authored and
javanna
committed
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
1 parent 9bb8bd0 commit c1721c6

File tree

3 files changed

+74
-10
lines changed

3 files changed

+74
-10
lines changed

‎core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java

+12-6
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,7 @@ public List<String> resolve(Context context, List<String> expressions) {
579579

580580
private Set<String> innerResolve(Context context, List<String> expressions, IndicesOptions options, MetaData metaData) {
581581
Set<String> result = null;
582+
boolean wildcardSeen = false;
582583
for (int i = 0; i < expressions.size(); i++) {
583584
String expression = expressions.get(i);
584585
if (aliasOrIndexExists(metaData, expression)) {
@@ -598,13 +599,14 @@ private Set<String> innerResolve(Context context, List<String> expressions, Indi
598599
}
599600
expression = expression.substring(1);
600601
} else if (expression.charAt(0) == '-') {
601-
// if its the first, fill it with all the indices...
602-
if (i == 0) {
603-
List<String> concreteIndices = resolveEmptyOrTrivialWildcard(options, metaData, false);
604-
result = new HashSet<>(concreteIndices);
602+
// if there is a negation without a wildcard being previously seen, add it verbatim,
603+
// otherwise return the expression
604+
if (wildcardSeen) {
605+
add = false;
606+
expression = expression.substring(1);
607+
} else {
608+
add = true;
605609
}
606-
add = false;
607-
expression = expression.substring(1);
608610
}
609611
if (result == null) {
610612
// add all the previous ones...
@@ -634,6 +636,10 @@ private Set<String> innerResolve(Context context, List<String> expressions, Indi
634636
if (!noIndicesAllowedOrMatches(options, matches)) {
635637
throw infe(expression);
636638
}
639+
640+
if (Regex.isSimpleMatchPattern(expression)) {
641+
wildcardSeen = true;
642+
}
637643
}
638644
return result;
639645
}

‎core/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java

+59-1
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ public void testIndexOptionsWildcardExpansion() {
305305
assertEquals(1, results.length);
306306
assertEquals("bar", results[0]);
307307

308-
results = indexNameExpressionResolver.concreteIndexNames(context, "-foo*");
308+
results = indexNameExpressionResolver.concreteIndexNames(context, "*", "-foo*");
309309
assertEquals(1, results.length);
310310
assertEquals("bar", results[0]);
311311

@@ -585,6 +585,64 @@ public void testConcreteIndicesWildcardExpansion() {
585585
assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "testX*")), equalTo(newHashSet("testXXX", "testXXY", "testXYY")));
586586
}
587587

588+
public void testConcreteIndicesWildcardWithNegation() {
589+
MetaData.Builder mdBuilder = MetaData.builder()
590+
.put(indexBuilder("testXXX").state(State.OPEN))
591+
.put(indexBuilder("testXXY").state(State.OPEN))
592+
.put(indexBuilder("testXYY").state(State.OPEN))
593+
.put(indexBuilder("-testXYZ").state(State.OPEN))
594+
.put(indexBuilder("-testXZZ").state(State.OPEN))
595+
.put(indexBuilder("-testYYY").state(State.OPEN))
596+
.put(indexBuilder("testYYY").state(State.OPEN))
597+
.put(indexBuilder("testYYX").state(State.OPEN));
598+
ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build();
599+
600+
IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(state,
601+
IndicesOptions.fromOptions(true, true, true, true));
602+
assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "testX*")),
603+
equalTo(newHashSet("testXXX", "testXXY", "testXYY")));
604+
605+
assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "test*", "-testX*")),
606+
equalTo(newHashSet("testYYY", "testYYX")));
607+
608+
assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "-testX*")),
609+
equalTo(newHashSet("-testXYZ", "-testXZZ")));
610+
611+
assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "testXXY", "-testX*")),
612+
equalTo(newHashSet("testXXY", "-testXYZ", "-testXZZ")));
613+
614+
assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "*", "--testX*")),
615+
equalTo(newHashSet("testXXX", "testXXY", "testXYY", "testYYX", "testYYY", "-testYYY")));
616+
617+
assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "-testXXX", "test*")),
618+
equalTo(newHashSet("testYYX", "testXXX", "testXYY", "testYYY", "testXXY")));
619+
620+
assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "test*", "-testXXX")),
621+
equalTo(newHashSet("testYYX", "testXYY", "testYYY", "testXXY")));
622+
623+
assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "+testXXX", "+testXXY", "+testYYY", "-testYYY")),
624+
equalTo(newHashSet("testXXX", "testXXY", "testYYY", "-testYYY")));
625+
626+
assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "testYYY", "testYYX", "testX*", "-testXXX")),
627+
equalTo(newHashSet("testYYY", "testYYX", "testXXY", "testXYY")));
628+
629+
assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "-testXXX", "*testY*", "-testYYY")),
630+
equalTo(newHashSet("testYYX", "testYYY", "-testYYY")));
631+
632+
String[] indexNames = indexNameExpressionResolver.concreteIndexNames(state, IndicesOptions.lenientExpandOpen(), "-doesnotexist");
633+
assertEquals(0, indexNames.length);
634+
635+
assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(state, IndicesOptions.lenientExpandOpen(), "-*")),
636+
equalTo(newHashSet("-testXYZ", "-testXZZ", "-testYYY")));
637+
638+
assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(state, IndicesOptions.lenientExpandOpen(),
639+
"+testXXX", "+testXXY", "+testXYY", "-testXXY")),
640+
equalTo(newHashSet("testXXX", "testXYY", "testXXY")));
641+
642+
indexNames = indexNameExpressionResolver.concreteIndexNames(state, IndicesOptions.lenientExpandOpen(), "*", "-*");
643+
assertEquals(0, indexNames.length);
644+
}
645+
588646
/**
589647
* test resolving _all pattern (null, empty array or "_all") for random IndicesOptions
590648
*/

‎core/src/test/java/org/elasticsearch/cluster/metadata/WildcardExpressionResolverTests.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ public void testConvertWildcardsJustIndicesTests() {
5050
assertThat(newHashSet(resolver.resolve(context, Arrays.asList("*"))), equalTo(newHashSet("testXXX", "testXYY", "testYYY", "kuku")));
5151
assertThat(newHashSet(resolver.resolve(context, Arrays.asList("*", "-kuku"))), equalTo(newHashSet("testXXX", "testXYY", "testYYY")));
5252
assertThat(newHashSet(resolver.resolve(context, Arrays.asList("testXXX", "+testYYY"))), equalTo(newHashSet("testXXX", "testYYY")));
53-
assertThat(newHashSet(resolver.resolve(context, Arrays.asList("testXXX", "-testXXX"))).size(), equalTo(0));
53+
assertThat(newHashSet(resolver.resolve(context, Arrays.asList("testXXX", "-testXXX"))), equalTo(newHashSet("testXXX", "-testXXX")));
5454
assertThat(newHashSet(resolver.resolve(context, Arrays.asList("testXXX", "+testY*"))), equalTo(newHashSet("testXXX", "testYYY")));
55-
assertThat(newHashSet(resolver.resolve(context, Arrays.asList("testXXX", "-testX*"))).size(), equalTo(0));
55+
assertThat(newHashSet(resolver.resolve(context, Arrays.asList("testXXX", "-testX*"))), equalTo(newHashSet("testXXX")));
5656
}
5757

5858
public void testConvertWildcardsTests() {
@@ -66,7 +66,7 @@ public void testConvertWildcardsTests() {
6666

6767
IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(state, IndicesOptions.lenientExpandOpen());
6868
assertThat(newHashSet(resolver.resolve(context, Arrays.asList("testYY*", "alias*"))), equalTo(newHashSet("testXXX", "testXYY", "testYYY")));
69-
assertThat(newHashSet(resolver.resolve(context, Arrays.asList("-kuku"))), equalTo(newHashSet("testXXX", "testXYY", "testYYY")));
69+
assertThat(newHashSet(resolver.resolve(context, Arrays.asList("-kuku"))), equalTo(newHashSet("-kuku")));
7070
assertThat(newHashSet(resolver.resolve(context, Arrays.asList("+test*", "-testYYY"))), equalTo(newHashSet("testXXX", "testXYY")));
7171
assertThat(newHashSet(resolver.resolve(context, Arrays.asList("+testX*", "+testYYY"))), equalTo(newHashSet("testXXX", "testXYY", "testYYY")));
7272
assertThat(newHashSet(resolver.resolve(context, Arrays.asList("+testYYY", "+testX*"))), equalTo(newHashSet("testXXX", "testXYY", "testYYY")));

0 commit comments

Comments
 (0)