-
Notifications
You must be signed in to change notification settings - Fork 148
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
Update docker build to support adding a non-root default user #68
base: branch-23.11
Are you sure you want to change the base?
Changes from 6 commits
60cf5ed
fb85b2a
791ffb1
07e0055
fc432f0
d598b3e
3dee821
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -149,6 +149,23 @@ RUN --mount=type=ssh \ | |
# Setup container for runtime environment | ||
FROM conda_env as runtime | ||
|
||
ARG MORPHEUS_USER="root" | ||
|
||
# Conditionally create user and install sudo | ||
RUN if [ ! -z "${MORPHEUS_USER}" ] && [ "${MORPHEUS_USER}" != "root" ] ; then \ | ||
useradd -ms /workspace ${MORPHEUS_USER} \ | ||
&& usermod -aG sudo ${MORPHEUS_USER} \ | ||
&& apt-get update \ | ||
&& apt-get install -y sudo \ | ||
&& apt-get clean \ | ||
&& echo '%sudo ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers ; \ | ||
else \ | ||
echo "Skipping user creation..." ;\ | ||
fi | ||
|
||
# Allow default user to execute sudo commands in container | ||
RUN echo '%sudo ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldnt this go in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, good catch. I'll move it into the check. |
||
|
||
# Manually need to install some pip-only dependencies. Once these can get moved to conda, they can be removed. | ||
COPY docker/conda/environments/requirements.txt ./docker/conda/environments/ | ||
COPY docker/conda/environments/cuda${CUDA_VER}_runtime.yml ./docker/conda/environments/ | ||
|
@@ -172,13 +189,29 @@ COPY "./models" "./models" | |
COPY "./scripts" "./scripts" | ||
COPY ["*.md", "LICENSE", "./"] | ||
|
||
USER ${MORPHEUS_USER} | ||
|
||
# Use morpheus by default | ||
CMD [ "morpheus" ] | ||
|
||
# ============ Stage: development ============ | ||
# Install and configure development only packages | ||
FROM conda_env_dev as development | ||
|
||
ARG MORPHEUS_USER="root" | ||
|
||
# Conditionally create user, install sudo, and allow user to run sudo commands | ||
RUN if [ ! -z "${MORPHEUS_USER}" ] && [ "${MORPHEUS_USER}" != "root" ] ; then \ | ||
useradd -ms /workspace ${MORPHEUS_USER} \ | ||
&& usermod -aG sudo ${MORPHEUS_USER} \ | ||
&& apt-get update \ | ||
&& apt-get install -y sudo \ | ||
&& apt-get clean \ | ||
&& echo '%sudo ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers ; \ | ||
else \ | ||
echo "Skipping user creation..." ;\ | ||
fi | ||
|
||
Comment on lines
+204
to
+214
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we move this into one of the above layers (such as base or conda_env) to avoid duplicating it in both the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this makes sense. I'll move it up. |
||
# Copy the source | ||
# COPY . ./ | ||
|
||
|
@@ -189,3 +222,5 @@ RUN npm install -g camouflage-server | |
# greater. See https://marc.info/?l=git&m=164989570902912&w=2. Only enable for | ||
# development | ||
RUN git config --global --add safe.directory "*" | ||
|
||
USER ${MORPHEUS_USER} |
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.
I think this is pretty safe to include by default in the
build_container_dev.sh
and omit frombuild_container_release.sh
is it not? We could just add the following tobuild_container_dev.sh
:Or better yet, make a new option
DOCKER_IMAGE_USER
inbuild_container.sh
which will set the new build arg.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.
Generally speaking, it probably makes sense to specify a user for the release container too, just from a security standpoint, since we do things like mount volumes by default. We could set it to something default, like 'morpheus'. I don't think there is any downside to doing so.