Change default value of action.destructive_requires_name to True. (#66908)

YashJipkate · web-flow · commit 60f4d2272205 · 2021-03-31T15:59:57.000-04:00
This PR sets the default value of `action.destructive_requires_name` to `true.` Fixes #61074. Additionally, we set this value explicitly in test classes that rely on wildcard deletions to clear test state.
diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java
@@ -1180,6 +1180,8 @@ private void createConfiguration() {
             baseConfig.put("cluster.service.slow_master_task_logging_threshold", "5s");
         }
 
+        baseConfig.put("action.destructive_requires_name", "false");
+
         HashSet<String> overriden = new HashSet<>(baseConfig.keySet());
         overriden.retainAll(settings.keySet());
         overriden.removeAll(OVERRIDABLE_SETTINGS);
diff --git a/distribution/docker/docker-compose.yml b/distribution/docker/docker-compose.yml
@@ -30,6 +30,7 @@ services:
        - xpack.http.ssl.verification_mode=certificate
        - xpack.security.transport.ssl.verification_mode=certificate
        - xpack.license.self_generated.type=trial
+       - action.destructive_requires_name=false
     volumes:
        - ./build/repo:/tmp/es-repo
        - ./build/certs/testnode.pem:/usr/share/elasticsearch/config/testnode.pem
@@ -81,6 +82,7 @@ services:
        - xpack.http.ssl.verification_mode=certificate
        - xpack.security.transport.ssl.verification_mode=certificate
        - xpack.license.self_generated.type=trial
+       - action.destructive_requires_name=false
     volumes:
        - ./build/repo:/tmp/es-repo
        - ./build/certs/testnode.pem:/usr/share/elasticsearch/config/testnode.pem
diff --git a/distribution/src/config/elasticsearch.yml b/distribution/src/config/elasticsearch.yml
@@ -77,6 +77,6 @@ ${path.logs}
 #
 # ---------------------------------- Various -----------------------------------
 #
-# Require explicit names when deleting indices:
+# Allow wildcard deletion of indices:
 #
-#action.destructive_requires_name: true
+#action.destructive_requires_name: false
diff --git a/docs/reference/index-modules/blocks.asciidoc b/docs/reference/index-modules/blocks.asciidoc
@@ -74,9 +74,9 @@ PUT /my-index-000001/_block/write
 
 include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=index]
 +
-To add blocks to all indices, use `_all` or `*`. To disallow the adding
-of blocks to indices with `_all` or wildcard expressions,
-change the `action.destructive_requires_name` cluster setting to `true`.
+By default, you must explicitly name the indices you are adding blocks to.
+To allow the adding of blocks to indices with `_all`, `*``, or other wildcard
+expressions, change the `action.destructive_requires_name` setting to `false`.
 You can update this setting in the `elasticsearch.yml` file
 or using the <<cluster-update-settings,cluster update settings>> API.
 `<block>`::
diff --git a/docs/reference/indices/close.asciidoc b/docs/reference/indices/close.asciidoc
@@ -38,8 +38,9 @@ include::{es-repo-dir}/indices/open-close.asciidoc[tag=closed-index]
 include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=index]
 +
 To close all indices, use `_all` or `*`.
-To disallow the closing of indices with `_all` or wildcard expressions,
-change the `action.destructive_requires_name` cluster setting to `true`.
+By default, you must explicitly name the indices you are closing.
+To specify indices to close with `_all`, `*``, or other wildcard
+expressions, change the `action.destructive_requires_name` setting to `false`.
 You can update this setting in the `elasticsearch.yml` file
 or using the <<cluster-update-settings,cluster update settings>> API.
 
diff --git a/docs/reference/indices/delete-index.asciidoc b/docs/reference/indices/delete-index.asciidoc
@@ -36,10 +36,10 @@ delete.
 In this parameter, wildcard expressions match only open, concrete indices. You
 cannot delete an index using an <<indices-aliases,alias>>.
 
-To delete all indices, use `_all` or `*` . To disallow the deletion of indices
-with `_all` or wildcard expressions, change the
-`action.destructive_requires_name` cluster setting to `true`. You can update
-this setting in the `elasticsearch.yml` file or using the
+By default, you must explicitly name the indices you are deleting.
+To specify indices to delete with `_all`, `*``, or other wildcard
+expressions, change the `action.destructive_requires_name` setting to `false`.
+You can update this setting in the `elasticsearch.yml` file or using the
 <<cluster-update-settings,cluster update settings>> API.
 
 NOTE: You cannot delete the current write index of a data stream. To delete the
diff --git a/docs/reference/indices/open-close.asciidoc b/docs/reference/indices/open-close.asciidoc
@@ -53,11 +53,10 @@ You can open and close multiple indices. An error is thrown
 if the request explicitly refers to a missing index. This behaviour can be
 disabled using the `ignore_unavailable=true` parameter.
 
-All indices can be opened or closed at once using `_all` as the index name
-or specifying patterns that identify them all (e.g. `*`).
 
-Identifying indices via wildcards or `_all` can be disabled by setting the
-`action.destructive_requires_name` flag in the config file to `true`.
+By default, you must explicitly name the indices you are opening or closing.
+To open or close indices with `_all`, `*``, or other wildcard
+expressions, change the `action.destructive_requires_name` setting to `false`.
 This setting can also be changed via the cluster update settings api.
 
 Closed indices consume a significant amount of disk-space which can cause
@@ -88,10 +87,10 @@ index creation applies to the `_open` and `_close` index actions as well.
 Comma-separated list of data streams, indices, and index aliases used to limit
 the request. Wildcard (`*`) expressions are supported.
 +
-To target all data streams and indices, use `_all` or `*`.
 +
-To disallow use of `_all` or wildcard expressions,
-change the `action.destructive_requires_name` cluster setting to `true`.
+By default, you must explicitly name the indices you using to limit the request.
+To limit a request using `_all`, `*``, or other wildcard
+expressions, change the `action.destructive_requires_name` setting to `false`.
 You can update this setting in the `elasticsearch.yml` file
 or using the <<cluster-update-settings,cluster update settings>> API.
 
diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/ServerUtils.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/ServerUtils.java
@@ -229,7 +229,7 @@ public static void runElasticsearchTests() throws Exception {
         String count = makeRequest(Request.Get("http://localhost:9200/_count?pretty"));
         assertThat(count, containsString("\"count\" : 2"));
 
-        makeRequest(Request.Delete("http://localhost:9200/_all"));
+        makeRequest(Request.Delete("http://localhost:9200/library"));
     }
 
     public static String makeRequest(Request request) throws Exception {
diff --git a/qa/remote-clusters/docker-compose-oss.yml b/qa/remote-clusters/docker-compose-oss.yml
@@ -16,6 +16,7 @@ services:
        - cluster.routing.allocation.disk.watermark.high=1b
        - cluster.routing.allocation.disk.watermark.flood_stage=1b
        - node.store.allow_mmap=false
+       - action.destructive_requires_name=false
     volumes:
        - ./build/oss-repo:/tmp/es-repo
        - ./build/logs/oss-1:/usr/share/elasticsearch/logs
@@ -50,6 +51,7 @@ services:
        - cluster.routing.allocation.disk.watermark.high=1b
        - cluster.routing.allocation.disk.watermark.flood_stage=1b
        - node.store.allow_mmap=false
+       - action.destructive_requires_name=false
     volumes:
        - ./build/oss-repo:/tmp/es-repo
        - ./build/logs/oss-2:/usr/share/elasticsearch/logs
diff --git a/qa/remote-clusters/docker-compose.yml b/qa/remote-clusters/docker-compose.yml
@@ -30,6 +30,7 @@ services:
        - xpack.http.ssl.verification_mode=certificate
        - xpack.security.transport.ssl.verification_mode=certificate
        - xpack.license.self_generated.type=trial
+       - action.destructive_requires_name=false
     volumes:
        - ./build/repo:/tmp/es-repo
        - ./build/certs/testnode.pem:/usr/share/elasticsearch/config/testnode.pem
@@ -82,6 +83,7 @@ services:
        - xpack.http.ssl.verification_mode=certificate
        - xpack.security.transport.ssl.verification_mode=certificate
        - xpack.license.self_generated.type=trial
+       - action.destructive_requires_name=false
     volumes:
        - ./build/repo:/tmp/es-repo
        - ./build/certs/testnode.pem:/usr/share/elasticsearch/config/testnode.pem
diff --git a/server/src/main/java/org/elasticsearch/action/support/DestructiveOperations.java b/server/src/main/java/org/elasticsearch/action/support/DestructiveOperations.java
@@ -24,7 +24,8 @@ public final class DestructiveOperations {
      * Setting which controls whether wildcard usage (*, prefix*, _all) is allowed.
      */
     public static final Setting<Boolean> REQUIRES_NAME_SETTING =
-        Setting.boolSetting("action.destructive_requires_name", false, Property.Dynamic, Property.NodeScope);
+        Setting.boolSetting("action.destructive_requires_name", true, 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
diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java
@@ -44,6 +44,7 @@
 import org.elasticsearch.action.search.ClearScrollResponse;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.action.support.DefaultShardOperationFailedException;
+import org.elasticsearch.action.support.DestructiveOperations;
 import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.client.AdminClient;
 import org.elasticsearch.client.Client;
@@ -1734,6 +1735,7 @@ private int getNumClientNodes() {
      */
     protected Settings nodeSettings(int nodeOrdinal) {
         Settings.Builder builder = Settings.builder()
+            .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), false)
             // Default the watermarks to absurdly low to prevent the tests
             // from failing on nodes without enough disk space
             .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey(), "1b")
diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java
@@ -12,6 +12,7 @@
 import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
 import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
 import org.elasticsearch.action.admin.indices.get.GetIndexResponse;
+import org.elasticsearch.action.support.DestructiveOperations;
 import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.client.Requests;
@@ -188,6 +189,7 @@ private Node newNode() {
 
         Settings settings = Settings.builder()
             .put(ClusterName.CLUSTER_NAME_SETTING.getKey(), InternalTestCluster.clusterName("single-node-cluster", random().nextLong()))
+            .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), false)
             .put(Environment.PATH_HOME_SETTING.getKey(), tempDir)
             .put(Environment.PATH_REPO_SETTING.getKey(), tempDir.resolve("repo"))
             // TODO: use a consistent data path for custom paths
diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java
@@ -25,6 +25,7 @@
 import org.elasticsearch.action.admin.cluster.node.stats.NodeStats;
 import org.elasticsearch.action.admin.indices.stats.CommonStatsFlags;
 import org.elasticsearch.action.admin.indices.stats.CommonStatsFlags.Flag;
+import org.elasticsearch.action.support.DestructiveOperations;
 import org.elasticsearch.action.support.replication.TransportReplicationAction;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.cluster.ClusterName;
@@ -388,6 +389,7 @@ public InternalTestCluster(
         // TODO: currently we only randomize "cluster.no_master_block" between "write" and "metadata_write", as "all" is fragile
         // and fails shards when a master abdicates, which breaks many tests.
         builder.put(NoMasterBlockService.NO_MASTER_BLOCK_SETTING.getKey(), randomFrom(random,"write", "metadata_write"));
+        builder.put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), false);
         defaultSettings = builder.build();
         executor = EsExecutors.newScaling("internal_test_cluster_executor", 0, Integer.MAX_VALUE, 0, TimeUnit.SECONDS,
                 EsExecutors.daemonThreadFactory("test_" + clusterName), new ThreadContext(Settings.EMPTY));

Original file line number	Diff line number	Diff line change
`@@ -1180,6 +1180,8 @@ private void createConfiguration() {`
`1180`	`1180`	`baseConfig.put("cluster.service.slow_master_task_logging_threshold", "5s");`
`1181`	`1181`	`}`
`1182`	`1182`
	`1183`	`+ baseConfig.put("action.destructive_requires_name", "false");`
	`1184`	`+`
`1183`	`1185`	`HashSet<String> overriden = new HashSet<>(baseConfig.keySet());`
`1184`	`1186`	`overriden.retainAll(settings.keySet());`
`1185`	`1187`	`overriden.removeAll(OVERRIDABLE_SETTINGS);`
Original file line number	Diff line number	Diff line change
`@@ -77,6 +77,6 @@ ${path.logs}`
`77`	`77`	`#`
`78`	`78`	`# ---------------------------------- Various -----------------------------------`
`79`	`79`	`#`
`80`		`-# Require explicit names when deleting indices:`
	`80`	`+# Allow wildcard deletion of indices:`
`81`	`81`	`#`
`82`		`-#action.destructive_requires_name: true`
	`82`	`+#action.destructive_requires_name: false`
Original file line number	Diff line number	Diff line change
`@@ -229,7 +229,7 @@ public static void runElasticsearchTests() throws Exception {`
`229`	`229`	`String count = makeRequest(Request.Get("http://localhost:9200/_count?pretty"));`
`230`	`230`	`assertThat(count, containsString("\"count\" : 2"));`
`231`	`231`
`232`		`- makeRequest(Request.Delete("http://localhost:9200/_all"));`
	`232`	`+ makeRequest(Request.Delete("http://localhost:9200/library"));`
`233`	`233`	`}`
`234`	`234`
`235`	`235`	`public static String makeRequest(Request request) throws Exception {`