Added action.destructive_requires_name that controls whether wildca…

…rd expressions and `_all` is allowed to be used for destructive operat Also the delete index api requires always an index to be specified (either concrete index, alias or wildcard expression) Closes elastic#4549 elastic#4481
martijnvg · Jan 9, 2014 · bd17e7d · s1monw · Jan 9, 2014 · martijnvg
1 parent e6f8324
commit bd17e7d
Show file tree

Hide file tree

Showing 18 changed files with 430 additions and 129 deletions.
diff --git a/docs/reference/indices/delete-index.asciidoc b/docs/reference/indices/delete-index.asciidoc
@@ -8,10 +8,12 @@ The delete index API allows to delete an existing index.
 $ curl -XDELETE 'http://localhost:9200/twitter/'
 --------------------------------------------------
 
-The above example deletes an index called `twitter`.
+The above example deletes an index called `twitter`. Specifying an index,
+alias or wildcard expression is required.
 
 The delete index API can also be applied to more than one index, or on
-`_all` indices (be careful!). All indices will also be deleted when no
-specific index is provided. In order to disable allowing to delete all
-indices, set `action.disable_delete_all_indices` setting in the config
-to `true`.
+all indices (be careful!) by using `_all` or `*` as index.
+
+In order to disable allowing to delete indices via wildcards or `_all`,
+set `action.destructive_requires_name` setting in the config to `true`.
+This setting can also be changed via the cluster update settings api.
diff --git a/docs/reference/indices/open-close.asciidoc b/docs/reference/indices/open-close.asciidoc
@@ -23,6 +23,7 @@ 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. `*`).
-Closing all indices can be disabled by setting the `action.disable_close_all_indices`
-flag in the config file to `true`.
 
+Identifying indices via wildcards or `_all` can be disabled by setting the
+`action.destructive_requires_name` flag in the config file to `true`.
+This setting can also be changed via the cluster update settings api.
diff --git a/rest-api-spec/api/indices.delete.json b/rest-api-spec/api/indices.delete.json
@@ -8,7 +8,7 @@
       "parts": {
         "index": {
           "type" : "list",
-          "description" : "A comma-separated list of indices to delete; use `_all` or empty string to delete all indices"
+          "description" : "A comma-separated list of indices to delete; use `_all` or `*` string to delete all indices"
         }
       },
       "params": {

diff --git a/src/main/java/org/elasticsearch/action/admin/indices/close/TransportCloseIndexAction.java b/src/main/java/org/elasticsearch/action/admin/indices/close/TransportCloseIndexAction.java
@@ -20,8 +20,8 @@
 package org.elasticsearch.action.admin.indices.close;
 
 import org.elasticsearch.ElasticsearchException;
-import org.elasticsearch.ElasticsearchIllegalArgumentException;
 import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.support.DestructiveOperations;
 import org.elasticsearch.action.support.master.TransportMasterNodeOperationAction;
 import org.elasticsearch.cluster.ClusterService;
 import org.elasticsearch.cluster.ClusterState;
@@ -32,24 +32,25 @@
 import org.elasticsearch.cluster.metadata.MetaDataIndexStateService;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.node.settings.NodeSettingsService;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 
 /**
  * Close index action
  */
-public class TransportCloseIndexAction extends TransportMasterNodeOperationAction<CloseIndexRequest, CloseIndexResponse> {
+public class TransportCloseIndexAction extends TransportMasterNodeOperationAction<CloseIndexRequest, CloseIndexResponse> implements NodeSettingsService.Listener {
 
     private final MetaDataIndexStateService indexStateService;
-    private final boolean disableCloseAllIndices;
-
+    private volatile boolean destructiveRequiresName;
 
     @Inject
     public TransportCloseIndexAction(Settings settings, TransportService transportService, ClusterService clusterService,
-                                     ThreadPool threadPool, MetaDataIndexStateService indexStateService) {
+                                     ThreadPool threadPool, MetaDataIndexStateService indexStateService, NodeSettingsService nodeSettingsService) {
         super(settings, transportService, clusterService, threadPool);
         this.indexStateService = indexStateService;
-        this.disableCloseAllIndices = settings.getAsBoolean("action.disable_close_all_indices", false);
+        this.destructiveRequiresName = settings.getAsBoolean(DestructiveOperations.REQUIRES_NAME, false);
+        nodeSettingsService.addListener(this);
     }
 
     @Override
@@ -75,17 +76,7 @@ protected CloseIndexResponse newResponse() {
 
     @Override
     protected void doExecute(CloseIndexRequest request, ActionListener<CloseIndexResponse> listener) {
-        ClusterState state = clusterService.state();
-        String[] indicesOrAliases = request.indices();
-        request.indices(state.metaData().concreteIndices(indicesOrAliases, request.indicesOptions()));
-
-        if (disableCloseAllIndices) {
-            if (state.metaData().isExplicitAllIndices(indicesOrAliases) ||
-                    state.metaData().isPatternMatchingAllIndices(indicesOrAliases, request.indices())) {
-                throw new ElasticsearchIllegalArgumentException("closing all indices is disabled");
-            }
-        }
-
+        DestructiveOperations.failDestructive(request.indices(), destructiveRequiresName);
         super.doExecute(request, listener);
     }
 
@@ -96,7 +87,7 @@ protected ClusterBlockException checkBlock(CloseIndexRequest request, ClusterSta
 
     @Override
     protected void masterOperation(final CloseIndexRequest request, final ClusterState state, final ActionListener<CloseIndexResponse> listener) throws ElasticsearchException {
-
+        request.indices(state.metaData().concreteIndices(request.indices(), request.indicesOptions()));
         CloseIndexClusterStateUpdateRequest updateRequest = new CloseIndexClusterStateUpdateRequest()
                 .ackTimeout(request.timeout()).masterNodeTimeout(request.masterNodeTimeout())
                 .indices(request.indices());
@@ -115,4 +106,13 @@ public void onFailure(Throwable t) {
             }
         });
     }
+
+    @Override
+    public void onRefreshSettings(Settings settings) {
+        boolean newValue = settings.getAsBoolean("action.destructive_requires_name", destructiveRequiresName);
+        if (destructiveRequiresName != newValue) {
+            logger.info("updating [action.operate_all_indices] from [{}] to [{}]", destructiveRequiresName, newValue);
+            this.destructiveRequiresName = newValue;
+        }
+    }
 }
diff --git a/src/main/java/org/elasticsearch/action/admin/indices/delete/DeleteIndexRequest.java b/src/main/java/org/elasticsearch/action/admin/indices/delete/DeleteIndexRequest.java
@@ -68,7 +68,7 @@ public DeleteIndexRequest indicesOptions(IndicesOptions indicesOptions) {
     @Override
     public ActionRequestValidationException validate() {
         ActionRequestValidationException validationException = null;
-        if (indices == null) {
+        if (indices == null || indices.length == 0) {
             validationException = addValidationError("index / indices is missing", validationException);
         }
         return validationException;
@@ -114,25 +114,15 @@ public DeleteIndexRequest timeout(String timeout) {
     @Override
     public void readFrom(StreamInput in) throws IOException {
         super.readFrom(in);
-        indices = new String[in.readVInt()];
-        for (int i = 0; i < indices.length; i++) {
-            indices[i] = in.readString();
-        }
+        indices = in.readStringArray();
         indicesOptions = IndicesOptions.readIndicesOptions(in);
         timeout = readTimeValue(in);
     }
 
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         super.writeTo(out);
-        if (indices == null) {
-            out.writeVInt(0);
-        } else {
-            out.writeVInt(indices.length);
-            for (String index : indices) {
-                out.writeString(index);
-            }
-        }
+        out.writeStringArray(indices);
         indicesOptions.writeIndicesOptions(out);
         timeout.writeTo(out);
     }

diff --git a/src/main/java/org/elasticsearch/action/admin/indices/delete/TransportDeleteIndexAction.java b/src/main/java/org/elasticsearch/action/admin/indices/delete/TransportDeleteIndexAction.java
@@ -20,9 +20,8 @@
 package org.elasticsearch.action.admin.indices.delete;
 
 import org.elasticsearch.ElasticsearchException;
-import org.elasticsearch.ElasticsearchIllegalArgumentException;
 import org.elasticsearch.action.ActionListener;
-import org.elasticsearch.action.admin.indices.mapping.delete.TransportDeleteMappingAction;
+import org.elasticsearch.action.support.DestructiveOperations;
 import org.elasticsearch.action.support.master.TransportMasterNodeOperationAction;
 import org.elasticsearch.cluster.ClusterService;
 import org.elasticsearch.cluster.ClusterState;
@@ -32,28 +31,26 @@
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.CountDown;
+import org.elasticsearch.node.settings.NodeSettingsService;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 
 /**
  * Delete index action.
  */
-public class TransportDeleteIndexAction extends TransportMasterNodeOperationAction<DeleteIndexRequest, DeleteIndexResponse> {
+public class TransportDeleteIndexAction extends TransportMasterNodeOperationAction<DeleteIndexRequest, DeleteIndexResponse> implements NodeSettingsService.Listener {
 
     private final MetaDataDeleteIndexService deleteIndexService;
-
-    private final TransportDeleteMappingAction deleteMappingAction;
-
-    private final boolean disableDeleteAllIndices;
+    private volatile boolean destructiveRequiresName;
 
     @Inject
     public TransportDeleteIndexAction(Settings settings, TransportService transportService, ClusterService clusterService,
-                                      ThreadPool threadPool, MetaDataDeleteIndexService deleteIndexService, TransportDeleteMappingAction deleteMappingAction) {
+                                      ThreadPool threadPool, MetaDataDeleteIndexService deleteIndexService,
+                                      NodeSettingsService nodeSettingsService) {
         super(settings, transportService, clusterService, threadPool);
         this.deleteIndexService = deleteIndexService;
-        this.deleteMappingAction = deleteMappingAction;
-
-        this.disableDeleteAllIndices = settings.getAsBoolean("action.disable_delete_all_indices", false);
+        this.destructiveRequiresName = settings.getAsBoolean(DestructiveOperations.REQUIRES_NAME, false);
+        nodeSettingsService.addListener(this);
     }
 
     @Override
@@ -78,17 +75,7 @@ protected DeleteIndexResponse newResponse() {
 
     @Override
     protected void doExecute(DeleteIndexRequest request, ActionListener<DeleteIndexResponse> listener) {
-        ClusterState state = clusterService.state();
-        String[] indicesOrAliases = request.indices();
-
-        request.indices(state.metaData().concreteIndices(request.indices(), request.indicesOptions()));
-
-        if (disableDeleteAllIndices) {
-            if (state.metaData().isAllIndices(indicesOrAliases) ||
-                    state.metaData().isPatternMatchingAllIndices(indicesOrAliases, request.indices())) {
-                throw new ElasticsearchIllegalArgumentException("deleting all indices is disabled");
-            }
-        }
+        DestructiveOperations.failDestructive(request.indices(), destructiveRequiresName);
         super.doExecute(request, listener);
     }
 
@@ -99,6 +86,7 @@ protected ClusterBlockException checkBlock(DeleteIndexRequest request, ClusterSt
 
     @Override
     protected void masterOperation(final DeleteIndexRequest request, final ClusterState state, final ActionListener<DeleteIndexResponse> listener) throws ElasticsearchException {
+        request.indices(state.metaData().concreteIndices(request.indices(), request.indicesOptions()));
         if (request.indices().length == 0) {
             listener.onResponse(new DeleteIndexResponse(true));
             return;
@@ -136,4 +124,13 @@ public void onFailure(Throwable t) {
             });
         }
     }
+
+    @Override
+    public void onRefreshSettings(Settings settings) {
+        boolean newValue = settings.getAsBoolean("action.destructive_requires_name", destructiveRequiresName);
+        if (destructiveRequiresName != newValue) {
+            logger.info("updating [action.operate_all_indices] from [{}] to [{}]", destructiveRequiresName, newValue);
+            this.destructiveRequiresName = newValue;
+        }
+    }
 }
diff --git a/...a/org/elasticsearch/action/admin/indices/mapping/delete/TransportDeleteMappingAction.java b/...a/org/elasticsearch/action/admin/indices/mapping/delete/TransportDeleteMappingAction.java
@@ -27,6 +27,7 @@
 import org.elasticsearch.action.admin.indices.refresh.TransportRefreshAction;
 import org.elasticsearch.action.deletebyquery.DeleteByQueryResponse;
 import org.elasticsearch.action.deletebyquery.TransportDeleteByQueryAction;
+import org.elasticsearch.action.support.DestructiveOperations;
 import org.elasticsearch.action.support.QuerySourceBuilder;
 import org.elasticsearch.action.support.master.TransportMasterNodeOperationAction;
 import org.elasticsearch.client.Requests;
@@ -41,28 +42,33 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.query.FilterBuilders;
 import org.elasticsearch.index.query.QueryBuilders;
+import org.elasticsearch.node.settings.NodeSettingsService;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 
 /**
  * Delete mapping action.
  */
-public class TransportDeleteMappingAction extends TransportMasterNodeOperationAction<DeleteMappingRequest, DeleteMappingResponse> {
+public class TransportDeleteMappingAction extends TransportMasterNodeOperationAction<DeleteMappingRequest, DeleteMappingResponse> implements NodeSettingsService.Listener {
 
     private final MetaDataMappingService metaDataMappingService;
     private final TransportFlushAction flushAction;
     private final TransportDeleteByQueryAction deleteByQueryAction;
     private final TransportRefreshAction refreshAction;
+    private volatile boolean destructiveRequiresName;
 
     @Inject
     public TransportDeleteMappingAction(Settings settings, TransportService transportService, ClusterService clusterService,
                                         ThreadPool threadPool, MetaDataMappingService metaDataMappingService,
-                                        TransportDeleteByQueryAction deleteByQueryAction, TransportRefreshAction refreshAction, TransportFlushAction flushAction) {
+                                        TransportDeleteByQueryAction deleteByQueryAction, TransportRefreshAction refreshAction,
+                                        TransportFlushAction flushAction, NodeSettingsService nodeSettingsService) {
         super(settings, transportService, clusterService, threadPool);
         this.metaDataMappingService = metaDataMappingService;
         this.deleteByQueryAction = deleteByQueryAction;
         this.refreshAction = refreshAction;
         this.flushAction = flushAction;
+        this.destructiveRequiresName = settings.getAsBoolean(DestructiveOperations.REQUIRES_NAME, false);
+        nodeSettingsService.addListener(this);
     }
 
     @Override
@@ -88,7 +94,7 @@ protected DeleteMappingResponse newResponse() {
 
     @Override
     protected void doExecute(DeleteMappingRequest request, ActionListener<DeleteMappingResponse> listener) {
-        request.indices(clusterService.state().metaData().concreteIndices(request.indices(), request.indicesOptions()));
+        DestructiveOperations.failDestructive(request.indices(), destructiveRequiresName);
         super.doExecute(request, listener);
     }
 
@@ -99,6 +105,7 @@ protected ClusterBlockException checkBlock(DeleteMappingRequest request, Cluster
 
     @Override
     protected void masterOperation(final DeleteMappingRequest request, final ClusterState state, final ActionListener<DeleteMappingResponse> listener) throws ElasticsearchException {
+        request.indices(state.metaData().concreteIndices(request.indices(), request.indicesOptions()));
         flushAction.execute(Requests.flushRequest(request.indices()), new ActionListener<FlushResponse>() {
             @Override
             public void onResponse(FlushResponse flushResponse) {
@@ -152,4 +159,13 @@ public void onFailure(Throwable t) {
             }
         });
     }
+
+    @Override
+    public void onRefreshSettings(Settings settings) {
+        boolean newValue = settings.getAsBoolean("action.destructive_requires_name", destructiveRequiresName);
+        if (destructiveRequiresName != newValue) {
+            logger.info("updating [action.operate_all_indices] from [{}] to [{}]", destructiveRequiresName, newValue);
+            this.destructiveRequiresName = newValue;
+        }
+    }
 }
diff --git a/src/main/java/org/elasticsearch/action/admin/indices/open/TransportOpenIndexAction.java b/src/main/java/org/elasticsearch/action/admin/indices/open/TransportOpenIndexAction.java
@@ -21,6 +21,7 @@
 
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.support.DestructiveOperations;
 import org.elasticsearch.action.support.master.TransportMasterNodeOperationAction;
 import org.elasticsearch.cluster.ClusterService;
 import org.elasticsearch.cluster.ClusterState;
@@ -31,21 +32,25 @@
 import org.elasticsearch.cluster.metadata.MetaDataIndexStateService;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.node.settings.NodeSettingsService;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 
 /**
  * Open index action
  */
-public class TransportOpenIndexAction extends TransportMasterNodeOperationAction<OpenIndexRequest, OpenIndexResponse> {
+public class TransportOpenIndexAction extends TransportMasterNodeOperationAction<OpenIndexRequest, OpenIndexResponse> implements NodeSettingsService.Listener {
 
     private final MetaDataIndexStateService indexStateService;
+    private volatile boolean destructiveRequiresName;
 
     @Inject
     public TransportOpenIndexAction(Settings settings, TransportService transportService, ClusterService clusterService,
-                                    ThreadPool threadPool, MetaDataIndexStateService indexStateService) {
+                                    ThreadPool threadPool, MetaDataIndexStateService indexStateService, NodeSettingsService nodeSettingsService) {
         super(settings, transportService, clusterService, threadPool);
         this.indexStateService = indexStateService;
+        this.destructiveRequiresName = settings.getAsBoolean(DestructiveOperations.REQUIRES_NAME, false);
+        nodeSettingsService.addListener(this);
     }
 
     @Override
@@ -71,7 +76,7 @@ protected OpenIndexResponse newResponse() {
 
     @Override
     protected void doExecute(OpenIndexRequest request, ActionListener<OpenIndexResponse> listener) {
-        request.indices(clusterService.state().metaData().concreteIndices(request.indices(), request.indicesOptions()));
+        DestructiveOperations.failDestructive(request.indices(), destructiveRequiresName);
         super.doExecute(request, listener);
     }
 
@@ -82,7 +87,7 @@ protected ClusterBlockException checkBlock(OpenIndexRequest request, ClusterStat
 
     @Override
     protected void masterOperation(final OpenIndexRequest request, final ClusterState state, final ActionListener<OpenIndexResponse> listener) throws ElasticsearchException {
-
+        request.indices(state.metaData().concreteIndices(request.indices(), request.indicesOptions()));
         OpenIndexClusterStateUpdateRequest updateRequest = new OpenIndexClusterStateUpdateRequest()
                 .ackTimeout(request.timeout()).masterNodeTimeout(request.masterNodeTimeout())
                 .indices(request.indices());
@@ -101,4 +106,13 @@ public void onFailure(Throwable t) {
             }
         });
     }
+
+    @Override
+    public void onRefreshSettings(Settings settings) {
+        boolean newValue = settings.getAsBoolean("action.destructive_requires_name", destructiveRequiresName);
+        if (destructiveRequiresName != newValue) {
+            logger.info("updating [action.operate_all_indices] from [{}] to [{}]", destructiveRequiresName, newValue);
+            this.destructiveRequiresName = newValue;
+        }
+    }
 }