Pass config path as a system property

A previous change enabled it so that users could configure the
configuration path via a command-line option --path.conf. However, a
subsequent change has made it so that we expect users to set the
configuration path via the environment variable CONF_DIR. To enable
this, we now pass the value of CONF_DIR as the value for the
command-line option --path.conf. This has two problems:
 - the presence of --path.conf always being on the command line breaks
   other flags like --help for multi-commands
 - the scripts for which --help is not broken say that you can pass
   --path.conf but this is a lie since passing it will make it appear
   twice in the command-line arguments breaking the script

Since --path.conf is no longer the way that we want users to set the
configuration path, we should remove the --path.conf option. However, we
still need a way to get the configuration path from the scripts to the
running Java process. To do this, we now pass the configuration path as
a system property. This keeps it off the script command line fixing the
above problems.

The only remaining question (that I can see) is whether or not to
respect -Des.path.conf=<some path> if the user sets this in their
jvm.options or via ES_JAVA_OPTS. I think that we should not do this (as
has been our tradition), es.path.home and es.path.conf are special,
should be set by our scripts only so users should not be setting them at
all so we should not take any effort to respect these flags if the user
tries to otherwise use them.

Relates #25943
This commit is contained in:
Jason Tedor 2017-07-28 12:15:22 +09:00 committed by GitHub
parent 6c650874c9
commit 8639bf4a1a
5 changed files with 48 additions and 24 deletions

View File

@ -33,18 +33,16 @@ import java.util.Arrays;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
/** A cli command which requires an {@link org.elasticsearch.env.Environment} to use current paths and settings. */
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
@ -70,13 +68,17 @@ public abstract class EnvironmentAwareCommand extends Command {
putSystemPropertyIfSettingIsMissing(settings, "path.home", "es.path.home");
putSystemPropertyIfSettingIsMissing(settings, "path.logs", "es.path.logs");
final String pathConf = pathConfOption.value(options);
final String pathConf = System.getProperty("es.path.conf");
if (pathConf == null) {
throw new UserException(ExitCodes.CONFIG, "the system property es.path.conf must be set");
}
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);
return Paths.get(pathConf);
}
/** Create an {@link Environment} for the command to use. Overrideable for tests. */

View File

@ -38,17 +38,27 @@ ES_JVM_OPTIONS="$CONF_DIR"/jvm.options
ES_JAVA_OPTS="$(parse_jvm_options "$ES_JVM_OPTIONS") $ES_JAVA_OPTS"
declare -a args=("$@")
args=("${args[@]}" --path.conf "$CONF_DIR")
# manual parsing to find out, if process should be detached
daemonized=`echo $* | egrep -- '(^-d |-d$| -d |--daemonize$|--daemonize )'`
if [ -z "$daemonized" ] ; then
exec "$JAVA" $ES_JAVA_OPTS -Des.path.home="$ES_HOME" -cp "$ES_CLASSPATH" \
org.elasticsearch.bootstrap.Elasticsearch "${args[@]}"
exec \
"$JAVA" \
$ES_JAVA_OPTS \
-Des.path.home="$ES_HOME" \
-Des.path.conf="$CONF_DIR" \
-cp "$ES_CLASSPATH" \
org.elasticsearch.bootstrap.Elasticsearch \
"$@"
else
exec "$JAVA" $ES_JAVA_OPTS -Des.path.home="$ES_HOME" -cp "$ES_CLASSPATH" \
org.elasticsearch.bootstrap.Elasticsearch "${args[@]}" <&- &
exec \
"$JAVA" \
$ES_JAVA_OPTS \
-Des.path.home="$ES_HOME" \
-Des.path.conf="$CONF_DIR" \
-cp "$ES_CLASSPATH" \
org.elasticsearch.bootstrap.Elasticsearch \
"$@" \
<&- &
retval=$?
pid=$!
[ $retval -eq 0 ] || exit $retval

View File

@ -2,7 +2,11 @@
source "`dirname "$0"`"/elasticsearch-env
declare -a args=("$@")
args=("${args[@]}" --path.conf "$CONF_DIR")
exec "$JAVA" $ES_JAVA_OPTS -Des.path.home="$ES_HOME" -cp "$ES_CLASSPATH" org.elasticsearch.common.settings.KeyStoreCli "${args[@]}"
exec \
"$JAVA" \
$ES_JAVA_OPTS \
-Des.path.home="$ES_HOME" \
-Des.path.conf="$CONF_DIR" \
-cp "$ES_CLASSPATH" \
org.elasticsearch.common.settings.KeyStoreCli \
"$@"

View File

@ -2,7 +2,11 @@
source "`dirname "$0"`"/elasticsearch-env
declare -a args=("$@")
args=("${args[@]}" --path.conf "$CONF_DIR")
exec "$JAVA" $ES_JAVA_OPTS -Des.path.home="$ES_HOME" -cp "$ES_CLASSPATH" org.elasticsearch.plugins.PluginCli "${args[@]}"
exec \
"$JAVA" \
$ES_JAVA_OPTS \
-Des.path.home="$ES_HOME" \
-Des.path.conf="$CONF_DIR" \
-cp "$ES_CLASSPATH" \
org.elasticsearch.plugins.PluginCli \
"$@"

View File

@ -2,7 +2,11 @@
source "`dirname "$0"`"/elasticsearch-env
declare -a args=("$@")
args=("${args[@]}" --path.conf "$CONF_DIR")
exec "$JAVA" $ES_JAVA_OPTS -Des.path.home="$ES_HOME" -cp "$ES_CLASSPATH" org.elasticsearch.index.translog.TranslogToolCli "${args[@]}"
exec \
"$JAVA" \
$ES_JAVA_OPTS \
-Des.path.home="$ES_HOME" \
-Des.path.conf="$CONF_DIR" \
-cp "$ES_CLASSPATH" \
org.elasticsearch.index.translog.TranslogToolCli \
"$@"