Fix remove alias with must_exist (elastic#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 elastic#62642
Relates elastic#58100

Co-Authored-By: Luca Cavanna <javanna@users.noreply.github.com>

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.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

server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java

@@ -182,6 +182,9 @@ private static ObjectParser<AliasActions, Void> parser(String name, Supplier<Ali
         }
 
         private static final ObjectParser<AliasActions, Void> ADD_PARSER = parser(ADD.getPreferredName(), AliasActions::add);
+        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(),
+            AliasActions::removeIndex);
         static {
             ADD_PARSER.declareObject(AliasActions::filter, (parser, m) -> {
                 try {
@@ -196,11 +199,8 @@ private static ObjectParser<AliasActions, Void> parser(String name, Supplier<Ali
             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);
+            REMOVE_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(),
-                AliasActions::removeIndex);
 
         /**
          * Parser for any one {@link AliasAction}.
@@ -547,6 +547,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;
@@ -567,6 +570,7 @@ public String toString() {
                     + ",indexRouting=" + indexRouting
                     + ",searchRouting=" + searchRouting
                     + ",writeIndex=" + writeIndex
+                    + ",mustExist=" + mustExist
                     + "]";
         }
 
@@ -585,12 +589,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);
         }
     }
 

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

@@ -170,6 +170,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

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;
             }

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));
         }
     }
 

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;
@@ -139,7 +140,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"));
     }

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));