make prompt placeholders consistent with existing placeholders

In #10918, we introduced the prompt placeholders. These were had a different format
than our existing placeholders. This changes the prompt placeholders to follow the
format of the existing placeholders.

Relates to #11455

core/src/main/java/org/elasticsearch/common/property/PropertyPlaceholder.java

@@ -109,7 +109,11 @@ public class PropertyPlaceholder {
                     propVal = defaultValue;
                 }
                 if (propVal == null && placeholderResolver.shouldIgnoreMissing(placeholder)) {
-                    propVal = "";
+                    if (placeholderResolver.shouldRemoveMissingPlaceholder(placeholder)) {
+                        propVal = "";
+                    } else {
+                        return strVal;
+                    }
                 }
                 if (propVal != null) {
                     // Recursive invocation, parsing placeholders contained in the
@@ -170,5 +174,13 @@ public class PropertyPlaceholder {
         String resolvePlaceholder(String placeholderName);
 
         boolean shouldIgnoreMissing(String placeholderName);
+
+        /**
+         * Allows for special handling for ignored missing placeholders that may be resolved elsewhere
+         *
+         * @param placeholderName the name of the placeholder to resolve.
+         * @return true if the placeholder should be replaced with a empty string
+         */
+        boolean shouldRemoveMissingPlaceholder(String placeholderName);
     }
 }

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

@@ -1245,7 +1245,7 @@ public final class Settings implements ToXContent {
          * tries and resolve it against an environment variable ({@link System#getenv(String)}), and last, tries
          * and replace it with another setting already set on this builder.
          */
-        public Builder replacePropertyPlaceholders(String... ignoredValues) {
+        public Builder replacePropertyPlaceholders() {
             PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false);
             PropertyPlaceholder.PlaceholderResolver placeholderResolver = new PropertyPlaceholder.PlaceholderResolver() {
                     @Override
@@ -1268,26 +1268,22 @@ public final class Settings implements ToXContent {
                     @Override
                     public boolean shouldIgnoreMissing(String placeholderName) {
                         // if its an explicit env var, we are ok with not having a value for it and treat it as optional
-                        if (placeholderName.startsWith("env.")) {
+                        if (placeholderName.startsWith("env.") || placeholderName.startsWith("prompt.")) {
                             return true;
                         }
                         return false;
                     }
+
+                    @Override
+                    public boolean shouldRemoveMissingPlaceholder(String placeholderName) {
+                        if (placeholderName.startsWith("prompt.")) {
+                            return false;
+                        }
+                        return true;
+                    }
                 };
             for (Map.Entry<String, String> entry : Maps.newHashMap(map).entrySet()) {
-                String possiblePlaceholder = entry.getValue();
-                boolean ignored = false;
-                for (String ignoredValue : ignoredValues) {
-                    if (ignoredValue.equals(possiblePlaceholder)) {
-                        ignored = true;
-                        break;
-                    }
-                }
-                if (ignored) {
-                    continue;
-                }
-
-                String value = propertyPlaceholder.replacePlaceholders(possiblePlaceholder, placeholderResolver);
+                String value = propertyPlaceholder.replacePlaceholders(entry.getValue(), placeholderResolver);
                 // if the values exists and has length, we should maintain it  in the map
                 // otherwise, the replace process resolved into removing it
                 if (Strings.hasLength(value)) {

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

@@ -44,8 +44,8 @@ public class InternalSettingsPreparer {
 
     static final List<String> ALLOWED_SUFFIXES = ImmutableList.of(".yml", ".yaml", ".json", ".properties");
 
-    public static final String SECRET_PROMPT_VALUE = "${prompt::secret}";
-    public static final String TEXT_PROMPT_VALUE = "${prompt::text}";
+    public static final String SECRET_PROMPT_VALUE = "${prompt.secret}";
+    public static final String TEXT_PROMPT_VALUE = "${prompt.text}";
     public static final String IGNORE_SYSTEM_PROPERTIES_SETTING = "config.ignore_system_properties";
 
     /**
@@ -72,9 +72,6 @@ public class InternalSettingsPreparer {
     public static Tuple<Settings, Environment> prepareSettings(Settings pSettings, boolean loadConfigSettings, Terminal terminal) {
         // ignore this prefixes when getting properties from es. and elasticsearch.
         String[] ignorePrefixes = new String[]{"es.default.", "elasticsearch.default."};
-        // ignore the special prompt placeholders since they have the same format as property placeholders and will be resolved
-        // as having a default value because of the ':' in the format
-        String[] ignoredPlaceholders = new String[] { SECRET_PROMPT_VALUE, TEXT_PROMPT_VALUE };
         boolean useSystemProperties = !pSettings.getAsBoolean(IGNORE_SYSTEM_PROPERTIES_SETTING, false);
         // just create enough settings to build the environment
         Settings.Builder settingsBuilder = settingsBuilder().put(pSettings);
@@ -84,7 +81,7 @@ public class InternalSettingsPreparer {
                     .putProperties("elasticsearch.", System.getProperties(), ignorePrefixes)
                     .putProperties("es.", System.getProperties(), ignorePrefixes);
         }
-        settingsBuilder.replacePropertyPlaceholders(ignoredPlaceholders);
+        settingsBuilder.replacePropertyPlaceholders();
 
         Environment environment = new Environment(settingsBuilder.build());
 
@@ -122,7 +119,7 @@ public class InternalSettingsPreparer {
             settingsBuilder.putProperties("elasticsearch.", System.getProperties(), ignorePrefixes)
                     .putProperties("es.", System.getProperties(), ignorePrefixes);
         }
-        settingsBuilder.replacePropertyPlaceholders(ignoredPlaceholders);
+        settingsBuilder.replacePropertyPlaceholders();
 
         // allow to force set properties based on configuration of the settings provided
         for (Map.Entry<String, String> entry : pSettings.getAsMap().entrySet()) {
@@ -132,7 +129,7 @@ public class InternalSettingsPreparer {
                 settingsBuilder.put(setting.substring("force.".length()), entry.getValue());
             }
         }
-        settingsBuilder.replacePropertyPlaceholders(ignoredPlaceholders);
+        settingsBuilder.replacePropertyPlaceholders();
 
         // generate the name
         if (settingsBuilder.get("name") == null) {

core/src/test/java/org/elasticsearch/common/property/PropertyPlaceholderTest.java

@@ -33,7 +33,7 @@ public class PropertyPlaceholderTest extends ElasticsearchTestCase {
         Map<String, String> map = new LinkedHashMap<>();
         map.put("foo1", "bar1");
         map.put("foo2", "bar2");
-        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
+        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
         assertEquals("bar1", propertyPlaceholder.replacePlaceholders("{foo1}", placeholderResolver));
         assertEquals("a bar1b", propertyPlaceholder.replacePlaceholders("a {foo1}b", placeholderResolver));
         assertEquals("bar1bar2", propertyPlaceholder.replacePlaceholders("{foo1}{foo2}", placeholderResolver));
@@ -48,7 +48,7 @@ public class PropertyPlaceholderTest extends ElasticsearchTestCase {
         PropertyPlaceholder ppShorterPrefix = new PropertyPlaceholder("{", "}}", false);
         Map<String, String> map = new LinkedHashMap<>();
         map.put("foo", "bar");
-        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
+        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
         assertEquals("bar", ppEqualsPrefix.replacePlaceholders("{foo}", placeholderResolver));
         assertEquals("bar", ppLongerPrefix.replacePlaceholders("${foo}", placeholderResolver));
         assertEquals("bar", ppShorterPrefix.replacePlaceholders("{foo}}", placeholderResolver));
@@ -58,7 +58,7 @@ public class PropertyPlaceholderTest extends ElasticsearchTestCase {
     public void testDefaultValue() {
         PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false);
         Map<String, String> map = new LinkedHashMap<>();
-        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
+        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
         assertEquals("bar", propertyPlaceholder.replacePlaceholders("${foo:bar}", placeholderResolver));
         assertEquals("", propertyPlaceholder.replacePlaceholders("${foo:}", placeholderResolver));
     }
@@ -67,7 +67,7 @@ public class PropertyPlaceholderTest extends ElasticsearchTestCase {
     public void testIgnoredUnresolvedPlaceholder() {
         PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", true);
         Map<String, String> map = new LinkedHashMap<>();
-        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
+        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
         assertEquals("${foo}", propertyPlaceholder.replacePlaceholders("${foo}", placeholderResolver));
     }
 
@@ -75,7 +75,7 @@ public class PropertyPlaceholderTest extends ElasticsearchTestCase {
     public void testNotIgnoredUnresolvedPlaceholder() {
         PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false);
         Map<String, String> map = new LinkedHashMap<>();
-        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
+        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
         propertyPlaceholder.replacePlaceholders("${foo}", placeholderResolver);
     }
 
@@ -83,7 +83,7 @@ public class PropertyPlaceholderTest extends ElasticsearchTestCase {
     public void testShouldIgnoreMissing() {
         PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false);
         Map<String, String> map = new LinkedHashMap<>();
-        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, true);
+        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, true, true);
         assertEquals("bar", propertyPlaceholder.replacePlaceholders("bar${foo}", placeholderResolver));
     }
 
@@ -94,7 +94,7 @@ public class PropertyPlaceholderTest extends ElasticsearchTestCase {
         map.put("foo", "${foo1}");
         map.put("foo1", "${foo2}");
         map.put("foo2", "bar");
-        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
+        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
         assertEquals("bar", propertyPlaceholder.replacePlaceholders("${foo}", placeholderResolver));
         assertEquals("abarb", propertyPlaceholder.replacePlaceholders("a${foo}b", placeholderResolver));
     }
@@ -107,7 +107,7 @@ public class PropertyPlaceholderTest extends ElasticsearchTestCase {
         map.put("foo1", "${foo2}");
         map.put("foo2", "bar");
         map.put("barbar", "baz");
-        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
+        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
         assertEquals("baz", propertyPlaceholder.replacePlaceholders("${bar${foo}}", placeholderResolver));
     }
 
@@ -119,7 +119,7 @@ public class PropertyPlaceholderTest extends ElasticsearchTestCase {
         map.put("foo1", "{foo2}");
         map.put("foo2", "bar");
         map.put("barbar", "baz");
-        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
+        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
         assertEquals("baz", propertyPlaceholder.replacePlaceholders("{bar{foo}}", placeholderResolver));
     }
 
@@ -131,7 +131,7 @@ public class PropertyPlaceholderTest extends ElasticsearchTestCase {
         map.put("foo1", "{foo2}}");
         map.put("foo2", "bar");
         map.put("barbar", "baz");
-        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
+        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
         assertEquals("baz", propertyPlaceholder.replacePlaceholders("{bar{foo}}}}", placeholderResolver));
     }
 
@@ -141,17 +141,27 @@ public class PropertyPlaceholderTest extends ElasticsearchTestCase {
         Map<String, String> map = new LinkedHashMap<>();
         map.put("foo", "${bar}");
         map.put("bar", "${foo}");
-        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
+        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
         propertyPlaceholder.replacePlaceholders("${foo}", placeholderResolver);
     }
 
+    @Test
+    public void testShouldRemoveMissing() {
+        PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false);
+        Map<String, String> map = new LinkedHashMap<>();
+        PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, true, false);
+        assertEquals("bar${foo}", propertyPlaceholder.replacePlaceholders("bar${foo}", placeholderResolver));
+    }
+
     private class SimplePlaceholderResolver implements PropertyPlaceholder.PlaceholderResolver {
         private Map<String, String> map;
         private boolean shouldIgnoreMissing;
+        private boolean shouldRemoveMissing;
 
-        SimplePlaceholderResolver(Map<String, String> map, boolean shouldIgnoreMissing) {
+        SimplePlaceholderResolver(Map<String, String> map, boolean shouldIgnoreMissing, boolean shouldRemoveMissing) {
             this.map = map;
             this.shouldIgnoreMissing = shouldIgnoreMissing;
+            this.shouldRemoveMissing = shouldRemoveMissing;
         }
 
         @Override
@@ -163,5 +173,10 @@ public class PropertyPlaceholderTest extends ElasticsearchTestCase {
         public boolean shouldIgnoreMissing(String placeholderName) {
             return shouldIgnoreMissing;
         }
+
+        @Override
+        public boolean shouldRemoveMissingPlaceholder(String placeholderName) {
+            return shouldRemoveMissing;
+        }
     }
 }

core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java

@@ -138,14 +138,14 @@ public class SettingsTests extends ElasticsearchTestCase {
     }
 
     @Test
-    public void testReplacePropertiesPlaceholderIgnores() {
+    public void testReplacePropertiesPlaceholderIgnoresPrompt() {
         Settings settings = settingsBuilder()
-                .put("setting1", "${foo.bar}")
-                .put("setting2", "${foo.bar1}")
-                .replacePropertyPlaceholders("${foo.bar}", "${foo.bar1}")
+                .put("setting1", "${prompt.text}")
+                .put("setting2", "${prompt.secret}")
+                .replacePropertyPlaceholders()
                 .build();
-        assertThat(settings.get("setting1"), is("${foo.bar}"));
-        assertThat(settings.get("setting2"), is("${foo.bar1}"));
+        assertThat(settings.get("setting1"), is("${prompt.text}"));
+        assertThat(settings.get("setting2"), is("${prompt.secret}"));
     }
 
     @Test

docs/reference/setup/configuration.asciidoc

@@ -271,15 +271,15 @@ file which will resolve to an environment setting, for example:
 --------------------------------------------------
 
 Additionally, for settings that you do not wish to store in the configuration
-file, you can use the value `${prompt::text}` or `${prompt::secret}` and start
-Elasticsearch in the foreground. `${prompt::secret}` has echoing disabled so
-that the value entered will not be shown in your terminal; `${prompt::text}`
+file, you can use the value `${prompt.text}` or `${prompt.secret}` and start
+Elasticsearch in the foreground. `${prompt.secret}` has echoing disabled so
+that the value entered will not be shown in your terminal; `${prompt.text}`
 will allow you to see the value as you type it in. For example:
 
 [source,yaml]
 --------------------------------------------------
 node:
-  name: ${prompt::text}
+  name: ${prompt.text}
 --------------------------------------------------
 
 On execution of the `elasticsearch` command, you will be prompted to enter
@@ -290,7 +290,7 @@ the actual value like so:
 Enter value for [node.name]:
 --------------------------------------------------
 
-NOTE: Elasticsearch will not start if `${prompt::text}` or `${prompt::secret}`
+NOTE: Elasticsearch will not start if `${prompt.text}` or `${prompt.secret}`
 is used in the settings and the process is run as a service or in the background.
 
 The location of the configuration file can be set externally using a