Skip to content

Commit

Permalink
Add validation for component templates
Browse files Browse the repository at this point in the history
This change adds validation to make sure that settings and mappings are correct in
component template. It's done the same way as in index templates - code is reused.

Reletes to elastic#53101
  • Loading branch information
probakowski committed Mar 23, 2020
1 parent 1016e20 commit 83adca7
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.settings.IndexScopedSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.tasks.Task;
import org.elasticsearch.threadpool.ThreadPool;
Expand All @@ -45,14 +46,17 @@ public class TransportPutComponentTemplateAction
extends TransportMasterNodeAction<PutComponentTemplateAction.Request, AcknowledgedResponse> {

private final MetaDataIndexTemplateService indexTemplateService;
private final IndexScopedSettings indexScopedSettings;

@Inject
public TransportPutComponentTemplateAction(TransportService transportService, ClusterService clusterService,
ThreadPool threadPool, MetaDataIndexTemplateService indexTemplateService,
ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver) {
ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver,
IndexScopedSettings indexScopedSettings) {
super(PutComponentTemplateAction.NAME, transportService, clusterService, threadPool, actionFilters,
PutComponentTemplateAction.Request::new, indexNameExpressionResolver);
this.indexTemplateService = indexTemplateService;
this.indexScopedSettings = indexScopedSettings;
}

@Override
Expand All @@ -78,8 +82,10 @@ protected void masterOperation(Task task, final PutComponentTemplateAction.Reque
Template template = componentTemplate.template();
// Normalize the index settings if necessary
if (template.settings() != null) {
Settings.Builder settings = Settings.builder().put(template.settings()).normalizePrefix(IndexMetaData.INDEX_SETTING_PREFIX);
template = new Template(settings.build(), template.mappings(), template.aliases());
Settings.Builder builder = Settings.builder().put(template.settings()).normalizePrefix(IndexMetaData.INDEX_SETTING_PREFIX);
Settings settings = builder.build();
indexScopedSettings.validate(settings, true);
template = new Template(settings, template.mappings(), template.aliases());
componentTemplate = new ComponentTemplate(template, componentTemplate.version(), componentTemplate.metadata());
}
indexTemplateService.putComponentTemplate(request.cause(), request.create(), request.name(), request.masterNodeTimeout(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.UUIDs;
import org.elasticsearch.common.ValidationException;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.settings.IndexScopedSettings;
Expand Down Expand Up @@ -154,7 +155,7 @@ public void onFailure(String source, Exception e) {
}

@Override
public ClusterState execute(ClusterState currentState) {
public ClusterState execute(ClusterState currentState) throws Exception{
return addComponentTemplate(currentState, create, name, template);
}

Expand All @@ -166,14 +167,15 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS
}

// Package visible for testing
static ClusterState addComponentTemplate(final ClusterState currentState, final boolean create,
final String name, final ComponentTemplate template) {
ClusterState addComponentTemplate(final ClusterState currentState, final boolean create,
final String name, final ComponentTemplate template) throws Exception {
if (create && currentState.metaData().componentTemplates().containsKey(name)) {
throw new IllegalArgumentException("component template [" + name + "] already exists");
}

// TODO: validation of component template
// validateAndAddTemplate(request, templateBuilder, indicesService, xContentRegistry);
CompressedXContent mappings = template.template().mappings();
String stringMappings = mappings == null ? null : mappings.string();
validateTemplate(template.template().settings(), stringMappings, indicesService, xContentRegistry);

logger.info("adding component template [{}]", name);
return ClusterState.builder(currentState)
Expand Down Expand Up @@ -274,7 +276,20 @@ public ClusterState execute(ClusterState currentState) throws Exception {
throw new IllegalArgumentException("index_template [" + request.name + "] already exists");
}

validateAndAddTemplate(request, templateBuilder, indicesService, xContentRegistry);
templateBuilder.order(request.order);
templateBuilder.version(request.version);
templateBuilder.patterns(request.indexPatterns);
templateBuilder.settings(request.settings);

if(request.mappings!=null) {
try {
templateBuilder.putMapping(MapperService.SINGLE_MAPPING_NAME, request.mappings);
} catch (Exception e) {
throw new MapperParsingException("Failed to parse mapping: {}", e, request.mappings);
}
}

validateTemplate(request.settings, request.mappings, indicesService, xContentRegistry);

for (Alias alias : request.aliases) {
AliasMetaData aliasMetaData = AliasMetaData.builder(alias.name()).filter(alias.filter())
Expand Down Expand Up @@ -355,20 +370,20 @@ public static List<IndexTemplateMetaData> findTemplates(MetaData metaData, Strin
return matchedTemplates;
}

private static void validateAndAddTemplate(final PutRequest request, IndexTemplateMetaData.Builder templateBuilder,
IndicesService indicesService, NamedXContentRegistry xContentRegistry) throws Exception {
private static void validateTemplate(Settings settings, String mappings,
IndicesService indicesService, NamedXContentRegistry xContentRegistry) throws Exception {
Index createdIndex = null;
final String temporaryIndexName = UUIDs.randomBase64UUID();
try {
// use the provided values, otherwise just pick valid dummy values
int dummyPartitionSize = IndexMetaData.INDEX_ROUTING_PARTITION_SIZE_SETTING.get(request.settings);
int dummyShards = request.settings.getAsInt(IndexMetaData.SETTING_NUMBER_OF_SHARDS,
int dummyPartitionSize = IndexMetaData.INDEX_ROUTING_PARTITION_SIZE_SETTING.get(settings);
int dummyShards = settings.getAsInt(IndexMetaData.SETTING_NUMBER_OF_SHARDS,
dummyPartitionSize == 1 ? 1 : dummyPartitionSize + 1);

//create index service for parsing and validating "mappings"
Settings dummySettings = Settings.builder()
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
.put(request.settings)
.put(settings)
.put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, dummyShards)
.put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0)
.put(IndexMetaData.SETTING_INDEX_UUID, UUIDs.randomBase64UUID())
Expand All @@ -378,19 +393,9 @@ private static void validateAndAddTemplate(final PutRequest request, IndexTempla
IndexService dummyIndexService = indicesService.createIndex(tmpIndexMetadata, Collections.emptyList(), false);
createdIndex = dummyIndexService.index();

templateBuilder.order(request.order);
templateBuilder.version(request.version);
templateBuilder.patterns(request.indexPatterns);
templateBuilder.settings(request.settings);

if (request.mappings != null) {
try {
templateBuilder.putMapping(MapperService.SINGLE_MAPPING_NAME, request.mappings);
} catch (Exception e) {
throw new MapperParsingException("Failed to parse mapping: {}", e, request.mappings);
}
if (mappings != null) {
dummyIndexService.mapperService().merge(MapperService.SINGLE_MAPPING_NAME,
MapperService.parseMapping(xContentRegistry, request.mappings), MergeReason.MAPPING_UPDATE);
MapperService.parseMapping(xContentRegistry, mappings), MergeReason.MAPPING_UPDATE);
}

} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public static ComponentTemplate randomInstance() {
return new ComponentTemplate(template, randomBoolean() ? null : randomNonNegativeLong(), meta);
}

private static Map<String, AliasMetaData> randomAliases() {
public static Map<String, AliasMetaData> randomAliases() {
String aliasName = randomAlphaOfLength(5);
AliasMetaData aliasMeta = AliasMetaData.builder(aliasName)
.filter(Collections.singletonMap(randomAlphaOfLength(2), randomAlphaOfLength(2)))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@

package org.elasticsearch.cluster.metadata;

import com.fasterxml.jackson.core.JsonParseException;
import org.elasticsearch.action.admin.indices.alias.Alias;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.MetaDataIndexTemplateService.PutRequest;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.settings.IndexScopedSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
Expand All @@ -37,6 +39,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
Expand Down Expand Up @@ -197,21 +200,40 @@ public void testPutGlobalTemplateWithIndexHiddenSetting() throws Exception {
assertThat(errors.get(0).getMessage(), containsString("global templates may not specify the setting index.hidden"));
}

public void testAddComponentTemplate() {
public void testAddComponentTemplate() throws Exception{
MetaDataIndexTemplateService metaDataIndexTemplateService = getMetaDataIndexTemplateService();
ClusterState state = ClusterState.EMPTY_STATE;
ComponentTemplate template = ComponentTemplateTests.randomInstance();
state = MetaDataIndexTemplateService.addComponentTemplate(state, false, "foo", template);
Template template = new Template(Settings.builder().build(), null, ComponentTemplateTests.randomAliases());
ComponentTemplate componentTemplate = new ComponentTemplate(template, 1L, new HashMap<>());
state = metaDataIndexTemplateService.addComponentTemplate(state, false, "foo", componentTemplate);

assertNotNull(state.metaData().componentTemplates().get("foo"));
assertThat(state.metaData().componentTemplates().get("foo"), equalTo(template));
assertThat(state.metaData().componentTemplates().get("foo"), equalTo(componentTemplate));

final ClusterState throwState = ClusterState.builder(state).build();
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> MetaDataIndexTemplateService.addComponentTemplate(throwState, true, "foo", template));
() -> metaDataIndexTemplateService.addComponentTemplate(throwState, true, "foo", componentTemplate));
assertThat(e.getMessage(), containsString("component template [foo] already exists"));

state = MetaDataIndexTemplateService.addComponentTemplate(state, randomBoolean(), "bar", template);
state = metaDataIndexTemplateService.addComponentTemplate(state, randomBoolean(), "bar", componentTemplate);
assertNotNull(state.metaData().componentTemplates().get("bar"));

template = new Template(Settings.builder().build(), new CompressedXContent("{\"invalid\"}"), ComponentTemplateTests.randomAliases());
ComponentTemplate componentTemplate2 = new ComponentTemplate(template, 1L, new HashMap<>());
expectThrows(JsonParseException.class,
()->metaDataIndexTemplateService.addComponentTemplate(throwState, true, "foo2", componentTemplate2));

template = new Template(Settings.builder().build(), new CompressedXContent("{\"invalid\":\"invalid\"}"),
ComponentTemplateTests.randomAliases());
ComponentTemplate componentTemplate3 = new ComponentTemplate(template, 1L, new HashMap<>());
expectThrows(MapperParsingException.class,
()->metaDataIndexTemplateService.addComponentTemplate(throwState, true, "foo2", componentTemplate3));

template = new Template(Settings.builder().put("invalid", "invalid").build(), new CompressedXContent("{}"),
ComponentTemplateTests.randomAliases());
ComponentTemplate componentTemplate4 = new ComponentTemplate(template, 1L, new HashMap<>());
expectThrows(IllegalArgumentException.class,
()->metaDataIndexTemplateService.addComponentTemplate(throwState, true, "foo2", componentTemplate4));
}

private static List<Throwable> putTemplate(NamedXContentRegistry xContentRegistry, PutRequest request) {
Expand Down Expand Up @@ -247,23 +269,7 @@ public void onFailure(Exception e) {
}

private List<Throwable> putTemplateDetail(PutRequest request) throws Exception {
IndicesService indicesService = getInstanceFromNode(IndicesService.class);
ClusterService clusterService = getInstanceFromNode(ClusterService.class);
MetaDataCreateIndexService createIndexService = new MetaDataCreateIndexService(
Settings.EMPTY,
clusterService,
indicesService,
null,
null,
new Environment(builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()).build(), null),
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
null,
xContentRegistry(),
Collections.emptyList(),
true);
MetaDataIndexTemplateService service = new MetaDataIndexTemplateService(
clusterService, createIndexService, new AliasValidator(), indicesService,
new IndexScopedSettings(Settings.EMPTY, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS), xContentRegistry());
MetaDataIndexTemplateService service = getMetaDataIndexTemplateService();

final List<Throwable> throwables = new ArrayList<>();
final CountDownLatch latch = new CountDownLatch(1);
Expand All @@ -282,4 +288,24 @@ public void onFailure(Exception e) {
latch.await();
return throwables;
}

private MetaDataIndexTemplateService getMetaDataIndexTemplateService() {
IndicesService indicesService = getInstanceFromNode(IndicesService.class);
ClusterService clusterService = getInstanceFromNode(ClusterService.class);
MetaDataCreateIndexService createIndexService = new MetaDataCreateIndexService(
Settings.EMPTY,
clusterService,
indicesService,
null,
null,
new Environment(builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()).build(), null),
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
null,
xContentRegistry(),
Collections.emptyList(),
true);
return new MetaDataIndexTemplateService(
clusterService, createIndexService, new AliasValidator(), indicesService,
new IndexScopedSettings(Settings.EMPTY, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS), xContentRegistry());
}
}

0 comments on commit 83adca7

Please sign in to comment.