Move docker env var settings handling out of bash (#85913)

In docker ES allows settings to be set via environment variables. This
is currently handled in complex bash logic. This commit moves that logic
into EnvironmentAwareCommand where the rest of the initial settings are
found.

relates #85758

Author: rjernst

distribution/src/bin/elasticsearch-env

@@ -92,57 +92,6 @@ ES_PATH_CONF=`cd "$ES_PATH_CONF"; pwd`
 [email protected]@
 
 if [[ "$ES_DISTRIBUTION_TYPE" == "docker" ]]; then
-  # Allow environment variables to be set by creating a file with the
-  # contents, and setting an environment variable with the suffix _FILE to
-  # point to it. This can be used to provide secrets to a container, without
-  # the values being specified explicitly when running the container.
-  source "$ES_HOME/bin/elasticsearch-env-from-file"
-
-  # Parse Docker env vars to customize Elasticsearch
-  #
-  # e.g. Setting the env var cluster.name=testcluster or ES_CLUSTER_NAME=testcluster
-  #
-  # will cause Elasticsearch to be invoked with -Ecluster.name=testcluster
-  #
-  # see https://www.elastic.co/guide/en/elasticsearch/reference/current/settings.html#_setting_default_settings
-
-  declare -a es_arg_array
-
-  containsElement () {
-    local e match="$1"
-    shift
-    for e; do [[ "$e" == "$match" ]] && return 0; done
-    return 1
-  }
-
-  # Elasticsearch settings need to either:
-  # a. have at least two dot separated lower case words, e.g. `cluster.name`, or
-  while IFS='=' read -r envvar_key envvar_value; do
-    es_opt=""
-    if [[ -n "$envvar_value" ]]; then
-      es_opt="-E${envvar_key}=${envvar_value}"
-    fi
-    if [[ ! -z "${es_opt}" ]] && ! containsElement "${es_opt}" "$@" ; then
-      es_arg_array+=("${es_opt}")
-    fi
-  done <<< "$(env | grep -E '^[-a-z0-9_]+(\.[-a-z0-9_]+)+=')"
-
-  # b. be upper cased with underscore separators and prefixed with `ES_SETTING_`, e.g. `ES_SETTING_CLUSTER_NAME`.
-  #    Underscores in setting names are escaped by writing them as a double-underscore e.g. "__"
-  while IFS='=' read -r envvar_key envvar_value; do
-    es_opt=""
-    if [[ -n "$envvar_value" ]]; then
-      # The long-hand sed `y` command works in any sed variant.
-      envvar_key="$(echo "$envvar_key" | sed -e 's/^ES_SETTING_//; s/_/./g ; s/\.\./_/g; y/ABCDEFGHIJKLMNOPQRSTUVWXYZ/abcdefghijklmnopqrstuvwxyz/' )"
-      es_opt="-E${envvar_key}=${envvar_value}"
-    fi
-    if [[ ! -z "${es_opt}" ]] && ! containsElement "${es_opt}" "$@" ; then
-      es_arg_array+=("${es_opt}")
-    fi
-  done <<< "$(env | grep -E '^ES_SETTING(_{1,2}[A-Z]+)+=')"
-
-  # Reset the positional parameters to the es_arg_array values and any existing positional params
-  set -- "$@" "${es_arg_array[@]}"
 
   # The virtual file /proc/self/cgroup should list the current cgroup
   # membership. For each hierarchy, you can follow the cgroup path from

libs/cli/src/main/java/org/elasticsearch/cli/Command.java

@@ -22,6 +22,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Properties;
 import java.util.stream.Collectors;
 
@@ -55,8 +56,8 @@ public abstract class Command implements Closeable {
      */
     public Command(final String description) {
         this.description = description;
-        this.sysprops = captureSystemProperties();
-        this.envVars = captureEnvironmentVariables();
+        this.sysprops = Objects.requireNonNull(captureSystemProperties());
+        this.envVars = Objects.requireNonNull(captureEnvironmentVariables());
     }
 
     private Thread shutdownHookThread;

qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java

@@ -71,7 +71,6 @@
 import static org.elasticsearch.packaging.util.docker.Docker.waitForElasticsearch;
 import static org.elasticsearch.packaging.util.docker.DockerFileMatcher.file;
 import static org.elasticsearch.packaging.util.docker.DockerRun.builder;
-import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.arrayContaining;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsInAnyOrder;
@@ -754,86 +753,6 @@ public void test085EnvironmentVariablesAreRespectedUnderDockerExec() {
         assertThat(result.stdout(), containsString("java.net.UnknownHostException: this.is.not.valid"));
     }
 
-    /**
-     * Check that settings are applied when they are supplied as environment variables with names that are:
-     * <ul>
-     *     <li>Prefixed with {@code ES_SETTING_}</li>
-     *     <li>All uppercase</li>
-     *     <li>Dots (periods) are converted to underscores</li>
-     *     <li>Underscores in setting names are escaped by doubling them</li>
-     * </ul>
-     */
-    public void test086EnvironmentVariablesInSnakeCaseAreTranslated() {
-        // Note the double-underscore in the var name here, which retains the underscore in translation
-        installation = runContainer(distribution(), builder().envVar("ES_SETTING_XPACK_SECURITY_FIPS__MODE_ENABLED", "false"));
-
-        final Optional<String> commandLine = sh.run("bash -c 'COLUMNS=2000 ps ax'")
-            .stdout()
-            .lines()
-            .filter(line -> line.contains("org.elasticsearch.bootstrap.Elasticsearch"))
-            .findFirst();
-
-        assertThat(commandLine.isPresent(), equalTo(true));
-
-        assertThat(commandLine.get(), containsString("-Expack.security.fips_mode.enabled=false"));
-    }
-
-    /**
-     * Check that environment variables that do not match the criteria for translation to settings are ignored.
-     */
-    public void test087EnvironmentVariablesInIncorrectFormatAreIgnored() {
-        installation = runContainer(
-            distribution(),
-            builder()
-                // No ES_SETTING_ prefix
-                .envVar("XPACK_SECURITY_FIPS__MODE_ENABLED", "false")
-                // Incomplete prefix
-                .envVar("ES_XPACK_SECURITY_FIPS__MODE_ENABLED", "false")
-                // Not underscore-separated
-                .envVar("ES.SETTING.XPACK.SECURITY.FIPS_MODE.ENABLED", "false")
-                // Not uppercase
-                .envVar("es_setting_xpack_security_fips__mode_enabled", "false")
-        );
-
-        final Optional<String> commandLine = sh.run("bash -c 'COLUMNS=2000 ps ax'")
-            .stdout()
-            .lines()
-            .filter(line -> line.contains("org.elasticsearch.bootstrap.Elasticsearch"))
-            .findFirst();
-
-        assertThat(commandLine.isPresent(), equalTo(true));
-
-        assertThat(commandLine.get(), not(containsString("-Expack.security.fips_mode.enabled=false")));
-    }
-
-    /**
-     * Check that settings are applied when they are supplied as environment variables with names that:
-     * <ul>
-     *     <li>Consist only of lowercase letters, numbers, underscores and hyphens</li>
-     *     <li>Separated by periods</li>
-     * </ul>
-     */
-    public void test088EnvironmentVariablesInDottedFormatArePassedThrough() {
-        // Note the double-underscore in the var name here, which retains the underscore in translation
-        installation = runContainer(
-            distribution(),
-            builder().envVar("xpack.security.fips_mode.enabled", "false").envVar("http.cors.allow-methods", "GET")
-        );
-
-        final Optional<String> commandLine = sh.run("bash -c 'COLUMNS=2000 ps ax'")
-            .stdout()
-            .lines()
-            .filter(line -> line.contains("org.elasticsearch.bootstrap.Elasticsearch"))
-            .findFirst();
-
-        assertThat(commandLine.isPresent(), equalTo(true));
-
-        assertThat(
-            commandLine.get(),
-            allOf(containsString("-Expack.security.fips_mode.enabled=false"), containsString("-Ehttp.cors.allow-methods=GET"))
-        );
-    }
-
     /**
      * Check whether the elasticsearch-certutil tool has been shipped correctly,
      * and if present then it can execute.

qa/os/src/test/java/org/elasticsearch/packaging/util/ServerUtils.java

@@ -39,7 +39,6 @@
 import java.security.cert.X509Certificate;
 import java.util.List;
 import java.util.Objects;
-import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -77,16 +76,14 @@ public static void waitForElasticsearch(Installation installation) throws Except
             String configFile = Files.readString(configFilePath, StandardCharsets.UTF_8);
             securityEnabled = configFile.contains(SECURITY_DISABLED) == false;
         } else {
-            final Optional<String> commandLine = dockerShell.run("bash -c 'COLUMNS=2000 ps ax'")
+            securityEnabled = dockerShell.run("env")
                 .stdout()
                 .lines()
-                .filter(line -> line.contains("org.elasticsearch.bootstrap.Elasticsearch"))
-                .findFirst();
-            if (commandLine.isPresent() == false) {
-                throw new RuntimeException("Installation distribution is docker but a docker container is not running");
-            }
-            // security is enabled by default, the only way for it to be disabled is to be explicitly disabled
-            securityEnabled = commandLine.get().contains("-Expack.security.enabled=false") == false;
+                .filter(each -> each.startsWith("xpack.security.enabled"))
+                .findFirst()
+                .map(line -> Boolean.parseBoolean(line.split("=")[1]))
+                // security is enabled by default, the only way for it to be disabled is to be explicitly disabled
+                .orElse(true);
         }
 
         if (securityEnabled) {

server/src/main/java/org/elasticsearch/common/cli/EnvironmentAwareCommand.java

@@ -12,6 +12,7 @@
 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.Terminal;
@@ -26,10 +27,14 @@
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
+import java.util.regex.Pattern;
 
 /** A cli command which requires an {@link org.elasticsearch.env.Environment} to use current paths and settings. */
 public abstract class EnvironmentAwareCommand extends Command {
 
+    private static final String DOCKER_UPPERCASE_SETTING_PREFIX = "ES_SETTING_";
+    private static final Pattern DOCKER_LOWERCASE_SETTING_REGEX = Pattern.compile("[-a-z0-9_]+(\\.[-a-z0-9_]+)+");
+
     private final OptionSpec<KeyValuePair> settingOption;
 
     /**
@@ -48,6 +53,27 @@ protected void execute(Terminal terminal, OptionSet options) throws Exception {
         execute(terminal, options, createEnv(options));
     }
 
+    private void putDockerEnvSettings(Map<String, String> settings, Map<String, String> envVars) {
+        for (var envVar : envVars.entrySet()) {
+            String key = envVar.getKey();
+            if (DOCKER_LOWERCASE_SETTING_REGEX.matcher(key).matches()) {
+                // all lowercase, like cluster.name, so just put directly
+                settings.put(key, envVar.getValue());
+            } else if (key.startsWith(DOCKER_UPPERCASE_SETTING_PREFIX)) {
+                // remove prefix
+                key = key.substring(DOCKER_UPPERCASE_SETTING_PREFIX.length());
+                // insert dots for underscores
+                key = key.replace('_', '.');
+                // unescape double dots, which were originally double underscores
+                key = key.replace("..", "_");
+                // lowercase the whole thing
+                key = key.toLowerCase(Locale.ROOT);
+
+                settings.put(key, envVar.getValue());
+            }
+        }
+    }
+
     /** Create an {@link Environment} for the command to use. Overrideable for tests. */
     protected Environment createEnv(OptionSet options) throws UserException {
         final Map<String, String> settings = new HashMap<>();
@@ -68,6 +94,10 @@ protected Environment createEnv(OptionSet options) throws UserException {
             settings.put(kvp.key, kvp.value);
         }
 
+        if (getBuildType() == Build.Type.DOCKER) {
+            putDockerEnvSettings(settings, envVars);
+        }
+
         putSystemPropertyIfSettingIsMissing(sysprops, settings, "path.data", "es.path.data");
         putSystemPropertyIfSettingIsMissing(sysprops, settings, "path.home", "es.path.home");
         putSystemPropertyIfSettingIsMissing(sysprops, settings, "path.logs", "es.path.logs");
@@ -85,6 +115,11 @@ protected Environment createEnv(OptionSet options) throws UserException {
         );
     }
 
+    // protected to allow tests to override
+    protected Build.Type getBuildType() {
+        return Build.CURRENT.type();
+    }
+
     @SuppressForbidden(reason = "need path to construct environment")
     private static Path getConfigPath(final String pathConf) {
         return Paths.get(pathConf);