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 all commits
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
127 changes: 70 additions & 57 deletions k8s-jenkins-agent/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,70 +1,83 @@
# Для сборки использован стандартный шаблон 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

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
COPY ./k8s-jenkins-agent/docker-entrypoint.sh /docker-entrypoint.sh

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 \
RUN chmod +x /usr/local/bin/jenkins-agent \
&& ln -s /usr/local/bin/jenkins-agent /usr/local/bin/jenkins-slave \
&& chmod 755 /docker-entrypoint.sh \
&& chmod +x /docker-entrypoint.sh

ENTRYPOINT ["/docker-entrypoint.sh"]
# 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 ["/docker-entrypoint.sh"]
Comment on lines +1 to +83
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Security review completed - several concerns need addressing

  1. SSH host key verification is not configured, which could expose the agent to MITM attacks during git operations
  2. The agent runs as root (USER directive is commented out), violating the principle of least privilege
  3. File permissions are set correctly for agent.jar (0644) and executables (755)
  4. No sensitive information is hardcoded in the image layers, secrets are properly passed via environment variables or arguments

Required changes:

  • Uncomment the USER ${user} directive to run as non-root
  • Add SSH configuration to enable strict host key checking:
RUN git config --system core.sshCommand 'ssh -o StrictHostKeyChecking=yes'
🔗 Analysis chain

Review security considerations

Please ensure:

  1. No sensitive information (keys, tokens) is included in the image layers
  2. SSH host verification is enabled for git operations
  3. Appropriate file permissions are set for sensitive directories

Let's check for potential security issues:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for sensitive files and permissions
echo "Checking for sensitive patterns..."
rg -i '(password|secret|key|token|credential)' ./k8s-jenkins-agent/

echo "Checking git config..."
cat ./k8s-jenkins-agent/docker-entrypoint.sh | grep -i "StrictHostKeyChecking"

Length of output: 1004


Script:

#!/bin/bash
# Let's check the entrypoint and agent scripts more thoroughly
echo "=== Checking docker-entrypoint.sh ==="
cat ./k8s-jenkins-agent/docker-entrypoint.sh

echo -e "\n=== Checking jenkins-agent script ==="
cat ./k8s-jenkins-agent/jenkins-agent

echo -e "\n=== Checking file permissions in Dockerfile ==="
grep -E "chmod|chown" ./k8s-jenkins-agent/Dockerfile

Length of output: 6193

2 changes: 1 addition & 1 deletion k8s-jenkins-agent/docker-entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ if [ -f "/init" ]; then
/init &
fi

wget -O agent.jar $JENKINS_URL/jnlpJars/agent.jar && java -Xms4096m -Xmx4096m -Dhudson.remoting.Launcher.pingIntervalSec=-1 -jar /home/jenkins/agent/agent.jar -jnlpUrl $JENKINS_URL/computer/$JENKINS_NAME/jenkins-agent.jnlp -secret $JENKINS_SECRET -noReconnect -workDir /home/jenkins/agent
/usr/local/bin/jenkins-agent "$@"
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 and error handling for jenkins-agent execution

The direct execution of jenkins-agent lacks proper validation and error handling.

+if [ ! -x "/usr/local/bin/jenkins-agent" ]; then
+    echo "Error: jenkins-agent not found or not executable"
+    exit 1
+fi
+
 /usr/local/bin/jenkins-agent "$@"
📝 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
/usr/local/bin/jenkins-agent "$@"
if [ ! -x "/usr/local/bin/jenkins-agent" ]; then
echo "Error: jenkins-agent not found or not executable"
exit 1
fi
/usr/local/bin/jenkins-agent "$@"

💡 Codebase verification

Executable permissions needed for jenkins-agent and docker-entrypoint.sh scripts

The Dockerfile correctly sets up the permissions with chmod +x commands, but the source files in the repository need executable permissions:

  • k8s-jenkins-agent/jenkins-agent (currently: -rw-r--r--)
  • k8s-jenkins-agent/docker-entrypoint.sh (currently: -rw-r--r--)

Both files should have executable permissions (chmod +x) in the repository to match their intended use as executable scripts.

🔗 Analysis chain

Verify jenkins-agent script installation and permissions

Let's verify that the jenkins-agent script is properly installed and has correct permissions in the container image.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify jenkins-agent script installation and permissions

# Check if jenkins-agent exists in the repository
echo "Checking for jenkins-agent script in repository..."
fd --type f "jenkins-agent" --exec ls -l {}

# Look for references to jenkins-agent in Dockerfile
echo -e "\nChecking references in Dockerfile..."
rg "jenkins-agent" --type dockerfile

Length of output: 654


Script:

#!/bin/bash
# Let's check the Dockerfile content and jenkins-agent script permissions more specifically

# Check Dockerfile content in k8s-jenkins-agent directory
echo "Checking Dockerfile content..."
cat k8s-jenkins-agent/Dockerfile

# Check jenkins-agent script content and permissions
echo -e "\nChecking jenkins-agent script content and permissions..."
ls -l k8s-jenkins-agent/jenkins-agent
cat k8s-jenkins-agent/jenkins-agent

# Check docker-entrypoint.sh permissions
echo -e "\nChecking docker-entrypoint.sh permissions..."
ls -l k8s-jenkins-agent/docker-entrypoint.sh

Length of output: 9069

24 changes: 16 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,7 +38,7 @@
# * 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.

Expand Down Expand Up @@ -106,28 +110,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