Reject follow request if following setting not enabled on follower (#32448)

dnhatn · dnhatn · commit 611031661357 · 2018-08-01T23:00:50.000-04:00
Today we do not check if the `following_index` setting of the follower is enabled or not when processing a follow-request. If that setting is disabled, the follower will use the default engine, not the following engine. This change checks and rejects such invalid follow requests. Relates #30086
diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/FollowIndexAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/FollowIndexAction.java
@@ -499,7 +499,10 @@ static void validate(Request request, IndexMetaData leaderIndex, IndexMetaData f
         if (leaderIndex.getState() != IndexMetaData.State.OPEN || followIndex.getState() != IndexMetaData.State.OPEN) {
             throw new IllegalArgumentException("leader and follow index must be open");
         }
-
+        if (CcrSettings.CCR_FOLLOWING_INDEX_SETTING.get(followIndex.getSettings()) == false) {
+            throw new IllegalArgumentException("the following index [" + request.followerIndex + "] is not ready " +
+                "to follow; the setting [" + CcrSettings.CCR_FOLLOWING_INDEX_SETTING.getKey() + "] must be enabled.");
+        }
         // Make a copy, remove settings that are allowed to be different and then compare if the settings are equal.
         Settings leaderSettings = filter(leaderIndex.getSettings());
         Settings followerSettings = filter(followIndex.getSettings());
diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/ShardChangesIT.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/ShardChangesIT.java
@@ -59,6 +59,7 @@
 import static java.util.Collections.singletonMap;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
@@ -426,6 +427,32 @@ public void testFollowNonExistentIndex() throws Exception {
         expectThrows(IllegalArgumentException.class, () -> client().execute(FollowIndexAction.INSTANCE, followRequest3).actionGet());
     }
 
+    public void testValidateFollowingIndexSettings() throws Exception {
+        assertAcked(client().admin().indices().prepareCreate("test-leader")
+            .setSettings(Settings.builder().put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), true)));
+        // TODO: indexing should be optional but the current mapping logic requires for now.
+        client().prepareIndex("test-leader", "doc", "id").setSource("{\"f\": \"v\"}", XContentType.JSON).get();
+        assertAcked(client().admin().indices().prepareCreate("test-follower").get());
+        IllegalArgumentException followError = expectThrows(IllegalArgumentException.class, () -> client().execute(
+            FollowIndexAction.INSTANCE, createFollowRequest("test-leader", "test-follower")).actionGet());
+        assertThat(followError.getMessage(), equalTo("the following index [test-follower] is not ready to follow;" +
+            " the setting [index.xpack.ccr.following_index] must be enabled."));
+        // updating the `following_index` with an open index must not be allowed.
+        IllegalArgumentException updateError = expectThrows(IllegalArgumentException.class, () -> {
+            client().admin().indices().prepareUpdateSettings("test-follower")
+                .setSettings(Settings.builder().put(CcrSettings.CCR_FOLLOWING_INDEX_SETTING.getKey(), true)).get();
+        });
+        assertThat(updateError.getMessage(), containsString("Can't update non dynamic settings " +
+            "[[index.xpack.ccr.following_index]] for open indices [[test-follower/"));
+        assertAcked(client().admin().indices().prepareClose("test-follower"));
+        assertAcked(client().admin().indices().prepareUpdateSettings("test-follower")
+            .setSettings(Settings.builder().put(CcrSettings.CCR_FOLLOWING_INDEX_SETTING.getKey(), true)));
+        assertAcked(client().admin().indices().prepareOpen("test-follower"));
+        assertAcked(client().execute(FollowIndexAction.INSTANCE,
+            createFollowRequest("test-leader", "test-follower")).actionGet());
+        unfollowIndex("test-follower");
+    }
+
     public void testFollowIndex_lowMaxTranslogBytes() throws Exception {
         final String leaderIndexSettings = getIndexSettings(1, singletonMap(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true"));
         assertAcked(client().admin().indices().prepareCreate("index1").setSource(leaderIndexSettings, XContentType.JSON));
diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/FollowIndexActionTests.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/FollowIndexActionTests.java
@@ -8,12 +8,12 @@
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.cluster.metadata.IndexMetaData.State;
-import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.MapperTestUtils;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.xpack.ccr.CcrSettings;
 import org.elasticsearch.xpack.ccr.ShardChangesIT;
 
 import java.io.IOException;
@@ -31,42 +31,44 @@ public void testValidation() throws IOException {
         }
         {
             // should fail, because follow index does not exist
-            IndexMetaData leaderIMD = createIMD("index1", 5);
+            IndexMetaData leaderIMD = createIMD("index1", 5, Settings.EMPTY);
             Exception e = expectThrows(IllegalArgumentException.class, () -> FollowIndexAction.validate(request, leaderIMD, null, null));
             assertThat(e.getMessage(), equalTo("follow index [index2] does not exist"));
         }
         {
             // should fail because leader index does not have soft deletes enabled
-            IndexMetaData leaderIMD = createIMD("index1", 5);
-            IndexMetaData followIMD = createIMD("index2", 5);
+            IndexMetaData leaderIMD = createIMD("index1", 5, Settings.EMPTY);
+            IndexMetaData followIMD = createIMD("index2", 5, Settings.EMPTY);
             Exception e = expectThrows(IllegalArgumentException.class,
                 () -> FollowIndexAction.validate(request, leaderIMD, followIMD, null));
             assertThat(e.getMessage(), equalTo("leader index [index1] does not have soft deletes enabled"));
         }
         {
             // should fail because the number of primary shards between leader and follow index are not equal
-            IndexMetaData leaderIMD = createIMD("index1", 5, new Tuple<>(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true"));
-            IndexMetaData followIMD = createIMD("index2", 4);
+            IndexMetaData leaderIMD = createIMD("index1", 5, Settings.builder()
+                .put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true").build());
+            IndexMetaData followIMD = createIMD("index2", 4, Settings.EMPTY);
             Exception e = expectThrows(IllegalArgumentException.class,
                 () -> FollowIndexAction.validate(request, leaderIMD, followIMD, null));
             assertThat(e.getMessage(),
                 equalTo("leader index primary shards [5] does not match with the number of shards of the follow index [4]"));
         }
         {
             // should fail, because leader index is closed
-            IndexMetaData leaderIMD = createIMD("index1", State.CLOSE, "{}", 5,
-                new Tuple<>(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true"));
-            IndexMetaData followIMD = createIMD("index2", State.OPEN, "{}", 5,
-                new Tuple<>(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true"));
+            IndexMetaData leaderIMD = createIMD("index1", State.CLOSE, "{}", 5, Settings.builder()
+                .put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true").build());
+            IndexMetaData followIMD = createIMD("index2", State.OPEN, "{}", 5, Settings.builder()
+                .put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true").build());
             Exception e = expectThrows(IllegalArgumentException.class,
                 () -> FollowIndexAction.validate(request, leaderIMD, followIMD, null));
             assertThat(e.getMessage(), equalTo("leader and follow index must be open"));
         }
         {
             // should fail, because leader has a field with the same name mapped as keyword and follower as text
             IndexMetaData leaderIMD = createIMD("index1", State.OPEN, "{\"properties\": {\"field\": {\"type\": \"keyword\"}}}", 5,
-                new Tuple<>(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true"));
-            IndexMetaData followIMD = createIMD("index2", State.OPEN, "{\"properties\": {\"field\": {\"type\": \"text\"}}}", 5);
+                Settings.builder().put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true").build());
+            IndexMetaData followIMD = createIMD("index2", State.OPEN, "{\"properties\": {\"field\": {\"type\": \"text\"}}}", 5,
+                Settings.builder().put(CcrSettings.CCR_FOLLOWING_INDEX_SETTING.getKey(), true).build());
             MapperService mapperService = MapperTestUtils.newMapperService(xContentRegistry(), createTempDir(), Settings.EMPTY, "index2");
             mapperService.updateMapping(followIMD);
             Exception e = expectThrows(IllegalArgumentException.class,
@@ -76,35 +78,54 @@ public void testValidation() throws IOException {
         {
             // should fail because of non whitelisted settings not the same between leader and follow index
             String mapping = "{\"properties\": {\"field\": {\"type\": \"text\", \"analyzer\": \"my_analyzer\"}}}";
-            IndexMetaData leaderIMD = createIMD("index1", State.OPEN, mapping, 5,
-                new Tuple<>(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true"),
-                new Tuple<>("index.analysis.analyzer.my_analyzer.type", "custom"),
-                new Tuple<>("index.analysis.analyzer.my_analyzer.tokenizer", "whitespace"));
-            IndexMetaData followIMD = createIMD("index2", State.OPEN, mapping, 5,
-                new Tuple<>("index.analysis.analyzer.my_analyzer.type", "custom"),
-                new Tuple<>("index.analysis.analyzer.my_analyzer.tokenizer", "standard"));
+            IndexMetaData leaderIMD = createIMD("index1", State.OPEN, mapping, 5, Settings.builder()
+                .put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true")
+                .put("index.analysis.analyzer.my_analyzer.type", "custom")
+                .put("index.analysis.analyzer.my_analyzer.tokenizer", "whitespace").build());
+            IndexMetaData followIMD = createIMD("index2", State.OPEN, mapping, 5, Settings.builder()
+                .put(CcrSettings.CCR_FOLLOWING_INDEX_SETTING.getKey(), true)
+                .put("index.analysis.analyzer.my_analyzer.type", "custom")
+                .put("index.analysis.analyzer.my_analyzer.tokenizer", "standard").build());
             Exception e = expectThrows(IllegalArgumentException.class,
                 () -> FollowIndexAction.validate(request, leaderIMD, followIMD, null));
             assertThat(e.getMessage(), equalTo("the leader and follower index settings must be identical"));
         }
+        {
+            // should fail because the following index does not have the following_index settings
+            IndexMetaData leaderIMD = createIMD("index1", 5,
+                Settings.builder().put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true").build());
+            Settings followingIndexSettings = randomBoolean() ? Settings.EMPTY :
+                Settings.builder().put(CcrSettings.CCR_FOLLOWING_INDEX_SETTING.getKey(), false).build();
+            IndexMetaData followIMD = createIMD("index2", 5, followingIndexSettings);
+            MapperService mapperService = MapperTestUtils.newMapperService(xContentRegistry(), createTempDir(),
+                followingIndexSettings, "index2");
+            mapperService.updateMapping(followIMD);
+            IllegalArgumentException error = expectThrows(IllegalArgumentException.class,
+                () -> FollowIndexAction.validate(request, leaderIMD, followIMD, mapperService));
+            assertThat(error.getMessage(), equalTo("the following index [index2] is not ready to follow; " +
+                "the setting [index.xpack.ccr.following_index] must be enabled."));
+        }
         {
             // should succeed
-            IndexMetaData leaderIMD = createIMD("index1", 5, new Tuple<>(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true"));
-            IndexMetaData followIMD = createIMD("index2", 5);
+            IndexMetaData leaderIMD = createIMD("index1", 5, Settings.builder()
+                .put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true").build());
+            IndexMetaData followIMD = createIMD("index2", 5, Settings.builder()
+                .put(CcrSettings.CCR_FOLLOWING_INDEX_SETTING.getKey(), true).build());
             MapperService mapperService = MapperTestUtils.newMapperService(xContentRegistry(), createTempDir(), Settings.EMPTY, "index2");
             mapperService.updateMapping(followIMD);
             FollowIndexAction.validate(request, leaderIMD, followIMD, mapperService);
         }
         {
             // should succeed, index settings are identical
             String mapping = "{\"properties\": {\"field\": {\"type\": \"text\", \"analyzer\": \"my_analyzer\"}}}";
-            IndexMetaData leaderIMD = createIMD("index1", State.OPEN, mapping, 5,
-                new Tuple<>(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true"),
-                new Tuple<>("index.analysis.analyzer.my_analyzer.type", "custom"),
-                new Tuple<>("index.analysis.analyzer.my_analyzer.tokenizer", "standard"));
-            IndexMetaData followIMD = createIMD("index2", State.OPEN, mapping, 5,
-                new Tuple<>("index.analysis.analyzer.my_analyzer.type", "custom"),
-                new Tuple<>("index.analysis.analyzer.my_analyzer.tokenizer", "standard"));
+            IndexMetaData leaderIMD = createIMD("index1", State.OPEN, mapping, 5, Settings.builder()
+                .put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true")
+                .put("index.analysis.analyzer.my_analyzer.type", "custom")
+                .put("index.analysis.analyzer.my_analyzer.tokenizer", "standard").build());
+            IndexMetaData followIMD = createIMD("index2", State.OPEN, mapping, 5, Settings.builder()
+                .put(CcrSettings.CCR_FOLLOWING_INDEX_SETTING.getKey(), true)
+                .put("index.analysis.analyzer.my_analyzer.type", "custom")
+                .put("index.analysis.analyzer.my_analyzer.tokenizer", "standard").build());
             MapperService mapperService = MapperTestUtils.newMapperService(xContentRegistry(), createTempDir(),
                 followIMD.getSettings(), "index2");
             mapperService.updateMapping(followIMD);
@@ -113,37 +134,35 @@ public void testValidation() throws IOException {
         {
             // should succeed despite whitelisted settings being different
             String mapping = "{\"properties\": {\"field\": {\"type\": \"text\", \"analyzer\": \"my_analyzer\"}}}";
-            IndexMetaData leaderIMD = createIMD("index1", State.OPEN, mapping, 5,
-                new Tuple<>(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true"),
-                new Tuple<>(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "1s"),
-                new Tuple<>("index.analysis.analyzer.my_analyzer.type", "custom"),
-                new Tuple<>("index.analysis.analyzer.my_analyzer.tokenizer", "standard"));
-            IndexMetaData followIMD = createIMD("index2", State.OPEN, mapping, 5,
-                new Tuple<>(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "10s"),
-                new Tuple<>("index.analysis.analyzer.my_analyzer.type", "custom"),
-                new Tuple<>("index.analysis.analyzer.my_analyzer.tokenizer", "standard"));
+            IndexMetaData leaderIMD = createIMD("index1", State.OPEN, mapping, 5, Settings.builder()
+                .put(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true")
+                .put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "1s")
+                .put("index.analysis.analyzer.my_analyzer.type", "custom")
+                .put("index.analysis.analyzer.my_analyzer.tokenizer", "standard").build());
+            IndexMetaData followIMD = createIMD("index2", State.OPEN, mapping, 5, Settings.builder()
+                .put(CcrSettings.CCR_FOLLOWING_INDEX_SETTING.getKey(), true)
+                .put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "10s")
+                .put("index.analysis.analyzer.my_analyzer.type", "custom")
+                .put("index.analysis.analyzer.my_analyzer.tokenizer", "standard").build());
             MapperService mapperService = MapperTestUtils.newMapperService(xContentRegistry(), createTempDir(),
                 followIMD.getSettings(), "index2");
             mapperService.updateMapping(followIMD);
             FollowIndexAction.validate(request, leaderIMD, followIMD, mapperService);
         }
     }
 
-    private static IndexMetaData createIMD(String index, int numShards, Tuple<?, ?>... settings) throws IOException {
-        return createIMD(index, State.OPEN, "{\"properties\": {}}", numShards, settings);
+    private static IndexMetaData createIMD(String index, int numberOfShards, Settings settings) throws IOException {
+        return createIMD(index, State.OPEN, "{\"properties\": {}}", numberOfShards, settings);
     }
 
-    private static IndexMetaData createIMD(String index, State state, String mapping, int numShards,
-                                           Tuple<?, ?>... settings) throws IOException {
-        Settings.Builder settingsBuilder = settings(Version.CURRENT);
-        for (Tuple<?, ?> setting : settings) {
-            settingsBuilder.put((String) setting.v1(), (String) setting.v2());
-        }
-        return IndexMetaData.builder(index).settings(settingsBuilder)
-            .numberOfShards(numShards)
+    private static IndexMetaData createIMD(String index, State state, String mapping, int numberOfShards,
+                                           Settings settings) throws IOException {
+        return IndexMetaData.builder(index)
+            .settings(settings(Version.CURRENT).put(settings))
+            .numberOfShards(numberOfShards)
             .state(state)
             .numberOfReplicas(0)
-            .setRoutingNumShards(numShards)
+            .setRoutingNumShards(numberOfShards)
             .putMapping("_doc", mapping)
             .build();
     }
