Commit Graph

88 Commits

Author SHA1 Message Date
Tanguy Leroux 44ac5d057a Remove empty javadoc (#20871)
This commit removes as many as empty javadocs comments my regexp has found
2016-10-12 10:27:09 +02:00
Jack Conradson 3b3baa6e6c Made deprecation of Groovy, Javascript, and Python more explicit. 2016-08-31 15:56:31 -07:00
Jack Conradson 7930233527 Deprecate Groovy, Python, and Javascript scripts. 2016-08-30 09:06:18 -07:00
Lee Hinman 2be52eff09 Circuit break the number of inline scripts compiled per minute
When compiling many dynamically changing scripts, parameterized
scripts (<https://www.elastic.co/guide/en/elasticsearch/reference/master/modules-scripting-using.html#prefer-params>)
should be preferred. This enforces a limit to the number of scripts that
can be compiled within a minute. A new dynamic setting is added -
`script.max_compilations_per_minute`, which defaults to 15.

If more dynamic scripts are sent, a user will get the following
exception:

```json
{
  "error" : {
    "root_cause" : [
      {
        "type" : "circuit_breaking_exception",
        "reason" : "[script] Too many dynamic script compilations within one minute, max: [15/min]; please use on-disk, indexed, or scripts with parameters instead",
        "bytes_wanted" : 0,
        "bytes_limit" : 0
      }
    ],
    "type" : "search_phase_execution_exception",
    "reason" : "all shards failed",
    "phase" : "query",
    "grouped" : true,
    "failed_shards" : [
      {
        "shard" : 0,
        "index" : "i",
        "node" : "a5V1eXcZRYiIk8lecjZ4Jw",
        "reason" : {
          "type" : "general_script_exception",
          "reason" : "Failed to compile inline script [\"aaaaaaaaaaaaaaaa\"] using lang [painless]",
          "caused_by" : {
            "type" : "circuit_breaking_exception",
            "reason" : "[script] Too many dynamic script compilations within one minute, max: [15/min]; please use on-disk, indexed, or scripts with parameters instead",
            "bytes_wanted" : 0,
            "bytes_limit" : 0
          }
        }
      }
    ],
    "caused_by" : {
      "type" : "general_script_exception",
      "reason" : "Failed to compile inline script [\"aaaaaaaaaaaaaaaa\"] using lang [painless]",
      "caused_by" : {
        "type" : "circuit_breaking_exception",
        "reason" : "[script] Too many dynamic script compilations within one minute, max: [15/min]; please use on-disk, indexed, or scripts with parameters instead",
        "bytes_wanted" : 0,
        "bytes_limit" : 0
      }
    }
  },
  "status" : 500
}
```

This also fixes a bug in `ScriptService` where requests being executed
concurrently on a single node could cause a script to be compiled
multiple times (many in the case of a powerful node with many shards)
due to no synchronization between checking the cache and compiling the
script. There is now synchronization so that a script being compiled
will only be compiled once regardless of the number of concurrent
searches on a node.

Relates to #19396
2016-08-09 10:26:27 -06:00
Nik Everett 9270e8b22b Rename client yaml test infrastructure
This makes it obvious that these tests are for running the client yaml
suites. Now that there are other ways of running tests using the REST
client against a running cluster we can't go on calling the shared
client yaml tests "REST tests". They are rest tests, but they aren't
**the** rest tests.
2016-07-26 13:53:44 -04:00
Nik Everett a95d4f4ee7 Add Location header and improve REST testing
This adds a header that looks like `Location: /test/test/1` to the
response for the index/create/update API. The requirement for the header
comes from https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

https://tools.ietf.org/html/rfc7231#section-7.1.2 claims that relative
URIs are OK. So we use an absolute path which should resolve to the
appropriate location.

Closes #19079

This makes large changes to our rest test infrastructure, allowing us
to write junit tests that test a running cluster via the rest client.
It does this by splitting ESRestTestCase into two classes:
* ESRestTestCase is the superclass of all tests that use the rest client
to interact with a running cluster.
* ESClientYamlSuiteTestCase is the superclass of all tests that use the
rest client to run the yaml tests. These tests are shared across all
official clients, thus the `ClientYamlSuite` part of the name.
2016-07-25 17:02:40 -04:00
Tanguy Leroux 0e7faf1005 Enable Checkstyle RedundantModifier 2016-07-04 15:22:12 +02:00
Jason Tedor 3343ceeae4 Do not catch throwable
Today throughout the codebase, catch throwable is used with reckless
abandon. This is dangerous because the throwable could be a fatal
virtual machine error resulting from an internal error in the JVM, or an
out of memory error or a stack overflow error that leaves the virtual
machine in an unstable and unpredictable state. This commit removes
catch throwable from the codebase and removes the temptation to use it
by modifying listener APIs to receive instances of Exception instead of
the top-level Throwable.

Relates #19231
2016-07-04 08:41:06 -04:00
Tanguy Leroux 98951b1203 Compile each Groovy script in its own classloader
closes #18572
2016-06-20 08:17:09 +02:00
Ryan Ernst 8196cf01e3 Merge branch 'master' into plugin_name_api 2016-06-16 13:49:28 -07:00
Simon Willnauer 18ff051ad5 Simplify ScriptModule and script registration (#18903)
Registering a script engine or native scripts still uses Guice today
and is much more complicated than needed. This change moves to a pull
based model where script plugins have to implement a dedicated interface
`ScriptPlugin` and defines simple getter returning instances rather than
classes.
2016-06-16 09:35:13 +02:00
Ryan Ernst a4503c2aed Plugins: Remove name() and description() from api
In 2.0 we added plugin descriptors which require defining a name and
description for the plugin. However, we still have name() and
description() which must be overriden from the Plugin class. This still
exists for classpath plugins. But classpath plugins are mainly for
tests, and even then, referring to classpath plugins with their class is
a better idea. This change removes name() and description(), replacing
the name for classpath plugins with the full class name.
2016-06-15 17:12:22 -07:00
Robert Muir 8edf213492 Remove LeafSearchScript.runAsFloat(): Nothing calls it. 2016-05-15 22:59:28 -04:00
Robert Muir 2028691e66 painless: improve exception stacktraces
closes #18319
2016-05-13 15:40:45 -04:00
Lee Hinman 9bcdafedda Allow only a single extension for a scripting engine
Previously multiple extensions could be provided, however, this can lead
to confusion with on-disk scripts (ie, "foo.js" and "foo.javascript")
having different content. Only a single extension is now supported.

The only language currently supporting multiple extensions was the
Javascript engine ("js" and "javascript"). It now only supports the
`.js` extension.

Relates to #10598
2016-05-13 09:54:31 -06:00
Lee Hinman efff3918d8 Remove support for mulitple languages per scripting engine 2016-05-13 09:24:31 -06:00
Lee Hinman a4060f7436 Remove vestiges of script engine sandboxing
This removes all the mentions of the sandbox from the script engine
services and permissions model. This means that the following settings
are no longer supported:

```yaml
script.inline: sandbox
script.stored: sandbox
```

Instead, only a `true` or `false` value can be specified.

Since this would otherwise break the default-allow parameter for
languages like expressions, painless, and mustache, all script engines
have been updated to have individual settings, for instance:

```yaml
script.engine.groovy.inline: true
```

Would enable all inline scripts for groovy. (they can still be
overridden on a per-operation basis).

Expressions, Painless, and Mustache all default to `true` for inline,
file, and stored scripts to preserve the old scripting behavior.

Resolves #17114
2016-05-13 09:24:31 -06:00
Martijn van Groningen c5ad2e2865 Changed indexed scripts to be stored in the cluster state instead of the `.scripts` index.
Also added max script size soft limit for stored scripts.

Closes #16651
2016-04-22 13:42:55 +02:00
Adrien Grand 068c788ec8 Disable fielddata on text fields by defaults. #17386
`text` fields will have fielddata disabled by default. Fielddata can still be
enabled on an existing index by setting `fielddata=true` in the mappings.
2016-03-30 14:35:32 +02:00
Jason Tedor 618441aea3 Merge pull request #17088 from jasontedor/simplify-bootstrap-settings
Bootstrap does not set system properties
2016-03-15 19:25:16 -04:00
Jason Tedor 66ba044ec5 Use setting in integration test cluster config 2016-03-15 17:45:17 -04:00
Yannick Welsch d14ae5f8b6 Remove Python and Javascript Benchmark classes 2016-03-15 15:02:50 +01:00
Jason Tedor 8a05c2a2be Bootstrap does not set system properties
Today, certain bootstrap properties are set and read via system
properties. This action-at-distance way of managing these properties is
rather confusing, and completely unnecessary. But another problem exists
with setting these as system properties. Namely, these system properties
are interpreted as Elasticsearch settings, not all of which are
registered. This leads to Elasticsearch failing to startup if any of
these special properties are set. Instead, these properties should be
kept as local as possible, and passed around as method parameters where
needed. This eliminates the action-at-distance way of handling these
properties, and eliminates the need to register these non-setting
properties. This commit does exactly that.

Additionally, today we use the "-D" command line flag to set the
properties, but this is confusing because "-D" is a special flag to the
JVM for setting system properties. This creates confusion because some
"-D" properties should be passed via arguments to the JVM (so via
ES_JAVA_OPTS), and some should be passed as arguments to
Elasticsearch. This commit changes the "-D" flag for Elasticsearch
settings to "-E".
2016-03-13 20:09:15 -04:00
Lee Hinman 6adbbff97c Fix organization rename in all files in project
Basically a query-replace of "https://github.com/elasticsearch/" with "https://github.com/elastic/"
2016-03-03 12:04:13 -07:00
Nik Everett ba5be0332d Remove optional logger wrappers
Removes all our logger wrappers except the wrapper for log4j1.2. If you
depend on Elasticsearch's jar in your application you'll need to declare
log4j 1.2 and/or some bridge to your favorite logger.

We did this to simplify our builds and code. No more commons-logging like
log implementation sniffing. No more optional dependency hacks in gradle.

We might one day want to use j.u.l instead of log4j. If we do want that
we can recover its wrapper by studying this commit. We didn't go directly
to j.u.l in this commit because that is a bigger change. Our logging
configuration is based on log4j1.2 and people are used to it. So it'd
be a much more fraught breaking change to do that conversion.
2016-02-26 16:41:07 -05:00
Yannick Welsch 380393a5b7 Add permission to access sun.reflect.MethodAccessorImpl from Groovy scripts
Groovy uses reflection to invoke closures. These reflective calls are optimized by the JVM after "sun.reflect.inflationThreshold" number of invocations.
After inflation, access to sun.reflect.MethodAccessorImpl is required from the security manager.

Closes #16536
2016-02-09 16:47:38 +01:00
Jason Tedor 284cc3a048 Script mode settings as booleans
This commit modifies the accept values for script mode settings from
"on", "off", and "sandbox" to "true", "false", and "sandbox".
2016-01-27 06:26:58 -05:00
Jason Tedor 9944573449 Rename methods on ScriptEngineService
This commit method renames the ScriptEngineService interface methods
types, extensions, and sandboxed to getTypes, getExtensions, and
isSandboxed, respectively.
2016-01-27 06:26:04 -05:00
Jason Tedor 087e55cc51 Script mode settings
This commit converts the script mode settings to the new settings
infrastructure. This is a major refactoring of the handling of script
mode settings. This refactoring is necessary because these settings are
determined at runtime based on the registered script engines and the
registered script contexts.
2016-01-27 06:26:04 -05:00
Ryan Ernst ef4f0a8699 Test: Make rest test framework accept http directly for the test cluster
The rest test framework, because it used to be tightly integrated with
ESIntegTestCase, currently expects the addresses for the test cluster to
be passed using the transport protocol port. However, it only uses this
to then find the http address.

This change makes ESRestTestCase extend from ESTestCase instead of
ESIntegTestCase, and changes the sysprop used to tests.rest.cluster,
which now takes the http address.

closes #15459
2016-01-18 16:44:14 -08:00
Jason Tedor 871d1b4885 Remove and forbid use of j.u.c.ThreadLocalRandom
This commit removes and now forbids all uses of
java.util.concurrent.ThreadLocalRandom across the codebase. The
underlying issue with ThreadLocalRandom is that it can not be
seeded. This means that if ThreadLocalRandom is used in production code,
then tests that cover any code path containing ThreadLocalRandom will be
prevented from being reproducible by use of ThreadLocalRandom. Instead,
using org.elasticsearch.common.random.Randomness#get will give
reproducible sources of random when running under tests and otherwise
still give an instance of ThreadLocalRandom when running as production
code.
2016-01-08 12:23:48 -05:00
Nik Everett 244120a065 Remove more Xlint skips 2016-01-06 23:53:05 -05:00
Robert Muir 180ab2493e Improve thirdPartyAudit check, round 3 2015-12-28 22:38:55 -05:00
Adrien Grand 5eb7555ffb Make text parsing less lenient.
It now requires that the parser is on a value.
2015-12-28 16:06:42 +01:00
Ryan Ernst 4ea19995cf Remove wildcard imports 2015-12-18 12:43:47 -08:00
Robert Muir a7cc91e868 Merge pull request #15501 from rmuir/sheisty_classes
thirdPartyAudit round 2
2015-12-17 03:44:27 -05:00
Robert Muir 6692e42d9a thirdPartyAudit round 2
This fixes the `lenient` parameter to be `missingClasses`. I will remove this boolean and we can handle them via the normal whitelist.
It also adds a check for sheisty classes (jar hell with the jdk).
This is inspired by the lucene "sheisty" classes check, but it has false positives. This check is more evil, it validates every class file against the extension classloader as a resource, to see if it exists there. If so: jar hell.

This jar hell is a problem for several reasons:

1. causes insanely-hard-to-debug problems (like bugs in forbidden-apis)
2. hides problems (like internal api access)
3. the code you think is executing, is not really executing
4. security permissions are not what you think they are
5. brings in unnecessary dependencies
6. its jar hell

The more difficult problems are stuff like jython, where these classes are simply 'uberjared' directly in, so you cant just fix them by removing a bogus dependency. And there is a legit reason for them to do that, they want to support java 1.4.
2015-12-17 02:35:00 -05:00
Jack Conradson 4523eaec88 Added plumbing for compile time script parameters.
Closes #15464
2015-12-16 18:29:21 -08:00
Robert Muir 42138007db add some more comments about internal api usage 2015-12-16 18:56:02 -05:00
Robert Muir ee79d46583 Add gradle thirdPartyAudit to precommit tasks 2015-12-16 16:38:16 -05:00
Robert Muir 2169a123a5 Filter classes loaded by scripts
Since 2.2 we run all scripts with minimal privileges, similar to applets in your browser.
The problem is, they have unrestricted access to other things they can muck with (ES, JDK, whatever).
So they can still easily do tons of bad things

This PR restricts what classes scripts can load via the classloader mechanism, to make life more difficult.
The "standard" list was populated from the old list used for the groovy sandbox: though
a few more were needed for tests to pass (java.lang.String, java.util.Iterator, nothing scary there).

Additionally, each scripting engine typically needs permissions to some runtime stuff.
That is the downside of this "good old classloader" approach, but I like the transparency and simplicity,
and I don't want to waste my time with any feature provided by the engine itself for this, I don't trust them.

This is not perfect and the engines are not perfect but you gotta start somewhere. For expert users that
need to tweak the permissions, we already support that via the standard java security configuration files, the
specification is simple, supports wildcards, etc (though we do not use them ourselves).
2015-12-05 21:46:52 -05:00
Robert Muir b0c64910b0 ban RuntimePermission("getClassLoader")
this gives more isolation between modules and plugins.
2015-12-04 15:58:02 -05:00
Robert Muir 30529c008d Ban write access to system properties
* Forbid System.setProperties & co in forbidden APIs.
* Ban property write access at runtime with security manager.

Plugins that need to modify system properties will need to request permission in their plugin-security.policy
2015-11-21 22:33:06 -05:00
Robert Muir b6a2318563 upgrade rhino for plugins/lang-javascript
the current jar is over 3 years old, we should upgrade it for bugfixes.
the current integration could be more secure: set a global policy and enforce additional (compile-time) checks

closes #14466
2015-11-03 09:53:07 -05:00
Ryan Ernst 542522531a Build: Remove maven pom files and supporting ant files
This change removes the leftover pom files. A couple files were left for
reference, namely in qa tests that have not yet been migrated (vagrant
and multinode). The deb and rpm assemblies also still exist for
reference when finishing their setup in gradle.

See #13930
2015-10-29 23:53:49 -07:00
Ryan Ernst c86100f636 Switch build system to Gradle
See #13930
2015-10-29 11:40:19 -07:00
Nik Everett 2cc97a0d3e Remove and ban @Test
There are three ways `@Test` was used. Way one:

```java
@Test
public void flubTheBlort() {
```

This way was always replaced with:

```java
public void testFlubTheBlort() {
```

Or, maybe with a better method name if I was feeling generous.

Way two:

```java
@Test(throws=IllegalArgumentException.class)
public void testFoo() {
    methodThatThrows();
}
```

This way of using `@Test` is actually pretty OK, but to get the tools to ban
`@Test` entirely it can't be used. Instead:

```java
public void testFoo() {
    try {
        methodThatThrows();
        fail("Expected IllegalArgumentException");
    } catch (IllegalArgumentException e ) {
        assertThat(e.getMessage(), containsString("something"));
    }
}
```

This is longer but tests more than the old ways and is much more precise.
Compare:

```java
@Test(throws=IllegalArgumentException.class)
public void testFoo() {
    some();
    copy();
    and();
    pasted();
    methodThatThrows();
    code();  // <---- This was left here by mistake and is never called
}
```

to:

```java
@Test(throws=IllegalArgumentException.class)
public void testFoo() {
    some();
    copy();
    and();
    pasted();
    try {
        methodThatThrows();
        fail("Expected IllegalArgumentException");
    } catch (IllegalArgumentException e ) {
        assertThat(e.getMessage(), containsString("something"));
    }
}
```

The final use of test is:

```java
@Test(timeout=1000)
public void testFoo() {
    methodThatWasSlow();
}
```

This is the most insidious use of `@Test` because its tempting but tragically
flawed. Its flaws are:
1. Hard and fast timeouts can look like they are asserting that something is
faster and even do an ok job of it when you compare the timings on the same
machine but as soon as you take them to another machine they start to be
invalid. On a slow VM both the new and old methods fail. On a super-fast
machine the slower and faster ways succeed.
2. Tests often contain slow `assert` calls so the performance of tests isn't
sure to predict the performance of non-test code.
3. These timeouts are rude to debuggers because the test just drops out from
under it after the timeout.

Confusingly, timeouts are useful in tests because it'd be rude for a broken
test to cause CI to abort the whole build after it hits a global timeout. But
those timeouts should be very very long "backstop" timeouts and aren't useful
assertions about speed.

For all its flaws `@Test(timeout=1000)` doesn't have a good replacement __in__
__tests__. Nightly benchmarks like http://benchmarks.elasticsearch.org/ are
useful here because they run on the same machine but they aren't quick to check
and it takes lots of time to figure out the regressions. Sometimes its useful
to compare dueling implementations but that requires keeping both
implementations around. All and all we don't have a satisfactory answer to the
question "what do you replace `@Test(timeout=1000)`" with. So we handle each
occurrence on a case by case basis.

For files with `@Test` this also:
1. Removes excess blank lines. They don't help anything.
2. Removes underscores from method names. Those would fail any code style
checks we ever care to run and don't add to readability. Since I did this manually
I didn't do it consistently.
3. Make sure all test method names start with `test`. Some used to end in `Test` or start
with `verify` or `check` and they were picked up using the annotation. Without the
annotation they always need to start with `test`.
4. Organizes imports using the rules we generate for Eclipse. For the most part
this just removes `*` imports which is a win all on its own. It was "required"
to quickly remove `@Test`.
5. Removes unneeded casts. This is just a setting I have enabled in Eclipse and
forgot to turn off before I did this work. It probably isn't hurting anything.
6. Removes trailing whitespace. Again, another Eclipse setting I forgot to turn
off that doesn't hurt anything. Hopefully.
7. Swaps some tests override superclass tests to make them empty with
`assumeTrue` so that the reasoning for the skips is logged in the test run and
it doesn't "look like" that thing is being tested when it isn't.
8. Adds an oxford comma to an error message.

The total test count doesn't change. I know. I counted.
```bash
git checkout master && mvn clean && mvn install | tee with_test
git no_test_annotation master && mvn clean && mvn install | tee not_test
grep 'Tests summary' with_test > with_test_summary
grep 'Tests summary' not_test > not_test_summary
diff with_test_summary not_test_summary
```

These differ somewhat because some tests are skipped based on the random seed.
The total shouldn't differ. But it does!
```
1c1
< [INFO] Tests summary: 564 suites (1 ignored), 3171 tests, 31 ignored (31 assumptions)
---
> [INFO] Tests summary: 564 suites (1 ignored), 3167 tests, 17 ignored (17 assumptions)
```

These are the core unit tests. So we dig further:
```bash
cat with_test | perl -pe 's/\n// if /^Suite/;s/.*\n// if /IGNOR/;s/.*\n// if /Assumption #/;s/.*\n// if /HEARTBEAT/;s/Completed .+?,//' | grep Suite > with_test_suites
cat not_test | perl -pe 's/\n// if /^Suite/;s/.*\n// if /IGNOR/;s/.*\n// if /Assumption #/;s/.*\n// if /HEARTBEAT/;s/Completed .+?,//' | grep Suite > not_test_suites
diff <(sort with_test_suites) <(sort not_test_suites)
```

The four tests with lower test numbers are all extend `AbstractQueryTestCase`
and all have a method that looks like this:

```java
@Override
public void testToQuery() throws IOException {
    assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);
    super.testToQuery();
}
```

It looks like this method was being double counted on master and isn't anymore.

Closes #14028
2015-10-20 17:37:36 -04:00
Colin Goodheart-Smithe c618f75b76 Merge branch 'master' into feature/search-request-refactoring
# Conflicts:
#	core/src/main/java/org/elasticsearch/search/SearchService.java
2015-10-15 11:02:34 +01:00
Robert Muir 5d001d1578 Decentralize plugin security
* Add ability for plugins to declare additional permissions with a custom plugin-security.policy file and corresponding AccessController logic. See the plugin author's guide for more information.
* Add warning messages to users for extra plugin permissions in bin/plugin.
* When bin/plugin is run interactively (stdin is a controlling terminal and -b/--batch not supplied), require user confirmation.
* Improve unit test and IDE support for plugins with additional permissions by exposing plugin's metadata as a maven test resource.

Closes #14108

Squashed commit of the following:

commit cf8ace65a7397aaccd356bf55f95d6fbb8bb571c
Author: Robert Muir <rmuir@apache.org>
Date:   Wed Oct 14 13:36:05 2015 -0400

    fix new unit test from master merge

commit 9be3c5aa38f2d9ae50f3d54924a30ad9cddeeb65
Merge: 2f168b8 7368231
Author: Robert Muir <rmuir@apache.org>
Date:   Wed Oct 14 12:58:31 2015 -0400

    Merge branch 'master' into off_my_back

commit 2f168b8038e32672f01ad0279fb5db77ba902ae8
Author: Robert Muir <rmuir@apache.org>
Date:   Wed Oct 14 12:56:04 2015 -0400

    improve plugin author documentation

commit 6e6c2bfda68a418d92733ac22a58eec35508b2d0
Author: Robert Muir <rmuir@apache.org>
Date:   Wed Oct 14 12:52:14 2015 -0400

    move security confirmation after 'plugin already installed' check, to prevent user from answering unnecessary questions.

commit 08233a2972554afef2a6a7521990283102e20d92
Author: Robert Muir <rmuir@apache.org>
Date:   Wed Oct 14 05:36:42 2015 -0400

    Add documentation and pluginmanager support

commit 05dad86c51488ba43ccbd749f0164f3fbd3aee62
Author: Robert Muir <rmuir@apache.org>
Date:   Wed Oct 14 02:22:24 2015 -0400

    Decentralize plugin permissions (modulo docs and pluginmanager work)
2015-10-14 14:46:45 -04:00
javanna 1915c74e93 Merge branch 'master' into feature/search-request-refactoring 2015-10-06 16:21:58 +02:00