-
Notifications
You must be signed in to change notification settings - Fork 345
feat(iroh-relay): Embeddable relay server #3832
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
base: main
Are you sure you want to change the base?
Conversation
This commit makes the following changes to enable embedding the iroh relay server directly into other HTTP servers: 1. Made RelayService public and exposed it from the server module 2. Made RelayService::new(), shutdown(), and handle_connection() public 3. Made Handlers public for constructing RelayService 4. Exported KeyCache publicly instead of pub(crate) These changes allow applications to: - Create a RelayService instance without spawning a full HTTP server - Integrate the relay service into existing axum/hyper-based HTTP servers - Use the relay service as a fallback handler for unmatched routes Breaking changes: None - only exposes previously private APIs Based on stable release v0.95.1
- Add axum as optional dependency - Create axum_integration module with relay_handler - Implement AxumWebSocketAdapter for Stream/Sink traits - Note: Full integration requires refactoring internal protocol code to be generic over WebSocket type
- Made RelayedStream generic over stream types - Updated Client and Actor to be generic - Made Clients::register() generic - Completed AxumWebSocketAdapter implementation - Implemented full relay protocol in handle_relay_websocket
- Wrap AccessConfig in Arc since it can't be cloned - Use Clients::default() instead of non-existent new() method - Fix error type conversions from axum::Error to std::io::Error - Fix AxumMessage::Binary to use Bytes directly instead of Vec<u8>
- Properly match on Poll results to convert error types - This fixes type mismatches between axum::Error and std::io::Error
StreamError is tokio_websockets::Error, not std::io::Error. Wrap std::io::Error in WsError::Io variant for proper type conversion.
matheus23
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hoping we could get away with exposing some functions in iroh-relay required for making an integration with axum.
I feel strongly about not adding the axum dependency (even if it's optional).
I'm unsure about abstracting away the WebSocketStream inside RelayedStream. I think this can be achieved with axum::extract::ConnectInfo. If that ends up working, you even get to re-use more iroh-relay code.
Also adds comments to now public types. All types needed to run the exact same axum_integration code in an external crate.
Resolve Cargo.lock conflicts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one general change, and then I'd say this is good to go.
I talked to the team, and we generally want to make it possible for users of iroh-relay to be embedded into axum or other webservers! :)
| S: Stream<Item = Result<bytes::Bytes, crate::protos::streams::StreamError>> | ||
| + Sink<bytes::Bytes, Error = crate::protos::streams::StreamError> | ||
| + Unpin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have BytesStreamSink in iroh-relay/src/proto/streams.rs. We should re-use this here.
| S: Stream<Item = Result<bytes::Bytes, crate::protos::streams::StreamError>> | ||
| + Sink<bytes::Bytes, Error = crate::protos::streams::StreamError> | ||
| + Unpin, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use BytesStreamSink here, too.
|
There's also unfortunately more things that need to be made To fix the formatting CI check, run |
|
The Wasm CI check should be fixed by updating to the |
|
The netsim integration tests might currently fail, due to reasons unrelated to this PR IIRC. I'll keep an eye on this once you addressed the other CI issues. |
|
Great 🙂 I'll try to address the failing tests shortly |
|
Ok, everything is passing now except for the netsim integration tests. |
|
Thank you for addressing CI. There are still two review comments from myself that you've missed @lucksus |
Description
This PR renders the iroh-relay server embeddable as a library by making
RelayServicepublicand adding a native axum integration around it, enabling the relay service to be integrated directly into axum applications as a standard route handler without requiring connection-level routing, separate ports, or reverse proxies.Adds docs, examples and tests for all new and/or now public functions/structs.
Key changes:
Added new axum feature and server::axum_integration moduleImplemented AxumWebSocketAdapter that bridges axum's WebSocket to the relay's Stream/Sink traitsCreated RelayState and relay_handler for easy integration into axum routersFull relay protocol support including handshake, authentication, authorization, and client registrationBreaking Changes
None. All changes are internal refactoring or new additions. The generic implementations are backward compatible with existing relay server usage through type aliases and preserved public APIs.
Notes & open questions
Rate limiting: Client-side rate limiting configuration is accepted but not fully enforced yet in the axum integration (similar limitation exists in the existing HTTP server integration)TLS key export: AxumWebSocketAdapter implements ExportKeyingMaterial but returns None since axum's WebSocket doesn't support TLS key material export. This matches the behavior of non-TLS connections in the existing relay server.Error handling: Axum WebSocket errors are wrapped in tokio_websockets::Error::Io variant for consistency with the relay's error handlingTesting: Integration tests verify both non-TLS and TLS scenarios with full end-to-end relay communicationCould We Reduce This?
We could potentially create a single public "escape hatch" function:
This would hide all the intermediate types and just expose one function. However, this would:
Change checklist