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

Added Certbot image #24

Closed
wants to merge 2 commits into from
Closed

Added Certbot image #24

wants to merge 2 commits into from

Conversation

Megajin
Copy link

@Megajin Megajin commented Mar 30, 2019

The Certbot image was missing in the .yml file.
I added the image and set the volumes.

Megajin added 2 commits March 30, 2019 13:56
This error occured when used "letsencrypt\letsencrypt-init.sh"
with no certbot image installed for the current user.
@mjstealey
Copy link
Owner

@Megajin - I had left the certbot definition outside of the docker-compose scope due to the infrequency of it needing to be run. The init step is once per installation and the renew script is periodic. I'll tend to run the renew script as a cron task every 15 days, so haven't found it to be necessary to wire it into the compose definition as an additional service.

The docker run call for certbot is defined in the init and renew scripts (i.e. from init: https://github.com/mjstealey/wordpress-nginx-docker/blob/master/letsencrypt/letsencrypt-init.sh#L89-L95). It uses the .env file and does some manipulation of the deployed Nginx container to accommodate the well-known directory structure that Let's Encrypt is looking for. Even if certbot is deployed as a service there is still some configuration that needs to be done to have all things work as expected. So I hadn't personally seen the advantage of wiring it as a compose service...

All that said there is no harm in having it defined in compose, but it should be documented as to how it should be used within the context of both init and renew. Perhaps even commented out with some deployment notes as an optional method of use... will have to think on it a bit.

@Megajin
Copy link
Author

Megajin commented Apr 10, 2019

@mjstealey you are right it is not needed in the setup. I had another problem and was thinking the cause might be the missing image. However I was wrong. So feel free to reject this pull request if you come to the conclusion that it is not needed in the setup.

I used this reference: https://medium.com/@pentacent/nginx-and-lets-encrypt-with-docker-in-less-than-5-minutes-b4b8a60d3a71

@mjstealey mjstealey closed this Oct 7, 2019
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.

2 participants