Browse Source

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.
Yannick Welsch 5 years ago
parent
commit
b305454e23

+ 4 - 0
docs/reference/indices/aliases.asciidoc

@@ -113,6 +113,10 @@ unless overriden in the request using the `expand_wildcards` parameter,
 similar to <<index-hidden,hidden indices>>. 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.

+ 23 - 0
server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java

@@ -98,6 +98,7 @@ public class IndicesAliasesRequest extends AcknowledgedRequest<IndicesAliasesReq
         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 @@ public class IndicesAliasesRequest extends AcknowledgedRequest<IndicesAliasesReq
             ADD_PARSER.declareField(AliasActions::searchRouting, XContentParser::text, SEARCH_ROUTING, ValueType.INT);
             ADD_PARSER.declareField(AliasActions::writeIndex, XContentParser::booleanValue, IS_WRITE_INDEX, ValueType.BOOLEAN);
             ADD_PARSER.declareField(AliasActions::isHidden, XContentParser::booleanValue, IS_HIDDEN, ValueType.BOOLEAN);
+            ADD_PARSER.declareField(AliasActions::mustExist, XContentParser::booleanValue, MUST_EXIST, ValueType.BOOLEAN);
         }
         private static final ObjectParser<AliasActions, Void> REMOVE_PARSER = parser(REMOVE.getPreferredName(), AliasActions::remove);
         private static final ObjectParser<AliasActions, Void> REMOVE_INDEX_PARSER = parser(REMOVE_INDEX.getPreferredName(),
@@ -234,6 +236,7 @@ public class IndicesAliasesRequest extends AcknowledgedRequest<IndicesAliasesReq
         private String searchRouting;
         private Boolean writeIndex;
         private Boolean isHidden;
+        private Boolean mustExist;
 
         public AliasActions(AliasActions.Type type) {
             this.type = type;
@@ -255,6 +258,11 @@ public class IndicesAliasesRequest extends AcknowledgedRequest<IndicesAliasesReq
                 isHidden = in.readOptionalBoolean();
             }
             originalAliases = in.readStringArray();
+            if (in.getVersion().onOrAfter(Version.V_8_0_0)) {
+                mustExist = in.readOptionalBoolean();
+            } else {
+                mustExist = null;
+            }
         }
 
         @Override
@@ -271,6 +279,9 @@ public class IndicesAliasesRequest extends AcknowledgedRequest<IndicesAliasesReq
                 out.writeOptionalBoolean(isHidden);
             }
             out.writeStringArray(originalAliases);
+            if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
+                out.writeOptionalBoolean(mustExist);
+            }
         }
 
         /**
@@ -455,6 +466,18 @@ public class IndicesAliasesRequest extends AcknowledgedRequest<IndicesAliasesReq
             return isHidden;
         }
 
+        public AliasActions mustExist(Boolean mustExist) {
+            if (type != Type.REMOVE) {
+                throw new IllegalArgumentException("[" + MUST_EXIST.getPreferredName() + "] is unsupported for [" + type + "]");
+            }
+            this.mustExist = mustExist;
+            return this;
+        }
+
+        public Boolean mustExist() {
+            return mustExist;
+        }
+
         @Override
         public String[] aliases() {
             return aliases;

+ 1 - 1
server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java

@@ -131,7 +131,7 @@ public class TransportIndicesAliasesAction extends TransportMasterNodeAction<Ind
                     break;
                 case REMOVE:
                     for (String alias : concreteAliases(action, state.metadata(), index.getName())) {
-                        finalActions.add(new AliasAction.Remove(index.getName(), alias));
+                        finalActions.add(new AliasAction.Remove(index.getName(), alias, action.mustExist()));
                     }
                     break;
                 case REMOVE_INDEX:

+ 1 - 1
server/src/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java

@@ -227,7 +227,7 @@ public class MetadataRolloverService {
         } 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));
         }
     }
 

+ 7 - 1
server/src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java

@@ -149,16 +149,19 @@ public abstract class AliasAction {
      */
     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 @@ public abstract class AliasAction {
         @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));

+ 11 - 0
server/src/test/java/org/elasticsearch/action/admin/indices/alias/AliasActionsTests.java

@@ -108,6 +108,17 @@ public class AliasActionsTests extends ESTestCase {
         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);

+ 43 - 3
server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java

@@ -84,7 +84,7 @@ public class MetadataIndexAliasesServiceTests extends ESTestCase {
         // 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 class MetadataIndexAliasesServiceTests extends ESTestCase {
 
         // 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<String>(length);
@@ -183,7 +223,7 @@ public class MetadataIndexAliasesServiceTests extends ESTestCase {
         // now perform a remove and add for each alias which is idempotent, the resulting aliases are unchanged
         final var removeAndAddActions = new ArrayList<AliasAction>(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);