Wrap channel in an asyncio.Transport to eliminate loop back connection

[bdraco]

Breaking change

SniTunClientAioHttp.start() no longer uses endpoint_connection_error_callback. Passing it will now generate a DeprecationWarning that it will be removed in a future release. This failure condition is no longer possible since there is no longer a loop back connection, and the protocol is connected directly to the aiohttp RequestHandler (server).

hass_nabucasa currently uses this callback in https://github.com/NabuCasa/hass-nabucasa/blob/cb14a2cad747b882f407e72e69f539f5d8fa20bb/hass_nabucasa/remote.py#L321

Its unclear if hass_nabucasa can simply remove passing the callback or another change will be needed.

Technically Breaking change

The signature of snitun.client.connector.Connector has changed since it no longer connects to loop-back. Instead of end_host and end_port it takes the ssl_context and protocol_factory (an asyncio.Protocol factory, in Home Assistant's case this is an aiohttp RequestHandler)

This doesn't appear to be used externally as SniTunClientAioHttp is the entry point that creates these.

Deployment considerations

The new ChannelTransport is hard coded to pass the remote IP as 127.0.0.1 for compatibility with the currently deployed Cloud servers.

https://github.com/NabuCasa/snitun/pull/303/files#diff-962be76744351102b2907e3aab99d5a45bc091cac1cc65f53f0fe2db9033c47fR30 should be adjusted to True when channel.ip_address is the actual remote IP address of the client that connected to the cloud server and not the IP Address of the internal forwarder.

Proposed change

The concept is that the SelectorTransport is replaced with the new ChannelTransport that is a wrapper around the channel so we don't need to connect back to localhost since we can create the request handler, connect up the transport and protocol and start_tls

Since aiohttp gets the ip address from the transport it can be set to whatever is desired in a future up. See Deployment considerations

Testing TODO:

• download multi gig file - This is a problem because the protocol doesn't pause when back pressure builds -- this is fixed but there are existing HOL blocking issues (won't be fixed by this PR and exist in production today) Websocket API warnings in log since 2024.8.x home-assistant/core#126754 and will need a new queue strategy to implement some type of round-robin/fairness to avoid one channel starving the others. Implement a fair queue for the multiplexer #352
• fix back pressure problem Implement pause and resume requests for a channel #353
• add tests for pausing/resuming the writing protocol

[bdraco]

Last step is to replace all the connector tests with ones that test connecting to the handler

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@8315a3f). Learn more about missing BASE report.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #303   +/-   ##
=======================================
  Coverage        ?   84.59%           
=======================================
  Files           ?       20           
  Lines           ?     1298           
  Branches        ?      127           
=======================================
  Hits            ?     1098           
  Misses          ?      168           
  Partials        ?       32           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bdraco]

Whatever changed in pytest-asyncio broke the test, and it's not immediately apparent what's wrong