rethrow script compilation exceptions into ingest configuration excep…

…tions (#19318) * rethrow script compilation exceptions into ingest configuration exceptions * update readProcessor to rethrow any exception as an ElasticsearchException
elastic · Jul 20, 2016 · f7cd86e · f7cd86e
1 parent 3a82c61
commit f7cd86e
Show file tree

Hide file tree

Showing 11 changed files with 141 additions and 26 deletions.
diff --git 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 static Object readObject(String processorType, String processorTag, Map<S
         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 static List<Processor> readProcessorConfigs(List<Map<String, Map<String,
         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<String, Processor.Factory> processorFactories,
                                            String type, Map<String, Object> config) throws Exception {
         Processor.Factory factory = processorFactories.get(type);
@@ -261,20 +283,25 @@ public static Processor readProcessor(Map<String, Processor.Factory> processorFa
 
             List<Processor> 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
@@ -78,6 +78,8 @@ public AppendProcessor create(Map<String, Processor.Factory> registry, String pr
                                       Map<String, Object> 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
@@ -68,7 +68,9 @@ public Factory(TemplateService templateService) {
         public FailProcessor create(Map<String, Processor.Factory> registry, String processorTag,
                                     Map<String, Object> 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
@@ -67,7 +67,9 @@ public Factory(TemplateService templateService) {
         public RemoveProcessor create(Map<String, Processor.Factory> registry, String processorTag,
                                       Map<String, Object> 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
@@ -89,9 +89,11 @@ public SetProcessor create(Map<String, Processor.Factory> registry, String proce
             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/...est-common/src/test/java/org/elasticsearch/ingest/common/AppendProcessorFactoryTests.java b/...est-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 void testCreateNullValue() throws Exception {
             assertThat(e.getMessage(), equalTo("[value] required property is missing"));
         }
     }
+
+    public void testInvalidMustacheTemplate() throws Exception {
+        AppendProcessor.Factory factory = new AppendProcessor.Factory(TestTemplateService.instance(true));
+        Map<String, Object> 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/...ngest-common/src/test/java/org/elasticsearch/ingest/common/FailProcessorFactoryTests.java b/...ngest-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 void testCreateMissingMessageField() throws Exception {
         }
     }
 
+    public void testInvalidMustacheTemplate() throws Exception {
+        FailProcessor.Factory factory = new FailProcessor.Factory(TestTemplateService.instance(true));
+        Map<String, Object> 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/...est-common/src/test/java/org/elasticsearch/ingest/common/RemoveProcessorFactoryTests.java b/...est-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 void testCreateMissingField() throws Exception {
         }
     }
 
+    public void testInvalidMustacheTemplate() throws Exception {
+        RemoveProcessor.Factory factory = new RemoveProcessor.Factory(TestTemplateService.instance(true));
+        Map<String, Object> 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/...ingest-common/src/test/java/org/elasticsearch/ingest/common/SetProcessorFactoryTests.java b/...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 void testCreateNullValue() throws Exception {
         }
     }
 
+    public void testInvalidMustacheTemplate() throws Exception {
+        SetProcessor.Factory factory = new SetProcessor.Factory(TestTemplateService.instance(true));
+        Map<String, Object> 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/...ies/src/test/resources/rest-api-spec/test/ingest/10_pipeline_with_mustache_templates.yaml b/...ies/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
@@ -22,17 +22,27 @@
 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 {