Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove deprecated code from stored scripts #78643

Merged
merged 10 commits into from
Oct 5, 2021
Merged
27 changes: 27 additions & 0 deletions docs/reference/migration/migrate_8_0/scripting.asciidoc
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[]
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
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
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