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

Add Let's Encrypt support #300

Closed
wants to merge 53 commits into from
Closed

Add Let's Encrypt support #300

wants to merge 53 commits into from

Conversation

dmp1ce
Copy link

@dmp1ce dmp1ce commented Nov 21, 2015

Using the simp_le Let's Encrypt client a bash script runs and continuously tries to create/renew certificates if the LETSENCRYPT_HOST variable is set for a container.

Certificates are automatically downloaded and placed in the correct location for nginx-proxy to use them.

@dmp1ce
Copy link
Author

dmp1ce commented Nov 21, 2015

I'm not expecting that this actually gets merged because simp_le adds a lot of bloat to the container, I think it is worth a look for anyone wanting to integrate Let's Encrypt into their nginx-proxy.

@dmp1ce dmp1ce mentioned this pull request Nov 21, 2015
dmp1ce and others added 4 commits November 21, 2015 18:47
Avoid 'ln: failed to create symbolic link' errors
Update server URL to use the production ready API URL.
@hadim
Copy link

hadim commented Nov 22, 2015

Must have feature !

dmp1ce and others added 4 commits November 22, 2015 17:26
Update or create the certificates as soon as possible
Merge JrCs letsencrypt_service improvements

- Create new certificate for each domain instead of trying to combine domains into one certificate
- Enhance letsencrypt_service so that new or updated containers don't wait for new certificates
@md5
Copy link
Contributor

md5 commented Nov 23, 2015

@dmp1ce What if instead of merging this into the default image/tag or maintaining it as a fork, it were to be made available as a derived image or add-on somehow?

@dmp1ce
Copy link
Author

dmp1ce commented Nov 23, 2015

@md5 I thought about that but I think I couldn't come up with a solution where I wasn't overriding the nginx.tmpl and Procfile. So, I would be managing changes from nginx-proxy either way.

@hadim
Copy link

hadim commented Nov 24, 2015

@md5 is there is a possibility for you to merge this feature ?

Letsencrypt is just a specific service but I think in a near future ACME client will become widely used and implemented by a several certificate authorities (not just letsencrypt). I really think this feaure is a must have. More over it's opt-in meaning that usual user won't see anything different from before.

I'll respect if you are against merging it. In this case I think @dmp1ce you should think about forking it.

@jwilder
Copy link
Collaborator

jwilder commented Nov 24, 2015

@hadim I won't be merging this as is since it adds a lot of weight to the image and I haven't had time to look at LE.

@dmp1ce
Copy link
Author

dmp1ce commented Nov 24, 2015

@hadim This project is already forked and the Docker image is available here: https://hub.docker.com/r/dmp1ce/nginx-proxy-letsencrypt/

@JrCs
Copy link

JrCs commented Nov 24, 2015

@jwilder In my PR request (dmp1ce#2) i have reduce the image size a lot: from 469 MB to 224.1 MB.
Only files needed by simp_le are add to your base image.

Use jwilder/nginx-proxy as base image and reduce final image size
@hadim
Copy link

hadim commented Nov 25, 2015

@dmp1ce very nice ! Could you please open Github issues in your fork ? I have an issue when $domain contains multiple host such as "example.com,www.example.com". simp_le fails with this error ERROR:simp_le: ACME server returned an error: unauthorized :: The client lacks sufficient authorization :: Error creating new authz :: Syntax error.

The syntax should be -d example.com -d www.example.com I think.

Please enable issues in your fork, I don't want to create unnecessary noise here.

@hadim
Copy link

hadim commented Nov 25, 2015

For the record, the following diff against letsencrypt_service file should fix it :

@@ -17,7 +17,9 @@
         hosts_array=$host_varname[@]
         email_varname="LETSENCRYPT_${cid}_EMAIL"

-        for domain in "${!hosts_array}"; do
+        IFS=', ' read -r -a hosts_splitted <<< "${!hosts_array}"
+
+        for domain in "${hosts_splitted[@]}"; do

             # Create the domain directory
             mkdir -p /etc/nginx/certs/$domain

@thaJeztah
Copy link
Contributor

I wonder if generating and downloading certificates is something that belongs in the proxy image. Yes, I love to see more widespread use of letsencrypt, just not sure it needs to be in the same image.

I haven't looked into the details, but after the certificate has been generated and downloaded, it's just a "regular" host, and nginx-proxy does not need to know where the certificate came from (correct me if I'm over-simplifying)

Would it be possible to create a separate project/image (e.g. "letsencrypt-generator"), that listens to docker events, checks for LETSENCRYPT_xx env vars, writes the certificates to a shared volume, and triggers the nginx-proxy container to reload the configuration?

dmp1ce and others added 11 commits December 9, 2015 17:07
Update to use the new simp_le API
Add support for alternative names
Cleanup test containers when possible during tests.
Add 'test-clean' make target for remaining bat-* containers.
- Specify assert lines for tests to avoid btrfs errors with CircleCI
- Update Docker version to 1.9.1 in CircleCI
We don't need a connection over a proxy to a proxy.
eliminate confusion in example of Let's Encrypt
New ACME_CA_URI env parameter
@janeczku
Copy link

@dmp1ce

I'm not expecting that this actually gets merged because simp_le adds a lot of bloat to the container

Please have a look at https://github.com/xenolf/lego. It's a LE client in a single Go binary without any dependencies. This should reduce the overhead to a mere 10 MB.

@dmp1ce
Copy link
Author

dmp1ce commented Dec 29, 2015

@janeczku

I believe @JrCs has gotten simp_le install down to 20 MB so the size isn't that big of a deal. Are there any other reasons to use lego over simp_le? simp_le is working well for this project so far.

@janeczku
Copy link

@dmp1ce

I believe @JrCs has gotten simp_le install down to 20 MB

All good then! 😄

@JrCs
Copy link

JrCs commented Jan 1, 2016

I have create a new dedicated container: letsencrypt-nginx-proxy-companion that create/renew Let's Encrypt certificates automatically but using the official nginx-proxy.

@thaJeztah
Copy link
Contributor

Awesome work, @JrCs!

@jacobtomlinson
Copy link

I'm using @JrCs's letsencrypt-nginx-proxy-companion and it does the job perfectly.

@tmiklas
Copy link

tmiklas commented Jan 27, 2016

Thanks @JrCs for your work - it does the job perfectly!

@dmp1ce - if possible, it would be useful to configure vhosts in a way that if there's only HTTP version of the site (no LETSENCRYPT_HOST variable set == no keys generated), there is a dummy/default error page or simple HTTP error code returned instead of some other random HTTPS served by the same proxy instance.

Scenario below:

  • Site 1 with LETSENCRYPT_HOST set - available on HTTP (redir to HTTPS) and HTTPS
  • Site 2 w/o LETSENCRYPT_HOST set - available on HTTP, when accessed over HTTPS shows Site 1

@dmp1ce
Copy link
Author

dmp1ce commented Jan 27, 2016

@tmiklas, If you set DEFAULT_HOST to "Site 1" then it might work. https://github.com/jwilder/nginx-proxy#default-host

You can also add your own custom nginx configuration if you need. https://github.com/jwilder/nginx-proxy#custom-nginx-configuration.

In what situation would you not just get a certificate for all sites?

@tmiklas
Copy link

tmiklas commented Jan 27, 2016

That would work @dmp1ce... will test shortly.
The situation where site doesn't have certificate would mostly be when user decides to not have it (dunno why, but hey...) and effectively mixing HTTP and HTTP/HTTPS sites behind single proxy. Matter of applying common sense and best practices.
Thanks for quick advice!

@18601673727
Copy link

I have serval proxies fighting each other for port 80 since @dmp1ce 's fork has own auto letsencrypt feature and @rckrdstrgrd one doesn't but his fork has own docker-compose V2 feature, so is there someone could possible to merge their forks and give my proxies some peace?

We just need one nginx-proxy for god sake.

@dmp1ce
Copy link
Author

dmp1ce commented Apr 6, 2016

@18601673727 No one is supporting my fork any longer. I recommend using another nginx-proxy. I use jwilder/nginx-proxy and JrCs/docker-letsencrypt-nginx-proxy-companion for Let's Encrypt.

@18601673727
Copy link

Yeah, @JrCs really did a nice job, now I have both docker-compose V2 and auto letsencrypt feature in my build, problem solved!

@giovannicandido
Copy link

Awesome. This feature is great. Nice work guys :-)

@corradio
Copy link

Any progress here?

@jwilder
Copy link
Collaborator

jwilder commented Jan 6, 2017

Going to close this since https://github.com/JrCs/docker-letsencrypt-nginx-proxy-companion can be used for Let's Encrypt SSL support.

@jwilder jwilder closed this Jan 6, 2017
Alexander-Krause-Glau pushed a commit to Alexander-Krause-Glau/rpi-docker-nginx-proxy that referenced this pull request Mar 31, 2018
+ add foundation for future support of other container management services
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

Successfully merging this pull request may close these issues.