feat: enhance entrypoint and Dockerfiles for flexible volume and permission management #260
Conversation
…ission management\n\n- Support batch mount and permission fix in entrypoint.sh\n- Add coreutils/shadow (alpine) and coreutils/passwd (ubuntu) for UID/GID/ownership\n- Use ENTRYPOINT for unified startup\n- Make local dev and prod Dockerfile behavior consistent\n- Improve security and user experience\n\nBREAKING CHANGE: entrypoint.sh and Dockerfile now require additional packages for permission management, and support batch volume mount via RUSTFS_VOLUMES.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the Docker setup with flexible volume and permission management capabilities. The changes introduce batch volume mounting support via RUSTFS_VOLUMES environment variable, unified permission handling across different mount points, and improved security defaults.
- Refactored entrypoint script to support multiple volumes and flexible UID/GID management
- Updated both Dockerfiles to include required system tools (coreutils, shadow/passwd) for permission operations
- Added security improvements including default credential warnings and proper non-root user configuration
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| entrypoint.sh | Complete rewrite to support batch volume processing, flexible UID/GID handling, and improved permission management |
| Dockerfile.source | Added required system packages and entrypoint integration for Debian-based builds |
| Dockerfile | Updated Alpine-based build with architecture handling improvements and required packages |
| if [ "$dir_uid" != "$APP_UID" ] || [ "$dir_gid" != "$APP_GID" ]; then | ||
| if [[ "$SKIP_CHOWN" != "true" ]]; then | ||
| # Prefer to update rustfs user/group UID/GID | ||
| update_user_group_ids "$dir_uid" "$dir_gid" "$APP_USER" "$APP_GROUP" || \ |
There was a problem hiding this comment.
The function update_user_group_ids uses return $updated where updated is 0 or 1, but the logic expects failure when user/group modification isn't possible. This creates incorrect fallback behavior since return 0 indicates success in bash. Consider returning 1 when modifications fail or restructure the logic.
Copilot uses AI. Check for mistakes.
| echo "ARCH=${ARCH}" > /build/arch.env | ||
|
|
||
| # Download and extract RustFS binary | ||
| RUN . /build/arch.env && \ | ||
| BASE_URL="https://dl.rustfs.com/artifacts/rustfs/release" && \ |
There was a problem hiding this comment.
The parameter expansion ${RELEASE#v} will remove a leading 'v' from RELEASE, but if RELEASE doesn't start with 'v', it remains unchanged. This could cause download failures if the naming convention isn't consistent. Consider validating the RELEASE format or using a more robust parsing approach.
| BASE_URL="https://dl.rustfs.com/artifacts/rustfs/release" && \ | |
| BASE_URL="https://dl.rustfs.com/artifacts/rustfs/release" && \ | |
| if [[ "${RELEASE}" != v* ]]; then \ | |
| echo "Invalid RELEASE format: ${RELEASE}. Expected format: 'v<version>'." >&2 && exit 1; \ | |
| fi && \ |
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Summary
BREAKING CHANGE:
Testing
Closes # (please fill if related)