honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

ingest: compile mustache template only if field includes '{{'' (#37207) 

* ingest: compile mustache template only if field includes '{{''

Prior to this change, any field in an ingest node processor that supports
script templates would be compiled as mustache template regardless if they
contain a template or not. Compiling normal text as mustache templates is
harmless. However, each compilation counts against the script compilation
circuit breaker. A large number of processors without any templates or scripts
could un-intuitively trip the too many script compilations circuit breaker.

This change simple checks for '{{' in the text before it attempts to compile.

fixes #37120
This commit is contained in:
Jake Landis 2019-01-09 14:47:47 -06:00 committed by GitHub
parent df3b58cb04
commit 195873002b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 62 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/AppendProcessorFactoryTests.java
View File

@ -95,7 +95,7 @@ public class AppendProcessorFactoryTests extends ESTestCase {
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("field", "{{field1}}");
config.put("value", "value1");
String processorTag = randomAlphaOfLength(10);
ElasticsearchException exception = expectThrows(ElasticsearchException.class, () -> factory.create(null, processorTag, config));

2
modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/FailProcessorFactoryTests.java
View File

@ -62,7 +62,7 @@ public class FailProcessorFactoryTests extends ESTestCase {
public void testInvalidMustacheTemplate() throws Exception {
FailProcessor.Factory factory = new FailProcessor.Factory(TestTemplateService.instance(true));
Map<String, Object> config = new HashMap<>();
config.put("message", "error");
config.put("message", "{{error}}");
String processorTag = randomAlphaOfLength(10);
ElasticsearchException exception = expectThrows(ElasticsearchException.class, () -> factory.create(null, processorTag, config));
assertThat(exception.getMessage(), equalTo("java.lang.RuntimeException: could not compile script"));

2
modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/RemoveProcessorFactoryTests.java
View File

@ -75,7 +75,7 @@ public class RemoveProcessorFactoryTests extends ESTestCase {
public void testInvalidMustacheTemplate() throws Exception {
RemoveProcessor.Factory factory = new RemoveProcessor.Factory(TestTemplateService.instance(true));
Map<String, Object> config = new HashMap<>();
config.put("field", "field1");
config.put("field", "{{field1}}");
String processorTag = randomAlphaOfLength(10);
ElasticsearchException exception = expectThrows(ElasticsearchException.class, () -> factory.create(null, processorTag, config));
assertThat(exception.getMessage(), equalTo("java.lang.RuntimeException: could not compile script"));

2
modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/SetProcessorFactoryTests.java
View File

@ -103,7 +103,7 @@ public class SetProcessorFactoryTests extends ESTestCase {
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("field", "{{field1}}");
config.put("value", "value1");
String processorTag = randomAlphaOfLength(10);
ElasticsearchException exception = expectThrows(ElasticsearchException.class, () -> factory.create(null, processorTag, config));

2
server/src/main/java/org/elasticsearch/ingest/ConfigurationUtils.java
View File

@ -335,7 +335,7 @@ public final class ConfigurationUtils {
// installed for use by REST tests. `propertyValue` will not be
// modified if templating is not available so a script that simply returns an unmodified `propertyValue`
// is returned.
if (scriptService.isLangSupported(DEFAULT_TEMPLATE_LANG)) {
if (scriptService.isLangSupported(DEFAULT_TEMPLATE_LANG) && propertyValue.contains("{{")) {
Script script = new Script(ScriptType.INLINE, DEFAULT_TEMPLATE_LANG, propertyValue, Collections.emptyMap());
return scriptService.compile(script, TemplateScript.CONTEXT);
} else {

2
server/src/main/java/org/elasticsearch/ingest/ValueSource.java
View File

@ -75,7 +75,7 @@ public interface ValueSource {
// This check is here because the DEFAULT_TEMPLATE_LANG(mustache) is not
// installed for use by REST tests. `value` will not be
// modified if templating is not available
if (scriptService.isLangSupported(DEFAULT_TEMPLATE_LANG)) {
if (scriptService.isLangSupported(DEFAULT_TEMPLATE_LANG) && ((String) value).contains("{{")) {
Script script = new Script(ScriptType.INLINE, DEFAULT_TEMPLATE_LANG, (String) value, Collections.emptyMap());
return new TemplatedValue(scriptService.compile(script, TemplateScript.CONTEXT));
} else {

29
server/src/test/java/org/elasticsearch/ingest/ConfigurationUtilsTests.java
View File

@ -21,6 +21,7 @@ package org.elasticsearch.ingest;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.script.TemplateScript;
import org.elasticsearch.test.ESTestCase;
import org.junit.Before;
@ -36,7 +37,12 @@ import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.sameInstance;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
public class ConfigurationUtilsTests extends ESTestCase {
@ -181,4 +187,27 @@ public class ConfigurationUtilsTests extends ESTestCase {
assertThat(ex.getMessage(), equalTo("property isn't a map, but of type [" + invalidConfig.getClass().getName() + "]"));
}
public void testNoScriptCompilation() {
ScriptService scriptService = mock(ScriptService.class);
when(scriptService.isLangSupported(anyString())).thenReturn(true);
String propertyValue = randomAlphaOfLength(10);
TemplateScript.Factory result;
result = ConfigurationUtils.compileTemplate(randomAlphaOfLength(10), randomAlphaOfLength(10), randomAlphaOfLength(10),
propertyValue, scriptService);
assertThat(result.newInstance(null).execute(), equalTo(propertyValue));
verify(scriptService, times(0)).compile(any(), any());
}
public void testScriptShouldCompile() {
ScriptService scriptService = mock(ScriptService.class);
when(scriptService.isLangSupported(anyString())).thenReturn(true);
String propertyValue = "{{" + randomAlphaOfLength(10) + "}}";
String compiledValue = randomAlphaOfLength(10);
when(scriptService.compile(any(), any())).thenReturn(new TestTemplateService.MockTemplateScript.Factory(compiledValue));
TemplateScript.Factory result;
result = ConfigurationUtils.compileTemplate(randomAlphaOfLength(10), randomAlphaOfLength(10), randomAlphaOfLength(10),
propertyValue, scriptService);
assertThat(result.newInstance(null).execute(), equalTo(compiledValue));
verify(scriptService, times(1)).compile(any(), any());
}
}

27
server/src/test/java/org/elasticsearch/ingest/ValueSourceTests.java
View File

@ -19,6 +19,7 @@
package org.elasticsearch.ingest;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.test.ESTestCase;
import java.util.ArrayList;
@ -30,6 +31,12 @@ import java.util.Map;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.sameInstance;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
public class ValueSourceTests extends ESTestCase {
@ -69,4 +76,24 @@ public class ValueSourceTests extends ESTestCase {
assertThat(myPreciousList.size(), equalTo(1));
assertThat(myPreciousList.get(0), equalTo("value"));
}
public void testNoScriptCompilation() {
ScriptService scriptService = mock(ScriptService.class);
when(scriptService.isLangSupported(anyString())).thenReturn(true);
String propertyValue = randomAlphaOfLength(10);
ValueSource result = ValueSource.wrap(propertyValue, scriptService);
assertThat(result.copyAndResolve(null), equalTo(propertyValue));
verify(scriptService, times(0)).compile(any(), any());
}
public void testScriptShouldCompile() {
ScriptService scriptService = mock(ScriptService.class);
when(scriptService.isLangSupported(anyString())).thenReturn(true);
String propertyValue = "{{" + randomAlphaOfLength(10) + "}}";
String compiledValue = randomAlphaOfLength(10);
when(scriptService.compile(any(), any())).thenReturn(new TestTemplateService.MockTemplateScript.Factory(compiledValue));
ValueSource result = ValueSource.wrap(propertyValue, scriptService);
assertThat(result.copyAndResolve(Collections.emptyMap()), equalTo(compiledValue));
verify(scriptService, times(1)).compile(any(), any());
}
}