Skip to content
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

Fix #493: Expose ACME structures for user-managed certificate renewal #494

Merged
merged 6 commits into from
Jun 22, 2023

Conversation

andrewbaxter
Copy link
Contributor

@andrewbaxter andrewbaxter commented Jan 29, 2023

Sorry for plowing ahead with this, but I was blocked on it in another project and wanted to make some progress.

Basically this exposes issue_cert along with the surrounding necessary structures, methods, etc. I made the following big changes:

  • issue_cert was made public and its responsibilities were restricted to only completing the ACME procedure. Using the certificate is left to the caller (no serializing, no updating the resolver, etc).
  • A new simplified (no caching, no spawned TLS update loop) 443 listener was added
  • The Http01 port 80 listener was made public

The following shared state structures were also made public

  • AcmeClient - used by issue_cert but may be shared across invocations
  • ResolveServerCert - used by issue_cert, issue_cert caller, and 443 listener
  • http01 key map - used by issue_cert, 80 listener

And some other helper functionality was exposed, methods for avoiding transitive deps, etc.

There's an example showing the usage (just showing the parallel to the fully poem-managed ACME http01 process). I believe it's minimal: there's not a lot of boilerplate and exposed functions and objects have clear uses, etc.

I'm happy to rename/reorg things, improve comments if things are unclear, or if there are other changes needed.

@andrewbaxter
Copy link
Contributor Author

key_pair was only used by issue_cert and was already accessible via the AcmeClient so I changed issue_cert to access it that way. Otherwise more internal structures would need to be exposed and passed through multiple layers, and I couldn't think of other ways that this would be needed.

@sunli829
Copy link
Collaborator

I’m so sorry for taking so long to handle this PR because I’ve been busy with other things. 🙂

@sunli829 sunli829 merged commit 5d8048c into poem-web:master Jun 22, 2023
5 of 7 checks passed
@andrewbaxter
Copy link
Contributor Author

Oh wow, thanks! No worries, and I'm glad you thought it was reasonable!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants