Deprecate Fine Grain Settings for Scripts (#24573)

This commit is contained in:
Jack Conradson 2017-05-10 13:09:31 -07:00 committed by GitHub
parent 743217a430
commit 6ac8a1eb85
7 changed files with 93 additions and 20 deletions

View File

@ -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<ScriptContext, Setting<Boolean>> 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<Settings, String> defaultLangAndTypeFn = settings -> {
final Setting<Boolean> globalTypeSetting = scriptTypeSettingMap.get(scriptType);
final Setting<Boolean> 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<Boolean> 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<Boolean> globalOpSetting = scriptContextSettingMap.get(scriptContext);
final Setting<Boolean> globalTypeSetting = scriptTypeSettingMap.get(scriptType);
final Setting<Boolean> 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<Boolean> setting = Setting.boolSetting(langAndTypeAndContextName, defaultSettingFn, Property.NodeScope);
Setting<Boolean> setting =
Setting.boolSetting(langAndTypeAndContextName, defaultSettingFn, Property.NodeScope, Property.Deprecated);
scriptModeSettings.add(setting);
}
}

View File

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

View File

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

View File

@ -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<String> 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<String> 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) {

View File

@ -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<String> deprecated = new ArrayList<>();
//set the selected fine-grained settings
Settings.Builder builder = Settings.builder();
for (Map.Entry<ScriptType, Boolean> 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<ScriptContext, Boolean> 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<String, Boolean> 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 {

View File

@ -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 {

View File

@ -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)