Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Правки в k8s агент. Привел его к стандартному варианту. #58

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 69 additions & 59 deletions k8s-jenkins-agent/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,70 +1,80 @@
# Для сборки использован стандартный шаблон jenkins-inbound-agent https://github.com/jenkinsci/docker-agent/blob/master/debian/Dockerfile
# Стадию с as jre-build убрал, т.к. у нас на этот момент уже всегда есть джава в контейнере.
ARG DOCKER_REGISTRY_URL
ARG BASE_IMAGE
ARG BASE_TAG
ARG user=jenkins

FROM ${DOCKER_REGISTRY_URL}/${BASE_IMAGE}:${BASE_TAG}
FROM ${DOCKER_REGISTRY_URL}/${BASE_IMAGE}:${BASE_TAG} as agent

LABEL maintainer="Nikita Gryzlov <[email protected]>, FirstBit"

RUN set -xe \
&& apt-get update \
&& DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \
curl \
git \
locales \
openssh-client \
wget \
init \
openssh-server openssh-client \
apt-transport-https \
# git-lfs
&& curl -s https://packagecloud.io/install/repositories/github/git-lfs/script.deb.sh | bash \
&& DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \
ARG user=jenkins
ARG group=jenkins
ARG uid=1000
ARG gid=1000

RUN groupadd -g "${gid}" "${group}" \
&& useradd -l -c "Jenkins user" -d /home/"${user}" -u "${uid}" -g "${gid}" -m "${user}" || echo "user ${user} already exists."
Comment on lines +16 to +17
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve user and group creation to handle existing entities

The current groupadd and useradd commands may fail if the group or user already exists, causing the build to fail. To handle existing users and groups gracefully, check if they exist before attempting to create them.

Apply this diff to enhance robustness:

 RUN groupadd -g "${gid}" "${group}" || true \
-  && useradd -l -c "Jenkins user" -d /home/"${user}" -u "${uid}" -g "${gid}" -m "${user}" || echo "user ${user} already exists."
+  && id -u "${user}" &>/dev/null || useradd -l -c "Jenkins user" -d /home/"${user}" -u "${uid}" -g "${gid}" -m "${user}"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RUN groupadd -g "${gid}" "${group}" \
&& useradd -l -c "Jenkins user" -d /home/"${user}" -u "${uid}" -g "${gid}" -m "${user}" || echo "user ${user} already exists."
RUN groupadd -g "${gid}" "${group}" || true \
&& id -u "${user}" &>/dev/null || useradd -l -c "Jenkins user" -d /home/"${user}" -u "${uid}" -g "${gid}" -m "${user}"


ARG AGENT_WORKDIR=/home/"${user}"/agent

## Always use the latest Debian packages: no need for versions
# hadolint ignore=DL3008
RUN apt-get update \
&& apt-get --yes --no-install-recommends install \
ca-certificates \
curl \
fontconfig \
git \
git-lfs \
&& rm -rf \
/var/lib/apt/lists/* \
/var/cache/debconf \
&& localedef -i ru_RU -c -f UTF-8 -A /usr/share/locale/locale.alias ru_RU.UTF-8

RUN echo "deb http://security.ubuntu.com/ubuntu bionic-security main" | tee -a /etc/apt/sources.list
RUN apt-key adv --recv-keys --keyserver keyserver.ubuntu.com 3B4FE6ACC0B21F32 && apt-get update
#RUN apt-add-repository 'deb http://security.debian.org/debian-security stretch/updates main' && apt-get update
#RUN apt-get update && apt install -y openjdk-11-jdk

less \
netbase \
openssh-client \
patch \
tzdata \
&& apt-get clean \
&& rm -rf /tmp/* /var/cache/* /var/lib/apt/lists/*

ARG VERSION=3283.v92c105e0f819
ADD --chown="${user}":"${group}" "https://repo.jenkins-ci.org/public/org/jenkins-ci/main/remoting/${VERSION}/remoting-${VERSION}.jar" /usr/share/jenkins/agent.jar
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Replace ADD with curl for downloading remote files

Using ADD to fetch remote URLs is discouraged as it doesn't leverage caching and may have security implications. Instead, use curl to download the file and then set the appropriate ownership.

Apply this diff to make the change:

 ARG VERSION=3283.v92c105e0f819
-ADD --chown="${user}":"${group}" "https://repo.jenkins-ci.org/public/org/jenkins-ci/main/remoting/${VERSION}/remoting-${VERSION}.jar" /usr/share/jenkins/agent.jar
+RUN curl -fsSL "https://repo.jenkins-ci.org/public/org/jenkins-ci/main/remoting/${VERSION}/remoting-${VERSION}.jar" -o /usr/share/jenkins/agent.jar \
+    && chown "${user}":"${group}" /usr/share/jenkins/agent.jar
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ADD --chown="${user}":"${group}" "https://repo.jenkins-ci.org/public/org/jenkins-ci/main/remoting/${VERSION}/remoting-${VERSION}.jar" /usr/share/jenkins/agent.jar
RUN curl -fsSL "https://repo.jenkins-ci.org/public/org/jenkins-ci/main/remoting/${VERSION}/remoting-${VERSION}.jar" -o /usr/share/jenkins/agent.jar \
&& chown "${user}":"${group}" /usr/share/jenkins/agent.jar

RUN chmod 0644 /usr/share/jenkins/agent.jar \
&& ln -sf /usr/share/jenkins/agent.jar /usr/share/jenkins/slave.jar

USER "${user}"
ENV AGENT_WORKDIR=${AGENT_WORKDIR}
RUN mkdir -p /home/"${user}"/.jenkins && mkdir -p "${AGENT_WORKDIR}"

VOLUME /home/"${user}"/.jenkins
VOLUME "${AGENT_WORKDIR}"
WORKDIR /home/"${user}"
ENV USER=${user}
LABEL \
org.opencontainers.image.vendor="Jenkins project" \
org.opencontainers.image.title="Official Jenkins Agent Base Docker image" \
org.opencontainers.image.description="This is a base image, which provides the Jenkins agent executable (agent.jar)" \
org.opencontainers.image.version="${VERSION}" \
org.opencontainers.image.url="https://www.jenkins.io/" \
org.opencontainers.image.source="https://github.com/jenkinsci/docker-agent" \
org.opencontainers.image.licenses="MIT"

## Inbound Agent image target
FROM agent AS inbound-agent

RUN groupadd -g 1000 jenkins \
&& useradd -l -d /home/jenkins -u 1000 -g 1000 -m jenkins


ARG AGENT_WORKDIR=/home/jenkins/agent

# Install jenkins jnlp
ARG VERSION=4.14
RUN curl --create-dirs -sSLo /usr/share/jenkins/slave.jar https://repo.jenkins-ci.org/public/org/jenkins-ci/main/remoting/4.14/remoting-4.14.jar \
&& chmod 755 /usr/share/jenkins \
&& chmod 644 /usr/share/jenkins/slave.jar \
&& chown jenkins:jenkins /usr/share/jenkins/slave.jar
ARG user=jenkins

USER root
COPY ./k8s-jenkins-agent/jenkins-agent /usr/local/bin/jenkins-agent
RUN chmod +x /usr/local/bin/jenkins-agent \
&& chown jenkins:jenkins /usr/local/bin/jenkins-agent \
&& ln -s /usr/local/bin/jenkins-agent /usr/local/bin/jenkins-slave

RUN mkdir -p /home/jenkins/.jenkins \
&& mkdir -p /home/jenkins/agent \
&& chown -R jenkins:jenkins /home/jenkins


ENV LANG ru_RU.UTF-8

VOLUME /home/jenkins/.jenkins
VOLUME /home/jenkins/agent

WORKDIR /home/jenkins/agent

COPY ./k8s-jenkins-agent/docker-entrypoint.sh /
RUN chmod 755 /docker-entrypoint.sh \
&& chmod +x /docker-entrypoint.sh

ENTRYPOINT ["/docker-entrypoint.sh"]
RUN chmod +x /usr/local/bin/jenkins-agent &&\
ln -s /usr/local/bin/jenkins-agent /usr/local/bin/jenkins-slave
# USER ${user}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Switch back to the non-root user after performing root operations

For better security practices, it's advisable to switch back to the ${user} after completing operations that require root privileges.

Apply this diff to uncomment the USER directive:

-# USER ${user}
+USER ${user}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# USER ${user}
USER ${user}


LABEL \
org.opencontainers.image.vendor="Jenkins project" \
org.opencontainers.image.title="Official Jenkins Inbound Agent Base Docker image" \
org.opencontainers.image.description="This is an image for Jenkins agents using TCP or WebSockets to establish inbound connection to the Jenkins controller" \
org.opencontainers.image.version="${VERSION}" \
org.opencontainers.image.url="https://www.jenkins.io/" \
org.opencontainers.image.source="https://github.com/jenkinsci/docker-agent" \
org.opencontainers.image.licenses="MIT"

ENTRYPOINT ["/usr/local/bin/jenkins-agent"]
7 changes: 0 additions & 7 deletions k8s-jenkins-agent/docker-entrypoint.sh

This file was deleted.

30 changes: 22 additions & 8 deletions k8s-jenkins-agent/jenkins-agent
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,14 @@
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
# THE SOFTWARE.

# Usage jenkins-agent.sh [options] -url http://jenkins [SECRET] [AGENT_NAME]
# Usage jenkins-agent.sh [options] -url http://jenkins -secret [SECRET] -name [AGENT_NAME]
# Optional environment variables :
# * JENKINS_JAVA_BIN : Java executable to use instead of the default in PATH or obtained from JAVA_HOME
# * JENKINS_JAVA_OPTS : Java Options to use for the remoting process, otherwise obtained from JAVA_OPTS
# * JENKINS_AGENT_FILE : Jenkins agent jar file location, /usr/share/jenkins/agent.jar will be used if this is not set
# * REMOTING_OPTS : Generic way to pass additional CLI options to agent.jar (see -help)
#
# Deprecated environment variables (prefer setting REMOTING_OPTS)
# * JENKINS_TUNNEL : HOST:PORT for a tunnel to route TCP traffic to jenkins host, when jenkins can't be directly accessed over network
# * JENKINS_URL : alternate jenkins URL
# * JENKINS_SECRET : agent secret, if not set as an argument
Expand All @@ -34,10 +38,16 @@
# * JENKINS_WEB_SOCKET: true if the connection should be made via WebSocket rather than TCP
# * JENKINS_DIRECT_CONNECTION: Connect directly to this TCP agent port, skipping the HTTP(S) connection parameter download.
# Value: "<HOST>:<PORT>"
# * JENKINS_INSTANCE_IDENTITY: The base64 encoded InstanceIdentity byte array of the Jenkins master. When this is set,
# * JENKINS_INSTANCE_IDENTITY: The base64 encoded InstanceIdentity byte array of the Jenkins controller. When this is set,
# the agent skips connecting to an HTTP(S) port for connection info.
# * JENKINS_PROTOCOLS: Specify the remoting protocols to attempt when instanceIdentity is provided.

############## ВСТАВКА СТАРТА ИКСОВ В СТАНДАРТНЫЙ jenkins-agent ############
if [ -f "/init" ]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Может вынести это в docker-entrypoint.sh? А оттуда запускать и этот файл? Не люблю ручные правки, рано или поздно забудем при обновлении и снесем.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Да, согласен, сделаю.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nixel2007 сделал

/init &
fi
############################### Конец вставки ##############################
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance X server initialization robustness

The X server initialization block could benefit from:

  1. Error handling and logging
  2. Verification of script permissions
  3. Status reporting for background process

Consider this improvement:

 ############## ВСТАВКА СТАРТА ИКСОВ В СТАНДАРТНЫЙ jenkins-agent ############
 if [ -f "/init" ]; then
-    /init &
+    if [ -x "/init" ]; then
+        echo "Starting X server initialization..."
+        /init & 
+        INIT_PID=$!
+        echo "X server initialization started with PID: $INIT_PID"
+    else
+        echo "Warning: /init exists but is not executable"
+    fi
 fi
 ############################### Конец вставки ##############################
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
############## ВСТАВКА СТАРТА ИКСОВ В СТАНДАРТНЫЙ jenkins-agent ############
if [ -f "/init" ]; then
/init &
fi
############################### Конец вставки ##############################
############## ВСТАВКА СТАРТА ИКСОВ В СТАНДАРТНЫЙ jenkins-agent ############
if [ -f "/init" ]; then
if [ -x "/init" ]; then
echo "Starting X server initialization..."
/init &
INIT_PID=$!
echo "X server initialization started with PID: $INIT_PID"
else
echo "Warning: /init exists but is not executable"
fi
fi
############################### Конец вставки ##############################


if [ $# -eq 1 ] && [ "${1#-}" = "$1" ] ; then

# if `docker run` only has one arguments and it is not an option as `-help`, we assume user is running alternate command like `bash` to inspect the image
Expand Down Expand Up @@ -106,28 +116,32 @@ else
fi
fi

if [ "$JENKINS_AGENT_FILE" ]; then
AGENT_FILE="$JENKINS_AGENT_FILE"
else
AGENT_FILE="/usr/share/jenkins/agent.jar"
fi
Comment on lines +113 to +117
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add validation for agent jar file

The script should validate the existence and permissions of the agent jar file before execution.

Consider this improvement:

 if [ "$JENKINS_AGENT_FILE" ]; then
     AGENT_FILE="$JENKINS_AGENT_FILE"
 else
     AGENT_FILE="/usr/share/jenkins/agent.jar"
 fi
+if [ ! -f "$AGENT_FILE" ]; then
+    echo "Error: Agent jar file not found: $AGENT_FILE"
+    exit 1
+fi
+if [ ! -r "$AGENT_FILE" ]; then
+    echo "Error: Agent jar file not readable: $AGENT_FILE"
+    exit 1
+fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ "$JENKINS_AGENT_FILE" ]; then
AGENT_FILE="$JENKINS_AGENT_FILE"
else
AGENT_FILE="/usr/share/jenkins/agent.jar"
fi
if [ "$JENKINS_AGENT_FILE" ]; then
AGENT_FILE="$JENKINS_AGENT_FILE"
else
AGENT_FILE="/usr/share/jenkins/agent.jar"
fi
if [ ! -f "$AGENT_FILE" ]; then
echo "Error: Agent jar file not found: $AGENT_FILE"
exit 1
fi
if [ ! -r "$AGENT_FILE" ]; then
echo "Error: Agent jar file not readable: $AGENT_FILE"
exit 1
fi


# if both required options are defined, do not pass the parameters
OPT_JENKINS_SECRET=""
if [ -n "$JENKINS_SECRET" ]; then
case "$@" in
*"${JENKINS_SECRET}"*) echo "Warning: SECRET is defined twice in command-line arguments and the environment variable" ;;
*)
OPT_JENKINS_SECRET="${JENKINS_SECRET}" ;;
SECRET="-secret ${JENKINS_SECRET}" ;;
esac
fi

OPT_JENKINS_AGENT_NAME=""
if [ -n "$JENKINS_AGENT_NAME" ]; then
case "$@" in
*"${JENKINS_AGENT_NAME}"*) echo "Warning: AGENT_NAME is defined twice in command-line arguments and the environment variable" ;;
*)
OPT_JENKINS_AGENT_NAME="${JENKINS_AGENT_NAME}" ;;
AGENT_NAME="-name ${JENKINS_AGENT_NAME}" ;;
esac
fi

#TODO: Handle the case when the command-line and Environment variable contain different values.
#It is fine it blows up for now since it should lead to an error anyway.

exec $JAVA_BIN $JAVA_OPTIONS -cp /usr/share/jenkins/agent.jar hudson.remoting.jnlp.Main -headless $TUNNEL $URL $WORKDIR $WEB_SOCKET $DIRECT $PROTOCOLS $INSTANCE_IDENTITY $OPT_JENKINS_SECRET $OPT_JENKINS_AGENT_NAME "$@"
exec $JAVA_BIN $JAVA_OPTIONS -jar $AGENT_FILE $SECRET $AGENT_NAME $TUNNEL $URL $WORKDIR $WEB_SOCKET $DIRECT $PROTOCOLS $INSTANCE_IDENTITY $REMOTING_OPTS "$@"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve command execution robustness

The execution command has unquoted variables which could cause issues with paths or arguments containing spaces.

Apply this fix:

-        exec $JAVA_BIN $JAVA_OPTIONS -jar $AGENT_FILE $SECRET $AGENT_NAME $TUNNEL $URL $WORKDIR $WEB_SOCKET $DIRECT $PROTOCOLS $INSTANCE_IDENTITY $REMOTING_OPTS "$@"
+        exec "${JAVA_BIN}" ${JAVA_OPTIONS:+"${JAVA_OPTIONS}"} -jar "${AGENT_FILE}" \
+            ${SECRET:+"${SECRET}"} \
+            ${AGENT_NAME:+"${AGENT_NAME}"} \
+            ${TUNNEL:+"${TUNNEL}"} \
+            ${URL:+"${URL}"} \
+            ${WORKDIR:+"${WORKDIR}"} \
+            ${WEB_SOCKET:+"${WEB_SOCKET}"} \
+            ${DIRECT:+"${DIRECT}"} \
+            ${PROTOCOLS:+"${PROTOCOLS}"} \
+            ${INSTANCE_IDENTITY:+"${INSTANCE_IDENTITY}"} \
+            ${REMOTING_OPTS:+"${REMOTING_OPTS}"} \
+            "$@"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
exec $JAVA_BIN $JAVA_OPTIONS -jar $AGENT_FILE $SECRET $AGENT_NAME $TUNNEL $URL $WORKDIR $WEB_SOCKET $DIRECT $PROTOCOLS $INSTANCE_IDENTITY $REMOTING_OPTS "$@"
exec "${JAVA_BIN}" ${JAVA_OPTIONS:+"${JAVA_OPTIONS}"} -jar "${AGENT_FILE}" \
${SECRET:+"${SECRET}"} \
${AGENT_NAME:+"${AGENT_NAME}"} \
${TUNNEL:+"${TUNNEL}"} \
${URL:+"${URL}"} \
${WORKDIR:+"${WORKDIR}"} \
${WEB_SOCKET:+"${WEB_SOCKET}"} \
${DIRECT:+"${DIRECT}"} \
${PROTOCOLS:+"${PROTOCOLS}"} \
${INSTANCE_IDENTITY:+"${INSTANCE_IDENTITY}"} \
${REMOTING_OPTS:+"${REMOTING_OPTS}"} \
"$@"


fi
fi