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

Conversation

Segate-ekb
Copy link

@Segate-ekb Segate-ekb commented Nov 22, 2024

Summary by CodeRabbit

  • New Features

    • Enhanced Jenkins agent setup with improved clarity and functionality.
    • Introduced new environment variables for better configuration options, including JENKINS_AGENT_FILE and REMOTING_OPTS.
  • Bug Fixes

    • Refined command-line argument handling for the Jenkins agent script.
  • Chores

    • Streamlined the initialization process by simplifying the entry point for the Jenkins agent.

…й сборкой с несколькими изменениями:

1) Запускаются иксы, если они есть
2) пользователь по умолчанию оставлен root, а не jenkins. (как в сварме)
3) Локаль и ТЗ не трогаем
Copy link

coderabbitai bot commented Nov 23, 2024

Walkthrough

The changes involve significant modifications to the Dockerfile for the Jenkins agent, including the removal of the as jre-build stage, the introduction of user and working directory arguments, and an updated package installation process. The docker-entrypoint.sh script has been removed, with its functionality integrated into the jenkins-agent.sh script, which has been enhanced for better command-line argument handling and environment variable management. These updates aim to streamline the setup and operation of the Jenkins agent within a Docker container.

Changes

File Path Change Summary
k8s-jenkins-agent/Dockerfile Removed as jre-build stage, added user/group arguments, introduced AGENT_WORKDIR, updated package installation, set ownership for remoting JAR, modified entry point to jenkins-agent.
k8s-jenkins-agent/docker-entrypoint.sh Deleted file; previously initialized the Jenkins agent with a shell script.
k8s-jenkins-agent/jenkins-agent Updated usage instructions, added JENKINS_AGENT_FILE and REMOTING_OPTS, refined environment variable handling, and modified execution command.

Poem

🐰 In the garden where agents play,
Docker's dance has found a new way.
With scripts that sing and paths that twine,
Jenkins hops on, feeling just fine.
A world of ease in every line! 🌼


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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 codebase

While 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 to jenkins-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

📥 Commits

Reviewing files that changed from the base of the PR and between 9cf9607 and ff5b5fe.

📒 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

Comment on lines +119 to +123
if [ "$JENKINS_AGENT_FILE" ]; then
AGENT_FILE="$JENKINS_AGENT_FILE"
else
AGENT_FILE="/usr/share/jenkins/agent.jar"
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

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

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}"} \
"$@"

Comment on lines 45 to 49
############## ВСТАВКА СТАРТА ИКСОВ В СТАНДАРТНЫЙ jenkins-agent ############
if [ -f "/init" ]; then
/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
############################### Конец вставки ##############################

&& 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

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

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}

Copy link

@coderabbitai coderabbitai bot left a 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 and openssh-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

📥 Commits

Reviewing files that changed from the base of the PR and between ff5b5fe and 76c900a.

📒 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

# 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 сделал

Copy link

@coderabbitai coderabbitai bot left a 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 logging

The 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 improvements

The 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 accessibility

The 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 builds

While 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

📥 Commits

Reviewing files that changed from the base of the PR and between 76c900a and 4f66881.

📒 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 "$@"
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

Comment on lines +1 to +83
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"]
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

@nixel2007 nixel2007 merged commit 905a96b into firstBitMarksistskaya:feature/first-bit Nov 23, 2024
9 of 11 checks passed
@nixel2007
Copy link
Member

Спасибо! Осталось с кавереджом разобраться

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants