Skip to content

Commit

Permalink
Remove deprecated code from stored scripts (elastic#78643)
Browse files Browse the repository at this point in the history 
This change removes several pieces of deprecated code from stored scripts.

Stored scripts/templates are no longer allowed to be an empty and will throw an exception when used 
with PutStoredScript.

ScriptMetadata will now drop any existing stored scripts that are empty with a deprecation warning in 
the case they have not been previously removed.

The code field is now only allowed as source as part of a PutStoredScript JSON blob.
  • Loading branch information
@jdconrad
jdconrad committed Oct 5, 2021
1 parent a74573b commit 2cf160f
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 263 deletions.
27 changes: 27 additions & 0 deletions docs/reference/migration/migrate_8_0/scripting.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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[]
2 changes: 1 addition & 1 deletion server/src/internalClusterTest/java/org/elasticsearch/script/StoredScriptsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
94 changes: 15 additions & 79 deletions server/src/main/java/org/elasticsearch/script/ScriptMetadata.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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<String, StoredScriptSource> scripts = new HashMap<>();
String id = null;
StoredScriptSource source;
StoredScriptSource exists;

Token token = parser.currentToken();

Expand All @@ -189,84 +183,28 @@ 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 [<id>, <code>, {]");
}

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) {
throw new ParsingException(parser.getTokenLocation(),
"unexpected token [" + token + "], expected [<id>, <code>, {]");
}

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 [<id>, <code>, {]");
Expand Down Expand Up @@ -318,8 +256,6 @@ public void writeTo(StreamOutput out) throws IOException {
}
}



/**
* This will write XContent from {@link ScriptMetadata}. The following format will be written:
*
Expand Down
70 changes: 9 additions & 61 deletions server/src/main/java/org/elasticsearch/script/StoredScriptSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -63,7 +62,7 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> 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.
Expand Down Expand Up @@ -121,8 +120,7 @@ private void setOptions(Map<String, String> 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) {
Expand All @@ -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) {
Expand All @@ -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
* {
Expand All @@ -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" : "<lang>",
* "code" : "<source>",
* "options" : {
* "option0" : "<option0>",
* "option1" : "<option1>",
* ...
* }
* }
* }
* }
*
* Note that the "source" parameter can also handle template parsing including from
* a complex JSON object.
*
Expand All @@ -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() + "]");
Expand Down
Loading

0 comments on commit 2cf160f

Please sign in to comment.