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

@@ -55,7 +55,8 @@ public class Environment {
     public static final Setting<String> DEFAULT_PATH_CONF_SETTING = Setting.simpleString("default.path.conf", Property.NodeScope);
     public static final Setting<String> PATH_CONF_SETTING =
             new Setting<>("path.conf", DEFAULT_PATH_CONF_SETTING, Function.identity(), Property.NodeScope);
-    public static final Setting<String> PATH_SCRIPTS_SETTING = Setting.simpleString("path.scripts", Property.NodeScope);
+    public static final Setting<String> PATH_SCRIPTS_SETTING =
+        Setting.simpleString("path.scripts", Property.NodeScope, Property.Deprecated);
     public static final Setting<List<String>> DEFAULT_PATH_DATA_SETTING =
             Setting.listSetting("default.path.data", Collections.emptyList(), Function.identity(), Property.NodeScope);
     public static final Setting<List<String>> PATH_DATA_SETTING =

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<TimeValue> SCRIPT_CACHE_EXPIRE_SETTING =
         Setting.positiveTimeSetting("script.cache.expire", TimeValue.timeValueMillis(0), Property.NodeScope);
     public static final Setting<Boolean> 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<Integer> SCRIPT_MAX_SIZE_IN_BYTES =
         Setting.intSetting("script.max_size_in_bytes", 65535, Property.NodeScope);
     public static final Setting<Integer> 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 {

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.");
     }
 }

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());

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});
     }
 
 }

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)];

core/src/test/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetricTests.java

@@ -69,8 +69,6 @@ protected InternalScriptedMetric createTestInstance(String name, List<PipelineAg
     protected ScriptService mockScriptService() {
         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();
         // mock script always retuns the size of the input aggs list as result
         @SuppressWarnings("unchecked")

core/src/test/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregatorTests.java

@@ -198,9 +198,7 @@ public void testScriptedMetricWithCombineAccessesScores() throws IOException {
     @Override
     protected QueryShardContext queryShardContextMock(MapperService mapperService, final MappedFieldType[] fieldTypes,
             CircuitBreakerService circuitBreakerService) {
-        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();
+        Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()).build();
         MockScriptEngine scriptEngine = new MockScriptEngine(MockScriptEngine.NAME, SCRIPTS);
         ScriptEngineRegistry scriptEngineRegistry = new ScriptEngineRegistry(Collections.singletonList(scriptEngine));
         ScriptContextRegistry scriptContextRegistry = new ScriptContextRegistry(Collections.emptyList());

core/src/test/java/org/elasticsearch/tribe/TribeServiceTests.java

@@ -23,8 +23,11 @@
 import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.network.NetworkModule;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.set.Sets;
+import org.elasticsearch.env.Environment;
+import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.TestCustomMetaData;
 
@@ -76,6 +79,7 @@ public void testEnvironmentSettings() {
             TribeService.buildClientSettings("tribe1", "parent_id", globalSettings, tribeSettings);
         });
         assertTrue(e.getMessage(), e.getMessage().contains("Setting [path.home] not allowed in tribe client"));
+        assertSettingDeprecationsAndWarnings(new Setting[] {Environment.PATH_SCRIPTS_SETTING});
     }
 
     public void testPassthroughSettings() {

qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilSecurityTests.java

@@ -22,6 +22,7 @@
 import org.apache.lucene.util.Constants;
 import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.io.PathUtils;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.test.ESTestCase;
@@ -118,6 +119,7 @@ public void testEnvironmentPaths() throws Exception {
         assertExactPermissions(new FilePermission(environment.configFile().toString(), "read,readlink"), permissions);
         // scripts file: ro
         assertExactPermissions(new FilePermission(environment.scriptsFile().toString(), "read,readlink"), permissions);
+        assertSettingDeprecationsAndWarnings(new Setting<?>[] {Environment.PATH_SCRIPTS_SETTING});
         // plugins: ro
         assertExactPermissions(new FilePermission(environment.pluginsFile().toString(), "read,readlink"), permissions);
 

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());

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<ScriptPlugin> 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);

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"));