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

Only renew certificate if expires within 30 days #14

Merged
merged 1 commit into from
Jun 18, 2018

Conversation

q3k
Copy link
Contributor

@q3k q3k commented Mar 12, 2018

This is a quick hack to not hammer the LE API and thus not hit the weekly limit, if, for instance, your container keeps restarting.

The openssl incantation will only check the certificate expiration time, and not whether it is valid (ie. checking against the system CA store) - implementing that is left as an exercise to the reader.

We also make the renew check happen every day, instead of every 60 days, now that we can afford it.

This fixes #13 .

This is a quick hack to not hammer the LE API and thus not hit the
weekly limit, if, for instance, your container keeps restarting.

The openssl incantation will only check the certificate expiration time,
and not whether it is valid (ie. checking against the system CA store) -
implementing that is left as an exercise to the reader.

We also make the renew check happen every day, instead of every 60 days,
now that we can afford it.

Signed-off-by: Sergiusz Bazanski <q3k@q3k.org>
@umputun
Copy link
Collaborator

umputun commented Mar 12, 2018

interesting idea. I'll play with this a little bit, but looks very promising.

@@ -46,7 +46,7 @@ mv -v /etc/nginx/conf.d /etc/nginx/conf.d.disabled
mv -v /etc/nginx/conf.d.disabled /etc/nginx/conf.d #enable
echo "reload nginx with ssl"
nginx -s reload
sleep 60d
sleep 1d
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will reload nginx every day. not sure how smart nginx handles reloads but I think it will be safer to add status code of le.run and skip reload if != 0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my experience this should not be a problem at all. The documentation also says:

Configuration reload
Start the new worker processes with a new configuration
Gracefully shutdown the old worker processes

So this shouldn't kill any active connections.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will happen if service.conf changed to something broken from syntax point of view? E.g. I'm editing file right now and nginx think it's time to reload?
With old behavior it happens once in two months, now it could happen every day.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nginx will keep running the old config file if reload is triggered while the current config is invalid.

Also, why would you be editing the file on production, anyway? :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tend to agree with @paskal - unneeded reloads are better to be avoided. We don't know how users manage they servers. If someone is brave enough to do it directly - fine with me, do whatever you want with your own server :) I think we should just eliminate daily reloads and perform it only in case of certs update/generation.

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.

--renew-by-default option used intentionally?
3 participants