-
Notifications
You must be signed in to change notification settings - Fork 129
Update dockerfile to use a rootless user #837
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
base: main
Are you sure you want to change the base?
Conversation
7d4b7b0
to
254e743
Compare
RUN ln -s /usr/lib/*-linux-gnu/libjemalloc.so.2 /usr/lib/libjemalloc.so.2 | ||
USER $USERNAME |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried moving this line up? I assume if we login using this username before running all the copy, we wouldn't need to specify the owner in the command
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I already tried that. Not sure if there is another workaround.
@@ -61,19 +61,32 @@ RUN apt-get update -qq && \ | |||
apt-get install --no-install-recommends -y curl libpq-dev libvips libjemalloc2 && \ | |||
apt-get clean | |||
|
|||
ENV USERNAME rails_api_base |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this way we won't need to change if for all the projects 🙂
ENV USERNAME rails_api_base | |
ENV USERNAME rails |
ENV USERNAME rails_api_base | ||
ENV USER_UID 1000 | ||
ENV USER_GID 1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use ENV
or ARG
here? 🤔 https://docs.docker.com/build/building/variables/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At first I tried using ARG but I remember I had some issues with that. I will take another look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the Docker configuration to run the application using a rootless user. Key changes include adding a node_modules volume in docker-compose.yml; creating and switching to a non-root user in Dockerfile.dev; and implementing a similar rootless user setup in Dockerfile with appropriate file ownership and permission adjustments.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
docker-compose.yml | Adds a dedicated node_modules volume to support the new user setup |
Dockerfile.dev | Introduces environment variables and commands to create/switch to a rootless user, including ownership and permission adjustments |
Dockerfile | Similar rootless user setup with additional commands for symlink creation and temporary user switching |
COPY --link --chown=$USERNAME:$USERNAME --chmod=700 . . | ||
COPY --from=builder --chown=$USERNAME:$USERNAME --chmod=700 $APP_HOME/public/ $APP_HOME/public/ | ||
COPY --from=builder --chown=$USERNAME:$USERNAME --chmod=700 $APP_HOME/tmp/ $APP_HOME/tmp/ | ||
COPY --from=builder --chown=$USERNAME:$USERNAME --chmod=700 $APP_HOME/vendor/ $APP_HOME/vendor/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a comment explaining the temporary switch to root for symlink creation to improve future maintainability and clarity.
# Temporarily switch to root to create a symlink for libjemalloc. |
Copilot uses AI. Check for mistakes.
Board:
Description:
Notes:
Tasks:
Risk:
Preview: