Skip to content

Adding a deprecation info api check for custom types in templates #80676

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

Merged
merged 4 commits into from
Nov 16, 2021
Merged

Adding a deprecation info api check for custom types in templates #80676

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
688abdf
Adding a deprecation info api check for custom types in templates
masseyke Nov 11, 2021
70e2aff
combining logic for multiple types and custom types into one method
masseyke Nov 11, 2021
d47707f
spotlessApply, plus code simplification
masseyke Nov 11, 2021
e6f503f
minor fix
masseyke Nov 16, 2021
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
29 changes: 29 additions & 0 deletions ...precation/src/main/java/org/elasticsearch/xpack/deprecation/ClusterDeprecationChecks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
Expand Down Expand Up @@ -234,6 +235,34 @@ static DeprecationIssue checkTemplatesWithMultipleTypes(ClusterState state) {
);
}

static DeprecationIssue checkTemplatesWithCustomTypes(ClusterState state) {
Set<String> templatesWithCustomTypes = new TreeSet<>();
state.getMetadata().getTemplates().forEach((templateCursor) -> {
String templateName = templateCursor.key;
ImmutableOpenMap<String, CompressedXContent> mappings = templateCursor.value.mappings();
if (mappings != null && mappings.size() == 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should check the type name even if there is more than one mapping type. For example, a user could have both a _default_ mapping and a custom one like some_type.

This makes me think -- maybe this check could be combined with the one above checkTemplatesWithMultipleTypes? That way we could report all errors at once (both that they are using a _default_ mapping, and that they have an custom type name). The error message about "multiple types" could be improved to talk about a _default_ mapping to be more specific.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was only checking for single mapping types with the thought that checkTemplatesWithMultipleTypes would catch the others. I'll see if I can find a clean way to combine them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My earlier comment was based on a misunderstanding -- I thought you couldn't have templates with multiple custom types in 7.x, but you actually can if they are carried over from 6.x! See #72540 for details. I thought this was only possible when using the _default_ mapping, but that is not true. So no need to add special handling for _default_.

String typeName = mappings.iterator().next().key;
if ("_doc".equals(typeName) == false) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny comment, this could be MapperService.SINGLE_MAPPING_NAME.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I'd meant to go back and find that constant!

templatesWithCustomTypes.add(templateName);
}
}
});
if (templatesWithCustomTypes.isEmpty()) {
return null;
}

return new DeprecationIssue(
DeprecationIssue.Level.CRITICAL,
"Custom mapping types in index templates are deprecated",
"https://ela.st/es-deprecation-7-custom-types",
"Update or remove the following index templates before upgrading to 8.0: "
+ templatesWithCustomTypes
+ ". See https://ela.st/es-deprecation-7-removal-of-types for alternatives to mapping types.",
false,
null
);
}

static DeprecationIssue checkClusterRoutingAllocationIncludeRelocationsSetting(final ClusterState clusterState) {
return checkRemovedSetting(
clusterState.metadata().settings(),
Expand Down
1 change: 1 addition & 0 deletions ...ugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ private DeprecationChecks() {}
ClusterDeprecationChecks::checkPollIntervalTooLow,
ClusterDeprecationChecks::checkTemplatesWithFieldNamesDisabled,
ClusterDeprecationChecks::checkTemplatesWithMultipleTypes,
ClusterDeprecationChecks::checkTemplatesWithCustomTypes,
ClusterDeprecationChecks::checkClusterRoutingAllocationIncludeRelocationsSetting,
ClusterDeprecationChecks::checkGeoShapeTemplates,
ClusterDeprecationChecks::checkSparseVectorTemplates,
Expand Down
33 changes: 32 additions & 1 deletion ...tion/src/test/java/org/elasticsearch/xpack/deprecation/ClusterDeprecationChecksTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ public void testIndexTemplatesWithMultipleTypes() throws IOException {
.build();
IndexTemplateMetadata singleType = IndexTemplateMetadata.builder("single-type")
.patterns(Collections.singletonList("foo"))
.putMapping("type1", "{\"type1\":{}}")
.putMapping("_doc", "{\"type1\":{}}")
.build();
ImmutableOpenMap<String, IndexTemplateMetadata> templates = ImmutableOpenMap.<String, IndexTemplateMetadata>builder()
.fPut("multiple-types", multipleTypes)
Expand Down Expand Up @@ -375,6 +375,37 @@ public void testIndexTemplatesWithMultipleTypes() throws IOException {
assertThat(DeprecationChecks.filterChecks(CLUSTER_SETTINGS_CHECKS, c -> c.apply(goodState)), hasSize(0));
}

public void testIndexTemplatesWithCustomTypes() throws IOException {
IndexTemplateMetadata template1 = IndexTemplateMetadata.builder("template1")
.patterns(Collections.singletonList("foo"))
.putMapping("type1", "{\"type1\":{}}")
.build();
IndexTemplateMetadata template2 = IndexTemplateMetadata.builder("template2")
.patterns(Collections.singletonList("foo"))
.putMapping("type2", "{\"type2\":{}}")
.build();
IndexTemplateMetadata template3 = IndexTemplateMetadata.builder("template3")
.patterns(Collections.singletonList("foo"))
.putMapping("_doc", "{\"_doc\":{}}")
.build();
ImmutableOpenMap<String, IndexTemplateMetadata> templates = ImmutableOpenMap.<String, IndexTemplateMetadata>builder()
.fPut("template1", template1)
.fPut("template2", template2)
.fPut("template3", template3)
.build();
Metadata badMetadata = Metadata.builder().templates(templates).build();
ClusterState badState = ClusterState.builder(new ClusterName("test")).metadata(badMetadata).build();
List<DeprecationIssue> issues = DeprecationChecks.filterChecks(CLUSTER_SETTINGS_CHECKS, c -> c.apply(badState));
assertThat(issues, hasSize(1));
assertThat(
issues.get(0).getDetails(),
equalTo(
"Update or remove the following index templates before upgrading to 8.0: [template1, template2]. See "
+ "https://ela.st/es-deprecation-7-removal-of-types for alternatives to mapping types."
)
);
}

public void testClusterRoutingAllocationIncludeRelocationsSetting() {
boolean settingValue = randomBoolean();
String settingKey = CLUSTER_ROUTING_ALLOCATION_INCLUDE_RELOCATIONS_SETTING.getKey();
Expand Down