Refactor environment variable processing for Docker

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/

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

@@ -42,58 +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`, except for `processors` which we handle
-  # specially
-  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)
+# 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
 
 # The virtual file /proc/self/cgroup should list the current cgroup
 # membership. For each hierarchy, you can follow the cgroup path from
@@ -131,4 +84,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,38 @@ 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`, except for `processors` which we handle
+    # specially
+    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[@]}"
+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
+

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

@@ -306,6 +306,23 @@ public void test82EnvironmentVariablesUsingFilesHaveCorrectPermissions() 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() {
+        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.

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/tool/CronEvalTool.java

@@ -7,11 +7,12 @@
 
 import joptsimple.OptionSet;
 import joptsimple.OptionSpec;
+import org.elasticsearch.cli.EnvironmentAwareCommand;
 import org.elasticsearch.cli.ExitCodes;
-import org.elasticsearch.cli.LoggingAwareCommand;
 import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.cli.UserException;
 import org.elasticsearch.common.time.DateFormatter;
+import org.elasticsearch.env.Environment;
 import org.elasticsearch.xpack.core.scheduler.Cron;
 
 import java.time.Instant;
@@ -22,7 +23,12 @@
 import java.util.List;
 import java.util.Locale;
 
-public class CronEvalTool extends LoggingAwareCommand {
+/*
+ * This class extends EnvironmentAwareCommand so that -E options are accepted,
+ * even though they are unused. This makes the CLI interface the same as the
+ * other CLI tools.
+ */
+public class CronEvalTool extends EnvironmentAwareCommand {
 
     public static void main(String[] args) throws Exception {
         exit(new CronEvalTool().main(args, Terminal.DEFAULT));
@@ -46,8 +52,11 @@ public static void main(String[] args) throws Exception {
         this.arguments = parser.nonOptions("expression");
     }
 
+    /*
+     * env is not used here, but is part of the EnvironmentAwareCommand contract.
+     */
     @Override
-    protected void execute(Terminal terminal, OptionSet options) throws Exception {
+    protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception {
         int count = countOption.value(options);
         List<String> args = arguments.values(options);
         if (args.size() != 1) {
@@ -56,7 +65,7 @@ protected void execute(Terminal terminal, OptionSet options) throws Exception {
         execute(terminal, args.get(0), count);
     }
 
-    void execute(Terminal terminal, String expression, int count) throws Exception {
+    private void execute(Terminal terminal, String expression, int count) throws Exception {
         Cron.validate(expression);
         terminal.println("Valid!");