Actually enable the proper rustls features. #572
Conversation
Technically the issue is in hyper_serve, but I'd rather just fix moq-native.
WalkthroughVersion bumps for three Rust crates. Added a new features section in rs/moq-native/Cargo.toml to toggle TLS backends with a default. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rs/moq-native/Cargo.toml (1)
21-31: Ensure no residualringpull-ins underaws-lc-rs
Cargo.toml still definesring = "0.17"(line 30) and the rustlsringfeature (features ring = ["rustls/ring"] at lines 15–17), and depswebpki = "0.22"(line 42) andrcgen = "0.14"(line 28) which transitively pull inring. Remove or replacewebpki/rcgen(e.g., switch to rustls-native-certs or rustls-pki-types) to fully eliminateringlinkage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
rs/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
rs/hang-cli/Cargo.toml(1 hunks)rs/moq-native/Cargo.toml(1 hunks)rs/moq-relay/Cargo.toml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
rs/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place Rust crates under the rs/ directory
Files:
rs/hang-cli/Cargo.tomlrs/moq-relay/Cargo.tomlrs/moq-native/Cargo.toml
rs/moq-relay/**
📄 CodeRabbit inference engine (CLAUDE.md)
The CDN/relay (rs/moq-relay) must not include application logic, media codecs, or track details
Files:
rs/moq-relay/Cargo.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check
🔇 Additional comments (3)
rs/hang-cli/Cargo.toml (1)
8-8: Patch bump looks correct.No API surface here; patch bump to 0.2.8 is appropriate.
rs/moq-relay/Cargo.toml (1)
8-8: Patch bump is fine.No manifest or API changes beyond version; 0.9.1 makes sense.
rs/moq-native/Cargo.toml (1)
8-8: Patch bump to 0.8.1 is appropriate.Matches feature tweak scope; no semver concerns.
| [features] | ||
| default = ["aws-lc-rs"] | ||
| aws-lc-rs = ["rustls/aws-lc-rs"] | ||
| ring = ["rustls/ring"] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Propagate backend choice to rcgen and make backends mutually exclusive at the crate boundary.
Right now the rustls flags are wired, but rcgen defaults to ring and you also depend on ring unconditionally, so aws-lc-rs won’t meaningfully remove ring from the build. Map features through to rcgen and gate ring as optional.
Apply this diff to the features block:
[features]
-default = ["aws-lc-rs"]
-aws-lc-rs = ["rustls/aws-lc-rs"]
-ring = ["rustls/ring"]
+default = ["aws-lc-rs"]
+# Select exactly one crypto backend for rustls and rcgen.
+aws-lc-rs = ["rustls/aws-lc-rs", "rcgen/aws_lc_rs"]
+ring = ["rustls/ring", "rcgen/ring"]And update dependencies (outside this hunk) to avoid always pulling ring:
# Replace these in [dependencies]
# - rcgen = "0.14"
# - ring = "0.17"
rcgen = { version = "0.14", default-features = false }
ring = { version = "0.17", optional = true }Optionally add a compile guard in lib.rs to prevent both features at once:
#[cfg(all(feature = "aws-lc-rs", feature = "ring"))]
compile_error!("Enable only one of: `aws-lc-rs` or `ring`.");🤖 Prompt for AI Agents
In rs/moq-native/Cargo.toml around lines 14-17, the features need to propagate
the chosen tls backend to rcgen and make ring optional so backends are mutually
exclusive: add feature mappings so rcgen uses the same backend feature (e.g.,
aws-lc-rs = ["rustls/aws-lc-rs", "rcgen/aws-lc-rs"] and ring = ["rustls/ring",
"rcgen/ring"]) and mark ring as a crate-level mutually exclusive backend (remove
unconditional ring dependency). Also update the [dependencies] section (outside
this hunk) to set rcgen = { version = "0.14", default-features = false } and
ring = { version = "0.17", optional = true }, and add an optional compile-time
guard in lib.rs to error if both features are enabled simultaneously.
Technically the issue is in hyper_serve, but I'd rather just fix moq-native.
Summary by CodeRabbit
New Features
Chores