Remove path.conf setting

This commit removes path.conf as a valid setting and replaces it with a
command-line flag for specifying a non-default path for configuration.

Relates #25392

10 files changed, 76 insertions, 51 deletions

diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
index 4674f1ea93..ffe5dfa3e4 100644
--- a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
+++ b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
@@ -238,9 +238,12 @@ final class Bootstrap {
         return keystore;
     }
 
-
-    private static Environment createEnvironment(boolean foreground, Path pidFile,
-                                                 SecureSettings secureSettings, Settings initialSettings) {
+    private static Environment createEnvironment(
+            final boolean foreground,
+            final Path pidFile,
+            final SecureSettings secureSettings,
+            final Settings initialSettings,
+            final Path configPath) {
         Terminal terminal = foreground ? Terminal.DEFAULT : null;
         Settings.Builder builder = Settings.builder();
         if (pidFile != null) {
@@ -250,7 +253,7 @@ final class Bootstrap {
         if (secureSettings != null) {
             builder.setSecureSettings(secureSettings);
         }
-        return InternalSettingsPreparer.prepareEnvironment(builder.build(), terminal, Collections.emptyMap());
+        return InternalSettingsPreparer.prepareEnvironment(builder.build(), terminal, Collections.emptyMap(), configPath);
     }
 
     private void start() throws NodeValidationException {
@@ -281,7 +284,7 @@ final class Bootstrap {
         INSTANCE = new Bootstrap();
 
         final SecureSettings keystore = loadSecureSettings(initialEnv);
-        Environment environment = createEnvironment(foreground, pidFile, keystore, initialEnv.settings());
+        final Environment environment = createEnvironment(foreground, pidFile, keystore, initialEnv.settings(), initialEnv.configFile());
         try {
             LogConfigurator.configure(environment);
         } catch (IOException e) {
diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Security.java b/core/src/main/java/org/elasticsearch/bootstrap/Security.java
index 5ffb89b6ee..81dc6f9d40 100644
--- a/core/src/main/java/org/elasticsearch/bootstrap/Security.java
+++ b/core/src/main/java/org/elasticsearch/bootstrap/Security.java
@@ -256,7 +256,7 @@ final class Security {
         addPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.libFile(), "read,readlink");
         addPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.modulesFile(), "read,readlink");
         addPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.pluginsFile(), "read,readlink");
-        addPath(policy, Environment.PATH_CONF_SETTING.getKey(), environment.configFile(), "read,readlink");
+        addPath(policy, "path.conf'", environment.configFile(), "read,readlink");
         // read-write dirs
         addPath(policy, "java.io.tmpdir", environment.tmpFile(), "read,readlink,write,delete");
         addPath(policy, Environment.PATH_LOGS_SETTING.getKey(), environment.logsFile(), "read,readlink,write,delete");
diff --git a/core/src/main/java/org/elasticsearch/cli/EnvironmentAwareCommand.java b/core/src/main/java/org/elasticsearch/cli/EnvironmentAwareCommand.java
index 79a4fd7329..e06d227a24 100644
--- a/core/src/main/java/org/elasticsearch/cli/EnvironmentAwareCommand.java
+++ b/core/src/main/java/org/elasticsearch/cli/EnvironmentAwareCommand.java
@@ -22,10 +22,14 @@ package org.elasticsearch.cli;
 import joptsimple.OptionSet;
 import joptsimple.OptionSpec;
 import joptsimple.util.KeyValuePair;
+import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.node.InternalSettingsPreparer;
 
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
@@ -34,10 +38,13 @@ import java.util.Map;
 public abstract class EnvironmentAwareCommand extends Command {
 
     private final OptionSpec<KeyValuePair> settingOption;
+    private final OptionSpec<String> pathConfOption;
 
     public EnvironmentAwareCommand(String description) {
         super(description);
         this.settingOption = parser.accepts("E", "Configure a setting").withRequiredArg().ofType(KeyValuePair.class);
+        this.pathConfOption =
+                parser.acceptsAll(Arrays.asList("c", "path.conf"), "Configure config path").withRequiredArg().ofType(String.class);
     }
 
     @Override
@@ -59,17 +66,22 @@ public abstract class EnvironmentAwareCommand extends Command {
             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, createEnv(terminal, settings));
+        final String pathConf = pathConfOption.value(options);
+        execute(terminal, options, createEnv(terminal, settings, getConfigPath(pathConf)));
+    }
+
+    @SuppressForbidden(reason = "need path to construct environment")
+    private static Path getConfigPath(final String pathConf) {
+        return pathConf == null ? null : Paths.get(pathConf);
     }
 
     /** Create an {@link Environment} for the command to use. Overrideable for tests. */
-    protected Environment createEnv(Terminal terminal, Map<String, String> settings) {
-        return InternalSettingsPreparer.prepareEnvironment(Settings.EMPTY, terminal, settings);
+    protected Environment createEnv(Terminal terminal, Map<String, String> settings, Path configPath) {
+        return InternalSettingsPreparer.prepareEnvironment(Settings.EMPTY, terminal, settings, configPath);
     }
 
     /** Ensure the given setting exists, reading it from system properties if not already set. */
diff --git a/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java b/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java
index 5879f1e357..1cf79c26d9 100644
--- a/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java
+++ b/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java
@@ -97,7 +97,7 @@ public abstract class TransportClient extends AbstractClient {
                 .put(InternalSettingsPreparer.prepareSettings(settings))
                 .put(NetworkService.NETWORK_SERVER.getKey(), false)
                 .put(CLIENT_TYPE_SETTING_S.getKey(), CLIENT_TYPE);
-        return new PluginsService(settingsBuilder.build(), null, null, plugins);
+        return new PluginsService(settingsBuilder.build(), null, null, null, plugins);
     }
 
     protected static Collection<Class<? extends Plugin>> addPlugins(Collection<Class<? extends Plugin>> collection,
diff --git a/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java
index d8ee93fe88..d6a6b88764 100644
--- a/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java
+++ b/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java
@@ -314,8 +314,6 @@ public final class ClusterSettings extends AbstractScopedSettings {
                     HunspellService.HUNSPELL_IGNORE_CASE,
                     HunspellService.HUNSPELL_DICTIONARY_OPTIONS,
                     IndicesStore.INDICES_STORE_DELETE_SHARD_TIMEOUT,
-                    Environment.DEFAULT_PATH_CONF_SETTING,
-                    Environment.PATH_CONF_SETTING,
                     Environment.DEFAULT_PATH_DATA_SETTING,
                     Environment.PATH_DATA_SETTING,
                     Environment.PATH_HOME_SETTING,
diff --git a/core/src/main/java/org/elasticsearch/env/Environment.java b/core/src/main/java/org/elasticsearch/env/Environment.java
index ce2b15d2d7..f46f15b9d6 100644
--- a/core/src/main/java/org/elasticsearch/env/Environment.java
+++ b/core/src/main/java/org/elasticsearch/env/Environment.java
@@ -46,9 +46,6 @@ import java.util.function.Function;
 // public+forbidden api!
 public class Environment {
     public static final Setting<String> PATH_HOME_SETTING = Setting.simpleString("path.home", Property.NodeScope);
-    public static final Setting<String> DEFAULT_PATH_CONF_SETTING = Setting.simpleString("default.path.conf", Property.NodeScope);
-    public static final Setting<String> PATH_CONF_SETTING =
-            new Setting<>("path.conf", DEFAULT_PATH_CONF_SETTING, Function.identity(), Property.NodeScope);
     public static final Setting<List<String>> DEFAULT_PATH_DATA_SETTING =
             Setting.listSetting("default.path.data", Collections.emptyList(), Function.identity(), Property.NodeScope);
     public static final Setting<List<String>> PATH_DATA_SETTING =
@@ -92,6 +89,10 @@ public class Environment {
     private final Path tmpFile = PathUtils.get(System.getProperty("java.io.tmpdir"));
 
     public Environment(Settings settings) {
+        this(settings, null);
+    }
+
+    public Environment(final Settings settings, final Path configPath) {
         final Path homeFile;
         if (PATH_HOME_SETTING.exists(settings)) {
             homeFile = PathUtils.get(PATH_HOME_SETTING.get(settings)).normalize();
@@ -99,9 +100,8 @@ public class Environment {
             throw new IllegalStateException(PATH_HOME_SETTING.getKey() + " is not configured");
         }
 
-        // this is trappy, Setting#get(Settings) will get a fallback setting yet return false for Settings#exists(Settings)
-        if (PATH_CONF_SETTING.exists(settings) || DEFAULT_PATH_CONF_SETTING.exists(settings)) {
-            configFile = PathUtils.get(PATH_CONF_SETTING.get(settings)).normalize();
+        if (configPath != null) {
+            configFile = configPath.normalize();
         } else {
             configFile = homeFile.resolve("config");
         }
@@ -160,7 +160,6 @@ public class Environment {
         }
         finalSettings.put(PATH_LOGS_SETTING.getKey(), logsFile);
         this.settings = finalSettings.build();
-
     }
 
     /**
diff --git a/core/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java b/core/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java
index 9ee0812642..93c5a18222 100644
--- a/core/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java
+++ b/core/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java
@@ -63,7 +63,7 @@ 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());
+        return prepareEnvironment(input, terminal, Collections.emptyMap(), null);
     }
 
     /**
@@ -71,16 +71,18 @@ public class InternalSettingsPreparer {
      * and then replacing all property placeholders. If a {@link Terminal} is provided and configuration settings are loaded,
      * settings with a value of <code>${prompt.text}</code> or <code>${prompt.secret}</code> will result in a prompt for
      * the setting to the user.
-     * @param 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)
+     *
+     * @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)
+     * @param configPath path to config directory; (use null to indicate the default)
      * @return the {@link Settings} and {@link Environment} as a {@link Tuple}
      */
-    public static Environment prepareEnvironment(Settings input, Terminal terminal, Map<String, String> properties) {
+    public static Environment prepareEnvironment(Settings input, Terminal terminal, Map<String, String> properties, Path configPath) {
         // just create enough settings to build the environment, to get the config dir
         Settings.Builder output = Settings.builder();
         initializeSettings(output, input, properties);
-        Environment environment = new Environment(output.build());
+        Environment environment = new Environment(output.build(), configPath);
 
         if (Files.exists(environment.configFile().resolve("elasticsearch.yaml"))) {
             throw new SettingsException("elasticsearch.yaml was deprecated in 5.5.0 and must be renamed to elasticsearch.yml");
@@ -104,11 +106,11 @@ public class InternalSettingsPreparer {
         initializeSettings(output, input, properties);
         finalizeSettings(output, terminal);
 
-        environment = new Environment(output.build());
+        environment = new Environment(output.build(), configPath);
 
         // we put back the path.logs so we can use it in the logging configuration file
         output.put(Environment.PATH_LOGS_SETTING.getKey(), environment.logsFile().toAbsolutePath().normalize().toString());
-        return new Environment(output.build());
+        return new Environment(output.build(), configPath);
     }
 
     /**
diff --git a/core/src/main/java/org/elasticsearch/node/Node.java b/core/src/main/java/org/elasticsearch/node/Node.java
index 0945d58e45..bb7a2fd7f2 100644
--- a/core/src/main/java/org/elasticsearch/node/Node.java
+++ b/core/src/main/java/org/elasticsearch/node/Node.java
@@ -302,16 +302,15 @@ public class Node implements Closeable {
                     environment.configFile(), Arrays.toString(environment.dataFiles()), environment.logsFile(), environment.pluginsFile());
             }
 
-            this.pluginsService = new PluginsService(tmpSettings, environment.modulesFile(), environment.pluginsFile(), classpathPlugins);
+            this.pluginsService = new PluginsService(tmpSettings, environment.configFile(), environment.modulesFile(), environment.pluginsFile(), classpathPlugins);
             this.settings = pluginsService.updatedSettings();
             localNodeFactory = new LocalNodeFactory(settings, nodeEnvironment.nodeId());
 
             // create the environment based on the finalized (processed) view of the settings
             // this is just to makes sure that people get the same settings, no matter where they ask them from
-            this.environment = new Environment(this.settings);
+            this.environment = new Environment(this.settings, environment.configFile());
             Environment.assertEquivalent(environment, this.environment);
 
-
             final List<ExecutorBuilder<?>> executorBuilders = pluginsService.getExecutorBuilders(settings);
 
             final ThreadPool threadPool = new ThreadPool(settings, executorBuilders.toArray(new ExecutorBuilder[0]));
@@ -387,8 +386,14 @@ public class Node implements Closeable {
                     .flatMap(p -> p.getNamedXContent().stream()),
                 ClusterModule.getNamedXWriteables().stream())
                 .flatMap(Function.identity()).collect(toList()));
-            final TribeService tribeService = new TribeService(settings, clusterService, nodeId, namedWriteableRegistry,
-                s -> newTribeClientNode(s, classpathPlugins));
+            final TribeService tribeService =
+                    new TribeService(
+                            settings,
+                            environment.configFile(),
+                            clusterService,
+                            nodeId,
+                            namedWriteableRegistry,
+                            (s, p) -> newTribeClientNode(s, classpathPlugins, p));
             resourcesToClose.add(tribeService);
             modules.add(new RepositoriesModule(this.environment, pluginsService.filterPlugins(RepositoryPlugin.class), xContentRegistry));
             final MetaStateService metaStateService = new MetaStateService(settings, nodeEnvironment, xContentRegistry);
@@ -980,8 +985,8 @@ public class Node implements Closeable {
     }
 
     /** Constructs an internal node used as a client into a cluster fronted by this tribe node. */
-    protected Node newTribeClientNode(Settings settings, Collection<Class<? extends Plugin>> classpathPlugins) {
-        return new Node(new Environment(settings), classpathPlugins);
+    protected Node newTribeClientNode(Settings settings, Collection<Class<? extends Plugin>> classpathPlugins, Path configPath) {
+        return new Node(new Environment(settings, configPath), classpathPlugins);
     }
 
     /** Constructs a ClusterInfoService which may be mocked for tests. */
diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java
index b2cea4c0ad..5219fabf24 100644
--- a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java
+++ b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java
@@ -61,12 +61,13 @@ import java.util.Objects;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 import static org.elasticsearch.common.io.FileSystemUtils.isAccessibleDirectory;
 
 public class PluginsService extends AbstractComponent {
 
+    private final Path configPath;
+
     /**
      * We keep around a list of plugins and modules
      */
@@ -90,14 +91,16 @@ public class PluginsService extends AbstractComponent {
      * @param pluginsDirectory The directory plugins exist in, or null if plugins should not be loaded from the filesystem
      * @param classpathPlugins Plugins that exist in the classpath which should be loaded
      */
-    public PluginsService(Settings settings, Path modulesDirectory, Path pluginsDirectory, Collection<Class<? extends Plugin>> classpathPlugins) {
+    public PluginsService(Settings settings, Path configPath, Path modulesDirectory, Path pluginsDirectory, Collection<Class<? extends Plugin>> classpathPlugins) {
         super(settings);
 
+        this.configPath = configPath;
+
         List<Tuple<PluginInfo, Plugin>> pluginsLoaded = new ArrayList<>();
         List<PluginInfo> pluginsList = new ArrayList<>();
         // first we load plugins that are on the classpath. this is for tests and transport clients
         for (Class<? extends Plugin> pluginClass : classpathPlugins) {
-            Plugin plugin = loadPlugin(pluginClass, settings);
+            Plugin plugin = loadPlugin(pluginClass, settings, configPath);
             PluginInfo pluginInfo = new PluginInfo(pluginClass.getName(), "classpath plugin", "NA", pluginClass.getName(), false);
             if (logger.isTraceEnabled()) {
                 logger.trace("plugin loaded from classpath [{}]", pluginInfo);
@@ -381,7 +384,7 @@ public class PluginsService extends AbstractComponent {
             reloadLuceneSPI(loader);
             final Class<? extends Plugin> pluginClass =
                 loadPluginClass(bundle.plugin.getClassname(), loader);
-            final Plugin plugin = loadPlugin(pluginClass, settings);
+            final Plugin plugin = loadPlugin(pluginClass, settings, configPath);
             plugins.add(new Tuple<>(bundle.plugin, plugin));
         }
 
@@ -414,17 +417,21 @@ public class PluginsService extends AbstractComponent {
         }
     }
 
-    private Plugin loadPlugin(Class<? extends Plugin> pluginClass, Settings settings) {
+    private Plugin loadPlugin(Class<? extends Plugin> pluginClass, Settings settings, Path configPath) {
         try {
             try {
-                return pluginClass.getConstructor(Settings.class).newInstance(settings);
+                return pluginClass.getConstructor(Settings.class, Path.class).newInstance(settings, configPath);
             } catch (NoSuchMethodException e) {
                 try {
-                    return pluginClass.getConstructor().newInstance();
+                    return pluginClass.getConstructor(Settings.class).newInstance(settings);
                 } catch (NoSuchMethodException e1) {
-                    throw new ElasticsearchException("No constructor for [" + pluginClass + "]. A plugin class must " +
-                        "have either an empty default constructor or a single argument constructor accepting a " +
-                        "Settings instance");
+                    try {
+                        return pluginClass.getConstructor().newInstance();
+                    } catch (NoSuchMethodException e2) {
+                        throw new ElasticsearchException("No constructor for [" + pluginClass + "]. A plugin class must " +
+                                "have either an empty default constructor, a single argument constructor accepting a " +
+                                "Settings instance, or a single argument constructor accepting a pair of Settings, Path instances");
+                    }
                 }
             }
         } catch (Exception e) {
diff --git a/core/src/main/java/org/elasticsearch/tribe/TribeService.java b/core/src/main/java/org/elasticsearch/tribe/TribeService.java
index 614abc0af9..81ed347382 100644
--- a/core/src/main/java/org/elasticsearch/tribe/TribeService.java
+++ b/core/src/main/java/org/elasticsearch/tribe/TribeService.java
@@ -68,6 +68,7 @@ import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.transport.TransportSettings;
 
 import java.io.IOException;
+import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -77,6 +78,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
@@ -216,8 +218,8 @@ public class TribeService extends AbstractLifecycleComponent {
 
     private final NamedWriteableRegistry namedWriteableRegistry;
 
-    public TribeService(Settings settings, ClusterService clusterService, final String tribeNodeId,
-                        NamedWriteableRegistry namedWriteableRegistry, Function<Settings, Node> clientNodeBuilder) {
+    public TribeService(Settings settings, Path configPath, ClusterService clusterService, final String tribeNodeId,
+                        NamedWriteableRegistry namedWriteableRegistry, BiFunction<Settings, Path, Node> clientNodeBuilder) {
         super(settings);
         this.clusterService = clusterService;
         this.namedWriteableRegistry = namedWriteableRegistry;
@@ -226,7 +228,7 @@ public class TribeService extends AbstractLifecycleComponent {
         nodesSettings.remove("on_conflict"); // remove prefix settings that don't indicate a client
         for (Map.Entry<String, Settings> entry : nodesSettings.entrySet()) {
             Settings clientSettings = buildClientSettings(entry.getKey(), tribeNodeId, settings, entry.getValue());
-            nodes.add(clientNodeBuilder.apply(clientSettings));
+            nodes.add(clientNodeBuilder.apply(clientSettings, configPath));
         }
 
         this.blockIndicesMetadata = BLOCKS_METADATA_INDICES_SETTING.get(settings).toArray(Strings.EMPTY_ARRAY);
@@ -253,9 +255,6 @@ public class TribeService extends AbstractLifecycleComponent {
         Settings.Builder sb = Settings.builder().put(tribeSettings);
         sb.put(Node.NODE_NAME_SETTING.getKey(), Node.NODE_NAME_SETTING.get(globalSettings) + "/" + tribeName);
         sb.put(Environment.PATH_HOME_SETTING.getKey(), Environment.PATH_HOME_SETTING.get(globalSettings)); // pass through ES home dir
-        if (Environment.PATH_CONF_SETTING.exists(globalSettings)) {
-            sb.put(Environment.PATH_CONF_SETTING.getKey(), Environment.PATH_CONF_SETTING.get(globalSettings));
-        }
         if (Environment.PATH_LOGS_SETTING.exists(globalSettings)) {
             sb.put(Environment.PATH_LOGS_SETTING.getKey(), Environment.PATH_LOGS_SETTING.get(globalSettings));
         }