Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Deprecate Fine Grain Settings for Scripts #24573

Merged
merged 10 commits into from May 10, 2017
Merged

Deprecate Fine Grain Settings for Scripts #24573

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7a0baa5
Deprecate extraneous script settings.
jdconrad May 9, 2017
29f27f8
Added deprecation warning checks for settings in script tests.
jdconrad May 9, 2017
198db69
Add missing deprecations.
jdconrad May 9, 2017
ca85e77
Merge branch 'master' into settings
jdconrad May 9, 2017
cc13429
Fix arraycopy.
jdconrad May 9, 2017
52a6efc
Merge branch 'master' into settings
jdconrad May 9, 2017
5a75b2f
Remove unnecessary default settings.
jdconrad May 10, 2017
92a981d
Merge branch 'master' into settings
jdconrad May 10, 2017
e3d4a5e
Merge branch 'master' into settings
jdconrad May 10, 2017
ae9d666
Merge branch 'master' into settings
jdconrad May 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 8 additions & 6 deletions core/src/main/java/org/elasticsearch/script/ScriptSettings.java
View file
Open in desktop
Expand Up @@ -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);
}
Expand All @@ -61,7 +62,7 @@ private static Map<ScriptContext, Setting<Boolean>> contextSettings(ScriptContex
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;
}
Expand Down Expand Up @@ -91,7 +92,7 @@ private static List<Setting<Boolean>> languageSettings(Map<ScriptType, Setting<B
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
Expand All @@ -106,7 +107,7 @@ private static List<Setting<Boolean>> languageSettings(Map<ScriptType, Setting<B

// 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()) {
Expand All @@ -117,7 +118,7 @@ private static List<Setting<Boolean>> languageSettings(Map<ScriptType, Setting<B
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)) {
Expand All @@ -138,7 +139,8 @@ private static List<Setting<Boolean>> languageSettings(Map<ScriptType, Setting<B
}
};
// 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);
}
}
Expand Down
15 changes: 12 additions & 3 deletions core/src/test/java/org/elasticsearch/script/FileScriptTests.java
View file
Open in desktop
Expand Up @@ -31,6 +31,8 @@
// 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");
Expand All @@ -47,7 +49,7 @@ ScriptService makeScriptService(Settings settings) throws Exception {
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);
}

Expand All @@ -60,7 +62,9 @@ public void testFileScriptFound() throws Exception {
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.");
}

Expand All @@ -81,7 +85,12 @@ public void testAllOpsDisabled() throws Exception {
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.");
}
}
29 changes: 21 additions & 8 deletions core/src/test/java/org/elasticsearch/script/ScriptContextTests.java
View file
Open in desktop
Expand Up @@ -37,14 +37,18 @@
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"));
Expand All @@ -54,7 +58,7 @@ ScriptService makeScriptService() throws Exception {
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();
Expand All @@ -67,6 +71,8 @@ ScriptService makeScriptService() throws Exception {
return scriptService;
}



public void testCustomGlobalScriptContextSettings() throws Exception {
ScriptService scriptService = makeScriptService();
for (ScriptType scriptType : ScriptType.values()) {
Expand All @@ -78,7 +84,9 @@ 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});
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 {
Expand All @@ -95,7 +103,9 @@ 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});
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 {
Expand All @@ -109,7 +119,9 @@ 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});
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 {
Expand All @@ -129,7 +141,8 @@ public String getKey() {
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));
}

}
14 changes: 14 additions & 0 deletions core/src/test/java/org/elasticsearch/script/ScriptModesTests.java
View file
Open in desktop
Expand Up @@ -20,15 +20,18 @@
package org.elasticsearch.script;

import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.search.lookup.SearchLookup;
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;

Expand Down Expand Up @@ -123,9 +126,11 @@ public void testScriptTypeGenericSettings() {
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());

Expand All @@ -141,6 +146,8 @@ public void testScriptTypeGenericSettings() {
if (randomScriptTypesSet.contains(ScriptType.INLINE) == false) {
assertScriptModesAllOps(false, ScriptType.INLINE);
}
assertSettingDeprecationsAndWarnings(
ScriptSettingsTests.buildDeprecatedSettingsArray(scriptSettings, deprecated.toArray(new String[] {})));
}

public void testScriptContextGenericSettings() {
Expand All @@ -155,9 +162,11 @@ public void testScriptContextGenericSettings() {
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());

Expand All @@ -168,6 +177,8 @@ public void testScriptContextGenericSettings() {
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() {
Expand All @@ -182,6 +193,9 @@ public void testConflictingScriptTypeAndOpGenericSettings() {
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) {
Expand Down
17 changes: 16 additions & 1 deletion core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java
View file
Open in desktop
Expand Up @@ -28,6 +28,7 @@
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.Streams;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentType;
Expand All @@ -40,10 +41,12 @@
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;
Expand Down Expand Up @@ -263,6 +266,7 @@ public void testFineGrainedSettings() throws IOException {
} 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()) {
Expand All @@ -271,13 +275,15 @@ public void testFineGrainedSettings() throws IOException {
} 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()) {
builder.put("script" + "." + entry.getKey().getKey(), "true");
} 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('.');
Expand All @@ -290,6 +296,7 @@ public void testFineGrainedSettings() throws IOException {
} else {
builder.put("script.engine" + "." + lang + "." + part2, "false");
}
deprecated.add("script.engine" + "." + lang + "." + part2);
}

buildScriptService(builder.build());
Expand Down Expand Up @@ -320,7 +327,9 @@ public void testFineGrainedSettings() throws IOException {
}
}
}
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 {
Expand Down Expand Up @@ -381,6 +390,8 @@ public void testCompilationStatsOnCacheHit() throws IOException {
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 {
Expand All @@ -406,6 +417,8 @@ public void testCacheEvictionCountedInCacheEvictionsStats() throws IOException {
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 {
Expand All @@ -415,6 +428,8 @@ public void testDefaultLanguage() throws IOException {
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 {
Expand Down