Refactor environment variable processing for Docker (elastic#49612)

Closes elastic#45223.

The current Docker entrypoint script picks up environment variables and
translates them into -E command line arguments. However, since any tool
executes via `docker exec` doesn't run the entrypoint, it results in
a poorer user experience.

Therefore, refactor the env var handling so that the -E options are
generated in `elasticsearch-env`. These have to be appended to any
existing command arguments, since some CLI tools have subcommands and
-E arguments must come after the subcommand.

Also extract the support for `_FILE` env vars into a separate script, so
that it can be called from more than once place (the behaviour is
idempotent).

Finally, add noop -E handling to CronEvalTool for parity, and support
`-E` in MultiCommand before subcommands.

distribution/docker/src/docker/Dockerfile

@@ -29,7 +29,7 @@ ${source_elasticsearch}
 
 RUN tar zxf /opt/${elasticsearch} --strip-components=1
 RUN grep ES_DISTRIBUTION_TYPE=tar /usr/share/elasticsearch/bin/elasticsearch-env \
-    && sed -ie 's/ES_DISTRIBUTION_TYPE=tar/ES_DISTRIBUTION_TYPE=docker/' /usr/share/elasticsearch/bin/elasticsearch-env
+    && sed -i -e 's/ES_DISTRIBUTION_TYPE=tar/ES_DISTRIBUTION_TYPE=docker/' /usr/share/elasticsearch/bin/elasticsearch-env
 RUN mkdir -p config data logs
 RUN chmod 0775 config data logs
 COPY config/elasticsearch.yml config/log4j2.properties config/
@@ -46,7 +46,7 @@ FROM ${base_image}
 ENV ELASTIC_CONTAINER true
 
 RUN for iter in {1..10}; do ${package_manager} update --setopt=tsflags=nodocs -y && \
-    ${package_manager} install --setopt=tsflags=nodocs -y nc shadow-utils && \
+    ${package_manager} install --setopt=tsflags=nodocs -y nc shadow-utils zip unzip && \
     ${package_manager} clean all && exit_code=0 && break || exit_code=\$? && echo "${package_manager} error: retry \$iter in 10s" && sleep 10; done; \
     (exit \$exit_code)
 

distribution/docker/src/docker/bin/docker-entrypoint.sh

@@ -42,71 +42,11 @@ fi
 # 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.
-for VAR_NAME_FILE in $(env | cut -f1 -d= | grep '_FILE$'); do
-  if [[ -n "$VAR_NAME_FILE" ]]; then
-    VAR_NAME="${VAR_NAME_FILE%_FILE}"
-
-    if env | grep "^${VAR_NAME}="; then
-      echo "ERROR: Both $VAR_NAME_FILE and $VAR_NAME are set. These are mutually exclusive." >&2
-      exit 1
-    fi
-
-    if [[ ! -e "${!VAR_NAME_FILE}" ]]; then
-      echo "ERROR: File ${!VAR_NAME_FILE} from $VAR_NAME_FILE does not exist" >&2
-      exit 1
-    fi
-
-    FILE_PERMS="$(stat -c '%a' ${!VAR_NAME_FILE})"
-
-    if [[ "$FILE_PERMS" != "400" && "$FILE_PERMS" != 600 ]]; then
-        echo "ERROR: File ${!VAR_NAME_FILE} from $VAR_NAME_FILE must have file permissions 400 or 600, but actually has: $FILE_PERMS" >&2
-        exit 1
-    fi
-
-    echo "Setting $VAR_NAME from $VAR_NAME_FILE at ${!VAR_NAME_FILE}" >&2
-    export "$VAR_NAME"="$(cat ${!VAR_NAME_FILE})"
-
-    unset VAR_NAME
-    # Unset the suffixed environment variable
-    unset "$VAR_NAME_FILE"
-  fi
-done
-
-# Parse Docker env vars to customize Elasticsearch
-#
-# e.g. Setting the env var 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_opts
-
-while IFS='=' read -r envvar_key envvar_value
-do
-  # Elasticsearch settings need to have at least two dot separated lowercase
-  # words, e.g. `cluster.name`
-  if [[ "$envvar_key" =~ ^[a-z0-9_]+\.[a-z0-9_]+ ]]; then
-    if [[ ! -z $envvar_value ]]; then
-      es_opt="-E${envvar_key}=${envvar_value}"
-      es_opts+=("${es_opt}")
-    fi
-  fi
-done < <(env)
-
-# The virtual file /proc/self/cgroup should list the current cgroup
-# membership. For each hierarchy, you can follow the cgroup path from
-# this file to the cgroup filesystem (usually /sys/fs/cgroup/) and
-# introspect the statistics for the cgroup for the given
-# hierarchy. Alas, Docker breaks this by mounting the container
-# statistics at the root while leaving the cgroup paths as the actual
-# paths. Therefore, Elasticsearch provides a mechanism to override
-# reading the cgroup path from /proc/self/cgroup and instead uses the
-# cgroup path defined the JVM system property
-# es.cgroups.hierarchy.override. Therefore, we set this value here so
-# that cgroup statistics are available for the container this process
-# will run in.
-export ES_JAVA_OPTS="-Des.cgroups.hierarchy.override=/ $ES_JAVA_OPTS"
+# This is also sourced in elasticsearch-env, and is only needed here
+# as well because we use ELASTIC_PASSWORD below. Sourcing this script
+# is idempotent.
+source /usr/share/elasticsearch/bin/elasticsearch-env-from-file
 
 if [[ -f bin/elasticsearch-users ]]; then
   # Check for the ELASTIC_PASSWORD environment variable to set the
@@ -130,4 +70,4 @@ if [[ "$(id -u)" == "0" ]]; then
   fi
 fi
 
-run_as_other_user_if_needed /usr/share/elasticsearch/bin/elasticsearch "${es_opts[@]}"
+run_as_other_user_if_needed /usr/share/elasticsearch/bin/elasticsearch

distribution/src/bin/elasticsearch-env

@@ -86,4 +86,51 @@ ES_DISTRIBUTION_FLAVOR=${es.distribution.flavor}
 ES_DISTRIBUTION_TYPE=${es.distribution.type}
 ES_BUNDLED_JDK=${es.bundled_jdk}
 
+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
+  #
+  # 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
+
+  while IFS='=' read -r envvar_key envvar_value
+  do
+    # Elasticsearch settings need to have at least two dot separated lowercase
+    # words, e.g. `cluster.name`
+    if [[ "$envvar_key" =~ ^[a-z0-9_]+\.[a-z0-9_]+ ]]; then
+      if [[ ! -z $envvar_value ]]; then
+        es_opt="-E${envvar_key}=${envvar_value}"
+        es_arg_array+=("${es_opt}")
+      fi
+    fi
+  done < <(env)
+
+  # 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
+  # this file to the cgroup filesystem (usually /sys/fs/cgroup/) and
+  # introspect the statistics for the cgroup for the given
+  # hierarchy. Alas, Docker breaks this by mounting the container
+  # statistics at the root while leaving the cgroup paths as the actual
+  # paths. Therefore, Elasticsearch provides a mechanism to override
+  # reading the cgroup path from /proc/self/cgroup and instead uses the
+  # cgroup path defined the JVM system property
+  # es.cgroups.hierarchy.override. Therefore, we set this value here so
+  # that cgroup statistics are available for the container this process
+  # will run in.
+  export ES_JAVA_OPTS="-Des.cgroups.hierarchy.override=/ $ES_JAVA_OPTS"
+fi
+
 cd "$ES_HOME"

distribution/src/bin/elasticsearch-env-from-file

@@ -0,0 +1,42 @@
+#!/bin/bash
+
+set -e -o pipefail
+
+# 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.
+#
+# This script is intended to be sourced, not executed, and modifies the
+# environment.
+
+for VAR_NAME_FILE in $(env | cut -f1 -d= | grep '_FILE$'); do
+  if [[ -n "$VAR_NAME_FILE" ]]; then
+    VAR_NAME="${VAR_NAME_FILE%_FILE}"
+
+    if env | grep "^${VAR_NAME}="; then
+      echo "ERROR: Both $VAR_NAME_FILE and $VAR_NAME are set. These are mutually exclusive." >&2
+      exit 1
+    fi
+
+    if [[ ! -e "${!VAR_NAME_FILE}" ]]; then
+      echo "ERROR: File ${!VAR_NAME_FILE} from $VAR_NAME_FILE does not exist" >&2
+      exit 1
+    fi
+
+    FILE_PERMS="$(stat -c '%a' ${!VAR_NAME_FILE})"
+
+    if [[ "$FILE_PERMS" != "400" && "$FILE_PERMS" != 600 ]]; then
+        echo "ERROR: File ${!VAR_NAME_FILE} from $VAR_NAME_FILE must have file permissions 400 or 600, but actually has: $FILE_PERMS" >&2
+        exit 1
+    fi
+
+    echo "Setting $VAR_NAME from $VAR_NAME_FILE at ${!VAR_NAME_FILE}" >&2
+    export "$VAR_NAME"="$(cat ${!VAR_NAME_FILE})"
+
+    unset VAR_NAME
+    # Unset the suffixed environment variable
+    unset "$VAR_NAME_FILE"
+  fi
+done
+

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

@@ -21,11 +21,14 @@
 
 import joptsimple.NonOptionArgumentSpec;
 import joptsimple.OptionSet;
+import joptsimple.OptionSpec;
+import joptsimple.util.KeyValuePair;
 import org.elasticsearch.core.internal.io.IOUtils;
 
 import java.io.IOException;
-import java.util.Arrays;
+import java.util.ArrayList;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 
 /**
@@ -36,6 +39,7 @@ public class MultiCommand extends Command {
     protected final Map<String, Command> subcommands = new LinkedHashMap<>();
 
     private final NonOptionArgumentSpec<String> arguments = parser.nonOptions("command");
+    private final OptionSpec<KeyValuePair> settingOption;
 
     /**
      * Construct the multi-command with the specified command description and runnable to execute before main is invoked.
@@ -45,6 +49,7 @@ public class MultiCommand extends Command {
      */
     public MultiCommand(final String description, final Runnable beforeMain) {
         super(description, beforeMain);
+        this.settingOption = parser.accepts("E", "Configure a setting").withRequiredArg().ofType(KeyValuePair.class);
         parser.posixlyCorrect(true);
     }
 
@@ -66,15 +71,24 @@ protected void execute(Terminal terminal, OptionSet options) throws Exception {
         if (subcommands.isEmpty()) {
             throw new IllegalStateException("No subcommands configured");
         }
-        String[] args = arguments.values(options).toArray(new String[0]);
-        if (args.length == 0) {
+
+        // .values(...) returns an unmodifiable list
+        final List<String> args = new ArrayList<>(arguments.values(options));
+        if (args.isEmpty()) {
             throw new UserException(ExitCodes.USAGE, "Missing command");
         }
-        Command subcommand = subcommands.get(args[0]);
+
+        String subcommandName = args.remove(0);
+        Command subcommand = subcommands.get(subcommandName);
         if (subcommand == null) {
-            throw new UserException(ExitCodes.USAGE, "Unknown command [" + args[0] + "]");
+            throw new UserException(ExitCodes.USAGE, "Unknown command [" + subcommandName + "]");
         }
-        subcommand.mainWithoutErrorHandling(Arrays.copyOfRange(args, 1, args.length), terminal);
+
+        for (final KeyValuePair pair : this.settingOption.values(options)) {
+            args.add("-E" + pair);
+        }
+
+        subcommand.mainWithoutErrorHandling(args.toArray(new String[0]), terminal);
     }
 
     @Override

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

@@ -390,6 +390,27 @@ public void test082EnvironmentVariablesUsingFilesHaveCorrectPermissions() throws
         );
     }
 
+    /**
+     * Check that environment variables are translated to -E options even for commands invoked under
+     * `docker exec`, where the Docker image's entrypoint is not executed.
+     */
+    public void test83EnvironmentVariablesAreRespectedUnderDockerExec() {
+        // This test relies on a CLI tool attempting to connect to Elasticsearch, and the
+        // tool in question is only in the default distribution.
+        assumeTrue(distribution.isDefault());
+
+        runContainer(distribution(), null, Map.of("http.host", "this.is.not.valid"));
+
+        // This will fail if the env var above is passed as a -E argument
+        final Result result = sh.runIgnoreExitCode("elasticsearch-setup-passwords auto");
+
+        assertFalse("elasticsearch-setup-passwords command should have failed", result.isSuccess());
+        assertThat(
+            result.stdout,
+            containsString("java.net.UnknownHostException: this.is.not.valid: Name or service not known")
+        );
+    }
+
     /**
      * 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/Docker.java

@@ -46,10 +46,11 @@
 import static org.elasticsearch.packaging.util.FileMatcher.p775;
 import static org.elasticsearch.packaging.util.FileUtils.getCurrentVersion;
 import static org.elasticsearch.packaging.util.ServerUtils.makeRequest;
-import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 /**
@@ -276,7 +277,7 @@ public static class DockerShell extends Shell {
         protected String[] getScriptCommand(String script) {
             assert containerId != null;
 
-            return super.getScriptCommand("docker exec " + "--user elasticsearch:root " + "--tty " + containerId + " " + script);
+            return super.getScriptCommand("docker exec --user elasticsearch:root --tty " + containerId + " " + script);
         }
     }
 
@@ -438,14 +439,21 @@ private static void verifyOssInstallation(Installation es) {
             "elasticsearch",
             "elasticsearch-cli",
             "elasticsearch-env",
-            "elasticsearch-enve",
             "elasticsearch-keystore",
             "elasticsearch-node",
             "elasticsearch-plugin",
             "elasticsearch-shard"
         ).forEach(executable -> assertPermissionsAndOwnership(es.bin(executable), p755));
 
         Stream.of("LICENSE.txt", "NOTICE.txt", "README.textile").forEach(doc -> assertPermissionsAndOwnership(es.home.resolve(doc), p644));
+
+        // These are installed to help users who are working with certificates.
+        Stream.of("zip", "unzip").forEach(cliPackage -> {
+            // We could run `yum list installed $pkg` but that causes yum to call out to the network.
+            // rpm does the job just as well.
+            final Shell.Result result = dockerShell.runIgnoreExitCode("rpm -q " + cliPackage);
+            assertTrue(cliPackage + " ought to be installed. " + result, result.isSuccess());
+        });
     }
 
     private static void verifyDefaultInstallation(Installation es) {