Support using sandboxed Painless scripts instead of Groovy with ES 5.…

…x. Painless scripts are enabled by default and do not require custom dynamic scripting configuration to use. Added property, elasticsearch.groovy.inline, to control whether Groovy dynamic scripting is enabled on embedded ES instances. Signed-off-by: sjudeng <[email protected]>
JanusGraph · Apr 1, 2017 · 6963319 · 6963319
1 parent 47c50da
commit 6963319
Show file tree

Hide file tree

Showing 12 changed files with 87 additions and 27 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -20,8 +20,8 @@ env:
     - MODULE='berkeleyje'
     - MODULE='cassandra'
     - MODULE='es' ARGS='-DthreadCount=1'
-    - MODULE='es' ARGS='-DthreadCount=1 -Delasticsearch.dist.version=2.4.4'
-    - MODULE='es' ARGS='-DthreadCount=1 -Delasticsearch.dist.version=2.4.4 -Dtest=**/Transport*'
+    - MODULE='es' ARGS='-DthreadCount=1 -Delasticsearch.dist.version=2.4.4 -Delasticsearch.groovy.inline=true'
+    - MODULE='es' ARGS='-DthreadCount=1 -Delasticsearch.dist.version=2.4.4 -Dtest=**/Transport* -Delasticsearch.groovy.inline=true'
     - MODULE='hadoop-parent/janusgraph-hadoop-2'
     - MODULE='hbase-parent/janusgraph-hbase-098'
     - MODULE='hbase-parent/janusgraph-hbase-10'
@@ -43,8 +43,8 @@ matrix:
     # Currently broken due to too many log statements (exceeds 4MB)
     # https://travis-ci.org/JanusGraph/janusgraph/jobs/197472453
     - env: MODULE='es' ARGS='-DthreadCount=1'
-    - env: MODULE='es' ARGS='-DthreadCount=1 -Delasticsearch.dist.version=2.4.4'
-    - env: MODULE='es' ARGS='-DthreadCount=1 -Delasticsearch.dist.version=2.4.4 -Dtest=**/Transport*'
+    - env: MODULE='es' ARGS='-DthreadCount=1 -Delasticsearch.dist.version=2.4.4 -Delasticsearch.groovy.inline=true'
+    - env: MODULE='es' ARGS='-DthreadCount=1 -Delasticsearch.dist.version=2.4.4 -Dtest=**/Transport* -Delasticsearch.groovy.inline=true'
 
     # Currently broken due to too many log statements (exceeds 4MB)
     # https://travis-ci.org/JanusGraph/janusgraph/jobs/197672947

diff --git a/docs/elasticsearch.txt b/docs/elasticsearch.txt
@@ -17,7 +17,11 @@ JanusGraph supports https://www.elastic.co/[Elasticsearch] as an index backend.
 Please see <<version-compat>> for details on what versions of ES will work with JanusGraph.
 
 [IMPORTANT]
-JanusGraph currently requires https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-scripting-security.html#enable-dynamic-scripting[Elasticsearch's dynamic scripting feature].  The `script.engine.groovy.inline.update` setting must be set to `true` on the Elasticsearch cluster.  This configuration requirement may be removed in future JanusGraph versions.
+===============================
+Beginning with Elasticsearch 5.0 JanusGraph uses sandboxed https://www.elastic.co/guide/en/elasticsearch/reference/master/modules-scripting-painless.html[Painless scripts] for inline updates, which are enabled by default in Elasticsearch 5.x.
+
+Using JanusGraph with Elasticsearch 2.x requires enabling Groovy inline scripting by setting `script.engine.groovy.inline.update` to `true` on the Elasticsearch cluster (see https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-scripting-security.html#enable-dynamic-scripting[dynamic scripting documentation] for more information).
+===============================
 
 === Running Elasticsearch
 

diff --git a/docs/searchpredicates.txt b/docs/searchpredicates.txt
@@ -86,6 +86,7 @@ While JanusGraph's composite indexes support any data type that can be stored in
 
 Additional data types will be supported in the future.
 
+[[geoshape]]
 === Geoshape Data Type
 The Geoshape data type supports representing a point, circle, box, line, polygon, multi-point, multi-line and multi-polygon. Index backends currently support indexing points, lines and polygons. Indexing multi-point, multi-line and multi-polygon properties has not been tested.
 Geospatial index lookups are only supported via mixed indexes.

diff --git a/janusgraph-dist/src/assembly/descriptor/common.component.xml b/janusgraph-dist/src/assembly/descriptor/common.component.xml
@@ -32,7 +32,7 @@
         <fileSet>
             <directory>${assembly.static.dir}/conf/es</directory>
             <outputDirectory>/elasticsearch/config</outputDirectory>
-            <filtered>false</filtered>
+            <filtered>true</filtered>
         </fileSet>
     </fileSets>
 

diff --git a/janusgraph-dist/src/assembly/static/conf/es/elasticsearch.yml b/janusgraph-dist/src/assembly/static/conf/es/elasticsearch.yml
@@ -1,4 +1,3 @@
 path.data: db/es/data
 path.logs: log
-script.engine.groovy.inline.update: true
-
+script.engine.groovy.inline.update: ${elasticsearch.groovy.inline}
diff --git a/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchClient.java b/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchClient.java
@@ -24,6 +24,8 @@
 
 public interface ElasticSearchClient extends Closeable {
 
+    int getMajorVersion();
+
     void clusterHealthRequest(String timeout) throws IOException;
 
     boolean indexExists(String indexName) throws IOException;

diff --git a/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchIndex.java b/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchIndex.java
@@ -20,8 +20,6 @@
 import com.google.common.collect.LinkedListMultimap;
 import com.google.common.collect.Multimap;
 import org.apache.commons.lang.StringUtils;
-import org.apache.tinkerpop.shaded.jackson.core.type.TypeReference;
-import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper;
 import org.elasticsearch.Version;
 import org.elasticsearch.common.geo.ShapeRelation;
 import org.elasticsearch.common.geo.builders.LineStringBuilder;
@@ -178,6 +176,7 @@ public class ElasticSearchIndex implements IndexProvider {
     private final ElasticSearchClient client;
     private final String indexName;
     private final int maxResultsSize;
+    private final String scriptLang;
 
     public ElasticSearchIndex(Configuration config) throws BackendException {
         indexName = config.get(INDEX_NAME);
@@ -190,6 +189,9 @@ public ElasticSearchIndex(Configuration config) throws BackendException {
         maxResultsSize = config.get(INDEX_MAX_RESULT_SET_SIZE);
         log.debug("Configured ES query result set max size to {}", maxResultsSize);
 
+        scriptLang = client.getMajorVersion() < 5 ? "groovy" : "painless";
+        log.debug("Using {} script language", scriptLang);
+
         try {
             client.clusterHealthRequest(config.get(HEALTH_REQUEST_TIMEOUT));
             checkForOrCreateIndex(config);
@@ -476,7 +478,7 @@ public void mutate(Map<String, Map<String, IndexMutation>> mutations, KeyInforma
                             requests.add(ElasticSearchMutation.createDeleteRequest(indexName, storename, docid));
                         } else {
                             String script = getDeletionScript(informations, storename, mutation);
-                            requests.add(ElasticSearchMutation.createUpdateRequest(indexName, storename, docid, script));
+                            requests.add(ElasticSearchMutation.createUpdateRequest(indexName, storename, docid, script, scriptLang));
                             log.trace("Adding script {}", script);
                         }
                     }
@@ -490,9 +492,9 @@ public void mutate(Map<String, Map<String, IndexMutation>> mutations, KeyInforma
                             String script = getAdditionScript(informations, storename, mutation);
                             if (needUpsert) {
                                 Map doc = getNewDocument(mutation.getAdditions(), informations.get(storename));
-                                requests.add(ElasticSearchMutation.createUpdateRequest(indexName, storename, docid, script, doc));
+                                requests.add(ElasticSearchMutation.createUpdateRequest(indexName, storename, docid, script, scriptLang, doc));
                             } else {
-                                requests.add(ElasticSearchMutation.createUpdateRequest(indexName, storename, docid, script));
+                                requests.add(ElasticSearchMutation.createUpdateRequest(indexName, storename, docid, script, scriptLang));
                             }
 
                             log.trace("Adding script {}", script);
@@ -524,7 +526,7 @@ private String getDeletionScript(KeyInformation.IndexRetriever informations, Str
                     break;
                 case SET:
                 case LIST:
-                    String jsValue = convertToJsType(deletion.value);
+                    String jsValue = convertToJsType(deletion.value, scriptLang);
                     script.append("def index = ctx._source[\"" + deletion.field + "\"].indexOf(" + jsValue + "); ctx._source[\"" + deletion.field + "\"].remove(index);");
                     if (hasDualStringMapping(informations.get(storename, deletion.field))) {
                         script.append("def index = ctx._source[\"" + getDualMappingName(deletion.field) + "\"].indexOf(" + jsValue + "); ctx._source[\"" + getDualMappingName(deletion.field) + "\"].remove(index);");
@@ -542,18 +544,16 @@ private String getAdditionScript(KeyInformation.IndexRetriever informations, Str
             KeyInformation keyInformation = informations.get(storename).get(e.field);
             switch (keyInformation.getCardinality()) {
                 case SINGLE:
-                    script.append("ctx._source[\"" + e.field + "\"] = " + convertToJsType(e.value) + ";");
+                    script.append("ctx._source[\"" + e.field + "\"] = " + convertToJsType(e.value, scriptLang) + ";");
                     if (hasDualStringMapping(keyInformation)) {
-                        script.append("ctx._source[\"" + getDualMappingName(e.field) + "\"] = " + convertToJsType(e.value) + ";");
+                        script.append("ctx._source[\"" + getDualMappingName(e.field) + "\"] = " + convertToJsType(e.value, scriptLang) + ";");
                     }
                     break;
                 case SET:
                 case LIST:
-                    script.append("if(ctx._source[\"" + e.field + "\"] == null) {ctx._source[\"" + e.field + "\"] = []};");
-                    script.append("ctx._source[\"" + e.field + "\"].add(" + convertToJsType(e.value) + ");");
+                    script.append("ctx._source[\"" + e.field + "\"].add(" + convertToJsType(e.value, scriptLang) + ");");
                     if (hasDualStringMapping(keyInformation)) {
-                        script.append("if(ctx._source[\"" + getDualMappingName(e.field) + "\"] == null) {ctx._source[\"" + e.field + "\"] = []};");
-                        script.append("ctx._source[\"" + getDualMappingName(e.field) + "\"].add(" + convertToJsType(e.value) + ");");
+                        script.append("ctx._source[\"" + getDualMappingName(e.field) + "\"].add(" + convertToJsType(e.value, scriptLang) + ");");
                     }
                     break;
 
@@ -563,7 +563,7 @@ private String getAdditionScript(KeyInformation.IndexRetriever informations, Str
         return script.toString();
     }
 
-    private static String convertToJsType(Object value) throws PermanentBackendException {
+    private static String convertToJsType(Object value, String scriptLang) throws PermanentBackendException {
         try {
             XContentBuilder builder = XContentFactory.jsonBuilder().startObject();
 
@@ -579,7 +579,9 @@ private static String convertToJsType(Object value) throws PermanentBackendExcep
             int prefixLength = "{\"value\":".length();
             int suffixLength = "}".length();
             String result = s.substring(prefixLength, s.length() - suffixLength);
-            result = result.replace("$", "\\$");
+            if (scriptLang.equals("groovy")) {
+                result = result.replace("$", "\\$");
+            }
             return result;
         } catch (IOException e) {
             throw new PermanentBackendException("Could not write json");

diff --git a/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchMutation.java b/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchMutation.java
@@ -56,12 +56,12 @@ public static ElasticSearchMutation createIndexRequest(String index, String type
         return new ElasticSearchMutation(RequestType.INDEX, index, type, id, source);
     }
 
-    public static ElasticSearchMutation createUpdateRequest(String index, String type, String id, String script) {
-        return new ElasticSearchMutation(RequestType.UPDATE, index, type, id, ImmutableMap.of("script", ImmutableMap.of("inline", script, "lang", "groovy")));
+    public static ElasticSearchMutation createUpdateRequest(String index, String type, String id, String script, String lang) {
+        return new ElasticSearchMutation(RequestType.UPDATE, index, type, id, ImmutableMap.of("script", ImmutableMap.of("inline", script, "lang", lang)));
     }
 
-    public static ElasticSearchMutation createUpdateRequest(String index, String type, String id, String script, Map upsert) {
-        return new ElasticSearchMutation(RequestType.UPDATE, index, type, id, ImmutableMap.of("script", ImmutableMap.of("inline", script, "lang", "groovy"), "upsert", upsert));
+    public static ElasticSearchMutation createUpdateRequest(String index, String type, String id, String script, String lang, Map upsert) {
+        return new ElasticSearchMutation(RequestType.UPDATE, index, type, id, ImmutableMap.of("script", ImmutableMap.of("inline", script, "lang", lang), "upsert", upsert));
     }
 
     public RequestType getRequestType() {

diff --git a/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/TransportElasticSearchClient.java b/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/TransportElasticSearchClient.java
@@ -194,6 +194,11 @@ public void close() throws IOException {
         client.close();
     }
 
+    @Override
+    public int getMajorVersion() {
+        return 2;
+    }
+
     public void setBulkRefresh(boolean bulkRefresh) {
         this.bulkRefresh = bulkRefresh;
     }

diff --git a/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/rest/RestElasticSearchClient.java b/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/rest/RestElasticSearchClient.java
@@ -17,6 +17,7 @@
 import com.google.common.collect.ImmutableMap;
 import org.apache.http.HttpEntity;
 import org.apache.http.entity.ByteArrayEntity;
+import org.apache.tinkerpop.shaded.jackson.annotation.JsonIgnoreProperties;
 import org.apache.tinkerpop.shaded.jackson.core.type.TypeReference;
 import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper;
 import org.apache.tinkerpop.shaded.jackson.databind.ObjectReader;
@@ -43,6 +44,8 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 public class RestElasticSearchClient implements ElasticSearchClient {
@@ -64,17 +67,43 @@ public class RestElasticSearchClient implements ElasticSearchClient {
 
     private RestClient delegate;
 
+    private Integer majorVersion;
+
     private String bulkRefresh;
 
     public RestElasticSearchClient(RestClient delegate) {
         this.delegate = delegate;
+        majorVersion = getMajorVersion();
     }
 
     @Override
     public void close() throws IOException {
         delegate.close();
     }
 
+    @Override
+    public int getMajorVersion() {
+        if (majorVersion == null) {
+            final Pattern pattern = Pattern.compile("(\\d+)\\.\\d+\\.\\d+");
+            majorVersion = 2;
+            try {
+                final Response response = delegate.performRequest("GET", "/");
+                try (final InputStream inputStream = response.getEntity().getContent()) {
+                    final ClusterInfo info = mapper.readValue(inputStream, ClusterInfo.class);
+                    final Matcher m = info.getVersion() != null ? pattern.matcher((String) info.getVersion().get("number")) : null;
+                    if (m == null || !m.find()) {
+                        majorVersion = 2;
+                    } else {
+                        majorVersion = Integer.valueOf(m.group(1));
+                    }
+                }
+            } catch (Exception e) {
+                log.warn("Unable to determine Elasticsearch server version. Assuming 2.x.", e);
+            }
+        }
+        return majorVersion;
+    }
+
     @Override
     public void clusterHealthRequest(String timeout) throws IOException {
         Map<String,String> params = ImmutableMap.of("wait_for_status","yellow","timeout",timeout);
@@ -223,4 +252,19 @@ private Response performRequest(String method, String path, byte[] requestData)
         return response;
     }
 
+    @JsonIgnoreProperties(ignoreUnknown=true)
+    private static final class ClusterInfo {
+
+        private Map<String,Object> version;
+
+        public Map<String, Object> getVersion() {
+            return version;
+        }
+
+        public void setVersion(Map<String, Object> version) {
+            this.version = version;
+        }
+
+    }
+
 }
diff --git a/janusgraph-es/src/test/config/elasticsearch.yml b/janusgraph-es/src/test/config/elasticsearch.yml
@@ -1 +1 @@
-script.engine.groovy.inline.update: true
+script.engine.groovy.inline.update: ${elasticsearch.groovy.inline}
diff --git a/pom.xml b/pom.xml
@@ -90,6 +90,9 @@
         <elasticsearch.version>2.4.4</elasticsearch.version>
         <elasticsearch.rest.version>5.3.0</elasticsearch.rest.version>
         <elasticsearch.dist.version>5.3.0</elasticsearch.dist.version>
+        <!-- Groovy dynamic scripting must be manually enabled on ES 2.x servers.
+             From ES 5.0 Painless inline scripts are used which are enabled by default. -->
+        <elasticsearch.groovy.inline>false</elasticsearch.groovy.inline>
         <commons.beanutils.version>1.7.0</commons.beanutils.version>
         <commons.collections.version>3.2.2</commons.collections.version>
         <joda.version>2.8.2</joda.version>