Only attempt renewal of certificates that are close to expiry date #111
Conversation
Stored expiry date is checked before renewal. If less than 30 days out renewal is attempted. Tests are update to reflect the fact that dehydrated is not going to be called.
|
Looks good to me. @GUI what do you think? |
|
@brianlund: Thanks for the PR! Sorry about the belated response (been swamped with other things), but I think all this looks good and makes sense. I'd just like to do a bit more benchmarking before merging and shipping this (just so I personally have a better sense of where any other bottlenecks might be), but I'm currently tied up with some other things, so it might be a week or two before I can get to it. But I think we definitely want to get this merged in, so thanks again! |
bin/letsencrypt_hooks
Outdated
| local EXPIRY=$(date --date="$(openssl x509 -enddate -noout -in "$CERTFILE"|cut -d= -f 2)" +%s) | ||
| if [ $? -ne 0 ]; then | ||
| echo "failed to get the expiry date." | ||
| exit 1 |
There was a problem hiding this comment.
I am not sure whether this should really be a fatal error.
There was a problem hiding this comment.
I tend to agree as the certificate deployment will work fine even if EXPIRY is not set.
In addition I see that the check would actually never fail. When declaring and setting a local variable in a single command, it's first set and then restricted to local scope, so $? reflects the local declaration, not the variable assignment.
I propose testing if $EXPIRY is empty and if so logging the problem but skip the fatal error. What do you think?
|
Sounds good to me :)
|
EXPIRY is declared seperately to avoid masking return value
|
Many thanks for tracking this down and for the pull request! It's now in the v0.12.0 release. |
Based on the discussion in #110 on high cpu load when attempting renewal on large amounts of certificates. This code limits renewal attempts to the last 30 days of a certificates validity.
When new certificates are generated, the expiry date is extracted from the certificate with openssl and saved to the storage adapter.
When the renewal job is triggered, it attempts to get the expiry date from storage:
I had to adjust the tests in redis.t and file.t as the renewal skipped message is now coming from auto-ssl and not dehydrated. I also moved the "checking certificate renewals for" ngx.NOTICE to the start of the function so it's not only logged when we actually call dehydrated.
The code for extracting and storing the expiry date was taken from the fork at https://github.com/ryokdy/lua-resty-auto-ssl