chore: update Docker configuration#425
Conversation
…tices - Upgrade Alpine from 3.22.0 to 3.23.3 (security fixes) - Add multi-platform build support (ARM64, ARM, x86) - Add Go module caching for faster builds - Add non-root user for security - Add health check configuration - Add resource limits in docker-compose - Add logging configuration with rotation - Add ca-certificates for HTTPS support - Add volume mount as read-only for config
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Docker configuration for the application, focusing on security, performance, and operational robustness. It updates the base image to a more secure version, enables multi-platform builds, and introduces build optimizations. Furthermore, it strengthens container security by adopting a non-root user and read-only mounts, while also improving deployability and monitoring through integrated health checks, resource limits, and structured logging. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces several improvements to the Docker build process and deployment configuration. Key changes include enabling multi-platform builds, adding a non-root user (appuser) for enhanced security, and configuring a health check for the Docker image. Additionally, the docker-compose.yml has been updated to include a health check, resource limits, and logging configurations, and to mount the application configuration as read-only. Review comments highlight critical issues: the TARGETOS and TARGETARCH variables in the Dockerfile are incorrectly defined as shell variables instead of ARG instructions, the wget command used in the Dockerfile's health check is missing from the image, the timezone setting command in the Dockerfile runs after switching to a non-root user causing permission errors, and the docker-compose.yml mounts authentication tokens to /root which is inaccessible by the new appuser, breaking authentication.
| TARGETOS=${TARGETOS:-linux} | ||
| TARGETARCH=${TARGETARCH:-amd64} |
There was a problem hiding this comment.
These lines use shell syntax (VAR=${VAR:-default}) which is not a valid Dockerfile instruction for defining build arguments. To define build-time arguments with default values, you should use the ARG instruction. For multi-platform builds with buildx, TARGETOS and TARGETARCH are automatically populated, and using ARG will correctly receive them.
ARG TARGETOS=linux
ARG TARGETARCH=amd64
| RUN addgroup -g 1000 appgroup && \ | ||
| adduser -u 1000 -G appgroup -s /bin/sh -D appuser | ||
|
|
||
| RUN mkdir /CLIProxyAPI | ||
| RUN apk add --no-cache tzdata ca-certificates | ||
|
|
||
| COPY --from=builder ./app/CLIProxyAPIPlus /CLIProxyAPI/CLIProxyAPIPlus | ||
| RUN mkdir -p /CLIProxyAPI && chown -R appuser:appgroup /CLIProxyAPI |
There was a problem hiding this comment.
The HEALTHCHECK instruction on line 46 uses wget, but it is not installed in the final image. This will cause the health check to fail. You should add wget to the package installation.
Additionally, to reduce image layers and improve build efficiency, it's best to combine related RUN commands into a single instruction.
RUN addgroup -g 1000 appgroup && \
adduser -u 1000 -G appgroup -s /bin/sh -D appuser && \
apk add --no-cache tzdata ca-certificates wget && \
mkdir -p /CLIProxyAPI && chown -R appuser:appgroup /CLIProxyAPI
| volumes: | ||
| - ${CLI_PROXY_CONFIG_PATH:-./config.yaml}:/CLIProxyAPI/config.yaml | ||
| - ${CLI_PROXY_CONFIG_PATH:-./config.yaml}:/CLIProxyAPI/config.yaml:ro | ||
| - ${CLI_PROXY_AUTH_PATH:-./auths}:/root/.cli-proxy-api |
There was a problem hiding this comment.
The volume for authentication tokens is mounted to /root/.cli-proxy-api. Since the container now runs as the non-root user appuser, and the application expects tokens in the user's home directory (defaulting to ~/.cli-proxy-api), the application will be unable to access the mounted tokens due to permission restrictions on the /root directory. This breaks the intended security model, may lead users to revert to running as root, and will cause authentication data to be non-persistent. Please update the mount path to the correct directory for appuser.
- ${CLI_PROXY_AUTH_PATH:-./auths}:/home/appuser/.cli-proxy-api| EXPOSE 8317 8085 1455 54545 51121 11451 | ||
|
|
||
| ENV TZ=Asia/Shanghai | ||
| RUN cp /usr/share/zoneinfo/${TZ} /etc/localtime && echo "${TZ}" > /etc/timezone |
There was a problem hiding this comment.
The RUN command to set the timezone is executed after switching to the non-root user appuser. This will cause the build to fail because appuser does not have permission to write to /etc/localtime and /etc/timezone. Move these commands before the USER appuser instruction.
1 participant
Summary
Update Docker configuration with latest versions and best practices.
Changes
--mount=type=cache)Breaking Changes
None - these are backward compatible improvements.