Skip to content

Commit

Permalink
Don't allow invalid template combinations (elastic#56397)
Browse files Browse the repository at this point in the history
This commit removes the ability to put V2 index templates that reference missing component templates.
It also prevents removing component templates that are being referenced by an existing V2 index
template.

Relates to elastic#53101
Resolves elastic#56314
  • Loading branch information
dakrone committed May 14, 2020
1 parent 0fd756d commit 00e847b
Show file tree
Hide file tree
Showing 4 changed files with 146 additions and 11 deletions.
36 changes: 27 additions & 9 deletions docs/reference/indices/index-templates.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,32 @@ the settings from the create index request take precedence over settings specifi

[source,console]
--------------------------------------------------
PUT _component_template/component_template1
{
"template": {
"mappings": {
"properties": {
"@timestamp": {
"type": "date"
}
}
}
}
}
PUT _component_template/other_component_template
{
"template": {
"mappings": {
"properties": {
"ip_address": {
"type": "ip"
}
}
}
}
}
PUT _index_template/template_1
{
"index_patterns": ["te*", "bar*"],
Expand Down Expand Up @@ -233,7 +259,7 @@ When multiple component templates are specified in the `composed_of` field for a
they are merged in the order specified, meaning that later component templates override earlier
component templates.

For two component templates:
For two component templates, the order they are specified changes the number of shards for an index:

[source,console]
--------------------------------------------------
Expand All @@ -245,10 +271,7 @@ PUT /_component_template/template_with_2_shards
}
}
}
--------------------------------------------------
[source,console]
--------------------------------------------------
PUT /_component_template/template_with_3_shards
{
"template": {
Expand All @@ -257,12 +280,7 @@ PUT /_component_template/template_with_3_shards
}
}
}
--------------------------------------------------
The order they are specified changes the number of shards for an index:

[source,console]
--------------------------------------------------
PUT /_index_template/template_1
{
"index_patterns": ["t*"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,15 @@
reason: "format changed in 7.8 to accomodate V2 index templates"
features: allowed_warnings

- do:
cluster.put_component_template:
name: foo
body:
template:
settings:
number_of_shards: 1
number_of_replicas: 0

- do:
indices.put_template:
name: test
Expand All @@ -310,7 +319,7 @@
index_patterns: [v2-test]
priority: 4
version: 3
composed_of: [foo, bar]
composed_of: [foo]

- do:
cat.templates: {}
Expand All @@ -332,5 +341,5 @@
\[v2-test\] \s+
4 \s+
3 \s+
\[foo,\ bar\]
\[foo\]
/
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.elasticsearch.common.settings.IndexScopedSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.Index;
Expand Down Expand Up @@ -246,6 +247,7 @@ ClusterState addComponentTemplate(final ClusterState currentState, final boolean
*/
public void removeComponentTemplate(final String name, final TimeValue masterTimeout,
final ActionListener<AcknowledgedResponse> listener) {
validateNotInUse(clusterService.state().metadata(), name);
clusterService.submitStateUpdateTask("remove-component-template [" + name + "]",
new ClusterStateUpdateTask(Priority.URGENT) {

Expand Down Expand Up @@ -291,6 +293,33 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS
});
}

/**
* Validates that the given component template is not used by any index
* templates, throwing an error if it is still in use
*/
static void validateNotInUse(Metadata metadata, String templateNameOrWildcard) {
final Set<String> matchingComponentTemplates = metadata.componentTemplates().keySet().stream()
.filter(name -> Regex.simpleMatch(templateNameOrWildcard, name))
.collect(Collectors.toSet());
final Set<String> componentsBeingUsed = new HashSet<>();
final List<String> templatesStillUsing = metadata.templatesV2().entrySet().stream()
.filter(e -> {
Set<String> intersecting = Sets.intersection(new HashSet<>(e.getValue().composedOf()), matchingComponentTemplates);
if (intersecting.size() > 0) {
componentsBeingUsed.addAll(intersecting);
return true;
}
return false;
})
.map(Map.Entry::getKey)
.collect(Collectors.toList());

if (templatesStillUsing.size() > 0) {
throw new IllegalArgumentException("component templates " + componentsBeingUsed +
" cannot be removed as they are still in use by index templates " + templatesStillUsing);
}
}

/**
* Add the given index template to the cluster state. If {@code create} is true, an
* exception will be thrown if the component template already exists
Expand Down Expand Up @@ -331,6 +360,16 @@ static void validateV2TemplateRequest(Metadata metadata, String name, IndexTempl
+ IndexMetadata.INDEX_HIDDEN_SETTING.getKey());
}
}

final Map<String, ComponentTemplate> componentTemplates = metadata.componentTemplates();
final List<String> missingComponentTemplates = template.composedOf().stream()
.filter(componentTemplate -> componentTemplates.containsKey(componentTemplate) == false)
.collect(Collectors.toList());

if (missingComponentTemplates.size() > 0) {
throw new InvalidIndexTemplateException(name, "index template [" + name + "] specifies component templates " +
missingComponentTemplates + " that do not exist");
}
}

public ClusterState addIndexTemplateV2(final ClusterState currentState, final boolean create,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;

import static java.util.Collections.singletonList;
Expand Down Expand Up @@ -785,6 +786,74 @@ public void testResolveAliases() throws Exception {
assertThat(resolvedAliases, equalTo(Arrays.asList(a3, a1, a2)));
}

public void testAddInvalidTemplate() throws Exception {
IndexTemplateV2 template = new IndexTemplateV2(Collections.singletonList("a"), null,
Arrays.asList("good", "bad"), null, null, null);
ComponentTemplate ct = new ComponentTemplate(new Template(Settings.EMPTY, null, null), null, null);

final MetadataIndexTemplateService service = getMetadataIndexTemplateService();
CountDownLatch ctLatch = new CountDownLatch(1);
service.putComponentTemplate("api", randomBoolean(), "good", TimeValue.timeValueSeconds(5), ct,
ActionListener.wrap(r -> ctLatch.countDown(), e -> {
logger.error("unexpected error", e);
fail("unexpected error");
}));
ctLatch.await(5, TimeUnit.SECONDS);
InvalidIndexTemplateException e = expectThrows(InvalidIndexTemplateException.class,
() -> {
CountDownLatch latch = new CountDownLatch(1);
AtomicReference<Exception> err = new AtomicReference<>();
service.putIndexTemplateV2("api", randomBoolean(), "template", TimeValue.timeValueSeconds(30), template,
ActionListener.wrap(r -> fail("should have failed!"), exception -> {
err.set(exception);
latch.countDown();
}));
latch.await(5, TimeUnit.SECONDS);
if (err.get() != null) {
throw err.get();
}
});

assertThat(e.name(), equalTo("template"));
assertThat(e.getMessage(), containsString("index template [template] specifies " +
"component templates [bad] that do not exist"));
}

public void testRemoveComponentTemplateInUse() throws Exception {
IndexTemplateV2 template = new IndexTemplateV2(Collections.singletonList("a"), null,
Collections.singletonList("ct"), null, null, null);
ComponentTemplate ct = new ComponentTemplate(new Template(null, new CompressedXContent("{}"), null), null, null);

final MetadataIndexTemplateService service = getMetadataIndexTemplateService();
CountDownLatch ctLatch = new CountDownLatch(1);
service.putComponentTemplate("api", false, "ct", TimeValue.timeValueSeconds(5), ct,
ActionListener.wrap(r -> ctLatch.countDown(), e -> fail("unexpected error")));
ctLatch.await(5, TimeUnit.SECONDS);

CountDownLatch latch = new CountDownLatch(1);
service.putIndexTemplateV2("api", false, "template", TimeValue.timeValueSeconds(30), template,
ActionListener.wrap(r -> latch.countDown(), e -> fail("unexpected error")));
latch.await(5, TimeUnit.SECONDS);

IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> {
AtomicReference<Exception> err = new AtomicReference<>();
CountDownLatch errLatch = new CountDownLatch(1);
service.removeComponentTemplate("c*", TimeValue.timeValueSeconds(30),
ActionListener.wrap(r -> fail("should have failed!"), exception -> {
err.set(exception);
errLatch.countDown();
}));
errLatch.await(5, TimeUnit.SECONDS);
if (err.get() != null) {
throw err.get();
}
});

assertThat(e.getMessage(),
containsString("component templates [ct] cannot be removed as they are still in use by index templates [template]"));
}

private static List<Throwable> putTemplate(NamedXContentRegistry xContentRegistry, PutRequest request) {
MetadataCreateIndexService createIndexService = new MetadataCreateIndexService(
Settings.EMPTY,
Expand Down

0 comments on commit 00e847b

Please sign in to comment.