Browse Source

Remove settings and system properties entanglement

Today when parsing settings during bootstrap, we add a system property
for every Elasticsearch setting. Additionally, settings can be set via
system properties. This commit simplifies this situation.
 - settings are no longer propogated to system properties
 - system properties can not be used to set settings
 - the "es." prefix on settings is no longer required (nor permitted)
 - test logging has a dedicated system property (tests.logger.level)

Relates #18198
Jason Tedor 9 years ago
parent
commit
c257e2c51f
55 changed files with 554 additions and 495 deletions
  1. 1 1
      TESTING.asciidoc
  2. 1 1
      buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy
  3. 2 2
      buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy
  4. 7 20
      core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
  5. 4 0
      core/src/main/java/org/elasticsearch/bootstrap/BootstrapInfo.java
  6. 6 21
      core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java
  7. 4 3
      core/src/main/java/org/elasticsearch/cli/Command.java
  8. 77 0
      core/src/main/java/org/elasticsearch/cli/SettingCommand.java
  9. 2 4
      core/src/main/java/org/elasticsearch/common/logging/LogConfigurator.java
  10. 0 1
      core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java
  11. 12 50
      core/src/main/java/org/elasticsearch/common/settings/Settings.java
  12. 27 21
      core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java
  13. 14 12
      core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java
  14. 12 11
      core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java
  15. 13 8
      core/src/main/java/org/elasticsearch/plugins/PluginCli.java
  16. 11 7
      core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java
  17. 1 1
      core/src/main/resources/org/elasticsearch/bootstrap/security.policy
  18. 8 66
      core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java
  19. 0 1
      core/src/test/java/org/elasticsearch/client/transport/TransportClientIT.java
  20. 0 1
      core/src/test/java/org/elasticsearch/client/transport/TransportClientRetryIT.java
  21. 3 6
      core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java
  22. 21 18
      core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java
  23. 5 7
      core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java
  24. 4 3
      core/src/test/resources/org/elasticsearch/common/logging/config/logging.yml
  25. 1 1
      dev-tools/smoke_test_rc.py
  26. 1 1
      distribution/deb/src/main/packaging/init.d/elasticsearch
  27. 1 1
      distribution/rpm/src/main/packaging/init.d/elasticsearch
  28. 3 3
      distribution/src/main/packaging/systemd/elasticsearch.service
  29. 3 3
      distribution/src/main/resources/bin/elasticsearch-plugin
  30. 4 3
      distribution/src/main/resources/config/logging.yml
  31. 1 1
      docs/plugins/plugin-script.asciidoc
  32. 1 1
      docs/reference/getting-started.asciidoc
  33. 1 1
      docs/reference/index-modules/allocation/filtering.asciidoc
  34. 12 0
      docs/reference/migration/migrate_5_0/packaging.asciidoc
  35. 3 8
      docs/reference/migration/migrate_5_0/settings.asciidoc
  36. 1 1
      docs/reference/modules/cluster/allocation_awareness.asciidoc
  37. 1 1
      docs/reference/modules/node.asciidoc
  38. 4 5
      docs/reference/setup/configuration.asciidoc
  39. 3 4
      docs/reference/setup/install/windows.asciidoc
  40. 1 1
      docs/reference/setup/install/zip-targz.asciidoc
  41. 62 0
      qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java
  42. 86 109
      qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java
  43. 30 28
      qa/evil-tests/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java
  44. 22 20
      qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java
  45. 0 19
      qa/evil-tests/src/test/java/org/elasticsearch/tribe/TribeUnitTests.java
  46. 0 1
      qa/smoke-test-client/src/test/java/org/elasticsearch/smoketest/ESSmokeClientTestCase.java
  47. 1 1
      qa/vagrant/src/test/resources/packaging/scripts/module_and_plugin_test_cases.bash
  48. 1 1
      qa/vagrant/src/test/resources/packaging/scripts/packaging_test_utils.bash
  49. 65 0
      test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java
  50. 0 1
      test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java
  51. 0 1
      test/framework/src/main/java/org/elasticsearch/test/ExternalNode.java
  52. 0 1
      test/framework/src/main/java/org/elasticsearch/test/ExternalTestCluster.java
  53. 7 9
      test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java
  54. 2 2
      test/framework/src/main/java/org/elasticsearch/test/junit/listeners/ReproduceInfoPrinter.java
  55. 2 2
      test/framework/src/main/resources/log4j.properties

+ 1 - 1
TESTING.asciidoc

@@ -201,7 +201,7 @@ gradle test -Dtests.timeoutSuite=5000! ...
 Change the logging level of ES (not gradle)
 
 --------------------------------
-gradle test -Des.logger.level=DEBUG
+gradle test -Dtests.logger.level=DEBUG
 --------------------------------
 
 Print all the logging output from the test runs to the commandline

+ 1 - 1
buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

@@ -456,7 +456,7 @@ class BuildPlugin implements Plugin<Project> {
             // default test sysprop values
             systemProperty 'tests.ifNoTests', 'fail'
             // TODO: remove setting logging level via system property
-            systemProperty 'es.logger.level', 'WARN'
+            systemProperty 'tests.logger.level', 'WARN'
             for (Map.Entry<String, String> property : System.properties.entrySet()) {
                 if (property.getKey().startsWith('tests.') ||
                     property.getKey().startsWith('es.')) {

+ 2 - 2
buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy

@@ -129,7 +129,7 @@ class NodeInfo {
         }
 
         env = [ 'JAVA_HOME' : project.javaHome ]
-        args.addAll("-E", "es.node.portsfile=true")
+        args.addAll("-E", "node.portsfile=true")
         String collectedSystemProperties = config.systemProperties.collect { key, value -> "-D${key}=${value}" }.join(" ")
         String esJavaOpts = config.jvmArgs.isEmpty() ? collectedSystemProperties : collectedSystemProperties + " " + config.jvmArgs
         env.put('ES_JAVA_OPTS', esJavaOpts)
@@ -140,7 +140,7 @@ class NodeInfo {
             }
         }
         env.put('ES_JVM_OPTIONS', new File(confDir, 'jvm.options'))
-        args.addAll("-E", "es.path.conf=${confDir}")
+        args.addAll("-E", "path.conf=${confDir}")
         if (Os.isFamily(Os.FAMILY_WINDOWS)) {
             args.add('"') // end the entire command, quoted
         }

+ 7 - 20
core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java

@@ -177,15 +177,7 @@ final class Bootstrap {
         // install SM after natives, shutdown hooks, etc.
         Security.configure(environment, BootstrapSettings.SECURITY_FILTER_BAD_DEFAULTS_SETTING.get(settings));
 
-        // We do not need to reload system properties here as we have already applied them in building the settings and
-        // reloading could cause multiple prompts to the user for values if a system property was specified with a prompt
-        // placeholder
-        Settings nodeSettings = Settings.builder()
-                .put(settings)
-                .put(InternalSettingsPreparer.IGNORE_SYSTEM_PROPERTIES_SETTING.getKey(), true)
-                .build();
-
-        node = new Node(nodeSettings) {
+        node = new Node(settings) {
             @Override
             protected void validateNodeBeforeAcceptingRequests(Settings settings, BoundTransportAddress boundTransportAddress) {
                 BootstrapCheck.check(settings, boundTransportAddress);
@@ -193,13 +185,13 @@ final class Bootstrap {
         };
     }
 
-    private static Environment initialSettings(boolean foreground, String pidFile) {
+    private static Environment initialSettings(boolean foreground, String pidFile, Map<String, String> esSettings) {
         Terminal terminal = foreground ? Terminal.DEFAULT : null;
         Settings.Builder builder = Settings.builder();
         if (Strings.hasLength(pidFile)) {
             builder.put(Environment.PIDFILE_SETTING.getKey(), pidFile);
         }
-        return InternalSettingsPreparer.prepareEnvironment(builder.build(), terminal);
+        return InternalSettingsPreparer.prepareEnvironment(builder.build(), terminal, esSettings);
     }
 
     private void start() {
@@ -233,11 +225,13 @@ final class Bootstrap {
         // Set the system property before anything has a chance to trigger its use
         initLoggerPrefix();
 
-        elasticsearchSettings(esSettings);
+        // force the class initializer for BootstrapInfo to run before
+        // the security manager is installed
+        BootstrapInfo.init();
 
         INSTANCE = new Bootstrap();
 
-        Environment environment = initialSettings(foreground, pidFile);
+        Environment environment = initialSettings(foreground, pidFile, esSettings);
         Settings settings = environment.settings();
         LogConfigurator.configure(settings, true);
         checkForCustomConfFile();
@@ -295,13 +289,6 @@ final class Bootstrap {
         }
     }
 
-    @SuppressForbidden(reason = "Sets system properties passed as CLI parameters")
-    private static void elasticsearchSettings(Map<String, String> esSettings) {
-        for (Map.Entry<String, String> esSetting : esSettings.entrySet()) {
-            System.setProperty(esSetting.getKey(), esSetting.getValue());
-        }
-    }
-
     @SuppressForbidden(reason = "System#out")
     private static void closeSystOut() {
         System.out.close();

+ 4 - 0
core/src/main/java/org/elasticsearch/bootstrap/BootstrapInfo.java

@@ -120,4 +120,8 @@ public final class BootstrapInfo {
         }
         return SYSTEM_PROPERTIES;
     }
+
+    public static void init() {
+    }
+
 }

+ 6 - 21
core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java

@@ -21,28 +21,25 @@ package org.elasticsearch.bootstrap;
 
 import joptsimple.OptionSet;
 import joptsimple.OptionSpec;
-import joptsimple.util.KeyValuePair;
 import org.elasticsearch.Build;
-import org.elasticsearch.cli.Command;
 import org.elasticsearch.cli.ExitCodes;
+import org.elasticsearch.cli.SettingCommand;
 import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.cli.UserError;
 import org.elasticsearch.monitor.jvm.JvmInfo;
 
 import java.io.IOException;
 import java.util.Arrays;
-import java.util.HashMap;
 import java.util.Map;
 
 /**
  * This class starts elasticsearch.
  */
-class Elasticsearch extends Command {
+class Elasticsearch extends SettingCommand {
 
     private final OptionSpec<Void> versionOption;
     private final OptionSpec<Void> daemonizeOption;
     private final OptionSpec<String> pidfileOption;
-    private final OptionSpec<KeyValuePair> propertyOption;
 
     // visible for testing
     Elasticsearch() {
@@ -56,7 +53,6 @@ class Elasticsearch extends Command {
         pidfileOption = parser.acceptsAll(Arrays.asList("p", "pidfile"),
             "Creates a pid file in the specified path on start")
             .withRequiredArg();
-        propertyOption = parser.accepts("E", "Configure an Elasticsearch setting").withRequiredArg().ofType(KeyValuePair.class);
     }
 
     /**
@@ -75,7 +71,7 @@ class Elasticsearch extends Command {
     }
 
     @Override
-    protected void execute(Terminal terminal, OptionSet options) throws Exception {
+    protected void execute(Terminal terminal, OptionSet options, Map<String, String> settings) throws Exception {
         if (options.nonOptionArguments().isEmpty() == false) {
             throw new UserError(ExitCodes.USAGE, "Positional arguments not allowed, found " + options.nonOptionArguments());
         }
@@ -84,26 +80,15 @@ class Elasticsearch extends Command {
                 throw new UserError(ExitCodes.USAGE, "Elasticsearch version option is mutually exclusive with any other option");
             }
             terminal.println("Version: " + org.elasticsearch.Version.CURRENT
-                + ", Build: " + Build.CURRENT.shortHash() + "/" + Build.CURRENT.date()
-                + ", JVM: " + JvmInfo.jvmInfo().version());
+                    + ", Build: " + Build.CURRENT.shortHash() + "/" + Build.CURRENT.date()
+                    + ", JVM: " + JvmInfo.jvmInfo().version());
             return;
         }
 
         final boolean daemonize = options.has(daemonizeOption);
         final String pidFile = pidfileOption.value(options);
 
-        final Map<String, String> esSettings = new HashMap<>();
-        for (final KeyValuePair kvp : propertyOption.values(options)) {
-            if (!kvp.key.startsWith("es.")) {
-                throw new UserError(ExitCodes.USAGE, "Elasticsearch settings must be prefixed with [es.] but was [" + kvp.key + "]");
-            }
-            if (kvp.value.isEmpty()) {
-                throw new UserError(ExitCodes.USAGE, "Elasticsearch setting [" + kvp.key + "] must not be empty");
-            }
-            esSettings.put(kvp.key, kvp.value);
-        }
-
-        init(daemonize, pidFile, esSettings);
+        init(daemonize, pidFile, settings);
     }
 
     void init(final boolean daemonize, final String pidFile, final Map<String, String> esSettings) {

+ 4 - 3
core/src/main/java/org/elasticsearch/cli/Command.java

@@ -19,15 +19,15 @@
 
 package org.elasticsearch.cli;
 
-import java.io.IOException;
-import java.util.Arrays;
-
 import joptsimple.OptionException;
 import joptsimple.OptionParser;
 import joptsimple.OptionSet;
 import joptsimple.OptionSpec;
 import org.elasticsearch.common.SuppressForbidden;
 
+import java.io.IOException;
+import java.util.Arrays;
+
 /**
  * An action to execute within a cli.
  */
@@ -112,4 +112,5 @@ public abstract class Command {
      *
      * Any runtime user errors (like an input file that does not exist), should throw a {@link UserError}. */
     protected abstract void execute(Terminal terminal, OptionSet options) throws Exception;
+
 }

+ 77 - 0
core/src/main/java/org/elasticsearch/cli/SettingCommand.java

@@ -0,0 +1,77 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.cli;
+
+import joptsimple.OptionSet;
+import joptsimple.OptionSpec;
+import joptsimple.util.KeyValuePair;
+
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+
+public abstract class SettingCommand extends Command {
+
+    private final OptionSpec<KeyValuePair> settingOption;
+
+    public SettingCommand(String description) {
+        super(description);
+        this.settingOption = parser.accepts("E", "Configure a setting").withRequiredArg().ofType(KeyValuePair.class);
+    }
+
+    @Override
+    protected void execute(Terminal terminal, OptionSet options) throws Exception {
+        final Map<String, String> settings = new HashMap<>();
+        for (final KeyValuePair kvp : settingOption.values(options)) {
+            if (kvp.value.isEmpty()) {
+                throw new UserError(ExitCodes.USAGE, "Setting [" + kvp.key + "] must not be empty");
+            }
+            settings.put(kvp.key, kvp.value);
+        }
+
+        putSystemPropertyIfSettingIsMissing(settings, "path.conf", "es.path.conf");
+        putSystemPropertyIfSettingIsMissing(settings, "path.data", "es.path.data");
+        putSystemPropertyIfSettingIsMissing(settings, "path.home", "es.path.home");
+        putSystemPropertyIfSettingIsMissing(settings, "path.logs", "es.path.logs");
+
+        execute(terminal, options, settings);
+    }
+
+    protected static void putSystemPropertyIfSettingIsMissing(final Map<String, String> settings, final String setting, final String key) {
+        final String value = System.getProperty(key);
+        if (value != null) {
+            if (settings.containsKey(setting)) {
+                final String message =
+                        String.format(
+                                Locale.ROOT,
+                                "duplicate setting [%s] found via command-line [%s] and system property [%s]",
+                                setting,
+                                settings.get(setting),
+                                value);
+                throw new IllegalArgumentException(message);
+            } else {
+                settings.put(setting, value);
+            }
+        }
+    }
+
+    protected abstract void execute(Terminal terminal, OptionSet options, Map<String, String> settings) throws Exception;
+
+}

+ 2 - 4
core/src/main/java/org/elasticsearch/common/logging/LogConfigurator.java

@@ -21,7 +21,6 @@ package org.elasticsearch.common.logging;
 
 import org.apache.log4j.PropertyConfigurator;
 import org.elasticsearch.ElasticsearchException;
-import org.elasticsearch.bootstrap.BootstrapInfo;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsException;
 import org.elasticsearch.env.Environment;
@@ -93,8 +92,7 @@ public class LogConfigurator {
 
     /**
      * Consolidates settings and converts them into actual log4j settings, then initializes loggers and appenders.
-     *
-     * @param settings      custom settings that should be applied
+     *  @param settings      custom settings that should be applied
      * @param resolveConfig controls whether the logging conf file should be read too or not.
      */
     public static void configure(Settings settings, boolean resolveConfig) {
@@ -109,7 +107,7 @@ public class LogConfigurator {
         if (resolveConfig) {
             resolveConfig(environment, settingsBuilder);
         }
-        settingsBuilder.putProperties("es.", BootstrapInfo.getSystemProperties());
+
         // add custom settings after config was added so that they are not overwritten by config
         settingsBuilder.put(settings);
         settingsBuilder.replacePropertyPlaceholders();

+ 0 - 1
core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java

@@ -375,7 +375,6 @@ public final class ClusterSettings extends AbstractScopedSettings {
                     BaseRestHandler.MULTI_ALLOW_EXPLICIT_INDEX,
                     ClusterName.CLUSTER_NAME_SETTING,
                     Client.CLIENT_TYPE_SETTING_S,
-                    InternalSettingsPreparer.IGNORE_SYSTEM_PROPERTIES_SETTING,
                     ClusterModule.SHARDS_ALLOCATOR_TYPE_SETTING,
                     EsExecutors.PROCESSORS_SETTING,
                     ThreadContext.DEFAULT_HEADERS_SETTING,

+ 12 - 50
core/src/main/java/org/elasticsearch/common/settings/Settings.java

@@ -58,9 +58,11 @@ import java.util.Set;
 import java.util.SortedMap;
 import java.util.TreeMap;
 import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
 import static org.elasticsearch.common.unit.ByteSizeValue.parseBytesSizeValue;
 import static org.elasticsearch.common.unit.SizeValue.parseSizeValue;
@@ -942,66 +944,27 @@ public final class Settings implements ToXContent {
             return this;
         }
 
-        /**
-         * Puts all the properties with keys starting with the provided <tt>prefix</tt>.
-         *
-         * @param prefix     The prefix to filter property key by
-         * @param properties The properties to put
-         * @return The builder
-         */
-        public Builder putProperties(String prefix, Dictionary<Object, Object> properties) {
-            for (Object property : Collections.list(properties.keys())) {
-                String key = Objects.toString(property);
-                String value = Objects.toString(properties.get(property));
-                if (key.startsWith(prefix)) {
-                    map.put(key.substring(prefix.length()), value);
+        public Builder putProperties(Map<String, String> esSettings, Predicate<String> keyPredicate, Function<String, String> keyFunction) {
+            for (final Map.Entry<String, String> esSetting : esSettings.entrySet()) {
+                final String key = esSetting.getKey();
+                if (keyPredicate.test(key)) {
+                    map.put(keyFunction.apply(key), esSetting.getValue());
                 }
             }
             return this;
         }
 
         /**
-         * Puts all the properties with keys starting with the provided <tt>prefix</tt>.
-         *
-         * @param prefix     The prefix to filter property key by
-         * @param properties The properties to put
-         * @return The builder
-         */
-        public Builder putProperties(String prefix, Dictionary<Object, Object> properties, String ignorePrefix) {
-            for (Object property : Collections.list(properties.keys())) {
-                String key = Objects.toString(property);
-                String value = Objects.toString(properties.get(property));
-                if (key.startsWith(prefix)) {
-                    if (!key.startsWith(ignorePrefix)) {
-                        map.put(key.substring(prefix.length()), value);
-                    }
-                }
-            }
-            return this;
-        }
-
-        /**
-         * Runs across all the settings set on this builder and replaces <tt>${...}</tt> elements in the
-         * each setting value according to the following logic:
-         * <p>
-         * First, tries to resolve it against a System property ({@link System#getProperty(String)}), next,
-         * 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.
+         * Runs across all the settings set on this builder and
+         * replaces <tt>${...}</tt> elements in each setting with
+         * another setting already set on this builder.
          */
         public Builder replacePropertyPlaceholders() {
             PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false);
             PropertyPlaceholder.PlaceholderResolver placeholderResolver = new PropertyPlaceholder.PlaceholderResolver() {
                     @Override
                     public String resolvePlaceholder(String placeholderName) {
-                        if (placeholderName.startsWith("env.")) {
-                            // explicit env var prefix
-                            return System.getenv(placeholderName.substring("env.".length()));
-                        }
-                        String value = System.getProperty(placeholderName);
-                        if (value != null) {
-                            return value;
-                        }
-                        value = System.getenv(placeholderName);
+                        final String value = System.getenv(placeholderName);
                         if (value != null) {
                             return value;
                         }
@@ -1010,8 +973,7 @@ 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.") || placeholderName.startsWith("prompt.")) {
+                        if (placeholderName.startsWith("prompt.")) {
                             return true;
                         }
                         return false;

+ 27 - 21
core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java

@@ -19,14 +19,11 @@
 
 package org.elasticsearch.node.internal;
 
-import org.elasticsearch.bootstrap.BootstrapInfo;
+import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.common.Randomness;
 import org.elasticsearch.common.Strings;
-import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.common.collect.Tuple;
-import org.elasticsearch.common.settings.Setting;
-import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsException;
 import org.elasticsearch.env.Environment;
@@ -39,10 +36,13 @@ 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.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Function;
+import java.util.function.Predicate;
 
 import static org.elasticsearch.common.Strings.cleanPath;
 
@@ -52,20 +52,18 @@ import static org.elasticsearch.common.Strings.cleanPath;
 public class InternalSettingsPreparer {
 
     private static final String[] ALLOWED_SUFFIXES = {".yml", ".yaml", ".json", ".properties"};
-    static final String PROPERTY_PREFIX = "es.";
-    static final String PROPERTY_DEFAULTS_PREFIX = "es.default.";
+    static final String PROPERTY_DEFAULTS_PREFIX = "default.";
+    static final Predicate<String> PROPERTY_DEFAULTS_PREDICATE = key -> key.startsWith(PROPERTY_DEFAULTS_PREFIX);
 
     public static final String SECRET_PROMPT_VALUE = "${prompt.secret}";
     public static final String TEXT_PROMPT_VALUE = "${prompt.text}";
-    public static final Setting<Boolean> IGNORE_SYSTEM_PROPERTIES_SETTING =
-        Setting.boolSetting("config.ignore_system_properties", false, Property.NodeScope);
 
     /**
      * Prepares the settings by gathering all elasticsearch system properties and setting defaults.
      */
     public static Settings prepareSettings(Settings input) {
         Settings.Builder output = Settings.builder();
-        initializeSettings(output, input, true);
+        initializeSettings(output, input, true, Collections.emptyMap());
         finalizeSettings(output, null, null);
         return output.build();
     }
@@ -80,9 +78,23 @@ public class InternalSettingsPreparer {
      * @return the {@link Settings} and {@link Environment} as a {@link Tuple}
      */
     public static Environment prepareEnvironment(Settings input, Terminal terminal) {
+        return prepareEnvironment(input, terminal, Collections.emptyMap());
+    }
+
+    /**
+     * Prepares the settings by gathering all elasticsearch system properties, optionally loading the configuration settings,
+     * 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 input The custom settings to use. These are not overwritten by settings in the configuration file.
+     * @param terminal the Terminal to use for input/output
+     * @param properties Map of properties key/value pairs (usually from the command-line)
+     * @return the {@link Settings} and {@link Environment} as a {@link Tuple}
+     */
+    public static Environment prepareEnvironment(Settings input, Terminal terminal, Map<String, String> properties) {
         // just create enough settings to build the environment, to get the config dir
         Settings.Builder output = Settings.builder();
-        initializeSettings(output, input, true);
+        initializeSettings(output, input, true, properties);
         Environment environment = new Environment(output.build());
 
         boolean settingsFileFound = false;
@@ -103,7 +115,7 @@ public class InternalSettingsPreparer {
 
         // 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);
+        initializeSettings(output, input, false, properties);
         finalizeSettings(output, terminal, environment.configFile());
 
         environment = new Environment(output.build());
@@ -113,22 +125,16 @@ public class InternalSettingsPreparer {
         return new Environment(output.build());
     }
 
-    private static boolean useSystemProperties(Settings input) {
-        return !IGNORE_SYSTEM_PROPERTIES_SETTING.get(input);
-    }
-
     /**
      * 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) {
+    private static void initializeSettings(Settings.Builder output, Settings input, boolean loadDefaults, Map<String, String> esSettings) {
         output.put(input);
-        if (useSystemProperties(input)) {
-            if (loadDefaults) {
-                output.putProperties(PROPERTY_DEFAULTS_PREFIX, BootstrapInfo.getSystemProperties());
-            }
-            output.putProperties(PROPERTY_PREFIX, BootstrapInfo.getSystemProperties(), PROPERTY_DEFAULTS_PREFIX);
+        if (loadDefaults) {
+            output.putProperties(esSettings, PROPERTY_DEFAULTS_PREDICATE, key -> key.substring(PROPERTY_DEFAULTS_PREFIX.length()));
         }
+        output.putProperties(esSettings, PROPERTY_DEFAULTS_PREDICATE.negate(), Function.identity());
         output.replacePropertyPlaceholders();
     }
 

+ 14 - 12
core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java

@@ -27,11 +27,14 @@ import org.elasticsearch.Version;
 import org.elasticsearch.bootstrap.JarHell;
 import org.elasticsearch.cli.Command;
 import org.elasticsearch.cli.ExitCodes;
+import org.elasticsearch.cli.SettingCommand;
 import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.cli.UserError;
 import org.elasticsearch.common.hash.MessageDigests;
 import org.elasticsearch.common.io.FileSystemUtils;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
+import org.elasticsearch.node.internal.InternalSettingsPreparer;
 
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -56,6 +59,7 @@ import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.zip.ZipEntry;
@@ -95,7 +99,7 @@ import static org.elasticsearch.common.util.set.Sets.newHashSet;
  * elasticsearch config directory, using the name of the plugin. If any files to be installed
  * already exist, they will be skipped.
  */
-class InstallPluginCommand extends Command {
+class InstallPluginCommand extends SettingCommand {
 
     private static final String PROPERTY_SUPPORT_STAGING_URLS = "es.plugins.staging";
 
@@ -132,7 +136,6 @@ class InstallPluginCommand extends Command {
             "store-smb",
             "x-pack")));
 
-    private final Environment env;
     private final OptionSpec<Void> batchOption;
     private final OptionSpec<String> arguments;
 
@@ -160,9 +163,8 @@ class InstallPluginCommand extends Command {
         FILE_PERMS = Collections.unmodifiableSet(filePerms);
     }
 
-    InstallPluginCommand(Environment env) {
+    InstallPluginCommand() {
         super("Install a plugin");
-        this.env = env;
         this.batchOption = parser.acceptsAll(Arrays.asList("b", "batch"),
                 "Enable batch mode explicitly, automatic confirmation of security permission");
         this.arguments = parser.nonOptions("plugin id");
@@ -178,7 +180,7 @@ class InstallPluginCommand extends Command {
     }
 
     @Override
-    protected void execute(Terminal terminal, OptionSet options) throws Exception {
+    protected void execute(Terminal terminal, OptionSet options, Map<String, String> settings) throws Exception {
         // TODO: in jopt-simple 5.0 we can enforce a min/max number of positional args
         List<String> args = arguments.values(options);
         if (args.size() != 1) {
@@ -186,12 +188,12 @@ class InstallPluginCommand extends Command {
         }
         String pluginId = args.get(0);
         boolean isBatch = options.has(batchOption) || System.console() == null;
-        execute(terminal, pluginId, isBatch);
+        execute(terminal, pluginId, isBatch, settings);
     }
 
     // pkg private for testing
-    void execute(Terminal terminal, String pluginId, boolean isBatch) throws Exception {
-
+    void execute(Terminal terminal, String pluginId, boolean isBatch, Map<String, String> settings) throws Exception {
+        final Environment env = InternalSettingsPreparer.prepareEnvironment(Settings.EMPTY, terminal, settings);
         // TODO: remove this leniency!! is it needed anymore?
         if (Files.exists(env.pluginsFile()) == false) {
             terminal.println("Plugins directory [" + env.pluginsFile() + "] does not exist. Creating...");
@@ -200,7 +202,7 @@ class InstallPluginCommand extends Command {
 
         Path pluginZip = download(terminal, pluginId, env.tmpFile());
         Path extractedZip = unzip(pluginZip, env.pluginsFile());
-        install(terminal, isBatch, extractedZip);
+        install(terminal, isBatch, extractedZip, env);
     }
 
     /** Downloads the plugin and returns the file it was downloaded to. */
@@ -349,7 +351,7 @@ class InstallPluginCommand extends Command {
     }
 
     /** Load information about the plugin, and verify it can be installed with no errors. */
-    private PluginInfo verify(Terminal terminal, Path pluginRoot, boolean isBatch) throws Exception {
+    private PluginInfo verify(Terminal terminal, Path pluginRoot, boolean isBatch, Environment env) throws Exception {
         // read and validate the plugin descriptor
         PluginInfo info = PluginInfo.readFromProperties(pluginRoot);
         terminal.println(VERBOSE, info.toString());
@@ -398,12 +400,12 @@ class InstallPluginCommand extends Command {
      * Installs the plugin from {@code tmpRoot} into the plugins dir.
      * If the plugin has a bin dir and/or a config dir, those are copied.
      */
-    private void install(Terminal terminal, boolean isBatch, Path tmpRoot) throws Exception {
+    private void install(Terminal terminal, boolean isBatch, Path tmpRoot, Environment env) throws Exception {
         List<Path> deleteOnFailure = new ArrayList<>();
         deleteOnFailure.add(tmpRoot);
 
         try {
-            PluginInfo info = verify(terminal, tmpRoot, isBatch);
+            PluginInfo info = verify(terminal, tmpRoot, isBatch, env);
 
             final Path destination = env.pluginsFile().resolve(info.getName());
             if (Files.exists(destination)) {

+ 12 - 11
core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java

@@ -19,6 +19,13 @@
 
 package org.elasticsearch.plugins;
 
+import joptsimple.OptionSet;
+import org.elasticsearch.cli.SettingCommand;
+import org.elasticsearch.cli.Terminal;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.env.Environment;
+import org.elasticsearch.node.internal.InternalSettingsPreparer;
+
 import java.io.IOException;
 import java.nio.file.DirectoryStream;
 import java.nio.file.Files;
@@ -26,26 +33,20 @@ import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
-
-import joptsimple.OptionSet;
-import org.elasticsearch.cli.Command;
-import org.elasticsearch.cli.Terminal;
-import org.elasticsearch.env.Environment;
+import java.util.Map;
 
 /**
  * A command for the plugin cli to list plugins installed in elasticsearch.
  */
-class ListPluginsCommand extends Command {
-
-    private final Environment env;
+class ListPluginsCommand extends SettingCommand {
 
-    ListPluginsCommand(Environment env) {
+    ListPluginsCommand() {
         super("Lists installed elasticsearch plugins");
-        this.env = env;
     }
 
     @Override
-    protected void execute(Terminal terminal, OptionSet options) throws Exception {
+    protected void execute(Terminal terminal, OptionSet options, Map<String, String> settings) throws Exception {
+        final Environment env = InternalSettingsPreparer.prepareEnvironment(Settings.EMPTY, terminal, settings);
         if (Files.exists(env.pluginsFile()) == false) {
             throw new IOException("Plugins directory missing: " + env.pluginsFile());
         }

+ 13 - 8
core/src/main/java/org/elasticsearch/plugins/PluginCli.java

@@ -26,21 +26,24 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.node.internal.InternalSettingsPreparer;
 
+import java.util.Collections;
+
 /**
  * A cli tool for adding, removing and listing plugins for elasticsearch.
  */
 public class PluginCli extends MultiCommand {
 
-    public PluginCli(Environment env) {
+    public PluginCli() {
         super("A tool for managing installed elasticsearch plugins");
-        subcommands.put("list", new ListPluginsCommand(env));
-        subcommands.put("install", new InstallPluginCommand(env));
-        subcommands.put("remove", new RemovePluginCommand(env));
+        subcommands.put("list", new ListPluginsCommand());
+        subcommands.put("install", new InstallPluginCommand());
+        subcommands.put("remove", new RemovePluginCommand());
     }
 
     public static void main(String[] args) throws Exception {
         // initialize default for es.logger.level because we will not read the logging.yml
         String loggerLevel = System.getProperty("es.logger.level", "INFO");
+        String pathHome = System.getProperty("es.path.home");
         // Set the appender for all potential log files to terminal so that other components that use the logger print out the
         // same terminal.
         // The reason for this is that the plugin cli cannot be configured with a file appender because when the plugin command is
@@ -48,12 +51,14 @@ public class PluginCli extends MultiCommand {
         // is run as service then the logs should be at /var/log/elasticsearch but when started from the tar they should be at es.home/logs.
         // Therefore we print to Terminal.
         Environment loggingEnvironment = InternalSettingsPreparer.prepareEnvironment(Settings.builder()
+                .put("path.home", pathHome)
                 .put("appender.terminal.type", "terminal")
-                .put("rootLogger", "${es.logger.level}, terminal")
-                .put("es.logger.level", loggerLevel)
+                .put("rootLogger", "${logger.level}, terminal")
+                .put("logger.level", loggerLevel)
                 .build(), Terminal.DEFAULT);
         LogConfigurator.configure(loggingEnvironment.settings(), false);
-        Environment env = InternalSettingsPreparer.prepareEnvironment(Settings.EMPTY, Terminal.DEFAULT);
-        exit(new PluginCli(env).main(args, Terminal.DEFAULT));
+
+        exit(new PluginCli().main(args, Terminal.DEFAULT));
     }
+
 }

+ 11 - 7
core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java

@@ -24,45 +24,49 @@ import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 
 import joptsimple.OptionSet;
 import joptsimple.OptionSpec;
 import org.apache.lucene.util.IOUtils;
 import org.elasticsearch.cli.Command;
 import org.elasticsearch.cli.ExitCodes;
+import org.elasticsearch.cli.SettingCommand;
 import org.elasticsearch.cli.UserError;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.cli.Terminal;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
+import org.elasticsearch.node.internal.InternalSettingsPreparer;
 
 import static org.elasticsearch.cli.Terminal.Verbosity.VERBOSE;
 
 /**
  * A command for the plugin cli to remove a plugin from elasticsearch.
  */
-class RemovePluginCommand extends Command {
+class RemovePluginCommand extends SettingCommand {
 
-    private final Environment env;
     private final OptionSpec<String> arguments;
 
-    RemovePluginCommand(Environment env) {
+    RemovePluginCommand() {
         super("Removes a plugin from elasticsearch");
-        this.env = env;
         this.arguments = parser.nonOptions("plugin name");
     }
 
     @Override
-    protected void execute(Terminal terminal, OptionSet options) throws Exception {
+    protected void execute(Terminal terminal, OptionSet options, Map<String, String> settings) throws Exception {
         // TODO: in jopt-simple 5.0 we can enforce a min/max number of positional args
         List<String> args = arguments.values(options);
         if (args.size() != 1) {
             throw new UserError(ExitCodes.USAGE, "Must supply a single plugin id argument");
         }
-        execute(terminal, args.get(0));
+        execute(terminal, args.get(0), settings);
     }
 
     // pkg private for testing
-    void execute(Terminal terminal, String pluginName) throws Exception {
+    void execute(Terminal terminal, String pluginName, Map<String, String> settings) throws Exception {
+        final Environment env = InternalSettingsPreparer.prepareEnvironment(Settings.EMPTY, terminal, settings);
+
         terminal.println("-> Removing " + Strings.coalesceToEmpty(pluginName) + "...");
 
         Path pluginDir = env.pluginsFile().resolve(pluginName);

+ 1 - 1
core/src/main/resources/org/elasticsearch/bootstrap/security.policy

@@ -71,7 +71,7 @@ grant {
 
   // set by ESTestCase to improve test reproducibility
   // TODO: set this with gradle or some other way that repros with seed?
-  permission java.util.PropertyPermission "es.processors.override", "write";
+  permission java.util.PropertyPermission "processors.override", "write";
 
   // TODO: these simply trigger a noisy warning if its unable to clear the properties
   // fix that in randomizedtesting

+ 8 - 66
core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java

@@ -22,25 +22,15 @@ package org.elasticsearch.bootstrap;
 import org.elasticsearch.Build;
 import org.elasticsearch.Version;
 import org.elasticsearch.cli.ExitCodes;
-import org.elasticsearch.cli.MockTerminal;
-import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.monitor.jvm.JvmInfo;
-import org.elasticsearch.test.ESTestCase;
-import org.junit.After;
-import org.junit.Before;
-
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.concurrent.atomic.AtomicBoolean;
+
 import java.util.function.Consumer;
 
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.Matchers.hasEntry;
 
-public class ElasticsearchCliTests extends ESTestCase {
+public class ElasticsearchCliTests extends ESElasticsearchCliTestCase {
 
     public void testVersion() throws Exception {
         runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "-d");
@@ -96,7 +86,7 @@ public class ElasticsearchCliTests extends ESTestCase {
             false,
             output -> assertThat(output, containsString("Positional arguments not allowed, found [foo]")),
             (foreground, pidFile, esSettings) -> {},
-            "-E", "something", "foo", "-E", "somethingelse"
+            "-E", "foo=bar", "foo", "-E", "baz=qux"
         );
     }
 
@@ -138,26 +128,10 @@ public class ElasticsearchCliTests extends ESTestCase {
                 output -> {},
                 (foreground, pidFile, esSettings) -> {
                     assertThat(esSettings.size(), equalTo(2));
-                    assertThat(esSettings, hasEntry("es.foo", "bar"));
-                    assertThat(esSettings, hasEntry("es.baz", "qux"));
+                    assertThat(esSettings, hasEntry("foo", "bar"));
+                    assertThat(esSettings, hasEntry("baz", "qux"));
                 },
-                "-Ees.foo=bar", "-E", "es.baz=qux"
-        );
-    }
-
-    public void testElasticsearchSettingPrefix() throws Exception {
-        runElasticsearchSettingPrefixTest("-E", "foo");
-        runElasticsearchSettingPrefixTest("-E", "foo=bar");
-        runElasticsearchSettingPrefixTest("-E", "=bar");
-    }
-
-    private void runElasticsearchSettingPrefixTest(String... args) throws Exception {
-        runTest(
-                ExitCodes.USAGE,
-                false,
-                output -> assertThat(output, containsString("Elasticsearch settings must be prefixed with [es.] but was [")),
-                (foreground, pidFile, esSettings) -> {},
-                args
+                "-Efoo=bar", "-E", "baz=qux"
         );
     }
 
@@ -165,9 +139,9 @@ public class ElasticsearchCliTests extends ESTestCase {
         runTest(
                 ExitCodes.USAGE,
                 false,
-                output -> assertThat(output, containsString("Elasticsearch setting [es.foo] must not be empty")),
+                output -> assertThat(output, containsString("Setting [foo] must not be empty")),
                 (foreground, pidFile, esSettings) -> {},
-                "-E", "es.foo="
+                "-E", "foo="
         );
     }
 
@@ -180,36 +154,4 @@ public class ElasticsearchCliTests extends ESTestCase {
                 "--network.host");
     }
 
-    private interface InitConsumer {
-        void accept(final boolean foreground, final String pidFile, final Map<String, String> esSettings);
-    }
-
-    private void runTest(
-            final int expectedStatus,
-            final boolean expectedInit,
-            final Consumer<String> outputConsumer,
-            final InitConsumer initConsumer,
-            String... args) throws Exception {
-        final MockTerminal terminal = new MockTerminal();
-        try {
-            final AtomicBoolean init = new AtomicBoolean();
-            final int status = Elasticsearch.main(args, new Elasticsearch() {
-                @Override
-                void init(final boolean daemonize, final String pidFile, final Map<String, String> esSettings) {
-                    init.set(true);
-                    initConsumer.accept(!daemonize, pidFile, esSettings);
-                }
-            }, terminal);
-            assertThat(status, equalTo(expectedStatus));
-            assertThat(init.get(), equalTo(expectedInit));
-            outputConsumer.accept(terminal.getOutput());
-        } catch (Throwable t) {
-            // if an unexpected exception is thrown, we log
-            // terminal output to aid debugging
-            logger.info(terminal.getOutput());
-            // rethrow so the test fails
-            throw t;
-        }
-    }
-
 }

+ 0 - 1
core/src/test/java/org/elasticsearch/client/transport/TransportClientIT.java

@@ -59,7 +59,6 @@ public class TransportClientIT extends ESIntegTestCase {
                 .put("http.enabled", false)
                 .put(Node.NODE_DATA_SETTING.getKey(), false)
                 .put("cluster.name", "foobar")
-                .put(InternalSettingsPreparer.IGNORE_SYSTEM_PROPERTIES_SETTING.getKey(), true) // make sure we get what we set :)
                 .build());
         node.start();
         try {

+ 0 - 1
core/src/test/java/org/elasticsearch/client/transport/TransportClientRetryIT.java

@@ -55,7 +55,6 @@ public class TransportClientRetryIT extends ESIntegTestCase {
                 .put("node.name", "transport_client_retry_test")
                 .put(Node.NODE_MODE_SETTING.getKey(), internalCluster().getNodeMode())
                 .put(ClusterName.CLUSTER_NAME_SETTING.getKey(), internalCluster().getClusterName())
-                .put(InternalSettingsPreparer.IGNORE_SYSTEM_PROPERTIES_SETTING.getKey(), true)
                 .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir());
 
         try (TransportClient client = TransportClient.builder().settings(builder.build()).build()) {

+ 3 - 6
core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java

@@ -302,11 +302,8 @@ public class ScopedSettingsTests extends ESTestCase {
     public void testLoggingUpdates() {
         final String level = ESLoggerFactory.getRootLogger().getLevel();
         final String testLevel = ESLoggerFactory.getLogger("test").getLevel();
-        String property = System.getProperty("es.logger.level");
-        Settings.Builder builder = Settings.builder();
-        if (property != null) {
-            builder.put("logger.level", property);
-        }
+        String property = randomFrom(ESLoggerFactory.LogLevel.values()).toString();
+        Settings.Builder builder = Settings.builder().put("logger.level", property);
         try {
             ClusterSettings settings = new ClusterSettings(builder.build(), ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
             try {
@@ -319,7 +316,7 @@ public class ScopedSettingsTests extends ESTestCase {
             settings.applySettings(Settings.builder().put("logger._root", "TRACE").build());
             assertEquals("TRACE", ESLoggerFactory.getRootLogger().getLevel());
             settings.applySettings(Settings.builder().build());
-            assertEquals(level, ESLoggerFactory.getRootLogger().getLevel());
+            assertEquals(property, ESLoggerFactory.getRootLogger().getLevel());
             settings.applySettings(Settings.builder().put("logger.test", "TRACE").build());
             assertEquals("TRACE", ESLoggerFactory.getLogger("test").getLevel());
             settings.applySettings(Settings.builder().build());

+ 21 - 18
core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java

@@ -31,7 +31,9 @@ import java.util.Set;
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.arrayContaining;
 import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasToString;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
@@ -42,31 +44,32 @@ public class SettingsTests extends ESTestCase {
         String value = System.getProperty("java.home");
         assertFalse(value.isEmpty());
         Settings settings = Settings.builder()
-                 .put("setting1", "${java.home}")
+                 .put("property.placeholder", value)
+                 .put("setting1", "${property.placeholder}")
                  .replacePropertyPlaceholders()
                  .build();
         assertThat(settings.get("setting1"), equalTo(value));
+    }
 
-        assertNull(System.getProperty("_test_property_should_not_exist"));
-        settings = Settings.builder()
-                .put("setting1", "${_test_property_should_not_exist:defaultVal1}")
-                .replacePropertyPlaceholders()
-                .build();
-        assertThat(settings.get("setting1"), equalTo("defaultVal1"));
-
-        settings = Settings.builder()
-                .put("setting1", "${_test_property_should_not_exist:}")
+    public void testReplacePropertiesPlaceholderSystemVariablesHaveNoEffect() {
+        final String value = System.getProperty("java.home");
+        assertNotNull(value);
+        final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> Settings.builder()
+                .put("setting1", "${java.home}")
                 .replacePropertyPlaceholders()
-                .build();
-        assertThat(settings.get("setting1"), is(nullValue()));
+                .build());
+        assertThat(e, hasToString(containsString("Could not resolve placeholder 'java.home'")));
     }
 
-    public void testReplacePropertiesPlaceholderIgnoreEnvUnset() {
-        Settings settings = Settings.builder()
-                .put("setting1", "${env.UNSET_ENV_VAR}")
-                .replacePropertyPlaceholders()
-                .build();
-        assertThat(settings.get("setting1"), is(nullValue()));
+    public void testReplacePropertiesPlaceholderByEnvironmentVariables() {
+        final Map.Entry<String, String> entry = randomSubsetOf(1, System.getenv().entrySet()).get(0);
+        assertNotNull(entry.getValue());
+
+        final Settings implicitEnvSettings = Settings.builder()
+            .put("setting1", "${" + entry.getKey() + "}")
+            .replacePropertyPlaceholders()
+            .build();
+        assertThat(implicitEnvSettings.get("setting1"), equalTo(entry.getValue()));
     }
 
     public void testReplacePropertiesPlaceholderIgnoresPrompt() {

+ 5 - 7
core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java

@@ -19,11 +19,6 @@
 
 package org.elasticsearch.node.internal;
 
-import java.io.IOException;
-import java.io.InputStream;
-import java.nio.file.Files;
-import java.nio.file.Path;
-
 import org.elasticsearch.cli.MockTerminal;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.common.settings.Settings;
@@ -33,6 +28,11 @@ import org.elasticsearch.test.ESTestCase;
 import org.junit.After;
 import org.junit.Before;
 
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 
@@ -134,7 +134,6 @@ public class InternalSettingsPreparerTests extends ESTestCase {
             Files.createDirectory(config);
             Files.copy(garbage, config.resolve("elasticsearch.yml"));
             InternalSettingsPreparer.prepareEnvironment(Settings.builder()
-                .put("config.ignore_system_properties", true)
                 .put(baseEnvSettings)
                 .build(), null);
         } catch (SettingsException e) {
@@ -153,7 +152,6 @@ public class InternalSettingsPreparerTests extends ESTestCase {
 
         try {
             InternalSettingsPreparer.prepareEnvironment(Settings.builder()
-                .put("config.ignore_system_properties", true)
                 .put(baseEnvSettings)
                 .build(), null);
         } catch (SettingsException e) {

+ 4 - 3
core/src/test/resources/org/elasticsearch/common/logging/config/logging.yml

@@ -1,6 +1,7 @@
-# you can override this using by setting a system property, for example -Ees.logger.level=DEBUG
-es.logger.level: INFO
-rootLogger: ${es.logger.level}, console
+# you can override using a command-line parameter
+# -E logger.level=(ERROR|WARN|INFO|DEBUG|TRACE)
+logger.level: INFO
+rootLogger: ${logger.level}, console
 logger:
   test: TRACE, console
 

+ 1 - 1
dev-tools/smoke_test_rc.py

@@ -203,7 +203,7 @@ def smoke_test_release(release, files, expected_hash, plugins):
       headers = {}
     print('  Starting elasticsearch deamon from [%s]' % es_dir)
     try:
-      run('%s; %s -Ees.node.name=smoke_tester -Ees.cluster.name=prepare_release -Ees.script.inline=true -Ees.script.stored=true -Ees.repositories.url.allowed_urls=http://snapshot.test* %s -Ees.pidfile=%s -Ees.node.portsfile=true'
+      run('%s; %s -Enode.name=smoke_tester -Ecluster.name=prepare_release -Escript.inline=true -Escript.stored=true -Erepositories.url.allowed_urls=http://snapshot.test* %s -Epidfile=%s -Enode.portsfile=true'
           % (java_exe(), es_run_path, '-d', os.path.join(es_dir, 'es-smoke.pid')))
       if not wait_for_node_startup(es_dir, header=headers):
         print("elasticsearch logs:")

+ 1 - 1
distribution/deb/src/main/packaging/init.d/elasticsearch

@@ -79,7 +79,7 @@ fi
 # Define other required variables
 PID_FILE="$PID_DIR/$NAME.pid"
 DAEMON=$ES_HOME/bin/elasticsearch
-DAEMON_OPTS="-d -p $PID_FILE -Ees.default.path.logs=$LOG_DIR -Ees.default.path.data=$DATA_DIR -Ees.default.path.conf=$CONF_DIR"
+DAEMON_OPTS="-d -p $PID_FILE -Edefault.path.logs=$LOG_DIR -Edefault.path.data=$DATA_DIR -Edefault.path.conf=$CONF_DIR"
 
 export ES_JAVA_OPTS
 export JAVA_HOME

+ 1 - 1
distribution/rpm/src/main/packaging/init.d/elasticsearch

@@ -114,7 +114,7 @@ start() {
     cd $ES_HOME
     echo -n $"Starting $prog: "
     # if not running, start it up here, usually something like "daemon $exec"
-    daemon --user $ES_USER --pidfile $pidfile $exec -p $pidfile -d -Ees.default.path.home=$ES_HOME -Ees.default.path.logs=$LOG_DIR -Ees.default.path.data=$DATA_DIR -Ees.default.path.conf=$CONF_DIR
+    daemon --user $ES_USER --pidfile $pidfile $exec -p $pidfile -d -Edefault.path.logs=$LOG_DIR -Edefault.path.data=$DATA_DIR -Edefault.path.conf=$CONF_DIR
     retval=$?
     echo
     [ $retval -eq 0 ] && touch $lockfile

+ 3 - 3
distribution/src/main/packaging/systemd/elasticsearch.service

@@ -21,9 +21,9 @@ ExecStartPre=/usr/share/elasticsearch/bin/elasticsearch-systemd-pre-exec
 
 ExecStart=/usr/share/elasticsearch/bin/elasticsearch \
                                                 -p ${PID_DIR}/elasticsearch.pid \
-                                                -Ees.default.path.logs=${LOG_DIR} \
-                                                -Ees.default.path.data=${DATA_DIR} \
-                                                -Ees.default.path.conf=${CONF_DIR}
+                                                -Edefault.path.logs=${LOG_DIR} \
+                                                -Edefault.path.data=${DATA_DIR} \
+                                                -Edefault.path.conf=${CONF_DIR}
 
 StandardOutput=journal
 StandardError=inherit

+ 3 - 3
distribution/src/main/resources/bin/elasticsearch-plugin

@@ -81,10 +81,10 @@ fi
 HOSTNAME=`hostname | cut -d. -f1`
 export HOSTNAME
 
-declare -a properties=(-Delasticsearch -Des.path.home="$ES_HOME")
+declare -a args=("$@")
 
 if [ -e "$CONF_DIR" ]; then
-  properties=("${properties[@]}" -Des.default.path.conf="$CONF_DIR")
+  args=("${args[@]}" -Edefault.path.conf="$CONF_DIR")
 fi
 
-exec "$JAVA" $ES_JAVA_OPTS "${properties[@]}" -cp "$ES_HOME/lib/*" org.elasticsearch.plugins.PluginCli "$@"
+exec "$JAVA" $ES_JAVA_OPTS -Delasticsearch -Des.path.home="$ES_HOME" -cp "$ES_HOME/lib/*" org.elasticsearch.plugins.PluginCli "${args[@]}"

+ 4 - 3
distribution/src/main/resources/config/logging.yml

@@ -1,6 +1,7 @@
-# you can override this using by setting a system property, for example -Ees.logger.level=DEBUG
-es.logger.level: INFO
-rootLogger: ${es.logger.level}, console, file
+# you can override using a command-line parameter
+# -E logger.level=(ERROR|WARN|INFO|DEBUG|TRACE)
+logger.level: INFO
+rootLogger: ${logger.level}, console, file
 logger:
   # log action execution errors for easier debugging
   action: DEBUG

+ 1 - 1
docs/plugins/plugin-script.asciidoc

@@ -135,7 +135,7 @@ can do this as follows:
 
 [source,sh]
 ---------------------
-sudo bin/elasticsearch-plugin -Ees.path.conf=/path/to/custom/config/dir install <plugin name>
+sudo bin/elasticsearch-plugin -Epath.conf=/path/to/custom/config/dir install <plugin name>
 ---------------------
 
 You can also set the `CONF_DIR` environment variable to the custom config

+ 1 - 1
docs/reference/getting-started.asciidoc

@@ -163,7 +163,7 @@ As mentioned previously, we can override either the cluster or node name. This c
 
 [source,sh]
 --------------------------------------------------
-./elasticsearch -Ees.cluster.name=my_cluster_name -Ees.node.name=my_node_name
+./elasticsearch -Ecluster.name=my_cluster_name -Enode.name=my_node_name
 --------------------------------------------------
 
 Also note the line marked http with information about the HTTP address (`192.168.8.112`) and port (`9200`) that our node is reachable from. By default, Elasticsearch uses port `9200` to provide access to its REST API. This port is configurable if necessary.

+ 1 - 1
docs/reference/index-modules/allocation/filtering.asciidoc

@@ -14,7 +14,7 @@ attribute as follows:
 
 [source,sh]
 ------------------------
-bin/elasticsearch -Ees.node.attr.rack=rack1 -Ees.node.attr.size=big  <1>
+bin/elasticsearch -Enode.attr.rack=rack1 -Enode.attr.size=big  <1>
 ------------------------
 <1> These attribute settings can also be specified in the `elasticsearch.yml` config file.
 

+ 12 - 0
docs/reference/migration/migrate_5_0/packaging.asciidoc

@@ -43,3 +43,15 @@ Previously, the scripts used to start Elasticsearch and run plugin
 commands only required a Bourne-compatible shell. Starting in
 Elasticsearch 5.0.0, the bash shell is now required and `/bin/bash` is a
 hard-dependency for the RPM and Debian packages.
+
+==== Environmental Settings
+
+Previously, Elasticsearch could be configured via environment variables
+in two ways: first by using the placeholder syntax
+`${env.ENV_VAR_NAME}` and the second by using the same syntax without
+the `env` prefix: `${ENV_VAR_NAME}`. The first method has been removed
+from Elasticsearch.
+
+Additionally, it was previously possible to set any setting in
+Elasticsearch via JVM system properties. This has been removed from
+Elasticsearch.

+ 3 - 8
docs/reference/migration/migrate_5_0/settings.asciidoc

@@ -202,19 +202,14 @@ the cache implementation used for the request cache and the field data cache.
 
 ==== Using system properties to configure Elasticsearch
 
-Elasticsearch can be configured by setting system properties on the
-command line via `-Des.name.of.property=value.of.property`. This will be
-removed in a future version of Elasticsearch. Instead, use
-`-E es.name.of.setting=value.of.setting`. Note that in all cases the
-name of the setting must be prefixed with `es.`.
+Elasticsearch can no longer be configured by setting system properties.
+Instead, use `-Ename.of.setting=value.of.setting`.
 
 ==== Removed using double-dashes to configure Elasticsearch
 
 Elasticsearch could previously be configured on the command line by
 setting settings via `--name.of.setting value.of.setting`. This feature
-has been removed. Instead, use
-`-Ees.name.of.setting=value.of.setting`. Note that in all cases the
-name of the setting must be prefixed with `es.`.
+has been removed. Instead, use `-Ename.of.setting=value.of.setting`.
 
 ==== Discovery Settings
 

+ 1 - 1
docs/reference/modules/cluster/allocation_awareness.asciidoc

@@ -21,7 +21,7 @@ attribute called `rack_id` -- we could use any attribute name.  For example:
 
 [source,sh]
 ----------------------
-./bin/elasticsearch -Ees.node.attr.rack_id=rack_one <1>
+./bin/elasticsearch -Enode.attr.rack_id=rack_one <1>
 ----------------------
 <1> This setting could also be specified in the `elasticsearch.yml` config file.
 

+ 1 - 1
docs/reference/modules/node.asciidoc

@@ -265,7 +265,7 @@ Like all node settings, it can also be specified on the command line as:
 
 [source,sh]
 -----------------------
-./bin/elasticsearch -Ees.path.data=/var/elasticsearch/data
+./bin/elasticsearch -Epath.data=/var/elasticsearch/data
 -----------------------
 
 TIP: When using the `.zip` or `.tar.gz` distributions, the `path.data` setting

+ 4 - 5
docs/reference/setup/configuration.asciidoc

@@ -26,7 +26,7 @@ setting, as follows:
 
 [source,sh]
 -------------------------------
-./bin/elasticsearch -E es.path.conf=/path/to/my/config/
+./bin/elasticsearch -Epath.conf=/path/to/my/config/
 -------------------------------
 
 [float]
@@ -93,15 +93,14 @@ is used in the settings and the process is run as a service or in the background
 === Setting default settings
 
 New default settings may be specified on the command line using the
-`es.default.` prefix instead of the `es.` prefix.  This will specify a value
-that will be used by default unless another value is specified in the config
-file.
+`default.` prefix.  This will specify a value that will be used by
+default unless another value is specified in the config file.
 
 For instance, if Elasticsearch is started as follows:
 
 [source,sh]
 ---------------------------
-./bin/elasticsearch -E es.default.node.name=My_Node
+./bin/elasticsearch -Edefault.node.name=My_Node
 ---------------------------
 
 the value for `node.name` will be `My_Node`, unless it is overwritten on the

+ 3 - 4
docs/reference/setup/install/windows.asciidoc

@@ -45,15 +45,14 @@ file by default.  The format of this config file is explained in
 <<settings>>.
 
 Any settings that can be specified in the config file can also be specified on
-the command line, using the `-E` syntax, and prepending `es.` to the setting
-name, as follows:
+the command line, using the `-E` syntax as follows:
 
 [source,sh]
 --------------------------------------------
-./bin/elasticsearch -E es.cluster.name=my_cluster -E es.node.name=node_1
+./bin/elasticsearch -Ecluster.name=my_cluster -Enode.name=node_1
 --------------------------------------------
 
-NOTE: Values that contain spaces must be surrounded with quotes.  For instance `-E es.path.logs="C:\My Logs\logs"`.
+NOTE: Values that contain spaces must be surrounded with quotes.  For instance `-Epath.logs="C:\My Logs\logs"`.
 
 TIP: Typically, any cluster-wide settings (like `cluster.name`) should be
 added to the `elasticsearch.yml` config file, while any node-specific settings

+ 1 - 1
docs/reference/setup/install/zip-targz.asciidoc

@@ -93,7 +93,7 @@ name, as follows:
 
 [source,sh]
 --------------------------------------------
-./bin/elasticsearch -d -E es.cluster.name=my_cluster -E es.node.name=node_1
+./bin/elasticsearch -d -Ecluster.name=my_cluster -Enode.name=node_1
 --------------------------------------------
 
 TIP: Typically, any cluster-wide settings (like `cluster.name`) should be

+ 62 - 0
qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java

@@ -0,0 +1,62 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.bootstrap;
+
+import org.elasticsearch.cli.ExitCodes;
+import org.elasticsearch.common.SuppressForbidden;
+import org.elasticsearch.test.ESTestCase;
+
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.Matchers.hasEntry;
+
+public class EvilElasticsearchCliTests extends ESElasticsearchCliTestCase {
+
+    @SuppressForbidden(reason = "manipulates system properties for testing")
+    public void testPathHome() throws Exception {
+        final String pathHome = System.getProperty("es.path.home");
+        final String value = randomAsciiOfLength(16);
+        System.setProperty("es.path.home", value);
+
+        runTest(
+                ExitCodes.OK,
+                true,
+                output -> {},
+                (foreground, pidFile, esSettings) -> {
+                    assertThat(esSettings.size(), equalTo(1));
+                    assertThat(esSettings, hasEntry("path.home", value));
+                });
+
+        System.clearProperty("es.path.home");
+        final String commandLineValue = randomAsciiOfLength(16);
+        runTest(
+                ExitCodes.OK,
+                true,
+                output -> {},
+                (foreground, pidFile, esSettings) -> {
+                    assertThat(esSettings.size(), equalTo(1));
+                    assertThat(esSettings, hasEntry("path.home", commandLineValue));
+                },
+                "-Epath.home=" + commandLineValue);
+
+        if (pathHome != null) System.setProperty("es.path.home", pathHome);
+        else System.clearProperty("es.path.home");
+    }
+
+}

+ 86 - 109
qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java

@@ -27,6 +27,7 @@ import org.apache.lucene.util.SuppressForbidden;
 import org.elasticsearch.Version;
 import org.elasticsearch.cli.MockTerminal;
 import org.elasticsearch.cli.UserError;
+import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.io.PathUtilsForTesting;
 import org.elasticsearch.common.settings.Settings;
@@ -54,8 +55,10 @@ import java.nio.file.attribute.PosixFileAttributeView;
 import java.nio.file.attribute.PosixFileAttributes;
 import java.nio.file.attribute.PosixFilePermission;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.function.Supplier;
@@ -129,7 +132,7 @@ public class InstallPluginCommandTests extends ESTestCase {
     }
 
     /** Creates a test environment with bin, config and plugins directories. */
-    static Environment createEnv(FileSystem fs, Function<String, Path> temp) throws IOException {
+    static Tuple<Path, Environment> createEnv(FileSystem fs, Function<String, Path> temp) throws IOException {
         Path home = temp.apply("install-plugin-command-tests");
         Files.createDirectories(home.resolve("bin"));
         Files.createFile(home.resolve("bin").resolve("elasticsearch"));
@@ -140,7 +143,7 @@ public class InstallPluginCommandTests extends ESTestCase {
         Settings settings = Settings.builder()
             .put("path.home", home)
             .build();
-        return new Environment(settings);
+        return Tuple.tuple(home, new Environment(settings));
     }
 
     static Path createPluginDir(Function<String, Path> temp) throws IOException {
@@ -185,20 +188,22 @@ public class InstallPluginCommandTests extends ESTestCase {
         return writeZip(structure, "elasticsearch");
     }
 
-    static MockTerminal installPlugin(String pluginUrl, Environment env) throws Exception {
-        return installPlugin(pluginUrl, env, false);
+    static MockTerminal installPlugin(String pluginUrl, Path home) throws Exception {
+        return installPlugin(pluginUrl, home, false);
     }
 
-    static MockTerminal installPlugin(String pluginUrl, Environment env, boolean jarHellCheck) throws Exception {
+    static MockTerminal installPlugin(String pluginUrl, Path home, boolean jarHellCheck) throws Exception {
+        Map<String, String> settings = new HashMap<>();
+        settings.put("path.home", home.toString());
         MockTerminal terminal = new MockTerminal();
-        new InstallPluginCommand(env) {
+        new InstallPluginCommand() {
             @Override
             void jarHellCheck(Path candidate, Path pluginsDir) throws Exception {
                 if (jarHellCheck) {
                     super.jarHellCheck(candidate, pluginsDir);
                 }
             }
-        }.execute(terminal, pluginUrl, true);
+        }.execute(terminal, pluginUrl, true, settings);
         return terminal;
     }
 
@@ -275,192 +280,176 @@ public class InstallPluginCommandTests extends ESTestCase {
     }
 
     public void testSomethingWorks() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
         String pluginZip = createPlugin("fake", pluginDir);
-        installPlugin(pluginZip, env);
-        assertPlugin("fake", pluginDir, env);
+        installPlugin(pluginZip, env.v1());
+        assertPlugin("fake", pluginDir, env.v2());
     }
 
     public void testSpaceInUrl() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
         String pluginZip = createPlugin("fake", pluginDir);
         Path pluginZipWithSpaces = createTempFile("foo bar", ".zip");
         try (InputStream in = new URL(pluginZip).openStream()) {
             Files.copy(in, pluginZipWithSpaces, StandardCopyOption.REPLACE_EXISTING);
         }
-        installPlugin(pluginZipWithSpaces.toUri().toURL().toString(), env);
-        assertPlugin("fake", pluginDir, env);
+        installPlugin(pluginZipWithSpaces.toUri().toURL().toString(), env.v1());
+        assertPlugin("fake", pluginDir, env.v2());
     }
 
     public void testMalformedUrlNotMaven() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         // has two colons, so it appears similar to maven coordinates
-        MalformedURLException e = expectThrows(MalformedURLException.class, () -> {
-            installPlugin("://host:1234", env);
-        });
+        MalformedURLException e = expectThrows(MalformedURLException.class, () -> installPlugin("://host:1234", env.v1()));
         assertTrue(e.getMessage(), e.getMessage().contains("no protocol"));
     }
 
     public void testPluginsDirMissing() throws Exception {
-        Environment env = createEnv(fs, temp);
-        Files.delete(env.pluginsFile());
+        Tuple<Path, Environment> env = createEnv(fs, temp);
+        Files.delete(env.v2().pluginsFile());
         Path pluginDir = createPluginDir(temp);
         String pluginZip = createPlugin("fake", pluginDir);
-        installPlugin(pluginZip, env);
-        assertPlugin("fake", pluginDir, env);
+        installPlugin(pluginZip, env.v1());
+        assertPlugin("fake", pluginDir, env.v2());
     }
 
     public void testPluginsDirReadOnly() throws Exception {
         assumeTrue("posix and filesystem", isPosix && isReal);
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
-        try (PosixPermissionsResetter pluginsAttrs = new PosixPermissionsResetter(env.pluginsFile())) {
+        try (PosixPermissionsResetter pluginsAttrs = new PosixPermissionsResetter(env.v2().pluginsFile())) {
             pluginsAttrs.setPermissions(new HashSet<>());
             String pluginZip = createPlugin("fake", pluginDir);
-            IOException e = expectThrows(IOException.class, () -> {
-                installPlugin(pluginZip, env);
-            });
-            assertTrue(e.getMessage(), e.getMessage().contains(env.pluginsFile().toString()));
+            IOException e = expectThrows(IOException.class, () -> installPlugin(pluginZip, env.v1()));
+            assertTrue(e.getMessage(), e.getMessage().contains(env.v2().pluginsFile().toString()));
         }
-        assertInstallCleaned(env);
+        assertInstallCleaned(env.v2());
     }
 
     public void testBuiltinModule() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
         String pluginZip = createPlugin("lang-groovy", pluginDir);
-        UserError e = expectThrows(UserError.class, () -> {
-            installPlugin(pluginZip, env);
-        });
+        UserError e = expectThrows(UserError.class, () -> installPlugin(pluginZip, env.v1()));
         assertTrue(e.getMessage(), e.getMessage().contains("is a system module"));
-        assertInstallCleaned(env);
+        assertInstallCleaned(env.v2());
     }
 
     public void testJarHell() throws Exception {
         // jar hell test needs a real filesystem
         assumeTrue("real filesystem", isReal);
-        Environment environment = createEnv(fs, temp);
+        Tuple<Path, Environment> environment = createEnv(fs, temp);
         Path pluginDirectory = createPluginDir(temp);
         writeJar(pluginDirectory.resolve("other.jar"), "FakePlugin");
         String pluginZip = createPlugin("fake", pluginDirectory); // adds plugin.jar with FakePlugin
-        IllegalStateException e = expectThrows(IllegalStateException.class, () -> {
-            installPlugin(pluginZip, environment, true);
-        });
+        IllegalStateException e = expectThrows(IllegalStateException.class, () -> installPlugin(pluginZip, environment.v1(), true));
         assertTrue(e.getMessage(), e.getMessage().contains("jar hell"));
-        assertInstallCleaned(environment);
+        assertInstallCleaned(environment.v2());
     }
 
     public void testIsolatedPlugins() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         // these both share the same FakePlugin class
         Path pluginDir1 = createPluginDir(temp);
         String pluginZip1 = createPlugin("fake1", pluginDir1);
-        installPlugin(pluginZip1, env);
+        installPlugin(pluginZip1, env.v1());
         Path pluginDir2 = createPluginDir(temp);
         String pluginZip2 = createPlugin("fake2", pluginDir2);
-        installPlugin(pluginZip2, env);
-        assertPlugin("fake1", pluginDir1, env);
-        assertPlugin("fake2", pluginDir2, env);
+        installPlugin(pluginZip2, env.v1());
+        assertPlugin("fake1", pluginDir1, env.v2());
+        assertPlugin("fake2", pluginDir2, env.v2());
     }
 
     public void testExistingPlugin() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
         String pluginZip = createPlugin("fake", pluginDir);
-        installPlugin(pluginZip, env);
-        UserError e = expectThrows(UserError.class, () -> {
-            installPlugin(pluginZip, env);
-        });
+        installPlugin(pluginZip, env.v1());
+        UserError e = expectThrows(UserError.class, () -> installPlugin(pluginZip, env.v1()));
         assertTrue(e.getMessage(), e.getMessage().contains("already exists"));
-        assertInstallCleaned(env);
+        assertInstallCleaned(env.v2());
     }
 
     public void testBin() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
         Path binDir = pluginDir.resolve("bin");
         Files.createDirectory(binDir);
         Files.createFile(binDir.resolve("somescript"));
         String pluginZip = createPlugin("fake", pluginDir);
-        installPlugin(pluginZip, env);
-        assertPlugin("fake", pluginDir, env);
+        installPlugin(pluginZip, env.v1());
+        assertPlugin("fake", pluginDir, env.v2());
     }
 
     public void testBinNotDir() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
         Path binDir = pluginDir.resolve("bin");
         Files.createFile(binDir);
         String pluginZip = createPlugin("fake", pluginDir);
-        UserError e = expectThrows(UserError.class, () -> {
-            installPlugin(pluginZip, env);
-        });
+        UserError e = expectThrows(UserError.class, () -> installPlugin(pluginZip, env.v1()));
         assertTrue(e.getMessage(), e.getMessage().contains("not a directory"));
-        assertInstallCleaned(env);
+        assertInstallCleaned(env.v2());
     }
 
     public void testBinContainsDir() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
         Path dirInBinDir = pluginDir.resolve("bin").resolve("foo");
         Files.createDirectories(dirInBinDir);
         Files.createFile(dirInBinDir.resolve("somescript"));
         String pluginZip = createPlugin("fake", pluginDir);
-        UserError e = expectThrows(UserError.class, () -> {
-            installPlugin(pluginZip, env);
-        });
+        UserError e = expectThrows(UserError.class, () -> installPlugin(pluginZip, env.v1()));
         assertTrue(e.getMessage(), e.getMessage().contains("Directories not allowed in bin dir for plugin"));
-        assertInstallCleaned(env);
+        assertInstallCleaned(env.v2());
     }
 
     public void testBinConflict() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
         Path binDir = pluginDir.resolve("bin");
         Files.createDirectory(binDir);
         Files.createFile(binDir.resolve("somescript"));
         String pluginZip = createPlugin("elasticsearch", pluginDir);
-        FileAlreadyExistsException e = expectThrows(FileAlreadyExistsException.class, () -> {
-            installPlugin(pluginZip, env);
-        });
-        assertTrue(e.getMessage(), e.getMessage().contains(env.binFile().resolve("elasticsearch").toString()));
-        assertInstallCleaned(env);
+        FileAlreadyExistsException e = expectThrows(FileAlreadyExistsException.class, () -> installPlugin(pluginZip, env.v1()));
+        assertTrue(e.getMessage(), e.getMessage().contains(env.v2().binFile().resolve("elasticsearch").toString()));
+        assertInstallCleaned(env.v2());
     }
 
     public void testBinPermissions() throws Exception {
         assumeTrue("posix filesystem", isPosix);
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
         Path binDir = pluginDir.resolve("bin");
         Files.createDirectory(binDir);
         Files.createFile(binDir.resolve("somescript"));
         String pluginZip = createPlugin("fake", pluginDir);
-        try (PosixPermissionsResetter binAttrs = new PosixPermissionsResetter(env.binFile())) {
+        try (PosixPermissionsResetter binAttrs = new PosixPermissionsResetter(env.v2().binFile())) {
             Set<PosixFilePermission> perms = binAttrs.getCopyPermissions();
             // make sure at least one execute perm is missing, so we know we forced it during installation
             perms.remove(PosixFilePermission.GROUP_EXECUTE);
             binAttrs.setPermissions(perms);
-            installPlugin(pluginZip, env);
-            assertPlugin("fake", pluginDir, env);
+            installPlugin(pluginZip, env.v1());
+            assertPlugin("fake", pluginDir, env.v2());
         }
     }
 
     public void testConfig() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
         Path configDir = pluginDir.resolve("config");
         Files.createDirectory(configDir);
         Files.createFile(configDir.resolve("custom.yaml"));
         String pluginZip = createPlugin("fake", pluginDir);
-        installPlugin(pluginZip, env);
-        assertPlugin("fake", pluginDir, env);
+        installPlugin(pluginZip, env.v1());
+        assertPlugin("fake", pluginDir, env.v2());
     }
 
     public void testExistingConfig() throws Exception {
-        Environment env = createEnv(fs, temp);
-        Path envConfigDir = env.configFile().resolve("fake");
+        Tuple<Path, Environment> env = createEnv(fs, temp);
+        Path envConfigDir = env.v2().configFile().resolve("fake");
         Files.createDirectories(envConfigDir);
         Files.write(envConfigDir.resolve("custom.yaml"), "existing config".getBytes(StandardCharsets.UTF_8));
         Path pluginDir = createPluginDir(temp);
@@ -469,8 +458,8 @@ public class InstallPluginCommandTests extends ESTestCase {
         Files.write(configDir.resolve("custom.yaml"), "new config".getBytes(StandardCharsets.UTF_8));
         Files.createFile(configDir.resolve("other.yaml"));
         String pluginZip = createPlugin("fake", pluginDir);
-        installPlugin(pluginZip, env);
-        assertPlugin("fake", pluginDir, env);
+        installPlugin(pluginZip, env.v1());
+        assertPlugin("fake", pluginDir, env.v2());
         List<String> configLines = Files.readAllLines(envConfigDir.resolve("custom.yaml"), StandardCharsets.UTF_8);
         assertEquals(1, configLines.size());
         assertEquals("existing config", configLines.get(0));
@@ -478,80 +467,68 @@ public class InstallPluginCommandTests extends ESTestCase {
     }
 
     public void testConfigNotDir() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
         Path configDir = pluginDir.resolve("config");
         Files.createFile(configDir);
         String pluginZip = createPlugin("fake", pluginDir);
-        UserError e = expectThrows(UserError.class, () -> {
-            installPlugin(pluginZip, env);
-        });
+        UserError e = expectThrows(UserError.class, () -> installPlugin(pluginZip, env.v1()));
         assertTrue(e.getMessage(), e.getMessage().contains("not a directory"));
-        assertInstallCleaned(env);
+        assertInstallCleaned(env.v2());
     }
 
     public void testConfigContainsDir() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
         Path dirInConfigDir = pluginDir.resolve("config").resolve("foo");
         Files.createDirectories(dirInConfigDir);
         Files.createFile(dirInConfigDir.resolve("myconfig.yml"));
         String pluginZip = createPlugin("fake", pluginDir);
-        UserError e = expectThrows(UserError.class, () -> {
-            installPlugin(pluginZip, env);
-        });
+        UserError e = expectThrows(UserError.class, () -> installPlugin(pluginZip, env.v1()));
         assertTrue(e.getMessage(), e.getMessage().contains("Directories not allowed in config dir for plugin"));
-        assertInstallCleaned(env);
+        assertInstallCleaned(env.v2());
     }
 
     public void testConfigConflict() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
         Path configDir = pluginDir.resolve("config");
         Files.createDirectory(configDir);
         Files.createFile(configDir.resolve("myconfig.yml"));
         String pluginZip = createPlugin("elasticsearch.yml", pluginDir);
-        FileAlreadyExistsException e = expectThrows(FileAlreadyExistsException.class, () -> {
-            installPlugin(pluginZip, env);
-        });
-        assertTrue(e.getMessage(), e.getMessage().contains(env.configFile().resolve("elasticsearch.yml").toString()));
-        assertInstallCleaned(env);
+        FileAlreadyExistsException e = expectThrows(FileAlreadyExistsException.class, () -> installPlugin(pluginZip, env.v1()));
+        assertTrue(e.getMessage(), e.getMessage().contains(env.v2().configFile().resolve("elasticsearch.yml").toString()));
+        assertInstallCleaned(env.v2());
     }
 
     public void testMissingDescriptor() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
         Files.createFile(pluginDir.resolve("fake.yml"));
         String pluginZip = writeZip(pluginDir, "elasticsearch");
-        NoSuchFileException e = expectThrows(NoSuchFileException.class, () -> {
-            installPlugin(pluginZip, env);
-        });
+        NoSuchFileException e = expectThrows(NoSuchFileException.class, () -> installPlugin(pluginZip, env.v1()));
         assertTrue(e.getMessage(), e.getMessage().contains("plugin-descriptor.properties"));
-        assertInstallCleaned(env);
+        assertInstallCleaned(env.v2());
     }
 
     public void testMissingDirectory() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
         Files.createFile(pluginDir.resolve(PluginInfo.ES_PLUGIN_PROPERTIES));
         String pluginZip = writeZip(pluginDir, null);
-        UserError e = expectThrows(UserError.class, () -> {
-            installPlugin(pluginZip, env);
-        });
+        UserError e = expectThrows(UserError.class, () -> installPlugin(pluginZip, env.v1()));
         assertTrue(e.getMessage(), e.getMessage().contains("`elasticsearch` directory is missing in the plugin zip"));
-        assertInstallCleaned(env);
+        assertInstallCleaned(env.v2());
     }
 
     public void testZipRelativeOutsideEntryName() throws Exception {
-        Environment env = createEnv(fs, temp);
+        Tuple<Path, Environment> env = createEnv(fs, temp);
         Path zip = createTempDir().resolve("broken.zip");
         try (ZipOutputStream stream = new ZipOutputStream(Files.newOutputStream(zip))) {
             stream.putNextEntry(new ZipEntry("elasticsearch/../blah"));
         }
         String pluginZip = zip.toUri().toURL().toString();
-        IOException e = expectThrows(IOException.class, () -> {
-            installPlugin(pluginZip, env);
-        });
+        IOException e = expectThrows(IOException.class, () -> installPlugin(pluginZip, env.v1()));
         assertTrue(e.getMessage(), e.getMessage().contains("resolving outside of plugin directory"));
     }
 

+ 30 - 28
qa/evil-tests/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java

@@ -25,35 +25,47 @@ import java.nio.file.Files;
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.stream.Collectors;
 
 import org.apache.lucene.util.LuceneTestCase;
 import org.elasticsearch.cli.ExitCodes;
 import org.elasticsearch.cli.MockTerminal;
+import org.elasticsearch.common.inject.spi.HasDependencies;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.Version;
+import org.junit.Before;
 
 @LuceneTestCase.SuppressFileSystems("*")
 public class ListPluginsCommandTests extends ESTestCase {
 
-    Environment createEnv() throws IOException {
-        Path home = createTempDir();
+    private Path home;
+    private Environment env;
+
+    @Before
+    public void setUp() throws Exception {
+        super.setUp();
+        home = createTempDir();
         Files.createDirectories(home.resolve("plugins"));
         Settings settings = Settings.builder()
-            .put("path.home", home)
-            .build();
-        return new Environment(settings);
+                .put("path.home", home)
+                .build();
+        env = new Environment(settings);
     }
 
-    static MockTerminal listPlugins(Environment env) throws Exception {
-        return listPlugins(env, new String[0]);
+    static MockTerminal listPlugins(Path home) throws Exception {
+        return listPlugins(home, new String[0]);
     }
     
-    static MockTerminal listPlugins(Environment env, String[] args) throws Exception {
+    static MockTerminal listPlugins(Path home, String[] args) throws Exception {
+        String[] argsAndHome = new String[args.length + 1];
+        System.arraycopy(args, 0, argsAndHome, 0, args.length);
+        argsAndHome[args.length] = "-Epath.home=" + home;
         MockTerminal terminal = new MockTerminal();
-        int status = new ListPluginsCommand(env).main(args, terminal);
+        int status = new ListPluginsCommand().main(argsAndHome, terminal);
         assertEquals(ExitCodes.OK, status);
         return terminal;
     }
@@ -74,49 +86,42 @@ public class ListPluginsCommandTests extends ESTestCase {
 
 
     public void testPluginsDirMissing() throws Exception {
-        Environment env = createEnv();
         Files.delete(env.pluginsFile());
-        IOException e = expectThrows(IOException.class, () -> {
-           listPlugins(env);
-        });
+        IOException e = expectThrows(IOException.class, () -> listPlugins(home));
         assertEquals(e.getMessage(), "Plugins directory missing: " + env.pluginsFile());
     }
 
     public void testNoPlugins() throws Exception {
-        MockTerminal terminal = listPlugins(createEnv());
+        MockTerminal terminal = listPlugins(home);
         assertTrue(terminal.getOutput(), terminal.getOutput().isEmpty());
     }
 
     public void testOnePlugin() throws Exception {
-        Environment env = createEnv();
         buildFakePlugin(env, "fake desc", "fake", "org.fake");
-        MockTerminal terminal = listPlugins(env);
+        MockTerminal terminal = listPlugins(home);
         assertEquals(terminal.getOutput(), buildMultiline("fake"));
     }
 
     public void testTwoPlugins() throws Exception {
-        Environment env = createEnv();
         buildFakePlugin(env, "fake desc", "fake1", "org.fake");
         buildFakePlugin(env, "fake desc 2", "fake2", "org.fake");
-        MockTerminal terminal = listPlugins(env);
+        MockTerminal terminal = listPlugins(home);
         assertEquals(terminal.getOutput(), buildMultiline("fake1", "fake2"));
     }
     
     public void testPluginWithVerbose() throws Exception {
-        Environment env = createEnv();
         buildFakePlugin(env, "fake desc", "fake_plugin", "org.fake");
         String[] params = { "-v" };
-        MockTerminal terminal = listPlugins(env, params);
+        MockTerminal terminal = listPlugins(home, params);
         assertEquals(terminal.getOutput(), buildMultiline("Plugins directory: " + env.pluginsFile(), "fake_plugin",
                 "- Plugin information:", "Name: fake_plugin", "Description: fake desc", "Version: 1.0", " * Classname: org.fake"));
     }
     
     public void testPluginWithVerboseMultiplePlugins() throws Exception {
-        Environment env = createEnv();
         buildFakePlugin(env, "fake desc 1", "fake_plugin1", "org.fake");
         buildFakePlugin(env, "fake desc 2", "fake_plugin2", "org.fake2");
         String[] params = { "-v" };
-        MockTerminal terminal = listPlugins(env, params);
+        MockTerminal terminal = listPlugins(home, params);
         assertEquals(terminal.getOutput(), buildMultiline("Plugins directory: " + env.pluginsFile(),
                 "fake_plugin1", "- Plugin information:", "Name: fake_plugin1", "Description: fake desc 1", "Version: 1.0",
                 " * Classname: org.fake", "fake_plugin2", "- Plugin information:", "Name: fake_plugin2",
@@ -124,26 +129,23 @@ public class ListPluginsCommandTests extends ESTestCase {
     }
         
     public void testPluginWithoutVerboseMultiplePlugins() throws Exception {
-        Environment env = createEnv();
         buildFakePlugin(env, "fake desc 1", "fake_plugin1", "org.fake");
         buildFakePlugin(env, "fake desc 2", "fake_plugin2", "org.fake2");
-        MockTerminal terminal = listPlugins(env, new String[0]);
+        MockTerminal terminal = listPlugins(home, new String[0]);
         String output = terminal.getOutput();
         assertEquals(output, buildMultiline("fake_plugin1", "fake_plugin2"));
     }
     
     public void testPluginWithoutDescriptorFile() throws Exception{
-        Environment env = createEnv();
         Files.createDirectories(env.pluginsFile().resolve("fake1"));
-        NoSuchFileException e = expectThrows(NoSuchFileException.class, () -> listPlugins(env));
+        NoSuchFileException e = expectThrows(NoSuchFileException.class, () -> listPlugins(home));
         assertEquals(e.getFile(), env.pluginsFile().resolve("fake1").resolve(PluginInfo.ES_PLUGIN_PROPERTIES).toString());
     }
     
     public void testPluginWithWrongDescriptorFile() throws Exception{
-        Environment env = createEnv();        
         PluginTestUtil.writeProperties(env.pluginsFile().resolve("fake1"),
                 "description", "fake desc");
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> listPlugins(env));
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> listPlugins(home));
         assertEquals(e.getMessage(), "Property [name] is missing in [" +
                 env.pluginsFile().resolve("fake1").resolve(PluginInfo.ES_PLUGIN_PROPERTIES).toString() + "]");
     }

+ 22 - 20
qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java

@@ -23,6 +23,8 @@ import java.io.IOException;
 import java.nio.file.DirectoryStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.Map;
 
 import org.apache.lucene.util.LuceneTestCase;
 import org.elasticsearch.cli.UserError;
@@ -30,25 +32,32 @@ import org.elasticsearch.cli.MockTerminal;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.test.ESTestCase;
+import org.junit.Before;
 
 @LuceneTestCase.SuppressFileSystems("*")
 public class RemovePluginCommandTests extends ESTestCase {
 
-    /** Creates a test environment with bin, config and plugins directories. */
-    static Environment createEnv() throws IOException {
-        Path home = createTempDir();
+    private Path home;
+    private Environment env;
+
+    @Before
+    public void setUp() throws Exception {
+        super.setUp();
+        home = createTempDir();
         Files.createDirectories(home.resolve("bin"));
         Files.createFile(home.resolve("bin").resolve("elasticsearch"));
         Files.createDirectories(home.resolve("plugins"));
         Settings settings = Settings.builder()
-            .put("path.home", home)
-            .build();
-        return new Environment(settings);
+                .put("path.home", home)
+                .build();
+        env = new Environment(settings);
     }
 
-    static MockTerminal removePlugin(String name, Environment env) throws Exception {
+    static MockTerminal removePlugin(String name, Path home) throws Exception {
+        Map<String, String> settings = new HashMap<>();
+        settings.put("path.home", home.toString());
         MockTerminal terminal = new MockTerminal();
-        new RemovePluginCommand(env).execute(terminal, name);
+        new RemovePluginCommand().execute(terminal, name, settings);
         return terminal;
     }
 
@@ -63,33 +72,28 @@ public class RemovePluginCommandTests extends ESTestCase {
     }
 
     public void testMissing() throws Exception {
-        Environment env = createEnv();
-        UserError e = expectThrows(UserError.class, () -> {
-           removePlugin("dne", env);
-        });
+        UserError e = expectThrows(UserError.class, () -> removePlugin("dne", home));
         assertTrue(e.getMessage(), e.getMessage().contains("Plugin dne not found"));
         assertRemoveCleaned(env);
     }
 
     public void testBasic() throws Exception {
-        Environment env = createEnv();
         Files.createDirectory(env.pluginsFile().resolve("fake"));
         Files.createFile(env.pluginsFile().resolve("fake").resolve("plugin.jar"));
         Files.createDirectory(env.pluginsFile().resolve("fake").resolve("subdir"));
         Files.createDirectory(env.pluginsFile().resolve("other"));
-        removePlugin("fake", env);
+        removePlugin("fake", home);
         assertFalse(Files.exists(env.pluginsFile().resolve("fake")));
         assertTrue(Files.exists(env.pluginsFile().resolve("other")));
         assertRemoveCleaned(env);
     }
 
     public void testBin() throws Exception {
-        Environment env = createEnv();
         Files.createDirectories(env.pluginsFile().resolve("fake"));
         Path binDir = env.binFile().resolve("fake");
         Files.createDirectories(binDir);
         Files.createFile(binDir.resolve("somescript"));
-        removePlugin("fake", env);
+        removePlugin("fake", home);
         assertFalse(Files.exists(env.pluginsFile().resolve("fake")));
         assertTrue(Files.exists(env.binFile().resolve("elasticsearch")));
         assertFalse(Files.exists(binDir));
@@ -97,14 +101,12 @@ public class RemovePluginCommandTests extends ESTestCase {
     }
 
     public void testBinNotDir() throws Exception {
-        Environment env = createEnv();
         Files.createDirectories(env.pluginsFile().resolve("elasticsearch"));
-        UserError e = expectThrows(UserError.class, () -> {
-            removePlugin("elasticsearch", env);
-        });
+        UserError e = expectThrows(UserError.class, () -> removePlugin("elasticsearch", home));
         assertTrue(e.getMessage(), e.getMessage().contains("not a directory"));
         assertTrue(Files.exists(env.pluginsFile().resolve("elasticsearch"))); // did not remove
         assertTrue(Files.exists(env.binFile().resolve("elasticsearch")));
         assertRemoveCleaned(env);
     }
+
 }

+ 0 - 19
qa/evil-tests/src/test/java/org/elasticsearch/tribe/TribeUnitTests.java

@@ -84,29 +84,10 @@ public class TribeUnitTests extends ESTestCase {
         tribe2 = null;
     }
 
-    public void testThatTribeClientsIgnoreGlobalSysProps() throws Exception {
-        System.setProperty("es.cluster.name", "tribe_node_cluster");
-        System.setProperty("es.tribe.t1.cluster.name", "tribe1");
-        System.setProperty("es.tribe.t2.cluster.name", "tribe2");
-        System.setProperty("es.tribe.t1.node_id.seed", Long.toString(random().nextLong()));
-        System.setProperty("es.tribe.t2.node_id.seed", Long.toString(random().nextLong()));
-
-        try {
-            assertTribeNodeSuccessfullyCreated(Settings.EMPTY);
-        } finally {
-            System.clearProperty("es.cluster.name");
-            System.clearProperty("es.tribe.t1.cluster.name");
-            System.clearProperty("es.tribe.t2.cluster.name");
-            System.clearProperty("es.tribe.t1.node_id.seed");
-            System.clearProperty("es.tribe.t2.node_id.seed");
-        }
-    }
-
     public void testThatTribeClientsIgnoreGlobalConfig() throws Exception {
         Path pathConf = getDataPath("elasticsearch.yml").getParent();
         Settings settings = Settings
             .builder()
-            .put(InternalSettingsPreparer.IGNORE_SYSTEM_PROPERTIES_SETTING.getKey(), true)
             .put(Environment.PATH_CONF_SETTING.getKey(), pathConf)
             .build();
         assertTribeNodeSuccessfullyCreated(settings);

+ 0 - 1
qa/smoke-test-client/src/test/java/org/elasticsearch/smoketest/ESSmokeClientTestCase.java

@@ -75,7 +75,6 @@ public abstract class ESSmokeClientTestCase extends LuceneTestCase {
     private static Client startClient(Path tempDir, TransportAddress... transportAddresses) {
         Settings clientSettings = Settings.builder()
                 .put("node.name", "qa_smoke_client_" + counter.getAndIncrement())
-                .put(InternalSettingsPreparer.IGNORE_SYSTEM_PROPERTIES_SETTING.getKey(), true) // prevents any settings to be replaced by system properties.
                 .put("client.transport.ignore_cluster_name", true)
                 .put(Environment.PATH_HOME_SETTING.getKey(), tempDir)
                 .put(Node.NODE_MODE_SETTING.getKey(), "network").build(); // we require network here!

+ 1 - 1
qa/vagrant/src/test/resources/packaging/scripts/module_and_plugin_test_cases.bash

@@ -103,7 +103,7 @@ fi
         echo "CONF_FILE=$CONF_FILE" >> /etc/sysconfig/elasticsearch;
     fi
 
-    run_elasticsearch_service 1 -Ees.default.config="$CONF_FILE"
+    run_elasticsearch_service 1 -Edefault.config="$CONF_FILE"
 
     # remove settings again otherwise cleaning up before next testrun will fail
     if is_dpkg ; then

+ 1 - 1
qa/vagrant/src/test/resources/packaging/scripts/packaging_test_utils.bash

@@ -340,7 +340,7 @@ run_elasticsearch_service() {
             local CONF_DIR=""
             local ES_PATH_CONF=""
         else
-            local ES_PATH_CONF="-Ees.path.conf=$CONF_DIR"
+            local ES_PATH_CONF="-Epath.conf=$CONF_DIR"
         fi
         # we must capture the exit code to compare so we don't want to start as background process in case we expect something other than 0
         local background=""

+ 65 - 0
test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java

@@ -0,0 +1,65 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.bootstrap;
+
+import org.elasticsearch.cli.MockTerminal;
+import org.elasticsearch.test.ESTestCase;
+
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Consumer;
+
+import static org.hamcrest.CoreMatchers.equalTo;
+
+abstract class ESElasticsearchCliTestCase extends ESTestCase {
+
+    interface InitConsumer {
+        void accept(final boolean foreground, final String pidFile, final Map<String, String> esSettings);
+    }
+
+    void runTest(
+            final int expectedStatus,
+            final boolean expectedInit,
+            final Consumer<String> outputConsumer,
+            final InitConsumer initConsumer,
+            String... args) throws Exception {
+        final MockTerminal terminal = new MockTerminal();
+        try {
+            final AtomicBoolean init = new AtomicBoolean();
+            final int status = Elasticsearch.main(args, new Elasticsearch() {
+                @Override
+                void init(final boolean daemonize, final String pidFile, final Map<String, String> esSettings) {
+                    init.set(true);
+                    initConsumer.accept(!daemonize, pidFile, esSettings);
+                }
+            }, terminal);
+            assertThat(status, equalTo(expectedStatus));
+            assertThat(init.get(), equalTo(expectedInit));
+            outputConsumer.accept(terminal.getOutput());
+        } catch (Throwable t) {
+            // if an unexpected exception is thrown, we log
+            // terminal output to aid debugging
+            logger.info(terminal.getOutput());
+            // rethrow so the test fails
+            throw t;
+        }
+    }
+
+}

+ 0 - 1
test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java

@@ -185,7 +185,6 @@ public abstract class ESSingleNodeTestCase extends ESTestCase {
             .put("http.enabled", false)
             .put(Node.NODE_LOCAL_SETTING.getKey(), true)
             .put(Node.NODE_DATA_SETTING.getKey(), true)
-            .put(InternalSettingsPreparer.IGNORE_SYSTEM_PROPERTIES_SETTING.getKey(), true) // make sure we get what we set :)
             .put(nodeSettings()) // allow test cases to provide their own settings or override these
             .build();
         Node build = new MockNode(settings, getVersion(), getPlugins());

+ 0 - 1
test/framework/src/main/java/org/elasticsearch/test/ExternalNode.java

@@ -51,7 +51,6 @@ import java.util.concurrent.TimeUnit;
 final class ExternalNode implements Closeable {
 
     public static final Settings REQUIRED_SETTINGS = Settings.builder()
-            .put(InternalSettingsPreparer.IGNORE_SYSTEM_PROPERTIES_SETTING.getKey(), true)
             .put(DiscoveryModule.DISCOVERY_TYPE_SETTING.getKey(), "zen")
             .put(Node.NODE_MODE_SETTING.getKey(), "network").build(); // we need network mode for this
 

+ 0 - 1
test/framework/src/main/java/org/elasticsearch/test/ExternalTestCluster.java

@@ -73,7 +73,6 @@ public final class ExternalTestCluster extends TestCluster {
         Settings clientSettings = Settings.builder()
                 .put(additionalSettings)
                 .put("node.name", InternalTestCluster.TRANSPORT_CLIENT_PREFIX + EXTERNAL_CLUSTER_PREFIX + counter.getAndIncrement())
-                .put(InternalSettingsPreparer.IGNORE_SYSTEM_PROPERTIES_SETTING.getKey(), true) // prevents any settings to be replaced by system properties.
                 .put("client.transport.ignore_cluster_name", true)
                 .put(Environment.PATH_HOME_SETTING.getKey(), tempDir)
                 .put(Node.NODE_MODE_SETTING.getKey(), "network").build(); // we require network here!

+ 7 - 9
test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java

@@ -291,11 +291,10 @@ public final class InternalTestCluster extends TestCluster {
         builder.put(Environment.PATH_REPO_SETTING.getKey(), baseDir.resolve("repos"));
         builder.put(TransportSettings.PORT.getKey(), TRANSPORT_BASE_PORT + "-" + (TRANSPORT_BASE_PORT + PORTS_PER_CLUSTER));
         builder.put("http.port", HTTP_BASE_PORT + "-" + (HTTP_BASE_PORT + PORTS_PER_CLUSTER));
-        builder.put(InternalSettingsPreparer.IGNORE_SYSTEM_PROPERTIES_SETTING.getKey(), true);
         builder.put(Node.NODE_MODE_SETTING.getKey(), nodeMode);
         builder.put("http.pipelining", enableHttpPipelining);
-        if (Strings.hasLength(System.getProperty("es.logger.level"))) {
-            builder.put("logger.level", System.getProperty("es.logger.level"));
+        if (Strings.hasLength(System.getProperty("tests.logger.level"))) {
+            builder.put("logger.level", System.getProperty("tests.logger.level"));
         }
         if (Strings.hasLength(System.getProperty("es.logger.prefix"))) {
             builder.put("logger.prefix", System.getProperty("es.logger.prefix"));
@@ -319,14 +318,14 @@ public final class InternalTestCluster extends TestCluster {
 
     public static String configuredNodeMode() {
         Builder builder = Settings.builder();
-        if (Strings.isEmpty(System.getProperty("es.node.mode")) && Strings.isEmpty(System.getProperty("es.node.local"))) {
+        if (Strings.isEmpty(System.getProperty("node.mode")) && Strings.isEmpty(System.getProperty("node.local"))) {
             return "local"; // default if nothing is specified
         }
-        if (Strings.hasLength(System.getProperty("es.node.mode"))) {
-            builder.put(Node.NODE_MODE_SETTING.getKey(), System.getProperty("es.node.mode"));
+        if (Strings.hasLength(System.getProperty("node.mode"))) {
+            builder.put(Node.NODE_MODE_SETTING.getKey(), System.getProperty("node.mode"));
         }
-        if (Strings.hasLength(System.getProperty("es.node.local"))) {
-            builder.put(Node.NODE_LOCAL_SETTING.getKey(), System.getProperty("es.node.local"));
+        if (Strings.hasLength(System.getProperty("node.local"))) {
+            builder.put(Node.NODE_LOCAL_SETTING.getKey(), System.getProperty("node.local"));
         }
         if (DiscoveryNode.isLocalNode(builder.build())) {
             return "local";
@@ -882,7 +881,6 @@ public final class InternalTestCluster extends TestCluster {
                     .put(Node.NODE_MODE_SETTING.getKey(), Node.NODE_MODE_SETTING.exists(nodeSettings) ? Node.NODE_MODE_SETTING.get(nodeSettings) : nodeMode)
                     .put("logger.prefix", nodeSettings.get("logger.prefix", ""))
                     .put("logger.level", nodeSettings.get("logger.level", "INFO"))
-                    .put(InternalSettingsPreparer.IGNORE_SYSTEM_PROPERTIES_SETTING.getKey(), true)
                     .put(settings);
 
             if (Node.NODE_LOCAL_SETTING.exists(nodeSettings)) {

+ 2 - 2
test/framework/src/main/java/org/elasticsearch/test/junit/listeners/ReproduceInfoPrinter.java

@@ -137,10 +137,10 @@ public class ReproduceInfoPrinter extends RunListener {
         }
 
         public ReproduceErrorMessageBuilder appendESProperties() {
-            appendProperties("es.logger.level");
+            appendProperties("tests.logger.level");
             if (inVerifyPhase()) {
                 // these properties only make sense for integration tests
-                appendProperties("es.node.mode", "es.node.local", TESTS_CLUSTER,
+                appendProperties("node.mode", "node.local", TESTS_CLUSTER,
                     ESIntegTestCase.TESTS_ENABLE_MOCK_MODULES);
             }
             appendProperties("tests.assertion.disabled", "tests.security.manager", "tests.nightly", "tests.jvms",

+ 2 - 2
test/framework/src/main/resources/log4j.properties

@@ -1,5 +1,5 @@
-es.logger.level=INFO
-log4j.rootLogger=${es.logger.level}, out
+tests.logger.level=INFO
+log4j.rootLogger=${tests.logger.level}, out
 
 log4j.logger.org.apache.http=INFO, out
 log4j.additivity.org.apache.http=false