Merge auto-ssl/lua-resty-auto-ssl #1
Merged
Conversation
This file contains 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
Something seems to have changed and the tests using "lsof" are no longer behaving the same in the alpine container. So switch to debian for testing, which should probably be a bit more stable.
The allow_domain check might be somewhat costly - involving e.g. an external HTTP request to query a central registry. Make it so that this gets only done before requesting new certificates, not for every HTTPS connection.
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.
Default to issuing certs, and return an error message if certs disabled
EXPIRY is declared seperately to avoid masking return value
Generate a new certificate unless ssl_options["generate_certs"] == false
Allow users to configure which db of redis to use
This sets up the JSON encoding/decoding so you can use a configurable library, using cjson as the default, but allowing optional usage of dkjson for a pure-lua library (in environments like ARM where cjson may be more difficult to get setup). We keep cjson as the default (since we assume its bundled with OpenResty), and don't add an explicit dependency for dkjson (since it's not available on OPM, and trying to keep that in mind for future installation). But if you manually add dkjson then the dkjson adapter should work. See auto-ssl#85
The storage's `get_cert` method was not returning the right number of arguments when an error was returned by a storage adapter's `get` method. This could lead to inconsistent error handling, since the returned error was not in the expected place. Fixing this uncovered a difference between between how the redis and file storage adapters were handling not found results. The file adapter was just returning nil with no error, but the redis adapter was returning "not found" as an error. Fix this so that the redis adapter behaves like the file adapter and no longer returns "not found" results as an error.
This eliminates the double "get_cert" call that was happening inside the renewal function and eliminates the possibility of multiple workers fetching the same cert and doing the same expiry checks (the actual renewal was still inside the locks, but I think it might still make sense to include the cert fetch and expiry checks inside the locks too).
This refactors some of the internal functions to return data in a single table instead of as multiple return values. With the recent addition of return both expiry details and error handling to the "get_certs" method, these ended up conflicting since it changed the position of the results. Since returning this many values from a single function was getting a bit unwieldy, it seemed best to refactor things so that all the cert methods return and pass along the cert data as a single table, rather than as positional returns/arguments. This will also make it easier if we add more metadata to the cert data in the future, since the return signature can now remain the same (table, err), rather than needing to worry about the exact position of return values.
…to-ssl into Cargo-cargo-changes
This tweaks the changes made in auto-ssl#107 This shifts the allow_domain callback to happen after the cert lookup in memory, but before the storage lookup. Since storage lookups could also be costly, this tries to strike a middle-ground approach to when allow_domain is performed.
Give each option a header so that we get automatic anchor tags on GitHub so it's easier to then link to individual configuration options in the readme.
auto-ssl#109 If the file storage adapter was being used and the directory contained too many files to perform glob matching on, the file adapter would report no files when checking for renewals. This should fix it by using "find" instead of shell globbing. Any errors should also now be reported.
Fix for our OpenResty 1.11.2 tests, where `nil` options seem to cause issues, but an empty table works as expected.
To account for the addition of the `renewal` argument.
Handle parsing the OpenSSL expiry times ourselves, since the BusyBox and and BSD versions of the "date" command don't support parsing like the GNU version does. auto-ssl#195
The "accounts" directory may or may not exist on initial startup depending on whether cached account config is being used.
Eliminate dependency on GNU version of "date"
All other early returns in renew_check_cert drop this lock. Would warrant a comment if this is not needed here.
Spelling fixes
Unlock the certificate renewal lock when allow_domain returns false
…ocks. These are loaded by default in OpenResty 1.15.8.1+, but this will ensure this library is loaded in older versions. openresty/lua-nginx-module#1207 (comment) auto-ssl#43 auto-ssl#220
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.
No description provided.