From 7a6d8e9f30cb177b4b2576bd633b96440c99a629 Mon Sep 17 00:00:00 2001 From: Gus Cairo Date: Fri, 17 Jan 2025 10:24:15 +0000 Subject: [PATCH 1/3] Adopt `ServerContext` changes --- .../Internal/Channel+AddressInfo.swift | 70 +++++++------------ .../Server/CommonHTTP2ServerTransport.swift | 16 +++-- .../ControlService.swift | 4 +- 3 files changed, 39 insertions(+), 51 deletions(-) diff --git a/Sources/GRPCNIOTransportCore/Internal/Channel+AddressInfo.swift b/Sources/GRPCNIOTransportCore/Internal/Channel+AddressInfo.swift index d2f04c3..7a94f96 100644 --- a/Sources/GRPCNIOTransportCore/Internal/Channel+AddressInfo.swift +++ b/Sources/GRPCNIOTransportCore/Internal/Channel+AddressInfo.swift @@ -18,67 +18,47 @@ internal import NIOCore extension NIOAsyncChannel { var remoteAddressInfo: String { - guard let remote = self.channel.remoteAddress else { - return "" - } - - switch remote { - case .v4(let address): - // '!' is safe, v4 always has a port. - return "ipv4:\(address.host):\(remote.port!)" - - case .v6(let address): - // '!' is safe, v6 always has a port. - return "ipv6:[\(address.host)]:\(remote.port!)" - - case .unixDomainSocket: - // '!' is safe, UDS always has a path. - if remote.pathname!.isEmpty { - guard let local = self.channel.localAddress else { - return "unix:" - } - - switch local { - case .unixDomainSocket: - // '!' is safe, UDS always has a path. - return "unix:\(local.pathname!)" - - case .v4, .v6: - // Remote address is UDS but local isn't. This shouldn't ever happen. - return "unix:" - } - } else { - // '!' is safe, UDS always has a path. - return "unix:\(remote.pathname!)" - } - } + getAddressInfoWithFallbackIfUDS( + address: self.channel.remoteAddress, + udsFallback: self.channel.localAddress + ) } var localAddressInfo: String { - guard let local = self.channel.localAddress else { + getAddressInfoWithFallbackIfUDS( + address: self.channel.localAddress, + udsFallback: self.channel.remoteAddress + ) + } + + private func getAddressInfoWithFallbackIfUDS( + address: NIOCore.SocketAddress?, + udsFallback: NIOCore.SocketAddress? + ) -> String { + guard let address else { return "" } - switch local { - case .v4(let address): + switch address { + case .v4(let ipv4Address): // '!' is safe, v4 always has a port. - return "ipv4:\(address.host):\(local.port!)" + return "ipv4:\(ipv4Address.host):\(address.port!)" - case .v6(let address): + case .v6(let ipv6Address): // '!' is safe, v6 always has a port. - return "ipv6:[\(address.host)]:\(local.port!)" + return "ipv6:[\(ipv6Address.host)]:\(address.port!)" case .unixDomainSocket: // '!' is safe, UDS always has a path. - if local.pathname!.isEmpty { - guard let remote = self.channel.remoteAddress else { + if address.pathname!.isEmpty { + guard let udsFallback else { return "unix:" } - switch remote { + switch udsFallback { case .unixDomainSocket: // '!' is safe, UDS always has a path. - return "unix:\(remote.pathname!)" + return "unix:\(udsFallback.pathname!)" case .v4, .v6: // Remote address is UDS but local isn't. This shouldn't ever happen. @@ -86,7 +66,7 @@ extension NIOAsyncChannel { } } else { // '!' is safe, UDS always has a path. - return "unix:\(local.pathname!)" + return "unix:\(address.pathname!)" } } } diff --git a/Sources/GRPCNIOTransportCore/Server/CommonHTTP2ServerTransport.swift b/Sources/GRPCNIOTransportCore/Server/CommonHTTP2ServerTransport.swift index 46c0eff..b0fd652 100644 --- a/Sources/GRPCNIOTransportCore/Server/CommonHTTP2ServerTransport.swift +++ b/Sources/GRPCNIOTransportCore/Server/CommonHTTP2ServerTransport.swift @@ -199,7 +199,8 @@ package final class CommonHTTP2ServerTransport< _ context: ServerContext ) async -> Void ) async throws { - let peer = connection.remoteAddressInfo + let remotePeer = connection.remoteAddressInfo + let localPeer = connection.localAddressInfo try await connection.executeThenClose { inbound, _ in await withDiscardingTaskGroup { group in group.addTask { @@ -218,7 +219,8 @@ package final class CommonHTTP2ServerTransport< stream, handler: streamHandler, descriptor: descriptor, - peer: peer + remotePeer: remotePeer, + localPeer: localPeer ) } } @@ -236,7 +238,8 @@ package final class CommonHTTP2ServerTransport< _ context: ServerContext ) async -> Void, descriptor: EventLoopFuture, - peer: String + remotePeer: String, + localPeer: String ) async { // It's okay to ignore these errors: // - If we get an error because the http2Stream failed to close, then there's nothing we can do @@ -274,7 +277,12 @@ package final class CommonHTTP2ServerTransport< ) ) - let context = ServerContext(descriptor: descriptor, peer: peer, cancellation: handle) + let context = ServerContext( + descriptor: descriptor, + remotePeer: remotePeer, + localPeer: localPeer, + cancellation: handle + ) await streamHandler(rpcStream, context) } } diff --git a/Tests/GRPCNIOTransportHTTP2Tests/ControlService.swift b/Tests/GRPCNIOTransportHTTP2Tests/ControlService.swift index c73aa1d..c7c123c 100644 --- a/Tests/GRPCNIOTransportHTTP2Tests/ControlService.swift +++ b/Tests/GRPCNIOTransportHTTP2Tests/ControlService.swift @@ -113,11 +113,11 @@ extension ControlService { } private func serverRemotePeerInfo(context: ServerContext) -> String { - context.peer + context.remotePeer } private func serverLocalPeerInfo(context: ServerContext) -> String { - "" + context.localPeer } private func clientRemotePeerInfo(request: StreamingServerRequest) -> String { From ef1a83a225bd91ec3e4cde26fb54d44c14a61b25 Mon Sep 17 00:00:00 2001 From: Gus Cairo Date: Fri, 17 Jan 2025 10:29:32 +0000 Subject: [PATCH 2/3] PR comments --- .../GRPCNIOTransportCore/Internal/Channel+AddressInfo.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/GRPCNIOTransportCore/Internal/Channel+AddressInfo.swift b/Sources/GRPCNIOTransportCore/Internal/Channel+AddressInfo.swift index 7a94f96..d7ca5d2 100644 --- a/Sources/GRPCNIOTransportCore/Internal/Channel+AddressInfo.swift +++ b/Sources/GRPCNIOTransportCore/Internal/Channel+AddressInfo.swift @@ -18,14 +18,14 @@ internal import NIOCore extension NIOAsyncChannel { var remoteAddressInfo: String { - getAddressInfoWithFallbackIfUDS( + self.getAddressInfoWithFallbackIfUDS( address: self.channel.remoteAddress, udsFallback: self.channel.localAddress ) } var localAddressInfo: String { - getAddressInfoWithFallbackIfUDS( + self.getAddressInfoWithFallbackIfUDS( address: self.channel.localAddress, udsFallback: self.channel.remoteAddress ) From b258767d3a45b0b4eceff05b592942b8cbc6b8bb Mon Sep 17 00:00:00 2001 From: Gus Cairo Date: Fri, 17 Jan 2025 10:46:39 +0000 Subject: [PATCH 3/3] Update tests --- .../HTTP2TransportTests.swift | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/Tests/GRPCNIOTransportHTTP2Tests/HTTP2TransportTests.swift b/Tests/GRPCNIOTransportHTTP2Tests/HTTP2TransportTests.swift index 142b27e..0d45776 100644 --- a/Tests/GRPCNIOTransportHTTP2Tests/HTTP2TransportTests.swift +++ b/Tests/GRPCNIOTransportHTTP2Tests/HTTP2TransportTests.swift @@ -1636,13 +1636,11 @@ final class HTTP2TransportTests: XCTestCase { let serverRemotePeerMatches = peerInfo.server.remote.wholeMatch(of: /ipv4:127\.0\.0\.1:(\d+)/) let clientPort = try XCTUnwrap(serverRemotePeerMatches).1 - // TODO: Uncomment when server local peer info is implemented + let serverLocalPeerMatches = peerInfo.server.local.wholeMatch(of: /ipv4:127.0.0.1:(\d+)/) + let serverPort = try XCTUnwrap(serverLocalPeerMatches).1 - // let serverLocalPeerMatches = peerInfo.server.local.wholeMatch(of: //) - // let serverPort = XCTUnwrap(serverLocalPeerMatches).1 - - // let clientRemotePeerMatches = peerInfo.client.remote.wholeMatch(of: /ipv4:127.0.0.1:(\d+)/) - // XCTAssertEqual(try XCTUnwrap(clientRemotePeerMatches).1, serverPort) + let clientRemotePeerMatches = peerInfo.client.remote.wholeMatch(of: /ipv4:127.0.0.1:(\d+)/) + XCTAssertEqual(try XCTUnwrap(clientRemotePeerMatches).1, serverPort) let clientLocalPeerMatches = peerInfo.client.local.wholeMatch(of: /ipv4:127\.0\.0\.1:(\d+)/) XCTAssertEqual(try XCTUnwrap(clientLocalPeerMatches).1, clientPort) @@ -1658,13 +1656,11 @@ final class HTTP2TransportTests: XCTestCase { let serverRemotePeerMatches = peerInfo.server.remote.wholeMatch(of: /ipv6:\[::1\]:(\d+)/) let clientPort = try XCTUnwrap(serverRemotePeerMatches).1 - // TODO: Uncomment when server local peer info is implemented - - // let serverLocalPeerMatches = peerInfo.server.local.wholeMatch(of: //) - // let serverPort = XCTUnwrap(serverLocalPeerMatches).1 + let serverLocalPeerMatches = peerInfo.server.local.wholeMatch(of: /ipv6:\[::1\]:(\d+)/) + let serverPort = try XCTUnwrap(serverLocalPeerMatches).1 - // let clientRemotePeerMatches = peerInfo.client.remote.wholeMatch(of: /ipv6:\[::1\]:(\d+)/) - // XCTAssertEqual(try XCTUnwrap(clientRemotePeerMatches).1, serverPort) + let clientRemotePeerMatches = peerInfo.client.remote.wholeMatch(of: /ipv6:\[::1\]:(\d+)/) + XCTAssertEqual(try XCTUnwrap(clientRemotePeerMatches).1, serverPort) let clientLocalPeerMatches = peerInfo.client.local.wholeMatch(of: /ipv6:\[::1\]:(\d+)/) XCTAssertEqual(try XCTUnwrap(clientLocalPeerMatches).1, clientPort) @@ -1679,7 +1675,7 @@ final class HTTP2TransportTests: XCTestCase { let peerInfo = try await control.peerInfo() XCTAssertNotNil(peerInfo.server.remote.wholeMatch(of: /unix:peer-info-uds/)) - XCTAssertNotNil(peerInfo.server.local.wholeMatch(of: //)) + XCTAssertNotNil(peerInfo.server.local.wholeMatch(of: /unix:peer-info-uds/)) XCTAssertNotNil(peerInfo.client.remote.wholeMatch(of: /unix:peer-info-uds/)) XCTAssertNotNil(peerInfo.client.local.wholeMatch(of: /unix:peer-info-uds/))