Support using default, sandboxed Painless scripts with ES 5.x, which …

…are enabled by default. Added property, elasticsearch.groovy.inline, to control whether Groovy dynamic scripting does (2.x) or does not (5.x) need to be enabled on the server (for testing/releases). Signed-off-by: sjudeng <[email protected]>
JanusGraph · Mar 10, 2017 · 7a735b3 · 7a735b3
1 parent 8e8db85
commit 7a735b3
Show file tree

Hide file tree

Showing 12 changed files with 81 additions and 20 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
@@ -511,6 +511,7 @@ public void mutate(Map<String, Map<String, IndexMutation>> mutations, KeyInforma
     }
 
     private String getDeletionScript(KeyInformation.IndexRetriever informations, String storename, IndexMutation mutation) throws PermanentBackendException {
+        int version = client.getMajorVersion();
         StringBuilder script = new StringBuilder();
         for (IndexEntry deletion : mutation.getDeletions()) {
             KeyInformation keyInformation = informations.get(storename).get(deletion.field);
@@ -524,7 +525,7 @@ private String getDeletionScript(KeyInformation.IndexRetriever informations, Str
                     break;
                 case SET:
                 case LIST:
-                    String jsValue = convertToJsType(deletion.value);
+                    String jsValue = convertToJsType(deletion.value, version);
                     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);");
@@ -537,23 +538,22 @@ private String getDeletionScript(KeyInformation.IndexRetriever informations, Str
     }
 
     private String getAdditionScript(KeyInformation.IndexRetriever informations, String storename, IndexMutation mutation) throws PermanentBackendException {
+        int version = client.getMajorVersion();
         StringBuilder script = new StringBuilder();
         for (IndexEntry e : mutation.getAdditions()) {
             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, version) + ";");
                     if (hasDualStringMapping(keyInformation)) {
-                        script.append("ctx._source[\"" + getDualMappingName(e.field) + "\"] = " + convertToJsType(e.value) + ";");
+                        script.append("ctx._source[\"" + getDualMappingName(e.field) + "\"] = " + convertToJsType(e.value, version) + ";");
                     }
                     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, version) + ");");
                     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, version) + ");");
                     }
                     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, int version) throws PermanentBackendException {
         try {
             XContentBuilder builder = XContentFactory.jsonBuilder().startObject();
 
@@ -579,7 +579,10 @@ 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 (version < 5.0) {
+                // required for Groovy (2.x) but fails with Painless (5.x)
+                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
@@ -57,11 +57,11 @@ public static ElasticSearchMutation createIndexRequest(String index, String type
     }
 
     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")));
+        return new ElasticSearchMutation(RequestType.UPDATE, index, type, id, ImmutableMap.of("script", ImmutableMap.of("inline", script)));
     }
 
     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));
+        return new ElasticSearchMutation(RequestType.UPDATE, index, type, id, ImmutableMap.of("script", ImmutableMap.of("inline", script), "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 (IOException 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
@@ -89,6 +89,9 @@
         <elasticsearch.version>2.4.4</elasticsearch.version>
         <elasticsearch.rest.version>5.2.2</elasticsearch.rest.version>
         <elasticsearch.dist.version>5.2.2</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>