feat: adding support for user not root to Docker#2489
feat: adding support for user not root to Docker#2489aycsi wants to merge 1 commit intospotDL:devfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR addresses issue #1729 by modifying the Docker setup to run the container as a non-root user instead of root, allowing users to access downloaded music files without requiring sudo permissions.
- Added user/group ID customization through build arguments with sensible defaults
- Updated docker-compose configuration to support user ID passing and improved volume mapping
- Enhanced documentation with docker-compose usage instructions including user ID setup
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Dockerfile | Creates a non-root user 'spotdl' with customizable UID/GID and sets proper permissions |
| docker-compose.yml | Adds build context with UID/GID args and updates volume mapping to ./downloads |
| README.md | Adds docker-compose usage documentation with user ID configuration instructions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| # Change Workdir to download location | ||
| WORKDIR /music | ||
|
|
||
| # Fix permissions for music directory | ||
| RUN chown spotdl:spotdl /music |
There was a problem hiding this comment.
Setting ownership on the VOLUME /music directory will not persist when the volume is mounted at runtime. The chown command here affects the image layer but gets overridden when a host directory is mounted to /music. Consider removing this step or documenting that host directory permissions need to be managed separately.
| # Change Workdir to download location | |
| WORKDIR /music | |
| # Fix permissions for music directory | |
| RUN chown spotdl:spotdl /music | |
| # NOTE: If you mount a host directory to /music, you must ensure the directory is writable by the 'spotdl' user (UID/GID), as permissions set here will not persist. | |
| # Change Workdir to download location | |
| WORKDIR /music | |
| # (chown on /music removed; see note above) |
| image: "spotdl/spotify-downloader" | ||
| volumes: | ||
| - .:/music | ||
| - ./downloads:/music |
There was a problem hiding this comment.
The volume mapping has changed from mounting the current directory (.) to ./downloads. This is a breaking change that should be documented in the description as it changes the expected directory structure for existing users.
There was a problem hiding this comment.
thanks for your PR and sorry for the late review.
I see that you've set a default volume mapping when using compose, one thing to note is that since the
Docker container would not be root anymore, it wouldn't be able to access the files under /music in the container because of permissions.
Copilot points it out:
https://github.com/spotDL/spotify-downloader/pull/2489/changes#r2320603547
So we have two options:
- Move from bind volumes to named volumes. This would add a little more complexity to UX since now we would have to
docker cpfiles from the volume to the host. - Revert changes and keep using
rootuser, but keep using bind mounts.
cc: @Silverarmor, if we want to improve security by not using root user I personally think option no. 1 is the way to go, even with the added complexity.
nevertheless, @aycsi please let me know what you think or other solutions you can come up with!
Docker container running as root #1729.
Description
Fixing the Docker container running as root issue at #1729.
Motivation and Context
So users can access and manage their downloaded music files without using sudo.
Types of Changes
Checklist