Skip to content

Commit

Permalink
Fail rollover if duplicated alias found in template (#28110)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dnhatn authored Jan 8, 2018
1 parent 04ce0e7 commit 4963699
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<IndicesStatsResponse>() {
@Override
Expand Down Expand Up @@ -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<IndexTemplateMetaData> 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()));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,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<IndexTemplateMetaData> templates = findTemplates(request, currentState);
List<IndexTemplateMetaData> templates = MetaDataIndexTemplateService.findTemplates(currentState.metaData(), request.index());

Map<String, Custom> customs = new HashMap<>();

Expand Down Expand Up @@ -564,22 +564,6 @@ public void onFailure(String source, Exception e) {
}
super.onFailure(source, e);
}

private List<IndexTemplateMetaData> findTemplates(CreateIndexClusterStateUpdateRequest request, ClusterState state) throws IOException {
List<IndexTemplateMetaData> templateMetadata = new ArrayList<>();
for (ObjectCursor<IndexTemplateMetaData> 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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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<IndexTemplateMetaData> findTemplates(MetaData metaData, String indexName) {
final List<IndexTemplateMetaData> matchedTemplates = new ArrayList<>();
for (ObjectCursor<IndexTemplateMetaData> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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() {
Expand Down Expand Up @@ -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<Throwable> putTemplate(NamedXContentRegistry xContentRegistry, PutRequest request) {
MetaDataCreateIndexService createIndexService = new MetaDataCreateIndexService(
Expand Down

0 comments on commit 4963699

Please sign in to comment.