From 0cdcc8c6b3a8d1e4edc379170683f3cef52d20c4 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Fri, 27 Sep 2024 20:07:59 +0200 Subject: [PATCH] synthetic source index setting provider should check source field mapper (#113522) Adds logic to SyntheticSourceIndexSettingsProvider to check source field mapper to whether synthetic source is enabled. Before this change the SyntheticSourceIndexSettingsProvider would only check the index.mode setting, which doesn't cover all usages of synthtic source. --- .../xpack/logsdb/LogsdbRestIT.java | 26 ++- .../xpack/logsdb/LogsdbRestIT.java | 26 ++- .../xpack/logsdb/LogsDBPlugin.java | 5 +- .../SyntheticSourceIndexSettingsProvider.java | 82 +++++++- .../logsdb/SyntheticSourceLicenseService.java | 4 +- ...heticSourceIndexSettingsProviderTests.java | 190 ++++++++++++++++++ 6 files changed, 318 insertions(+), 15 deletions(-) create mode 100644 x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProviderTests.java diff --git a/x-pack/plugin/logsdb/qa/with-basic/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbRestIT.java b/x-pack/plugin/logsdb/qa/with-basic/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbRestIT.java index e7d267810424c..813a181045f2e 100644 --- a/x-pack/plugin/logsdb/qa/with-basic/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbRestIT.java +++ b/x-pack/plugin/logsdb/qa/with-basic/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbRestIT.java @@ -40,7 +40,31 @@ public void testFeatureUsageWithLogsdbIndex() throws IOException { assertThat(features, Matchers.empty()); } { - createIndex("test-index", Settings.builder().put("index.mode", "logsdb").build()); + if (randomBoolean()) { + createIndex("test-index", Settings.builder().put("index.mode", "logsdb").build()); + } else if (randomBoolean()) { + String mapping = """ + { + "properties": { + "field1": { + "type": "keyword", + "time_series_dimension": true + } + } + } + """; + var settings = Settings.builder().put("index.mode", "time_series").put("index.routing_path", "field1").build(); + createIndex("test-index", settings, mapping); + } else { + String mapping = """ + { + "_source": { + "mode": "synthetic" + } + } + """; + createIndex("test-index", Settings.EMPTY, mapping); + } var response = getAsMap("/_license/feature_usage"); @SuppressWarnings("unchecked") List> features = (List>) response.get("features"); diff --git a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbRestIT.java b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbRestIT.java index efff6d0579838..b2d2978a254df 100644 --- a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbRestIT.java +++ b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbRestIT.java @@ -42,7 +42,31 @@ public void testFeatureUsageWithLogsdbIndex() throws IOException { assertThat(features, Matchers.empty()); } { - createIndex("test-index", Settings.builder().put("index.mode", "logsdb").build()); + if (randomBoolean()) { + createIndex("test-index", Settings.builder().put("index.mode", "logsdb").build()); + } else if (randomBoolean()) { + String mapping = """ + { + "properties": { + "field1": { + "type": "keyword", + "time_series_dimension": true + } + } + } + """; + var settings = Settings.builder().put("index.mode", "time_series").put("index.routing_path", "field1").build(); + createIndex("test-index", settings, mapping); + } else { + String mapping = """ + { + "_source": { + "mode": "synthetic" + } + } + """; + createIndex("test-index", Settings.EMPTY, mapping); + } var response = getAsMap("/_license/feature_usage"); @SuppressWarnings("unchecked") List> features = (List>) response.get("features"); diff --git a/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsDBPlugin.java b/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsDBPlugin.java index 5cb7bf9e75252..501cb8536ee7e 100644 --- a/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsDBPlugin.java +++ b/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsDBPlugin.java @@ -51,7 +51,10 @@ public Collection getAdditionalIndexSettingProviders(Index if (DiscoveryNode.isStateless(settings)) { return List.of(logsdbIndexModeSettingsProvider); } - return List.of(new SyntheticSourceIndexSettingsProvider(licenseService), logsdbIndexModeSettingsProvider); + return List.of( + new SyntheticSourceIndexSettingsProvider(licenseService, parameters.mapperServiceFactory()), + logsdbIndexModeSettingsProvider + ); } @Override diff --git a/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProvider.java b/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProvider.java index 5b7792de0622a..a9f060528f05b 100644 --- a/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProvider.java +++ b/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProvider.java @@ -9,28 +9,41 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.CheckedFunction; import org.elasticsearch.index.IndexMode; import org.elasticsearch.index.IndexSettingProvider; import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.index.IndexVersion; +import org.elasticsearch.index.mapper.MapperService; +import java.io.IOException; +import java.io.UncheckedIOException; import java.time.Instant; import java.util.List; -import java.util.Locale; + +import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_ROUTING_PATH; /** * An index setting provider that overwrites the source mode from synthetic to stored if synthetic source isn't allowed to be used. */ -public class SyntheticSourceIndexSettingsProvider implements IndexSettingProvider { +final class SyntheticSourceIndexSettingsProvider implements IndexSettingProvider { private static final Logger LOGGER = LogManager.getLogger(SyntheticSourceIndexSettingsProvider.class); private final SyntheticSourceLicenseService syntheticSourceLicenseService; + private final CheckedFunction mapperServiceFactory; - public SyntheticSourceIndexSettingsProvider(SyntheticSourceLicenseService syntheticSourceLicenseService) { + SyntheticSourceIndexSettingsProvider( + SyntheticSourceLicenseService syntheticSourceLicenseService, + CheckedFunction mapperServiceFactory + ) { this.syntheticSourceLicenseService = syntheticSourceLicenseService; + this.mapperServiceFactory = mapperServiceFactory; } @Override @@ -43,7 +56,7 @@ public Settings getAdditionalIndexSettings( Settings indexTemplateAndCreateRequestSettings, List combinedTemplateMappings ) { - if (newIndexHasSyntheticSourceUsage(indexTemplateAndCreateRequestSettings) + if (newIndexHasSyntheticSourceUsage(indexName, isTimeSeries, indexTemplateAndCreateRequestSettings, combinedTemplateMappings) && syntheticSourceLicenseService.fallbackToStoredSource()) { LOGGER.debug("creation of index [{}] with synthetic source without it being allowed", indexName); // TODO: handle falling back to stored source @@ -51,11 +64,60 @@ public Settings getAdditionalIndexSettings( return Settings.EMPTY; } - boolean newIndexHasSyntheticSourceUsage(Settings indexTemplateAndCreateRequestSettings) { - // TODO: build tmp MapperService and check whether SourceFieldMapper#isSynthetic() to determine synthetic source usage. - // Not using IndexSettings.MODE.get() to avoid validation that may fail at this point. - var rawIndexMode = indexTemplateAndCreateRequestSettings.get(IndexSettings.MODE.getKey()); - IndexMode indexMode = rawIndexMode != null ? Enum.valueOf(IndexMode.class, rawIndexMode.toUpperCase(Locale.ROOT)) : null; - return indexMode != null && indexMode.isSyntheticSourceEnabled(); + boolean newIndexHasSyntheticSourceUsage( + String indexName, + boolean isTimeSeries, + Settings indexTemplateAndCreateRequestSettings, + List combinedTemplateMappings + ) { + if ("validate-index-name".equals(indexName)) { + // This index name is used when validating component and index templates, we should skip this check in that case. + // (See MetadataIndexTemplateService#validateIndexTemplateV2(...) method) + return false; + } + + var tmpIndexMetadata = buildIndexMetadataForMapperService(indexName, isTimeSeries, indexTemplateAndCreateRequestSettings); + try (var mapperService = mapperServiceFactory.apply(tmpIndexMetadata)) { + // combinedTemplateMappings can be null when creating system indices + // combinedTemplateMappings can be empty when creating a normal index that doesn't match any template and without mapping. + if (combinedTemplateMappings == null || combinedTemplateMappings.isEmpty()) { + combinedTemplateMappings = List.of(new CompressedXContent("{}")); + } + mapperService.merge(MapperService.SINGLE_MAPPING_NAME, combinedTemplateMappings, MapperService.MergeReason.INDEX_TEMPLATE); + return mapperService.documentMapper().sourceMapper().isSynthetic(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + // Create a dummy IndexMetadata instance that can be used to create a MapperService in order to check whether synthetic source is used: + private IndexMetadata buildIndexMetadataForMapperService( + String indexName, + boolean isTimeSeries, + Settings indexTemplateAndCreateRequestSettings + ) { + var tmpIndexMetadata = IndexMetadata.builder(indexName); + + int dummyPartitionSize = IndexMetadata.INDEX_ROUTING_PARTITION_SIZE_SETTING.get(indexTemplateAndCreateRequestSettings); + int dummyShards = indexTemplateAndCreateRequestSettings.getAsInt( + IndexMetadata.SETTING_NUMBER_OF_SHARDS, + dummyPartitionSize == 1 ? 1 : dummyPartitionSize + 1 + ); + int shardReplicas = indexTemplateAndCreateRequestSettings.getAsInt(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0); + var finalResolvedSettings = Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()) + .put(indexTemplateAndCreateRequestSettings) + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, dummyShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, shardReplicas) + .put(IndexMetadata.SETTING_INDEX_UUID, UUIDs.randomBase64UUID()); + + if (isTimeSeries) { + finalResolvedSettings.put(IndexSettings.MODE.getKey(), IndexMode.TIME_SERIES); + // Avoid failing because index.routing_path is missing (in case fields are marked as dimension) + finalResolvedSettings.putList(INDEX_ROUTING_PATH.getKey(), List.of("path")); + } + + tmpIndexMetadata.settings(finalResolvedSettings); + return tmpIndexMetadata.build(); } } diff --git a/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceLicenseService.java b/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceLicenseService.java index 4e3e916762fab..89733c9ef1f2b 100644 --- a/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceLicenseService.java +++ b/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/SyntheticSourceLicenseService.java @@ -16,7 +16,7 @@ /** * Determines based on license and fallback setting whether synthetic source usages should fallback to stored source. */ -public final class SyntheticSourceLicenseService { +final class SyntheticSourceLicenseService { private static final String MAPPINGS_FEATURE_FAMILY = "mappings"; @@ -39,7 +39,7 @@ public final class SyntheticSourceLicenseService { private XPackLicenseState licenseState; private volatile boolean syntheticSourceFallback; - public SyntheticSourceLicenseService(Settings settings) { + SyntheticSourceLicenseService(Settings settings) { syntheticSourceFallback = FALLBACK_SETTING.get(settings); } diff --git a/x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProviderTests.java b/x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProviderTests.java new file mode 100644 index 0000000000000..9a46b8de8b662 --- /dev/null +++ b/x-pack/plugin/logsdb/src/test/java/org/elasticsearch/xpack/logsdb/SyntheticSourceIndexSettingsProviderTests.java @@ -0,0 +1,190 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.logsdb; + +import org.elasticsearch.cluster.metadata.DataStream; +import org.elasticsearch.common.compress.CompressedXContent; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.MapperTestUtils; +import org.elasticsearch.test.ESTestCase; +import org.junit.Before; + +import java.io.IOException; +import java.util.List; + +public class SyntheticSourceIndexSettingsProviderTests extends ESTestCase { + + private SyntheticSourceIndexSettingsProvider provider; + + @Before + public void setup() { + SyntheticSourceLicenseService syntheticSourceLicenseService = new SyntheticSourceLicenseService(Settings.EMPTY); + provider = new SyntheticSourceIndexSettingsProvider( + syntheticSourceLicenseService, + im -> MapperTestUtils.newMapperService(xContentRegistry(), createTempDir(), im.getSettings(), im.getIndex().getName()) + ); + } + + public void testNewIndexHasSyntheticSourceUsage() throws IOException { + String dataStreamName = "logs-app1"; + String indexName = DataStream.getDefaultBackingIndexName(dataStreamName, 0); + Settings settings = Settings.EMPTY; + { + String mapping = """ + { + "_doc": { + "_source": { + "mode": "synthetic" + }, + "properties": { + "my_field": { + "type": "keyword" + } + } + } + } + """; + boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, settings, List.of(new CompressedXContent(mapping))); + assertTrue(result); + } + { + String mapping; + if (randomBoolean()) { + mapping = """ + { + "_doc": { + "_source": { + "mode": "stored" + }, + "properties": { + "my_field": { + "type": "keyword" + } + } + } + } + """; + } else { + mapping = """ + { + "_doc": { + "properties": { + "my_field": { + "type": "keyword" + } + } + } + } + """; + } + boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, settings, List.of(new CompressedXContent(mapping))); + assertFalse(result); + } + } + + public void testValidateIndexName() throws IOException { + String indexName = "validate-index-name"; + String mapping = """ + { + "_doc": { + "_source": { + "mode": "synthetic" + }, + "properties": { + "my_field": { + "type": "keyword" + } + } + } + } + """; + Settings settings = Settings.EMPTY; + boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, settings, List.of(new CompressedXContent(mapping))); + assertFalse(result); + } + + public void testNewIndexHasSyntheticSourceUsageLogsdbIndex() throws IOException { + String dataStreamName = "logs-app1"; + String indexName = DataStream.getDefaultBackingIndexName(dataStreamName, 0); + String mapping = """ + { + "_doc": { + "properties": { + "my_field": { + "type": "keyword" + } + } + } + } + """; + { + Settings settings = Settings.builder().put("index.mode", "logsdb").build(); + boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, settings, List.of(new CompressedXContent(mapping))); + assertTrue(result); + } + { + Settings settings = Settings.builder().put("index.mode", "logsdb").build(); + boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, settings, List.of()); + assertTrue(result); + } + { + boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, Settings.EMPTY, List.of()); + assertFalse(result); + } + { + boolean result = provider.newIndexHasSyntheticSourceUsage( + indexName, + false, + Settings.EMPTY, + List.of(new CompressedXContent(mapping)) + ); + assertFalse(result); + } + } + + public void testNewIndexHasSyntheticSourceUsageTimeSeries() throws IOException { + String dataStreamName = "logs-app1"; + String indexName = DataStream.getDefaultBackingIndexName(dataStreamName, 0); + String mapping = """ + { + "_doc": { + "properties": { + "my_field": { + "type": "keyword", + "time_series_dimension": true + } + } + } + } + """; + { + Settings settings = Settings.builder().put("index.mode", "time_series").put("index.routing_path", "my_field").build(); + boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, settings, List.of(new CompressedXContent(mapping))); + assertTrue(result); + } + { + Settings settings = Settings.builder().put("index.mode", "time_series").put("index.routing_path", "my_field").build(); + boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, settings, List.of()); + assertTrue(result); + } + { + boolean result = provider.newIndexHasSyntheticSourceUsage(indexName, false, Settings.EMPTY, List.of()); + assertFalse(result); + } + { + boolean result = provider.newIndexHasSyntheticSourceUsage( + indexName, + false, + Settings.EMPTY, + List.of(new CompressedXContent(mapping)) + ); + assertFalse(result); + } + } + +}