Delete local certificate files after dehydrated fails #206
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For each issue that doesn't succeed we're still creating three files (cert-*.csr, cert-*.pem, privkey-*.pem) in dehydrated's cert directory that don't get deleted. While they are not big in size, a seizable number of (sub-) domains can this silently exhaust the number of available inodes. This is a follow-up to 49256f8
GUI
added a commit
that referenced
this pull request
Jul 19, 2020
Building on top of #206, some tweaks: - There was similar logic to cleanup dehydrated files both in the hook file (if the hook succeeded) and in the SSL provider (in case the dehydrated call failed). Since this means the files are deleted in either case (success or failure), try to simplify this by shifting the logic to always run in the SSL provider regardless of success status. - For the new delete logic introduced in the SSL provider, switch it to use non-blocking shell execution to prevent the `rm` from potentially blocking (even though this should be quick). - Since dehydrated's files are always being removed now, remove the logic we had in place to handle an edge-case of storage failing, but files remaining in dehydrated's cache, since this scenario is now moot. - Update tests to account for new delete logic..
|
@gohai: Many thanks, as always (and as always sorry for the delays too)! I've tweaked some of this in aa018af before merging. The commit message attempts to explain the changes I made, but let me know if you have any feedback regarding the tweaks I made:
|
|
Thank you @GUI for merging and improving this patch! Your changes look much cleaner to me, we'll pick them up next time we'll rebase our tree. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For each issue that doesn't succeed we're still creating three files (cert-.csr, cert-.pem, privkey-*.pem) in dehydrated's cert directory that don't get deleted. While they are not big in size, a seizable number of (sub-) domains can this silently exhaust the number of available inodes.
Without this, or a similar patch to the same end, a single certificate that fails to renew (e.g. because the domain has been moved elsewhere) would add 90 files (three per day), which aren't ever deleted. This is compounded by having each subdomain its own certificate.
This is a follow-up to 49256f8