Skip to content

Commit 9f069f7

Browse files
pugnascotiapugnascotia
authored
Refactor environment variable processing for Docker (#49612)
Closes #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.
1 parent abd969e commit 9f069f7

File tree

10 files changed

+193
-96
lines changed

10 files changed

+193
-96
lines changed

‎distribution/docker/src/docker/Dockerfile

+2-2
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ ${source_elasticsearch}
2929

3030
RUN tar zxf /opt/${elasticsearch} --strip-components=1
3131
RUN grep ES_DISTRIBUTION_TYPE=tar /usr/share/elasticsearch/bin/elasticsearch-env \
32-
&& sed -ie 's/ES_DISTRIBUTION_TYPE=tar/ES_DISTRIBUTION_TYPE=docker/' /usr/share/elasticsearch/bin/elasticsearch-env
32+
&& sed -i -e 's/ES_DISTRIBUTION_TYPE=tar/ES_DISTRIBUTION_TYPE=docker/' /usr/share/elasticsearch/bin/elasticsearch-env
3333
RUN mkdir -p config data logs
3434
RUN chmod 0775 config data logs
3535
COPY config/elasticsearch.yml config/log4j2.properties config/
@@ -46,7 +46,7 @@ FROM ${base_image}
4646
ENV ELASTIC_CONTAINER true
4747

4848
RUN for iter in {1..10}; do ${package_manager} update --setopt=tsflags=nodocs -y && \
49-
${package_manager} install --setopt=tsflags=nodocs -y nc shadow-utils && \
49+
${package_manager} install --setopt=tsflags=nodocs -y nc shadow-utils zip unzip && \
5050
${package_manager} clean all && exit_code=0 && break || exit_code=\$? && echo "${package_manager} error: retry \$iter in 10s" && sleep 10; done; \
5151
(exit \$exit_code)
5252

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

+5-65
Original file line numberDiff line numberDiff line change
@@ -42,71 +42,11 @@ fi
4242
# contents, and setting an environment variable with the suffix _FILE to
4343
# point to it. This can be used to provide secrets to a container, without
4444
# the values being specified explicitly when running the container.
45-
for VAR_NAME_FILE in $(env | cut -f1 -d= | grep '_FILE$'); do
46-
if [[ -n "$VAR_NAME_FILE" ]]; then
47-
VAR_NAME="${VAR_NAME_FILE%_FILE}"
48-
49-
if env | grep "^${VAR_NAME}="; then
50-
echo "ERROR: Both $VAR_NAME_FILE and $VAR_NAME are set. These are mutually exclusive." >&2
51-
exit 1
52-
fi
53-
54-
if [[ ! -e "${!VAR_NAME_FILE}" ]]; then
55-
echo "ERROR: File ${!VAR_NAME_FILE} from $VAR_NAME_FILE does not exist" >&2
56-
exit 1
57-
fi
58-
59-
FILE_PERMS="$(stat -c '%a' ${!VAR_NAME_FILE})"
60-
61-
if [[ "$FILE_PERMS" != "400" && "$FILE_PERMS" != 600 ]]; then
62-
echo "ERROR: File ${!VAR_NAME_FILE} from $VAR_NAME_FILE must have file permissions 400 or 600, but actually has: $FILE_PERMS" >&2
63-
exit 1
64-
fi
65-
66-
echo "Setting $VAR_NAME from $VAR_NAME_FILE at ${!VAR_NAME_FILE}" >&2
67-
export "$VAR_NAME"="$(cat ${!VAR_NAME_FILE})"
68-
69-
unset VAR_NAME
70-
# Unset the suffixed environment variable
71-
unset "$VAR_NAME_FILE"
72-
fi
73-
done
74-
75-
# Parse Docker env vars to customize Elasticsearch
76-
#
77-
# e.g. Setting the env var cluster.name=testcluster
7845
#
79-
# will cause Elasticsearch to be invoked with -Ecluster.name=testcluster
80-
#
81-
# see https://www.elastic.co/guide/en/elasticsearch/reference/current/settings.html#_setting_default_settings
82-
83-
declare -a es_opts
84-
85-
while IFS='=' read -r envvar_key envvar_value
86-
do
87-
# Elasticsearch settings need to have at least two dot separated lowercase
88-
# words, e.g. `cluster.name`
89-
if [[ "$envvar_key" =~ ^[a-z0-9_]+\.[a-z0-9_]+ ]]; then
90-
if [[ ! -z $envvar_value ]]; then
91-
es_opt="-E${envvar_key}=${envvar_value}"
92-
es_opts+=("${es_opt}")
93-
fi
94-
fi
95-
done < <(env)
96-
97-
# The virtual file /proc/self/cgroup should list the current cgroup
98-
# membership. For each hierarchy, you can follow the cgroup path from
99-
# this file to the cgroup filesystem (usually /sys/fs/cgroup/) and
100-
# introspect the statistics for the cgroup for the given
101-
# hierarchy. Alas, Docker breaks this by mounting the container
102-
# statistics at the root while leaving the cgroup paths as the actual
103-
# paths. Therefore, Elasticsearch provides a mechanism to override
104-
# reading the cgroup path from /proc/self/cgroup and instead uses the
105-
# cgroup path defined the JVM system property
106-
# es.cgroups.hierarchy.override. Therefore, we set this value here so
107-
# that cgroup statistics are available for the container this process
108-
# will run in.
109-
export ES_JAVA_OPTS="-Des.cgroups.hierarchy.override=/ $ES_JAVA_OPTS"
46+
# This is also sourced in elasticsearch-env, and is only needed here
47+
# as well because we use ELASTIC_PASSWORD below. Sourcing this script
48+
# is idempotent.
49+
source /usr/share/elasticsearch/bin/elasticsearch-env-from-file
11050

11151
if [[ -f bin/elasticsearch-users ]]; then
11252
# Check for the ELASTIC_PASSWORD environment variable to set the
@@ -130,4 +70,4 @@ if [[ "$(id -u)" == "0" ]]; then
13070
fi
13171
fi
13272

133-
run_as_other_user_if_needed /usr/share/elasticsearch/bin/elasticsearch "${es_opts[@]}"
73+
run_as_other_user_if_needed /usr/share/elasticsearch/bin/elasticsearch

‎distribution/src/bin/elasticsearch-env

+47
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,51 @@ ES_DISTRIBUTION_FLAVOR=${es.distribution.flavor}
8686
ES_DISTRIBUTION_TYPE=${es.distribution.type}
8787
ES_BUNDLED_JDK=${es.bundled_jdk}
8888

89+
if [[ "$ES_DISTRIBUTION_TYPE" == "docker" ]]; then
90+
# Allow environment variables to be set by creating a file with the
91+
# contents, and setting an environment variable with the suffix _FILE to
92+
# point to it. This can be used to provide secrets to a container, without
93+
# the values being specified explicitly when running the container.
94+
source "$ES_HOME/bin/elasticsearch-env-from-file"
95+
96+
# Parse Docker env vars to customize Elasticsearch
97+
#
98+
# e.g. Setting the env var cluster.name=testcluster
99+
#
100+
# will cause Elasticsearch to be invoked with -Ecluster.name=testcluster
101+
#
102+
# see https://www.elastic.co/guide/en/elasticsearch/reference/current/settings.html#_setting_default_settings
103+
104+
declare -a es_arg_array
105+
106+
while IFS='=' read -r envvar_key envvar_value
107+
do
108+
# Elasticsearch settings need to have at least two dot separated lowercase
109+
# words, e.g. `cluster.name`
110+
if [[ "$envvar_key" =~ ^[a-z0-9_]+\.[a-z0-9_]+ ]]; then
111+
if [[ ! -z $envvar_value ]]; then
112+
es_opt="-E${envvar_key}=${envvar_value}"
113+
es_arg_array+=("${es_opt}")
114+
fi
115+
fi
116+
done < <(env)
117+
118+
# Reset the positional parameters to the es_arg_array values and any existing positional params
119+
set -- "$@" "${es_arg_array[@]}"
120+
121+
# The virtual file /proc/self/cgroup should list the current cgroup
122+
# membership. For each hierarchy, you can follow the cgroup path from
123+
# this file to the cgroup filesystem (usually /sys/fs/cgroup/) and
124+
# introspect the statistics for the cgroup for the given
125+
# hierarchy. Alas, Docker breaks this by mounting the container
126+
# statistics at the root while leaving the cgroup paths as the actual
127+
# paths. Therefore, Elasticsearch provides a mechanism to override
128+
# reading the cgroup path from /proc/self/cgroup and instead uses the
129+
# cgroup path defined the JVM system property
130+
# es.cgroups.hierarchy.override. Therefore, we set this value here so
131+
# that cgroup statistics are available for the container this process
132+
# will run in.
133+
export ES_JAVA_OPTS="-Des.cgroups.hierarchy.override=/ $ES_JAVA_OPTS"
134+
fi
135+
89136
cd "$ES_HOME"

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

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
#!/bin/bash
2+
3+
set -e -o pipefail
4+
5+
# Allow environment variables to be set by creating a file with the
6+
# contents, and setting an environment variable with the suffix _FILE to
7+
# point to it. This can be used to provide secrets to a container, without
8+
# the values being specified explicitly when running the container.
9+
#
10+
# This script is intended to be sourced, not executed, and modifies the
11+
# environment.
12+
13+
for VAR_NAME_FILE in $(env | cut -f1 -d= | grep '_FILE$'); do
14+
if [[ -n "$VAR_NAME_FILE" ]]; then
15+
VAR_NAME="${VAR_NAME_FILE%_FILE}"
16+
17+
if env | grep "^${VAR_NAME}="; then
18+
echo "ERROR: Both $VAR_NAME_FILE and $VAR_NAME are set. These are mutually exclusive." >&2
19+
exit 1
20+
fi
21+
22+
if [[ ! -e "${!VAR_NAME_FILE}" ]]; then
23+
echo "ERROR: File ${!VAR_NAME_FILE} from $VAR_NAME_FILE does not exist" >&2
24+
exit 1
25+
fi
26+
27+
FILE_PERMS="$(stat -c '%a' ${!VAR_NAME_FILE})"
28+
29+
if [[ "$FILE_PERMS" != "400" && "$FILE_PERMS" != 600 ]]; then
30+
echo "ERROR: File ${!VAR_NAME_FILE} from $VAR_NAME_FILE must have file permissions 400 or 600, but actually has: $FILE_PERMS" >&2
31+
exit 1
32+
fi
33+
34+
echo "Setting $VAR_NAME from $VAR_NAME_FILE at ${!VAR_NAME_FILE}" >&2
35+
export "$VAR_NAME"="$(cat ${!VAR_NAME_FILE})"
36+
37+
unset VAR_NAME
38+
# Unset the suffixed environment variable
39+
unset "$VAR_NAME_FILE"
40+
fi
41+
done
42+

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

+20-6
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,14 @@
2121

2222
import joptsimple.NonOptionArgumentSpec;
2323
import joptsimple.OptionSet;
24+
import joptsimple.OptionSpec;
25+
import joptsimple.util.KeyValuePair;
2426
import org.elasticsearch.core.internal.io.IOUtils;
2527

2628
import java.io.IOException;
27-
import java.util.Arrays;
29+
import java.util.ArrayList;
2830
import java.util.LinkedHashMap;
31+
import java.util.List;
2932
import java.util.Map;
3033

3134
/**
@@ -36,6 +39,7 @@ public class MultiCommand extends Command {
3639
protected final Map<String, Command> subcommands = new LinkedHashMap<>();
3740

3841
private final NonOptionArgumentSpec<String> arguments = parser.nonOptions("command");
42+
private final OptionSpec<KeyValuePair> settingOption;
3943

4044
/**
4145
* 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 {
4549
*/
4650
public MultiCommand(final String description, final Runnable beforeMain) {
4751
super(description, beforeMain);
52+
this.settingOption = parser.accepts("E", "Configure a setting").withRequiredArg().ofType(KeyValuePair.class);
4853
parser.posixlyCorrect(true);
4954
}
5055

@@ -66,15 +71,24 @@ protected void execute(Terminal terminal, OptionSet options) throws Exception {
6671
if (subcommands.isEmpty()) {
6772
throw new IllegalStateException("No subcommands configured");
6873
}
69-
String[] args = arguments.values(options).toArray(new String[0]);
70-
if (args.length == 0) {
74+
75+
// .values(...) returns an unmodifiable list
76+
final List<String> args = new ArrayList<>(arguments.values(options));
77+
if (args.isEmpty()) {
7178
throw new UserException(ExitCodes.USAGE, "Missing command");
7279
}
73-
Command subcommand = subcommands.get(args[0]);
80+
81+
String subcommandName = args.remove(0);
82+
Command subcommand = subcommands.get(subcommandName);
7483
if (subcommand == null) {
75-
throw new UserException(ExitCodes.USAGE, "Unknown command [" + args[0] + "]");
84+
throw new UserException(ExitCodes.USAGE, "Unknown command [" + subcommandName + "]");
7685
}
77-
subcommand.mainWithoutErrorHandling(Arrays.copyOfRange(args, 1, args.length), terminal);
86+
87+
for (final KeyValuePair pair : this.settingOption.values(options)) {
88+
args.add("-E" + pair);
89+
}
90+
91+
subcommand.mainWithoutErrorHandling(args.toArray(new String[0]), terminal);
7892
}
7993

8094
@Override

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

+21
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,27 @@ public void test082EnvironmentVariablesUsingFilesHaveCorrectPermissions() throws
390390
);
391391
}
392392

393+
/**
394+
* Check that environment variables are translated to -E options even for commands invoked under
395+
* `docker exec`, where the Docker image's entrypoint is not executed.
396+
*/
397+
public void test83EnvironmentVariablesAreRespectedUnderDockerExec() {
398+
// This test relies on a CLI tool attempting to connect to Elasticsearch, and the
399+
// tool in question is only in the default distribution.
400+
assumeTrue(distribution.isDefault());
401+
402+
runContainer(distribution(), null, Map.of("http.host", "this.is.not.valid"));
403+
404+
// This will fail if the env var above is passed as a -E argument
405+
final Result result = sh.runIgnoreExitCode("elasticsearch-setup-passwords auto");
406+
407+
assertFalse("elasticsearch-setup-passwords command should have failed", result.isSuccess());
408+
assertThat(
409+
result.stdout,
410+
containsString("java.net.UnknownHostException: this.is.not.valid: Name or service not known")
411+
);
412+
}
413+
393414
/**
394415
* Check whether the elasticsearch-certutil tool has been shipped correctly,
395416
* and if present then it can execute.

‎qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java

+11-3
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,11 @@
4646
import static org.elasticsearch.packaging.util.FileMatcher.p775;
4747
import static org.elasticsearch.packaging.util.FileUtils.getCurrentVersion;
4848
import static org.elasticsearch.packaging.util.ServerUtils.makeRequest;
49-
import static org.hamcrest.CoreMatchers.containsString;
5049
import static org.hamcrest.MatcherAssert.assertThat;
50+
import static org.hamcrest.Matchers.containsString;
5151
import static org.hamcrest.Matchers.equalTo;
5252
import static org.junit.Assert.assertEquals;
53+
import static org.junit.Assert.assertTrue;
5354
import static org.junit.Assert.fail;
5455

5556
/**
@@ -276,7 +277,7 @@ public static class DockerShell extends Shell {
276277
protected String[] getScriptCommand(String script) {
277278
assert containerId != null;
278279

279-
return super.getScriptCommand("docker exec " + "--user elasticsearch:root " + "--tty " + containerId + " " + script);
280+
return super.getScriptCommand("docker exec --user elasticsearch:root --tty " + containerId + " " + script);
280281
}
281282
}
282283

@@ -438,14 +439,21 @@ private static void verifyOssInstallation(Installation es) {
438439
"elasticsearch",
439440
"elasticsearch-cli",
440441
"elasticsearch-env",
441-
"elasticsearch-enve",
442442
"elasticsearch-keystore",
443443
"elasticsearch-node",
444444
"elasticsearch-plugin",
445445
"elasticsearch-shard"
446446
).forEach(executable -> assertPermissionsAndOwnership(es.bin(executable), p755));
447447

448448
Stream.of("LICENSE.txt", "NOTICE.txt", "README.textile").forEach(doc -> assertPermissionsAndOwnership(es.home.resolve(doc), p644));
449+
450+
// These are installed to help users who are working with certificates.
451+
Stream.of("zip", "unzip").forEach(cliPackage -> {
452+
// We could run `yum list installed $pkg` but that causes yum to call out to the network.
453+
// rpm does the job just as well.
454+
final Shell.Result result = dockerShell.runIgnoreExitCode("rpm -q " + cliPackage);
455+
assertTrue(cliPackage + " ought to be installed. " + result, result.isSuccess());
456+
});
449457
}
450458

451459
private static void verifyDefaultInstallation(Installation es) {

0 commit comments

Comments
 (0)