Allow the "*,-*" ("no-index") pattern for destructive actions when destructive_requires_name is true

rest-api-spec/src/main/resources/rest-api-spec/test/indices.delete/20_destructive_wildcard.yml

@@ -0,0 +1,22 @@
+---
+setup:
+  - do:
+      cluster.put_settings:
+        body:
+          transient:
+            action.destructive_requires_name: "true"
+        flat_settings: true
+---
+teardown:
+  - do:
+      cluster.put_settings:
+        body:
+          transient:
+            action.destructive_requires_name: "false"
+        flat_settings: true
+---
+"Delete nonexistent concrete index with wildcard expansion disallowed":
+  - do:
+      indices.delete:
+        index: index3
+        ignore_unavailable: true

server/src/internalClusterTest/java/org/elasticsearch/operateAllIndices/DestructiveOperationsIT.java

@@ -27,6 +27,7 @@
 import org.elasticsearch.test.ESIntegTestCase;
 import org.junit.After;
 
+import static org.elasticsearch.cluster.metadata.IndexMetadata.APIBlock.WRITE;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.hamcrest.Matchers.equalTo;
 
@@ -48,6 +49,8 @@ public void testDeleteIndexIsRejected() throws Exception {
 
         // Should succeed, since no wildcards
         assertAcked(client().admin().indices().prepareDelete("1index").get());
+        // Special "match none" pattern succeeds, since non-destructive
+        assertAcked(client().admin().indices().prepareDelete("*", "-*").get());
 
         expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareDelete("i*").get());
         expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareDelete("_all").get());
@@ -82,6 +85,8 @@ public void testCloseIndexIsRejected() throws Exception {
 
         // Should succeed, since no wildcards
         assertAcked(client().admin().indices().prepareClose("1index").get());
+        // Special "match none" pattern succeeds, since non-destructive
+        assertAcked(client().admin().indices().prepareClose("*", "-*").get());
 
         expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareClose("i*").get());
         expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareClose("_all").get());
@@ -118,6 +123,9 @@ public void testOpenIndexIsRejected() throws Exception {
         createIndex("index1", "1index");
         assertAcked(client().admin().indices().prepareClose("1index", "index1").get());
 
+        // Special "match none" pattern succeeds, since non-destructive
+        assertAcked(client().admin().indices().prepareOpen("*", "-*").get());
+
         expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareOpen("i*").get());
         expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareOpen("_all").get());
     }
@@ -144,4 +152,46 @@ public void testOpenIndexDefaultBehaviour() throws Exception {
             assertEquals(IndexMetadata.State.OPEN, indexMetadataObjectObjectCursor.value.getState());
         }
     }
+
+    public void testAddIndexBlockIsRejected() throws Exception {
+        Settings settings = Settings.builder()
+            .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), true)
+            .build();
+        assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
+
+        createIndex("index1", "1index");
+
+        // Should succeed, since no wildcards
+        assertAcked(client().admin().indices().prepareAddBlock(WRITE,"1index").get());
+        // Special "match none" pattern succeeds, since non-destructive
+        assertAcked(client().admin().indices().prepareAddBlock(WRITE,"*", "-*").get());
+
+        expectThrows(IllegalArgumentException.class,
+            () -> client().admin().indices().prepareAddBlock(WRITE,"i*").get());
+        expectThrows(IllegalArgumentException.class,
+            () -> client().admin().indices().prepareAddBlock(WRITE, "_all").get());
+    }
+
+    public void testAddIndexBlockDefaultBehaviour() throws Exception {
+        if (randomBoolean()) {
+            Settings settings = Settings.builder()
+                .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), false)
+                .build();
+            assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings));
+        }
+
+        createIndex("index1", "1index");
+
+        if (randomBoolean()) {
+            assertAcked(client().admin().indices().prepareAddBlock(WRITE, "_all").get());
+        } else {
+            assertAcked(client().admin().indices().prepareAddBlock(WRITE, "*").get());
+        }
+
+        ClusterState state = client().admin().cluster().prepareState().get().getState();
+        assertTrue("write block is set on index1",
+            state.getBlocks().hasIndexBlock("index1", IndexMetadata.INDEX_WRITE_BLOCK));
+        assertTrue("write block is set on 1index",
+            state.getBlocks().hasIndexBlock("1index", IndexMetadata.INDEX_WRITE_BLOCK));
+    }
 }

server/src/main/java/org/elasticsearch/action/support/DestructiveOperations.java

@@ -24,6 +24,8 @@
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.settings.Settings;
 
+import java.util.Arrays;
+
 /**
  * Helper for dealing with destructive operations and wildcard usage.
  */
@@ -34,6 +36,15 @@ public final class DestructiveOperations {
      */
     public static final Setting<Boolean> REQUIRES_NAME_SETTING =
         Setting.boolSetting("action.destructive_requires_name", false, Property.Dynamic, Property.NodeScope);
+    /**
+     * The "match none" pattern, "*,-*", will never actually be destructive
+     * because it operates on no indices. If plugins or other components add
+     * their own index resolution layers, they can pass this pattern to the
+     * core code in order to indicate that an operation won't run on any
+     * indices, relying on the core code to handle this situation.
+     */
+    private static final String[] MATCH_NONE_PATTERN = {"*", "-*"};
+
     private volatile boolean destructiveRequiresName;
 
     public DestructiveOperations(Settings settings, ClusterSettings clusterSettings) {
@@ -59,7 +70,7 @@ public void failDestructive(String[] aliasesOrIndices) {
             if (hasWildcardUsage(aliasesOrIndices[0])) {
                 throw new IllegalArgumentException("Wildcard expressions or all indices are not allowed");
             }
-        } else {
+        } else if (Arrays.equals(aliasesOrIndices, MATCH_NONE_PATTERN) == false) {
             for (String aliasesOrIndex : aliasesOrIndices) {
                 if (hasWildcardUsage(aliasesOrIndex)) {
                     throw new IllegalArgumentException("Wildcard expressions or all indices are not allowed");

server/src/test/java/org/elasticsearch/action/support/DestructiveOperationsTests.java

@@ -0,0 +1,93 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.action.support;
+
+import org.elasticsearch.common.settings.ClusterSettings;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.test.ESTestCase;
+
+import java.util.Set;
+
+import static org.hamcrest.Matchers.equalTo;
+
+public class DestructiveOperationsTests extends ESTestCase {
+
+    private DestructiveOperations destructiveOperations;
+
+    @Override
+    public void setUp() throws Exception {
+        super.setUp();
+        Settings nodeSettings = Settings.builder()
+            .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), "true")
+            .build();
+        destructiveOperations = new DestructiveOperations(
+            nodeSettings,
+            new ClusterSettings(nodeSettings, Set.of(DestructiveOperations.REQUIRES_NAME_SETTING)));
+    }
+
+    public void testDestructive() {
+        {
+            // requests that might resolve to all indices
+            assertFailsDestructive(null);
+            assertFailsDestructive(new String[]{});
+            assertFailsDestructive(new String[]{"_all"});
+            assertFailsDestructive(new String[]{"*"});
+        }
+        {
+            // various wildcards
+            assertFailsDestructive(new String[] {"-*"});
+            assertFailsDestructive(new String[] {"index*"});
+            assertFailsDestructive(new String[] {"index", "*"});
+            assertFailsDestructive(new String[] {"index", "-*"});
+            assertFailsDestructive(new String[] {"index", "test-*-index"});
+        }
+        {
+            // near versions of the "matchNone" pattern
+            assertFailsDestructive(new String[]{"-*", "*"});
+            assertFailsDestructive(new String[]{"*", "-*", "*"});
+        }
+    }
+
+    /**
+     * Test that non-wildcard expressions or the special "*,-*" don't throw an
+     * exception. Since {@link DestructiveOperations#failDestructive(String[])}
+     * has no return value, we run the statements without asserting anything
+     * about them.
+     */
+    public void testNonDestructive() {
+        {
+            // no wildcards
+            destructiveOperations.failDestructive(new String[]{"index"});
+            destructiveOperations.failDestructive(new String[]{"index", "-index2"});
+        }
+        {
+            // special "matchNone" pattern
+            destructiveOperations.failDestructive(new String[]{"*", "-*"});
+        }
+    }
+
+    private void assertFailsDestructive(String[] aliasesOrIndices) {
+        IllegalArgumentException e = expectThrows(
+            IllegalArgumentException.class,
+            () -> destructiveOperations.failDestructive(aliasesOrIndices));
+
+        assertThat(e.getMessage(), equalTo("Wildcard expressions or all indices are not allowed"));
+    }
+}

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/action/filter/DestructiveOperationsTests.java

@@ -49,6 +49,8 @@ public void testDeleteIndexDestructiveOperationsRequireName() {
             assertEquals("index1", indices[0]);
         }
 
+        // the "*,-*" pattern is specially handled because it makes a destructive action non-destructive
+        assertAcked(client().admin().indices().prepareDelete("*", "-*"));
         assertAcked(client().admin().indices().prepareDelete("index1"));
     }
 
@@ -114,6 +116,10 @@ public void testOpenCloseIndexDestructiveOperationsRequireName() {
             assertEquals("Wildcard expressions or all indices are not allowed", illegalArgumentException.getMessage());
         }
 
+        // the "*,-*" pattern is specially handled because it makes a destructive action non-destructive
+        assertAcked(client().admin().indices().prepareClose("*", "-*"));
+        assertAcked(client().admin().indices().prepareOpen("*", "-*"));
+
         createIndex("index1");
         assertAcked(client().admin().indices().prepareClose("index1"));
         assertAcked(client().admin().indices().prepareOpen("index1"));