From ebd3e5f73f1ac8849510bbe14f07f25ea591bbbc Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 9 May 2017 16:14:57 -0700 Subject: [PATCH] Scripting: Deprecate file script settings (#24555) File scripts have 2 related settings: the path of file scripts, and whether they can be dynamically reloaded. This commit deprecates those settings. relates #21798 --- core/src/main/java/org/elasticsearch/env/Environment.java | 3 ++- .../main/java/org/elasticsearch/script/ScriptService.java | 4 ++-- .../java/org/elasticsearch/script/FileScriptTests.java | 7 +++++-- .../java/org/elasticsearch/script/NativeScriptTests.java | 1 - .../java/org/elasticsearch/script/ScriptContextTests.java | 5 +++++ .../search/aggregations/AggregatorFactoriesTests.java | 3 +-- .../metrics/scripted/InternalScriptedMetricTests.java | 2 -- .../metrics/scripted/ScriptedMetricAggregatorTests.java | 4 +--- .../java/org/elasticsearch/tribe/TribeServiceTests.java | 4 ++++ .../org/elasticsearch/bootstrap/EvilSecurityTests.java | 2 ++ .../org/elasticsearch/ingest/AbstractScriptTestCase.java | 1 - .../java/org/elasticsearch/test/AbstractQueryTestCase.java | 3 --- .../src/main/java/org/elasticsearch/test/ESTestCase.java | 2 -- 13 files changed, 22 insertions(+), 19 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/env/Environment.java b/core/src/main/java/org/elasticsearch/env/Environment.java index 0d30f7b576c06..d1ad668e0eb31 100644 --- a/core/src/main/java/org/elasticsearch/env/Environment.java +++ b/core/src/main/java/org/elasticsearch/env/Environment.java @@ -55,7 +55,8 @@ public class Environment { public static final Setting DEFAULT_PATH_CONF_SETTING = Setting.simpleString("default.path.conf", Property.NodeScope); public static final Setting PATH_CONF_SETTING = new Setting<>("path.conf", DEFAULT_PATH_CONF_SETTING, Function.identity(), Property.NodeScope); - public static final Setting PATH_SCRIPTS_SETTING = Setting.simpleString("path.scripts", Property.NodeScope); + public static final Setting PATH_SCRIPTS_SETTING = + Setting.simpleString("path.scripts", Property.NodeScope, Property.Deprecated); public static final Setting> DEFAULT_PATH_DATA_SETTING = Setting.listSetting("default.path.data", Collections.emptyList(), Function.identity(), Property.NodeScope); public static final Setting> PATH_DATA_SETTING = diff --git a/core/src/main/java/org/elasticsearch/script/ScriptService.java b/core/src/main/java/org/elasticsearch/script/ScriptService.java index 3e612915a7b07..7a65fee327aaa 100644 --- a/core/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/core/src/main/java/org/elasticsearch/script/ScriptService.java @@ -86,7 +86,7 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust public static final Setting SCRIPT_CACHE_EXPIRE_SETTING = Setting.positiveTimeSetting("script.cache.expire", TimeValue.timeValueMillis(0), Property.NodeScope); public static final Setting SCRIPT_AUTO_RELOAD_ENABLED_SETTING = - Setting.boolSetting("script.auto_reload_enabled", true, Property.NodeScope); + Setting.boolSetting("script.auto_reload_enabled", true, Property.NodeScope, Property.Deprecated); public static final Setting SCRIPT_MAX_SIZE_IN_BYTES = Setting.intSetting("script.max_size_in_bytes", 65535, Property.NodeScope); public static final Setting SCRIPT_MAX_COMPILATIONS_PER_MINUTE = @@ -162,7 +162,7 @@ public ScriptService(Settings settings, Environment env, FileWatcher fileWatcher = new FileWatcher(scriptsDirectory); fileWatcher.addListener(new ScriptChangesListener()); - if (SCRIPT_AUTO_RELOAD_ENABLED_SETTING.get(settings)) { + if (SCRIPT_AUTO_RELOAD_ENABLED_SETTING.get(settings) && resourceWatcherService != null) { // automatic reload is enabled - register scripts resourceWatcherService.add(fileWatcher); } else { diff --git a/core/src/test/java/org/elasticsearch/script/FileScriptTests.java b/core/src/test/java/org/elasticsearch/script/FileScriptTests.java index c502bd5f03020..af32e8abec797 100644 --- a/core/src/test/java/org/elasticsearch/script/FileScriptTests.java +++ b/core/src/test/java/org/elasticsearch/script/FileScriptTests.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.script; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.script.MockScriptEngine.MockCompiledScript; @@ -59,7 +60,8 @@ public void testFileScriptFound() throws Exception { assertNotNull(compiledScript); MockCompiledScript executable = (MockCompiledScript) compiledScript.compiled(); assertEquals("script1.mockscript", executable.getName()); - assertWarnings("File scripts are deprecated. Use stored or inline scripts instead."); + assertSettingDeprecationsAndWarnings(new Setting[] {ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING}, + "File scripts are deprecated. Use stored or inline scripts instead."); } public void testAllOpsDisabled() throws Exception { @@ -79,6 +81,7 @@ public void testAllOpsDisabled() throws Exception { assertTrue(e.getMessage(), e.getMessage().contains("scripts of type [file], operation [" + context.getKey() + "] and lang [" + MockScriptEngine.NAME + "] are disabled")); } } - assertWarnings("File scripts are deprecated. Use stored or inline scripts instead."); + assertSettingDeprecationsAndWarnings(new Setting[] {ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING}, + "File scripts are deprecated. Use stored or inline scripts instead."); } } diff --git a/core/src/test/java/org/elasticsearch/script/NativeScriptTests.java b/core/src/test/java/org/elasticsearch/script/NativeScriptTests.java index aa2e260c7c2e1..0dfda8f7a1602 100644 --- a/core/src/test/java/org/elasticsearch/script/NativeScriptTests.java +++ b/core/src/test/java/org/elasticsearch/script/NativeScriptTests.java @@ -45,7 +45,6 @@ public void testNativeScript() throws InterruptedException { Settings settings = Settings.builder() .put("node.name", "testNativeScript") .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) - .put(ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING.getKey(), false) .build(); ScriptModule scriptModule = new ScriptModule(settings, new Environment(settings), null, singletonList(new NativeScriptEngineService(settings, singletonMap("my", new MyNativeScriptFactory()))), emptyList()); diff --git a/core/src/test/java/org/elasticsearch/script/ScriptContextTests.java b/core/src/test/java/org/elasticsearch/script/ScriptContextTests.java index e25335e5d68e4..83608a010b1ff 100644 --- a/core/src/test/java/org/elasticsearch/script/ScriptContextTests.java +++ b/core/src/test/java/org/elasticsearch/script/ScriptContextTests.java @@ -23,6 +23,7 @@ import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.test.ESTestCase; @@ -77,6 +78,7 @@ public void testCustomGlobalScriptContextSettings() throws Exception { assertThat(e.getMessage(), containsString("scripts of type [" + scriptType + "], operation [" + PLUGIN_NAME + "_custom_globally_disabled_op] and lang [" + MockScriptEngine.NAME + "] are disabled")); } } + assertSettingDeprecationsAndWarnings(new Setting[] {ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING}); } public void testCustomScriptContextSettings() throws Exception { @@ -93,6 +95,7 @@ public void testCustomScriptContextSettings() throws Exception { assertNotNull(scriptService.compile(script, ScriptContext.Standard.AGGS)); assertNotNull(scriptService.compile(script, ScriptContext.Standard.SEARCH)); assertNotNull(scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "custom_op"))); + assertSettingDeprecationsAndWarnings(new Setting[] {ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING}); } public void testUnknownPluginScriptContext() throws Exception { @@ -106,6 +109,7 @@ public void testUnknownPluginScriptContext() throws Exception { assertTrue(e.getMessage(), e.getMessage().contains("script context [" + PLUGIN_NAME + "_unknown] not supported")); } } + assertSettingDeprecationsAndWarnings(new Setting[] {ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING}); } public void testUnknownCustomScriptContext() throws Exception { @@ -125,6 +129,7 @@ public String getKey() { assertTrue(e.getMessage(), e.getMessage().contains("script context [test] not supported")); } } + assertSettingDeprecationsAndWarnings(new Setting[] {ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING}); } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/AggregatorFactoriesTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/AggregatorFactoriesTests.java index 180e639eb345f..a3ffd4108f4ef 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/AggregatorFactoriesTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/AggregatorFactoriesTests.java @@ -52,8 +52,7 @@ public void setUp() throws Exception { // we have to prefer CURRENT since with the range of versions we support // it's rather unlikely to get the current actually. Settings settings = Settings.builder().put("node.name", AbstractQueryTestCase.class.toString()) - .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) - .put(ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING.getKey(), false).build(); + .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()).build(); // create some random type with some default field, those types will // stick around for all of the subclasses currentTypes = new String[randomIntBetween(0, 5)]; diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetricTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetricTests.java index be06bdd56b46d..9d8ff94c47c2f 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetricTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetricTests.java @@ -69,8 +69,6 @@ protected InternalScriptedMetric createTestInstance(String name, List[] {Environment.PATH_SCRIPTS_SETTING}); // plugins: ro assertExactPermissions(new FilePermission(environment.pluginsFile().toString(), "read,readlink"), permissions); diff --git a/qa/smoke-test-ingest-with-all-dependencies/src/test/java/org/elasticsearch/ingest/AbstractScriptTestCase.java b/qa/smoke-test-ingest-with-all-dependencies/src/test/java/org/elasticsearch/ingest/AbstractScriptTestCase.java index 95810e089a275..1ca4c0252c39a 100644 --- a/qa/smoke-test-ingest-with-all-dependencies/src/test/java/org/elasticsearch/ingest/AbstractScriptTestCase.java +++ b/qa/smoke-test-ingest-with-all-dependencies/src/test/java/org/elasticsearch/ingest/AbstractScriptTestCase.java @@ -40,7 +40,6 @@ public abstract class AbstractScriptTestCase extends ESTestCase { public void init() throws Exception { Settings settings = Settings.builder() .put("path.home", createTempDir()) - .put(ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING.getKey(), false) .build(); ScriptEngineRegistry scriptEngineRegistry = new ScriptEngineRegistry(Arrays.asList(new MustacheScriptEngineService())); ScriptContextRegistry scriptContextRegistry = new ScriptContextRegistry(Collections.emptyList()); diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index 4cd321b4fb8a8..79d813940fa60 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -178,7 +178,6 @@ public static void beforeClass() { nodeSettings = Settings.builder() .put("node.name", AbstractQueryTestCase.class.toString()) .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) - .put(ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING.getKey(), false) .build(); indexSettings = Settings.builder() .put(IndexMetaData.SETTING_VERSION_CREATED, indexVersionCreated) @@ -1091,8 +1090,6 @@ ScriptModule createScriptModule(List scriptPlugins) { Settings settings = Settings.builder() .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) - // no file watching, so we don't need a ResourceWatcherService - .put(ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING.getKey(), false) .build(); Environment environment = new Environment(settings); return ScriptModule.create(settings, environment, null, scriptPlugins); diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index 58d67ea3b98ad..ed8f271bab14e 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -1172,8 +1172,6 @@ public static TestAnalysis createTestAnalysis(IndexSettings indexSettings, Setti public static ScriptModule newTestScriptModule() { Settings settings = Settings.builder() .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) - // no file watching, so we don't need a ResourceWatcherService - .put(ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING.getKey(), false) .build(); Environment environment = new Environment(settings); MockScriptEngine scriptEngine = new MockScriptEngine(MockScriptEngine.NAME, Collections.singletonMap("1", script -> "1"));