Skip to content

Commit 7dce829

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 1235716 commit 7dce829

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
@@ -307,7 +307,7 @@ public void testIndexOptionsWildcardExpansion() {
307307
assertEquals(1, results.length);
308308
assertEquals("bar", results[0]);
309309

310-
results = indexNameExpressionResolver.concreteIndexNames(context, "-foo*");
310+
results = indexNameExpressionResolver.concreteIndexNames(context, "*", "-foo*");
311311
assertEquals(1, results.length);
312312
assertEquals("bar", results[0]);
313313

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

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

‎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)