Skip to content

feat(relay)!: remove built-in ACME - #7803

Merged
kixelated merged 7 commits into
mainfrom
quest/m1/rind/acme-remove
Nov 4, 2026
Merged

kixelated merged 7 commits into
mainfrom
quest/m1/rind/acme-remove

Conversation

@kixelated

@kixelated kixelated commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

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

  • Remove the ACME client, startup issuance, renewal, cache, TLS-ALPN-01 handling, configuration, and tests. Drop rustls-acme and the relay dependencies only ACME used; regenerate the lockfile and workspace hack.
  • Remove the NixOS ACME options and environment wiring. Production docs and installed/demo configs use externally managed PEM files and retain certificate reload.
  • Delete quest/m1/rind/acme-remove.md and its links. TLS provider migration remains in transport on rind.

Impact

  • Breaking: remove the acme Cargo feature, moq_relay::acme::Config, Config::acme, [acme], --acme-*, and MOQ_ACME_*. Remove them from deployments and have an external certificate manager write the PEM files configured by listen.tls.cert / key and web.https.cert / key. Removed configuration is refused. Development can still use listen.tls.generate. Migration notes are under ## Unreleased in MIGRATE.md.
  • Breaking: remove services.moq-relay.acme.*. Use tls.certs, or tls.generate for development; configure an HTTPS listener through the service environment when needed. The migration also covers its low-port capability.
  • No MoQ wire or TLS provider change.

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.
  • The flake check fails building the runner image with unshare: write failed /proc/self/uid_map: Operation not permitted. nix develop --command quest gate nix flake check --print-build-logs path:./.tmp/base reproduces that error and the identical runner derivation on an unmodified archive of main at acef90cf0425d9f0e4e699759cede07adfb2ecfb. No check or harness was disabled.
  • cargo check --locked --package moq-relay --no-default-features passes in the Nix shell through quest gate and sh/local.sh.
  • A fresh relay binary's help exposes file/generated TLS and no ACME. All six removed flags and environment variables, including the old dir alias, and the [acme] table are refused.
  • NixOS evaluation passes with generated TLS and file certificates; ACME options and emitted variables are absent. Cargo metadata and the relay dependency tree contain no ACME feature or rustls-acme dependency.
  • quest check --base origin/main and git diff --check pass. 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

  • Existing relay ACME and rind adapter quests own reintroduction.
  • Recommend a separate infrastructure quest for the runner image's Nix builder user-namespace denial if the host remains affected. It reproduces on main and is outside ACME removal.

(Written by GPT-6.1 Sol)

@kixelated kixelated changed the title moq-relay drops ACME for now feat(relay)!: remove built-in ACME Nov 4, 2026
kixelated and others added 6 commits November 3, 2026 19:48
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>
@kixelated
kixelated marked this pull request as ready for review November 4, 2026 04:07
@coderabbitai

coderabbitai Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You'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.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 25d8c81c-6638-41ce-a979-8bd604a95301
Commits

Reviewing files that changed from the base of the PR and between 90081ff and b1e022f.

Files ignored due to path filters: 1
  • Cargo.lock is excluded by !**/*.lock
Files selected for processing: 25
  • .oss-scanner/threat_model.md
  • demo/relay/prod.toml
  • doc/bin/relay/config.md
  • doc/bin/relay/index.md
  • doc/setup/prod.md
  • nix/modules/moq-relay.nix
  • packaging/moq-relay/relay.toml
  • quest/m1/poll-first/no-spawn/README.md
  • quest/m1/rind/README.md
  • quest/m1/rind/acme-remove.md
  • quest/m1/rind/transport.md
  • quest/m2/audit-test-cost.md
  • rs/moq-relay/Cargo.toml
  • rs/moq-relay/MIGRATE.md
  • rs/moq-relay/src/acme.rs
  • rs/moq-relay/src/acme/client.rs
  • rs/moq-relay/src/acme/tests.rs
  • rs/moq-relay/src/cluster.rs
  • rs/moq-relay/src/config.rs
  • rs/moq-relay/src/lib.rs
  • rs/moq-relay/src/relay.rs
  • rs/moq-relay/src/settings.rs
  • rs/moq-relay/src/web.rs
  • rs/moq-tokio/src/tls.rs
  • rs/workspace-hack/Cargo.toml
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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))

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

  • ACME client removed: rs/moq-relay/src/acme.rs, acme/client.rs, and acme/tests.rs are deleted along with the acme feature, Config::acme, Relay.acme, Web::with_acme/https_listener, build_acme_config, and the acme-tls/1 ALPN close.
  • Startup path simplified: relay.rs keeps the non-ACME behavior exactly (served from workers, signal handlers installed after the last failing step); only the issuance wait and critical renewal task go.
  • HTTPS reload always runs: web.rs now spawns reload_https_config unconditionally, which is what non-ACME builds already did.
  • Config refusal preserved: removing the Acme settings struct and CLI fields means deny_unknown_fields, unknown-flag errors, and check_env reject removed [acme], --acme-*, and MOQ_ACME_* instead of silently ignoring them.
  • Dependencies: rustls-acme, async-trait, humantime, x509-parser, and time leave moq-relay; lockfile and workspace hack are regenerated.
  • NixOS module: services.moq-relay.acme.*, its assertion, network-online ordering, ACME env emission, and the ACME AmbientCapabilities branch are removed.
  • Docs and configs: production setup now uses externally managed PEMs for listen.tls and web.https; config.md, index.md, prod.toml, relay.toml, and the threat model are aligned, and MIGRATE.md documents the Rust, TOML/env, and NixOS migrations.
  • Quests: the completed acme-remove quest 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.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

@kixelated

Copy link
Copy Markdown
Collaborator Author

Heads-up from moq-dev/moq#7805, which deletes quest/m1/poll-first/no-spawn/README.md and moves its list of spawns that stay into quest/m2/spawn-audit.md. If #7805 lands first, this PR gets a modify/delete conflict on that README. Resolve it by keeping the deletion and dropping the `Critical`'s ACME renewal (`rs/moq-relay/src/relay.rs`) entry from quest/m2/spawn-audit.md instead. If this PR lands first, #7805 does the same when it merges main.

(Written by Claude Opus 5.5)

@kixelated
kixelated added this pull request to the merge queue Nov 4, 2026
Merged via the queue into main with commit 066b7cb Nov 4, 2026
11 checks passed
@kixelated
kixelated deleted the quest/m1/rind/acme-remove branch November 4, 2026 06:51
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.

1 participant