From f7cd86ef6d05243a13f00dcadc77c40dc52b9ef1 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Wed, 20 Jul 2016 10:37:56 -0700 Subject: [PATCH] rethrow script compilation exceptions into ingest configuration exceptions (#19318) * rethrow script compilation exceptions into ingest configuration exceptions * update readProcessor to rethrow any exception as an ElasticsearchException --- .../ingest/ConfigurationUtils.java | 67 +++++++++++++------ .../ingest/common/AppendProcessor.java | 2 + .../ingest/common/FailProcessor.java | 4 +- .../ingest/common/RemoveProcessor.java | 4 +- .../ingest/common/SetProcessor.java | 4 +- .../common/AppendProcessorFactoryTests.java | 12 ++++ .../common/FailProcessorFactoryTests.java | 10 +++ .../common/RemoveProcessorFactoryTests.java | 10 +++ .../common/SetProcessorFactoryTests.java | 12 ++++ .../10_pipeline_with_mustache_templates.yaml | 26 +++++++ .../ingest/TestTemplateService.java | 16 ++++- 11 files changed, 141 insertions(+), 26 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/ingest/ConfigurationUtils.java b/core/src/main/java/org/elasticsearch/ingest/ConfigurationUtils.java index 8b6077c667d..3f725c43b25 100644 --- a/core/src/main/java/org/elasticsearch/ingest/ConfigurationUtils.java +++ b/core/src/main/java/org/elasticsearch/ingest/ConfigurationUtils.java @@ -19,7 +19,9 @@ package org.elasticsearch.ingest; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.ExceptionsHelper; import java.util.ArrayList; import java.util.Arrays; @@ -221,19 +223,17 @@ public final class ConfigurationUtils { return value; } - public static ElasticsearchParseException newConfigurationException(String processorType, String processorTag, + public static ElasticsearchException newConfigurationException(String processorType, String processorTag, String propertyName, String reason) { ElasticsearchParseException exception = new ElasticsearchParseException("[" + propertyName + "] " + reason); + addHeadersToException(exception, processorType, processorTag, propertyName); + return exception; + } - if (processorType != null) { - exception.addHeader("processor_type", processorType); - } - if (processorTag != null) { - exception.addHeader("processor_tag", processorTag); - } - if (propertyName != null) { - exception.addHeader("property_name", propertyName); - } + public static ElasticsearchException newConfigurationException(String processorType, String processorTag, + String propertyName, Exception cause) { + ElasticsearchException exception = ExceptionsHelper.convertToElastic(cause); + addHeadersToException(exception, processorType, processorTag, propertyName); return exception; } @@ -251,6 +251,28 @@ public final class ConfigurationUtils { return processors; } + public static TemplateService.Template compileTemplate(String processorType, String processorTag, String propertyName, + String propertyValue, TemplateService templateService) { + try { + return templateService.compile(propertyValue); + } catch (Exception e) { + throw ConfigurationUtils.newConfigurationException(processorType, processorTag, propertyName, e); + } + } + + private static void addHeadersToException(ElasticsearchException exception, String processorType, + String processorTag, String propertyName) { + if (processorType != null) { + exception.addHeader("processor_type", processorType); + } + if (processorTag != null) { + exception.addHeader("processor_tag", processorTag); + } + if (propertyName != null) { + exception.addHeader("property_name", propertyName); + } + } + public static Processor readProcessor(Map processorFactories, String type, Map config) throws Exception { Processor.Factory factory = processorFactories.get(type); @@ -261,20 +283,25 @@ public final class ConfigurationUtils { List onFailureProcessors = readProcessorConfigs(onFailureProcessorConfigs, processorFactories); String tag = ConfigurationUtils.readOptionalStringProperty(null, null, config, TAG_KEY); - Processor processor = factory.create(processorFactories, tag, config); if (onFailureProcessorConfigs != null && onFailureProcessors.isEmpty()) { - throw newConfigurationException(processor.getType(), processor.getTag(), Pipeline.ON_FAILURE_KEY, + throw newConfigurationException(type, tag, Pipeline.ON_FAILURE_KEY, "processors list cannot be empty"); } - if (config.isEmpty() == false) { - throw new ElasticsearchParseException("processor [{}] doesn't support one or more provided configuration parameters {}", - type, Arrays.toString(config.keySet().toArray())); - } - if (onFailureProcessors.size() > 0 || ignoreFailure) { - return new CompoundProcessor(ignoreFailure, Collections.singletonList(processor), onFailureProcessors); - } else { - return processor; + + try { + Processor processor = factory.create(processorFactories, tag, config); + if (config.isEmpty() == false) { + throw new ElasticsearchParseException("processor [{}] doesn't support one or more provided configuration parameters {}", + type, Arrays.toString(config.keySet().toArray())); + } + if (onFailureProcessors.size() > 0 || ignoreFailure) { + return new CompoundProcessor(ignoreFailure, Collections.singletonList(processor), onFailureProcessors); + } else { + return processor; + } + } catch (Exception e) { + throw newConfigurationException(type, tag, null, e); } } throw new ElasticsearchParseException("No processor type exists with name [" + type + "]"); diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/AppendProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/AppendProcessor.java index 8df1cbd43bb..85cb8acbc06 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/AppendProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/AppendProcessor.java @@ -78,6 +78,8 @@ public final class AppendProcessor extends AbstractProcessor { Map config) throws Exception { String field = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "field"); Object value = ConfigurationUtils.readObject(TYPE, processorTag, config, "value"); + TemplateService.Template compiledTemplate = ConfigurationUtils.compileTemplate(TYPE, processorTag, + "field", field, templateService); return new AppendProcessor(processorTag, templateService.compile(field), ValueSource.wrap(value, templateService)); } } diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/FailProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/FailProcessor.java index 88fa1fa6435..7dbdedaca08 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/FailProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/FailProcessor.java @@ -68,7 +68,9 @@ public final class FailProcessor extends AbstractProcessor { public FailProcessor create(Map registry, String processorTag, Map config) throws Exception { String message = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "message"); - return new FailProcessor(processorTag, templateService.compile(message)); + TemplateService.Template compiledTemplate = ConfigurationUtils.compileTemplate(TYPE, processorTag, + "message", message, templateService); + return new FailProcessor(processorTag, compiledTemplate); } } } diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/RemoveProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/RemoveProcessor.java index 4802d67f5da..b381eed723d 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/RemoveProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/RemoveProcessor.java @@ -67,7 +67,9 @@ public final class RemoveProcessor extends AbstractProcessor { public RemoveProcessor create(Map registry, String processorTag, Map config) throws Exception { String field = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "field"); - return new RemoveProcessor(processorTag, templateService.compile(field)); + TemplateService.Template compiledTemplate = ConfigurationUtils.compileTemplate(TYPE, processorTag, + "field", field, templateService); + return new RemoveProcessor(processorTag, compiledTemplate); } } } diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/SetProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/SetProcessor.java index 06f64b38adf..1ee3ad0509f 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/SetProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/SetProcessor.java @@ -89,9 +89,11 @@ public final class SetProcessor extends AbstractProcessor { String field = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "field"); Object value = ConfigurationUtils.readObject(TYPE, processorTag, config, "value"); boolean overrideEnabled = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, "override", true); + TemplateService.Template compiledTemplate = ConfigurationUtils.compileTemplate(TYPE, processorTag, + "field", field, templateService); return new SetProcessor( processorTag, - templateService.compile(field), + compiledTemplate, ValueSource.wrap(value, templateService), overrideEnabled); } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/AppendProcessorFactoryTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/AppendProcessorFactoryTests.java index 4ed23bef846..e70bc3434ee 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/AppendProcessorFactoryTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/AppendProcessorFactoryTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.ingest.common; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.ingest.TestTemplateService; import org.elasticsearch.test.ESTestCase; @@ -90,4 +91,15 @@ public class AppendProcessorFactoryTests extends ESTestCase { assertThat(e.getMessage(), equalTo("[value] required property is missing")); } } + + public void testInvalidMustacheTemplate() throws Exception { + AppendProcessor.Factory factory = new AppendProcessor.Factory(TestTemplateService.instance(true)); + Map config = new HashMap<>(); + config.put("field", "field1"); + config.put("value", "value1"); + String processorTag = randomAsciiOfLength(10); + ElasticsearchException exception = expectThrows(ElasticsearchException.class, () -> factory.create(null, processorTag, config)); + assertThat(exception.getMessage(), equalTo("java.lang.RuntimeException: could not compile script")); + assertThat(exception.getHeader("processor_tag").get(0), equalTo(processorTag)); + } } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/FailProcessorFactoryTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/FailProcessorFactoryTests.java index 12d181e45a2..217a15cf5b3 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/FailProcessorFactoryTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/FailProcessorFactoryTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.ingest.common; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.ingest.TestTemplateService; import org.elasticsearch.test.ESTestCase; @@ -58,4 +59,13 @@ public class FailProcessorFactoryTests extends ESTestCase { } } + public void testInvalidMustacheTemplate() throws Exception { + FailProcessor.Factory factory = new FailProcessor.Factory(TestTemplateService.instance(true)); + Map config = new HashMap<>(); + config.put("message", "error"); + String processorTag = randomAsciiOfLength(10); + ElasticsearchException exception = expectThrows(ElasticsearchException.class, () -> factory.create(null, processorTag, config)); + assertThat(exception.getMessage(), equalTo("java.lang.RuntimeException: could not compile script")); + assertThat(exception.getHeader("processor_tag").get(0), equalTo(processorTag)); + } } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/RemoveProcessorFactoryTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/RemoveProcessorFactoryTests.java index b2500905a66..71e878744d5 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/RemoveProcessorFactoryTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/RemoveProcessorFactoryTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.ingest.common; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.ingest.TestTemplateService; import org.elasticsearch.test.ESTestCase; @@ -58,4 +59,13 @@ public class RemoveProcessorFactoryTests extends ESTestCase { } } + public void testInvalidMustacheTemplate() throws Exception { + RemoveProcessor.Factory factory = new RemoveProcessor.Factory(TestTemplateService.instance(true)); + Map config = new HashMap<>(); + config.put("field", "field1"); + String processorTag = randomAsciiOfLength(10); + ElasticsearchException exception = expectThrows(ElasticsearchException.class, () -> factory.create(null, processorTag, config)); + assertThat(exception.getMessage(), equalTo("java.lang.RuntimeException: could not compile script")); + assertThat(exception.getHeader("processor_tag").get(0), equalTo(processorTag)); + } } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/SetProcessorFactoryTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/SetProcessorFactoryTests.java index fd152a2f937..45f144e3305 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/SetProcessorFactoryTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/SetProcessorFactoryTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.ingest.common; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.ingest.TestTemplateService; import org.elasticsearch.test.ESTestCase; @@ -99,4 +100,15 @@ public class SetProcessorFactoryTests extends ESTestCase { } } + public void testInvalidMustacheTemplate() throws Exception { + SetProcessor.Factory factory = new SetProcessor.Factory(TestTemplateService.instance(true)); + Map config = new HashMap<>(); + config.put("field", "field1"); + config.put("value", "value1"); + String processorTag = randomAsciiOfLength(10); + ElasticsearchException exception = expectThrows(ElasticsearchException.class, () -> factory.create(null, processorTag, config)); + assertThat(exception.getMessage(), equalTo("java.lang.RuntimeException: could not compile script")); + assertThat(exception.getHeader("processor_tag").get(0), equalTo(processorTag)); + } + } diff --git a/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/10_pipeline_with_mustache_templates.yaml b/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/10_pipeline_with_mustache_templates.yaml index cb914b9afce..b0a729a6299 100644 --- a/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/10_pipeline_with_mustache_templates.yaml +++ b/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/10_pipeline_with_mustache_templates.yaml @@ -324,3 +324,29 @@ - length: { docs.0.processor_results.1.doc._source: 2 } - match: { docs.0.processor_results.1.doc._source.foo: "bar" } - match: { docs.0.processor_results.1.doc._source.error: "processor rename-status [rename]: field [status] doesn't exist" } + +--- +"Test invalid mustache template": + - do: + cluster.health: + wait_for_status: green + + - do: + catch: request + ingest.put_pipeline: + id: "my_pipeline_1" + body: > + { + "description": "_description", + "processors": [ + { + "set" : { + "field" : "field4", + "value": "{{#join}}{{/join}}" + } + } + ] + } + - match: { error.header.processor_type: "set" } + - match: { error.type: "general_script_exception" } + - match: { error.reason: "Failed to compile inline script [{{#join}}{{/join}}] using lang [mustache]" } diff --git a/test/framework/src/main/java/org/elasticsearch/ingest/TestTemplateService.java b/test/framework/src/main/java/org/elasticsearch/ingest/TestTemplateService.java index d44764fa8ac..190674ff61b 100644 --- a/test/framework/src/main/java/org/elasticsearch/ingest/TestTemplateService.java +++ b/test/framework/src/main/java/org/elasticsearch/ingest/TestTemplateService.java @@ -22,17 +22,27 @@ package org.elasticsearch.ingest; import java.util.Map; public class TestTemplateService implements TemplateService { + private boolean compilationException; public static TemplateService instance() { - return new TestTemplateService(); + return new TestTemplateService(false); } - private TestTemplateService() { + public static TemplateService instance(boolean compilationException) { + return new TestTemplateService(compilationException); + } + + private TestTemplateService(boolean compilationException) { + this.compilationException = compilationException; } @Override public Template compile(String template) { - return new MockTemplate(template); + if (this.compilationException) { + throw new RuntimeException("could not compile script"); + } else { + return new MockTemplate(template); + } } public static class MockTemplate implements TemplateService.Template {