search

taivokasper / docker-omnidb

OmniDB installed into a Docker container
36 stars 19 forks source link

Added --host parm and changed CMD into ENTRYPOINT exec form #5

Closed eembsen closed 5 years ago

eembsen commented 5 years ago

I added the --host param because OmniDB starts by default on 127.0.0.1, whereas it needs to bind on 0.0.0.0 to be accessible from outside.

taivokasper commented 5 years ago

Hi, I'm currently travelling. I will look into your PR in a couple of days. Please ignore the "failed checked" in meantime.

thomasboussekey commented 5 years ago

Thank you @eembsen ,

I updated my Dockerfile according to your entrypoint definition in the commit, and it works perfectly from the local connection!

taivokasper commented 5 years ago

Hi, What is the usecase? Are you running this container in a server and want it to be accessible from outside or are you claiming that it doesn't work because it is not accessible from the host machine running docker?

While I like the idea of using entrypoint instead of cmd I don't think it should bind to 0.0.0.0 by default.

It would be a huge security issue when people are walking around with laptops that have open database clients running and accessible to anyone in the same network.

I propose to remove --host=0.0.0.0 and when you really need to bind to all network interfaces then you run the container like this: docker run -it --rm -p 8080:8080 -p 25482:25482 taivokasper/omnidb --host=0.0.0.0

What do you think?

taivokasper commented 5 years ago

I just noticed that this container has been broken since OmniDb 2.11.0. I'll do a new release after merging this PR under tag v2.11.0-alpine_3.8-1.

I will also add some basic tests for this image