Skip to content
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

Open
wants to merge 7 commits into
base: branch-23.11
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,13 @@ This workflow utilizes a docker container to set up most dependencies ensuring a
DOCKER_IMAGE_TAG=my_tag ./docker/build_container_dev.sh
```
Would build the container `morpheus:my_tag`.
1. Note: This does not build any Morpheus or Neo code and defers building the code until the entire repo can be mounted into a running container. This allows for faster incremental builds during development.
1. If you would like to build the container to have a default user, other than root, add the following to your
`build_container_dev.sh` call.
```bash
DOCKER_EXTRA_ARGS="--build-arg MORPHEUS_USER=[YOUR_USER_NAME]" ./docker/build_container_dev.sh
```
Comment on lines +93 to +97
Copy link
Contributor

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 from build_container_release.sh is it not? We could just add the following to build_container_dev.sh:

DOCKER_EXTRA_ARGS="${DOCKER_EXTRA_ARGS} --build-arg MORPHEUS_USER=$USER"

Or better yet, make a new option DOCKER_IMAGE_USER in build_container.sh which will set the new build arg.

Copy link
Contributor Author

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.

1. Note: This does not build any Morpheus or Neo code and defers building the code until the entire repo can be
1. mounted into a running container. This allows for faster incremental builds during development.
drobison00 marked this conversation as resolved.
Show resolved Hide resolved
2. Set up `ssh-agent` to allow container to pull from private repos
```bash
eval `ssh-agent -s`
Expand All @@ -110,7 +116,6 @@ This workflow utilizes a docker container to set up most dependencies ensuring a
```bash
./docker/install_docker.sh
```

4. Compile Morpheus
```bash
./scripts/compile.sh
Expand Down
35 changes: 35 additions & 0 deletions docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldnt this go in the else branch of the above if statement? Instead of the line echo "Skipping user creation..." ;\. This would save a layer

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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/
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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 development and runtime targets? We dont need to set the USER until needed but we certainly can create the user in the base image.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 . ./

Expand All @@ -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}