Dockerfile improvements #31

Closed
opened 2026-02-04 16:19:13 +03:00 by OVERLORD · 5 comments
Owner

Originally created by @dustyrip on GitHub (Jul 11, 2018).

Hello, first of all thanks you @dani-garcia and @mprasil for this this project.

Is there a reason for exposing port 80 by default from Dockerfile? I think this should by up to user to decide with docker run.

Also can we get ROCKET_ENV set to prod or make it user-customizable with docker run ... -e ROCKET_ENV=prod... it should give us better performance so its worth to try.

EDIT: I have created PR.

Originally created by @dustyrip on GitHub (Jul 11, 2018). Hello, first of all thanks you @dani-garcia and @mprasil for this this project. Is there a reason for [exposing port 80](https://github.com/dani-garcia/bitwarden_rs/blob/master/Dockerfile#L79) by default from Dockerfile? I think this should by up to user to decide with `docker run`. Also can we get `ROCKET_ENV` set to `prod` or make it user-customizable with `docker run ... -e ROCKET_ENV=prod...` it should give us [better performance](https://github.com/SergioBenitez/Rocket/issues/315#issuecomment-308723800) so its worth to try. **EDIT**: I have created [PR](https://github.com/dani-garcia/bitwarden_rs/pull/70).
Author
Owner

@mprasil commented on GitHub (Jul 11, 2018):

Hi, it looks like your problem with EXPOSE comes from misunderstanding the EXPOSE directive. This just defines the port, that will be exposed when you run container with the -P option. It does not prevent you in any way to specify your own desired port mapping. So it's merely a default and that default is set based on the fact that server indeed starts on port 80 if you don't specify otherwise via ROCKET_PORT

@mprasil commented on GitHub (Jul 11, 2018): Hi, it looks like your problem with `EXPOSE` comes from misunderstanding the [EXPOSE directive](https://docs.docker.com/engine/reference/builder/#expose). This just defines the port, that will be exposed when you run container with the `-P` option. It does not prevent you in any way to [specify your own desired port mapping](https://docs.docker.com/engine/reference/run/#expose-incoming-ports). So it's merely a default and that default is set based on the fact that server indeed starts on port 80 if you don't specify otherwise via `ROCKET_PORT`
Author
Owner

@dustyrip commented on GitHub (Jul 11, 2018):

Yes, but I still think EXPOSE should not be used in Dockerfiles. docker container ls shows these ports even if not mapped into host so its confusing.

@dustyrip commented on GitHub (Jul 11, 2018): Yes, but I still think EXPOSE should not be used in Dockerfiles. `docker container ls` shows these ports even if not mapped into host so its confusing.
Author
Owner

@mprasil commented on GitHub (Jul 11, 2018):

Ah I see what you mean there. Unfortunately AFAIK there's no way to tell docker daemon during startup on which port is the application running internally. I'd still leave it in as it is the correct port as long as you don't deliberately change it in which case you're aware that it's different.

Having the EXPOSE there makes it a bit easier to use with some systems that can use the port automatically, so it is useful in some cases. Also for example this is handy when running the container with rkt when you try to expose the port.

@mprasil commented on GitHub (Jul 11, 2018): Ah I see what you mean there. Unfortunately AFAIK there's no way to tell docker daemon during startup on which port is the application running internally. I'd still leave it in as it is the correct port as long as you don't deliberately change it in which case you're aware that it's different. Having the `EXPOSE` there makes it a bit easier to use with some systems that can use the port automatically, so it is useful in some cases. Also for example this is handy when running the container with [rkt](https://coreos.com/rkt/) when you try to [expose the port](https://coreos.com/rkt/docs/latest/networking/overview.html#exposing-container-ports-on-the-host).
Author
Owner

@dani-garcia commented on GitHub (Jul 11, 2018):

I've merged the PR with the ENV changes.

About the EXPOSE instruction, I only have some basic knowledge about Docker, so you guys probably have a better idea about what's the most appropiate option. I'll leave that up to you!

@dani-garcia commented on GitHub (Jul 11, 2018): I've merged the PR with the ENV changes. About the `EXPOSE` instruction, I only have some basic knowledge about Docker, so you guys probably have a better idea about what's the most appropiate option. I'll leave that up to you!
Author
Owner

@mprasil commented on GitHub (Jul 11, 2018):

I'm for leaving it in if you don't mind @dustyrip. The EXPOSE directive is definitely useful in it's default configuration. Systems like Nomad, Traefik or Kubernetes can route to the proper container port based on that information without specifying any port mapping.

@mprasil commented on GitHub (Jul 11, 2018): I'm for leaving it in if you don't mind @dustyrip. The `EXPOSE` directive is definitely useful in it's default configuration. Systems like Nomad, Traefik or Kubernetes can route to the proper container port based on that information without specifying any port mapping.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/vaultwarden#31