-
Notifications
You must be signed in to change notification settings - Fork 75
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
Правки в k8s агент. Привел его к стандартному варианту. #58
Conversation
…й сборкой с несколькими изменениями: 1) Запускаются иксы, если они есть 2) пользователь по умолчанию оставлен root, а не jenkins. (как в сварме) 3) Локаль и ТЗ не трогаем
WalkthroughThe changes involve significant modifications to the Dockerfile for the Jenkins agent, including the removal of the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (2)
k8s-jenkins-agent/jenkins-agent (1)
Line range hint
25-41
: Consider maintaining consistent language usage across the codebaseWhile the documentation is well-structured, the codebase mixes English (in comments/docs) and Russian (in PR title and inline comments). Consider standardizing on English throughout for better maintainability and collaboration.
k8s-jenkins-agent/Dockerfile (1)
68-68
: Avoid deprecated terminology by removing the symlink tojenkins-slave
The term "slave" is deprecated in Jenkins. It's recommended to avoid using it to promote inclusive language.
Apply this diff to remove the deprecated symlink:
RUN chmod +x /usr/local/bin/jenkins-agent &&\ - ln -s /usr/local/bin/jenkins-agent /usr/local/bin/jenkins-slave
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
k8s-jenkins-agent/Dockerfile
(1 hunks)k8s-jenkins-agent/docker-entrypoint.sh
(0 hunks)k8s-jenkins-agent/jenkins-agent
(3 hunks)
💤 Files with no reviewable changes (1)
- k8s-jenkins-agent/docker-entrypoint.sh
🔇 Additional comments (2)
k8s-jenkins-agent/jenkins-agent (1)
130-130
: Add validation for secret and agent name
The script warns about duplicates but doesn't validate the values themselves. Also, there's an unaddressed TODO about handling conflicting values.
Let's check if this is a common issue in other Jenkins agent implementations:
Consider adding basic validation:
- SECRET="-secret ${JENKINS_SECRET}" ;;
+ if [ -z "${JENKINS_SECRET}" ]; then
+ echo "Error: Empty secret provided"
+ exit 1
+ fi
+ SECRET="-secret ${JENKINS_SECRET}" ;;
- AGENT_NAME="-name ${JENKINS_AGENT_NAME}" ;;
+ if [ -z "${JENKINS_AGENT_NAME}" ]; then
+ echo "Error: Empty agent name provided"
+ exit 1
+ fi
+ AGENT_NAME="-name ${JENKINS_AGENT_NAME}" ;;
Also applies to: 138-138
k8s-jenkins-agent/Dockerfile (1)
65-65
: Verify the source path in the COPY
command
Ensure that the source path ./k8s-jenkins-agent/jenkins-agent
is correct relative to the build context. If the jenkins-agent
script is located directly alongside the Dockerfile, the path should be adjusted.
Please confirm the path or update it as follows:
USER root
-COPY ./k8s-jenkins-agent/jenkins-agent /usr/local/bin/jenkins-agent
+COPY jenkins-agent /usr/local/bin/jenkins-agent
if [ "$JENKINS_AGENT_FILE" ]; then | ||
AGENT_FILE="$JENKINS_AGENT_FILE" | ||
else | ||
AGENT_FILE="/usr/share/jenkins/agent.jar" | ||
fi |
There was a problem hiding this comment.
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.
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 |
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 "$@" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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}"} \ | |
"$@" |
k8s-jenkins-agent/jenkins-agent
Outdated
############## ВСТАВКА СТАРТА ИКСОВ В СТАНДАРТНЫЙ jenkins-agent ############ | ||
if [ -f "/init" ]; then | ||
/init & | ||
fi | ||
############################### Конец вставки ############################## |
There was a problem hiding this comment.
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:
- Error handling and logging
- Verification of script permissions
- 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.
############## ВСТАВКА СТАРТА ИКСОВ В СТАНДАРТНЫЙ 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 | |
############################### Конец вставки ############################## |
&& 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 |
There was a problem hiding this comment.
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.
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 groupadd -g "${gid}" "${group}" \ | ||
&& useradd -l -c "Jenkins user" -d /home/"${user}" -u "${uid}" -g "${gid}" -m "${user}" || echo "user ${user} already exists." |
There was a problem hiding this comment.
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.
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}" |
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} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
# USER ${user} | |
USER ${user} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
k8s-jenkins-agent/Dockerfile (1)
23-36
: Consider pinning package versions for better reproducibility and security.While the comment indicates intentional use of latest packages, consider pinning versions for critical security packages like
ca-certificates
andopenssh-client
to ensure reproducible builds and avoid potential security issues from unexpected updates.RUN apt-get update \ && apt-get --yes --no-install-recommends install \ - ca-certificates \ + ca-certificates=20230311 \ curl \ fontconfig \ git \ git-lfs \ less \ netbase \ - openssh-client \ + openssh-client=1:9.2p1-2 \ patch \ tzdata \
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
k8s-jenkins-agent/Dockerfile
(1 hunks)
🔇 Additional comments (5)
k8s-jenkins-agent/Dockerfile (5)
1-7
: LGTM! Good use of build arguments for image configuration.
The base image setup is flexible and well-documented, with clear explanation about Java pre-installation assumption.
19-19
: LGTM! Good practice defining AGENT_WORKDIR as build argument.
The AGENT_WORKDIR argument allows for flexibility in agent workspace location configuration.
46-57
: LGTM! Well-structured volume configuration and comprehensive image labels.
The volume declarations and metadata labels follow Docker best practices, providing clear documentation and proper persistence configuration.
79-79
: LGTM! Proper ENTRYPOINT configuration.
The ENTRYPOINT is correctly configured using array syntax and points to the appropriate script location.
65-67
: Verify the jenkins-agent script permissions and symbolic link.
The script setup looks correct, but let's verify the jenkins-agent script exists and has the correct content.
✅ Verification successful
Let me verify the script's executable permissions and symbolic link setup with additional checks.
The jenkins-agent script exists but requires executable permissions
The script exists with proper content (146 lines including license and functionality), but the file permissions are currently set to -rw-r--r--
. The Dockerfile's chmod +x
command will correctly set the executable permissions during build, making this setup valid.
- File: k8s-jenkins-agent/jenkins-agent
- Current permissions: 644 (rw-r--r--)
- Will be set to: 755 (rwxr-xr-x) by Dockerfile
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if jenkins-agent script exists and verify its content
if [ -f k8s-jenkins-agent/jenkins-agent ]; then
echo "Script exists. Checking content..."
head -n 10 k8s-jenkins-agent/jenkins-agent
else
echo "WARNING: jenkins-agent script not found in k8s-jenkins-agent directory"
fi
Length of output: 571
Script:
#!/bin/bash
# Check script permissions and symbolic link
ls -l k8s-jenkins-agent/jenkins-agent
# Verify script is complete and not truncated
wc -l k8s-jenkins-agent/jenkins-agent
# Check script content for key functionality
grep -A 5 "JENKINS_AGENT_WORKDIR=" k8s-jenkins-agent/jenkins-agent
Length of output: 263
k8s-jenkins-agent/jenkins-agent
Outdated
# 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Может вынести это в docker-entrypoint.sh? А оттуда запускать и этот файл? Не люблю ручные правки, рано или поздно забудем при обновлении и снесем.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Да, согласен, сделаю.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nixel2007 сделал
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
k8s-jenkins-agent/docker-entrypoint.sh (2)
Line range hint
3-5
: Enhance init script handling with validation and loggingThe current implementation of init script execution lacks proper validation and error handling. Consider adding checks for execute permissions and logging.
if [ -f "/init" ]; then - /init & + if [ -x "/init" ]; then + echo "Executing init script in background" + /init & + else + echo "Warning: /init exists but is not executable" + fi fi
Line range hint
1-7
: Approve simplified entrypoint with architectural improvementsThe shift from downloading agent.jar at runtime to using a pre-installed jenkins-agent is a good architectural improvement that:
- Reduces runtime dependencies
- Improves security by eliminating runtime downloads
- Speeds up container startup
- Aligns with standard Jenkins agent patterns
Consider documenting these changes in the repository's README.md to help users understand the new setup process and requirements.
k8s-jenkins-agent/Dockerfile (2)
1-2
: Consider translating comments to English for better accessibilityThe comments provide valuable context about the template source and modifications, but they're currently in Russian. Consider translating them to English to improve collaboration with a wider audience.
-# Для сборки использован стандартный шаблон jenkins-inbound-agent https://github.com/jenkinsci/docker-agent/blob/master/debian/Dockerfile -# Стадию с as jre-build убрал, т.к. у нас на этот момент уже всегда есть джава в контейнере. +# Built using the standard jenkins-inbound-agent template: https://github.com/jenkinsci/docker-agent/blob/master/debian/Dockerfile +# Removed the 'jre-build' stage since Java is already present in our container
23-36
: Consider pinning package versions for reproducible buildsWhile using the latest packages ensures security updates, it can lead to inconsistent builds across different environments. Consider pinning package versions for better reproducibility.
RUN apt-get update \ && apt-get --yes --no-install-recommends install \ - ca-certificates \ - curl \ - fontconfig \ - git \ - git-lfs \ - less \ - netbase \ - openssh-client \ - patch \ - tzdata \ + ca-certificates=20230311 \ + curl=7.88.1-10+deb12u4 \ + fontconfig=2.14.1-4 \ + git=1:2.39.2-1.1 \ + git-lfs=3.2.0-1+b3 \ + less=590-1+b1 \ + netbase=6.4 \ + openssh-client=1:9.2p1-2+deb12u1 \ + patch=2.7.6-7 \ + tzdata=2023c-5 \
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
k8s-jenkins-agent/Dockerfile
(1 hunks)k8s-jenkins-agent/docker-entrypoint.sh
(1 hunks)k8s-jenkins-agent/jenkins-agent
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- k8s-jenkins-agent/jenkins-agent
🔇 Additional comments (1)
k8s-jenkins-agent/Dockerfile (1)
83-83
: Verify the correct ENTRYPOINT configuration
The ENTRYPOINT is set to /docker-entrypoint.sh
, but according to the AI summary, the functionality was supposed to be integrated into jenkins-agent.sh
. This seems inconsistent with the intended changes.
Let's verify the entrypoint scripts:
✅ Verification successful
ENTRYPOINT configuration is correct and consistent
The scripts show that docker-entrypoint.sh
is correctly configured as the entrypoint since it handles initialization tasks (checking and running /init
if present) and then delegates to jenkins-agent
script, which contains the main agent logic. This is a proper separation of concerns and matches the intended functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the content of both scripts to understand the flow
echo "Checking entrypoint scripts..."
cat ./k8s-jenkins-agent/docker-entrypoint.sh
echo "---"
cat ./k8s-jenkins-agent/jenkins-agent
Length of output: 5606
@@ -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 "$@" |
There was a problem hiding this comment.
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.
/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
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 | ||
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} | ||
|
||
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"] |
There was a problem hiding this comment.
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
- SSH host key verification is not configured, which could expose the agent to MITM attacks during git operations
- The agent runs as root (USER directive is commented out), violating the principle of least privilege
- File permissions are set correctly for agent.jar (0644) and executables (755)
- 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:
- No sensitive information (keys, tokens) is included in the image layers
- SSH host verification is enabled for git operations
- 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
905a96b
into
firstBitMarksistskaya:feature/first-bit
Спасибо! Осталось с кавереджом разобраться |
Summary by CodeRabbit
New Features
JENKINS_AGENT_FILE
andREMOTING_OPTS
.Bug Fixes
Chores