Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add check for invalid index in WildcardExpressionResolver #26409

Merged
merged 14 commits into from
Sep 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.elasticsearch.index.Index;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.indices.IndexClosedException;
import org.elasticsearch.indices.InvalidIndexNameException;
import org.joda.time.DateTimeZone;
import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;
Expand Down Expand Up @@ -601,6 +602,7 @@ private Set<String> innerResolve(Context context, List<String> expressions, Indi
if (Strings.isEmpty(expression)) {
throw indexNotFoundException(expression);
}
validateAliasOrIndex(expression);
if (aliasOrIndexExists(options, metaData, expression)) {
if (result != null) {
result.add(expression);
Expand Down Expand Up @@ -654,6 +656,16 @@ private Set<String> innerResolve(Context context, List<String> expressions, Indi
return result;
}

private static void validateAliasOrIndex(String expression) {
// Expressions can not start with an underscore. This is reserved for APIs. If the check gets here, the API
// does not exist and the path is interpreted as an expression. If the expression begins with an underscore,
// throw a specific error that is different from the [[IndexNotFoundException]], which is typically thrown
// if the expression can't be found.
if (expression.charAt(0) == '_') {
throw new InvalidIndexNameException(expression, "must not start with '_'.");
}
}

private static boolean aliasOrIndexExists(IndicesOptions options, MetaData metaData, String expression) {
AliasOrIndex aliasOrIndex = metaData.getAliasAndIndexLookup().get(expression);
//treat aliases as unavailable indices when ignoreAliases is set to true (e.g. delete index and update aliases api)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.indices.IndexClosedException;
import org.elasticsearch.indices.InvalidIndexNameException;
import org.elasticsearch.test.ESTestCase;

import java.util.Arrays;
Expand Down Expand Up @@ -641,7 +642,7 @@ public void testConcreteIndicesWildcardAndAliases() {
// when ignoreAliases option is set, concreteIndexNames resolves the provided expressions
// only against the defined indices
IndicesOptions ignoreAliasesOptions = IndicesOptions.fromOptions(false, false, true, false, true, false, true);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unneeded change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should i remove this? my intellij settings might be a bit aggressive :)

String[] indexNamesIndexWildcard = indexNameExpressionResolver.concreteIndexNames(state, ignoreAliasesOptions, "foo*");

assertEquals(1, indexNamesIndexWildcard.length);
Expand Down Expand Up @@ -1126,4 +1127,14 @@ public void testIndicesAliasesRequestIgnoresAliases() {
assertEquals("test-index", indices[0]);
}
}

public void testInvalidIndex() {
MetaData.Builder mdBuilder = MetaData.builder().put(indexBuilder("test"));
ClusterState state = ClusterState.builder(new ClusterName("_name")).metaData(mdBuilder).build();
IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(state, IndicesOptions.lenientExpandOpen());

InvalidIndexNameException iine = expectThrows(InvalidIndexNameException.class,
() -> indexNameExpressionResolver.concreteIndexNames(context, "_foo"));
assertEquals("Invalid index name [_foo], must not start with '_'.", iine.getMessage());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,11 @@ setup:
- is_true: test_index_2.settings
- is_true: test_index_3.settings

---
"Should return an exception when querying invalid indices":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test needs to assert on the HTTP status.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I was unaware we could do that. I must've missed it in the docs!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might have to add a specific catch for bad_request (400).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ill block this on it being merged


- do:
catch: bad_request
indices.get:
index: _foo