From 67e1128e05b0ac75c65fa0721b694bcc65b35b3e Mon Sep 17 00:00:00 2001 From: Henning Andersen <33268011+henningandersen@users.noreply.github.com> Date: Wed, 18 Nov 2020 16:00:15 +0100 Subject: [PATCH] Fix remove alias with must_exist (#65141) Remove alias now parses the must_exist flag and results in a 404 (not found), if the index does not have the alias. Closes #62642 Relates #58100 Co-Authored-By: Luca Cavanna --- .../indices.update_aliases/40_must_exist.yml | 84 +++++++++++++++++++ .../indices/alias/IndicesAliasesRequest.java | 17 ++-- .../alias/TransportIndicesAliasesAction.java | 3 + .../cluster/metadata/AliasAction.java | 5 +- .../indices/alias/AliasActionsTests.java | 6 ++ .../MetadataIndexAliasesServiceTests.java | 3 +- .../alias/RandomAliasActionsGenerator.java | 5 ++ 7 files changed, 114 insertions(+), 9 deletions(-) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml new file mode 100644 index 0000000000000..61e8d2a36b7ac --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml @@ -0,0 +1,84 @@ +--- +"Remove alias with must_exist": + + - skip: + version: " - 7.99.99" # TODO: 7.10.99 + reason: "must_exist does not work until 7.11" + + - do: + indices.create: + index: test_index + + - do: + indices.update_aliases: + body: + actions: + - add: + index: test_index + aliases: test_alias1 + - do: + indices.exists_alias: + name: test_alias1 + - is_true: '' + + - do: + indices.update_aliases: + body: + actions: + - remove: + index: test_index + alias: test_alias1 + must_exist: true + - add: + index: test_index + alias: test_alias2 + - do: + indices.exists_alias: + name: test_alias1 + - is_false: '' + - do: + indices.exists_alias: + name: test_alias2 + - is_true: '' + + - do: + catch: missing + indices.update_aliases: + body: + actions: + - remove: + index: test_index + alias: test_alias1 + must_exist: true + - add: + index: test_index + alias: test_alias3 + - do: + indices.exists_alias: + name: test_alias3 + - is_false: '' + +--- +"Alias must_exist only on remove": + - do: + indices.create: + index: test_index + + - do: + catch: bad_request + indices.update_aliases: + body: + actions: + - add: + index: test_index + aliases: test_alias1 + must_exist: true + + - do: + catch: bad_request + indices.update_aliases: + body: + actions: + - remove_index: + index: test_index + must_exist: true diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java index b238ee105061c..a3ce108008494 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java @@ -177,6 +177,9 @@ private static ObjectParser parser(String name, Supplier ADD_PARSER = parser(ADD.getPreferredName(), AliasActions::add); + private static final ObjectParser REMOVE_PARSER = parser(REMOVE.getPreferredName(), AliasActions::remove); + private static final ObjectParser REMOVE_INDEX_PARSER = parser(REMOVE_INDEX.getPreferredName(), + AliasActions::removeIndex); static { ADD_PARSER.declareObject(AliasActions::filter, (parser, m) -> { try { @@ -191,11 +194,8 @@ private static ObjectParser parser(String name, Supplier REMOVE_PARSER = parser(REMOVE.getPreferredName(), AliasActions::remove); - private static final ObjectParser REMOVE_INDEX_PARSER = parser(REMOVE_INDEX.getPreferredName(), - AliasActions::removeIndex); /** * Parser for any one {@link AliasAction}. @@ -524,6 +524,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (null != isHidden) { builder.field(IS_HIDDEN.getPreferredName(), isHidden); } + if (null != mustExist) { + builder.field(MUST_EXIST.getPreferredName(), mustExist); + } builder.endObject(); builder.endObject(); return builder; @@ -544,6 +547,7 @@ public String toString() { + ",indexRouting=" + indexRouting + ",searchRouting=" + searchRouting + ",writeIndex=" + writeIndex + + ",mustExist=" + mustExist + "]"; } @@ -562,12 +566,13 @@ public boolean equals(Object obj) { && Objects.equals(indexRouting, other.indexRouting) && Objects.equals(searchRouting, other.searchRouting) && Objects.equals(writeIndex, other.writeIndex) - && Objects.equals(isHidden, other.isHidden); + && Objects.equals(isHidden, other.isHidden) + && Objects.equals(mustExist, other.mustExist); } @Override public int hashCode() { - return Objects.hash(type, indices, aliases, filter, routing, indexRouting, searchRouting, writeIndex, isHidden); + return Objects.hash(type, indices, aliases, filter, routing, indexRouting, searchRouting, writeIndex, isHidden, mustExist); } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java index aed7cb66cd6da..6fd1aab5fcf8d 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java @@ -171,6 +171,9 @@ private static String[] concreteAliases(AliasActions action, Metadata metadata, finalAliases.add(aliasMeta.alias()); } } + if (finalAliases.isEmpty() && action.mustExist() != null && action.mustExist()) { + return action.aliases(); + } return finalAliases.toArray(new String[finalAliases.size()]); } else { //for ADD and REMOVE_INDEX we just return the current aliases diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java b/server/src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java index e95a96909af88..03e8ec9a683a0 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java @@ -19,6 +19,7 @@ package org.elasticsearch.cluster.metadata; +import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; @@ -179,8 +180,8 @@ boolean removeIndex() { @Override boolean apply(NewAliasValidator aliasValidator, Metadata.Builder metadata, IndexMetadata index) { if (false == index.getAliases().containsKey(alias)) { - if (mustExist != null && mustExist.booleanValue()) { - throw new IllegalArgumentException("required alias [" + alias + "] does not exist"); + if (mustExist != null && mustExist) { + throw new ResourceNotFoundException("required alias [" + alias + "] does not exist"); } return false; } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/alias/AliasActionsTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/alias/AliasActionsTests.java index 55f4e7edfafd2..26248abc74f6b 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/alias/AliasActionsTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/alias/AliasActionsTests.java @@ -216,6 +216,7 @@ public void testParseAddDefaultRouting() throws IOException { public void testParseRemove() throws IOException { String[] indices = generateRandomStringArray(10, 5, false, false); String[] aliases = generateRandomStringArray(10, 5, false, false); + Boolean mustExist = null; XContentBuilder b = XContentBuilder.builder(randomFrom(XContentType.values()).xContent()); b.startObject(); { @@ -231,6 +232,10 @@ public void testParseRemove() throws IOException { } else { b.field("alias", aliases[0]); } + if (randomBoolean()) { + mustExist = randomBoolean(); + b.field("must_exist", mustExist); + } } b.endObject(); } @@ -241,6 +246,7 @@ public void testParseRemove() throws IOException { assertEquals(AliasActions.Type.REMOVE, action.actionType()); assertThat(action.indices(), equalTo(indices)); assertThat(action.aliases(), equalTo(aliases)); + assertThat(action.mustExist(), equalTo(mustExist)); } } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java index 246a2eeac86d0..e2652ac5b5474 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.cluster.metadata; +import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.Version; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; @@ -138,7 +139,7 @@ public void testMustExist() { // Show that removing non-existing alias with mustExist == true fails final ClusterState finalCS = after; - final IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, + final ResourceNotFoundException iae = expectThrows(ResourceNotFoundException.class, () -> service.applyAliasActions(finalCS, singletonList(new AliasAction.Remove(index, "test_2", true)))); assertThat(iae.getMessage(), containsString("required alias [test_2] does not exist")); } diff --git a/test/framework/src/main/java/org/elasticsearch/index/alias/RandomAliasActionsGenerator.java b/test/framework/src/main/java/org/elasticsearch/index/alias/RandomAliasActionsGenerator.java index 31c93cbc1ddd7..bdc8f91f21616 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/alias/RandomAliasActionsGenerator.java +++ b/test/framework/src/main/java/org/elasticsearch/index/alias/RandomAliasActionsGenerator.java @@ -54,6 +54,11 @@ public static AliasActions randomAliasAction(boolean useStringAsFilter) { } action.indices(indices); } + if (action.actionType() == AliasActions.Type.REMOVE) { + if (randomBoolean()) { + action.mustExist(randomBoolean()); + } + } if (action.actionType() != AliasActions.Type.REMOVE_INDEX) { if (randomBoolean()) { action.alias(randomAlphaOfLength(5));