Generate a new certificate unless ssl_options["generate_certs"] == false #92
Conversation
|
This is in response to #91 |
|
@mklauber: Thanks for the pull request, but apologies for the delayed response! I've been tied up with other things, but I'll try to take a look at this more closely in the coming weeks and get this released. Thanks again. |
|
@mklauber this change looks very useful, thank you 👍 🙂 |
There was a problem hiding this comment.
Would you mind adding a test so we can easily ensure that it still works in the future?
Also, documentation in the README would be great, so people can actually use this without reading the code.
Thank you!
| fullchain_pem, privkey_pem = issue_cert(auto_ssl_instance, storage, domain) | ||
| if fullchain_pem and privkey_pem then | ||
| return convert_to_der_and_cache(domain, fullchain_pem, privkey_pem, true) | ||
| if ssl_options and ssl_options["generate_certs"] ~= false then |
There was a problem hiding this comment.
As far as I can tell, this defaults to not issuing any certificates when ssl_options is not there or the value is not specified. When users update to a version containing this change, their setup will stop working for new domains.
As to not break existing setups without need, I suggest to change the default for this to true.
| fullchain_pem, privkey_pem = issue_cert(auto_ssl_instance, storage, domain) | ||
| if fullchain_pem and privkey_pem then | ||
| return convert_to_der_and_cache(domain, fullchain_pem, privkey_pem, true) | ||
| end | ||
| end |
There was a problem hiding this comment.
get_cert returns an error message as the very last parameter. Please add a appropriate message telling the users what is going on, in case certificates are not being generated because of this setting:
return nil, nil, nil, "did not issue certificate, because the generate_certs setting is false"
|
@luto I'll make the defaults, error message and documentation changes. I actually don't really know lua, and basically just hacked this together via google till it worked for my needs, so I don't actually know how to write tests. Is there a tutorial for your test framework or something I can learn from somewhere? |
Default to issuing certs, and return an error message if certs disabled
Sometimes its beneficial to be able to serve requests using the letsencrypt certificates without also generating new ones. This allows a new
ssl_optionsoptiongenerate_certsthat, iffalse, results in resty-auto-ssl using certs from memory or the storage, but not requesting new ones.