diff --git a/docs/reference/migration/migrate_8_0/scripting.asciidoc b/docs/reference/migration/migrate_8_0/scripting.asciidoc index f4e9724d04c63..04a5ff40a6231 100644 --- a/docs/reference/migration/migrate_8_0/scripting.asciidoc +++ b/docs/reference/migration/migrate_8_0/scripting.asciidoc @@ -22,4 +22,31 @@ scripts. Any use of `getDayOfWeek` expecting a return value of `int` will result in a compilation error or runtime error and may not allow the upgraded node to start. ==== + +.Stored scripts no longer support empty scripts or search templates. +[%collapsible] +==== +*Details* + +The {ref}/create-stored-script-api.html[create or update stored script API]'s +`source` parameter cannot be empty. + +*Impact* + +Before upgrading, use the {ref}/delete-stored-script-api.html[delete stored +script API] to delete any empty stored scripts or search templates. +In 8.0, {es} will drop any empty stored scripts or empty search templates from +the cluster state. Requests to create a stored script or search template with +an empty `source` will return an error. +==== + +.The create or update stored script API's `code` parameter has been removed. +[%collapsible] +==== +*Details* + +The {ref}/create-stored-script-api.html[create or update stored script API]'s +`code` parameter has been removed. Use the `source` parameter instead. + +*Impact* + +Discontinue use of the `code` parameter. Requests that include the parameter +will return an error. +==== // end::notable-breaking-changes[] \ No newline at end of file diff --git a/server/src/internalClusterTest/java/org/elasticsearch/script/StoredScriptsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/script/StoredScriptsIT.java index 4874b67c339da..b1615cd04003b 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/script/StoredScriptsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/script/StoredScriptsIT.java @@ -55,7 +55,7 @@ public void testBasics() { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client().admin().cluster().preparePutStoredScript() .setId("id#") - .setContent(new BytesArray("{}"), XContentType.JSON) + .setContent(new BytesArray("{\"script\": {\"lang\": \"" + LANG + "\", \"source\": \"1\"} }"), XContentType.JSON) .get()); assertEquals("Validation Failed: 1: id cannot contain '#' for stored script;", e.getMessage()); } diff --git a/server/src/main/java/org/elasticsearch/script/ScriptMetadata.java b/server/src/main/java/org/elasticsearch/script/ScriptMetadata.java index 8f8b958b832a9..fb18105bb4c0a 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptMetadata.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptMetadata.java @@ -7,6 +7,8 @@ */ package org.elasticsearch.script; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.Version; import org.elasticsearch.cluster.ClusterState; @@ -18,8 +20,6 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.logging.DeprecationCategory; -import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; @@ -38,9 +38,9 @@ public final class ScriptMetadata implements Metadata.Custom, Writeable, ToXContentFragment { /** - * Standard deprecation logger for used to deprecate allowance of empty templates. + * Standard logger used to warn about dropped scripts. */ - private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(ScriptMetadata.class); + private static final Logger logger = LogManager.getLogger(ScriptMetadata.class); /** * A builder used to modify the currently stored scripts data held within @@ -162,16 +162,10 @@ static ScriptMetadata deleteStoredScript(ScriptMetadata previous, String id) { * ... * } * } - * - * When loading from a source prior to 6.0, if multiple scripts - * using the old namespace id format of [lang#id] are found to have the - * same id but different languages an error will occur. */ public static ScriptMetadata fromXContent(XContentParser parser) throws IOException { Map scripts = new HashMap<>(); String id = null; - StoredScriptSource source; - StoredScriptSource exists; Token token = parser.currentToken(); @@ -189,52 +183,6 @@ public static ScriptMetadata fromXContent(XContentParser parser) throws IOExcept switch (token) { case FIELD_NAME: id = parser.currentName(); - break; - case VALUE_STRING: - if (id == null) { - throw new ParsingException(parser.getTokenLocation(), - "unexpected token [" + token + "], expected [, , {]"); - } - - int split = id.indexOf('#'); - String lang; - - if (split == -1) { - throw new IllegalArgumentException("illegal stored script id [" + id + "], does not contain lang"); - } else { - lang = id.substring(0, split); - id = id.substring(split + 1); - source = new StoredScriptSource(lang, parser.text(), Collections.emptyMap()); - - if (source.getSource().isEmpty()) { - if (source.getLang().equals(Script.DEFAULT_TEMPLATE_LANG)) { - deprecationLogger.critical( - DeprecationCategory.TEMPLATES, - "empty_templates", - "empty templates should no longer be used" - ); - } else { - deprecationLogger.critical( - DeprecationCategory.TEMPLATES, - "empty_scripts", - "empty scripts should no longer be used" - ); - } - } - } - - exists = scripts.get(id); - - if (exists == null) { - scripts.put(id, source); - } else if (exists.getLang().equals(lang) == false) { - throw new IllegalArgumentException("illegal stored script, id [" + id + "] used for multiple scripts with " + - "different languages [" + exists.getLang() + "] and [" + lang + "]; scripts using the old namespace " + - "of [lang#id] as a stored script id will have to be updated to use only the new namespace of [id]"); - } - - id = null; - break; case START_OBJECT: if (id == null) { @@ -242,31 +190,21 @@ public static ScriptMetadata fromXContent(XContentParser parser) throws IOExcept "unexpected token [" + token + "], expected [, , {]"); } - exists = scripts.get(id); - source = StoredScriptSource.fromXContent(parser, true); - - if (exists == null) { - // due to a bug (https://github.com/elastic/elasticsearch/issues/47593) - // scripts may have been retained during upgrade that include the old-style - // id of lang#id; these scripts are unreachable after 7.0, so they are dropped - if (id.contains("#") == false) { - scripts.put(id, source); + StoredScriptSource source = StoredScriptSource.fromXContent(parser, true); + // as of 8.0 we drop scripts/templates with an empty source + // this check should be removed for the next upgradable version after 8.0 + // since there is a guarantee no more empty scripts will exist + if (source.getSource().isEmpty()) { + if (Script.DEFAULT_TEMPLATE_LANG.equals(source.getLang())) { + logger.warn("empty template [" + id + "] found and dropped"); + } else { + logger.warn("empty script [" + id + "] found and dropped"); } - } else if (exists.getLang().equals(source.getLang()) == false) { - throw new IllegalArgumentException( - "illegal stored script, id [" - + id - + "] used for multiple scripts with different languages [" - + exists.getLang() - + "] and [" - + source.getLang() - + "]; scripts using the old namespace of [lang#id] as a stored script id will have to be updated " - + "to use only the new namespace of [id]" - ); + } else { + scripts.put(id, source); } id = null; - break; default: throw new ParsingException(parser.getTokenLocation(), "unexpected token [" + token + "], expected [, , {]"); @@ -318,8 +256,6 @@ public void writeTo(StreamOutput out) throws IOException { } } - - /** * This will write XContent from {@link ScriptMetadata}. The following format will be written: * diff --git a/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java b/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java index afcbd4ba07766..ac6e8089c134d 100644 --- a/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java +++ b/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java @@ -11,19 +11,18 @@ import org.elasticsearch.cluster.AbstractDiffable; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.Diff; -import org.elasticsearch.common.xcontent.ParseField; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.logging.DeprecationCategory; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.ObjectParser.ValueType; +import org.elasticsearch.common.xcontent.ParseField; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; @@ -63,7 +62,7 @@ public class StoredScriptSource extends AbstractDiffable imp /** * Standard {@link ParseField} for source on the inner level. */ - public static final ParseField SOURCE_PARSE_FIELD = new ParseField("source", "code"); + public static final ParseField SOURCE_PARSE_FIELD = new ParseField("source"); /** * Standard {@link ParseField} for options on the inner level. @@ -121,8 +120,7 @@ private void setOptions(Map options) { * Validates the parameters and creates an {@link StoredScriptSource}. * * @param ignoreEmpty Specify as {@code true} to ignoreEmpty the empty source check. - * This allow empty templates to be loaded for backwards compatibility. - * This allow empty templates to be loaded for backwards compatibility. + * This allow empty templates to be dropped for backwards compatibility. */ private StoredScriptSource build(boolean ignoreEmpty) { if (lang == null) { @@ -132,29 +130,13 @@ private StoredScriptSource build(boolean ignoreEmpty) { } if (source == null) { - if (ignoreEmpty || Script.DEFAULT_TEMPLATE_LANG.equals(lang)) { - if (Script.DEFAULT_TEMPLATE_LANG.equals(lang)) { - deprecationLogger.critical(DeprecationCategory.TEMPLATES, "empty_templates", - "empty templates should no longer be used"); - } else { - deprecationLogger.critical(DeprecationCategory.TEMPLATES, "empty_scripts", - "empty scripts should no longer be used"); - } + if (ignoreEmpty) { + source = ""; } else { throw new IllegalArgumentException("must specify source for stored script"); } - } else if (source.isEmpty()) { - if (ignoreEmpty || Script.DEFAULT_TEMPLATE_LANG.equals(lang)) { - if (Script.DEFAULT_TEMPLATE_LANG.equals(lang)) { - deprecationLogger.critical(DeprecationCategory.TEMPLATES, "empty_templates", - "empty templates should no longer be used"); - } else { - deprecationLogger.critical(DeprecationCategory.TEMPLATES, "empty_scripts", - "empty scripts should no longer be used"); - } - } else { - throw new IllegalArgumentException("source cannot be empty"); - } + } else if (source.isEmpty() && ignoreEmpty == false) { + throw new IllegalArgumentException("source cannot be empty"); } if (options.size() > 1 || options.size() == 1 && options.get(Script.CONTENT_TYPE_OPTION) == null) { @@ -175,21 +157,9 @@ private StoredScriptSource build(boolean ignoreEmpty) { } /** - * This will parse XContent into a {@link StoredScriptSource}. The following formats can be parsed: - * - * The simple script format with no compiler options or user-defined params: - * - * Example: - * {@code - * {"script": "return Math.log(doc.popularity) * 100;"} - * } + * This will parse XContent into a {@link StoredScriptSource}. * - * The above format requires the lang to be specified using the deprecated stored script namespace - * (as a url parameter during a put request). See {@link ScriptMetadata} for more information about - * the stored script namespaces. - * - * The complex script format using the new stored script namespace - * where lang and source are required but options is optional: + * Examples of legal JSON: * * {@code * { @@ -215,22 +185,6 @@ private StoredScriptSource build(boolean ignoreEmpty) { * } * } * - * The use of "source" may also be substituted with "code" for backcompat with 5.3 to 5.5 format. For example: - * - * {@code - * { - * "script" : { - * "lang" : "", - * "code" : "", - * "options" : { - * "option0" : "", - * "option1" : "", - * ... - * } - * } - * } - * } - * * Note that the "source" parameter can also handle template parsing including from * a complex JSON object. * @@ -249,12 +203,6 @@ public static StoredScriptSource parse(BytesReference content, XContentType xCon token = parser.nextToken(); - if (token == Token.END_OBJECT) { - deprecationLogger.critical(DeprecationCategory.TEMPLATES, "empty_templates", "empty templates should no longer be used"); - - return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap()); - } - if (token != Token.FIELD_NAME) { throw new ParsingException(parser.getTokenLocation(), "unexpected token [" + token + ", expected [" + SCRIPT_PARSE_FIELD.getPreferredName() + "]"); diff --git a/server/src/test/java/org/elasticsearch/script/ScriptMetadataTests.java b/server/src/test/java/org/elasticsearch/script/ScriptMetadataTests.java index c9a0aa2033097..f2793289e6088 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptMetadataTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptMetadataTests.java @@ -21,48 +21,9 @@ import java.io.IOException; import java.io.UncheckedIOException; -import java.util.Collections; public class ScriptMetadataTests extends AbstractSerializingTestCase { - public void testFromXContentLoading() throws Exception { - // failure to load to old namespace scripts with the same id but different langs - XContentBuilder builder = XContentFactory.jsonBuilder(); - builder.startObject().field("lang0#id0", "script0").field("lang1#id0", "script1").endObject(); - XContentParser parser0 = XContentType.JSON.xContent() - .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, - BytesReference.bytes(builder).streamInput()); - expectThrows(IllegalArgumentException.class, () -> ScriptMetadata.fromXContent(parser0)); - - // failure to load a new namespace script and old namespace script with the same id but different langs - builder = XContentFactory.jsonBuilder(); - builder.startObject().field("lang0#id0", "script0") - .startObject("id0").field("lang", "lang1").field("source", "script1").endObject().endObject(); - XContentParser parser1 = XContentType.JSON.xContent() - .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, - BytesReference.bytes(builder).streamInput()); - expectThrows(IllegalArgumentException.class, () -> ScriptMetadata.fromXContent(parser1)); - - // failure to load a new namespace script and old namespace script with the same id but different langs with additional scripts - builder = XContentFactory.jsonBuilder(); - builder.startObject().field("lang0#id0", "script0").field("lang0#id1", "script1") - .startObject("id1").field("lang", "lang0").field("source", "script0").endObject() - .startObject("id0").field("lang", "lang1").field("source", "script1").endObject().endObject(); - XContentParser parser2 = XContentType.JSON.xContent() - .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, - BytesReference.bytes(builder).streamInput()); - expectThrows(IllegalArgumentException.class, () -> ScriptMetadata.fromXContent(parser2)); - - // okay to load the same script from the new and old namespace if the lang is the same - builder = XContentFactory.jsonBuilder(); - builder.startObject().field("lang0#id0", "script0") - .startObject("id0").field("lang", "lang0").field("source", "script1").endObject().endObject(); - XContentParser parser3 = XContentType.JSON.xContent() - .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, - BytesReference.bytes(builder).streamInput()); - ScriptMetadata.fromXContent(parser3); - } - public void testGetScript() throws Exception { ScriptMetadata.Builder builder = new ScriptMetadata.Builder(null); @@ -126,72 +87,18 @@ public void testBuilder() { public void testLoadEmptyScripts() throws IOException { XContentBuilder builder = XContentFactory.jsonBuilder(); - builder.startObject().field("mustache#empty", "").endObject(); - XContentParser parser = XContentType.JSON.xContent() - .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, - BytesReference.bytes(builder).streamInput()); - ScriptMetadata.fromXContent(parser); - assertWarnings("empty templates should no longer be used"); - - builder = XContentFactory.jsonBuilder(); - builder.startObject().field("lang#empty", "").endObject(); - parser = XContentType.JSON.xContent() - .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, - BytesReference.bytes(builder).streamInput()); - ScriptMetadata.fromXContent(parser); - assertWarnings("empty scripts should no longer be used"); - - builder = XContentFactory.jsonBuilder(); builder.startObject().startObject("script").field("lang", "lang").field("source", "").endObject().endObject(); - parser = XContentType.JSON.xContent() + XContentParser parser = XContentType.JSON.xContent() .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, BytesReference.bytes(builder).streamInput()); - ScriptMetadata.fromXContent(parser); - assertWarnings("empty scripts should no longer be used"); + assertTrue(ScriptMetadata.fromXContent(parser).getStoredScripts().isEmpty()); builder = XContentFactory.jsonBuilder(); builder.startObject().startObject("script").field("lang", "mustache").field("source", "").endObject().endObject(); parser = XContentType.JSON.xContent() .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, BytesReference.bytes(builder).streamInput()); - ScriptMetadata.fromXContent(parser); - assertWarnings("empty templates should no longer be used"); - } - - public void testOldStyleDropped() throws IOException { - XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent()); - - builder.startObject(); - { - builder.startObject("painless#test"); - { - builder.field("lang", "painless"); - builder.field("source", "code"); - } - builder.endObject(); - builder.startObject("lang#test"); - { - builder.field("lang", "test"); - builder.field("source", "code"); - } - builder.endObject(); - builder.startObject("test"); - { - builder.field("lang", "painless"); - builder.field("source", "code"); - } - builder.endObject(); - } - builder.endObject(); - - XContentParser parser = XContentType.JSON.xContent() - .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, - BytesReference.bytes(builder).streamInput()); - ScriptMetadata smd = ScriptMetadata.fromXContent(parser); - assertNull(smd.getStoredScript("painless#test")); - assertNull(smd.getStoredScript("lang#test")); - assertEquals(new StoredScriptSource("painless", "code", Collections.emptyMap()), smd.getStoredScript("test")); - assertEquals(1, smd.getStoredScripts().size()); + assertTrue(ScriptMetadata.fromXContent(parser).getStoredScripts().isEmpty()); } @Override diff --git a/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java b/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java index fc72296f86f78..88320e9f46698 100644 --- a/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java +++ b/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java @@ -84,17 +84,6 @@ public void testSourceParsing() throws Exception { assertThat(parsed, equalTo(source)); } - // complex script using "code" backcompat - try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { - builder.startObject().field("script").startObject().field("lang", "lang").field("code", "code").endObject().endObject(); - - StoredScriptSource parsed = StoredScriptSource.parse(BytesReference.bytes(builder), XContentType.JSON); - StoredScriptSource source = new StoredScriptSource("lang", "code", Collections.emptyMap()); - - assertThat(parsed, equalTo(source)); - } - assertWarnings("Deprecated field [code] used, expected [source] instead"); - // complex script with script object and empty options try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { builder.startObject().field("script").startObject().field("lang", "lang").field("source", "code") @@ -165,25 +154,13 @@ public void testSourceParsingErrors() throws Exception { } public void testEmptyTemplateDeprecations() throws IOException { - try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { - builder.startObject().endObject(); - - StoredScriptSource parsed = StoredScriptSource.parse(BytesReference.bytes(builder), XContentType.JSON); - StoredScriptSource source = new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap()); - - assertThat(parsed, equalTo(source)); - assertWarnings("empty templates should no longer be used"); - } - try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { builder.startObject().field("script").startObject().field("lang", "mustache") .field("source", "").endObject().endObject(); - StoredScriptSource parsed = StoredScriptSource.parse(BytesReference.bytes(builder), XContentType.JSON); - StoredScriptSource source = new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap()); - - assertThat(parsed, equalTo(source)); - assertWarnings("empty templates should no longer be used"); + IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> + StoredScriptSource.parse(BytesReference.bytes(builder), XContentType.JSON)); + assertEquals("source cannot be empty", iae.getMessage()); } }