From 81e51bb7ec127b393e1103273ca24fdf169a3ebb Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Thu, 28 Jan 2021 09:01:17 +0100 Subject: [PATCH] Disallow composable index template that have both data stream and aliases definitions (#68070) Backport of #67886 to 7.11 branch. Index aliases are not allowed to refer to backing indices of data streams. Adding an alias that points to a backing index results into a validation error. However when defining aliases and a data stream definition on an index template, it is still possible to for aliases that refer to backing indices to be created when a data stream or new backing index is created. This change add additional validation that prevents defining aliases and data stream definition together in a composable index template or component templates that are referred by an composable index template. This should fix the mentioned bug. This checks only enables this validation when adding/updating a composable index and component template. There may be templates in the wild that have both data stream and aliases definitions, so we currently can't fail cluster states that contain aliases that refer to backing indices. So instead a warning header is emitted. Closes #67730 --- .../rollover/MetadataRolloverService.java | 2 +- .../TransportSimulateIndexTemplateAction.java | 2 +- .../cluster/metadata/Metadata.java | 25 +++++ .../metadata/MetadataCreateIndexService.java | 4 +- .../MetadataIndexTemplateService.java | 29 +++++- .../MetadataRolloverServiceTests.java | 96 +++++++++++++++++++ .../ComposableIndexTemplateTests.java | 2 +- .../MetadataIndexTemplateServiceTests.java | 34 ++++++- .../xpack/datastreams/DataStreamsRestIT.java | 11 +++ 9 files changed, 197 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java index 4d099c9251697..8d63e772196c3 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java @@ -319,7 +319,7 @@ static void checkNoDuplicatedAliasInIndexTemplate(Metadata metadata, String roll final String matchedV2Template = findV2Template(metadata, rolloverIndexName, isHidden == null ? false : isHidden); if (matchedV2Template != null) { - List> aliases = MetadataIndexTemplateService.resolveAliases(metadata, matchedV2Template); + List> aliases = MetadataIndexTemplateService.resolveAliases(metadata, matchedV2Template, false); for (Map aliasConfig : aliases) { if (aliasConfig.containsKey(rolloverRequestAlias)) { throw new IllegalArgumentException(String.format(Locale.ROOT, diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/template/post/TransportSimulateIndexTemplateAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/template/post/TransportSimulateIndexTemplateAction.java index d835a71b27ede..26bc4e7957457 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/template/post/TransportSimulateIndexTemplateAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/template/post/TransportSimulateIndexTemplateAction.java @@ -160,7 +160,7 @@ public static Template resolveTemplate(final String matchingTemplate, final Stri Settings settings = resolveSettings(simulatedState.metadata(), matchingTemplate); List> resolvedAliases = MetadataIndexTemplateService.resolveAliases(simulatedState.metadata(), - matchingTemplate); + matchingTemplate, true); // create the index with dummy settings in the cluster state so we can parse and validate the aliases Settings dummySettings = Settings.builder() diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java index ee9c5959914c9..8a09434416c0e 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java @@ -45,6 +45,7 @@ import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.logging.HeaderWarning; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; @@ -1576,6 +1577,30 @@ static void validateDataStreams(SortedMap indicesLooku " including '" + conflicts.iterator().next() + "'"); } } + + // Sanity check, because elsewhere a more user friendly error should have occurred: + List conflictingAliases = indicesLookup.values().stream() + .filter(ia -> ia.getType() == IndexAbstraction.Type.ALIAS) + .filter(ia -> { + for (IndexMetadata index : ia.getIndices()) { + if (indicesLookup.get(index.getIndex().getName()).getParentDataStream() != null) { + return true; + } + } + + return false; + }) + .map(IndexAbstraction::getName) + .collect(Collectors.toList()); + if (conflictingAliases.isEmpty() == false) { + // Nn 7.x there might be aliases that refer to backing indices of a data stream and + // throwing an exception here would avoid the cluster from functioning: + String warning = "aliases " + conflictingAliases + " cannot refer to backing indices of data streams"; + // log as debug, this method is executed each time a new cluster state is created and + // could result in many logs: + logger.debug(warning); + HeaderWarning.addWarning(warning); + } } } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java index a22cfb6045845..7ea32ba98a4da 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java @@ -526,10 +526,10 @@ private ClusterState applyCreateIndexRequestWithV2Template(final ClusterState cu return applyCreateIndexWithTemporaryService(currentState, request, silent, null, tmpImd, mappings, indexService -> resolveAndValidateAliases(request.index(), request.aliases(), - MetadataIndexTemplateService.resolveAliases(currentState.metadata(), templateName), currentState.metadata(), aliasValidator, + MetadataIndexTemplateService.resolveAliases(currentState.metadata(), templateName, false), currentState.metadata(), // the context is only used for validation so it's fine to pass fake values for the // shard id and the current timestamp - xContentRegistry, indexService.newQueryShardContext(0, null, () -> 0L, null, emptyMap())), + aliasValidator, xContentRegistry, indexService.newQueryShardContext(0, null, () -> 0L, null, emptyMap())), Collections.singletonList(templateName), metadataTransformer); } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java index e762a0decfb7a..25f76cd312a27 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java @@ -1017,8 +1017,20 @@ public static List> resolveAliases(final List> resolveAliases(final Metadata metadata, final String templateName) { + public static List> resolveAliases(final Metadata metadata, + final String templateName, + final boolean failIfTemplateHasDataStream) { final ComposableIndexTemplate template = metadata.templatesV2().get(templateName); assert template != null : "attempted to resolve aliases for a template [" + templateName + "] that did not exist in the cluster state"; @@ -1038,6 +1050,19 @@ public static List> resolveAliases(final Metadata met Optional.ofNullable(template.template()) .map(Template::aliases) .ifPresent(aliases::add); + + // A template that creates data streams can't also create aliases. + // (otherwise we end up with aliases pointing to backing indices of data streams) + if (aliases.size() > 0 && template.getDataStreamTemplate() != null) { + if (failIfTemplateHasDataStream) { + throw new IllegalArgumentException("template [" + templateName + "] has alias and data stream definitions"); + } else { + String warning = "template [" + templateName + "] has alias and data stream definitions"; + logger.warn(warning); + HeaderWarning.addWarning(warning); + } + } + // Aliases are applied in order, but subsequent alias configuration from the same name is // ignored, so in order for the order to be correct, alias configuration should be in order // of precedence (with the index template first) @@ -1089,7 +1114,7 @@ private static void validateCompositeTemplate(final ClusterState state, tempIndexService -> { // Validate aliases MetadataCreateIndexService.resolveAndValidateAliases(temporaryIndexName, Collections.emptySet(), - MetadataIndexTemplateService.resolveAliases(stateWithIndex.metadata(), templateName), stateWithIndex.metadata(), + MetadataIndexTemplateService.resolveAliases(stateWithIndex.metadata(), templateName, true), stateWithIndex.metadata(), new AliasValidator(), // the context is only used for validation so it's fine to pass fake values for the // shard id and the current timestamp diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java index b4fc73be9ef07..3086d9ba5b2c3 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java @@ -636,6 +636,102 @@ protected String contentType() { } } + public void testRolloverDataStreamWorksWithTemplateThatAlsoCreatesAliases() throws Exception { + final DataStream dataStream = DataStreamTestHelper.randomInstance() + // ensure no replicate data stream + .promoteDataStream(); + ComposableIndexTemplate template = new ComposableIndexTemplate( + Collections.singletonList(dataStream.getName() + "*"), + new Template(null, null, Collections.singletonMap("my-alias", AliasMetadata.newAliasMetadataBuilder("my-alias").build())), + null, + null, + null, + null, + new ComposableIndexTemplate.DataStreamTemplate(), + null + ); + Metadata.Builder builder = Metadata.builder(); + builder.put("template", template); + for (Index index : dataStream.getIndices()) { + builder.put(DataStreamTestHelper.getIndexMetadataBuilderForIndex(index)); + } + builder.put(dataStream); + final ClusterState clusterState = ClusterState.builder(new ClusterName("test")).metadata(builder).build(); + + ThreadPool testThreadPool = new TestThreadPool(getTestName()); + try { + DateFieldMapper dateFieldMapper + = new DateFieldMapper.Builder("@timestamp", DateFieldMapper.Resolution.MILLISECONDS, null, false, Version.CURRENT) + .build(new ContentPath()); + MappedFieldType mockedTimestampFieldType = mock(MappedFieldType.class); + when(mockedTimestampFieldType.name()).thenReturn("_data_stream_timestamp"); + MetadataFieldMapper mockedTimestampField = new MetadataFieldMapper(mockedTimestampFieldType) { + @Override + protected String contentType() { + return null; + } + }; + MappingLookup mappingLookup = new MappingLookup( + "_doc", + org.elasticsearch.common.collect.List.of(mockedTimestampField, dateFieldMapper), + org.elasticsearch.common.collect.List.of(), + org.elasticsearch.common.collect.List.of(), + org.elasticsearch.common.collect.List.of(), + 0, + null, + false + ); + ClusterService clusterService = ClusterServiceUtils.createClusterService(testThreadPool); + Environment env = mock(Environment.class); + when(env.sharedDataFile()).thenReturn(null); + AllocationService allocationService = mock(AllocationService.class); + when(allocationService.reroute(any(ClusterState.class), any(String.class))).then(i -> i.getArguments()[0]); + DocumentMapper documentMapper = mock(DocumentMapper.class); + when(documentMapper.mappers()).thenReturn(mappingLookup); + when(documentMapper.type()).thenReturn("_doc"); + CompressedXContent mapping = + new CompressedXContent("{\"_doc\":" + generateMapping(dataStream.getTimeStampField().getName(), "date") + "}"); + when(documentMapper.mappingSource()).thenReturn(mapping); + RoutingFieldMapper routingFieldMapper = mock(RoutingFieldMapper.class); + when(routingFieldMapper.required()).thenReturn(false); + when(documentMapper.routingFieldMapper()).thenReturn(routingFieldMapper); + IndicesService indicesService = mockIndicesServices(documentMapper); + IndexNameExpressionResolver mockIndexNameExpressionResolver = mock(IndexNameExpressionResolver.class); + when(mockIndexNameExpressionResolver.resolveDateMathExpression(any())).then(returnsFirstArg()); + + ShardLimitValidator shardLimitValidator = new ShardLimitValidator(Settings.EMPTY, clusterService); + MetadataCreateIndexService createIndexService = new MetadataCreateIndexService(Settings.EMPTY, + clusterService, indicesService, allocationService, new AliasValidator(), shardLimitValidator, env, + IndexScopedSettings.DEFAULT_SCOPED_SETTINGS, testThreadPool, null, + new SystemIndices(org.elasticsearch.common.collect.Map.of()), false); + MetadataIndexAliasesService indexAliasesService = new MetadataIndexAliasesService(clusterService, indicesService, + new AliasValidator(), null, xContentRegistry()); + MetadataRolloverService rolloverService = new MetadataRolloverService(testThreadPool, createIndexService, indexAliasesService, + mockIndexNameExpressionResolver); + + MaxDocsCondition condition = new MaxDocsCondition(randomNonNegativeLong()); + List> metConditions = Collections.singletonList(condition); + CreateIndexRequest createIndexRequest = new CreateIndexRequest("_na_"); + + // Ensure that a warning header is emitted + MetadataRolloverService.RolloverResult rolloverResult = + rolloverService.rolloverClusterState(clusterState, dataStream.getName(), null, createIndexRequest, metConditions, + randomBoolean(), false); + assertWarnings( + "aliases [my-alias] cannot refer to backing indices of data streams", + "template [template] has alias and data stream definitions" + ); + + // Just checking that the rollover was successful: + String sourceIndexName = DataStream.getDefaultBackingIndexName(dataStream.getName(), dataStream.getGeneration()); + String newIndexName = DataStream.getDefaultBackingIndexName(dataStream.getName(), dataStream.getGeneration() + 1); + assertEquals(sourceIndexName, rolloverResult.sourceIndexName); + assertEquals(newIndexName, rolloverResult.rolloverIndexName); + } finally { + testThreadPool.shutdown(); + } + } + public void testValidation() throws Exception { final String rolloverTarget; final String sourceIndexName; diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java index 10d805f780502..ab6af9b012b3b 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplateTests.java @@ -78,7 +78,7 @@ public static ComposableIndexTemplate randomInstance() { if (dataStreamTemplate != null || randomBoolean()) { mappings = randomMappings(dataStreamTemplate); } - if (randomBoolean()) { + if (dataStreamTemplate == null && randomBoolean()) { aliases = randomAliases(); } template = new Template(settings, mappings, aliases); diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java index dd79c954c1192..263cb9d633aa0 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java @@ -1170,12 +1170,44 @@ public void testResolveAliases() throws Exception { Arrays.asList("ct_low", "ct_high"), 0L, 1L, null, null, null); state = service.addIndexTemplateV2(state, true, "my-template", it); - List> resolvedAliases = MetadataIndexTemplateService.resolveAliases(state.metadata(), "my-template"); + List> resolvedAliases = + MetadataIndexTemplateService.resolveAliases(state.metadata(), "my-template", true); // These should be order of precedence, so the index template (a3), then ct_high (a1), then ct_low (a2) assertThat(resolvedAliases, equalTo(Arrays.asList(a3, a1, a2))); } + public void testResolveAliasesDataStreams() throws Exception { + Map a1 = new HashMap<>(); + a1.put("logs", AliasMetadata.newAliasMetadataBuilder("logs").build()); + + // index template can't have data streams and aliases + ComposableIndexTemplate it = new ComposableIndexTemplate(Collections.singletonList("logs-*"), + new Template(null, null, a1), null, 0L, 1L, null, new ComposableIndexTemplate.DataStreamTemplate(), null); + ClusterState state1 = ClusterState.builder(ClusterState.EMPTY_STATE) + .metadata(Metadata.builder().put("1", it).build()) + .build(); + Exception e = + expectThrows(IllegalArgumentException.class, () -> MetadataIndexTemplateService.resolveAliases(state1.metadata(), "1", true)); + assertThat(e.getMessage(), equalTo("template [1] has alias and data stream definitions")); + // Ignoring validation + assertThat(MetadataIndexTemplateService.resolveAliases(state1.metadata(), "1", false), equalTo(Collections.singletonList(a1))); + assertWarnings("template [1] has alias and data stream definitions"); + + // index template can't have data streams and a component template with an aliases + ComponentTemplate componentTemplate = new ComponentTemplate(new Template(null, null, a1), null, null); + it = new ComposableIndexTemplate(Collections.singletonList("logs-*"), null, Collections.singletonList("c1"), 0L, 1L, null, + new ComposableIndexTemplate.DataStreamTemplate(), null); + ClusterState state2 = ClusterState.builder(ClusterState.EMPTY_STATE) + .metadata(Metadata.builder().put("1", it).put("c1", componentTemplate).build()) + .build(); + e = expectThrows(IllegalArgumentException.class, () -> MetadataIndexTemplateService.resolveAliases(state2.metadata(), "1", true)); + assertThat(e.getMessage(), equalTo("template [1] has alias and data stream definitions")); + // Ignoring validation + assertThat(MetadataIndexTemplateService.resolveAliases(state2.metadata(), "1", false), equalTo(Collections.singletonList(a1))); + assertWarnings("template [1] has alias and data stream definitions"); + } + public void testAddInvalidTemplate() throws Exception { ComposableIndexTemplate template = new ComposableIndexTemplate(Collections.singletonList("a"), null, Arrays.asList("good", "bad"), null, null, null); diff --git a/x-pack/plugin/data-streams/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/datastreams/DataStreamsRestIT.java b/x-pack/plugin/data-streams/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/datastreams/DataStreamsRestIT.java index 5bfdd29edef4d..768c3422db1f8 100644 --- a/x-pack/plugin/data-streams/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/datastreams/DataStreamsRestIT.java +++ b/x-pack/plugin/data-streams/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/datastreams/DataStreamsRestIT.java @@ -8,6 +8,7 @@ import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; +import org.elasticsearch.client.ResponseException; import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.test.rest.ESRestTestCase; import org.junit.After; @@ -16,6 +17,8 @@ import java.util.Collections; import java.util.Map; +import static org.hamcrest.Matchers.containsString; + public class DataStreamsRestIT extends ESRestTestCase { @After @@ -77,4 +80,12 @@ public void testHiddenDataStreamImplicitHiddenSearch() throws IOException { Map results = entityAsMap(response); assertEquals(1, XContentMapValues.extractValue("hits.total.value", results)); } + + public void testAddingIndexTemplateWithAliasesAndDataStream() { + Request putComposableIndexTemplateRequest = new Request("PUT", "/_index_template/my-template"); + String body = "{\"index_patterns\":[\"mypattern*\"],\"data_stream\":{},\"template\":{\"aliases\":{\"my-alias\":{}}}}"; + putComposableIndexTemplateRequest.setJsonEntity(body); + Exception e = expectThrows(ResponseException.class, () -> client().performRequest(putComposableIndexTemplateRequest)); + assertThat(e.getMessage(), containsString("template [my-template] has alias and data stream definitions")); + } }