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

Add Groovy as a scripting language, add groovy sandboxing #6233

Merged
merged 1 commit into from Jun 20, 2014

Conversation

Projects
None yet
8 participants
@dakrone
Copy link
Member

commented May 19, 2014

Sandboxes the groovy scripting language with multiple configurable
whitelists:

script.groovy.sandbox.receiver_whitelist: comma-separated list of string
classes for objects that may have methods invoked.
script.groovy.sandbox.package_whitelist: comma-separated list of
packages under which new objects may be constructed.
script.groovy.sandbox.class_whitelist comma-separated list of classes
that are allowed to be constructed.

As well as a method blacklist:

script.groovy.sandbox.method_blacklist: comma-separated list of
methods that are never allowed to be invoked, regardless of target
object.

The sandbox can be entirely disabled by setting:

script.groovy.sandbox.enabled: false

@dakrone dakrone added v1.3.0 labels May 19, 2014

@s1monw

View changes

docs/reference/modules/scripting.asciidoc Outdated
@@ -6,28 +6,30 @@ expressions. For example, scripts can be used to return "script fields"
as part of a search request, or can be used to evaluate a custom score
for a query and so on.

The scripting module uses by default http://mvel.codehaus.org/[mvel] as

This comment has been minimized.

Copy link
@s1monw

s1monw May 19, 2014

Contributor

we should really have a section that says that we added this in 1.3 and where to find the docs for the prev. version?

This comment has been minimized.

Copy link
@dakrone

dakrone May 19, 2014

Author Member

Sure, I added the "coming in 1.3.0" part, but not sure where to link for older documentation since 1.2 hasn't been released yet. Once it's released I'll add a commit to point to the older documentation.

This comment has been minimized.

Copy link
@clintongormley

clintongormley May 19, 2014

Member

Docs are built for branches, ie 1.x, master, not for released versions. So you should use a deprecated tag for mvel eg deprecated[1.3.0,Replace by groovy as the default scripting language, see old docs here <>]

This comment has been minimized.

Copy link
@s1monw

s1monw May 23, 2014

Contributor

@dakrone can you address clintons comments?

This comment has been minimized.

Copy link
@dakrone

dakrone May 30, 2014

Author Member

Added a commit to rearrange and (hopefully) clarify the mvel deprecation.

@@ -1346,7 +1354,7 @@
<version>0.6.4.201312101107</version>

This comment has been minimized.

Copy link
@s1monw

s1monw May 19, 2014

Contributor

do we want to shade groovy as we did with mvel? and remove mvel?

This comment has been minimized.

Copy link
@dakrone

dakrone May 19, 2014

Author Member

I'm not really sure what shading benefits us? Is there a reason to shade groovy?

As for removing mvel, I think that should be a PR from this one so we can discuss whether it goes to 1.3 or just 2.0.

This comment has been minimized.

Copy link
@s1monw

s1monw May 23, 2014

Contributor

maybe @kimchy can comment on this?

This comment has been minimized.

Copy link
@pickypg

pickypg May 27, 2014

Member

@dakrone If it's not shaded, and a Groovy client uses any of the popular Elasticsearch Groovy clients (only use Groovy with Gradle myself, so I am not aware of the popular ones), then will it cause a conflict/collision?

I'd expect not unless the Groovy scripts were interpreted on the client, which would be odd. Assuming it's not needed in some way by the client, then shading seems unnecessary.

This comment has been minimized.

Copy link
@nik9000

nik9000 May 28, 2014

Contributor

@dakrone If it's not shaded, and a Groovy client uses any of the popular Elasticsearch Groovy clients (only use Groovy with Gradle myself, so I am not aware of the popular ones), then will it cause a conflict/collision?

I think this'd still happen. The JVM normally loads each class only once [1] so you'll just get whichever groovy it finds first. Shading fixes this by copying the class to a new name.

[1]: technically its one per class loader and class loaders are hierarchical....

So, yeah, I'm +1 for shading.

This comment has been minimized.

Copy link
@pickypg

pickypg May 28, 2014

Member

@nik9000 Just to add: The JVM is very good about loading classes from the same jar only once. However, it is possible to get into situations where different versions of the same jar(s) are on the classpath that cause ClassNotFound issues (because of a lack of isolation, which can be solved with class loaders) because the different versions cannot coexist. My major point that I was trying to get at is that I believe this is a server side dependency only, and that should mean that there will never be a chance for collision except in the embedded use case (ignoring any potential forked projects that use Groovy, which should just drop whichever one they want to drop).

I think the embedded concern is what makes me push +1 for shading as well, although I think it would be preferable if we could avoid shading because it would be lighter weight given its intended-to-be optional nature.

This comment has been minimized.

Copy link
@nik9000

nik9000 May 28, 2014

Contributor

My major point that I was trying to get at is that I believe this is a server side dependency only

it would be preferable if we could avoid shading because it would be lighter weight given its intended-to-be optional nature.

Makes sense to me. Well put.

This comment has been minimized.

Copy link
@dakrone

dakrone May 30, 2014

Author Member

We discussed this and decided not to shade groovy as it negates the "optional" nature of it and it will reduce the jar size for clients using only the transport client.

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 12, 2014

Contributor

good! I also wonder if we want to remove mvel from shading as well and mark it optional as well?

@s1monw

View changes

src/main/java/org/elasticsearch/script/groovy/GroovySandboxExpressionChecker.java Outdated
"java.util.Arrays",
"java.util.HashMap",
"java.util.HashSet",
"java.util.UUID",

This comment has been minimized.

Copy link
@s1monw

s1monw May 19, 2014

Contributor

UUID? I guess if we make this default here we should really add a lot more though?

This comment has been minimized.

Copy link
@dakrone

dakrone May 19, 2014

Author Member

@s1monw what other classes do you see people creating that we need to whitelist?

It's also configurable, so users aren't locked in if they need to add other classes.

}

// Never allow calling these methods, regardless of the object type
public static String[] defaultMethodBlacklist = new String[]{

This comment has been minimized.

Copy link
@s1monw

s1monw May 19, 2014

Contributor

can we block calls to java.lang.System and friends?

This comment has been minimized.

Copy link
@dakrone

dakrone May 19, 2014

Author Member

These are already blocked by the Receiver whitelist, the blacklist is for methods from Object like .getClass() that every object has.

@s1monw

This comment has been minimized.

Copy link
Contributor

commented May 19, 2014

oh man this looks awesome!

@rmuir

View changes

src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java Outdated
"java.lang.Double", "[D", "[[D", "[[[D",
"java.lang.Long", "[J", "[[J", "[[[J",
"java.lang.Short", "[S", "[[S", "[[[S",
"java.lang.Char", "[C", "[[C", "[[[C",

This comment has been minimized.

Copy link
@rmuir

rmuir May 20, 2014

Contributor

shouldn't this be java.lang.Character?

This comment has been minimized.

Copy link
@dakrone

dakrone May 20, 2014

Author Member

Whoops, good catch, fixed.

@uboness

View changes

src/main/java/org/elasticsearch/script/groovy/GroovySandboxExpressionChecker.java Outdated

// Classes that are allowed to be constructed
public static String[] defaultClassConstructionWhitelist = new String[]{
"java.util.Date",

This comment has been minimized.

Copy link
@uboness

uboness May 20, 2014

Contributor

That's error prone, why not use .class.getName()?

This comment has been minimized.

Copy link
@dakrone

dakrone May 20, 2014

Author Member

good idea, I'll do that.

@uboness

View changes

src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java Outdated

// Default whitelisted receiver classes for the Groovy sandbox
private final static String[] defaultReceiverWhitelist = new String [] {
"java.lang.Math",

This comment has been minimized.

Copy link
@uboness

uboness May 20, 2014

Contributor

Again .class.getName()?

@s1monw

View changes

src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java Outdated

// Default whitelisted receiver classes for the Groovy sandbox
private final static String[] defaultReceiverWhitelist = new String [] {
java.lang.Math.class.getName(),

This comment has been minimized.

Copy link
@s1monw

s1monw May 23, 2014

Contributor

can we have a test for these that they all actually work? I am not entirely sure how this works but do we include subclasses? those are mostly interfaces so my question is can I create new HashMap() since it's not here?

This comment has been minimized.

Copy link
@dakrone

dakrone May 30, 2014

Author Member

Added tests to make sure the map/list/range datatype receivers whitelist works. It's possible to create a hashmap using Groovy's [key:value, key2:value, etc] syntax and call methods on it, since java.lang.Map is in the receivers whitelist.

@pickypg

View changes

src/main/java/org/elasticsearch/script/ScriptModule.java Outdated
try {
multibinder.addBinding().to(GroovyScriptEngineService.class);
} catch (Throwable t) {
// no Groovy

This comment has been minimized.

Copy link
@pickypg

pickypg May 27, 2014

Member

Feel like this kind of thing should be logged for debug purposes (realizing it was not done previously).

This comment has been minimized.

Copy link
@nik9000

nik9000 May 28, 2014

Contributor

It might be worth logging it at warning, maybe. Its not "normal" not to have it but is "OK". Its just one line on startup and so long as the line clearly states that everything is ok, we're just disabling groovy, then I think it should be logged every startup.

This comment has been minimized.

Copy link
@nik9000

nik9000 May 28, 2014

Contributor

Or info

This comment has been minimized.

Copy link
@dakrone

dakrone May 30, 2014

Author Member

Added some logging for this.

@nik9000

View changes

docs/reference/modules/scripting.asciidoc Outdated
extremely fast and very simple to use, and in most cases, simple
The scripting module uses by default http://groovy.codehaus.org/[groovy]
as the scripting language with some extensions. Groovy is used since it
is extremely fast and very simple to use, and in most cases, simple
expressions are needed (for example, mathematical equations).

This comment has been minimized.

Copy link
@nik9000

nik9000 May 28, 2014

Contributor

Do we need the ", simple expressions are needed ..." part? Its not really important for groovy which is known to be pretty fully featured and respectable. At least as respectable as perl and people write all kinds of stuff in perl (poke @clintongormley).

@nik9000

View changes

docs/reference/modules/scripting.asciidoc Outdated
All places where a `script` parameter can be used, a `lang` parameter
(on the same level) can be provided to define the language of the
script. The `lang` options are `mvel`, `js`, `groovy`, `python`, and
script. The `lang` options are `groovy`, `js`, `mvel`, `python`, and

This comment has been minimized.

Copy link
@nik9000

nik9000 May 28, 2014

Contributor

maybe sort these in the same order as the plugin list above while you are here?

This comment has been minimized.

Copy link
@dakrone

dakrone May 30, 2014

Author Member

They already are in the same order, Groovy is just mentioned first since it's the default.

This comment has been minimized.

Copy link
@nik9000

nik9000 May 30, 2014

Contributor

Sorry, was getting confused reading the - and + lines. Thanks.

@nik9000

View changes

docs/reference/modules/scripting.asciidoc Outdated
be used. Once a script has been placed in this directory, it can be referenced
by name. For example, a script called `calculate-score.mvel` can be referenced
in a request like this:
added[1.3.0, Groovy has replaced Mvel as the default scripting language since version 1.3.0]

This comment has been minimized.

Copy link
@nik9000

nik9000 May 28, 2014

Contributor

This line looks out of place

@nik9000

View changes

docs/reference/modules/scripting.asciidoc Outdated
@@ -275,8 +307,6 @@ They include:
[cols="<,<",options="header",]
|=======================================================================
|Function |Description
|`time()` |The current time in milliseconds.

This comment has been minimized.

Copy link
@nik9000

nik9000 May 28, 2014

Contributor

Is this too painful to do? I know I use it in MVEL, though I'll probably switch distance stuff before long. I imagine System.currentTimeMillis() still works, right?

This comment has been minimized.

Copy link
@dakrone

dakrone May 30, 2014

Author Member

The time() method is still gone, but you can be use Instant.now().getMillis() (from jodatime) for the current time in scripts as well (calls to System.<anything> are blocked).

This comment has been minimized.

Copy link
@nik9000

nik9000 May 30, 2014

Contributor

Wanting to get the time sounds like a pretty common so I imagine adding some documentation about how to get it would be useful.

// Classes that are allowed to be constructed
public static String[] defaultClassConstructionWhitelist = new String[]{
java.util.Date.class.getName(),
java.util.Map.class.getName(),

This comment has been minimized.

Copy link
@nik9000

nik9000 May 28, 2014

Contributor

Might want to document why you need to allow construction of interfaces.

@nik9000

View changes

src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java Outdated
private final boolean sandboxed;

// Default whitelisted receiver classes for the Groovy sandbox
private final static String[] defaultReceiverWhitelist = new String [] {

This comment has been minimized.

Copy link
@nik9000

nik9000 May 28, 2014

Contributor

Maybe merge all the groovy securing code into one place? It feels funky to have the default receiver whitelist here but method blacklist above.

This comment has been minimized.

Copy link
@dakrone

dakrone May 30, 2014

Author Member

I agree, it's cleaner to have it all in one place, I've moved the location of this into the ExpressionChecker.

@nik9000

View changes

src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java Outdated
scz.setClosuresAllowed(true);
// But defining methods is not
scz.setMethodDefinitionAllowed(false);
// Only allow the imports that we explicitly call out

This comment has been minimized.

Copy link
@nik9000

nik9000 May 28, 2014

Contributor

Does this block the equivalent of Java's import or rubys require? Like, without these imports you can't execute the methods at all? So, like, are these to block static method calls?

This comment has been minimized.

Copy link
@dakrone

dakrone May 30, 2014

Author Member

They're mostly to provide a more helpful error when someone attempts to import sun.misc.Unsafe.


@SuppressWarnings({"unchecked"})
@Override
public SearchScript search(Object compiledScript, SearchLookup lookup, @Nullable Map<String, Object> vars) {

This comment has been minimized.

Copy link
@nik9000

nik9000 May 28, 2014

Contributor

Search and executable, and execute look like they should delegate stuff to a single method. Orsomething.

This comment has been minimized.

Copy link
@dakrone

dakrone Jun 3, 2014

Author Member

I think the extra code needed to bundle them into a single method would be about the same length as the duplication, with the lookup checking and different exception messages.

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 12, 2014

Contributor

yeah I think it would make sense to add

private Script getScript(Object compiledScript, Map<String, Object> vars) {
   Class scriptClass = (Class) compiledScript;
   Script scriptObject = (Script) scriptClass.newInstance();
   Binding binding = new Binding(vars);
   scriptObject.setBinding(binding);
   return scriptObject;
}

and if you have 2 vars maps you construct a new one?

@nik9000

View changes

src/test/java/org/elasticsearch/script/GroovySandboxScriptTests.java Outdated
.setSource("{\"query\": {\"match_all\": {}}," +
"\"sort\":{\"_script\": {\"script\": \"doc['foo'].value + 2\", \"type\": \"number\"}}}").get();
assertNoFailures(resp);
assertThat(resp.getHits().getAt(0).getSortValues(), equalTo(new Object[]{7.0}));

This comment has been minimized.

Copy link
@nik9000

nik9000 May 28, 2014

Contributor

It'd be nice to know that some more things we expect to work do - stuff like the current time, all the listed methods. Most of the the stuff you need to fake out scoring is in the IndexLookupTests so that is cool.

@dakrone

This comment has been minimized.

Copy link
Member Author

commented Jun 3, 2014

Added a number of commits to address the feedback, @s1monw can you take another look?

@@ -74,7 +74,7 @@ public ScriptService(Settings settings, Environment env, Set<ScriptEngineService
TimeValue cacheExpire = componentSettings.getAsTime("cache.expire", null);
logger.debug("using script cache with max_size [{}], expire [{}]", cacheMaxSize, cacheExpire);

this.defaultLang = componentSettings.get("default_lang", "mvel");

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 12, 2014

Contributor

any chance we can make this a constant?

This comment has been minimized.

Copy link
@dakrone

dakrone Jun 16, 2014

Author Member

Yes, I will make everything a constant that can be for this.

@s1monw

View changes

src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java Outdated
*/
public class GroovyScriptEngineService extends AbstractComponent implements ScriptEngineService {

public static String GROOVY_SCRIPT_SANDBOX_ENABLED = "script.groovy.sandbox_enabled";

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 12, 2014

Contributor

should this just be script.groovy.sandboxed ?

This comment has been minimized.

Copy link
@pickypg

pickypg Jun 13, 2014

Member

Perhaps it should just be script.groovy.sandbox? It would then serve as the root property of all the child properties of it in a readable way:

script.groovy.sandbox = true
script.groovy.sandbox.class_whitelist = ...
script.groovy.sandbox.package_whitelist = ...

Versus:

script.groovy.sandboxed = true
script.groovy.sandbox.class_whitelist = ...
script.groovy.sandbox.package_whitelist = ...

Or, to go back to enabled, even script.groovy.sandbox.enabled. Longer, but I think it makes it better because it lives in the same hierarchy as the other sandbox properties.

This comment has been minimized.

Copy link
@dakrone

dakrone Jun 16, 2014

Author Member

I'm inclined to go with script.groovy.sandbox.enabled as it makes all settings live underneath the script.groovy.sandbox map, it also looks better when using indented yaml:

script:
  groovy:
    sandbox:
      enabled: true
      class_whitelist: ...
      package_whitelist: ...

This comment has been minimized.

Copy link
@pickypg

pickypg Jun 16, 2014

Member

+1 to that for the yaml configuration.

@s1monw

View changes

src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java Outdated
@SuppressWarnings({"unchecked"})
@Override
public void setNextVar(String name, Object value) {
script.getBinding().getVariables().put(name, value);

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 12, 2014

Contributor

maybe we can optimize this and pull the map into a member variable? during construction?

This comment has been minimized.

Copy link
@dakrone

dakrone Jun 16, 2014

Author Member

Yes, I'll do that.

@s1monw

View changes

src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java Outdated
@SuppressWarnings({"unchecked"})
@Override
public void setNextScore(float score) {
script.getBinding().getVariables().put("_score", score);

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 12, 2014

Contributor

same as above....

we might want to think about adding something like an updateableFloat to the map only once in the ctor and
then only update it here which safes us the lookup / update for score on every document? ie...

   private static final class UpdateableFloat extends Number {
        float value;
        @Override
        public int intValue() {
            return (int)value;
        }

        @Override
        public long longValue() {
            return (long)value;
        }

        @Override
        public float floatValue() {
            return value;
        }

        @Override
        public double doubleValue() {
            return value;
        }
    }

and then we can simply update the var:

public static class GroovySearchScript implements SearchScript {

        private final Script script;
        private final SearchLookup lookup;
        private final Map variables;
        private final UpdateableFloat score = new UpdateableFloat();

        public GroovySearchScript(Script script, SearchLookup lookup) {
            this.script = script;
            this.lookup = lookup;
            this.variables = script.getBinding().getVariables();
            variables.put("_score", score);

        }

        @SuppressWarnings({"unchecked"})
        @Override
        public void setNextScore(float score) {
            this.score.value = score;
        }

I just checked if it works and it seems to be just fine. Yet, I found that we don't test this at all in our code, can you add some tests for _score. Ping @brwe she had some benchmarks that we can turn into tests...

This comment has been minimized.

Copy link
@dakrone

dakrone Jun 16, 2014

Author Member

Added some tests for _score in #6509

@s1monw

View changes

src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java Outdated
}
}

public static class GroovySearchScript implements SearchScript {

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 12, 2014

Contributor

I wonder if the two script classes can share some code? and be final?

This comment has been minimized.

Copy link
@dakrone

dakrone Jun 16, 2014

Author Member

I believe they can, I'll make that change.

@s1monw

View changes

src/test/java/org/elasticsearch/action/bench/BenchmarkIntegrationTest.java Outdated
@@ -61,14 +61,14 @@

protected synchronized Settings nodeSettings(int nodeOrdinal) {
if (nodeOrdinal == 0) { // at least one
return ImmutableSettings.builder().put("node.bench", true).build();
return ImmutableSettings.builder().put("node.bench", true).put("script.groovy.sandbox_enabled", false).build();

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 12, 2014

Contributor

please use constants here

@s1monw

View changes

src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsTests.java Outdated
@@ -951,7 +951,7 @@ public void script_Score() {
SearchResponse response = client().prepareSearch("idx").setTypes("type")
.setQuery(functionScoreQuery(matchAllQuery()).add(ScoreFunctionBuilders.scriptFunction("doc['" + SINGLE_VALUED_FIELD_NAME + "'].value")))
.addAggregation(terms("terms")
.script("ceil(_doc.score/3)")
.script("ceil(_doc.score()/3)")

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 12, 2014

Contributor

mabye we can use _score here too?

This comment has been minimized.

Copy link
@dakrone

dakrone Jun 13, 2014

Author Member

I looked into this, _score doesn't work for either Groovy or Mvel (probably due to this being in an aggregation?). If this is something we want to support I think we should open a separate issue for it.

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 16, 2014

Contributor

oh I see makes sense!

@s1monw

View changes

src/test/java/org/elasticsearch/search/timeout/SearchTimeoutTests.java Outdated

@Override
protected Settings nodeSettings(int nodeOrdinal) {
return ImmutableSettings.settingsBuilder().put(super.nodeSettings(nodeOrdinal)).put("script.groovy.sandbox_enabled", false).build();

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 12, 2014

Contributor

use constants here?

@s1monw

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2014

did another review.... looks pretty close... left some commetns

@s1monw s1monw removed the review label Jun 12, 2014

@s1monw

View changes

src/main/java/org/elasticsearch/script/groovy/UpdateableFloat.java Outdated
/**
* Float encapsulation that allows updating the value with public member access
*/
public class UpdateableFloat extends Number {

This comment has been minimized.

Copy link
@s1monw

s1monw Jun 16, 2014

Contributor

IMO we should make this class package private and final? Can you also put in the comment where this is used and for what reason just be a little more verbose.... we might also just put it as a inner class?

@dakrone

This comment has been minimized.

Copy link
Member Author

commented Jun 16, 2014

@s1monw updated this PR with all of the changes you recommended :)

@dakrone dakrone added the review label Jun 17, 2014

@s1monw

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2014

this actually LGTM maybe @kimchy want's to take a look?
good job!

@kimchy

This comment has been minimized.

Copy link
Member

commented Jun 18, 2014

@dakrone this looks great!, a few comments:

  • I would add a 3 value setting to dynamic script enable now, true, false, and sandbox.

For 2.0:

  • I would remove mvel completely
  • Default to sandbox in dynamic script setting, and groovy as the lang.

For 1.3:

  • I would suggest also removing mvel completely, aside from security, its just buggy, if we decide to, then its the same changes as 2.0, including properly documenting how to install the mvel plugin and setting mvel as the lang by default, and properly mark it as a breaking change.
  • If there is resistance from removing mvel, then keep mvel around and keep it as the default lang, next to groovy being an option. Default dynamic script should still be sandbox.
@dakrone

This comment has been minimized.

Copy link
Member Author

commented Jun 18, 2014

@kimchy Sounds good, I updated the documentation and completely removed mvel as well. I also added the 3-value setting for dynamic scripting.

@dakrone dakrone added the breaking label Jun 18, 2014

@s1monw

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2014

so this means that upgrading to 1.3 from any prev version means you need to install the mvel plugin on the upgrade and pass mvel as a param in the request to opt in to this language? I think we really need to invest into an upgrade guide here!

@s1monw s1monw removed the review label Jun 18, 2014

@dakrone

This comment has been minimized.

Copy link
Member Author

commented Jun 19, 2014

@s1monw yes, the mvel plugin will have to be installed and the script.default_lang: mvel option would have to be added to continue using mvel as the default.

For an upgrade guide, do we want to add the two settings to the release notes? It will also be on the scripting blog post to come out after this is merged as well.

@dakrone dakrone changed the title Add Groovy as a scripting language, change default from mvel -> groovy Add Groovy as a scripting language, add groovy sandboxing Jun 20, 2014

@dakrone dakrone removed the breaking label Jun 20, 2014

Add Groovy as a scripting language, add sandboxing for Groovy
Sandboxes the groovy scripting language with multiple configurable
whitelists:

`script.groovy.sandbox.receiver_whitelist`: comma-separated list of string
classes for objects that may have methods invoked.
`script.groovy.sandbox.package_whitelist`: comma-separated list of
packages under which new objects may be constructed.
`script.groovy.sandbox.class_whitelist` comma-separated list of classes
that are allowed to be constructed.

As well as a method blacklist:

`script.groovy.sandbox.method_blacklist`: comma-separated list of
methods that are never allowed to be invoked, regardless of target
object.

The sandbox can be entirely disabled by setting:

`script.groovy.sandbox.enabled: false`

@dakrone dakrone merged commit c70f6d0 into elastic:master Jun 20, 2014

@clintongormley clintongormley changed the title Add Groovy as a scripting language, add groovy sandboxing Scripting: Add Groovy as a scripting language, add groovy sandboxing Jul 16, 2014

dadoonet added a commit to elastic/elasticsearch-river-couchdb that referenced this pull request Jul 18, 2014

Use groovy as default scripting language
As for elasticsearch 1.3.0, `groovy` is the new default scripting language.

Related to: elastic/elasticsearch#6233

Closes #61.
(cherry picked from commit 170a2cd)

dadoonet added a commit to elastic/elasticsearch-river-couchdb that referenced this pull request Jul 18, 2014

Use groovy as default scripting language
As for elasticsearch 1.3.0, `groovy` is the new default scripting language.

Related to: elastic/elasticsearch#6233

Closes #61.
(cherry picked from commit 170a2cd)

dadoonet added a commit to elastic/elasticsearch-river-couchdb that referenced this pull request Jul 18, 2014

Use groovy as default scripting language
As for elasticsearch 1.3.0, `groovy` is the new default scripting language.

Related to: elastic/elasticsearch#6233

Closes #61.

@dakrone dakrone deleted the dakrone:feature/add-groovy-scripting branch Sep 9, 2014

@clintongormley clintongormley changed the title Scripting: Add Groovy as a scripting language, add groovy sandboxing Add Groovy as a scripting language, add groovy sandboxing Jun 6, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.