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

Run as user deno by default #166

Open
wperron opened this issue Aug 25, 2021 · 7 comments
Open

Run as user deno by default #166

wperron opened this issue Aug 25, 2021 · 7 comments

Comments

@wperron
Copy link
Contributor

wperron commented Aug 25, 2021

Right now we only create the deno:deno user in the Dockerfile, but it is up to the user to add the USER deno:deno directive to their Dockerfiles. I think it would be beneficial to add that directive to the images so that it becomes the default behavior and running as root becomes a conscious choice.

@felipecrs
Copy link
Contributor

felipecrs commented Aug 25, 2021

Hm... this is interesting. But I think it would break some simpler flows such as running deno run -v $PWD:/app deno run myapp.ts, since the user on local machine will have a different UID than the deno user in the container.

It would require to pass --user $(id -u):$(id -g) to make it work again.

I don't recall if there is a docker advice regarding this for the official images, but for example node is an official image and it runs as root by default despite it has the node user.

My suggestion is to keep the root user as the default.

Maybe it's worthwhile to add --user $(id -u):$(id -g) in the example for running a main.ts in the current directory as a volume, as it could write files in the directory and then these files would not be owned by the user who ran the command. This situation is similar to matejak/argbash#115.

@wperron
Copy link
Contributor Author

wperron commented Aug 25, 2021

I don't mind this being a breaking change for the few users that will have behaviors that depend on being run as root, they can update their Dockerfiles to include USER root if they really want that.

@hayd
Copy link
Contributor

hayd commented Aug 25, 2021

This is going to break many people's docker files, in what I think could be a subtle way (potentially confusing for people). It's pretty normal to run scripts / install things that require root in a docker file. It would seem strange to do this if other official base images do not.

IIRC this was the case in the past but was undone as too much of a pain point.

Should you decide to do this please add a big warning/explanation about it in the README...

@felipecrs
Copy link
Contributor

I don't mind this being a breaking change for the few users that will have behaviors that depend on being run as root, they can update their Dockerfiles to include USER root if they really want that.

My concern is not about those building images based on deno, but those running deno through docker as a shim.

@hayd
Copy link
Contributor

hayd commented Aug 25, 2021

I guess it's also true that "pain point" might have been a couple of users complained 🤷 😆

@wperron
Copy link
Contributor Author

wperron commented Aug 25, 2021

@hayd oh you mean when people are building off of that deno image and running additional steps in their Dockerfile, like apt install where they might expect to run as root but suddenly they're not?

@hayd
Copy link
Contributor

hayd commented Aug 25, 2021

@wperron yep, that was the complaint(s) before.

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

No branches or pull requests

3 participants