From e63d7f4d336b3ea900be28777a3982333d341cf5 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Mon, 4 Oct 2021 11:14:46 -0700 Subject: [PATCH 1/9] remove dead code from parseXContent --- .../elasticsearch/script/ScriptMetadata.java | 86 +------------------ 1 file changed, 2 insertions(+), 84 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/script/ScriptMetadata.java b/server/src/main/java/org/elasticsearch/script/ScriptMetadata.java index 8f8b958b832a9..1e0536425c040 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptMetadata.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptMetadata.java @@ -18,8 +18,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; @@ -37,11 +35,6 @@ */ public final class ScriptMetadata implements Metadata.Custom, Writeable, ToXContentFragment { - /** - * Standard deprecation logger for used to deprecate allowance of empty templates. - */ - private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(ScriptMetadata.class); - /** * A builder used to modify the currently stored scripts data held within * the {@link ClusterState}. Scripts can be added or deleted, then built @@ -162,16 +155,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 +176,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 +183,10 @@ 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); - } - } 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]" - ); - } + StoredScriptSource source = StoredScriptSource.fromXContent(parser, true); + scripts.put(id, source); id = null; - break; default: throw new ParsingException(parser.getTokenLocation(), "unexpected token [" + token + "], expected [, , {]"); @@ -318,8 +238,6 @@ public void writeTo(StreamOutput out) throws IOException { } } - - /** * This will write XContent from {@link ScriptMetadata}. The following format will be written: * From c65b267a53945ac22d5e8d3038a2df74536051d6 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Mon, 4 Oct 2021 12:21:52 -0700 Subject: [PATCH 2/9] remove deprecations and disallow empty scripts/templates --- .../elasticsearch/script/ScriptMetadata.java | 22 +++++- .../script/StoredScriptSource.java | 67 +++---------------- .../script/StoredScriptTests.java | 29 +------- 3 files changed, 32 insertions(+), 86 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/script/ScriptMetadata.java b/server/src/main/java/org/elasticsearch/script/ScriptMetadata.java index 1e0536425c040..c922431f7029f 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptMetadata.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptMetadata.java @@ -18,6 +18,8 @@ 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; @@ -35,6 +37,11 @@ */ public final class ScriptMetadata implements Metadata.Custom, Writeable, ToXContentFragment { + /** + * Standard deprecation logger for used to deprecate allowance of empty templates. + */ + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(ScriptMetadata.class); + /** * A builder used to modify the currently stored scripts data held within * the {@link ClusterState}. Scripts can be added or deleted, then built @@ -184,7 +191,20 @@ public static ScriptMetadata fromXContent(XContentParser parser) throws IOExcept } StoredScriptSource source = StoredScriptSource.fromXContent(parser, true); - scripts.put(id, source); + // 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())) { + deprecationLogger.critical(DeprecationCategory.TEMPLATES, "empty_templates", + "empty template found and dropped"); + } else { + deprecationLogger.critical(DeprecationCategory.TEMPLATES, "empty_scripts", + "empty script found and dropped"); + } + } else { + scripts.put(id, source); + } id = null; break; diff --git a/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java b/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java index afcbd4ba07766..d3d019a940ebe 100644 --- a/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java +++ b/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java @@ -63,7 +63,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 +121,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 +131,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 +158,9 @@ private StoredScriptSource build(boolean ignoreEmpty) { } /** - * This will parse XContent into a {@link StoredScriptSource}. The following formats can be parsed: + * This will parse XContent into a {@link StoredScriptSource}. * - * The simple script format with no compiler options or user-defined params: - * - * Example: - * {@code - * {"script": "return Math.log(doc.popularity) * 100;"} - * } - * - * 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 +186,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 +204,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/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()); } } From defb1afe582a1ed55f67b4c2b55db86c5d93adc2 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Mon, 4 Oct 2021 12:35:46 -0700 Subject: [PATCH 3/9] add breaking changes docs --- .../migration/migrate_8_0/scripting.asciidoc | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/docs/reference/migration/migrate_8_0/scripting.asciidoc b/docs/reference/migration/migrate_8_0/scripting.asciidoc index f4e9724d04c63..087217357ab35 100644 --- a/docs/reference/migration/migrate_8_0/scripting.asciidoc +++ b/docs/reference/migration/migrate_8_0/scripting.asciidoc @@ -22,4 +22,29 @@ 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. ==== + +.Empty scripts/templates are no longer allowed as stored scripts. +[%collapsible] +==== +*Details* + +Empty scripts/templates cannot be specified as the source of a +PutStoredScript action. + +*Impact* + +Before upgrading, remove stored scripts/templates that are empty. Otherwise, +these scripts will be dropped, and may not align with the scripts on older +nodes. +==== + +.The `code` field can no longer be specified as part of a stored script/template. +[%collapsible] +==== +*Details* + +The `code` field is replaced by the `source` as the only way to specify the +source of a script. + +*Impact* + +Before upgrading, any calls to PutStoredScript should use `source` instead +of `code`. +==== // end::notable-breaking-changes[] \ No newline at end of file From ea688c17f094d2b70f23670cd740e15024e78d60 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Mon, 4 Oct 2021 12:43:28 -0700 Subject: [PATCH 4/9] fix doc word --- docs/reference/migration/migrate_8_0/scripting.asciidoc | 4 ++-- .../java/org/elasticsearch/script/StoredScriptSource.java | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/reference/migration/migrate_8_0/scripting.asciidoc b/docs/reference/migration/migrate_8_0/scripting.asciidoc index 087217357ab35..2ff10be3497c1 100644 --- a/docs/reference/migration/migrate_8_0/scripting.asciidoc +++ b/docs/reference/migration/migrate_8_0/scripting.asciidoc @@ -40,8 +40,8 @@ nodes. [%collapsible] ==== *Details* + -The `code` field is replaced by the `source` as the only way to specify the -source of a script. +The `code` field is replaced by the `source` field as the only way to specify +the source of a script. *Impact* + Before upgrading, any calls to PutStoredScript should use `source` instead diff --git a/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java b/server/src/main/java/org/elasticsearch/script/StoredScriptSource.java index d3d019a940ebe..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; From ff5cf0b83377b9644043168bd17a2ec28d4f39f2 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Mon, 4 Oct 2021 13:29:37 -0700 Subject: [PATCH 5/9] update script metadata tests --- .../script/ScriptMetadataTests.java | 96 +------------------ 1 file changed, 3 insertions(+), 93 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/script/ScriptMetadataTests.java b/server/src/test/java/org/elasticsearch/script/ScriptMetadataTests.java index c9a0aa2033097..bd4da8985e458 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptMetadataTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptMetadataTests.java @@ -25,44 +25,6 @@ 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,28 +88,12 @@ 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"); + assertWarnings("empty script found and dropped"); builder = XContentFactory.jsonBuilder(); builder.startObject().startObject("script").field("lang", "mustache").field("source", "").endObject().endObject(); @@ -155,43 +101,7 @@ public void testLoadEmptyScripts() throws IOException { .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()); + assertWarnings("empty template found and dropped"); } @Override From 725d15630dae3ebe70a304ac143e434a810e6290 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Mon, 4 Oct 2021 13:36:49 -0700 Subject: [PATCH 6/9] fix import --- .../test/java/org/elasticsearch/script/ScriptMetadataTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/script/ScriptMetadataTests.java b/server/src/test/java/org/elasticsearch/script/ScriptMetadataTests.java index bd4da8985e458..51d71c8ac20b8 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptMetadataTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptMetadataTests.java @@ -21,7 +21,6 @@ import java.io.IOException; import java.io.UncheckedIOException; -import java.util.Collections; public class ScriptMetadataTests extends AbstractSerializingTestCase { From c3287e8773fd2382ab9966a8180004669f71c995 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Mon, 4 Oct 2021 14:26:33 -0700 Subject: [PATCH 7/9] update stored script it --- .../java/org/elasticsearch/script/StoredScriptsIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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()); } From 79f19e71401446f1523953a4fd5654787d7bf488 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Tue, 5 Oct 2021 09:37:11 -0700 Subject: [PATCH 8/9] switch from deprecation logger to a standard logger --- .../org/elasticsearch/script/ScriptMetadata.java | 14 ++++++-------- .../elasticsearch/script/ScriptMetadataTests.java | 6 ++---- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/script/ScriptMetadata.java b/server/src/main/java/org/elasticsearch/script/ScriptMetadata.java index c922431f7029f..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 @@ -196,11 +196,9 @@ public static ScriptMetadata fromXContent(XContentParser parser) throws IOExcept // since there is a guarantee no more empty scripts will exist if (source.getSource().isEmpty()) { if (Script.DEFAULT_TEMPLATE_LANG.equals(source.getLang())) { - deprecationLogger.critical(DeprecationCategory.TEMPLATES, "empty_templates", - "empty template found and dropped"); + logger.warn("empty template [" + id + "] found and dropped"); } else { - deprecationLogger.critical(DeprecationCategory.TEMPLATES, "empty_scripts", - "empty script found and dropped"); + logger.warn("empty script [" + id + "] found and dropped"); } } else { scripts.put(id, source); diff --git a/server/src/test/java/org/elasticsearch/script/ScriptMetadataTests.java b/server/src/test/java/org/elasticsearch/script/ScriptMetadataTests.java index 51d71c8ac20b8..f2793289e6088 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptMetadataTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptMetadataTests.java @@ -91,16 +91,14 @@ public void testLoadEmptyScripts() throws IOException { XContentParser parser = XContentType.JSON.xContent() .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, BytesReference.bytes(builder).streamInput()); - ScriptMetadata.fromXContent(parser); - assertWarnings("empty script found and dropped"); + 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 template found and dropped"); + assertTrue(ScriptMetadata.fromXContent(parser).getStoredScripts().isEmpty()); } @Override From 943bb151e452ead7a9dd2aea1ae195bed01f5000 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Tue, 5 Oct 2021 09:44:49 -0700 Subject: [PATCH 9/9] update docs based on pr feedback --- .../migration/migrate_8_0/scripting.asciidoc | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/docs/reference/migration/migrate_8_0/scripting.asciidoc b/docs/reference/migration/migrate_8_0/scripting.asciidoc index 2ff10be3497c1..04a5ff40a6231 100644 --- a/docs/reference/migration/migrate_8_0/scripting.asciidoc +++ b/docs/reference/migration/migrate_8_0/scripting.asciidoc @@ -23,28 +23,30 @@ in a compilation error or runtime error and may not allow the upgraded node to start. ==== -.Empty scripts/templates are no longer allowed as stored scripts. +.Stored scripts no longer support empty scripts or search templates. [%collapsible] ==== *Details* + -Empty scripts/templates cannot be specified as the source of a -PutStoredScript action. +The {ref}/create-stored-script-api.html[create or update stored script API]'s +`source` parameter cannot be empty. *Impact* + -Before upgrading, remove stored scripts/templates that are empty. Otherwise, -these scripts will be dropped, and may not align with the scripts on older -nodes. +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 `code` field can no longer be specified as part of a stored script/template. +.The create or update stored script API's `code` parameter has been removed. [%collapsible] ==== *Details* + -The `code` field is replaced by the `source` field as the only way to specify -the source of a script. +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* + -Before upgrading, any calls to PutStoredScript should use `source` instead -of `code`. +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