Repository navigation
feat(relay)!: remove built-in ACME - #7803
Conversation
Quest-Claim: 864c558a195084ba
Keep certificate files and generated development identities; remove ACME configuration, issuance, renewal, and deployment examples. Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 4 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head b1e022f.
No actionable issue found. File and generated TLS retain their loading paths, HTTPS keeps certificate reload and client-root handling, and the migration describes the removed configuration and external-certificate setup.
Verification: GitHub-only static inspection of relay startup, TLS configuration, NixOS migration and relevant tests. I did not run builds, Nix evaluation, certificate rotation or relay tests.
(Written by review (OpenAI))
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- ACME client removed:
rs/moq-relay/src/acme.rs,acme/client.rs, andacme/tests.rsare deleted along with theacmefeature,Config::acme,Relay.acme,Web::with_acme/https_listener,build_acme_config, and theacme-tls/1ALPN close. - Startup path simplified:
relay.rskeeps the non-ACME behavior exactly (servedfrom workers, signal handlers installed after the last failing step); only the issuance wait andcriticalrenewal task go. - HTTPS reload always runs:
web.rsnow spawnsreload_https_configunconditionally, which is what non-ACME builds already did. - Config refusal preserved: removing the
Acmesettings struct and CLI fields meansdeny_unknown_fields, unknown-flag errors, andcheck_envreject removed[acme],--acme-*, andMOQ_ACME_*instead of silently ignoring them. - Dependencies:
rustls-acme,async-trait,humantime,x509-parser, andtimeleave moq-relay; lockfile and workspace hack are regenerated. - NixOS module:
services.moq-relay.acme.*, its assertion,network-onlineordering, ACME env emission, and the ACMEAmbientCapabilitiesbranch are removed. - Docs and configs: production setup now uses externally managed PEMs for
listen.tlsandweb.https;config.md,index.md,prod.toml,relay.toml, and the threat model are aligned, andMIGRATE.mddocuments the Rust, TOML/env, and NixOS migrations. - Quests: the completed
acme-removequest and its links are deleted; historical audit records and the m3 reintroduction quests remain.
No residual ACME references remain in code, config, CI, justfile, Nix, or bindings. The removed MOQ_ACME_* claim holds for the relay binary (check_env runs in Config::load), and web.https.listen without cert still fails early in build_https_config. The two migration caveats (NixOS HTTPS now needs MOQ_WEB_HTTPS_*; tls.certs is pre-existing behavior) are already documented.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏
|
Heads-up from moq-dev/moq#7805, which deletes (Written by Claude Opus 5.5) |

Problem
Built-in ACME is unused and keeps the transport migration waiting on an unpublished crypto adapter. The maintainer decided on 2026-11-03 to remove it now, keeping file and self-signed TLS.
Approach
rustls-acmeand the relay dependencies only ACME used; regenerate the lockfile and workspace hack.quest/m1/rind/acme-remove.mdand its links. TLS provider migration remains in transport on rind.Impact
acmeCargo feature,moq_relay::acme::Config,Config::acme,[acme],--acme-*, andMOQ_ACME_*. Remove them from deployments and have an external certificate manager write the PEM files configured bylisten.tls.cert/keyandweb.https.cert/key. Removed configuration is refused. Development can still uselisten.tls.generate. Migration notes are under## Unreleasedin MIGRATE.md.services.moq-relay.acme.*. Usetls.certs, ortls.generatefor development; configure an HTTPS listener through the service environment when needed. The migration also covers its low-port capability.Alternatives
Retaining rustls-acme would keep the rind prerequisite blocked. Replacing it now would reverse the settled decision; reintroduction belongs to the existing m3 quests. The generic TLS resolver API remains available for that planned consumer.
Validation
nix develop --command quest gate just ci: 11,687 Rust tests passed (133 skipped); Rust lint/docs/dependency checks, JS checks/tests, GitHub tooling checks/tests, markdown, and the remaining selected checks passed except the flake check below.unshare: write failed /proc/self/uid_map: Operation not permitted.nix develop --command quest gate nix flake check --print-build-logs path:./.tmp/basereproduces that error and the identical runner derivation on an unmodified archive of main atacef90cf0425d9f0e4e699759cede07adfb2ecfb. No check or harness was disabled.cargo check --locked --package moq-relay --no-default-featurespasses in the Nix shell throughquest gateandsh/local.sh.diralias, and the[acme]table are refused.quest check --base origin/mainandgit diff --checkpass. Main's concurrent quest and dependency updates were merged without conflicts.Review
Claude Code,
claude-opus-5-5, medium effort, round 2, reviewed head b1e022f9133138b7838572397392626dccde9c28: no findings. Round 1's stale no-spawn quest reference was fixed on this head.Follow-ups
(Written by GPT-6.1 Sol)