Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ITV2: disallow duplicate dynamic templates #56291

Merged
merged 6 commits into from
May 12, 2020
Merged

ITV2: disallow duplicate dynamic templates #56291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bb2b69e
ITV2: disallow duplicate dynamic templates
andreidan May 6, 2020
21794c1
Filter duplicate dynamic templates
andreidan May 7, 2020
21be731
Merge branch 'master' into dedup_dynamic_templates
elasticmachine May 7, 2020
27b528d
Fix line length
andreidan May 7, 2020
cc42cda
Extract method and re-name removeAll
andreidan May 7, 2020
d449e8f
Update doc
andreidan May 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 94 additions & 1 deletion server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
Expand Down Expand Up @@ -576,6 +577,7 @@ static Map<String, Object> parseV2Mappings(String mappingsJson, List<CompressedX
Map<String, Object> innerTemplateNonProperties = new HashMap<>(innerTemplateMapping);
Map<String, Object> maybeProperties = (Map<String, Object>) innerTemplateNonProperties.remove("properties");

nonProperties = removeDuplicatedDynamicTemplates(nonProperties, innerTemplateNonProperties);
XContentHelper.mergeDefaults(innerTemplateNonProperties, nonProperties);
nonProperties = innerTemplateNonProperties;

Expand All @@ -590,6 +592,7 @@ static Map<String, Object> parseV2Mappings(String mappingsJson, List<CompressedX
Map<String, Object> innerRequestNonProperties = new HashMap<>(innerRequestMappings);
Map<String, Object> maybeRequestProperties = (Map<String, Object>) innerRequestNonProperties.remove("properties");

nonProperties = removeDuplicatedDynamicTemplates(nonProperties, innerRequestMappings);
XContentHelper.mergeDefaults(innerRequestNonProperties, nonProperties);
nonProperties = innerRequestNonProperties;

Expand All @@ -598,11 +601,101 @@ static Map<String, Object> parseV2Mappings(String mappingsJson, List<CompressedX
}
}

Map<String, Object> finalMappings = new HashMap<>(nonProperties);
Map<String, Object> finalMappings = dedupDynamicTemplates(nonProperties);
finalMappings.put("properties", properties);
return Collections.singletonMap(MapperService.SINGLE_MAPPING_NAME, finalMappings);
}

/**
* Removes the already seen/processed dynamic templates from the previouslySeenMapping if they are defined (we're
* identifying the dynamic templates based on the name only, *not* on the full definition) in the newMapping we are about to
* process (and merge)
*/
@SuppressWarnings("unchecked")
private static Map<String, Object> removeDuplicatedDynamicTemplates(Map<String, Object> previouslySeenMapping,
Map<String, Object> newMapping) {
Map<String, Object> result = new HashMap<>(previouslySeenMapping);
List<Map<String, Object>> newDynamicTemplates = (List<Map<String, Object>>) newMapping.get("dynamic_templates");
List<Map<String, Object>> previouslySeenDynamicTemplates =
(List<Map<String, Object>>) previouslySeenMapping.get("dynamic_templates");

List<Map<String, Object>> filteredDynamicTemplates = removeOverlapping(previouslySeenDynamicTemplates, newDynamicTemplates);

// if we removed any mappings from the previously seen ones, we'll re-add them on merge time, see
// {@link XContentHelper#mergeDefaults}, so update the result to contain the filtered ones
if (filteredDynamicTemplates != previouslySeenDynamicTemplates) {
result.put("dynamic_templates", filteredDynamicTemplates);
}
return result;
}

/**
* Removes all the items from the first list that are already present in the second list
*
* Similar to {@link List#removeAll(Collection)} but the list parameters are not modified.
*
* This expects both list values to be Maps of size one and the "contains" operation that will determine if a value
* from the second list is present in the first list (and be removed from the first list) is based on key name.
*
* eg.
* removeAll([ {"key1" : {}}, {"key2" : {}} ], [ {"key1" : {}}, {"key3" : {}} ])
* Returns:
* [ {"key2" : {}} ]
*/
private static List<Map<String, Object>> removeOverlapping(List<Map<String, Object>> first, List<Map<String, Object>> second) {
if (first == null) {
return first;
} else {
validateValuesAreMapsOfSizeOne(first);
}

if (second == null) {
return first;
} else {
validateValuesAreMapsOfSizeOne(second);
}

Set<String> keys = second.stream()
.map(value -> value.keySet().iterator().next())
.collect(Collectors.toSet());

return first.stream().filter(value -> keys.contains(value.keySet().iterator().next()) == false).collect(toList());
}

private static void validateValuesAreMapsOfSizeOne(List<Map<String, Object>> second) {
for (Map<String, Object> map : second) {
// all are in the form of [ {"key1" : {}}, {"key2" : {}} ]
if (map.size() != 1) {
throw new IllegalArgumentException("unexpected argument, expected maps with one key, but got " + map);
}
}
}

/**
* Parses the `dynamic_templates` from the provided mappings, if any are configured, and returns a mappings map containing dynamic
* templates with unique names.
*
* The later templates in the provided mapping's `dynamic_templates` array will override the templates with the same name defined
* earlier in the `dynamic_templates` array.
*/
@SuppressWarnings("unchecked")
private static Map<String, Object> dedupDynamicTemplates(Map<String, Object> mappings) {
Objects.requireNonNull(mappings, "deduping the dynamic templates a non-null mapping");
Map<String, Object> results = new HashMap<>(mappings);
List<Map<String, Object>> dynamicTemplates = (List<Map<String, Object>>) mappings.get("dynamic_templates");
if (dynamicTemplates == null) {
return results;
}

LinkedHashMap<String, Map<String, Object>> dedupedDynamicTemplates = new LinkedHashMap<>(dynamicTemplates.size(), 1f);
for (Map<String, Object> dynamicTemplate : dynamicTemplates) {
dedupedDynamicTemplates.put(dynamicTemplate.keySet().iterator().next(), dynamicTemplate);
}

results.put("dynamic_templates", new ArrayList<>(dedupedDynamicTemplates.values()));
return results;
}

/**
* Add the objects in the second map to the first, where the keys in the {@code second} map have
* higher predecence and overwrite the keys in the {@code first} map. In the event of a key with
Expand Down
173 changes: 173 additions & 0 deletions server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1111,6 +1111,179 @@ public void testMergeIgnoringDots() throws Exception {
assertThat(results, equalTo(expected));
}

@SuppressWarnings("unchecked")
public void testDedupTemplateDynamicTemplates() throws Exception {
Template template = new Template(null,
new CompressedXContent("{\"_doc\":{\"_source\":{\"enabled\": false}, \"dynamic_templates\": [" +
"{\n" +
" \"docker.container.labels\": {\n" +
" \"mapping\": {\n" +
" \"type\": \"keyword\"\n" +
" },\n" +
" \"match_mapping_type\": \"string\",\n" +
" \"path_match\": \"labels.*\"\n" +
" }\n" +
" },\n" +
" {\n" +
" \"docker.container.labels\": {\n" +
" \"mapping\": {\n" +
" \"type\": \"keyword\"\n" +
" },\n" +
" \"match_mapping_type\": \"string\",\n" +
" \"path_match\": \"docker.container.labels.*\"\n" +
" }\n" +
"}]}}"), null);

IndexTemplateV2 indexTemplate = new IndexTemplateV2(Collections.singletonList("index"), template, null, null, null, null);

ClusterState state = ClusterState.builder(ClusterState.EMPTY_STATE)
.metadata(Metadata.builder(Metadata.EMPTY_METADATA)
.put("index-template", indexTemplate)
.build())
.build();

Map<String, Object> resolved =
MetadataCreateIndexService.resolveV2Mappings("{}", state,
"index-template", new NamedXContentRegistry(Collections.emptyList()));

Map<String, Object> doc = (Map<String, Object>) resolved.get(MapperService.SINGLE_MAPPING_NAME);
List<Map<String, Object>> dynamicTemplates = (List<Map<String, Object>>) doc.get("dynamic_templates");
assertThat(dynamicTemplates.size(), is(1));
Map<String, Object> dynamicMapping = (Map<String, Object>) dynamicTemplates.get(0).get("docker.container.labels");
assertThat(dynamicMapping, is(notNullValue()));
assertThat("last mapping with the same name must override previously defined mappings with the same name",
dynamicMapping.get("path_match"), is("docker.container.labels.*"));
}

public void testDedupRequestDynamicTemplates() throws Exception {
String requestMappingJson = "{\"_doc\":{\"_source\":{\"enabled\": false}, \"dynamic_templates\": [" +
"{\n" +
" \"docker.container.labels\": {\n" +
" \"mapping\": {\n" +
" \"type\": \"keyword\"\n" +
" },\n" +
" \"match_mapping_type\": \"string\",\n" +
" \"path_match\": \"labels.*\"\n" +
" }\n" +
" },\n" +
" {\n" +
" \"docker.container.labels\": {\n" +
" \"mapping\": {\n" +
" \"type\": \"keyword\"\n" +
" },\n" +
" \"match_mapping_type\": \"string\",\n" +
" \"path_match\": \"source.request.*\"\n" +
" }\n" +
"}]}}";

String templateMappingJson = "{\"_doc\":{\"_source\":{\"enabled\": false}, \"dynamic_templates\": [" +
"{\n" +
" \"docker.container.labels\": {\n" +
" \"mapping\": {\n" +
" \"type\": \"text\",\n" +
" \"copy_to\": \"text_labels\"\n" +
" },\n" +
" \"match_mapping_type\": \"string\",\n" +
" \"path_match\": \"source.template.*\"\n" +
" }\n" +
" }\n" +
"]}}";
Template template = new Template(null, new CompressedXContent(templateMappingJson), null);

IndexTemplateV2 indexTemplate = new IndexTemplateV2(Collections.singletonList("index"), template, null, null, null, null);

ClusterState state = ClusterState.builder(ClusterState.EMPTY_STATE)
.metadata(Metadata.builder(Metadata.EMPTY_METADATA)
.put("index-template", indexTemplate)
.build())
.build();

Map<String, Object> resolved =
MetadataCreateIndexService.resolveV2Mappings(requestMappingJson, state,
"index-template", new NamedXContentRegistry(Collections.emptyList()));

Map<String, Object> doc = (Map<String, Object>) resolved.get(MapperService.SINGLE_MAPPING_NAME);
List<Map<String, Object>> dynamicTemplates = (List<Map<String, Object>>) doc.get("dynamic_templates");
assertThat(dynamicTemplates.size(), is(1));
Map<String, Object> dynamicMapping = (Map<String, Object>) dynamicTemplates.get(0).get("docker.container.labels");
assertThat(dynamicMapping, is(notNullValue()));
assertThat("last mapping with the same name must override previously defined mappings with the same name",
dynamicMapping.get("path_match"), is("source.request.*"));
Map<String, Object> mapping = (Map<String, Object>) dynamicMapping.get("mapping");
assertThat("the dynamic template defined in the request must not be merged with the dynamic template with the " +
"same name defined in the index template", mapping.size(), is(1));
assertThat(mapping.get("type"), is("keyword"));
}

public void testMultipleComponentTemplatesDefineSameDynamicTemplate() throws Exception {
String ct1Mapping = "{\"_doc\":{\"_source\":{\"enabled\": false}, \"dynamic_templates\": [" +
"{\n" +
" \"docker.container.labels\": {\n" +
" \"mapping\": {\n" +
" \"type\": \"text\",\n" +
" \"copy_to\": \"text_labels\"\n" +
" },\n" +
" \"match_mapping_type\": \"string\",\n" +
" \"path_match\": \"source.first.ct.*\"\n" +
" }\n" +
" },\n" +
"{\n" +
" \"other.labels\": {\n" +
" \"mapping\": {\n" +
" \"type\": \"keyword\"\n" +
" },\n" +
" \"match_mapping_type\": \"string\",\n" +
" \"path_match\": \"source.first.ct.other.labels*\"\n" +
" }\n" +
" }\n" +
"]}}";
String ct2Mapping = "{\"_doc\":{\"_source\":{\"enabled\": false}, \"dynamic_templates\": [" +
"{\n" +
" \"docker.container.labels\": {\n" +
" \"mapping\": {\n" +
" \"type\": \"keyword\"\n" +
" },\n" +
" \"match_mapping_type\": \"string\",\n" +
" \"path_match\": \"source.second.ct.*\"\n" +
" }\n" +
" }\n" +
"]}}";

Template ctt1 = new Template(null, new CompressedXContent(ct1Mapping), null);
Template ctt2 = new Template(null, new CompressedXContent(ct2Mapping), null);
ComponentTemplate ct1 = new ComponentTemplate(ctt1, null, null);
ComponentTemplate ct2 = new ComponentTemplate(ctt2, null, null);

IndexTemplateV2 template = new IndexTemplateV2(Collections.singletonList("index"), null, Arrays.asList("ct1", "ct2"),
null, null, null);

ClusterState state = ClusterState.builder(ClusterState.EMPTY_STATE)
.metadata(Metadata.builder(Metadata.EMPTY_METADATA)
.put("ct1", ct1)
.put("ct2", ct2)
.put("index-template", template)
.build())
.build();

Map<String, Object> resolved =
MetadataCreateIndexService.resolveV2Mappings("{}", state,
"index-template", new NamedXContentRegistry(Collections.emptyList()));

Map<String, Object> doc = (Map<String, Object>) resolved.get(MapperService.SINGLE_MAPPING_NAME);
List<Map<String, Object>> dynamicTemplates = (List<Map<String, Object>>) doc.get("dynamic_templates");
assertThat(dynamicTemplates.size(), is(2));
Map<String, Object> dockerLabelsDynamicTemplate = dynamicTemplates.get(0).get("docker.container.labels") != null ?
dynamicTemplates.get(0) : dynamicTemplates.get(1);
Map<String, Object> dynamicMapping = (Map<String, Object>) dockerLabelsDynamicTemplate.get("docker.container.labels");
assertThat(dynamicMapping, is(notNullValue()));
assertThat("dynamic template defined in the last defined component template must override the previously defined dynamic templates",
dynamicMapping.get("path_match"), is("source.second.ct.*"));
Map<String, Object> mapping = (Map<String, Object>) dynamicMapping.get("mapping");
assertThat("the dynamic template defined in the second component template must not be merged with the dynamic template with the " +
"same name defined in the first component template", mapping.size(), is(1));
assertThat(mapping.get("type"), is("keyword"));
}

private IndexTemplateMetadata addMatchingTemplate(Consumer<IndexTemplateMetadata.Builder> configurator) {
IndexTemplateMetadata.Builder builder = templateMetadataBuilder("template1", "te*");
configurator.accept(builder);
Expand Down