Move docker env var settings handling out of bash

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;
 
@@ -59,8 +60,8 @@ public abstract class Command implements Closeable {
     public Command(final String description, final Runnable beforeMain) {
         this.description = description;
         this.beforeMain = beforeMain;
-        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;
 
     /**
@@ -59,6 +64,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<>();
@@ -79,6 +105,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");
@@ -96,6 +126,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);