Skip to content

Commit

Permalink
Reject follow request if following setting not enabled on follower (#…
Browse files Browse the repository at this point in the history 
…32448)

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
  • Loading branch information
@dnhatn
dnhatn committed Aug 2, 2018
1 parent 81cbbcb commit 6110316
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 49 deletions.
5 changes: 4 additions & 1 deletion x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/FollowIndexAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
27 changes: 27 additions & 0 deletions x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/ShardChangesIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down
115 changes: 67 additions & 48 deletions ...k/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/FollowIndexActionTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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);
Expand All @@ -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();
}
Expand Down

0 comments on commit 6110316

Please sign in to comment.