diff --git a/docs/changelog/85186.yaml b/docs/changelog/85186.yaml new file mode 100644 index 0000000000000..33cfe1fc8c290 --- /dev/null +++ b/docs/changelog/85186.yaml @@ -0,0 +1,6 @@ +pr: 85186 +summary: Allow yaml values for dynamic node settings +area: Infra/Core +type: enhancement +issues: + - 65577 diff --git a/server/src/main/java/org/elasticsearch/common/settings/Settings.java b/server/src/main/java/org/elasticsearch/common/settings/Settings.java index aa847867d2f6f..b622f81f8fe64 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -440,7 +440,8 @@ public List getAsList(String key, List defaultValue, Boolean com final List valuesAsList = (List) valueFromPrefix; return valuesAsList; } else if (commaDelimited) { - String[] strings = Strings.splitStringByCommaToArray(get(key)); + String value = get(key); + String[] strings = Strings.splitStringByCommaToArray(value); if (strings.length > 0) { for (String string : strings) { result.add(string.trim()); @@ -1210,19 +1211,10 @@ public Builder putProperties(final Map esSettings, final Functio * another setting already set on this builder. */ public Builder replacePropertyPlaceholders() { - return replacePropertyPlaceholders(System::getenv); - } - - // visible for testing - Builder replacePropertyPlaceholders(Function getenv) { PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false); PropertyPlaceholder.PlaceholderResolver placeholderResolver = new PropertyPlaceholder.PlaceholderResolver() { @Override public String resolvePlaceholder(String placeholderName) { - final String value = getenv.apply(placeholderName); - if (value != null) { - return value; - } return Settings.toString(map.get(placeholderName)); } diff --git a/server/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java b/server/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java index 6534f33168115..58ae937255c21 100644 --- a/server/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java +++ b/server/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java @@ -15,11 +15,13 @@ import org.elasticsearch.core.SuppressForbidden; import org.elasticsearch.env.Environment; +import java.io.BufferedReader; +import java.io.ByteArrayInputStream; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.function.Function; @@ -27,14 +29,6 @@ public class InternalSettingsPreparer { - // TODO: refactor this method out, it used to exist for the transport client - public static Settings prepareSettings(Settings input) { - Settings.Builder output = Settings.builder(); - initializeSettings(output, input, Collections.emptyMap()); - finalizeSettings(output, () -> null); - return output.build(); - } - /** * Prepares the settings by gathering all elasticsearch system properties, optionally loading the configuration settings. * @@ -62,16 +56,19 @@ public static Environment prepareEnvironment( Settings.Builder output = Settings.builder(); // start with a fresh output Path path = configFile.resolve("elasticsearch.yml"); + if (Files.exists(path)) { try { - output.loadFromPath(path); + loadConfigWithSubstitutions(output, path, System::getenv); } catch (IOException e) { throw new SettingsException("Failed to load settings from " + path.toString(), e); } } + loadOverrides(output, properties); + // re-initialize settings now that the config file has been loaded - initializeSettings(output, input, properties); + initializeSettings(output, input); finalizeSettings(output, defaultNodeName); return new Environment(output.build(), configFile); @@ -105,14 +102,69 @@ private static Path resolveConfigDir(String esHome) { * * @param output the settings builder to apply the input and default settings to * @param input the input settings - * @param esSettings a map from which to apply settings */ - static void initializeSettings(final Settings.Builder output, final Settings input, final Map esSettings) { + static void initializeSettings(final Settings.Builder output, final Settings input) { output.put(input); - output.putProperties(esSettings, Function.identity()); output.replacePropertyPlaceholders(); } + static void loadConfigWithSubstitutions(Settings.Builder output, Path configFile, Function substitutions) + throws IOException { + long existingSize = Files.size(configFile); + StringBuilder builder = new StringBuilder((int) existingSize); + try (BufferedReader reader = Files.newBufferedReader(configFile, StandardCharsets.UTF_8)) { + String line; + while ((line = reader.readLine()) != null) { + int dollarNdx; + int nextNdx = 0; + while ((dollarNdx = line.indexOf("${", nextNdx)) != -1) { + int closeNdx = line.indexOf('}', dollarNdx + 2); + if (closeNdx == -1) { + // No close substitution was found. Break to leniently copy the rest of the line as is. + break; + } + // copy up to the dollar + if (dollarNdx > nextNdx) { + builder.append(line, nextNdx, dollarNdx); + } + nextNdx = closeNdx + 1; + + String substKey = line.substring(dollarNdx + 2, closeNdx); + String substValue = substitutions.apply(substKey); + if (substValue != null) { + builder.append(substValue); + } else { + // the substitution name doesn't exist, defer to setting based substitution after yaml parsing + builder.append(line, dollarNdx, nextNdx); + } + } + if (nextNdx < line.length()) { + builder.append(line, nextNdx, line.length()); + } + builder.append(System.lineSeparator()); + } + } + var is = new ByteArrayInputStream(builder.toString().getBytes(StandardCharsets.UTF_8)); + output.loadFromStream(configFile.getFileName().toString(), is, false); + } + + static void loadOverrides(Settings.Builder output, Map overrides) { + StringBuilder builder = new StringBuilder(); + for (var entry : overrides.entrySet()) { + builder.append(entry.getKey()); + builder.append(": "); + builder.append(entry.getValue()); + builder.append(System.lineSeparator()); + } + var is = new ByteArrayInputStream(builder.toString().getBytes(StandardCharsets.UTF_8)); + // fake the resource name so it loads yaml + try { + output.loadFromStream("overrides.yml", is, false); + } catch (IOException e) { + throw new SettingsException("Malformed setting override value", e); + } + } + /** * Finish preparing settings by replacing forced settings and any defaults that need to be added. */ diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java index b78b455a91aad..64b7153d2a4ff 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java @@ -59,16 +59,6 @@ public void testReplacePropertiesPlaceholderSystemProperty() { assertThat(settings.get("setting1"), equalTo(value)); } - public void testReplacePropertiesPlaceholderSystemPropertyList() { - final String hostname = randomAlphaOfLength(16); - final String hostip = randomAlphaOfLength(16); - final Settings settings = Settings.builder() - .putList("setting1", "${HOSTNAME}", "${HOSTIP}") - .replacePropertyPlaceholders(name -> name.equals("HOSTNAME") ? hostname : name.equals("HOSTIP") ? hostip : null) - .build(); - assertThat(settings.getAsList("setting1"), contains(hostname, hostip)); - } - public void testReplacePropertiesPlaceholderSystemVariablesHaveNoEffect() { final String value = System.getProperty("java.home"); assertNotNull(value); @@ -79,15 +69,6 @@ public void testReplacePropertiesPlaceholderSystemVariablesHaveNoEffect() { assertThat(e, hasToString(containsString("Could not resolve placeholder 'java.home'"))); } - public void testReplacePropertiesPlaceholderByEnvironmentVariables() { - final String hostname = randomAlphaOfLength(16); - final Settings implicitEnvSettings = Settings.builder() - .put("setting1", "${HOSTNAME}") - .replacePropertyPlaceholders(name -> "HOSTNAME".equals(name) ? hostname : null) - .build(); - assertThat(implicitEnvSettings.get("setting1"), equalTo(hostname)); - } - public void testGetAsSettings() { Settings settings = Settings.builder() .put("bar", "hello world") diff --git a/server/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java b/server/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java index 3a7a0510e4fef..a5772f5fb7dee 100644 --- a/server/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java +++ b/server/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java @@ -29,6 +29,7 @@ import java.util.function.Supplier; import static java.util.Collections.emptyMap; +import static org.hamcrest.Matchers.equalTo; public class InternalSettingsPreparerTests extends ESTestCase { private static final Supplier DEFAULT_NODE_NAME_SHOULDNT_BE_CALLED = () -> { throw new AssertionError("shouldn't be called"); }; @@ -134,4 +135,87 @@ public void testDefaultPropertiesDoNothing() throws Exception { assertNull(env.settings().get("setting")); } + private Path copyConfig(String resourceName) throws IOException { + InputStream yaml = getClass().getResourceAsStream(resourceName); + Path configDir = homeDir.resolve("config"); + Files.createDirectory(configDir); + Path configFile = configDir.resolve("elasticsearch.yaml"); + Files.copy(yaml, configFile); + return configFile; + } + + private Settings loadConfigWithSubstitutions(Path configFile, Map env) throws IOException { + Settings.Builder output = Settings.builder(); + InternalSettingsPreparer.loadConfigWithSubstitutions(output, configFile, env::get); + return output.build(); + } + + public void testSubstitutionEntireLine() throws Exception { + Path config = copyConfig("subst-entire-line.yml"); + Settings settings = loadConfigWithSubstitutions(config, Map.of("mysubst", "foo: bar")); + assertThat(settings.get("foo"), equalTo("bar")); + } + + public void testSubstitutionFirstLine() throws Exception { + Path config = copyConfig("subst-first-line.yml"); + Settings settings = loadConfigWithSubstitutions(config, Map.of("mysubst", "v1")); + assertThat(settings.get("foo"), equalTo("v1")); + assertThat(settings.get("bar"), equalTo("v2")); + assertThat(settings.get("baz"), equalTo("v3")); + } + + public void testSubstitutionLastLine() throws Exception { + Path config = copyConfig("subst-last-line.yml"); + Settings settings = loadConfigWithSubstitutions(config, Map.of("mysubst", "kazaam")); + assertThat(settings.get("foo.bar.baz"), equalTo("kazaam")); + } + + public void testSubstitutionMultiple() throws Exception { + Path config = copyConfig("subst-multiple.yml"); + Settings settings = loadConfigWithSubstitutions(config, Map.of("s1", "substituted", "s2", "line")); + assertThat(settings.get("foo"), equalTo("substituted line value")); + } + + public void testSubstitutionMissingLenient() throws Exception { + Path config = copyConfig("subst-missing.yml"); + Settings settings = loadConfigWithSubstitutions(config, Map.of()); + assertThat(settings.get("foo"), equalTo("${dne}")); + } + + public void testSubstitutionBrokenLenient() throws Exception { + Path config = copyConfig("subst-broken.yml"); + Settings settings = loadConfigWithSubstitutions(config, Map.of("goodsubst", "replaced")); + assertThat(settings.get("foo"), equalTo("${no closing brace")); + assertThat(settings.get("bar"), equalTo("replaced")); + } + + public void testOverridesOverride() throws Exception { + Settings.Builder output = Settings.builder().put("foo", "bar"); + InternalSettingsPreparer.loadOverrides(output, Map.of("foo", "baz")); + Settings settings = output.build(); + assertThat(settings.get("foo"), equalTo("baz")); + } + + public void testOverridesEmpty() throws Exception { + Settings.Builder output = Settings.builder().put("foo", "bar"); + InternalSettingsPreparer.loadOverrides(output, Map.of()); + Settings settings = output.build(); + assertThat(settings.get("foo"), equalTo("bar")); + } + + public void testOverridesNew() throws Exception { + Settings.Builder output = Settings.builder().put("foo", "bar"); + InternalSettingsPreparer.loadOverrides(output, Map.of("baz", "wat")); + Settings settings = output.build(); + assertThat(settings.get("foo"), equalTo("bar")); + assertThat(settings.get("baz"), equalTo("wat")); + } + + public void testOverridesMultiple() throws Exception { + Settings.Builder output = Settings.builder().put("foo1", "bar").put("foo2", "baz"); + InternalSettingsPreparer.loadOverrides(output, Map.of("foo1", "wat", "foo2", "yas")); + Settings settings = output.build(); + assertThat(settings.get("foo1"), equalTo("wat")); + assertThat(settings.get("foo2"), equalTo("yas")); + } } diff --git a/server/src/test/resources/org/elasticsearch/node/subst-broken.yml b/server/src/test/resources/org/elasticsearch/node/subst-broken.yml new file mode 100644 index 0000000000000..31fc3e08eb42a --- /dev/null +++ b/server/src/test/resources/org/elasticsearch/node/subst-broken.yml @@ -0,0 +1,2 @@ +foo: ${no closing brace +bar: ${goodsubst} diff --git a/server/src/test/resources/org/elasticsearch/node/subst-entire-line.yml b/server/src/test/resources/org/elasticsearch/node/subst-entire-line.yml new file mode 100644 index 0000000000000..2d1f28eea6c99 --- /dev/null +++ b/server/src/test/resources/org/elasticsearch/node/subst-entire-line.yml @@ -0,0 +1 @@ +${mysubst} diff --git a/server/src/test/resources/org/elasticsearch/node/subst-first-line.yml b/server/src/test/resources/org/elasticsearch/node/subst-first-line.yml new file mode 100644 index 0000000000000..4f2ab1c7d029b --- /dev/null +++ b/server/src/test/resources/org/elasticsearch/node/subst-first-line.yml @@ -0,0 +1,3 @@ +foo: ${mysubst} +bar: v2 +baz: v3 diff --git a/server/src/test/resources/org/elasticsearch/node/subst-last-line.yml b/server/src/test/resources/org/elasticsearch/node/subst-last-line.yml new file mode 100644 index 0000000000000..bb07d9e9425cd --- /dev/null +++ b/server/src/test/resources/org/elasticsearch/node/subst-last-line.yml @@ -0,0 +1,3 @@ +foo: + bar: + baz: ${mysubst} diff --git a/server/src/test/resources/org/elasticsearch/node/subst-missing.yml b/server/src/test/resources/org/elasticsearch/node/subst-missing.yml new file mode 100644 index 0000000000000..913f7853147ce --- /dev/null +++ b/server/src/test/resources/org/elasticsearch/node/subst-missing.yml @@ -0,0 +1 @@ +foo: ${dne} diff --git a/server/src/test/resources/org/elasticsearch/node/subst-multiple.yml b/server/src/test/resources/org/elasticsearch/node/subst-multiple.yml new file mode 100644 index 0000000000000..b5e020247fa63 --- /dev/null +++ b/server/src/test/resources/org/elasticsearch/node/subst-multiple.yml @@ -0,0 +1 @@ +foo: ${s1} ${s2} value