Aliases: Throw exception if index is null or missing when creating an alias
Fixes a bug where alias creation would allow `null` for index name, which thereby applied the alias to _all_ indices. This patch makes the validator throw an exception if the index is null. ```bash POST /_aliases { "actions": [ { "add": { "alias": "empty-alias", "index": null } } ] } ``` ```json { "error": "ActionRequestValidationException[Validation Failed: 1: Alias action [add]: [index] may not be null;]", "status": 400 } ``` The reason this bug wasn't caught by the existing tests is because the old test for nullness only validated against a cluster which had zero indices. The null index is translated into "_all", and since there are no indices, this fails because the index doesn't exist. So the test passes. However, as soon as you add an index, "_all" resolves and you get the situation described in the original bug report: null index is accepted by the alias, resolves to "_all" and gets applied to everything. The REST tests, otoh, explicitly tested this bug as a real feature and therefore passed. The REST tests were modified to change this behavior. Fixes #7863
This commit is contained in:
parent
0be5c60bce
commit
f5b2dfd052
|
@ -183,7 +183,7 @@ An alias can also be added with the endpoint
|
|||
where
|
||||
|
||||
[horizontal]
|
||||
`index`:: The index the alias refers to. Can be any of `blank | * | _all | glob pattern | name1, name2, …`
|
||||
`index`:: The index the alias refers to. Can be any of `* | _all | glob pattern | name1, name2, …`
|
||||
`name`:: The name of the alias. This is a required option.
|
||||
`routing`:: An optional routing that can be associated with an alias.
|
||||
`filter`:: An optional filter that can be associated with an alias.
|
||||
|
|
|
@ -16,11 +16,17 @@ setup:
|
|||
- do:
|
||||
indices.put_alias:
|
||||
name: alias1
|
||||
index:
|
||||
- test_index1
|
||||
- foo
|
||||
body:
|
||||
routing: "routing value"
|
||||
- do:
|
||||
indices.put_alias:
|
||||
name: alias2
|
||||
index:
|
||||
- test_index2
|
||||
- foo
|
||||
body:
|
||||
routing: "routing value"
|
||||
|
||||
|
@ -31,14 +37,12 @@ setup:
|
|||
name: alias1
|
||||
|
||||
- match: {test_index1.aliases.alias1.search_routing: "routing value"}
|
||||
- match: {test_index2.aliases.alias1.search_routing: "routing value"}
|
||||
- match: {foo.aliases.alias1.search_routing: "routing value"}
|
||||
|
||||
- do:
|
||||
indices.get_alias:
|
||||
name: alias2
|
||||
|
||||
- match: {test_index1.aliases.alias2.search_routing: "routing value"}
|
||||
- match: {test_index2.aliases.alias2.search_routing: "routing value"}
|
||||
- match: {foo.aliases.alias2.search_routing: "routing value"}
|
||||
|
||||
|
@ -57,7 +61,6 @@ setup:
|
|||
indices.get_alias:
|
||||
name: alias2
|
||||
|
||||
- match: {test_index1.aliases.alias2.search_routing: "routing value"}
|
||||
- match: {test_index2.aliases.alias2.search_routing: "routing value"}
|
||||
- match: {foo.aliases.alias2.search_routing: "routing value"}
|
||||
|
||||
|
@ -76,7 +79,6 @@ setup:
|
|||
indices.get_alias:
|
||||
name: alias2
|
||||
|
||||
- match: {test_index1.aliases.alias2.search_routing: "routing value"}
|
||||
- match: {test_index2.aliases.alias2.search_routing: "routing value"}
|
||||
- match: {foo.aliases.alias2.search_routing: "routing value"}
|
||||
|
||||
|
@ -99,7 +101,6 @@ setup:
|
|||
indices.get_alias:
|
||||
name: alias2
|
||||
|
||||
- match: {test_index1.aliases.alias2.search_routing: "routing value"}
|
||||
- match: {test_index2.aliases.alias2.search_routing: "routing value"}
|
||||
- match: {foo.aliases.alias2.search_routing: "routing value"}
|
||||
|
||||
|
@ -122,7 +123,6 @@ setup:
|
|||
indices.get_alias:
|
||||
name: alias2
|
||||
|
||||
- match: {test_index1.aliases.alias2.search_routing: "routing value"}
|
||||
- match: {test_index2.aliases.alias2.search_routing: "routing value"}
|
||||
- match: {foo.aliases.alias2.search_routing: "routing value"}
|
||||
|
||||
|
@ -192,7 +192,6 @@ setup:
|
|||
indices.get_alias:
|
||||
name: alias2
|
||||
|
||||
- match: {test_index1.aliases.alias2.search_routing: "routing value"}
|
||||
- match: {test_index2.aliases.alias2.search_routing: "routing value"}
|
||||
- match: {foo.aliases.alias2.search_routing: "routing value"}
|
||||
|
||||
|
|
|
@ -106,16 +106,10 @@ setup:
|
|||
|
||||
|
||||
- do:
|
||||
catch: param
|
||||
indices.put_alias:
|
||||
name: alias
|
||||
|
||||
- do:
|
||||
indices.get_alias:
|
||||
name: alias
|
||||
|
||||
- match: {test_index1.aliases.alias: {}}
|
||||
- match: {test_index2.aliases.alias: {}}
|
||||
- match: {foo.aliases.alias: {}}
|
||||
|
||||
---
|
||||
"put alias with missing name":
|
||||
|
|
|
@ -294,10 +294,6 @@ public class IndicesAliasesRequest extends AcknowledgedRequest<IndicesAliasesReq
|
|||
+ "]: [alias] may not be empty string", validationException);
|
||||
}
|
||||
}
|
||||
if (CollectionUtils.isEmpty(aliasAction.indices)) {
|
||||
validationException = addValidationError("Alias action [" + aliasAction.actionType().name().toLowerCase(Locale.ENGLISH)
|
||||
+ "]: indices may not be empty", validationException);
|
||||
}
|
||||
}
|
||||
if (!CollectionUtils.isEmpty(aliasAction.indices)) {
|
||||
for (String index : aliasAction.indices) {
|
||||
|
@ -306,6 +302,9 @@ public class IndicesAliasesRequest extends AcknowledgedRequest<IndicesAliasesReq
|
|||
+ "]: [index] may not be empty string", validationException);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
validationException = addValidationError("Alias action [" + aliasAction.actionType().name().toLowerCase(Locale.ENGLISH)
|
||||
+ "]: Property [index] was either missing or null", validationException);
|
||||
}
|
||||
}
|
||||
return validationException;
|
||||
|
|
|
@ -754,9 +754,35 @@ public class IndexAliasesTests extends ElasticsearchIntegrationTest {
|
|||
assertThat(existsResponse.exists(), equalTo(false));
|
||||
}
|
||||
|
||||
@Test(expected = IndexMissingException.class)
|
||||
public void testAddAliasNullIndex() {
|
||||
admin().indices().prepareAliases().addAliasAction(AliasAction.newAddAliasAction(null, "alias1")).get();
|
||||
@Test
|
||||
public void testAddAliasNullWithoutExistingIndices() {
|
||||
try {
|
||||
assertAcked(admin().indices().prepareAliases().addAliasAction(AliasAction.newAddAliasAction(null, "alias1")));
|
||||
fail("create alias should have failed due to null index");
|
||||
} catch (ElasticsearchIllegalArgumentException e) {
|
||||
assertThat("Exception text does not contain \"Property [index] was either missing or null\"",
|
||||
e.getMessage().contains("Property [index] was either missing or null"),
|
||||
equalTo(true));
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testAddAliasNullWithExistingIndices() throws Exception {
|
||||
logger.info("--> creating index [test]");
|
||||
createIndex("test");
|
||||
ensureGreen();
|
||||
|
||||
logger.info("--> aliasing index [null] with [empty-alias]");
|
||||
|
||||
try {
|
||||
assertAcked(admin().indices().prepareAliases().addAlias((String) null, "empty-alias"));
|
||||
fail("create alias should have failed due to null index");
|
||||
} catch (ElasticsearchIllegalArgumentException e) {
|
||||
assertThat("Exception text does not contain \"Property [index] was either missing or null\"",
|
||||
e.getMessage().contains("Property [index] was either missing or null"),
|
||||
equalTo(true));
|
||||
}
|
||||
}
|
||||
|
||||
@Test(expected = ActionRequestValidationException.class)
|
||||
|
@ -781,7 +807,7 @@ public class IndexAliasesTests extends ElasticsearchIntegrationTest {
|
|||
assertTrue("Should throw " + ActionRequestValidationException.class.getSimpleName(), false);
|
||||
} catch (ActionRequestValidationException e) {
|
||||
assertThat(e.validationErrors(), notNullValue());
|
||||
assertThat(e.validationErrors().size(), equalTo(1));
|
||||
assertThat(e.validationErrors().size(), equalTo(2));
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -938,7 +964,7 @@ public class IndexAliasesTests extends ElasticsearchIntegrationTest {
|
|||
.addAlias("test", "a", FilterBuilders.matchAllFilter()) // <-- no fail, b/c no field mentioned
|
||||
.get();
|
||||
}
|
||||
|
||||
|
||||
private void checkAliases() {
|
||||
GetAliasesResponse getAliasesResponse = admin().indices().prepareGetAliases("alias1").get();
|
||||
assertThat(getAliasesResponse.getAliases().get("test").size(), equalTo(1));
|
||||
|
|
Loading…
Reference in New Issue