revert: changing owner while creating container for download support (#…

…2069) Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
SeleniumHQ · Dec 18, 2023 · 3e22a51 · 3e22a51
1 parent 56fc22a
commit 3e22a51
Show file tree

Hide file tree

Showing 8 changed files with 71 additions and 189 deletions.
diff --git a/Base/Dockerfile b/Base/Dockerfile
@@ -10,9 +10,10 @@ ARG GRPC_VERSION=1.57.1
 
 #Arguments to define the user running Selenium
 ARG SEL_USER=seluser
+ARG SEL_GROUP=${SEL_USER}
 ARG SEL_PASSWD=secret
-ARG SEL_UID=1200
-ARG SEL_GID=1201
+ARG UID=1200
+ARG GID=1201
 
 USER root
 #================================================
@@ -62,44 +63,24 @@ RUN ln -fs /usr/share/zoneinfo/${TZ} /etc/localtime && \
 # Configure environement
 #======================================
 ENV SEL_USER=${SEL_USER}
-ENV SEL_UID=${SEL_UID}
-ENV SEL_GID=${SEL_GID}
+ENV SEL_UID=${UID}
+ENV SEL_GID=${GID}
 ENV HOME=/home/${SEL_USER}
-ENV SEL_DIR=/opt/selenium
-ENV EXTERNAL_JARS=/external_jars
-ENV SE_DOWNLOAD_DIR=${HOME}/Downloads
-
-# Copy a script that we will use to correct permissions after running certain commands
-COPY fix-permissions /usr/local/bin/fix-permissions
-RUN chmod a+rx /usr/local/bin/fix-permissions
+ENV SEL_DOWNLOAD_DIR=${HOME}/Downloads
 
 #========================================
 # Add normal user and group with passwordless sudo
 #========================================
-RUN echo "auth requisite pam_deny.so" >> /etc/pam.d/su \
-    && sed -i.bak -e 's/^%admin/#%admin/' /etc/sudoers \
-    && sed -i.bak -e 's/^%sudo/#%sudo/' /etc/sudoers \
-    && echo 'ALL ALL = (ALL) NOPASSWD: ALL' >> /etc/sudoers
-
-RUN groupadd ${SEL_USER} \
-        --gid ${SEL_GID} \
-    && useradd ${SEL_USER} \
-        --no-log-init \
-        --create-home \
-        --gid ${SEL_GID} \
-        --shell /bin/bash \
-        --uid ${SEL_UID} \
-    && chmod g+w /etc/passwd \
-    && echo "${SEL_USER}:${SEL_PASSWD}" | chpasswd
-
-#======================================
-# Create directories needed
-#======================================
-RUN mkdir -p ${HOME}/.mozilla ${HOME}/.cache \
-      ${SEL_DIR} ${SEL_DIR}/assets \
-      /var/run/supervisor /var/log/supervisor \
-      ${EXTERNAL_JARS} \
-      ${SE_DOWNLOAD_DIR}
+RUN groupadd ${SEL_GROUP} \
+         --gid ${SEL_GID} \
+  && useradd ${SEL_USER} \
+         --create-home \
+         --gid ${SEL_GID} \
+         --shell /bin/bash \
+         --uid ${SEL_UID} \
+  && usermod -a -G sudo ${SEL_USER} \
+  && echo 'ALL ALL = (ALL) NOPASSWD: ALL' >> /etc/sudoers \
+  && echo "${SEL_USER}:${SEL_PASSWD}" | chpasswd
 
 #======================================
 # Add Grid check script
@@ -112,39 +93,42 @@ COPY --chown="${SEL_UID}:${SEL_GID}" check-grid.sh entry_point.sh /opt/bin/
 COPY supervisord.conf /etc
 
 #==========
-# Selenium
+# Selenium & relaxing permissions for OpenShift and other non-sudo environments
 #==========
-RUN touch ${SEL_DIR}/config.toml \
+RUN mkdir -p /opt/selenium /opt/selenium/assets /var/run/supervisor /var/log/supervisor ${SEL_DOWNLOAD_DIR} ${HOME}/.mozilla ${HOME}/.vnc \
+  && touch /opt/selenium/config.toml \
+  && chown -R ${SEL_USER}:${SEL_GROUP} /opt/selenium /var/run/supervisor /var/log/supervisor /etc/passwd ${HOME} \
+  && chmod -R 775 /opt/selenium /var/run/supervisor /var/log/supervisor /etc/passwd ${HOME} \
   && wget --no-verbose https://github.com/SeleniumHQ/selenium/releases/download/${RELEASE}/selenium-server-${VERSION}.jar \
-    -O ${SEL_DIR}/selenium-server.jar \
-  && echo "${SEL_PASSWD}" > ${SEL_DIR}/initialPasswd
+    -O /opt/selenium/selenium-server.jar \
+  && echo "${SEL_PASSWD}" > /opt/selenium/initialPasswd \
+  && chgrp -R 0 /opt/selenium ${HOME} /opt/selenium/assets /var/run/supervisor /var/log/supervisor \
+  && chmod -R g=u /opt/selenium ${HOME} /opt/selenium/assets /var/run/supervisor /var/log/supervisor \
+  && setfacl -Rm u:${SEL_USER}:rwx /opt /opt/selenium ${HOME} /opt/selenium/assets /var/run/supervisor /var/log/supervisor \
+  && setfacl -Rm g:${SEL_GROUP}:rwx /opt /opt/selenium ${HOME} /opt/selenium/assets /var/run/supervisor /var/log/supervisor
 
 #=====
 # Download observability related jaegar jars and make them available in a separate directory
 # so that the container can skip downloading them everytime it comes up
 #=====
 RUN curl -fLo /tmp/cs https://github.com/coursier/launchers/raw/master/coursier \
-  && chmod +x /tmp/cs
+  && chmod +x /tmp/cs \
+  && mkdir -p /external_jars \
+  && chmod -R 775 /external_jars
 
-RUN /tmp/cs fetch --classpath --cache ${EXTERNAL_JARS} \
+RUN /tmp/cs fetch --classpath --cache /external_jars \
     io.opentelemetry:opentelemetry-exporter-otlp:${OPENTELEMETRY_VERSION} \
     io.opentelemetry:opentelemetry-exporter-jaeger:${OPENTELEMETRY_VERSION} \
-    io.grpc:grpc-netty:${GRPC_VERSION} > ${EXTERNAL_JARS}/.classpath.txt
-RUN rm -fr /root/.cache/*
+    io.grpc:grpc-netty:${GRPC_VERSION} > /external_jars/.classpath.txt
 
-# Change ownership of directories
-RUN fix-permissions ${HOME} ${SEL_DIR} ${SEL_DIR}/assets ${EXTERNAL_JARS} ${SE_DOWNLOAD_DIR} /var/run/supervisor /var/log/supervisor
-
-#==========
-# Relaxing permissions for OpenShift and other non-sudo environments
-#==========
-RUN chmod g=u /etc/passwd
+RUN chmod 664 /external_jars/.classpath.txt
+RUN rm -fr /root/.cache/*
 
 #===================================================
 # Run the following commands as non-privileged user
 #===================================================
 USER ${SEL_UID}:${SEL_GID}
-VOLUME ${SE_DOWNLOAD_DIR}
+VOLUME ${SEL_DOWNLOAD_DIR}
 
 # Boolean value, maps "--bind-host"
 ENV SE_BIND_HOST false

diff --git a/Base/entry_point.sh b/Base/entry_point.sh
@@ -1,34 +1,14 @@
 #!/usr/bin/env bash
-_log () {
-    if [[ "$*" == "ERROR:"* ]] || [[ "$*" == "WARNING:"* ]] || [[ "${CONTAINER_LOGS_QUIET}" == "" ]]; then
-        echo "$@"
-    fi
-}
 
-# If the container started as the root user
-if [ "$(id -u)" == 0 ]; then
-    fix-permissions "${SE_DOWNLOAD_DIR}"
-elif [ "$(id -u)" == "$(id -u ${SEL_USER})" ] && [ "$(id -g)" == "$(id -g ${SEL_USER})" ]; then
-    # Trust SEL_USER is the desired non-root user to execute with sudo
-    sudo -E fix-permissions "${SE_DOWNLOAD_DIR}"
-else
-    # For non-root user to change ownership
-    # Relaxing permissions for OpenShift and other non-sudo environments
-    # (https://docs.openshift.com/container-platform/latest/openshift_images/create-images.html#use-uid_create-images)
-    if ! whoami &> /dev/null; then
-        _log "There is no entry in /etc/passwd for our UID=$(id -u). Attempting to fix..."
-        if [ -w /etc/passwd ]; then
-            _log "Renaming user to ${USER_NAME:-default} ($(id -u):$(id -g)"
-            # We cannot use "sed --in-place" since sed tries to create a temp file in
-            # /etc/ and we may not have write access. Apply sed on our own temp file:
-            sed --expression="s/^${SEL_USER}:/${USER_NAME:-default}:/" /etc/passwd > /tmp/passwd
-            echo "${USER_NAME:-default}:x:$(id -u):$(id -g):${USER_NAME:-default} user:${HOME}:/bin/bash" >> /tmp/passwd
-            cat /tmp/passwd > /etc/passwd
-            rm /tmp/passwd
-            _log "Added new ${USER_NAME:-default} user ($(id -u):$(id -g)). Fixed UID!"
-        fi
-    fi
-    fix-permissions "${SE_DOWNLOAD_DIR}"
+#==============================================
+# OpenShift or non-sudo environments support
+# https://docs.openshift.com/container-platform/3.11/creating_images/guidelines.html#openshift-specific-guidelines
+#==============================================
+
+if ! whoami &> /dev/null; then
+  if [ -w /etc/passwd ]; then
+    echo "${USER_NAME:-default}:x:$(id -u):0:${USER_NAME:-default} user:${HOME}:/sbin/nologin" >> /etc/passwd
+  fi
 fi
 
 /usr/bin/supervisord --configuration /etc/supervisord.conf &

diff --git a/Base/fix-permissions b/Base/fix-permissions
diff --git a/NodeBase/Dockerfile b/NodeBase/Dockerfile
@@ -114,18 +114,23 @@ RUN  wget -nv -O noVNC.zip \
 RUN chmod +x /dev/shm
 
 # Creating base directory for Xvfb
-RUN mkdir -p /tmp/.X11-unix && \
-    fix-permissions /tmp/.X11-unix
+RUN mkdir -p /tmp/.X11-unix
 
 #==============================
 # Generating the VNC password using initial password in Base image
 # Changing ownership to ${SEL_USER}, so the service can be started
 #==============================
 
 RUN mkdir -p ${HOME}/.vnc \
-    && x11vnc -storepasswd $(cat ${SEL_DIR}/initialPasswd) ${HOME}/.vnc/passwd \
-    && chown -R "${SEL_UID}:${SEL_GID}" ${HOME}/.vnc \
-    && fix-permissions ${HOME}/.vnc
+    && x11vnc -storepasswd $(cat /opt/selenium/initialPasswd) ${HOME}/.vnc/passwd \
+    && chown -R "${SEL_USER}:${SEL_GROUP}" ${HOME}/.vnc
+
+#==========
+# Relaxing permissions for OpenShift and other non-sudo environments
+#==========
+RUN chmod -R 775 ${HOME} /tmp/.X11-unix \
+  && chgrp -R 0 ${HOME} /tmp/.X11-unix \
+  && chmod -R g=u ${HOME} /tmp/.X11-unix
 
 #===================================================
 # Run the following commands as non-privileged user

diff --git a/NodeChrome/Dockerfile b/NodeChrome/Dockerfile
@@ -50,6 +50,7 @@ RUN if [ ! -z "$CHROME_DRIVER_VERSION" ]; \
   && unzip /tmp/chromedriver_linux64.zip -d /opt/selenium \
   && rm /tmp/chromedriver_linux64.zip \
   && mv /opt/selenium/chromedriver-linux64/chromedriver /opt/selenium/chromedriver-$CHROME_DRIVER_VERSION \
+  && chmod 755 /opt/selenium/chromedriver-$CHROME_DRIVER_VERSION \
   && ln -fs /opt/selenium/chromedriver-$CHROME_DRIVER_VERSION /usr/bin/chromedriver
 
 USER ${SEL_UID}

diff --git a/NodeEdge/Dockerfile b/NodeEdge/Dockerfile
@@ -43,6 +43,7 @@ RUN if [ -z "$EDGE_DRIVER_VERSION" ]; \
   && unzip /tmp/msedgedriver_linux64.zip -d /opt/selenium \
   && rm /tmp/msedgedriver_linux64.zip \
   && mv /opt/selenium/msedgedriver /opt/selenium/msedgedriver-$EDGE_DRIVER_VERSION \
+  && chmod 755 /opt/selenium/msedgedriver-$EDGE_DRIVER_VERSION \
   && ln -fs /opt/selenium/msedgedriver-$EDGE_DRIVER_VERSION /usr/bin/msedgedriver
 
 USER ${SEL_UID}

diff --git a/README.md b/README.md
@@ -900,6 +900,17 @@ $ BUILD_ARGS="--build-arg http_proxy=http://acme:3128 --build-arg https_proxy=ht
 _Note: Omitting_ `VERSION=local` _will build the images with the released version but replacing the date for the 
 current one._
 
+If you want to build the image with the host UID/GID, simply set an environment variable `BUILD_ARGS`
+
+``` bash
+$ BUILD_ARGS="--build-arg UID=$(id -u) --build-arg GID=$(id -g)" make build
+```
+
+If you want to build the image with different default user/password, simply set an environment variable `BUILD_ARGS`
+
+``` bash
+$ BUILD_ARGS="--build-arg SEL_USER=yourseluser --build-arg SEL_PASSWD=welcome" make build
+```
 ___
 
 ## Waiting for the Grid to be ready
@@ -1302,7 +1313,7 @@ that directory because it is running under the user
 `seluser`. This happens because that is how Docker mounts
 volumes in Linux, more details in this [issue](https://github.com/moby/moby/issues/2259).
 
-A workaround (to be done manually) for this is to create a directory on the
+A workaround for this is to create a directory on the
 host and change its permissions **before mounting the volume**. 
 Depending on your user permissions, you might need to use 
 `sudo` for some of these commands:
@@ -1313,61 +1324,7 @@ chown 1200:1201 /home/ubuntu/files
 ```
 
 After doing this, you should be able to download files
-to the mounted directory.
-
----
-Another introduced feature [#1947](https://github.com/SeleniumHQ/docker-selenium/issues/1947)
-that take action to change ownership when staring the container.
-
-You are able to configure another default browser download directory and mount the host with it in container by overriding `SE_DOWNLOAD_DIR`.
-
-For example, in test you might be scripting something
-```groovy
-ChromeOptions options = new ChromeOptions();
-HashMap<String, Object> chromePrefs = new HashMap<String, Object>();
-chromePrefs.put("download.default_directory", "/path/to/your/downloads");
-options.setExperimentalOption("prefs", chromePrefs);
-options.add_argument('disable-features=DownloadBubble,DownloadBubbleV2')
-WebDriver driver = new ChromeDriver(options);
-```
-
-When running the container, you set the `SE_DOWNLOAD_DIR` and mount the host with that directory in container.
-```bash
-docker run -d -p 4444:4444 --shm-size="2g" \
-  -e SE_DOWNLOAD_DIR=/path/to/your/downloads \
-  -v /home/ubuntu/files:/path/to/your/downloads \
-  selenium/standalone-chrome:4.16.1-20231212
-```
-
-**Note:** The changing ownership when starting container is not supported well when both overriding `SE_DOWNLOAD_DIR` and running non-root (e.g. Podman) or specifying user ids different from `1200` (OpenShift arbitrary user ids).
-
-In this case, you can use above workaround to create and set permissions for the directory on the host before mounting the volume.
-
-You also can run the container with `--user root` once to initialize and change ownership of the directory on the host, then run the container with non-root user again.
-
-For example, the first run with root user:
-```bash
-docker run -d -p 4444:4444 --shm-size="2g" \
-  --user root \
-  -e SE_DOWNLOAD_DIR=/path/to/your/downloads \
-  -v /home/ubuntu/files:/path/to/your/downloads \
-  selenium/standalone-chrome:4.16.1-20231212
-```
-
-Then stop it, rerun with switching to non-root user:
-```bash
-docker run -d -p 4444:4444 --shm-size="2g" \
-  --user 4496 \
-  -e SE_DOWNLOAD_DIR=/path/to/your/downloads \
-  -v /home/ubuntu/files:/path/to/your/downloads \
-  selenium/standalone-chrome:4.16.1-20231212
-```
-Summarize the supported use case for changing ownership when starting container:
+to the mounted directory. If you have a better workaround,
+please send us a pull request!
 
-| User (uid)           | Mount `SE_DOWNLOAD_DIR` in container | Auto changing |
-|----------------------|--------------------------------------|---------------|
-| seluser (uid `1200`) | default `/home/seluser/Downloads`    | Yes           |
-| seluser (uid `1200`) | any `/path/to/downloads`             | Yes           |
-| any (uid != `1200`)  | default `/home/seluser/Downloads`    | Yes           |
-| any (uid != `1200`)  | any `/path/to/downloads`             | No            |
 
diff --git a/tests/test.py b/tests/test.py
@@ -174,7 +174,7 @@ def standalone_browser_container_matches(container):
 
     use_random_user_id = USE_RANDOM_USER_ID == 'true'
     run_in_docker_compose = RUN_IN_DOCKER_COMPOSE == 'true'
-    random_user_id = "%s:%s" % (random.randint(2000, 65000), random.randint(2001, 65001))
+    random_user_id = random.randint(2000, 65000)
 
     if use_random_user_id:
         logger.info("Running tests with a random user ID -> %s" % random_user_id)