From 5d1bbb6a6bd3a368ec4bd8879aca18a404cf6b0e Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Mon, 8 Jan 2018 09:39:34 -0500 Subject: [PATCH] Fail rollover if duplicated alias found in template (#28110) If a newly created index from a rollover request matches with an index template whose aliases contains the rollover request alias, the alias will point to multiple indices. This will cause indexing requests to be rejected. To avoid such situation, we make sure that there is no duplicated alias before creating a new index; otherwise abort and report an error to the caller. Closes #26976 --- .../rollover/TransportRolloverAction.java | 18 ++++++++++++++++++ .../metadata/MetaDataCreateIndexService.java | 18 +----------------- .../MetaDataIndexTemplateService.java | 19 +++++++++++++++++++ .../admin/indices/rollover/RolloverIT.java | 11 +++++++++++ .../TransportRolloverActionTests.java | 16 ++++++++++++++++ .../MetaDataIndexTemplateServiceTests.java | 19 +++++++++++++++++-- 6 files changed, 82 insertions(+), 19 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java b/core/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java index 48d03f3ac94c6..2ed5192e6cfb2 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java +++ b/core/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java @@ -37,9 +37,11 @@ import org.elasticsearch.cluster.metadata.AliasOrIndex; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.metadata.IndexTemplateMetaData; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.metadata.MetaDataCreateIndexService; import org.elasticsearch.cluster.metadata.MetaDataIndexAliasesService; +import org.elasticsearch.cluster.metadata.MetaDataIndexTemplateService; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; @@ -115,6 +117,7 @@ protected void masterOperation(final RolloverRequest rolloverRequest, final Clus : generateRolloverIndexName(sourceProvidedName, indexNameExpressionResolver); final String rolloverIndexName = indexNameExpressionResolver.resolveDateMathExpression(unresolvedName); MetaDataCreateIndexService.validateIndexName(rolloverIndexName, state); // will fail if the index already exists + checkNoDuplicatedAliasInIndexTemplate(metaData, rolloverIndexName, rolloverRequest.getAlias()); client.admin().indices().prepareStats(sourceIndexName).clear().setDocs(true).execute( new ActionListener() { @Override @@ -238,4 +241,19 @@ static CreateIndexClusterStateUpdateRequest prepareCreateIndexRequest(final Stri .mappings(createIndexRequest.mappings()); } + /** + * If the newly created index matches with an index template whose aliases contains the rollover alias, + * the rollover alias will point to multiple indices. This causes indexing requests to be rejected. + * To avoid this, we make sure that there is no duplicated alias in index templates before creating a new index. + */ + static void checkNoDuplicatedAliasInIndexTemplate(MetaData metaData, String rolloverIndexName, String rolloverRequestAlias) { + final List matchedTemplates = MetaDataIndexTemplateService.findTemplates(metaData, rolloverIndexName); + for (IndexTemplateMetaData template : matchedTemplates) { + if (template.aliases().containsKey(rolloverRequestAlias)) { + throw new IllegalArgumentException(String.format(Locale.ROOT, + "Rollover alias [%s] can point to multiple indices, found duplicated alias [%s] in index template [%s]", + rolloverRequestAlias, template.aliases().keys(), template.name())); + } + } + } } diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java index 14839ce65e39b..d394c2c7d1479 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java @@ -282,7 +282,7 @@ public ClusterState execute(ClusterState currentState) throws Exception { // we only find a template when its an API call (a new index) // find templates, highest order are better matching - List templates = findTemplates(request, currentState); + List templates = MetaDataIndexTemplateService.findTemplates(currentState.metaData(), request.index()); Map customs = new HashMap<>(); @@ -561,22 +561,6 @@ public void onFailure(String source, Exception e) { } super.onFailure(source, e); } - - private List findTemplates(CreateIndexClusterStateUpdateRequest request, ClusterState state) throws IOException { - List templateMetadata = new ArrayList<>(); - for (ObjectCursor cursor : state.metaData().templates().values()) { - IndexTemplateMetaData metadata = cursor.value; - for (String template: metadata.patterns()) { - if (Regex.simpleMatch(template, request.index())) { - templateMetadata.add(metadata); - break; - } - } - } - - CollectionUtil.timSort(templateMetadata, Comparator.comparingInt(IndexTemplateMetaData::order).reversed()); - return templateMetadata; - } } private void validate(CreateIndexClusterStateUpdateRequest request, ClusterState state) { diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexTemplateService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexTemplateService.java index 883d7f2fc47ec..9d8da37cbeeba 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexTemplateService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexTemplateService.java @@ -20,6 +20,7 @@ import com.carrotsearch.hppc.cursors.ObjectCursor; +import org.apache.lucene.util.CollectionUtil; import org.elasticsearch.Version; import org.elasticsearch.action.admin.indices.alias.Alias; import org.elasticsearch.action.support.master.MasterNodeRequest; @@ -48,6 +49,7 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -193,6 +195,23 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS }); } + /** + * Finds index templates whose index pattern matched with the given index name. + * The result is sorted by {@link IndexTemplateMetaData#order} descending. + */ + public static List findTemplates(MetaData metaData, String indexName) { + final List matchedTemplates = new ArrayList<>(); + for (ObjectCursor cursor : metaData.templates().values()) { + final IndexTemplateMetaData template = cursor.value; + final boolean matched = template.patterns().stream().anyMatch(pattern -> Regex.simpleMatch(pattern, indexName)); + if (matched) { + matchedTemplates.add(template); + } + } + CollectionUtil.timSort(matchedTemplates, Comparator.comparingInt(IndexTemplateMetaData::order).reversed()); + return matchedTemplates; + } + private static void validateAndAddTemplate(final PutRequest request, IndexTemplateMetaData.Builder templateBuilder, IndicesService indicesService, NamedXContentRegistry xContentRegistry) throws Exception { Index createdIndex = null; diff --git a/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java b/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java index c047611f71932..e2c31db81ce60 100644 --- a/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java +++ b/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverIT.java @@ -277,4 +277,15 @@ public void testRolloverMaxSize() throws Exception { assertThat("No rollover with an empty index", response.isRolledOver(), equalTo(false)); } } + + public void testRejectIfAliasFoundInTemplate() throws Exception { + client().admin().indices().preparePutTemplate("logs") + .setPatterns(Collections.singletonList("logs-*")).addAlias(new Alias("logs-write")).get(); + assertAcked(client().admin().indices().prepareCreate("logs-000001").get()); + ensureYellow("logs-write"); + final IllegalArgumentException error = expectThrows(IllegalArgumentException.class, + () -> client().admin().indices().prepareRolloverIndex("logs-write").addMaxIndexSizeCondition(new ByteSizeValue(1)).get()); + assertThat(error.getMessage(), equalTo( + "Rollover alias [logs-write] can point to multiple indices, found duplicated alias [[logs-write]] in index template [logs]")); + } } diff --git a/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java b/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java index dcb3a87df74f4..3366646e24a79 100644 --- a/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java +++ b/core/src/test/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverActionTests.java @@ -29,6 +29,7 @@ import org.elasticsearch.cluster.metadata.AliasMetaData; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.metadata.IndexTemplateMetaData; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.settings.Settings; @@ -40,11 +41,13 @@ import org.elasticsearch.test.ESTestCase; import org.mockito.ArgumentCaptor; +import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.Set; import static org.elasticsearch.action.admin.indices.rollover.TransportRolloverAction.evaluateConditions; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.mockito.Matchers.any; @@ -241,6 +244,19 @@ public void testCreateIndexRequest() throws Exception { assertThat(createIndexRequest.cause(), equalTo("rollover_index")); } + public void testRejectDuplicateAlias() throws Exception { + final IndexTemplateMetaData template = IndexTemplateMetaData.builder("test-template") + .patterns(Arrays.asList("foo-*", "bar-*")) + .putAlias(AliasMetaData.builder("foo-write")).putAlias(AliasMetaData.builder("bar-write")) + .build(); + final MetaData metaData = MetaData.builder().put(createMetaData(), false).put(template).build(); + String indexName = randomFrom("foo-123", "bar-xyz"); + String aliasName = randomFrom("foo-write", "bar-write"); + final IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, + () -> TransportRolloverAction.checkNoDuplicatedAliasInIndexTemplate(metaData, indexName, aliasName)); + assertThat(ex.getMessage(), containsString("index template [test-template]")); + } + private IndicesStatsResponse createIndicesStatResponse(long totalDocs, long primaryDocs) { final CommonStats primaryStats = mock(CommonStats.class); when(primaryStats.getDocs()).thenReturn(new DocsStats(primaryDocs, 0, between(1, 10000))); diff --git a/core/src/test/java/org/elasticsearch/action/admin/indices/template/put/MetaDataIndexTemplateServiceTests.java b/core/src/test/java/org/elasticsearch/action/admin/indices/template/put/MetaDataIndexTemplateServiceTests.java index 58012909b8f2e..d3c133915e7b8 100644 --- a/core/src/test/java/org/elasticsearch/action/admin/indices/template/put/MetaDataIndexTemplateServiceTests.java +++ b/core/src/test/java/org/elasticsearch/action/admin/indices/template/put/MetaDataIndexTemplateServiceTests.java @@ -20,8 +20,10 @@ package org.elasticsearch.action.admin.indices.template.put; import org.elasticsearch.action.admin.indices.alias.Alias; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.AliasValidator; import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.cluster.metadata.IndexTemplateMetaData; import org.elasticsearch.cluster.metadata.MetaDataCreateIndexService; import org.elasticsearch.cluster.metadata.MetaDataIndexTemplateService; import org.elasticsearch.cluster.metadata.MetaDataIndexTemplateService.PutRequest; @@ -38,16 +40,17 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.concurrent.CountDownLatch; +import java.util.stream.Collectors; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.empty; public class MetaDataIndexTemplateServiceTests extends ESSingleNodeTestCase { public void testIndexTemplateInvalidNumberOfShards() { @@ -154,6 +157,18 @@ public void testAliasInvalidFilterInvalidJson() throws Exception { assertThat(errors.get(0).getMessage(), equalTo("failed to parse filter for alias [invalid_alias]")); } + public void testFindTemplates() throws Exception { + client().admin().indices().prepareDeleteTemplate("*").get(); // Delete all existing templates + putTemplateDetail(new PutRequest("test", "foo-1").patterns(Arrays.asList("foo-*")).order(1)); + putTemplateDetail(new PutRequest("test", "foo-2").patterns(Arrays.asList("foo-*")).order(2)); + putTemplateDetail(new PutRequest("test", "bar").patterns(Arrays.asList("bar-*")).order(between(0, 100))); + final ClusterState state = client().admin().cluster().prepareState().get().getState(); + assertThat(MetaDataIndexTemplateService.findTemplates(state.metaData(), "foo-1234").stream() + .map(IndexTemplateMetaData::name).collect(Collectors.toList()), contains("foo-2", "foo-1")); + assertThat(MetaDataIndexTemplateService.findTemplates(state.metaData(), "bar-xyz").stream() + .map(IndexTemplateMetaData::name).collect(Collectors.toList()), contains("bar")); + assertThat(MetaDataIndexTemplateService.findTemplates(state.metaData(), "baz"), empty()); + } private static List putTemplate(NamedXContentRegistry xContentRegistry, PutRequest request) { MetaDataCreateIndexService createIndexService = new MetaDataCreateIndexService(