From 9f4b32a20a776ff6c888e0534f176559c63be700 Mon Sep 17 00:00:00 2001
From: Rory Hunter <pugnascotia@users.noreply.github.com>
Date: Thu, 14 Jul 2022 09:12:04 +0100
Subject: [PATCH] Fix passing positional args to ES in Docker (#88502)

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.
---
 .../docker/src/docker/bin/docker-entrypoint.sh     |  2 +-
 docs/changelog/88502.yaml                          |  5 +++++
 .../elasticsearch/packaging/test/DockerTests.java  | 14 +++++++++++++-
 .../packaging/util/docker/DockerRun.java           |  8 ++++++++
 4 files changed, 27 insertions(+), 2 deletions(-)
 create mode 100644 docs/changelog/88502.yaml

diff --git a/distribution/docker/src/docker/bin/docker-entrypoint.sh b/distribution/docker/src/docker/bin/docker-entrypoint.sh
index 8ea9fcb2c0f86..d7b41b81bb7e3 100755
--- a/distribution/docker/src/docker/bin/docker-entrypoint.sh
+++ b/distribution/docker/src/docker/bin/docker-entrypoint.sh
@@ -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"
diff --git a/docs/changelog/88502.yaml b/docs/changelog/88502.yaml
new file mode 100644
index 0000000000000..f3e21006bd6ac
--- /dev/null
+++ b/docs/changelog/88502.yaml
@@ -0,0 +1,5 @@
+pr: 88502
+summary: Fix passing positional args to ES in Docker
+area: Packaging
+type: bug
+issues: []
diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java
index 0b4b48f8b87ea..1c006b3b563f2 100644
--- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java
+++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java
@@ -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.
      */
@@ -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(
diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/docker/DockerRun.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/docker/DockerRun.java
index caae6e2635c0f..feb95b5eb2d93 100644
--- a/qa/os/src/test/java/org/elasticsearch/packaging/util/docker/DockerRun.java
+++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/docker/DockerRun.java
@@ -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() {}
@@ -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<>();
 
@@ -144,6 +150,8 @@ String build() {
         // Image name
         cmd.add(getImageName(distribution));
 
+        cmd.addAll(this.runArgs);
+
         return String.join(" ", cmd);
     }