Skip to content

Commit

Permalink
Fix passing positional args to ES in Docker (#88502)
Browse files Browse the repository at this point in the history
As part of #50277, we removed the `TAKE_FILE_OWNERSHIP` option from the
Docker entrypoint script and the associated chroot calls, and instead
just defaulted to running the image as `elasticsearch` instead of
`root`.

However, we didn't check that it was still possible to pass CLI options
to Elasticsearch via CLI arguments, and broke this by mistake. This is
probably an uncommon pattern, versus environment variables or a config
file.  Nevertheless, it is supposed to be possible and is mentioned in
the documentation.

Fix the problem by suppling the missing positional params when calling
Elasticsearch, and add a test case so that we don't break it again.
  • Loading branch information
pugnascotia committed Jul 14, 2022
1 parent 6d889f7 commit 3776923
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 2 deletions.
2 changes: 1 addition & 1 deletion distribution/docker/src/docker/bin/docker-entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,4 @@ fi

# Signal forwarding and child reaping is handled by `tini`, which is the
# actual entrypoint of the container
exec /usr/share/elasticsearch/bin/elasticsearch $POSITIONAL_PARAMETERS <<<"$KEYSTORE_PASSWORD"
exec /usr/share/elasticsearch/bin/elasticsearch "$@" $POSITIONAL_PARAMETERS <<<"$KEYSTORE_PASSWORD"
5 changes: 5 additions & 0 deletions docs/changelog/88502.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 88502
summary: Fix passing positional args to ES in Docker
area: Packaging
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -1096,6 +1096,18 @@ public void test170DefaultShellIsBash() {
}
}

/**
* Ensure that it is possible to apply CLI options when running the image.
*/
public void test171AdditionalCliOptionsAreForwarded() throws Exception {
runContainer(distribution(), builder().runArgs("bin/elasticsearch", "-Ecluster.name=kimchy").envVar("ELASTIC_PASSWORD", PASSWORD));
waitForElasticsearch(installation, "elastic", PASSWORD);

final JsonNode node = getJson("/", "elastic", PASSWORD, ServerUtils.getCaCert(installation));

assertThat(node.get("cluster_name").textValue(), equalTo("kimchy"));
}

/**
* Check that the UBI images has the correct license information in the correct place.
*/
Expand Down Expand Up @@ -1193,7 +1205,7 @@ private List<String> listPlugins() {
/**
* Check that readiness listener works
*/
public void testReadiness001() throws Exception {
public void test500Readiness() throws Exception {
assertFalse(readinessProbe(9399));
// Disabling security so we wait for green
installation = runContainer(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public class DockerRun {
private Integer uid;
private Integer gid;
private final List<String> extraArgs = new ArrayList<>();
private final List<String> runArgs = new ArrayList<>();
private String memory = "2g"; // default to 2g memory limit

private DockerRun() {}
Expand Down Expand Up @@ -95,6 +96,11 @@ public DockerRun extraArgs(String... args) {
return this;
}

public DockerRun runArgs(String... args) {
Collections.addAll(this.runArgs, args);
return this;
}

String build() {
final List<String> cmd = new ArrayList<>();

Expand Down Expand Up @@ -144,6 +150,8 @@ String build() {
// Image name
cmd.add(getImageName(distribution));

cmd.addAll(this.runArgs);

return String.join(" ", cmd);
}

Expand Down

0 comments on commit 3776923

Please sign in to comment.