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

Validate V2 templates more strictly #56170

Merged
merged 4 commits into from
May 5, 2020
Merged

Validate V2 templates more strictly #56170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6e9da22
Validate V2 templates more strictly
dakrone May 4, 2020
25ee3ce
Disallow ':' in index patterns also
dakrone May 4, 2020
2ce90bb
Refactor validation methods to DRY them up
dakrone May 5, 2020
3521d31
Merge branch 'master' into itv2-enhance-template-validation
elasticmachine May 5, 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
3 changes: 2 additions & 1 deletion ...lasticsearch/action/admin/indices/template/post/TransportSimulateIndexTemplateAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
Expand Down Expand Up @@ -106,7 +107,7 @@ protected void masterOperation(Task task, SimulateIndexTemplateRequest request,
ClusterState simulateOnClusterState = state;
if (request.getIndexTemplateRequest() != null) {
// we'll "locally" add the template defined by the user in the cluster state (as if it existed in the system)
String simulateTemplateToAdd = "simulate_new_template_" + UUIDs.randomBase64UUID();
String simulateTemplateToAdd = "simulate_new_template_" + UUIDs.randomBase64UUID().toLowerCase(Locale.ROOT);
simulateOnClusterState = indexTemplateService.addIndexTemplateV2(state, request.getIndexTemplateRequest().create(),
simulateTemplateToAdd, request.getIndexTemplateRequest().indexTemplate());
}
Expand Down
2 changes: 1 addition & 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 @@ -981,7 +981,7 @@ private static List<String> validatePrivateSettingsNotExplicitlySet(Settings set
for (final String key : settings.keySet()) {
final Setting<?> setting = indexScopedSettings.get(key);
if (setting == null) {
assert indexScopedSettings.isPrivateSetting(key);
assert indexScopedSettings.isPrivateSetting(key) : "expected [" + key + "] to be private but it was not";
} else if (setting.isPrivateIndex()) {
validationErrors.add("private index setting [" + key + "] can not be set explicitly");
}
Expand Down
109 changes: 79 additions & 30 deletions server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ ClusterState addComponentTemplate(final ClusterState currentState, final boolean
final Template finalTemplate = new Template(finalSettings,
stringMappings == null ? null : new CompressedXContent(stringMappings), template.template().aliases());
final ComponentTemplate finalComponentTemplate = new ComponentTemplate(finalTemplate, template.version(), template.metadata());
validate(name, finalComponentTemplate);
logger.info("adding component template [{}]", name);
return ClusterState.builder(currentState)
.metadata(Metadata.builder(currentState.metadata()).put(name, finalComponentTemplate))
Expand Down Expand Up @@ -402,6 +403,7 @@ public ClusterState addIndexTemplateV2(final ClusterState currentState, final bo
template.priority(), template.version(), template.metadata());
}

validate(name, finalIndexTemplate);
logger.info("adding index template [{}]", name);
return ClusterState.builder(currentState)
.metadata(Metadata.builder(currentState.metadata()).put(name, finalIndexTemplate))
Expand Down Expand Up @@ -922,70 +924,117 @@ private static void validateTemplate(Settings validateSettings, String mappings,
}
}

private void validate(PutRequest request) {
private void validate(String name, ComponentTemplate template) {
dakrone marked this conversation as resolved.
Show resolved Hide resolved
validate(name, template.template(), Collections.emptyList());
}

private void validate(String name, IndexTemplateV2 template) {
validate(name, template.template(), template.indexPatterns());
}

private void validate(String name, Template template, List<String> indexPatterns) {
Optional<Template> maybeTemplate = Optional.ofNullable(template);
validate(name,
maybeTemplate.map(Template::settings).orElse(Settings.EMPTY),
indexPatterns,
maybeTemplate.map(Template::aliases)
.orElse(Collections.emptyMap())
.values().stream()
.map(MetadataIndexTemplateService::toAlias)
.collect(Collectors.toList()));
}

private static Alias toAlias(AliasMetadata aliasMeta) {
Alias a = new Alias(aliasMeta.alias());
if (aliasMeta.filter() != null) {
a.filter(aliasMeta.filter().string());
}
a.searchRouting(aliasMeta.searchRouting());
a.indexRouting(aliasMeta.indexRouting());
a.isHidden(aliasMeta.isHidden());
a.writeIndex(aliasMeta.writeIndex());
return a;
}

private void validate(PutRequest putRequest) {
validate(putRequest.name, putRequest.settings, putRequest.indexPatterns, putRequest.aliases);
}

private void validate(String name, @Nullable Settings settings, List<String> indexPatterns, List<Alias> aliases) {
List<String> validationErrors = new ArrayList<>();
if (request.name.contains(" ")) {
if (name.contains(" ")) {
validationErrors.add("name must not contain a space");
}
if (request.name.contains(",")) {
if (name.contains(",")) {
validationErrors.add("name must not contain a ','");
}
if (request.name.contains("#")) {
if (name.contains("#")) {
validationErrors.add("name must not contain a '#'");
}
if (request.name.startsWith("_")) {
if (name.contains("*")) {
validationErrors.add("name must not contain a '*'");
}
if (name.startsWith("_")) {
validationErrors.add("name must not start with '_'");
}
if (!request.name.toLowerCase(Locale.ROOT).equals(request.name)) {
if (name.toLowerCase(Locale.ROOT).equals(name) == false) {
validationErrors.add("name must be lower cased");
}
for(String indexPattern : request.indexPatterns) {
for(String indexPattern : indexPatterns) {
if (indexPattern.contains(" ")) {
validationErrors.add("template must not contain a space");
validationErrors.add("index_patterns [" + indexPattern + "] must not contain a space");
}
if (indexPattern.contains(",")) {
validationErrors.add("template must not contain a ','");
validationErrors.add("index_pattern [" + indexPattern + "] must not contain a ','");
}
if (indexPattern.contains("#")) {
validationErrors.add("template must not contain a '#'");
validationErrors.add("index_pattern [" + indexPattern + "] must not contain a '#'");
}
if (indexPattern.contains(":")) {
validationErrors.add("index_pattern [" + indexPattern + "] must not contain a ':'");
}
if (indexPattern.startsWith("_")) {
validationErrors.add("template must not start with '_'");
validationErrors.add("index_pattern [" + indexPattern + "] must not start with '_'");
}
if (!Strings.validFileNameExcludingAstrix(indexPattern)) {
validationErrors.add("template must not contain the following characters " + Strings.INVALID_FILENAME_CHARS);
if (Strings.validFileNameExcludingAstrix(indexPattern) == false) {
dakrone marked this conversation as resolved.
Show resolved Hide resolved
validationErrors.add("index_pattern [" + indexPattern + "] must not contain the following characters " +
Strings.INVALID_FILENAME_CHARS);
}
}

try {
indexScopedSettings.validate(request.settings, true); // templates must be consistent with regards to dependencies
} catch (IllegalArgumentException iae) {
validationErrors.add(iae.getMessage());
for (Throwable t : iae.getSuppressed()) {
validationErrors.add(t.getMessage());

if (settings != null) {
try {
// templates must be consistent with regards to dependencies
indexScopedSettings.validate(settings, true);
} catch (IllegalArgumentException iae) {
validationErrors.add(iae.getMessage());
for (Throwable t : iae.getSuppressed()) {
validationErrors.add(t.getMessage());
}
}
List<String> indexSettingsValidation = metadataCreateIndexService.getIndexSettingsValidationErrors(settings, true);
validationErrors.addAll(indexSettingsValidation);
}
List<String> indexSettingsValidation = metadataCreateIndexService.getIndexSettingsValidationErrors(request.settings, true);
validationErrors.addAll(indexSettingsValidation);

if (request.indexPatterns.stream().anyMatch(Regex::isMatchAllPattern)) {
if (IndexMetadata.INDEX_HIDDEN_SETTING.exists(request.settings)) {
if (indexPatterns.stream().anyMatch(Regex::isMatchAllPattern)) {
if (settings != null && IndexMetadata.INDEX_HIDDEN_SETTING.exists(settings)) {
validationErrors.add("global templates may not specify the setting " + IndexMetadata.INDEX_HIDDEN_SETTING.getKey());
}
}

if (!validationErrors.isEmpty()) {
if (validationErrors.size() > 0) {
ValidationException validationException = new ValidationException();
validationException.addValidationErrors(validationErrors);
throw new InvalidIndexTemplateException(request.name, validationException.getMessage());
throw new InvalidIndexTemplateException(name, validationException.getMessage());
}

for (Alias alias : request.aliases) {
//we validate the alias only partially, as we don't know yet to which index it'll get applied to
for (Alias alias : aliases) {
// we validate the alias only partially, as we don't know yet to which index it'll get applied to
aliasValidator.validateAliasStandalone(alias);
if (request.indexPatterns.contains(alias.name())) {
throw new IllegalArgumentException("Alias [" + alias.name() +
"] cannot be the same as any pattern in [" + String.join(", ", request.indexPatterns) + "]");
if (indexPatterns.contains(alias.name())) {
throw new IllegalArgumentException("alias [" + alias.name() +
"] cannot be the same as any pattern in [" + String.join(", ", indexPatterns) + "]");
}
}
}
Expand Down
16 changes: 8 additions & 8 deletions ...r/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public void testIndexTemplateValidationAccumulatesValidationErrors() {
assertEquals(throwables.size(), 1);
assertThat(throwables.get(0), instanceOf(InvalidIndexTemplateException.class));
assertThat(throwables.get(0).getMessage(), containsString("name must not contain a space"));
assertThat(throwables.get(0).getMessage(), containsString("template must not start with '_'"));
assertThat(throwables.get(0).getMessage(), containsString("index_pattern [_test_shards*] must not start with '_'"));
assertThat(throwables.get(0).getMessage(),
containsString("Failed to parse value [0] for setting [index.number_of_shards] must be >= 1"));
}
Expand All @@ -159,7 +159,7 @@ public void testIndexTemplateWithAliasNameEqualToTemplatePattern() {
List<Throwable> errors = putTemplate(xContentRegistry(), request);
assertThat(errors.size(), equalTo(1));
assertThat(errors.get(0), instanceOf(IllegalArgumentException.class));
assertThat(errors.get(0).getMessage(), equalTo("Alias [foobar] cannot be the same as any pattern in [foo, foobar]"));
assertThat(errors.get(0).getMessage(), equalTo("alias [foobar] cannot be the same as any pattern in [foo, foobar]"));
}

public void testIndexTemplateWithValidateMapping() throws Exception {
Expand Down Expand Up @@ -309,15 +309,15 @@ public void testUpdateComponentTemplateWithIndexHiddenSetting() throws Exception

IndexTemplateV2 firstGlobalIndexTemplate =
new IndexTemplateV2(List.of("*"), template, List.of("foo"), 1L, null, null);
state = metadataIndexTemplateService.addIndexTemplateV2(state, true, "globalIndexTemplate1", firstGlobalIndexTemplate);
state = metadataIndexTemplateService.addIndexTemplateV2(state, true, "globalindextemplate1", firstGlobalIndexTemplate);

IndexTemplateV2 secondGlobalIndexTemplate =
new IndexTemplateV2(List.of("*"), template, List.of("foo"), 2L, null, null);
state = metadataIndexTemplateService.addIndexTemplateV2(state, true, "globalIndexTemplate2", secondGlobalIndexTemplate);
state = metadataIndexTemplateService.addIndexTemplateV2(state, true, "globalindextemplate2", secondGlobalIndexTemplate);

IndexTemplateV2 fooPatternIndexTemplate =
new IndexTemplateV2(List.of("foo-*"), template, List.of("foo"), 3L, null, null);
state = metadataIndexTemplateService.addIndexTemplateV2(state, true, "fooPatternIndexTemplate", fooPatternIndexTemplate);
state = metadataIndexTemplateService.addIndexTemplateV2(state, true, "foopatternindextemplate", fooPatternIndexTemplate);

// update the component template to set the index.hidden setting
Template templateWithIndexHiddenSetting = new Template(Settings.builder().put(IndexMetadata.SETTING_INDEX_HIDDEN, true).build(),
Expand All @@ -328,9 +328,9 @@ public void testUpdateComponentTemplateWithIndexHiddenSetting() throws Exception
fail("expecting an exception as updating the component template would yield the global templates to include the index.hidden " +
"setting");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage(), containsStringIgnoringCase("globalIndexTemplate1"));
assertThat(e.getMessage(), containsStringIgnoringCase("globalIndexTemplate2"));
assertThat(e.getMessage(), not(containsStringIgnoringCase("fooPatternIndexTemplate")));
assertThat(e.getMessage(), containsStringIgnoringCase("globalindextemplate1"));
assertThat(e.getMessage(), containsStringIgnoringCase("globalindextemplate2"));
assertThat(e.getMessage(), not(containsStringIgnoringCase("foopatternindextemplate")));
}
}

Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/org/elasticsearch/cluster/shards/ClusterShardLimitIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public void testIndexCreationOverLimitFromTemplate() {

assertAcked(client().admin()
.indices()
.preparePutTemplate("should-fail*")
.preparePutTemplate("should-fail")
.setPatterns(Collections.singletonList("should-fail"))
.setOrder(1)
.setSettings(Settings.builder()
Expand Down