Browse Source

Updating TransportSimulateIndexTemplateAction.resolveTemplate() to account for data stream overrides (#132131)

Keith Massey 2 months ago
parent
commit
f8b2ed91dc

+ 7 - 0
docs/changelog/132131.yaml

@@ -0,0 +1,7 @@
+pr: 132131
+summary: Updating `TransportSimulateIndexTemplateAction.resolveTemplate()` to account
+  for data stream overrides
+area: Indices APIs
+type: bug
+issues:
+ - 131425

+ 97 - 0
qa/smoke-test-ingest-with-all-dependencies/src/yamlRestTest/resources/rest-api-spec/test/ingest/80_ingest_simulate.yml

@@ -2085,3 +2085,100 @@ setup:
   - match: { docs.0.doc._index: "subobject-index-1" }
   - match: { docs.0.doc._source.a\.b: "some text" }
   - not_exists: docs.0.doc.error
+
+---
+"Simulate ingest with data stream with mapping overrides":
+  - skip:
+      features: headers
+
+  - do:
+      indices.put_index_template:
+        name: test
+        body:
+          index_patterns: test*
+          template:
+            lifecycle:
+              data_retention: "7d"
+          data_stream: {}
+
+  - do:
+      indices.create_data_stream:
+        name: test
+  - is_true: acknowledged
+
+  - do:
+      cluster.health:
+        wait_for_status: yellow
+
+  - do:
+      indices.put_data_stream_mappings:
+        name: test
+        body:
+          properties:
+            foo:
+              type: boolean
+
+  - match: { data_streams.0.applied_to_data_stream: true }
+
+  # For an ordinary simulate request with no overrides, we fetch the mapping from the write index, and do not take the
+  # data stream mapping override into account. So the foo field is unmapped, and we can write text to it.
+  - do:
+      headers:
+        Content-Type: application/json
+      simulate.ingest:
+        index: test
+        body: >
+          {
+            "docs": [
+              {
+                "_id": "asdf",
+                "_source": {
+                  "@timestamp": 1234,
+                  "foo": "bar"
+                }
+              }
+            ]
+          }
+  - length: { docs: 1 }
+  - match: { docs.0.doc._index: "test" }
+  - match: { docs.0.doc._source.foo: "bar" }
+  - not_exists: docs.0.doc.error
+
+  # If template overrides are given, we go to the data stream's template and mapping override (even if the given
+  # template overrides are irrelevant as is the case in this test). Now we see that the foo field is mapped as a
+  # boolean, so we get a validation error when trying to write text to that field.
+  - do:
+      headers:
+        Content-Type: application/json
+      simulate.ingest:
+        index: test
+        body: >
+          {
+            "docs": [
+              {
+                "_id": "asdf",
+                "_source": {
+                  "@timestamp": 1234,
+                  "foo": "bar"
+                }
+              }
+            ],
+            "component_template_substitutions": {
+              "mappings_template": {
+                "template": {
+                  "mappings": {
+                    "dynamic": "true",
+                    "properties": {
+                      "foo": {
+                        "type": "keyword"
+                      }
+                    }
+                  }
+                }
+              }
+            }
+          }
+  - length: { docs: 1 }
+  - match: { docs.0.doc._index: "test" }
+  - match: { docs.0.doc._source.foo: "bar" }
+  - match: { docs.0.doc.error.type: "document_parsing_exception" }

+ 52 - 0
rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.simulate_index_template/10_basic.yml

@@ -250,3 +250,55 @@
   - match: {template.lifecycle.data_retention: "7d"}
   - is_true: template.lifecycle.rollover
   - match: {overlapping: []}
+
+---
+"Simulate index template with data stream with mapping and setting overrides":
+  - requires:
+      cluster_features: [ "logs_stream" ]
+      reason: requires setting 'logs_stream' to get or set data stream mappings
+
+  - do:
+      indices.put_index_template:
+        name: test
+        body:
+          index_patterns: test*
+          template:
+            lifecycle:
+              data_retention: "7d"
+          data_stream: {}
+
+  - do:
+      indices.create_data_stream:
+        name: test
+  - is_true: acknowledged
+
+  - do:
+      cluster.health:
+        wait_for_status: yellow
+
+  - do:
+      indices.put_data_stream_mappings:
+        name: test
+        body:
+          properties:
+            foo:
+              type: keyword
+
+  - match: { data_streams.0.applied_to_data_stream: true }
+
+  - do:
+      indices.put_data_stream_settings:
+        name: test
+        body:
+          index:
+            refresh_interval: "47s"
+
+  - match: { data_streams.0.applied_to_data_stream: true }
+
+  - do:
+      indices.simulate_index_template:
+        name: test
+        include_defaults: true
+
+  - match: {template.mappings.properties.foo.type: "keyword"}
+  - match: {template.settings.index.refresh_interval: "47s"}

+ 86 - 18
server/src/main/java/org/elasticsearch/action/admin/indices/template/post/TransportSimulateIndexTemplateAction.java

@@ -33,6 +33,7 @@ import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.EsExecutors;
+import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.UpdateForV10;
 import org.elasticsearch.index.IndexService;
 import org.elasticsearch.index.IndexSettingProvider;
@@ -48,7 +49,9 @@ import org.elasticsearch.tasks.Task;
 import org.elasticsearch.transport.TransportService;
 import org.elasticsearch.xcontent.NamedXContentRegistry;
 
+import java.io.IOException;
 import java.time.Instant;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -158,7 +161,6 @@ public class TransportSimulateIndexTemplateAction extends TransportLocalProjectM
             listener.onResponse(new SimulateIndexTemplateResponse(null, null));
             return;
         }
-
         final ProjectMetadata tempProjectMetadata = resolveTemporaryState(matchingTemplate, request.getIndexName(), projectWithTemplate);
         ComposableIndexTemplate templateV2 = tempProjectMetadata.templatesV2().get(matchingTemplate);
         assert templateV2 != null : "the matched template must exist";
@@ -167,6 +169,7 @@ public class TransportSimulateIndexTemplateAction extends TransportLocalProjectM
             matchingTemplate,
             request.getIndexName(),
             projectWithTemplate,
+            state.metadata().dataStreams().get(request.getIndexName()),
             isDslOnlyMode,
             xContentRegistry,
             indicesService,
@@ -233,22 +236,34 @@ public class TransportSimulateIndexTemplateAction extends TransportLocalProjectM
     /**
      * Take a template and index name as well as state where the template exists, and return a final
      * {@link Template} that represents all the resolved Settings, Mappings, Aliases and Lifecycle
+     *
+     * @param matchingTemplate
+     * @param indexName
+     * @param simulatedProject
+     * @param dataStream Used to get any data stream mapping or settings overrides to merge into the returned template
+     * @param isDslOnlyMode
+     * @param xContentRegistry
+     * @param indicesService
+     * @param systemIndices
+     * @param indexSettingProviders
+     * @return
+     * @throws Exception
      */
     public static Template resolveTemplate(
         final String matchingTemplate,
         final String indexName,
         final ProjectMetadata simulatedProject,
+        @Nullable final DataStream dataStream,
         final boolean isDslOnlyMode,
         final NamedXContentRegistry xContentRegistry,
         final IndicesService indicesService,
         final SystemIndices systemIndices,
         Set<IndexSettingProvider> indexSettingProviders
     ) throws Exception {
-        Settings templateSettings = resolveSettings(simulatedProject, matchingTemplate);
-
         List<Map<String, AliasMetadata>> resolvedAliases = MetadataIndexTemplateService.resolveAliases(simulatedProject, matchingTemplate);
 
         ComposableIndexTemplate template = simulatedProject.templatesV2().get(matchingTemplate);
+        Settings templateSettings = collectSettings(simulatedProject, dataStream, matchingTemplate, template);
         // create the index with dummy settings in the cluster state so we can parse and validate the aliases
         Settings.Builder dummySettings = Settings.builder()
             .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current())
@@ -256,21 +271,7 @@ public class TransportSimulateIndexTemplateAction extends TransportLocalProjectM
             .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
             .put(IndexMetadata.SETTING_INDEX_UUID, UUIDs.randomBase64UUID());
 
-        /*
-         * If the index name doesn't look like a data stream backing index, then MetadataCreateIndexService.collectV2Mappings() won't
-         * include data stream specific mappings in its response.
-         */
-        String simulatedIndexName = template.getDataStreamTemplate() != null
-            && indexName.startsWith(DataStream.BACKING_INDEX_PREFIX) == false
-                ? DataStream.getDefaultBackingIndexName(indexName, 1)
-                : indexName;
-        List<CompressedXContent> mappings = MetadataCreateIndexService.collectV2Mappings(
-            null, // empty request mapping as the user can't specify any explicit mappings via the simulate api
-            simulatedProject,
-            template,
-            xContentRegistry,
-            simulatedIndexName
-        );
+        List<CompressedXContent> mappings = collectMappings(simulatedProject, dataStream, template, indexName, xContentRegistry);
 
         // First apply settings sourced from index settings providers
         final var now = Instant.now();
@@ -360,6 +361,73 @@ public class TransportSimulateIndexTemplateAction extends TransportLocalProjectM
         );
     }
 
+    /**
+     * This method collects the mappings from the given template, pulling them from the given simulatedProject. If the template is a data
+     * stream template and the given dataStream is not null, this method also appends any mapping overrides from the data stream itself.
+     * @param simulatedProject Used to fetch the component templates referenced from the template
+     * @param dataStream Used to fetch any mappings explicitly set on the data stream
+     * @param template The template matching the index, used to fetch mappings
+     * @param indexName The name of the index whose templates we are fetching
+     * @param xContentRegistry Used to parse mappings if necessary
+     * @return A list of matching mappings in ascending priority order
+     * @throws IOException
+     */
+    private static List<CompressedXContent> collectMappings(
+        ProjectMetadata simulatedProject,
+        @Nullable DataStream dataStream,
+        ComposableIndexTemplate template,
+        String indexName,
+        NamedXContentRegistry xContentRegistry
+    ) throws IOException {
+        /*
+         * If the index name doesn't look like a data stream backing index, then MetadataCreateIndexService.collectV2Mappings() won't
+         * include data stream specific mappings in its response.
+         */
+        String simulatedIndexName = template.getDataStreamTemplate() != null
+            && indexName.startsWith(DataStream.BACKING_INDEX_PREFIX) == false
+                ? DataStream.getDefaultBackingIndexName(indexName, 1)
+                : indexName;
+        List<CompressedXContent> mappings = MetadataCreateIndexService.collectV2Mappings(
+            null, // empty request mapping as the user can't specify any explicit mappings via the simulate api
+            simulatedProject,
+            template,
+            xContentRegistry, // This is never used since requestMappings is always null, but it is not marked explicitly as @Nullable
+            simulatedIndexName
+        );
+        if (template.getDataStreamTemplate() != null && dataStream != null) {
+            CompressedXContent dataStreamMappingOverrides = dataStream.getMappings();
+            if (ComposableIndexTemplate.EMPTY_MAPPINGS.equals(dataStreamMappingOverrides) == false) {
+                // The data stream has had mapping overrides applied, so include these
+                mappings = new ArrayList<>(mappings);
+                mappings.add(dataStreamMappingOverrides);
+            }
+        }
+        return mappings;
+    }
+
+    /**
+     * This method collects the settings from the given template using the given simulatedProjcet. If dataStream is not null, it also merges
+     * in any settings overrides on the data stream itself.
+     * @param simulatedProject The project metadata used to get the settings
+     * @param dataStream If not null, this is used to get data stream settings overrides
+     * @param templateName The name of the template
+     * @param template The template, used to check whether this is a data strema template
+     * @return The settings to be used
+     */
+    private static Settings collectSettings(
+        final ProjectMetadata simulatedProject,
+        @Nullable final DataStream dataStream,
+        String templateName,
+        ComposableIndexTemplate template
+    ) {
+        Settings templateSettings = resolveSettings(simulatedProject, templateName);
+        if (template.getDataStreamTemplate() != null && dataStream != null) {
+            // The data stream has had settings overrides applied, so include them
+            templateSettings = templateSettings.merge(dataStream.getSettings());
+        }
+        return templateSettings;
+    }
+
     private static IndexLongFieldRange getEventIngestedRange(String indexName, ProjectMetadata simulatedProject) {
         final IndexMetadata indexMetadata = simulatedProject.index(indexName);
         return indexMetadata == null ? IndexLongFieldRange.NO_SHARDS : indexMetadata.getEventIngestedRange();

+ 1 - 0
server/src/main/java/org/elasticsearch/action/admin/indices/template/post/TransportSimulateTemplateAction.java

@@ -179,6 +179,7 @@ public class TransportSimulateTemplateAction extends TransportLocalProjectMetada
             matchingTemplate,
             temporaryIndexName,
             projectWithTemplate,
+            null, // we never match a data stream
             isDslOnlyMode,
             xContentRegistry,
             indicesService,

+ 1 - 0
server/src/main/java/org/elasticsearch/action/bulk/TransportSimulateBulkAction.java

@@ -270,6 +270,7 @@ public class TransportSimulateBulkAction extends TransportAbstractBulkAction {
                         matchingTemplate,
                         request.index(),
                         simulatedProjectMetadata,
+                        project.dataStreams().get(request.index()),
                         isDataStreamsLifecycleOnlyMode(clusterService.getSettings()),
                         xContentRegistry,
                         indicesService,

+ 1 - 0
server/src/test/java/org/elasticsearch/action/admin/indices/template/post/TransportSimulateIndexTemplateActionTests.java

@@ -97,6 +97,7 @@ public class TransportSimulateIndexTemplateActionTests extends ESTestCase {
             matchingTemplate,
             indexName,
             simulatedProject,
+            null,
             isDslOnlyMode,
             xContentRegistry(),
             indicesService,