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

Add a new trillium-acme crate to automatically manage HTTPS certificates #377

Merged
merged 1 commit into from
Oct 27, 2023

Conversation

joshtriplett
Copy link
Collaborator

trillium-acme helps you serve HTTPS with Trillium using automatic
certificates, via Let’s Encrypt and ACME tls-alpn-01 challenges.

I've tested this both with the staging and production Let's Encrypt
directories.

acme/src/lib.rs Fixed Show resolved Hide resolved
@joshtriplett
Copy link
Collaborator Author

Test failure looks unrelated (issue with fastrand not returning reproducible data).

@joshtriplett
Copy link
Collaborator Author

Anything further needed for this? Does this seem like a reasonable approach?

@jbr
Copy link
Contributor

jbr commented Oct 16, 2023

As a high level concern before I review this more closely:

I'd prefer this not to be in the monorepo, as I'm not sure that I'm the right person to commit to maintaining this for posterity, as I have no experience using acme in production. The goal for trillium is to foster an ecosystem of crates that are maintained by a community of users with a range of needs and use cases. The goal is not for every trillium crate to live within this monorepo or to be maintained by me, and I likely should be more clear about this on the contributing page.

I very much want trillium-acme to exist, and would like to ensure we find a home for it. In order of preference:

  1. You publish this under your own name — in practice I will PR anything necessary to track breaking changes and will always trillium-acme tests against changes, possibly even in CI.
  2. We publish this in a new trillium-rs repository that you have write access to and primary ownership of, with whatever maintenance expectations set distinctly from trillium itself.
  3. We add this to the current repo — as mentioned above, I'd prefer to avoid this as I don't feel that I have the context or expertise to maintain this crate, and as a process matter think it's healthiest for trillium crates to be owned and maintained by their authors

acme/src/lib.rs Outdated
);
let acme_acceptor = state.acceptor();

let future = async move {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This future is awkward, and possibly an indication that trillium needs a new affordance to spawn tasks from within init. When does this stream receive items? Is it generally sequenced with the acme_acceptor returning None? Is it fully async and unrelated to the request/response cycle?

Copy link
Collaborator Author

@joshtriplett joshtriplett Oct 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fully async, and also handles things like renewal.

I would be happy to hide this detail, but 1) I would need a runtime-independent way to spawn a task, and 2) I would need access to the stopper, which also needs to be runtime-independent.

@joshtriplett
Copy link
Collaborator Author

joshtriplett commented Oct 17, 2023

I'd prefer this not to be in the monorepo

I didn't realize this. I kinda liked the trillium monorepo approach. But that said, my primary concern was CI, and if you're offering to build-test trillium-acme in CI, that sounds great.

I very much want trillium-acme to exist, and would like to ensure we find a home for it. In order of preference:

  1. You publish this under your own name — in practice I will PR anything necessary to track breaking changes and will always trillium-acme tests against changes, possibly even in CI.

  2. We publish this in a new trillium-rs repository that you have write access to and primary ownership of, with whatever maintenance expectations set distinctly from trillium itself.

I have some preference towards a trillium-rs repository, because I would want this to live in an organization anyway, and because I would be happy for you to have push access, not just PR access. (Though I'm still happy to maintain it, and I'm thrilled that you're willing to PR it for changes to trillium.)

In an ideal world, I would love to find a way to test it in CI; that's just a substantial challenge that would require either a CI setup with a domain name pointed to it or a local ACME test server. Should be possible to set something like that up using https://github.com/letsencrypt/pebble .

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ede2897) 46.43% compared to head (39bd116) 46.43%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #377   +/-   ##
=======================================
  Coverage   46.43%   46.43%           
=======================================
  Files         161      161           
  Lines        6375     6375           
=======================================
  Hits         2960     2960           
  Misses       3415     3415           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

`trillium-acme` helps you serve HTTPS with Trillium using automatic
certificates, via Let’s Encrypt and ACME tls-alpn-01 challenges. Mention
it in the TLS documentation and link to the `trillium-acme`
documentation.
@joshtriplett
Copy link
Collaborator Author

joshtriplett commented Oct 27, 2023

@jbr I've now published trillium-acme as a standalone crate, with repository at https://github.com/joshtriplett/trillium-acme . I've invited you as a collaborator.

I managed to implement full CI testing for trillium-acme, using https://github.com/letsencrypt/pebble as an ACME server.

I've rewritten this PR into one that just adds a mention of trillium-acme to the documentation.

@jbr
Copy link
Contributor

jbr commented Oct 27, 2023

Thank you for this! I wasn't waiting you out, but was hoping to land an ability to spawn tasks from inside handlers before merging this. Sorry I didn't communicate that. We'll get that in for trillium-acme 0.2!

@jbr jbr merged commit 82510da into trillium-rs:main Oct 27, 2023
19 checks passed
@joshtriplett joshtriplett deleted the acme branch October 28, 2023 02:58
@joshtriplett
Copy link
Collaborator Author

@jbr No problem. I look forward to coordinating further on it. I'd be thrilled if it ends up being either in the trillium-rs organization or in this repo, but I also don't mind maintaining it as a standalone crate. And I do appreciate the idea of integrating the spawning of the future, as well, which will make it much easier to use.

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