Skip to content

Significantly improve the Dockerfile #433

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

Merged
merged 5 commits into from
Apr 4, 2019
Merged

Significantly improve the Dockerfile #433

merged 5 commits into from
Apr 4, 2019

Conversation

nhooyr
Copy link
Contributor

@nhooyr nhooyr commented Apr 4, 2019

Thanks to @RichardMcSorley and @sr229

Describe in detail the problem you had and how this PR fixes it

Is there an open issue you can link to?

@nhooyr nhooyr force-pushed the docker-fixes branch 3 times, most recently from 54cf52c to cae2b3f Compare April 4, 2019 21:52
@nhooyr nhooyr requested a review from ammario April 4, 2019 22:04
@nhooyr
Copy link
Contributor Author

nhooyr commented Apr 4, 2019

Will need to update the README.md with a command to mount into /home/coder/project.

@nhooyr
Copy link
Contributor Author

nhooyr commented Apr 4, 2019

Unfortunately, dumb-init is not sending signals properly when using the -t option to docker so I'm not sure what's going on.

@nhooyr
Copy link
Contributor Author

nhooyr commented Apr 4, 2019

Hikari and others added 5 commits April 4, 2019 18:41
im abusing the word slap today please help me
- Adds dumb-init so closes #403, closes #361, closes #383
- User mode docker so closes #192, closes #65
- Uses latest docker ubuntu instead of 18.10 which is the rolling tag so closes #404

Thanks to @RichardMcSorley and @sr229
@nhooyr nhooyr merged commit 6737d3d into master Apr 4, 2019
@nhooyr nhooyr deleted the docker-fixes branch April 4, 2019 23:42
@RichardMcSorley
Copy link
Contributor

@nhooyr Great you got it working.

@frol
Copy link

frol commented Apr 5, 2019

Passwordless sudo is like an always-open door (in the real world). There is definitely a room for improvement.

@sr229
Copy link
Contributor

sr229 commented Apr 5, 2019

@frol its still teaching basic practice anyways, so I don't think primarily its open door especially if its just a container (and the fact GCP practices this as well due to the fact - you know, they're SSH keys only by default).

@nhooyr
Copy link
Contributor Author

nhooyr commented Apr 5, 2019

We could prompt users to set a password on startup later @frol but for now this is a simple improvement.

@sr229
Copy link
Contributor

sr229 commented Apr 5, 2019

I vote against that IMHO, I don't think we need to support that kind of use case especially in Docker. The user should do it themselves if they really want to. We just provide a sane default for them.

@nhooyr
Copy link
Contributor Author

nhooyr commented Apr 5, 2019

Asking for a password is a safe default and the common use case. What we're providing is the exception.

code-asher pushed a commit that referenced this pull request Jun 19, 2019
Significantly improve the Dockerfile
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants