Skip to content

Commit 99e0268

Browse files
jasontedorjasontedor
authored
Remove support for default settings
Today Elasticsearch allows default settings to be used only if the actual setting is not set. These settings are trappy, and the complexity invites bugs. This commit removes support for default settings with the exception of default.path.data, default.path.conf, and default.path.logs which are maintainted to support packaging. A follow-up will remove support for these as well. Relates #24093
1 parent 52c9159 commit 99e0268

File tree

8 files changed

+115
-64
lines changed

8 files changed

+115
-64
lines changed

‎core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,12 @@ public void apply(Settings value, Settings current, Settings previous) {
311311
HunspellService.HUNSPELL_IGNORE_CASE,
312312
HunspellService.HUNSPELL_DICTIONARY_OPTIONS,
313313
IndicesStore.INDICES_STORE_DELETE_SHARD_TIMEOUT,
314+
Environment.DEFAULT_PATH_CONF_SETTING,
314315
Environment.PATH_CONF_SETTING,
316+
Environment.DEFAULT_PATH_DATA_SETTING,
315317
Environment.PATH_DATA_SETTING,
316318
Environment.PATH_HOME_SETTING,
319+
Environment.DEFAULT_PATH_LOGS_SETTING,
317320
Environment.PATH_LOGS_SETTING,
318321
Environment.PATH_REPO_SETTING,
319322
Environment.PATH_SCRIPTS_SETTING,

‎core/src/main/java/org/elasticsearch/common/settings/Settings.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,12 +1063,10 @@ public Builder loadFromStream(String resourceName, InputStream is) throws IOExce
10631063
return this;
10641064
}
10651065

1066-
public Builder putProperties(Map<String, String> esSettings, Predicate<String> keyPredicate, Function<String, String> keyFunction) {
1066+
public Builder putProperties(final Map<String, String> esSettings, final Function<String, String> keyFunction) {
10671067
for (final Map.Entry<String, String> esSetting : esSettings.entrySet()) {
10681068
final String key = esSetting.getKey();
1069-
if (keyPredicate.test(key)) {
1070-
map.put(keyFunction.apply(key), esSetting.getValue());
1071-
}
1069+
map.put(keyFunction.apply(key), esSetting.getValue());
10721070
}
10731071
return this;
10741072
}

‎core/src/main/java/org/elasticsearch/env/Environment.java

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,17 @@
4949
// public+forbidden api!
5050
public class Environment {
5151
public static final Setting<String> PATH_HOME_SETTING = Setting.simpleString("path.home", Property.NodeScope);
52-
public static final Setting<String> PATH_CONF_SETTING = Setting.simpleString("path.conf", Property.NodeScope);
52+
public static final Setting<String> DEFAULT_PATH_CONF_SETTING = Setting.simpleString("default.path.conf", Property.NodeScope);
53+
public static final Setting<String> PATH_CONF_SETTING =
54+
new Setting<>("path.conf", DEFAULT_PATH_CONF_SETTING, Function.identity(), Property.NodeScope);
5355
public static final Setting<String> PATH_SCRIPTS_SETTING = Setting.simpleString("path.scripts", Property.NodeScope);
56+
public static final Setting<List<String>> DEFAULT_PATH_DATA_SETTING =
57+
Setting.listSetting("default.path.data", Collections.emptyList(), Function.identity(), Property.NodeScope);
5458
public static final Setting<List<String>> PATH_DATA_SETTING =
55-
Setting.listSetting("path.data", Collections.emptyList(), Function.identity(), Property.NodeScope);
56-
public static final Setting<String> PATH_LOGS_SETTING = Setting.simpleString("path.logs", Property.NodeScope);
59+
Setting.listSetting("path.data", DEFAULT_PATH_DATA_SETTING, Function.identity(), Property.NodeScope);
60+
public static final Setting<String> DEFAULT_PATH_LOGS_SETTING = Setting.simpleString("default.path.logs", Property.NodeScope);
61+
public static final Setting<String> PATH_LOGS_SETTING =
62+
new Setting<>("path.logs", DEFAULT_PATH_LOGS_SETTING, Function.identity(), Property.NodeScope);
5763
public static final Setting<List<String>> PATH_REPO_SETTING =
5864
Setting.listSetting("path.repo", Collections.emptyList(), Function.identity(), Property.NodeScope);
5965
public static final Setting<String> PATH_SHARED_DATA_SETTING = Setting.simpleString("path.shared_data", Property.NodeScope);
@@ -115,7 +121,8 @@ public Environment(Settings settings) {
115121
throw new IllegalStateException(PATH_HOME_SETTING.getKey() + " is not configured");
116122
}
117123

118-
if (PATH_CONF_SETTING.exists(settings)) {
124+
// this is trappy, Setting#get(Settings) will get a fallback setting yet return false for Settings#exists(Settings)
125+
if (PATH_CONF_SETTING.exists(settings) || DEFAULT_PATH_CONF_SETTING.exists(settings)) {
119126
configFile = PathUtils.get(cleanPath(PATH_CONF_SETTING.get(settings)));
120127
} else {
121128
configFile = homeFile.resolve("config");
@@ -156,7 +163,9 @@ public Environment(Settings settings) {
156163
} else {
157164
repoFiles = new Path[0];
158165
}
159-
if (PATH_LOGS_SETTING.exists(settings)) {
166+
167+
// this is trappy, Setting#get(Settings) will get a fallback setting yet return false for Settings#exists(Settings)
168+
if (PATH_LOGS_SETTING.exists(settings) || DEFAULT_PATH_LOGS_SETTING.exists(settings)) {
160169
logsFile = PathUtils.get(cleanPath(PATH_LOGS_SETTING.get(settings)));
161170
} else {
162171
logsFile = homeFile.resolve("logs");

‎core/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,12 @@
3737
import java.util.Map;
3838
import java.util.Set;
3939
import java.util.function.Function;
40-
import java.util.function.Predicate;
41-
import java.util.function.UnaryOperator;
4240

4341
import static org.elasticsearch.common.Strings.cleanPath;
4442

4543
public class InternalSettingsPreparer {
4644

4745
private static final String[] ALLOWED_SUFFIXES = {".yml", ".yaml", ".json"};
48-
private static final String PROPERTY_DEFAULTS_PREFIX = "default.";
49-
private static final Predicate<String> PROPERTY_DEFAULTS_PREDICATE = key -> key.startsWith(PROPERTY_DEFAULTS_PREFIX);
50-
private static final UnaryOperator<String> STRIP_PROPERTY_DEFAULTS_PREFIX = key -> key.substring(PROPERTY_DEFAULTS_PREFIX.length());
5146

5247
public static final String SECRET_PROMPT_VALUE = "${prompt.secret}";
5348
public static final String TEXT_PROMPT_VALUE = "${prompt.text}";
@@ -125,22 +120,16 @@ public static Environment prepareEnvironment(Settings input, Terminal terminal,
125120
}
126121

127122
/**
128-
* Initializes the builder with the given input settings, and applies settings and default settings from the specified map (these
129-
* settings typically come from the command line). The default settings are applied only if the setting does not exist in the specified
130-
* output.
123+
* Initializes the builder with the given input settings, and applies settings from the specified map (these settings typically come
124+
* from the command line).
131125
*
132126
* @param output the settings builder to apply the input and default settings to
133127
* @param input the input settings
134-
* @param esSettings a map from which to apply settings and default settings
128+
* @param esSettings a map from which to apply settings
135129
*/
136130
static void initializeSettings(final Settings.Builder output, final Settings input, final Map<String, String> esSettings) {
137131
output.put(input);
138-
output.putProperties(esSettings,
139-
PROPERTY_DEFAULTS_PREDICATE
140-
.and(key -> output.get(STRIP_PROPERTY_DEFAULTS_PREFIX.apply(key)) == null)
141-
.and(key -> output.get(STRIP_PROPERTY_DEFAULTS_PREFIX.apply(key) + ".0") == null),
142-
STRIP_PROPERTY_DEFAULTS_PREFIX);
143-
output.putProperties(esSettings, PROPERTY_DEFAULTS_PREDICATE.negate(), Function.identity());
132+
output.putProperties(esSettings, Function.identity());
144133
output.replacePropertyPlaceholders();
145134
}
146135

‎core/src/test/java/org/elasticsearch/env/EnvironmentTests.java

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,12 @@
2323

2424
import java.io.IOException;
2525
import java.net.URL;
26+
import java.nio.file.Path;
2627

2728
import static org.hamcrest.CoreMatchers.endsWith;
2829
import static org.hamcrest.CoreMatchers.notNullValue;
2930
import static org.hamcrest.CoreMatchers.nullValue;
31+
import static org.hamcrest.Matchers.equalTo;
3032

3133
/**
3234
* Simple unit-tests for Environment.java
@@ -71,4 +73,91 @@ public void testRepositoryResolution() throws IOException {
7173
assertThat(environment.resolveRepoURL(new URL("jar:http://localhost/test/../repo1?blah!/repo/")), nullValue());
7274
}
7375

76+
public void testDefaultPathData() {
77+
final Path defaultPathData = createTempDir().toAbsolutePath();
78+
final Settings settings = Settings.builder()
79+
.put("path.home", createTempDir().toAbsolutePath())
80+
.put("default.path.data", defaultPathData)
81+
.build();
82+
final Environment environment = new Environment(settings);
83+
assertThat(environment.dataFiles(), equalTo(new Path[] { defaultPathData }));
84+
}
85+
86+
public void testPathDataOverrideDefaultPathData() {
87+
final Path pathData = createTempDir().toAbsolutePath();
88+
final Path defaultPathData = createTempDir().toAbsolutePath();
89+
final Settings settings = Settings.builder()
90+
.put("path.home", createTempDir().toAbsolutePath())
91+
.put("path.data", pathData)
92+
.put("default.path.data", defaultPathData)
93+
.build();
94+
final Environment environment = new Environment(settings);
95+
assertThat(environment.dataFiles(), equalTo(new Path[] { pathData }));
96+
}
97+
98+
public void testPathDataWhenNotSet() {
99+
final Path pathHome = createTempDir().toAbsolutePath();
100+
final Settings settings = Settings.builder().put("path.home", pathHome).build();
101+
final Environment environment = new Environment(settings);
102+
assertThat(environment.dataFiles(), equalTo(new Path[]{pathHome.resolve("data")}));
103+
}
104+
105+
public void testDefaultPathLogs() {
106+
final Path defaultPathLogs = createTempDir().toAbsolutePath();
107+
final Settings settings = Settings.builder()
108+
.put("path.home", createTempDir().toAbsolutePath())
109+
.put("default.path.logs", defaultPathLogs)
110+
.build();
111+
final Environment environment = new Environment(settings);
112+
assertThat(environment.logsFile(), equalTo(defaultPathLogs));
113+
}
114+
115+
public void testPathLogsOverrideDefaultPathLogs() {
116+
final Path pathLogs = createTempDir().toAbsolutePath();
117+
final Path defaultPathLogs = createTempDir().toAbsolutePath();
118+
final Settings settings = Settings.builder()
119+
.put("path.home", createTempDir().toAbsolutePath())
120+
.put("path.logs", pathLogs)
121+
.put("default.path.logs", defaultPathLogs)
122+
.build();
123+
final Environment environment = new Environment(settings);
124+
assertThat(environment.logsFile(), equalTo(pathLogs));
125+
}
126+
127+
public void testPathLogsWhenNotSet() {
128+
final Path pathHome = createTempDir().toAbsolutePath();
129+
final Settings settings = Settings.builder().put("path.home", pathHome).build();
130+
final Environment environment = new Environment(settings);
131+
assertThat(environment.logsFile(), equalTo(pathHome.resolve("logs")));
132+
}
133+
134+
public void testDefaultPathConf() {
135+
final Path defaultPathConf = createTempDir().toAbsolutePath();
136+
final Settings settings = Settings.builder()
137+
.put("path.home", createTempDir().toAbsolutePath())
138+
.put("default.path.conf", defaultPathConf)
139+
.build();
140+
final Environment environment = new Environment(settings);
141+
assertThat(environment.configFile(), equalTo(defaultPathConf));
142+
}
143+
144+
public void testPathConfOverrideDefaultPathConf() {
145+
final Path pathConf = createTempDir().toAbsolutePath();
146+
final Path defaultPathConf = createTempDir().toAbsolutePath();
147+
final Settings settings = Settings.builder()
148+
.put("path.home", createTempDir().toAbsolutePath())
149+
.put("path.conf", pathConf)
150+
.put("default.path.conf", defaultPathConf)
151+
.build();
152+
final Environment environment = new Environment(settings);
153+
assertThat(environment.configFile(), equalTo(pathConf));
154+
}
155+
156+
public void testPathConfWhenNotSet() {
157+
final Path pathHome = createTempDir().toAbsolutePath();
158+
final Settings settings = Settings.builder().put("path.home", pathHome).build();
159+
final Environment environment = new Environment(settings);
160+
assertThat(environment.configFile(), equalTo(pathHome.resolve("config")));
161+
}
162+
74163
}

‎core/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -182,29 +182,11 @@ public void testSecureSettings() {
182182
assertEquals("secret", fakeSetting.get(env.settings()).toString());
183183
}
184184

185-
public void testDefaultProperties() throws Exception {
185+
public void testDefaultPropertiesDoNothing() throws Exception {
186186
Map<String, String> props = Collections.singletonMap("default.setting", "foo");
187187
Environment env = InternalSettingsPreparer.prepareEnvironment(baseEnvSettings, null, props);
188-
assertEquals("foo", env.settings().get("setting"));
189-
}
190-
191-
public void testDefaultPropertiesOverride() throws Exception {
192-
Path configDir = homeDir.resolve("config");
193-
Files.createDirectories(configDir);
194-
Files.write(configDir.resolve("elasticsearch.yml"), Collections.singletonList("setting: bar"), StandardCharsets.UTF_8);
195-
Map<String, String> props = Collections.singletonMap("default.setting", "foo");
196-
Environment env = InternalSettingsPreparer.prepareEnvironment(baseEnvSettings, null, props);
197-
assertEquals("bar", env.settings().get("setting"));
198-
}
199-
200-
public void testDefaultWithArray() {
201-
final Settings.Builder output = Settings.builder().put("foobar.0", "bar").put("foobar.1", "baz");
202-
final Map<String, String> esSettings = Collections.singletonMap("default.foobar", "foo");
203-
InternalSettingsPreparer.initializeSettings(output, Settings.EMPTY, esSettings);
204-
final Settings settings = output.build();
205-
assertThat(settings.get("foobar.0"), equalTo("bar"));
206-
assertThat(settings.get("foobar.1"), equalTo("baz"));
207-
assertNull(settings.get("foobar"));
188+
assertEquals("foo", env.settings().get("default.setting"));
189+
assertNull(env.settings().get("setting"));
208190
}
209191

210192
}

‎docs/reference/setup/configuration.asciidoc

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -89,23 +89,6 @@ Enter value for [node.name]:
8989
NOTE: Elasticsearch will not start if `${prompt.text}` or `${prompt.secret}`
9090
is used in the settings and the process is run as a service or in the background.
9191

92-
[float]
93-
=== Setting default settings
94-
95-
New default settings may be specified on the command line using the
96-
`default.` prefix. This will specify a value that will be used by
97-
default unless another value is specified in the config file.
98-
99-
For instance, if Elasticsearch is started as follows:
100-
101-
[source,sh]
102-
---------------------------
103-
./bin/elasticsearch -Edefault.node.name=My_Node
104-
---------------------------
105-
106-
the value for `node.name` will be `My_Node`, unless it is overwritten on the
107-
command line with `es.node.name` or in the config file with `node.name`.
108-
10992
[float]
11093
[[logging]]
11194
== Logging configuration

‎docs/reference/setup/install/docker.asciidoc

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,6 @@ The image offers several methods for configuring Elasticsearch settings with the
227227
===== A. Present the parameters via Docker environment variables
228228
For example, to define the cluster name with `docker run` you can pass `-e "cluster.name=mynewclustername"`. Double quotes are required.
229229

230-
NOTE: There is a difference between defining <<_setting_default_settings,default settings>> and normal settings. The former are prefixed with `default.` and cannot override normal settings, if defined.
231-
232230
===== B. Bind-mounted configuration
233231
Create your custom config file and mount this over the image's corresponding file.
234232
For example, bind-mounting a `custom_elasticsearch.yml` with `docker run` can be accomplished with the parameter:

0 commit comments

Comments
 (0)