Skip to content

Commit eaf82a6

Browse files
authored
compile ScriptProcessor inline scripts when creating ingest pipelines (#21858)
Inline scripts defined in Ingest Pipelines are now compiled at creation time to preemptively catch errors on initialization of the pipeline. Fixes #21842.
1 parent f0c0f57 commit eaf82a6

File tree

4 files changed

+70
-5
lines changed

4 files changed

+70
-5
lines changed

modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,21 +28,21 @@
2828
import org.elasticsearch.script.ExecutableScript;
2929
import org.elasticsearch.script.Script;
3030
import org.elasticsearch.script.ScriptContext;
31+
import org.elasticsearch.script.ScriptException;
3132
import org.elasticsearch.script.ScriptService;
33+
import org.elasticsearch.script.ScriptType;
3234

3335
import static java.util.Collections.emptyMap;
3436
import static org.elasticsearch.common.Strings.hasLength;
3537
import static org.elasticsearch.ingest.ConfigurationUtils.newConfigurationException;
3638
import static org.elasticsearch.ingest.ConfigurationUtils.readOptionalMap;
3739
import static org.elasticsearch.ingest.ConfigurationUtils.readOptionalStringProperty;
38-
import static org.elasticsearch.ingest.ConfigurationUtils.readStringProperty;
3940
import static org.elasticsearch.script.ScriptType.FILE;
4041
import static org.elasticsearch.script.ScriptType.INLINE;
4142
import static org.elasticsearch.script.ScriptType.STORED;
4243

4344
/**
44-
* Processor that adds new fields with their corresponding values. If the field is already present, its value
45-
* will be replaced with the provided one.
45+
* Processor that evaluates a script with an ingest document in its context.
4646
*/
4747
public final class ScriptProcessor extends AbstractProcessor {
4848

@@ -51,12 +51,24 @@ public final class ScriptProcessor extends AbstractProcessor {
5151
private final Script script;
5252
private final ScriptService scriptService;
5353

54+
/**
55+
* Processor that evaluates a script with an ingest document in its context
56+
*
57+
* @param tag The processor's tag.
58+
* @param script The {@link Script} to execute.
59+
* @param scriptService The {@link ScriptService} used to execute the script.
60+
*/
5461
ScriptProcessor(String tag, Script script, ScriptService scriptService) {
5562
super(tag);
5663
this.script = script;
5764
this.scriptService = scriptService;
5865
}
5966

67+
/**
68+
* Executes the script with the Ingest document in context.
69+
*
70+
* @param document The Ingest document passed into the script context under the "ctx" object.
71+
*/
6072
@Override
6173
public void execute(IngestDocument document) {
6274
ExecutableScript executableScript = scriptService.executable(script, ScriptContext.Standard.INGEST);
@@ -111,16 +123,27 @@ public ScriptProcessor create(Map<String, Processor.Factory> registry, String pr
111123
}
112124

113125
final Script script;
126+
String scriptPropertyUsed;
114127
if (Strings.hasLength(file)) {
115128
script = new Script(FILE, lang, file, (Map<String, Object>)params);
129+
scriptPropertyUsed = "file";
116130
} else if (Strings.hasLength(inline)) {
117131
script = new Script(INLINE, lang, inline, (Map<String, Object>)params);
132+
scriptPropertyUsed = "inline";
118133
} else if (Strings.hasLength(id)) {
119134
script = new Script(STORED, lang, id, (Map<String, Object>)params);
135+
scriptPropertyUsed = "id";
120136
} else {
121137
throw newConfigurationException(TYPE, processorTag, null, "Could not initialize script");
122138
}
123139

140+
// verify script is able to be compiled before successfully creating processor.
141+
try {
142+
scriptService.compile(script, ScriptContext.Standard.INGEST, script.getOptions());
143+
} catch (ScriptException e) {
144+
throw newConfigurationException(TYPE, processorTag, scriptPropertyUsed, e);
145+
}
146+
124147
return new ScriptProcessor(processorTag, script, scriptService);
125148
}
126149
}

modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import org.elasticsearch.ElasticsearchException;
2323
import org.elasticsearch.script.Script;
24+
import org.elasticsearch.script.ScriptException;
2425
import org.elasticsearch.script.ScriptService;
2526
import org.elasticsearch.test.ESTestCase;
2627
import org.junit.Before;
@@ -31,7 +32,9 @@
3132

3233
import static org.hamcrest.Matchers.equalTo;
3334
import static org.hamcrest.Matchers.is;
35+
import static org.mockito.Matchers.any;
3436
import static org.mockito.Mockito.mock;
37+
import static org.mockito.Mockito.when;
3538

3639
public class ScriptProcessorFactoryTests extends ESTestCase {
3740

@@ -98,4 +101,22 @@ public void testFactoryValidationAtLeastOneScriptingType() throws Exception {
98101

99102
assertThat(exception.getMessage(), is("Need [file], [id], or [inline] parameter to refer to scripts"));
100103
}
104+
105+
public void testFactoryInvalidateWithInvalidCompiledScript() throws Exception {
106+
String randomType = randomFrom("inline", "file", "id");
107+
ScriptService mockedScriptService = mock(ScriptService.class);
108+
ScriptException thrownException = new ScriptException("compile-time exception", new RuntimeException(),
109+
Collections.emptyList(), "script", "mockscript");
110+
when(mockedScriptService.compile(any(), any(), any())).thenThrow(thrownException);
111+
factory = new ScriptProcessor.Factory(mockedScriptService);
112+
113+
Map<String, Object> configMap = new HashMap<>();
114+
configMap.put("lang", "mockscript");
115+
configMap.put(randomType, "my_script");
116+
117+
ElasticsearchException exception = expectThrows(ElasticsearchException.class,
118+
() -> factory.create(null, randomAsciiOfLength(10), configMap));
119+
120+
assertThat(exception.getMessage(), is("compile-time exception"));
121+
}
101122
}

modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorTests.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,10 @@
2424

2525
import org.elasticsearch.ingest.IngestDocument;
2626
import org.elasticsearch.ingest.RandomDocumentPicks;
27-
import org.elasticsearch.script.CompiledScript;
2827
import org.elasticsearch.script.ExecutableScript;
2928
import org.elasticsearch.script.Script;
3029
import org.elasticsearch.script.ScriptService;
3130
import org.elasticsearch.test.ESTestCase;
32-
import org.mockito.stubbing.Answer;
3331

3432
import static org.hamcrest.Matchers.hasKey;
3533
import static org.hamcrest.core.Is.is;

qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/50_script_processor_using_painless.yaml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,3 +115,26 @@
115115
- match: { _source.bytes_in: 1234 }
116116
- match: { _source.bytes_out: 4321 }
117117
- match: { _source.bytes_total: 5555 }
118+
119+
---
120+
"Test script processor with syntax error in inline script":
121+
- do:
122+
catch: request
123+
ingest.put_pipeline:
124+
id: "my_pipeline"
125+
body: >
126+
{
127+
"description": "_description",
128+
"processors": [
129+
{
130+
"script" : {
131+
"inline": "invalid painless, hear me roar!"
132+
}
133+
}
134+
]
135+
}
136+
- match: { error.header.processor_type: "script" }
137+
- match: { error.header.property_name: "inline" }
138+
- match: { error.type: "script_exception" }
139+
- match: { error.reason: "compile error" }
140+

0 commit comments

Comments
 (0)