[hotfix] Add/Switch (to) docker non-root user #52
Conversation
WalkthroughThe changes update Docker build and runtime configuration by introducing new build arguments, unifying directory references under Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Docker as Docker Build System
participant Image as API Container Image
Dev->>Docker: Build API image (docker-compose)
Docker->>Docker: Set build args (APP_VERSION, APP_DIR, BINARY_NAME)
Docker->>Docker: Build Go binary in ${APP_DIR}
Docker->>Image: Create non-root user/group, set up directories
Docker->>Image: Copy binary and fixtures to ${APP_DIR}
Docker->>Image: Set ownership and permissions
Docker->>Image: Set ENV variables (APP_DIR, BINARY_NAME)
Docker->>Image: Set entrypoint to exec ${APP_DIR}/${BINARY_NAME}
Dev->>Image: Run container as non-root user
Possibly related PRs
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
gocanto
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
config/makefile/db.mk(1 hunks)docker-compose.yml(1 hunks)docker/dockerfile-api(1 hunks)
🔇 Additional comments (6)
docker-compose.yml (2)
113-113: Verify the version downgrade is intentional.The
APP_VERSIONwas changed fromv1.0.0-releaseto0.0.0.1, which appears to be a downgrade from a release version to a development version. Please confirm this change is intentional and aligns with your release strategy.
117-118: LGTM: Build arguments align with Dockerfile refactoring.The new
APP_DIRandBINARY_NAMEbuild arguments properly align with the Dockerfile changes and help standardize the container configuration.config/makefile/db.mk (1)
49-50: LGTM: Improved SSL file permissions following security best practices.The explicit permission settings are correct:
600for the private key (owner read/write only)644for the certificate (world readable, owner writable)This is more secure and explicit than the previous single command approach.
docker/dockerfile-api (3)
36-53: LGTM: Excellent builder stage improvements.The builder stage changes are well-implemented:
- Proper use of
APP_DIRfor consistency- Correct Go cache and module cache configuration
- Clean directory structure
96-133: LGTM: Excellent non-root user security implementation.The non-root user setup follows security best practices:
- Creates dedicated system user and group
- Sets proper home directory
- Creates necessary storage directories
- Assigns correct ownership to the non-root user
- Switches to non-root user for runtime
This significantly improves container security by avoiding running as root.
144-144: Review the shell-form entrypoint for security implications.The shell-form entrypoint
["/bin/sh", "-c", "exec ${APP_DIR}/${BINARY_NAME}"]allows environment variable expansion but introduces a shell layer that could potentially be exploited if environment variables are not properly sanitized.Consider whether the exec-form would be more secure:
ENTRYPOINT ["/app/oullin_api"]However, if you need the flexibility of environment variable expansion, the current approach is acceptable with proper environment variable validation.
gocanto
force-pushed
the
hotfix/docker-user
branch
from
48c7194 to
1b64eb3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docker/dockerfile-api (1)
144-144: Consider simplifying the entrypoint.While the current entrypoint works correctly, it could be simplified by using the exec form with environment variable substitution.
-# Launch the binary (shell-form so that environment variables are expanded). -ENTRYPOINT ["/bin/sh", "-c", "exec ${APP_DIR}/${BINARY_NAME}"] +# Launch the binary directly. +ENTRYPOINT ["sh", "-c", "exec $APP_DIR/$BINARY_NAME"]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docker/dockerfile-api(1 hunks)
🔇 Additional comments (8)
docker/dockerfile-api (8)
1-32: Well-organized global build arguments section.The restructuring of build arguments with detailed comments and the introduction of
APP_DIRas a unified directory reference improves maintainability and clarity.
44-51: Excellent Go build cache configuration.The Go environment variables are properly configured to use
APP_DIRas the base, which keeps the build artifacts organized within the container's application directory.
69-73: Well-structured multi-line Go build command.The build command is clearly formatted with proper comments explaining each flag. The static linking and version injection are correctly implemented.
96-107: Excellent non-root user setup.The user and group creation is properly structured with separate commands for clarity. The explicit home directory creation ensures proper ownership setup.
128-134: Proper ownership and user switching.The ownership assignment and user switching to non-root is correctly implemented for security best practices.
135-144: Correct handling of runtime environment variables.The solution to expose build arguments as environment variables for runtime use is appropriate. The shell-form entrypoint correctly allows environment variable expansion.
36-36: Base image golang:1.24-alpine is available on Docker HubThe tag
golang:1.24-alpineexists (including variants1.24-alpineand1.24.5-alpine), so you can safely use it as your builder image.
78-78: Alpine 3.22 tag verified on Docker HubThe
FROM alpine:3.22line is valid—Alpine 3.22 exists in the official registry, so no change is needed.
- File: docker/dockerfile-api
- Line: 78
Summary by CodeRabbit