Improve and test jetty.sh behaviors (#10753)

* Issue #10696 - Addressing start-stop-daemon behaviors in jetty.sh
* disable internal pid-file management of start-stop-daemon
* IssueDo not test for file system permissions if user is root, or process will switch to JETTY_USER
* Fixing bad UID / JETTY_USER condition
* Avoid FS test with setuid use as well
* Fixing stop behavior
* Adding jetty.sh docker testing

---------

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Co-authored-by: Olivier Lamy <olamy@apache.org>

Author: joakime

jetty-home/src/main/resources/bin/jetty.sh

@@ -216,6 +216,46 @@ pidKill()
   fi
 }
 
+testFileSystemPermissions()
+{
+  # Don't test file system permissions if user is root
+  if [ $UID -eq 0 ] ; then
+    (( DEBUG )) && echo "Not testing file system permissions: uid is 0"
+    return 0
+  fi
+
+  # Don't test if JETTY_USER is specified
+  # as the Jetty process will switch to a different user id on startup
+  if [ -n "$JETTY_USER" ] ; then
+    (( DEBUG )) && echo "Not testing file system permissions: JETTY_USER=$JETTY_USER"
+    return 0
+  fi
+
+  # Don't test if setuid is specified
+  # as the Jetty process will switch to a different user id on startup
+  if expr "${JETTY_ARGS[*]}" : '.*setuid.*' >/dev/null
+  then
+    (( DEBUG )) && echo "Not testing file system permissions: setuid in use"
+    return 0
+  fi
+
+  # Test if PID can be written from this userid
+  if ! touch "$JETTY_PID"
+  then
+    echo "** ERROR: Unable to touch file: $JETTY_PID"
+    echo "          Correct issues preventing use of \$JETTY_PID and try again."
+    exit 1
+  fi
+
+  # Test if STATE can be written from this userid
+  if ! touch "$JETTY_STATE"
+  then
+    echo "** ERROR: Unable to touch file: $JETTY_STATE"
+    echo "          Correct issues preventing use of \$JETTY_STATE and try again."
+    exit 1
+  fi
+}
+
 readConfig()
 {
   (( DEBUG )) && echo "Reading $1.."
@@ -240,6 +280,10 @@ dumpEnv()
   echo "JETTY_START_TIMEOUT   =  $JETTY_START_TIMEOUT"
   echo "JETTY_SYS_PROPS       =  $JETTY_SYS_PROPS"
   echo "RUN_ARGS              =  ${RUN_ARGS[*]}"
+  echo "ID                    =  $(id)"
+  echo "JETTY_USER            =  $JETTY_USER"
+  echo "USE_START_STOP_DAEMON =  $USE_START_STOP_DAEMON"
+  echo "START_STOP_DAEMON     =  $START_STOP_DAEMON_AVAILABLE"
 }
 
 
@@ -249,6 +293,7 @@ dumpEnv()
 CONFIGS=()
 NO_START=0
 DEBUG=0
+USE_START_STOP_DAEMON=1
 
 while [[ $1 = -* ]]; do
   case $1 in
@@ -404,14 +449,14 @@ case "`uname`" in
 CYGWIN*) JETTY_STATE="`cygpath -w $JETTY_STATE`";;
 esac
 
-
 JETTY_ARGS=(${JETTY_ARGS[*]} "jetty.state=$JETTY_STATE" "jetty.pid=$JETTY_PID")
 
 ##################################################
 # Get the list of config.xml files from jetty.conf
 ##################################################
 if [ -f "$JETTY_CONF" ] && [ -r "$JETTY_CONF" ]
 then
+  (( DEBUG )) && echo "$JETTY_CONF: (begin read) JETTY_ARGS.length=${#JETTY_ARGS[@]}"
   while read -r CONF
   do
     if expr "$CONF" : '#' >/dev/null ; then
@@ -427,16 +472,17 @@ then
       do
         if [ -r "$XMLFILE" ] && [ -f "$XMLFILE" ]
         then
-          JETTY_ARGS=(${JETTY_ARGS[*]} "$XMLFILE")
+          JETTY_ARGS[${#JETTY_ARGS[@]}]=$XMLFILE
         else
           echo "** WARNING: Cannot read '$XMLFILE' specified in '$JETTY_CONF'"
         fi
       done
     else
       # assume it's a command line parameter (let start.jar deal with its validity)
-      JETTY_ARGS=(${JETTY_ARGS[*]} "$CONF")
+      JETTY_ARGS[${#JETTY_ARGS[@]}]=$CONF
     fi
   done < "$JETTY_CONF"
+  (( DEBUG )) && echo "$JETTY_CONF: (finished read) JETTY_ARGS.length=${#JETTY_ARGS[@]}"
 fi
 
 ##################################################
@@ -507,8 +553,22 @@ case "`uname`" in
 CYGWIN*) JETTY_START="`cygpath -w $JETTY_START`";;
 esac
 
+# Determine if we can use start-stop-daemon or not
+START_STOP_DAEMON_AVAILABLE=0
+
+if (( USE_START_STOP_DAEMON ))
+then
+  # only if root user is executing jetty.sh, and the start-stop-daemon exists
+  if [ $UID -eq 0 ] && type start-stop-daemon > /dev/null 2>&1
+  then
+    START_STOP_DAEMON_AVAILABLE=1
+  else
+    USE_START_STOP_DAEMON=0
+  fi
+fi
+
 # Collect the dry-run (of opts,path,main,args) from the jetty.base configuration
-JETTY_DRY_RUN=$("$JAVA" -jar "$JETTY_START" --dry-run=opts,path,main,args ${JETTY_ARGS[*]} ${JAVA_OPTIONS[*]})
+JETTY_DRY_RUN=$(echo "${JETTY_ARGS[*]} ${JAVA_OPTIONS[*]}" | xargs "$JAVA" -jar "$JETTY_START" --dry-run=opts,path,main,args)
 RUN_ARGS=($JETTY_SYS_PROPS ${JETTY_DRY_RUN[@]})
 
 if (( DEBUG ))
@@ -531,38 +591,27 @@ case "$ACTION" in
       exit
     fi
 
-    if ! touch "$JETTY_PID"
-    then
-      echo "** ERROR: Unable to touch file: $JETTY_PID"
-      echo "          Correct issues preventing use of \$JETTY_PID and try again."
-      exit 1
-    fi
-
-    if ! touch "$JETTY_STATE"
-    then
-      echo "** ERROR: Unable to touch file: $JETTY_STATE"
-      echo "          Correct issues preventing use of \$JETTY_STATE and try again."
-      exit 1
-    fi
+    testFileSystemPermissions
 
     echo -n "Starting Jetty: "
 
     # Startup from a service file
-    if [ $UID -eq 0 ] && type start-stop-daemon > /dev/null 2>&1
+    if (( USE_START_STOP_DAEMON ))
     then
       unset CH_USER
       if [ -n "$JETTY_USER" ]
       then
         CH_USER="--chuid $JETTY_USER"
       fi
 
-      echo ${RUN_ARGS[@]} --start-log-file="$JETTY_START_LOG" | xargs start-stop-daemon \
+      # use of --pidfile /dev/null disables internal pidfile
+      # management of the start-stop-daemon (see man page)
+      echo ${RUN_ARGS[@]} | xargs start-stop-daemon \
        --start $CH_USER \
-       --pidfile "$JETTY_PID" \
+       --pidfile /dev/null \
        --chdir "$JETTY_BASE" \
        --background \
-       --output "${JETTY_RUN}/start-stop.log"
-       --make-pidfile \
+       --output "${JETTY_RUN}/start-stop.log" \
        --startas "$JAVA" \
        --
       (( DEBUG )) && echo "Starting: start-stop-daemon"
@@ -618,25 +667,41 @@ case "$ACTION" in
 
   stop)
     echo -n "Stopping Jetty: "
-    # Stop from a service file
-    if [ $UID -eq 0 ] && type start-stop-daemon > /dev/null 2>&1; then
-      start-stop-daemon -K -p"$JETTY_PID" -d"$JETTY_HOME" -a "$JAVA" -s HUP
+    if [ ! -r "$JETTY_PID" ] ; then
+      echo "** ERROR: no pid found at $JETTY_PID"
+      exit 1
+    fi
+
+    PID=$(tail -1 "$JETTY_PID")
+    if [ -z "$PID" ] ; then
+      echo "** ERROR: no pid found in $JETTY_PID"
+      exit 1
+    fi
+
+    # Stopping service started with start-stop-daemon
+    if (( USE_START_STOP_DAEMON )) ; then
+      (( DEBUG )) && echo "Issuing HUP to $PID"
+      start-stop-daemon --stop \
+         --pid "$PID" \
+         --chdir "$JETTY_BASE" \
+         --startas "$JAVA" \
+         --signal HUP
 
       TIMEOUT=30
       while running "$JETTY_PID"; do
+        (( DEBUG )) && echo "Issuing KILL to $PID"
         if (( TIMEOUT-- == 0 )); then
-          start-stop-daemon -K -p"$JETTY_PID" -d"$JETTY_HOME" -a "$JAVA" -s KILL
+          start-stop-daemon --stop \
+            --pid "$PID" \
+            --chdir "$JETTY_BASE" \
+            --startas "$JAVA" \
+            --signal KILL
         fi
 
         sleep 1
       done
     else
-      # Stop from a non-service path
-      if [ ! -r "$JETTY_PID" ] ; then
-        echo "** ERROR: no pid found at $JETTY_PID"
-        exit 1
-      fi
-
+      # Stopping from non-service start
       pidKill "$JETTY_PID" 30
     fi
 

tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/jettysh/ImageFromDSL.java

@@ -0,0 +1,51 @@
+//
+// ========================================================================
+// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.tests.distribution.jettysh;
+
+import java.util.function.Consumer;
+
+import org.testcontainers.images.builder.ImageFromDockerfile;
+import org.testcontainers.images.builder.dockerfile.DockerfileBuilder;
+
+/**
+ * Simplify the use of {@link ImageFromDockerfile} to get some sanity in naming convention
+ * and {@link #toString()} behaviors so that Test execution makes sense.
+ */
+public class ImageFromDSL extends ImageFromDockerfile
+{
+    private ImageFromDSL parentImage;
+
+    public ImageFromDSL(ImageFromDSL baseImage, String suffix, Consumer<DockerfileBuilder> builderConsumer)
+    {
+        this(baseImage.getDockerImageName() + "-" + suffix, builderConsumer);
+        this.parentImage = baseImage;
+    }
+
+    public ImageFromDSL(String name, Consumer<DockerfileBuilder> builderConsumer)
+    {
+        super(name, false);
+        withDockerfileFromBuilder(builderConsumer);
+    }
+
+    public ImageFromDSL getParentImage()
+    {
+        return parentImage;
+    }
+
+    @Override
+    public String toString()
+    {
+        return getDockerImageName();
+    }
+}

tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/jettysh/ImageOS.java

@@ -0,0 +1,55 @@
+//
+// ========================================================================
+// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.tests.distribution.jettysh;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.function.Consumer;
+
+import org.eclipse.jetty.tests.hometester.JettyHomeTester;
+import org.testcontainers.images.builder.dockerfile.DockerfileBuilder;
+
+public abstract class ImageOS extends ImageFromDSL
+{
+    public static final String REGISTRY = "registry.jetty.org";
+    public static final String REPOSITORY = REGISTRY + "/jetty-sh";
+
+    private Path jettyHomePath;
+
+    public ImageOS(String osid, Consumer<DockerfileBuilder> builderConsumer)
+    {
+        super(REPOSITORY + ":" + osid, builderConsumer);
+    }
+
+    protected File getJettyHomeDir()
+    {
+        if (jettyHomePath == null)
+        {
+            String jettyVersion = System.getProperty("jettyVersion");
+            try
+            {
+                JettyHomeTester homeTester = JettyHomeTester.Builder.newInstance()
+                    .jettyVersion(jettyVersion)
+                    .mavenLocalRepository(System.getProperty("mavenRepoPath"))
+                    .build();
+                jettyHomePath = homeTester.getJettyHome();
+            }
+            catch (Exception e)
+            {
+                throw new RuntimeException("Unable to get unpacked JETTY_HOME dir", e);
+            }
+        }
+        return jettyHomePath.toFile();
+    }
+}

tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/jettysh/ImageOSAmazonCorretto11.java

@@ -0,0 +1,50 @@
+//
+// ========================================================================
+// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.tests.distribution.jettysh;
+
+/**
+ * An OS Image of linux specific for running on Amazon AWS.
+ * This is based on Amazon Linux 2 (which is based on Alpine 3).
+ * Amazon Corretto JDK 11 is installed.
+ * This image does NOT come with start-stop-daemon installed.
+ * Instead of apt, it uses yum (the redhat package manager)
+ */
+public class ImageOSAmazonCorretto11 extends ImageOS
+{
+    public ImageOSAmazonCorretto11()
+    {
+        super("amazoncorretto-jdk11",
+            builder ->
+                builder
+                    .from("amazoncorretto:11.0.20")
+                    .run("yum update -y ; " +
+                        "yum install -y curl tar gzip vim shadow-utils net-tools")
+                    .env("TEST_DIR", "/var/test")
+                    .env("JETTY_HOME", "$TEST_DIR/jetty-home")
+                    .env("JETTY_BASE", "$TEST_DIR/jetty-base")
+                    .env("PATH", "$PATH:${JETTY_HOME}/bin/")
+                    .user("root")
+                    // Configure /etc/default/jetty
+                    .run("echo \"JETTY_HOME=${JETTY_HOME}\" > /etc/default/jetty ; " +
+                        "echo \"JETTY_BASE=${JETTY_BASE}\" >> /etc/default/jetty ; " +
+                        "echo \"JETTY_RUN=${JETTY_BASE}\" >> /etc/default/jetty ")
+                    // setup Jetty Home
+                    .copy("/opt/jetty/", "${JETTY_HOME}/")
+                    .env("PATH", "$PATH:${JETTY_HOME}/bin/")
+                    .run("chmod ugo+x ${JETTY_HOME}/bin/jetty.sh")
+                    .build()
+        );
+        withFileFromFile("/opt/jetty/", getJettyHomeDir());
+    }
+}