Settings: Remove environment from transport client

Transport clients run embedded within external applications, so
elasticsearch should not be doing anything with the filesystem, as there
is not elasticsearch home.

This change makes a number of cleanups to the internal API for loading
settings and creating an environment. The loadFromConfig option was
removed, since it was always true except for tests. We now always
attempt to load settings from config a file when an environment is
created. The prepare methods were also simplified so there is now
prepareSettingsAndEnvironment which nodes use, and prepareSettings which
the transport client uses. I also attempted to improve the tests, but
there is a still a lot of follow up work to do there.

closes #13155

core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java

@@ -197,7 +197,7 @@ final class Bootstrap {
 
     private static Tuple<Settings, Environment> initialSettings(boolean foreground) {
         Terminal terminal = foreground ? Terminal.DEFAULT : null;
-        return InternalSettingsPreparer.prepareSettings(EMPTY_SETTINGS, true, terminal);
+        return InternalSettingsPreparer.prepareSettingsAndEnvironment(EMPTY_SETTINGS, terminal);
     }
 
     private void start() {

core/src/main/java/org/elasticsearch/client/transport/TransportClient.java

@@ -85,7 +85,6 @@ public class TransportClient extends AbstractClient {
 
         private Settings settings = Settings.EMPTY;
         private List<Class<? extends Plugin>> pluginClasses = new ArrayList<>();
-        private boolean loadConfigSettings = true;
 
         /**
          * The settings to configure the transport client with.
@@ -102,15 +101,6 @@ public class TransportClient extends AbstractClient {
             return this;
         }
 
-        /**
-         * Should the transport client load file based configuration automatically or not (and rely
-         * only on the provided settings), defaults to true.
-         */
-        public Builder loadConfigSettings(boolean loadConfigSettings) {
-            this.loadConfigSettings = loadConfigSettings;
-            return this;
-        }
-
         /**
          * Add the given plugin to the client when it is created.
          */
@@ -123,17 +113,16 @@ public class TransportClient extends AbstractClient {
          * Builds a new instance of the transport client.
          */
         public TransportClient build() {
-            Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettings(settings, loadConfigSettings);
-            Settings settings = settingsBuilder()
+            Settings settings = InternalSettingsPreparer.prepareSettings(this.settings);
+            settings = settingsBuilder()
                     .put(NettyTransport.PING_SCHEDULE, "5s") // enable by default the transport schedule ping interval
-                    .put(tuple.v1())
+                    .put(settings)
                     .put("network.server", false)
                     .put("node.client", true)
                     .put(CLIENT_TYPE_SETTING, CLIENT_TYPE)
                     .build();
-            Environment environment = tuple.v2();
 
-            PluginsService pluginsService = new PluginsService(settings, tuple.v2(), pluginClasses);
+            PluginsService pluginsService = new PluginsService(settings, null, pluginClasses);
             this.settings = pluginsService.updatedSettings();
 
             Version version = Version.CURRENT;
@@ -149,7 +138,6 @@ public class TransportClient extends AbstractClient {
                     modules.add(pluginModule);
                 }
                 modules.add(new PluginsModule(pluginsService));
-                modules.add(new EnvironmentModule(environment));
                 modules.add(new SettingsModule(this.settings));
                 modules.add(new NetworkModule());
                 modules.add(new ClusterNameModule(this.settings));

core/src/main/java/org/elasticsearch/common/cli/CliTool.java

@@ -104,7 +104,7 @@ public abstract class CliTool {
         Preconditions.checkArgument(config.cmds().size() != 0, "At least one command must be configured");
         this.config = config;
         this.terminal = terminal;
-        Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettings(EMPTY_SETTINGS, true, terminal);
+        Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettingsAndEnvironment(EMPTY_SETTINGS, terminal);
         settings = tuple.v1();
         env = tuple.v2();
     }

core/src/main/java/org/elasticsearch/node/Node.java

@@ -133,7 +133,7 @@ public class Node implements Releasable {
     Node(Settings preparedSettings, boolean loadConfigSettings, Version version, Collection<Class<? extends Plugin>> classpathPlugins) {
         final Settings pSettings = settingsBuilder().put(preparedSettings)
                 .put(Client.CLIENT_TYPE_SETTING, CLIENT_TYPE).build();
-        Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettings(pSettings, loadConfigSettings);
+        Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettingsAndEnvironment(pSettings, null);
         tuple = new Tuple<>(TribeService.processSettings(tuple.v1()), tuple.v2());
 
         ESLogger logger = Loggers.getLogger(Node.class, tuple.v1().get("name"));
@@ -147,7 +147,7 @@ public class Node implements Releasable {
                     env.configFile(), Arrays.toString(env.dataFiles()), env.logsFile(), env.pluginsFile());
         }
 
-        this.pluginsService = new PluginsService(tuple.v1(), tuple.v2(), classpathPlugins);
+        this.pluginsService = new PluginsService(tuple.v1(), tuple.v2().pluginsFile(), classpathPlugins);
         this.settings = pluginsService.updatedSettings();
         // create the environment based on the finalized (processed) view of the settings
         this.environment = new Environment(this.settings());

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

@@ -39,6 +39,7 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -52,22 +53,22 @@ import static org.elasticsearch.common.settings.Settings.settingsBuilder;
  */
 public class InternalSettingsPreparer {
 
-    static final List<String> ALLOWED_SUFFIXES = Arrays.asList(".yml", ".yaml", ".json", ".properties");
+    static final String[] ALLOWED_SUFFIXES = {".yml", ".yaml", ".json", ".properties"};
+    static final String[] PROPERTY_PREFIXES = {"es.", "elasticsearch."};
+    static final String[] PROPERTY_DEFAULTS_PREFIXES = {"es.default.", "elasticsearch.default."};
 
     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";
 
     /**
-     * Prepares the settings by gathering all elasticsearch system properties, optionally loading the configuration settings,
-     * and then replacing all property placeholders. This method will not work with settings that have <code>${prompt.text}</code>
-     * or <code>${prompt.secret}</code> as their value unless they have been resolved previously.
-     * @param pSettings The initial settings to use
-     * @param loadConfigSettings flag to indicate whether to load settings from the configuration directory/file
-     * @return the {@link Settings} and {@link Environment} as a {@link Tuple}
+     * Prepares the settings by gathering all elasticsearch system properties and setting defaults.
      */
-    public static Tuple<Settings, Environment> prepareSettings(Settings pSettings, boolean loadConfigSettings) {
-        return prepareSettings(pSettings, loadConfigSettings, null);
+    public static Settings prepareSettings(Settings input) {
+        Settings.Builder output = settingsBuilder();
+        initializeSettings(output, input, true);
+        finalizeSettings(output, null, null);
+        return output.build();
     }
 
     /**
@@ -75,126 +76,143 @@ public class InternalSettingsPreparer {
      * and then replacing all property placeholders. If a {@link Terminal} is provided and configuration settings are loaded,
      * settings with a value of <code>${prompt.text}</code> or <code>${prompt.secret}</code> will result in a prompt for
      * the setting to the user.
-     * @param pSettings The initial settings to use
-     * @param loadConfigSettings flag to indicate whether to load settings from the configuration directory/file
+     * @param input The initial settings to use
      * @param terminal the Terminal to use for input/output
      * @return the {@link Settings} and {@link Environment} as a {@link Tuple}
      */
-    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."};
-        boolean useSystemProperties = !pSettings.getAsBoolean(IGNORE_SYSTEM_PROPERTIES_SETTING, false);
+    public static Tuple<Settings, Environment> prepareSettingsAndEnvironment(Settings input, Terminal terminal) {
         // just create enough settings to build the environment
-        Settings.Builder settingsBuilder = settingsBuilder().put(pSettings);
-        if (useSystemProperties) {
-            settingsBuilder.putProperties("elasticsearch.default.", System.getProperties())
-                    .putProperties("es.default.", System.getProperties())
-                    .putProperties("elasticsearch.", System.getProperties(), ignorePrefixes)
-                    .putProperties("es.", System.getProperties(), ignorePrefixes);
-        }
-        settingsBuilder.replacePropertyPlaceholders();
+        Settings.Builder output = settingsBuilder();
+        initializeSettings(output, input, true);
+        Environment environment = new Environment(output.build());
 
-        Environment environment = new Environment(settingsBuilder.build());
-
-        if (loadConfigSettings) {
-            boolean loadFromEnv = true;
-            if (useSystemProperties) {
-                // if its default, then load it, but also load form env
-                if (Strings.hasText(System.getProperty("es.default.config"))) {
-                    loadFromEnv = true;
-                    settingsBuilder.loadFromPath(environment.configFile().resolve(System.getProperty("es.default.config")));
-                }
-                // if explicit, just load it and don't load from env
-                if (Strings.hasText(System.getProperty("es.config"))) {
-                    loadFromEnv = false;
-                    settingsBuilder.loadFromPath(environment.configFile().resolve(System.getProperty("es.config")));
-                }
-                if (Strings.hasText(System.getProperty("elasticsearch.config"))) {
-                    loadFromEnv = false;
-                    settingsBuilder.loadFromPath(environment.configFile().resolve(System.getProperty("elasticsearch.config")));
-                }
+        // TODO: can we simplify all of this and have a single filename, which is looked up in the config dir?
+        boolean loadFromEnv = true;
+        if (useSystemProperties(input)) {
+            // if its default, then load it, but also load form env
+            if (Strings.hasText(System.getProperty("es.default.config"))) {
+                // TODO: we don't allow multiple config files, but having loadFromEnv true here allows just that
+                loadFromEnv = true;
+                output.loadFromPath(environment.configFile().resolve(System.getProperty("es.default.config")));
+            }
+            // TODO: these should be elseifs so that multiple files cannot be loaded
+            // if explicit, just load it and don't load from env
+            if (Strings.hasText(System.getProperty("es.config"))) {
+                loadFromEnv = false;
+                output.loadFromPath(environment.configFile().resolve(System.getProperty("es.config")));
+            }
+            if (Strings.hasText(System.getProperty("elasticsearch.config"))) {
+                loadFromEnv = false;
+                output.loadFromPath(environment.configFile().resolve(System.getProperty("elasticsearch.config")));
             }
-            if (loadFromEnv) {
-                boolean settingsFileFound = false;
-                Set<String> foundSuffixes = Sets.newHashSet();
-                for (String allowedSuffix : ALLOWED_SUFFIXES) {
-                    Path path = environment.configFile().resolve("elasticsearch" + allowedSuffix);
-                    if (Files.exists(path)) {
-                        if (!settingsFileFound) {
-                            settingsBuilder.loadFromPath(path);
-                        }
-                        settingsFileFound = true;
-                        foundSuffixes.add(allowedSuffix);
+        }
+        if (loadFromEnv) {
+            boolean settingsFileFound = false;
+            Set<String> foundSuffixes = Sets.newHashSet();
+            for (String allowedSuffix : ALLOWED_SUFFIXES) {
+                Path path = environment.configFile().resolve("elasticsearch" + allowedSuffix);
+                if (Files.exists(path)) {
+                    if (!settingsFileFound) {
+                        output.loadFromPath(path);
                     }
-                }
-                if (foundSuffixes.size() > 1) {
-                    throw new SettingsException("multiple settings files found with suffixes: " + Strings.collectionToDelimitedString(foundSuffixes, ","));
+                    settingsFileFound = true;
+                    foundSuffixes.add(allowedSuffix);
                 }
             }
+            if (foundSuffixes.size() > 1) {
+                throw new SettingsException("multiple settings files found with suffixes: " + Strings.collectionToDelimitedString(foundSuffixes, ","));
+            }
         }
 
-        settingsBuilder.put(pSettings);
-        if (useSystemProperties) {
-            settingsBuilder.putProperties("elasticsearch.", System.getProperties(), ignorePrefixes)
-                    .putProperties("es.", System.getProperties(), ignorePrefixes);
+        // re-initialize settings now that the config file has been loaded
+        // TODO: only re-initialize if a config file was actually loaded
+        initializeSettings(output, input, false);
+        finalizeSettings(output, terminal, environment.configFile());
+
+        environment = new Environment(output.build());
+
+        // we put back the path.logs so we can use it in the logging configuration file
+        output.put("path.logs", cleanPath(environment.logsFile().toAbsolutePath().toString()));
+
+        return new Tuple<>(output.build(), environment);
+    }
+
+    private static boolean useSystemProperties(Settings input) {
+        return !input.getAsBoolean(IGNORE_SYSTEM_PROPERTIES_SETTING, false);
+    }
+
+    /**
+     * Initializes the builder with the given input settings, and loads system properties settings if allowed.
+     * If loadDefaults is true, system property default settings are loaded.
+     */
+    private static void initializeSettings(Settings.Builder output, Settings input, boolean loadDefaults) {
+        output.put(input);
+        if (useSystemProperties(input)) {
+            if (loadDefaults) {
+                for (String prefix : PROPERTY_DEFAULTS_PREFIXES) {
+                    output.putProperties(prefix, System.getProperties());
+                }
+            }
+            for (String prefix : PROPERTY_PREFIXES) {
+                output.putProperties(prefix, System.getProperties(), PROPERTY_DEFAULTS_PREFIXES);
+            }
         }
-        settingsBuilder.replacePropertyPlaceholders();
+        output.replacePropertyPlaceholders();
+    }
 
+    /**
+     * Finish preparing settings by replacing forced settings, prompts, and any defaults that need to be added.
+     * The provided terminal is used to prompt for settings needing to be replaced.
+     * The provided configDir is optional and will be used to lookup names.txt if the node name is not set, if provided.
+     */
+    private static void finalizeSettings(Settings.Builder output, Terminal terminal, Path configDir) {
         // allow to force set properties based on configuration of the settings provided
-        for (Map.Entry<String, String> entry : pSettings.getAsMap().entrySet()) {
-            String setting = entry.getKey();
+        List<String> forcedSettings = new ArrayList<>();
+        for (String setting : output.internalMap().keySet()) {
             if (setting.startsWith("force.")) {
-                settingsBuilder.remove(setting);
-                settingsBuilder.put(setting.substring("force.".length()), entry.getValue());
+                forcedSettings.add(setting);
             }
         }
-        settingsBuilder.replacePropertyPlaceholders();
+        for (String forcedSetting : forcedSettings) {
+            String value = output.remove(forcedSetting);
+            output.put(forcedSetting.substring("force.".length()), value);
+        }
+        output.replacePropertyPlaceholders();
 
         // check if name is set in settings, if not look for system property and set it
-        if (settingsBuilder.get("name") == null) {
+        if (output.get("name") == null) {
             String name = System.getProperty("name");
             if (name != null) {
-                settingsBuilder.put("name", name);
+                output.put("name", name);
             }
         }
 
         // put the cluster name
-        if (settingsBuilder.get(ClusterName.SETTING) == null) {
-            settingsBuilder.put(ClusterName.SETTING, ClusterName.DEFAULT.value());
+        if (output.get(ClusterName.SETTING) == null) {
+            output.put(ClusterName.SETTING, ClusterName.DEFAULT.value());
         }
 
-        String v = settingsBuilder.get(Settings.SETTINGS_REQUIRE_UNITS);
+        String v = output.get(Settings.SETTINGS_REQUIRE_UNITS);
         if (v != null) {
             Settings.setSettingsRequireUnits(Booleans.parseBoolean(v, true));
         }
 
-        Settings settings = replacePromptPlaceholders(settingsBuilder.build(), terminal);
+        replacePromptPlaceholders(output, terminal);
         // all settings placeholders have been resolved. resolve the value for the name setting by checking for name,
         // then looking for node.name, and finally generate one if needed
-        if (settings.get("name") == null) {
-            String name = settings.get("node.name");
+        if (output.get("name") == null) {
+            String name = output.get("node.name");
             if (name == null || name.isEmpty()) {
-                name = randomNodeName(environment);
+                name = randomNodeName(configDir);
             }
-            settings = settingsBuilder().put(settings).put("name", name).build();
+            output.put("name", name);
         }
-
-        environment = new Environment(settings);
-
-        // put back the env settings
-        settingsBuilder = settingsBuilder().put(settings);
-        // we put back the path.logs so we can use it in the logging configuration file
-        settingsBuilder.put("path.logs", cleanPath(environment.logsFile().toAbsolutePath().toString()));
-
-        settings = settingsBuilder.build();
-
-        return new Tuple<>(settings, environment);
     }
 
-    static String randomNodeName(Environment environment) {
+    private static String randomNodeName(Path configDir) {
         InputStream input;
-        Path namesPath = environment.configFile().resolve("names.txt");
-        if (Files.exists(namesPath)) {
+        if (configDir != null && Files.exists(configDir.resolve("names.txt"))) {
+            Path namesPath = configDir.resolve("names.txt");
             try {
                 input = Files.newInputStream(namesPath);
             } catch (IOException e) {
@@ -220,37 +238,40 @@ public class InternalSettingsPreparer {
         }
     }
 
-    static Settings replacePromptPlaceholders(Settings settings, Terminal terminal) {
-        UnmodifiableIterator<Map.Entry<String, String>> iter = settings.getAsMap().entrySet().iterator();
-        Settings.Builder builder = Settings.builder();
-
-        while (iter.hasNext()) {
-            Map.Entry<String, String> entry = iter.next();
-            String value = entry.getValue();
-            String key = entry.getKey();
-            switch (value) {
+    private static void replacePromptPlaceholders(Settings.Builder settings, Terminal terminal) {
+        List<String> secretToPrompt = new ArrayList<>();
+        List<String> textToPrompt = new ArrayList<>();
+        for (Map.Entry<String, String> entry : settings.internalMap().entrySet()) {
+            switch (entry.getValue()) {
                 case SECRET_PROMPT_VALUE:
-                    String secretValue = promptForValue(key, terminal, true);
-                    if (Strings.hasLength(secretValue)) {
-                        builder.put(key, secretValue);
-                    }
+                    secretToPrompt.add(entry.getKey());
                     break;
                 case TEXT_PROMPT_VALUE:
-                    String textValue = promptForValue(key, terminal, false);
-                    if (Strings.hasLength(textValue)) {
-                        builder.put(key, textValue);
-                    }
-                    break;
-                default:
-                    builder.put(key, value);
+                    textToPrompt.add(entry.getKey());
                     break;
             }
         }
-
-        return builder.build();
+        for (String setting : secretToPrompt) {
+            String secretValue = promptForValue(setting, terminal, true);
+            if (Strings.hasLength(secretValue)) {
+                settings.put(setting, secretValue);
+            } else {
+                // TODO: why do we remove settings if prompt returns empty??
+                settings.remove(setting);
+            }
+        }
+        for (String setting : textToPrompt) {
+            String textValue = promptForValue(setting, terminal, false);
+            if (Strings.hasLength(textValue)) {
+                settings.put(setting, textValue);
+            } else {
+                // TODO: why do we remove settings if prompt returns empty??
+                settings.remove(setting);
+            }
+        }
     }
 
-    static String promptForValue(String key, Terminal terminal, boolean secret) {
+    private static String promptForValue(String key, Terminal terminal, boolean secret) {
         if (terminal == null) {
             throw new UnsupportedOperationException("found property [" + key + "] with value [" + (secret ? SECRET_PROMPT_VALUE : TEXT_PROMPT_VALUE) +"]. prompting for property values is only supported when running elasticsearch in the foreground");
         }

core/src/main/java/org/elasticsearch/plugins/PluginManager.java

@@ -321,7 +321,7 @@ public class PluginManager {
         }
 
         // read existing bundles. this does some checks on the installation too.
-        List<Bundle> bundles = PluginsService.getPluginBundles(environment);
+        List<Bundle> bundles = PluginsService.getPluginBundles(environment.pluginsFile());
 
         // if we aren't isolated, we need to jarhellcheck against any other non-isolated plugins
         // thats always the first bundle

core/src/main/java/org/elasticsearch/plugins/PluginManagerCliParser.java

@@ -50,7 +50,7 @@ public class PluginManagerCliParser extends CliTool {
             .build();
 
     public static void main(String[] args) {
-        Tuple<Settings, Environment> initialSettings = InternalSettingsPreparer.prepareSettings(EMPTY, true, Terminal.DEFAULT);
+        Tuple<Settings, Environment> initialSettings = InternalSettingsPreparer.prepareSettingsAndEnvironment(EMPTY, Terminal.DEFAULT);
         LogConfigurator.configure(initialSettings.v1());
         int status = new PluginManagerCliParser().execute(args).status();
         System.exit(status);

core/src/main/java/org/elasticsearch/plugins/PluginsService.java

@@ -87,10 +87,10 @@ public class PluginsService extends AbstractComponent {
     /**
      * Constructs a new PluginService
      * @param settings The settings of the system
-     * @param environment The environment of the system
+     * @param pluginsDirectory The directory plugins exist in, or null if plugins should not be loaded from the filesystem
      * @param classpathPlugins Plugins that exist in the classpath which should be loaded
      */
-    public PluginsService(Settings settings, Environment environment, Collection<Class<? extends Plugin>> classpathPlugins) {
+    public PluginsService(Settings settings, Path pluginsDirectory, Collection<Class<? extends Plugin>> classpathPlugins) {
         super(settings);
 
         List<Tuple<PluginInfo, Plugin>> tupleBuilder = new ArrayList<>();
@@ -106,11 +106,13 @@ public class PluginsService extends AbstractComponent {
         }
 
         // now, find all the ones that are in plugins/
-        try {
-          List<Bundle> bundles = getPluginBundles(environment);
-          tupleBuilder.addAll(loadBundles(bundles));
-        } catch (IOException ex) {
-          throw new IllegalStateException("Unable to initialize plugins", ex);
+        if (pluginsDirectory != null) {
+            try {
+                List<Bundle> bundles = getPluginBundles(pluginsDirectory);
+                tupleBuilder.addAll(loadBundles(bundles));
+            } catch (IOException ex) {
+                throw new IllegalStateException("Unable to initialize plugins", ex);
+            }
         }
 
         plugins = Collections.unmodifiableList(tupleBuilder);
@@ -281,10 +283,9 @@ public class PluginsService extends AbstractComponent {
         List<URL> urls = new ArrayList<>();
     }
 
-    static List<Bundle> getPluginBundles(Environment environment) throws IOException {
+    static List<Bundle> getPluginBundles(Path pluginsDirectory) throws IOException {
         ESLogger logger = Loggers.getLogger(PluginsService.class);
 
-        Path pluginsDirectory = environment.pluginsFile();
         // TODO: remove this leniency, but tests bogusly rely on it
         if (!isAccessibleDirectory(pluginsDirectory, logger)) {
             return Collections.emptyList();

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

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.node.internal;
 
+import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.common.cli.CliToolTestCase;
 import org.elasticsearch.common.cli.Terminal;
 import org.elasticsearch.common.collect.Tuple;
@@ -37,49 +38,107 @@ import java.io.InputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.elasticsearch.common.settings.Settings.settingsBuilder;
 import static org.hamcrest.Matchers.*;
 
 public class InternalSettingsPreparerTests extends ESTestCase {
-    @Rule
-    public ExpectedException expectedException = ExpectedException.none();
+
+    Map<String, String> savedProperties = new HashMap<>();
+    Settings baseEnvSettings;
 
     @Before
-    public void setupSystemProperties() {
-        System.setProperty("es.node.zone", "foo");
-        System.setProperty("name", "sys-prop-name");
+    public void saveSettingsSystemProperties() {
+        // clear out any properties the settings preparer may look for
+        savedProperties.clear();
+        for (Object propObj : System.getProperties().keySet()) {
+            String property = (String)propObj;
+            // NOTE: these prefixes are prefixes of the defaults, so both are handled here
+            for (String prefix : InternalSettingsPreparer.PROPERTY_PREFIXES) {
+                if (property.startsWith(prefix)) {
+                    savedProperties.put(property, System.getProperty(property));
+                }
+            }
+        }
+        String name = System.getProperty("name");
+        if (name != null) {
+            savedProperties.put("name", name);
+        }
+        for (String property : savedProperties.keySet()) {
+            System.clearProperty(property);
+        }
     }
 
     @After
-    public void cleanupSystemProperties() {
-        System.clearProperty("es.node.zone");
-        System.clearProperty("name");
+    public void restoreSettingsSystemProperties() {
+        for (Map.Entry<String, String> property : savedProperties.entrySet()) {
+            System.setProperty(property.getKey(), property.getValue());
+        }
+    }
+
+    @Before
+    public void createBaseEnvSettings() {
+        baseEnvSettings = settingsBuilder()
+            .put("path.home", createTempDir())
+            .build();
+    }
+
+    @After
+    public void clearBaseEnvSettings() {
+        baseEnvSettings = null;
+    }
+
+    public void testEmptySettings() {
+        Settings settings = InternalSettingsPreparer.prepareSettings(Settings.EMPTY);
+        assertNotNull(settings.get("name")); // a name was set
+        assertNotNull(settings.get(ClusterName.SETTING)); // a cluster name was set
+        assertEquals(settings.toString(), 2, settings.names().size());
+
+        Tuple<Settings, Environment> settingsAndEnv = InternalSettingsPreparer.prepareSettingsAndEnvironment(baseEnvSettings, null);
+        settings = settingsAndEnv.v1();
+        assertNotNull(settings.get("name")); // a name was set
+        assertNotNull(settings.get(ClusterName.SETTING)); // a cluster name was set
+        assertEquals(settings.toString(), 3 /* path.home is in the base settings */, settings.names().size());
+        String home = baseEnvSettings.get("path.home");
+        String configDir = settingsAndEnv.v2().configFile().toString();
+        assertTrue(configDir, configDir.startsWith(home));
+    }
+
+    public void testClusterNameDefault() {
+        Settings settings = InternalSettingsPreparer.prepareSettings(Settings.EMPTY);
+        assertEquals(ClusterName.DEFAULT.value(), settings.get(ClusterName.SETTING));
+        settings = InternalSettingsPreparer.prepareSettingsAndEnvironment(baseEnvSettings, null).v1();
+        assertEquals(ClusterName.DEFAULT.value(), settings.get(ClusterName.SETTING));
     }
 
-    @Test
     public void testIgnoreSystemProperties() {
-        Settings settings = settingsBuilder()
+        try {
+            System.setProperty("es.node.zone", "foo");
+            Settings settings = settingsBuilder()
                 .put("node.zone", "bar")
-                .put("path.home", createTempDir().toString())
+                .put(baseEnvSettings)
                 .build();
-        Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettings(settings, true);
-        // Should use setting from the system property
-        assertThat(tuple.v1().get("node.zone"), equalTo("foo"));
+            Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettingsAndEnvironment(settings, null);
+            // Should use setting from the system property
+            assertThat(tuple.v1().get("node.zone"), equalTo("foo"));
 
-        settings = settingsBuilder()
+            settings = settingsBuilder()
                 .put(InternalSettingsPreparer.IGNORE_SYSTEM_PROPERTIES_SETTING, true)
                 .put("node.zone", "bar")
-                .put("path.home", createTempDir().toString())
+                .put(baseEnvSettings)
                 .build();
-        tuple = InternalSettingsPreparer.prepareSettings(settings, true);
-        // Should use setting from the system property
-        assertThat(tuple.v1().get("node.zone"), equalTo("bar"));
+            tuple = InternalSettingsPreparer.prepareSettingsAndEnvironment(settings, null);
+            // Should use setting from the system property
+            assertThat(tuple.v1().get("node.zone"), equalTo("bar"));
+        } finally {
+            System.clearProperty("es.node.zone");
+        }
     }
 
-    @Test
     public void testReplacePromptPlaceholders() {
         final List<String> replacedSecretProperties = new ArrayList<>();
         final List<String> replacedTextProperties = new ArrayList<>();
@@ -102,6 +161,7 @@ public class InternalSettingsPreparerTests extends ESTestCase {
         };
 
         Settings.Builder builder = settingsBuilder()
+                .put(baseEnvSettings)
                 .put("password.replace", InternalSettingsPreparer.SECRET_PROMPT_VALUE)
                 .put("dont.replace", "prompt:secret")
                 .put("dont.replace2", "_prompt:secret_")
@@ -109,8 +169,7 @@ public class InternalSettingsPreparerTests extends ESTestCase {
                 .put("dont.replace4", "__prompt:text_")
                 .put("dont.replace5", "prompt:secret__")
                 .put("replace_me", InternalSettingsPreparer.TEXT_PROMPT_VALUE);
-        Settings settings = builder.build();
-        settings = InternalSettingsPreparer.replacePromptPlaceholders(settings, terminal);
+        Settings settings = InternalSettingsPreparer.prepareSettingsAndEnvironment(builder.build(), terminal).v1();
 
         assertThat(replacedSecretProperties.size(), is(1));
         assertThat(replacedTextProperties.size(), is(1));
@@ -125,70 +184,70 @@ public class InternalSettingsPreparerTests extends ESTestCase {
         assertThat(settings.get("dont.replace5"), equalTo("prompt:secret__"));
     }
 
-    @Test
     public void testReplaceSecretPromptPlaceholderWithNullTerminal() {
         Settings.Builder builder = settingsBuilder()
+                .put(baseEnvSettings)
                 .put("replace_me1", InternalSettingsPreparer.SECRET_PROMPT_VALUE);
         try {
-            InternalSettingsPreparer.replacePromptPlaceholders(builder.build(), null);
+            InternalSettingsPreparer.prepareSettingsAndEnvironment(builder.build(), null);
             fail("an exception should have been thrown since no terminal was provided!");
         } catch (UnsupportedOperationException e) {
             assertThat(e.getMessage(), containsString("with value [" + InternalSettingsPreparer.SECRET_PROMPT_VALUE + "]"));
         }
     }
 
-    @Test
     public void testReplaceTextPromptPlaceholderWithNullTerminal() {
         Settings.Builder builder = settingsBuilder()
+                .put(baseEnvSettings)
                 .put("replace_me1", InternalSettingsPreparer.TEXT_PROMPT_VALUE);
         try {
-            InternalSettingsPreparer.replacePromptPlaceholders(builder.build(), null);
+            InternalSettingsPreparer.prepareSettingsAndEnvironment(builder.build(), null);
             fail("an exception should have been thrown since no terminal was provided!");
         } catch (UnsupportedOperationException e) {
             assertThat(e.getMessage(), containsString("with value [" + InternalSettingsPreparer.TEXT_PROMPT_VALUE + "]"));
         }
     }
 
-    @Test
     public void testNameSettingsPreference() {
-        // Test system property overrides node.name
-        Settings settings = settingsBuilder()
+        try {
+            System.setProperty("name", "sys-prop-name");
+            // Test system property overrides node.name
+            Settings settings = settingsBuilder()
                 .put("node.name", "node-name")
-                .put("path.home", createTempDir().toString())
+                .put(baseEnvSettings)
                 .build();
-        Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettings(settings, true);
-        assertThat(tuple.v1().get("name"), equalTo("sys-prop-name"));
+            Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettingsAndEnvironment(settings, null);
+            assertThat(tuple.v1().get("name"), equalTo("sys-prop-name"));
 
-        // test name in settings overrides sys prop and node.name
-        settings = settingsBuilder()
+            // test name in settings overrides sys prop and node.name
+            settings = settingsBuilder()
                 .put("name", "name-in-settings")
                 .put("node.name", "node-name")
-                .put("path.home", createTempDir().toString())
+                .put(baseEnvSettings)
                 .build();
-        tuple = InternalSettingsPreparer.prepareSettings(settings, true);
-        assertThat(tuple.v1().get("name"), equalTo("name-in-settings"));
+            tuple = InternalSettingsPreparer.prepareSettingsAndEnvironment(settings, null);
+            assertThat(tuple.v1().get("name"), equalTo("name-in-settings"));
 
-        // test only node.name in settings
-        System.clearProperty("name");
-        settings = settingsBuilder()
+            // test only node.name in settings
+            System.clearProperty("name");
+            settings = settingsBuilder()
                 .put("node.name", "node-name")
-                .put("path.home", createTempDir().toString())
+                .put(baseEnvSettings)
                 .build();
-        tuple = InternalSettingsPreparer.prepareSettings(settings, true);
-        assertThat(tuple.v1().get("name"), equalTo("node-name"));
+            tuple = InternalSettingsPreparer.prepareSettingsAndEnvironment(settings, null);
+            assertThat(tuple.v1().get("name"), equalTo("node-name"));
 
-        // test no name at all results in name being set
-        settings = settingsBuilder()
-                .put("path.home", createTempDir().toString())
-                .build();
-        tuple = InternalSettingsPreparer.prepareSettings(settings, true);
-        assertThat(tuple.v1().get("name"), not("name-in-settings"));
-        assertThat(tuple.v1().get("name"), not("sys-prop-name"));
-        assertThat(tuple.v1().get("name"), not("node-name"));
-        assertThat(tuple.v1().get("name"), notNullValue());
+            // test no name at all results in name being set
+            tuple = InternalSettingsPreparer.prepareSettingsAndEnvironment(baseEnvSettings, null);
+            assertThat(tuple.v1().get("name"), not("name-in-settings"));
+            assertThat(tuple.v1().get("name"), not("sys-prop-name"));
+            assertThat(tuple.v1().get("name"), not("node-name"));
+            assertThat(tuple.v1().get("name"), notNullValue());
+        } finally {
+            System.clearProperty("name");
+        }
     }
 
-    @Test
     public void testPromptForNodeNameOnlyPromptsOnce() {
         final AtomicInteger counter = new AtomicInteger();
         final Terminal terminal = new CliToolTestCase.MockTerminal() {
@@ -207,27 +266,30 @@ public class InternalSettingsPreparerTests extends ESTestCase {
 
         System.clearProperty("name");
         Settings settings = Settings.builder()
-                .put("path.home", createTempDir())
+                .put(baseEnvSettings)
                 .put("node.name", InternalSettingsPreparer.TEXT_PROMPT_VALUE)
                 .build();
-        Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettings(settings, false, terminal);
+        Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettingsAndEnvironment(settings, terminal);
         settings = tuple.v1();
         assertThat(counter.intValue(), is(1));
         assertThat(settings.get("name"), is("prompted name 0"));
         assertThat(settings.get("node.name"), is("prompted name 0"));
     }
 
-    @Test(expected = SettingsException.class)
     public void testGarbageIsNotSwallowed() throws IOException {
-        InputStream garbage = getClass().getResourceAsStream("/config/garbage/garbage.yml");
-        Path home = createTempDir();
-        Path config = home.resolve("config");
-        Files.createDirectory(config);
-        Files.copy(garbage, config.resolve("elasticsearch.yml"));
-        InternalSettingsPreparer.prepareSettings(settingsBuilder()
+        try {
+            InputStream garbage = getClass().getResourceAsStream("/config/garbage/garbage.yml");
+            Path home = createTempDir();
+            Path config = home.resolve("config");
+            Files.createDirectory(config);
+            Files.copy(garbage, config.resolve("elasticsearch.yml"));
+            InternalSettingsPreparer.prepareSettingsAndEnvironment(settingsBuilder()
                 .put("config.ignore_system_properties", true)
-                .put("path.home", home)
-                .build(), true);
+                .put(baseEnvSettings)
+                .build(), null);
+        } catch (SettingsException e) {
+            assertEquals("Failed to load settings from [elasticsearch.yml]", e.getMessage());
+        }
     }
 
     public void testMultipleSettingsFileNotAllowed() throws IOException {
@@ -239,14 +301,15 @@ public class InternalSettingsPreparerTests extends ESTestCase {
         Files.copy(yaml, config.resolve("elasticsearch.yaml"));
         Files.copy(properties, config.resolve("elasticsearch.properties"));
 
-        expectedException.expect(SettingsException.class);
-        expectedException.expectMessage("multiple settings files found with suffixes: ");
-        expectedException.expectMessage("yaml");
-        expectedException.expectMessage("properties");
-
-        InternalSettingsPreparer.prepareSettings(settingsBuilder()
+        try {
+            InternalSettingsPreparer.prepareSettingsAndEnvironment(settingsBuilder()
                 .put("config.ignore_system_properties", true)
-                .put("path.home", home)
-                .build(), true);
+                .put(baseEnvSettings)
+                .build(), null);
+        } catch (SettingsException e) {
+            assertTrue(e.getMessage(), e.getMessage().contains("multiple settings files found with suffixes"));
+            assertTrue(e.getMessage(), e.getMessage().contains(".yaml"));
+            assertTrue(e.getMessage(), e.getMessage().contains(".properties"));
+        }
     }
 }

core/src/test/java/org/elasticsearch/plugins/PluginManagerIT.java

@@ -652,7 +652,7 @@ public class PluginManagerIT extends ESIntegTestCase {
         Settings settings = settingsBuilder()
                 .put("http.enabled", true)
                 .put("path.home", createTempDir()).build();
-        return InternalSettingsPreparer.prepareSettings(settings, false);
+        return InternalSettingsPreparer.prepareSettingsAndEnvironment(settings, null);
     }
 
     private void assertStatusOk(String command) {

core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java

@@ -57,7 +57,7 @@ public class PluginsServiceTests extends ESTestCase {
     }
 
     static PluginsService newPluginsService(Settings settings, Class<? extends Plugin>... classpathPlugins) {
-        return new PluginsService(settings, new Environment(settings), Arrays.asList(classpathPlugins));
+        return new PluginsService(settings, new Environment(settings).pluginsFile(), Arrays.asList(classpathPlugins));
     }
 
     public void testAdditionalSettings() {

core/src/test/java/org/elasticsearch/transport/netty/NettyTransportMultiPortIntegrationIT.java

@@ -72,7 +72,7 @@ public class NettyTransportMultiPortIntegrationIT extends ESIntegTestCase {
                 .put(TransportModule.TRANSPORT_TYPE_KEY, "netty")
                 .put("path.home", createTempDir().toString())
                 .build();
-        try (TransportClient transportClient = TransportClient.builder().settings(settings).loadConfigSettings(false).build()) {
+        try (TransportClient transportClient = TransportClient.builder().settings(settings).build()) {
             transportClient.addTransportAddress(new InetSocketTransportAddress(InetAddress.getByName("127.0.0.1"), randomPort));
             ClusterHealthResponse response = transportClient.admin().cluster().prepareHealth().get();
             assertThat(response.getStatus(), is(ClusterHealthStatus.GREEN));