From 6ac8a1eb85636822bdd7d60718a6fb1a55fd4384 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Wed, 10 May 2017 13:09:31 -0700 Subject: [PATCH] Deprecate Fine Grain Settings for Scripts (#24573) --- .../elasticsearch/script/ScriptSettings.java | 14 +++++---- .../elasticsearch/script/FileScriptTests.java | 15 ++++++++-- .../script/ScriptContextTests.java | 29 ++++++++++++++----- .../script/ScriptModesTests.java | 13 +++++++++ .../script/ScriptServiceTests.java | 16 +++++++++- .../script/ScriptSettingsTests.java | 24 +++++++++++++++ .../test/ESSingleNodeTestCase.java | 2 -- 7 files changed, 93 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/script/ScriptSettings.java b/core/src/main/java/org/elasticsearch/script/ScriptSettings.java index cc928a8b597..9d9b1255869 100644 --- a/core/src/main/java/org/elasticsearch/script/ScriptSettings.java +++ b/core/src/main/java/org/elasticsearch/script/ScriptSettings.java @@ -41,7 +41,8 @@ public class ScriptSettings { scriptTypeSettingMap.put(scriptType, Setting.boolSetting( ScriptModes.sourceKey(scriptType), scriptType.isDefaultEnabled(), - Property.NodeScope)); + Property.NodeScope, + Property.Deprecated)); } SCRIPT_TYPE_SETTING_MAP = Collections.unmodifiableMap(scriptTypeSettingMap); } @@ -61,7 +62,7 @@ public class ScriptSettings { Map> scriptContextSettingMap = new HashMap<>(); for (ScriptContext scriptContext : scriptContextRegistry.scriptContexts()) { scriptContextSettingMap.put(scriptContext, - Setting.boolSetting(ScriptModes.operationKey(scriptContext), false, Property.NodeScope)); + Setting.boolSetting(ScriptModes.operationKey(scriptContext), false, Property.NodeScope, Property.Deprecated)); } return scriptContextSettingMap; } @@ -91,7 +92,7 @@ public class ScriptSettings { Function defaultLangAndTypeFn = settings -> { final Setting globalTypeSetting = scriptTypeSettingMap.get(scriptType); final Setting langAndTypeSetting = Setting.boolSetting(ScriptModes.getGlobalKey(language, scriptType), - defaultIfNothingSet, Property.NodeScope); + defaultIfNothingSet, Property.NodeScope, Property.Deprecated); if (langAndTypeSetting.exists(settings)) { // fine-grained e.g. script.engine.groovy.inline @@ -106,7 +107,7 @@ public class ScriptSettings { // Setting for something like "script.engine.groovy.inline" final Setting langAndTypeSetting = Setting.boolSetting(ScriptModes.getGlobalKey(language, scriptType), - defaultLangAndTypeFn, Property.NodeScope); + defaultLangAndTypeFn, Property.NodeScope, Property.Deprecated); scriptModeSettings.add(langAndTypeSetting); for (ScriptContext scriptContext : scriptContextRegistry.scriptContexts()) { @@ -117,7 +118,7 @@ public class ScriptSettings { final Setting globalOpSetting = scriptContextSettingMap.get(scriptContext); final Setting globalTypeSetting = scriptTypeSettingMap.get(scriptType); final Setting langAndTypeAndContextSetting = Setting.boolSetting(langAndTypeAndContextName, - defaultIfNothingSet, Property.NodeScope); + defaultIfNothingSet, Property.NodeScope, Property.Deprecated); // fallback logic for script mode settings if (langAndTypeAndContextSetting.exists(settings)) { @@ -138,7 +139,8 @@ public class ScriptSettings { } }; // The actual setting for finest grained script settings - Setting setting = Setting.boolSetting(langAndTypeAndContextName, defaultSettingFn, Property.NodeScope); + Setting setting = + Setting.boolSetting(langAndTypeAndContextName, defaultSettingFn, Property.NodeScope, Property.Deprecated); scriptModeSettings.add(setting); } } diff --git a/core/src/test/java/org/elasticsearch/script/FileScriptTests.java b/core/src/test/java/org/elasticsearch/script/FileScriptTests.java index af32e8abec7..0ad3bb08ab2 100644 --- a/core/src/test/java/org/elasticsearch/script/FileScriptTests.java +++ b/core/src/test/java/org/elasticsearch/script/FileScriptTests.java @@ -31,6 +31,8 @@ import java.util.Collections; // TODO: these really should just be part of ScriptService tests, there is nothing special about them public class FileScriptTests extends ESTestCase { + private ScriptSettings scriptSettings; + ScriptService makeScriptService(Settings settings) throws Exception { Path homeDir = createTempDir(); Path scriptsDir = homeDir.resolve("config").resolve("scripts"); @@ -47,7 +49,7 @@ public class FileScriptTests extends ESTestCase { MockScriptEngine scriptEngine = new MockScriptEngine(MockScriptEngine.NAME, Collections.singletonMap(scriptSource, script -> "1")); ScriptEngineRegistry scriptEngineRegistry = new ScriptEngineRegistry(Collections.singleton(scriptEngine)); ScriptContextRegistry scriptContextRegistry = new ScriptContextRegistry(Collections.emptyList()); - ScriptSettings scriptSettings = new ScriptSettings(scriptEngineRegistry, scriptContextRegistry); + scriptSettings = new ScriptSettings(scriptEngineRegistry, scriptContextRegistry); return new ScriptService(settings, new Environment(settings), null, scriptEngineRegistry, scriptContextRegistry, scriptSettings); } @@ -60,7 +62,9 @@ public class FileScriptTests extends ESTestCase { assertNotNull(compiledScript); MockCompiledScript executable = (MockCompiledScript) compiledScript.compiled(); assertEquals("script1.mockscript", executable.getName()); - assertSettingDeprecationsAndWarnings(new Setting[] {ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING}, + assertSettingDeprecationsAndWarnings(ScriptSettingsTests.buildDeprecatedSettingsArray( + new Setting[] {ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING}, + scriptSettings, "script.engine." + MockScriptEngine.NAME + ".file.aggs"), "File scripts are deprecated. Use stored or inline scripts instead."); } @@ -81,7 +85,12 @@ public class FileScriptTests extends ESTestCase { assertTrue(e.getMessage(), e.getMessage().contains("scripts of type [file], operation [" + context.getKey() + "] and lang [" + MockScriptEngine.NAME + "] are disabled")); } } - assertSettingDeprecationsAndWarnings(new Setting[] {ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING}, + assertSettingDeprecationsAndWarnings(ScriptSettingsTests.buildDeprecatedSettingsArray( + new Setting[] {ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING}, scriptSettings, + "script.engine." + MockScriptEngine.NAME + ".file.aggs", + "script.engine." + MockScriptEngine.NAME + ".file.search", + "script.engine." + MockScriptEngine.NAME + ".file.update", + "script.engine." + MockScriptEngine.NAME + ".file.ingest"), "File scripts are deprecated. Use stored or inline scripts instead."); } } diff --git a/core/src/test/java/org/elasticsearch/script/ScriptContextTests.java b/core/src/test/java/org/elasticsearch/script/ScriptContextTests.java index 83608a010b1..1d627c0d6a3 100644 --- a/core/src/test/java/org/elasticsearch/script/ScriptContextTests.java +++ b/core/src/test/java/org/elasticsearch/script/ScriptContextTests.java @@ -37,14 +37,18 @@ import static org.hamcrest.Matchers.containsString; public class ScriptContextTests extends ESTestCase { private static final String PLUGIN_NAME = "testplugin"; + private static final String SCRIPT_PLUGIN_CUSTOM_SETTING = "script." + PLUGIN_NAME + "_custom_globally_disabled_op"; + private static final String SCRIPT_ENGINE_CUSTOM_SETTING = "script.engine." + MockScriptEngine.NAME + ".inline." + PLUGIN_NAME + "_custom_exp_disabled_op"; + + private ScriptSettings scriptSettings; ScriptService makeScriptService() throws Exception { 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") - .put("script." + PLUGIN_NAME + "_custom_globally_disabled_op", "false") - .put("script.engine." + MockScriptEngine.NAME + ".inline." + PLUGIN_NAME + "_custom_exp_disabled_op", "false") + .put(SCRIPT_PLUGIN_CUSTOM_SETTING, "false") + .put(SCRIPT_ENGINE_CUSTOM_SETTING, "false") .build(); MockScriptEngine scriptEngine = new MockScriptEngine(MockScriptEngine.NAME, Collections.singletonMap("1", script -> "1")); @@ -54,7 +58,7 @@ public class ScriptContextTests extends ESTestCase { new ScriptContext.Plugin(PLUGIN_NAME, "custom_exp_disabled_op"), new ScriptContext.Plugin(PLUGIN_NAME, "custom_globally_disabled_op")); ScriptContextRegistry scriptContextRegistry = new ScriptContextRegistry(customContexts); - ScriptSettings scriptSettings = new ScriptSettings(scriptEngineRegistry, scriptContextRegistry); + scriptSettings = new ScriptSettings(scriptEngineRegistry, scriptContextRegistry); ScriptService scriptService = new ScriptService(settings, new Environment(settings), null, scriptEngineRegistry, scriptContextRegistry, scriptSettings); ClusterState empty = ClusterState.builder(new ClusterName("_name")).build(); @@ -67,6 +71,8 @@ public class ScriptContextTests extends ESTestCase { return scriptService; } + + public void testCustomGlobalScriptContextSettings() throws Exception { ScriptService scriptService = makeScriptService(); for (ScriptType scriptType : ScriptType.values()) { @@ -78,7 +84,9 @@ public class ScriptContextTests extends ESTestCase { 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}); + assertSettingDeprecationsAndWarnings( + ScriptSettingsTests.buildDeprecatedSettingsArray(new Setting[] {ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING}, + scriptSettings, SCRIPT_PLUGIN_CUSTOM_SETTING, SCRIPT_ENGINE_CUSTOM_SETTING)); } public void testCustomScriptContextSettings() throws Exception { @@ -95,7 +103,9 @@ public class ScriptContextTests extends ESTestCase { 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}); + assertSettingDeprecationsAndWarnings( + ScriptSettingsTests.buildDeprecatedSettingsArray(new Setting[] {ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING}, + scriptSettings, SCRIPT_PLUGIN_CUSTOM_SETTING, SCRIPT_ENGINE_CUSTOM_SETTING)); } public void testUnknownPluginScriptContext() throws Exception { @@ -109,7 +119,9 @@ public class ScriptContextTests extends ESTestCase { assertTrue(e.getMessage(), e.getMessage().contains("script context [" + PLUGIN_NAME + "_unknown] not supported")); } } - assertSettingDeprecationsAndWarnings(new Setting[] {ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING}); + assertSettingDeprecationsAndWarnings( + ScriptSettingsTests.buildDeprecatedSettingsArray(new Setting[] {ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING}, + scriptSettings, SCRIPT_PLUGIN_CUSTOM_SETTING, SCRIPT_ENGINE_CUSTOM_SETTING)); } public void testUnknownCustomScriptContext() throws Exception { @@ -129,7 +141,8 @@ public class ScriptContextTests extends ESTestCase { assertTrue(e.getMessage(), e.getMessage().contains("script context [test] not supported")); } } - assertSettingDeprecationsAndWarnings(new Setting[] {ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING}); + assertSettingDeprecationsAndWarnings( + ScriptSettingsTests.buildDeprecatedSettingsArray(new Setting[] {ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING}, + scriptSettings, SCRIPT_PLUGIN_CUSTOM_SETTING, SCRIPT_ENGINE_CUSTOM_SETTING)); } - } diff --git a/core/src/test/java/org/elasticsearch/script/ScriptModesTests.java b/core/src/test/java/org/elasticsearch/script/ScriptModesTests.java index a2db206858c..63bd3fd3777 100644 --- a/core/src/test/java/org/elasticsearch/script/ScriptModesTests.java +++ b/core/src/test/java/org/elasticsearch/script/ScriptModesTests.java @@ -26,9 +26,11 @@ import org.elasticsearch.test.ESTestCase; import org.junit.After; import org.junit.Before; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; @@ -123,9 +125,11 @@ public class ScriptModesTests extends ESTestCase { randomScriptModes[i] = randomBoolean(); } ScriptType[] randomScriptTypes = randomScriptTypesSet.toArray(new ScriptType[randomScriptTypesSet.size()]); + List deprecated = new ArrayList<>(); Settings.Builder builder = Settings.builder(); for (int i = 0; i < randomInt; i++) { builder.put("script" + "." + randomScriptTypes[i].getName(), randomScriptModes[i]); + deprecated.add("script" + "." + randomScriptTypes[i].getName()); } this.scriptModes = new ScriptModes(scriptSettings, builder.build()); @@ -141,6 +145,8 @@ public class ScriptModesTests extends ESTestCase { if (randomScriptTypesSet.contains(ScriptType.INLINE) == false) { assertScriptModesAllOps(false, ScriptType.INLINE); } + assertSettingDeprecationsAndWarnings( + ScriptSettingsTests.buildDeprecatedSettingsArray(scriptSettings, deprecated.toArray(new String[] {}))); } public void testScriptContextGenericSettings() { @@ -155,9 +161,11 @@ public class ScriptModesTests extends ESTestCase { randomScriptModes[i] = randomBoolean(); } ScriptContext[] randomScriptContexts = randomScriptContextsSet.toArray(new ScriptContext[randomScriptContextsSet.size()]); + List deprecated = new ArrayList<>(); Settings.Builder builder = Settings.builder(); for (int i = 0; i < randomInt; i++) { builder.put("script" + "." + randomScriptContexts[i].getKey(), randomScriptModes[i]); + deprecated.add("script" + "." + randomScriptContexts[i].getKey()); } this.scriptModes = new ScriptModes(scriptSettings, builder.build()); @@ -168,6 +176,8 @@ public class ScriptModesTests extends ESTestCase { ScriptContext[] complementOf = complementOf(randomScriptContexts); assertScriptModes(true, new ScriptType[]{ScriptType.FILE}, complementOf); assertScriptModes(false, new ScriptType[]{ScriptType.STORED, ScriptType.INLINE}, complementOf); + assertSettingDeprecationsAndWarnings( + ScriptSettingsTests.buildDeprecatedSettingsArray(scriptSettings, deprecated.toArray(new String[] {}))); } public void testConflictingScriptTypeAndOpGenericSettings() { @@ -182,6 +192,9 @@ public class ScriptModesTests extends ESTestCase { ScriptContext[] complementOf = complementOf(scriptContext); assertScriptModes(true, new ScriptType[]{ScriptType.FILE, ScriptType.STORED}, complementOf); assertScriptModes(true, new ScriptType[]{ScriptType.INLINE}, complementOf); + assertSettingDeprecationsAndWarnings( + ScriptSettingsTests.buildDeprecatedSettingsArray( + scriptSettings, "script." + scriptContext.getKey(), "script.stored", "script.inline")); } private void assertScriptModesAllOps(boolean expectedScriptEnabled, ScriptType... scriptTypes) { diff --git a/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java b/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java index 6928d565487..9fdbcd6f90d 100644 --- a/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java +++ b/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java @@ -40,10 +40,12 @@ import org.junit.Before; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import static org.hamcrest.CoreMatchers.containsString; @@ -263,6 +265,7 @@ public class ScriptServiceTests extends ESTestCase { } while (engineSettings.containsKey(settingKey)); engineSettings.put(settingKey, randomBoolean()); } + List deprecated = new ArrayList<>(); //set the selected fine-grained settings Settings.Builder builder = Settings.builder(); for (Map.Entry entry : scriptSourceSettings.entrySet()) { @@ -271,6 +274,7 @@ public class ScriptServiceTests extends ESTestCase { } else { builder.put("script" + "." + entry.getKey().getName(), "false"); } + deprecated.add("script" + "." + entry.getKey().getName()); } for (Map.Entry entry : scriptContextSettings.entrySet()) { if (entry.getValue()) { @@ -278,6 +282,7 @@ public class ScriptServiceTests extends ESTestCase { } else { builder.put("script" + "." + entry.getKey().getKey(), "false"); } + deprecated.add("script" + "." + entry.getKey().getKey()); } for (Map.Entry entry : engineSettings.entrySet()) { int delimiter = entry.getKey().indexOf('.'); @@ -290,6 +295,7 @@ public class ScriptServiceTests extends ESTestCase { } else { builder.put("script.engine" + "." + lang + "." + part2, "false"); } + deprecated.add("script.engine" + "." + lang + "." + part2); } buildScriptService(builder.build()); @@ -320,7 +326,9 @@ public class ScriptServiceTests extends ESTestCase { } } } - assertWarnings("File scripts are deprecated. Use stored or inline scripts instead."); + assertSettingDeprecationsAndWarnings( + ScriptSettingsTests.buildDeprecatedSettingsArray(scriptSettings, deprecated.toArray(new String[] {})), + "File scripts are deprecated. Use stored or inline scripts instead."); } public void testCompileNonRegisteredContext() throws IOException { @@ -381,6 +389,8 @@ public class ScriptServiceTests extends ESTestCase { scriptService.compile(script, randomFrom(scriptContexts)); scriptService.compile(script, randomFrom(scriptContexts)); assertEquals(1L, scriptService.stats().getCompilations()); + assertSettingDeprecationsAndWarnings( + ScriptSettingsTests.buildDeprecatedSettingsArray(scriptSettings, "script.inline")); } public void testFileScriptCountedInCompilationStats() throws IOException { @@ -406,6 +416,8 @@ public class ScriptServiceTests extends ESTestCase { scriptService.compile(new Script(ScriptType.INLINE, "test", "2+2", Collections.emptyMap()), randomFrom(scriptContexts)); assertEquals(2L, scriptService.stats().getCompilations()); assertEquals(1L, scriptService.stats().getCacheEvictions()); + assertSettingDeprecationsAndWarnings( + ScriptSettingsTests.buildDeprecatedSettingsArray(scriptSettings, "script.inline")); } public void testDefaultLanguage() throws IOException { @@ -415,6 +427,8 @@ public class ScriptServiceTests extends ESTestCase { CompiledScript script = scriptService.compile( new Script(ScriptType.INLINE, Script.DEFAULT_SCRIPT_LANG, "1 + 1", Collections.emptyMap()), randomFrom(scriptContexts)); assertEquals(script.lang(), Script.DEFAULT_SCRIPT_LANG); + assertSettingDeprecationsAndWarnings( + ScriptSettingsTests.buildDeprecatedSettingsArray(scriptSettings, "script.inline")); } public void testStoreScript() throws Exception { diff --git a/core/src/test/java/org/elasticsearch/script/ScriptSettingsTests.java b/core/src/test/java/org/elasticsearch/script/ScriptSettingsTests.java index 6eea1fa8010..1315ce4970b 100644 --- a/core/src/test/java/org/elasticsearch/script/ScriptSettingsTests.java +++ b/core/src/test/java/org/elasticsearch/script/ScriptSettingsTests.java @@ -33,6 +33,29 @@ import static org.hamcrest.Matchers.equalTo; public class ScriptSettingsTests extends ESTestCase { + public static Setting[] buildDeprecatedSettingsArray(ScriptSettings scriptSettings, String... keys) { + return buildDeprecatedSettingsArray(null, scriptSettings, keys); + } + + public static Setting[] buildDeprecatedSettingsArray(Setting[] deprecated, ScriptSettings scriptSettings, String... keys) { + Setting[] settings = new Setting[keys.length + (deprecated == null ? 0 : deprecated.length)]; + int count = 0; + + for (Setting setting : scriptSettings.getSettings()) { + for (String key : keys) { + if (setting.getKey().equals(key)) { + settings[count++] = setting; + } + } + } + + if (deprecated != null) { + System.arraycopy(deprecated, 0, settings, keys.length, deprecated.length); + } + + return settings; + } + public void testSettingsAreProperlyPropogated() { ScriptEngineRegistry scriptEngineRegistry = new ScriptEngineRegistry(Collections.singletonList(new CustomScriptEngine())); @@ -47,6 +70,7 @@ public class ScriptSettingsTests extends ESTestCase { assertThat(setting.getDefaultRaw(s), equalTo(Boolean.toString(enabled))); } } + assertSettingDeprecationsAndWarnings(buildDeprecatedSettingsArray(scriptSettings, "script.inline")); } private static class CustomScriptEngine implements ScriptEngine { diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java index 99cb7690c82..bc36a7999ba 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java @@ -172,8 +172,6 @@ public abstract class ESSingleNodeTestCase extends ESTestCase { // This needs to tie into the ESIntegTestCase#indexSettings() method .put(Environment.PATH_SHARED_DATA_SETTING.getKey(), createTempDir().getParent()) .put("node.name", "node_s_0") - .put("script.inline", "true") - .put("script.stored", "true") .put(ScriptService.SCRIPT_MAX_COMPILATIONS_PER_MINUTE.getKey(), 1000) .put(EsExecutors.PROCESSORS_SETTING.getKey(), 1) // limit the number of threads created .put(NetworkModule.HTTP_ENABLED.getKey(), false)