From b305454e23528094d7abdf5287f2309752a18b75 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Thu, 18 Jun 2020 10:05:14 +0200 Subject: [PATCH] Add new flag to check whether alias exists on remove (#58100) This allows doing true CAS operations on aliases, making sure that an alias is actually properly moved from a given source index onto a given target index. This is useful to ensure that an alias is actually moved from a given index to another one, and not just added to another index. --- docs/reference/indices/aliases.asciidoc | 4 ++ .../indices/alias/IndicesAliasesRequest.java | 23 ++++++++++ .../alias/TransportIndicesAliasesAction.java | 2 +- .../rollover/MetadataRolloverService.java | 2 +- .../cluster/metadata/AliasAction.java | 8 +++- .../indices/alias/AliasActionsTests.java | 11 +++++ .../MetadataIndexAliasesServiceTests.java | 46 +++++++++++++++++-- 7 files changed, 90 insertions(+), 6 deletions(-) diff --git a/docs/reference/indices/aliases.asciidoc b/docs/reference/indices/aliases.asciidoc index 5cb819ab1000a..363592eba4f9e 100644 --- a/docs/reference/indices/aliases.asciidoc +++ b/docs/reference/indices/aliases.asciidoc @@ -113,6 +113,10 @@ unless overriden in the request using the `expand_wildcards` parameter, similar to <>. This property must be set to the same value on all indices that share an alias. Defaults to `false`. +`must_exist`:: +(Optional, boolean) +If `true`, the alias to remove must exist. Defaults to `false`. + `is_write_index`:: (Optional, boolean) If `true`, assigns the index as an alias's write index. 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 8c77bfa24ebbf..be71c2ff8dc09 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 @@ -98,6 +98,7 @@ public static class AliasActions implements AliasesRequest, Writeable, ToXConten private static final ParseField SEARCH_ROUTING = new ParseField("search_routing", "searchRouting", "search-routing"); private static final ParseField IS_WRITE_INDEX = new ParseField("is_write_index"); private static final ParseField IS_HIDDEN = new ParseField("is_hidden"); + private static final ParseField MUST_EXIST = new ParseField("must_exist"); private static final ParseField ADD = new ParseField("add"); private static final ParseField REMOVE = new ParseField("remove"); @@ -195,6 +196,7 @@ 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(), @@ -234,6 +236,7 @@ private static ObjectParser parser(String name, Supplier rolloverAliasToNewIndex(String oldIndex, String newInde } else { return List.of( new AliasAction.Add(newIndex, alias, null, null, null, null, isHidden), - new AliasAction.Remove(oldIndex, alias)); + new AliasAction.Remove(oldIndex, alias, null)); } } 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 c4129357f2dd5..e95a96909af88 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java @@ -149,16 +149,19 @@ boolean apply(NewAliasValidator aliasValidator, Metadata.Builder metadata, Index */ public static class Remove extends AliasAction { private final String alias; + @Nullable + private final Boolean mustExist; /** * Build the operation. */ - public Remove(String index, String alias) { + public Remove(String index, String alias, @Nullable Boolean mustExist) { super(index); if (false == Strings.hasText(alias)) { throw new IllegalArgumentException("[alias] is required"); } this.alias = alias; + this.mustExist = mustExist; } /** @@ -176,6 +179,9 @@ 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"); + } return false; } metadata.put(IndexMetadata.builder(index).removeAlias(alias)); 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 73029b5e0ecbc..55f4e7edfafd2 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 @@ -108,6 +108,17 @@ public void testBadOptionsInNonIndex() { assertEquals("[filter] is unsupported for [" + action.actionType() + "]", e.getMessage()); } + public void testMustExistOption() { + final boolean mustExist = randomBoolean(); + AliasActions removeAliasAction = AliasActions.remove(); + assertNull(removeAliasAction.mustExist()); + removeAliasAction.mustExist(mustExist); + assertEquals(mustExist, removeAliasAction.mustExist()); + AliasActions action = randomBoolean() ? AliasActions.add() : AliasActions.removeIndex(); + Exception e = expectThrows(IllegalArgumentException.class, () -> action.mustExist(mustExist)); + assertEquals("[must_exist] is unsupported for [" + action.actionType() + "]", e.getMessage()); + } + public void testParseAdd() throws IOException { String[] indices = generateRandomStringArray(10, 5, false, false); String[] aliases = generateRandomStringArray(10, 5, false, false); 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 58c904aa42e4d..12851a9d28c27 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java @@ -84,7 +84,7 @@ public void testAddAndRemove() { // Remove the alias from it while adding another one before = after; after = service.applyAliasActions(before, Arrays.asList( - new AliasAction.Remove(index, "test"), + new AliasAction.Remove(index, "test", null), new AliasAction.Add(index, "test_2", null, null, null, null, null))); assertNull(after.metadata().getIndicesLookup().get("test")); alias = after.metadata().getIndicesLookup().get("test_2"); @@ -95,12 +95,52 @@ public void testAddAndRemove() { // Now just remove on its own before = after; - after = service.applyAliasActions(before, singletonList(new AliasAction.Remove(index, "test_2"))); + after = service.applyAliasActions(before, singletonList(new AliasAction.Remove(index, "test_2", randomBoolean()))); assertNull(after.metadata().getIndicesLookup().get("test")); assertNull(after.metadata().getIndicesLookup().get("test_2")); assertAliasesVersionIncreased(index, before, after); } + public void testMustExist() { + // Create a state with a single index + String index = randomAlphaOfLength(5); + ClusterState before = createIndex(ClusterState.builder(ClusterName.DEFAULT).build(), index); + + // Add an alias to it + ClusterState after = service.applyAliasActions(before, singletonList(new AliasAction.Add(index, "test", null, null, null, null, + null))); + IndexAbstraction alias = after.metadata().getIndicesLookup().get("test"); + assertNotNull(alias); + assertThat(alias.getType(), equalTo(IndexAbstraction.Type.ALIAS)); + assertThat(alias.getIndices(), contains(after.metadata().index(index))); + assertAliasesVersionIncreased(index, before, after); + + // Remove the alias from it with mustExist == true while adding another one + before = after; + after = service.applyAliasActions(before, Arrays.asList( + new AliasAction.Remove(index, "test", true), + new AliasAction.Add(index, "test_2", null, null, null, null, null))); + assertNull(after.metadata().getIndicesLookup().get("test")); + alias = after.metadata().getIndicesLookup().get("test_2"); + assertNotNull(alias); + assertThat(alias.getType(), equalTo(IndexAbstraction.Type.ALIAS)); + assertThat(alias.getIndices(), contains(after.metadata().index(index))); + assertAliasesVersionIncreased(index, before, after); + + // Now just remove on its own + before = after; + after = service.applyAliasActions(before, singletonList(new AliasAction.Remove(index, "test_2", randomBoolean()))); + assertNull(after.metadata().getIndicesLookup().get("test")); + assertNull(after.metadata().getIndicesLookup().get("test_2")); + assertAliasesVersionIncreased(index, before, after); + + // Show that removing non-existing alias with mustExist == true fails + final ClusterState finalCS = after; + final IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, + () -> service.applyAliasActions(finalCS, singletonList(new AliasAction.Remove(index, "test_2", true)))); + assertThat(iae.getMessage(), containsString("required alias [test_2] does not exist")); + } + public void testMultipleIndices() { final var length = randomIntBetween(2, 8); final var indices = new HashSet(length); @@ -183,7 +223,7 @@ public void testAliasesVersionUnchangedWhenActionsAreIdempotent() { // now perform a remove and add for each alias which is idempotent, the resulting aliases are unchanged final var removeAndAddActions = new ArrayList(2 * length); for (final var aliasName : aliasNames) { - removeAndAddActions.add(new AliasAction.Remove(index, aliasName)); + removeAndAddActions.add(new AliasAction.Remove(index, aliasName, null)); removeAndAddActions.add(new AliasAction.Add(index, aliasName, null, null, null, null, null)); } final ClusterState afterRemoveAndAddAlias = service.applyAliasActions(afterAddingAlias, removeAndAddActions);