search

scrapinghub / docker-devpi

pypi caching service using devpi and docker
28 stars 42 forks source link

Use Nginx as reverse proxy #2

Closed little-dude closed 7 years ago

little-dude commented 9 years ago

I found your image thanks to https://dantehranian.wordpress.com/2014/09/03/a-local-caching-proxy-for-pypi-python-org-via-docker/

I'd like to use NGINX to do https. This PR does not introduce HTTPS yet, but it adds nginx as reverse proxy, which is the first step.

There are some other smaller changes (see the commit message) which are not directly related, so let me know if you want me to do several PRs.

From a user point of view those changes should be transparent. The behaviour is unchanged, but hunder the hood, the image uses nginx as reverse proxy.

Note that despite the fact that everything is working, I'm seeing errors in nginx logs sometimes :

2015/04/09 15:45:05 [error] 78#0: *8 connect() failed (111: Connection refused) while connecting to upstream, client: 172.17.42.1, server: localhost, request: "GET /root/dev HTTP/1.1", upstream: "http://[::1]:3142/root/dev", host: "localhost:3141"

I'm not familiar with nginx so if you have any idea to fix this...

tehranian commented 9 years ago

Couldn't you run the nginx proxy on the host OS and have it handle all of the HTTPS bits, then proxy to the devpi app in the backend with regular HTTP?

In other words, does the nginx service really need to be within the container?

little-dude commented 9 years ago

Yes, what you say is tperfeclty doable. The reasons I'm doing it like this are :

dangra commented 9 years ago

Thanks for looking into this, I am generally +1 to merge but see my comments.

IMHO https is not a reason to include nginx into the container but serving static files using nginx it is. If you managed to send a pull request later with a non intrusive way to enable TLS (disabled by default), I'll merge it.

little-dude commented 9 years ago

Thanks for looking into this, I am generally +1 to merge but see my comments.

Thanks for reviewing and commenting. I'll repush soon.

IMHO https is not a reason to include nginx into the container but serving static files using nginx it is. If you managed to send a pull request later with a non intrusive way to enable TLS (disabled by default), I'll merge it.

I was planning on adding a HTTPS env variable. If set to true, both http and https would be enabled (on ports 3141 and 3142 respectively). The image would be shipped with a default self-signed certificate, but I'd specify in the README that it's recommended to use your own CA. Does that sound good?

dangra commented 9 years ago

I was planning on adding a HTTPS env variable. If set to true, both http and https would be enabled (on ports 3141 and 3142 respectively). The image would be shipped with a default self-signed certificate, but I'd specify in the README that it's recommended to use your own CA. Does that sound good?

:ok_hand:

little-dude commented 9 years ago

@dangra I re-pushed taking your comments into account, and squashed everything to make reviewing easier. I also added the HTTPS config to the nginx template. Thanks for the env | sed command, it's pretty awesome.

Please don't merge, since for the moment, https does not work and i have no idea why.

bubenkoff commented 8 years ago

wouldn't make more sense to keep the container running only one service you can always add nginx as proxy running in another container and use container linking to connect it with devpi. Here's the example how i do it when running in amazon elastic beanstalk https://github.com/InnovativeTravel/devpi-beanstalk