Fix passing positional args to ES in Docker (#88502)

pugnascotia · pugnascotia · commit 3776923e458c · 2022-07-14T09:12:55.000+01:00
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.
diff --git 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
@@ -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
@@ -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
@@ -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);
     }
 
