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
This commit is contained in:
Lee Hinman 2016-10-18 09:24:22 -06:00 committed by Luca Cavanna
parent 9bb8bd0578
commit c1721c6d79
3 changed files with 74 additions and 10 deletions

View File

@ -579,6 +579,7 @@ public class IndexNameExpressionResolver extends AbstractComponent {
private Set<String> innerResolve(Context context, List<String> expressions, IndicesOptions options, MetaData metaData) {
Set<String> 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<String> 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;
}

View File

@ -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
*/

View File

@ -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")));