-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
Update to hyper dependency to version 1 #50
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! I'm afraid this will need some effort to get to a mergeable state.
@djc thanks for the review! Just wondering there's a failed audit check due to license of a subdependency: https://github.com/instant-labs/instant-acme/actions/runs/9630966408/job/26563174365#step:4:92 This is due to the enabled What would you like to do about this? We could switch to a fallible replacement for |
Hmm, we should probably start using the rustls-platform-verifier crate instead. |
It's okay to bump the MSRV to what we need here. If you rebase on top of #52 that should fix the audit issue. |
I've merged in that change (anticipating this MR will be merged in squash anyway?) If the license check with webpki-roots included now passes due to these changes, can we consider this #50 (comment) resolved too, without needing to use rustls-platform-verifier yet? Perhaps can be done in next MR? |
No, I don't want to merge a dependency on webpki-roots. Instead you can make the |
@djc thanks again! I think I may have addressed all the outstanding comments? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good, a few more small things to address.
Can you also bump the version to 0.6.0?
Also need to address the CI issues:
|
@djc done! |
Thanks for your patience working through my feedback!
|
@djc and @kellpossible, just wanted to thank you both for getting this done. We are in the middle of building management features for around 10,000 certificates, and being able to work with great maintainers and developers is so crucial. Thank you again! |
Always curious to get a bit more context about how/where you're using instant-acme! |
@djc I can give a brief overview here, and if you'd like more details, reach out to me at appcove. We have several clients who operate saas type systems for thousands and thousands of businesses. These business oftentimes bring their own domains, or pick a subdomain, and some of them generated a multitude of new sites on a daily basis. There are some great tools out there for automatic TLS issuance, like the caddy webserver. The issue with such tools is that while they fully automate the process, they provide little central insight or control into how it's going. Also, we do not like the idea of giving each web-server potentially destructive DNS API access in order to respond to DNS challenges. This calls for something more centralized. We are building a central infrastructure management solution with TLS certificate audits/issuance/distribution being one aspect. It will control when and how certificates are renewed, be able to audit that, report on issues, and so on and so forth. Then on the webserver side, we are working to ensure that the ALPN verification challenges can be successfully answered in real-time by communicating with the central management system. This is the area that instant-acme helps us -- to be the workhorse of issuance and renewals across a number of providers, both with DNS and ALPN, and to provide strongly typed rust APIs for accessing both the challenge information and the issued certificates, so they can both be appropriately distributed to the respective web server clusters. That's it in a nutshell. We appreciate your efforts to make this possible. |
No description provided.