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

fix: run container as non-root #905

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

praktiskt
Copy link
Contributor

First stab at running the Tabby container as non-root.

I've not explored thoroughly if there are other directories that needs to be owned by the tabby user. I am able to run a simple serve (tabby serve --model ...) with this setup.

Hoping CI will tell me more, if there is more.

@praktiskt praktiskt changed the title fix: run as non-root fix: run container as non-root Nov 27, 2023
@@ -61,5 +61,10 @@ RUN ln -s /usr/lib/x86_64-linux-gnu/libnvidia-ml.so.1 \
COPY --from=build /opt/tabby /opt/tabby

ENV TABBY_ROOT=/data
ARG WRITE_DIRS="${TABBY_ROOT} ${TABBY_ROOT}/events"
Copy link
Member

Choose a reason for hiding this comment

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

This won't work - as $TABBY_ROOT could be mounted to external directory, which might not writable to 1001 uid.

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 was the intended effect. To me it makes sense if the container don't write to the user file system as root, even if its mounted.

Any alternate takes on how to solve this? Do you agree?

Copy link
Member

Choose a reason for hiding this comment

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

The tradeoff being considered here is that we want to avoid inconveniencing the first-time user with the manual execution of chmod (as they are likely running docker -it directly).

One solution I have implemented is to create a new Docker image based on Tabby's distribution, similar to the one provided for Hugging Face deployment: https://huggingface.co/spaces/TabbyML/tabby-template-space/blob/main/Dockerfile

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea isn't that the user should run chmod, it's that the tabby container shouldn't modify user space, especially not as root.

I agree that it should be as smooth as possible for new users.

Where does tabby need to write today?

@wsxiaoys wsxiaoys marked this pull request as draft November 28, 2023 01:45
@praktiskt
Copy link
Contributor Author

@wsxiaoys Do you think you'll ever want to approach something like this with the project? If not, I'll close the PR so we don't leave it lingering forever.

@wsxiaoys
Copy link
Member

@wsxiaoys Do you think you'll ever want to approach something like this with the project? If not, I'll close the PR so we don't leave it lingering forever.

Hey - sorry for missing the last response. As a reference, we do run tabby as non-root for huggingface space tutorials, https://huggingface.co/spaces/TabbyML/tabby-template-space/blob/main/Dockerfile

I'm still not sure whether it can be easily integrated into the repository, as we want to avoid the hassle of using 'chmod' when invoking 'docker run'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants