elastic · jasontedor · May 1, 2018
diff --git a/docs/reference/indices/shrink-index.asciidoc b/docs/reference/indices/shrink-index.asciidoc
@@ -127,7 +127,7 @@ during a shrink operation. With the exception of non-copyable settings, settings
 from the source index can be copied to the target index by adding the URL
 parameter `copy_settings=true` to the request.
 
-deprecated[6.4.0, `copy_settings` will default to `true` in 8.x and will be removed in 9.0.0]
+deprecated[6.4.0, `copy_settings` will default to `true` in 7.x and will be removed in 8.0.0]
 
 [float]
 === Monitoring the shrink process

diff --git a/docs/reference/indices/split-index.asciidoc b/docs/reference/indices/split-index.asciidoc
@@ -183,7 +183,7 @@ during a split operation. With the exception of non-copyable settings, settings
 from the source index can be copied to the target index by adding the URL
 parameter `copy_settings=true` to the request.
 
-deprecated[6.4.0, `copy_settings` will default to `true` in 8.x and will be removed in 9.0.0]
+deprecated[6.4.0, `copy_settings` will default to `true` in 7.x and will be removed in 8.0.0]
 
 [float]
 === Monitoring the split process

diff --git a/docs/reference/migration/migrate_7_0/api.asciidoc b/docs/reference/migration/migrate_7_0/api.asciidoc
@@ -65,3 +65,13 @@ deprecated in 6.3.0 and now removed in 7.0.0.
 In the past, `fields` could be provided either as a parameter, or as part of the request
 body. Specifying `fields` in the request body as opposed to a parameter was deprecated
 in 6.4.0, and is now unsupported in 7.0.0.
+
+[[resize-copy-settings-false]]
+==== Copy settings parameter on resize operations no longer accepts `false`
+
+In 6.4.0 the `copy_settings` parameter was introduced to enable copying the
+index settings of the source index on a shrink or split index operation to the
+target index. The default value in 6.4.0 was `false`. In accordance with that
+setting being immediately deprecated in 6.4.0, `copy_settings` now defaults to
+`true` in 7.0.0 and can no longer be set to `false`. The parameter will be
+removed in 8.0.0.
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/30_copy_settings.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/30_copy_settings.yml
@@ -64,8 +64,9 @@
   - match: { copy-settings-target.settings.index.blocks.write: "true" }
   - match: { copy-settings-target.settings.index.routing.allocation.include._id: $master }
 
-  # now we do a actual shrink and do not copy settings
+  # now we check that copy_settings can not be set to false
   - do:
+      catch: /illegal_argument_exception/
       indices.shrink:
         index: "source"
         target: "no-copy-settings-target"
@@ -79,16 +80,3 @@
       warnings:
         - "parameter [copy_settings] is deprecated but was [false]"
 
-  - do:
-      cluster.health:
-        wait_for_status: green
-
-  - do:
-      indices.get_settings:
-        index: "no-copy-settings-target"
-
-  # only the request setting should be copied
-  - is_false: no-copy-settings-target.settings.index.merge.scheduler.max_merge_count
-  - match: { no-copy-settings-target.settings.index.merge.scheduler.max_thread_count: "2" }
-  - is_false: no-copy-settings-target.settings.index.blocks.write
-  - is_false: no-copy-settings-target.settings.index.routing.allocation.include._id
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/30_copy_settings.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/30_copy_settings.yml
@@ -67,8 +67,9 @@
   - match: { copy-settings-target.settings.index.blocks.write: "true" }
   - match: { copy-settings-target.settings.index.routing.allocation.include._id: $master }
 
-  # now we do a actual shrink and do not copy settings
+  # now we check that copy_settings can not be set to false
   - do:
+      catch: /illegal_argument_exception/
       indices.split:
         index: "source"
         target: "no-copy-settings-target"
@@ -82,17 +83,3 @@
             index.merge.scheduler.max_thread_count: 2
       warnings:
         - "parameter [copy_settings] is deprecated but was [false]"
-
-  - do:
-      cluster.health:
-        wait_for_status: green
-
-  - do:
-      indices.get_settings:
-        index: "no-copy-settings-target"
-
-  # only the request setting should be copied
-  - is_false: no-copy-settings-target.settings.index.merge.scheduler.max_merge_count
-  - match: { no-copy-settings-target.settings.index.merge.scheduler.max_thread_count: "2" }
-  - is_false: no-copy-settings-target.settings.index.blocks.write
-  - is_false: no-copy-settings-target.settings.index.routing.allocation.include._id
diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/shrink/ResizeRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/shrink/ResizeRequest.java
@@ -56,7 +56,7 @@ public class ResizeRequest extends AcknowledgedRequest<ResizeRequest> implements
     private CreateIndexRequest targetIndexRequest;
     private String sourceIndex;
     private ResizeType type = ResizeType.SHRINK;
-    private boolean copySettings = false;
+    private boolean copySettings = true;
 
     ResizeRequest() {}
 
@@ -80,6 +80,9 @@ public ActionRequestValidationException validate() {
         if (type == ResizeType.SPLIT && IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.exists(targetIndexRequest.settings()) == false) {
             validationException = addValidationError("index.number_of_shards is required for split operations", validationException);
         }
+        if (copySettings == false) {
+            validationException = addValidationError("copySettings can not be set to [false]", validationException);
+        }
         return validationException;
     }
 

diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestResizeHandler.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestResizeHandler.java
@@ -57,6 +57,9 @@ public final RestChannelConsumer prepareRequest(final RestRequest request, final
                 copySettings = true;
             } else {
                 copySettings = Booleans.parseBoolean(rawCopySettings);
+                if (copySettings == false) {
+                    throw new IllegalArgumentException("copy_settings can not be set to [false]");
+                }
             }
         }
         resizeRequest.setCopySettings(copySettings);

diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/create/ShrinkIndexIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/create/ShrinkIndexIT.java
@@ -443,18 +443,20 @@ public void testCreateShrinkWithIndexSort() throws Exception {
         // check that index sort cannot be set on the target index
         IllegalArgumentException exc = expectThrows(IllegalArgumentException.class,
             () -> client().admin().indices().prepareResizeIndex("source", "target")
-                .setSettings(Settings.builder()
-                    .put("index.number_of_replicas", 0)
-                    .put("index.number_of_shards", "2")
-                    .put("index.sort.field", "foo")
-                    .build()).get());
+                    .setSettings(Settings.builder()
+                            .put("index.number_of_replicas", 0)
+                            .put("index.number_of_shards", "2")
+                            .put("index.blocks.write", (String) null)
+                            .put("index.sort.field", "foo")
+                            .build()).get());
         assertThat(exc.getMessage(), containsString("can't override index sort when resizing an index"));
 
         // check that the index sort order of `source` is correctly applied to the `target`
         assertAcked(client().admin().indices().prepareResizeIndex("source", "target")
-            .setSettings(Settings.builder()
-                .put("index.number_of_replicas", 0)
-                .put("index.number_of_shards", "2").build()).get());
+                .setSettings(Settings.builder()
+                        .put("index.number_of_replicas", 0)
+                        .put("index.number_of_shards", "2")
+                        .put("index.blocks.write", (String) null).build()).get());
         ensureGreen();
         flushAndRefresh();
         GetSettingsResponse settingsResponse =

diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/create/SplitIndexIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/create/SplitIndexIT.java
@@ -193,8 +193,9 @@ private void splitToN(int sourceShards, int firstSplitShards, int secondSplitSha
                 .put("index.blocks.write", true)).get();
         ensureGreen();
         Settings.Builder firstSplitSettingsBuilder = Settings.builder()
-            .put("index.number_of_replicas", 0)
-            .put("index.number_of_shards", firstSplitShards);
+                .put("index.number_of_replicas", 0)
+                .put("index.number_of_shards", firstSplitShards)
+                .put("index.blocks.write", (String) null);
         if (sourceShards == 1 && useRoutingPartition == false && randomBoolean()) { // try to set it if we have a source index with 1 shard
             firstSplitSettingsBuilder.put("index.number_of_routing_shards", secondSplitShards);
         }
@@ -225,10 +226,11 @@ private void splitToN(int sourceShards, int firstSplitShards, int secondSplitSha
         ensureGreen();
         // now split source into a new index
         assertAcked(client().admin().indices().prepareResizeIndex("first_split", "second_split")
-            .setResizeType(ResizeType.SPLIT)
-            .setSettings(Settings.builder()
-                .put("index.number_of_replicas", 0)
-                .put("index.number_of_shards", secondSplitShards).build()).get());
+                .setResizeType(ResizeType.SPLIT)
+                .setSettings(Settings.builder()
+                        .put("index.number_of_replicas", 0)
+                        .put("index.number_of_shards", secondSplitShards)
+                        .put("index.blocks.write", (String) null).build()).get());
         ensureGreen();
         assertHitCount(client().prepareSearch("second_split").setSize(100).setQuery(new TermsQueryBuilder("foo", "bar")).get(), numDocs);
         // let it be allocated anywhere and bump replicas
@@ -394,10 +396,11 @@ public void testCreateSplitIndex() {
 
             final boolean createWithReplicas = randomBoolean();
             assertAcked(client().admin().indices().prepareResizeIndex("source", "target")
-                .setResizeType(ResizeType.SPLIT)
-                .setSettings(Settings.builder()
-                    .put("index.number_of_replicas", createWithReplicas ? 1 : 0)
-                    .put("index.number_of_shards", 2).build()).get());
+                    .setResizeType(ResizeType.SPLIT)
+                    .setSettings(Settings.builder()
+                            .put("index.number_of_replicas", createWithReplicas ? 1 : 0)
+                            .put("index.number_of_shards", 2)
+                            .put("index.blocks.write", (String) null).build()).get());
             ensureGreen();
 
             final ClusterState state = client().admin().cluster().prepareState().get().getState();
@@ -431,8 +434,9 @@ public void testCreateSplitIndex() {
             if (createWithReplicas == false) {
                 // bump replicas
                 client().admin().indices().prepareUpdateSettings("target")
-                    .setSettings(Settings.builder()
-                        .put("index.number_of_replicas", 1)).get();
+                        .setSettings(Settings.builder()
+                                .put("index.number_of_replicas", 1)
+                                .put("index.blocks.write", (String) null)).get();
                 ensureGreen();
                 assertHitCount(client().prepareSearch("target").setSize(size).setQuery(new TermsQueryBuilder("foo", "bar")).get(), docs);
             }
@@ -496,21 +500,23 @@ public void testCreateSplitWithIndexSort() throws Exception {
 
         // check that index sort cannot be set on the target index
         IllegalArgumentException exc = expectThrows(IllegalArgumentException.class,
-            () -> client().admin().indices().prepareResizeIndex("source", "target")
-                .setResizeType(ResizeType.SPLIT)
-                .setSettings(Settings.builder()
-                    .put("index.number_of_replicas", 0)
-                    .put("index.number_of_shards", 4)
-                    .put("index.sort.field", "foo")
-                    .build()).get());
+                () -> client().admin().indices().prepareResizeIndex("source", "target")
+                        .setResizeType(ResizeType.SPLIT)
+                        .setSettings(Settings.builder()
+                                .put("index.number_of_replicas", 0)
+                                .put("index.number_of_shards", 4)
+                                .put("index.sort.field", "foo")
+                                .put("index.blocks.write", (String) null)
+                                .build()).get());
         assertThat(exc.getMessage(), containsString("can't override index sort when resizing an index"));
 
         // check that the index sort order of `source` is correctly applied to the `target`
         assertAcked(client().admin().indices().prepareResizeIndex("source", "target")
-            .setResizeType(ResizeType.SPLIT)
-            .setSettings(Settings.builder()
-                .put("index.number_of_replicas", 0)
-                .put("index.number_of_shards", 4).build()).get());
+                .setResizeType(ResizeType.SPLIT)
+                .setSettings(Settings.builder()
+                        .put("index.number_of_replicas", 0)
+                        .put("index.number_of_shards", 4)
+                        .put("index.blocks.write", (String) null).build()).get());
         ensureGreen();
         flushAndRefresh();
         GetSettingsResponse settingsResponse =

diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/shrink/ResizeRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/shrink/ResizeRequestTests.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.action.admin.indices.shrink;
 
+import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.admin.indices.alias.Alias;
 import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
 import org.elasticsearch.action.admin.indices.create.CreateIndexRequestTests;
@@ -29,14 +30,28 @@
 import org.elasticsearch.index.RandomCreateIndexGenerator;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.hamcrest.ElasticsearchAssertions;
+import org.hamcrest.collection.IsCollectionWithSize;
 
 import java.io.IOException;
 
 import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS;
 import static org.elasticsearch.common.xcontent.ToXContent.EMPTY_PARAMS;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasSize;
 
 public class ResizeRequestTests extends ESTestCase {
 
+    public void testCopySettingsValidation() {
+        ResizeRequest request = new ResizeRequest();
+        request.setSourceIndex("source");
+        request.setTargetIndex(new CreateIndexRequest("target"));
+        request.setCopySettings(false);
+        final ActionRequestValidationException e = request.validate();
+        assertNotNull(e);
+        assertThat(e.validationErrors(), hasSize(1));
+        assertThat(e.validationErrors().get(0), equalTo("copySettings can not be set to [false]"));
+    }
+
     public void testToXContent() throws IOException {
         {
             ResizeRequest request = new ResizeRequest("target", "source");

diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestResizeHandlerTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestResizeHandlerTests.java
@@ -29,34 +29,66 @@
 import java.io.IOException;
 import java.util.Collections;
 
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.hasToString;
 import static org.mockito.Mockito.mock;
 
 public class RestResizeHandlerTests extends ESTestCase {
 
     public void testShrinkCopySettingsDeprecated() throws IOException {
         final RestResizeHandler.RestShrinkIndexAction handler =
                 new RestResizeHandler.RestShrinkIndexAction(Settings.EMPTY, mock(RestController.class));
-        final String copySettings = randomFrom("true", "false");
+        final String copySettings = "true";
         final FakeRestRequest request =
                 new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY)
                         .withParams(Collections.singletonMap("copy_settings", copySettings))
                         .withPath("source/_shrink/target")
                         .build();
         handler.prepareRequest(request, mock(NodeClient.class));
-        assertWarnings("parameter [copy_settings] is deprecated but was [" + copySettings + "]");
+        assertWarnings("parameter [copy_settings] is deprecated but was [true]");
+    }
+
+    public void testShrinkCopySettingsFalse() {
+        final RestResizeHandler.RestShrinkIndexAction handler =
+                new RestResizeHandler.RestShrinkIndexAction(Settings.EMPTY, mock(RestController.class));
+        final String copySettings = "false";
+        final FakeRestRequest request =
+                new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY)
+                        .withParams(Collections.singletonMap("copy_settings", copySettings))
+                        .withPath("source/_split/target")
+                        .build();
+        final IllegalArgumentException e =
+                expectThrows(IllegalArgumentException.class, () -> handler.prepareRequest(request, mock(NodeClient.class)));
+        assertThat(e, hasToString(containsString("copy_settings can not be set to [false]")));
+        assertWarnings("parameter [copy_settings] is deprecated but was [false]");
     }
 
     public void testSplitCopySettingsDeprecated() throws IOException {
         final RestResizeHandler.RestSplitIndexAction handler =
                 new RestResizeHandler.RestSplitIndexAction(Settings.EMPTY, mock(RestController.class));
-        final String copySettings = randomFrom("true", "false");
+        final String copySettings = "true";
         final FakeRestRequest request =
                 new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY)
                         .withParams(Collections.singletonMap("copy_settings", copySettings))
                         .withPath("source/_split/target")
                         .build();
         handler.prepareRequest(request, mock(NodeClient.class));
-        assertWarnings("parameter [copy_settings] is deprecated but was [" + copySettings + "]");
+        assertWarnings("parameter [copy_settings] is deprecated but was [true]");
+    }
+
+    public void testSplitCopySettingsFalse() {
+        final RestResizeHandler.RestSplitIndexAction handler =
+                new RestResizeHandler.RestSplitIndexAction(Settings.EMPTY, mock(RestController.class));
+        final String copySettings = "false";
+        final FakeRestRequest request =
+                new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY)
+                        .withParams(Collections.singletonMap("copy_settings", copySettings))
+                        .withPath("source/_split/target")
+                        .build();
+        final IllegalArgumentException e =
+                expectThrows(IllegalArgumentException.class, () -> handler.prepareRequest(request, mock(NodeClient.class)));
+        assertThat(e, hasToString(containsString("copy_settings can not be set to [false]")));
+        assertWarnings("parameter [copy_settings] is deprecated but was [false]");
     }
 
 }