Allow TLS enabled connections when providing an established channel #526
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #526 +/- ##
=======================================
Coverage 61.77% 61.78%
=======================================
Files 125 125
Lines 10072 10074 +2
=======================================
+ Hits 6222 6224 +2
Misses 3850 3850
|
| @@ -193,8 +193,8 @@ extension PostgresConnection { | |||
| /// - channel: The `NIOCore/Channel` to use. The channel must already be active and connected to an | |||
| /// endpoint (i.e. `NIOCore/Channel/isActive` must be `true`). | |||
| /// - tls: The TLS mode to use. Defaults to ``TLS-swift.struct/disable``. | |||
| public init(establishedChannel channel: Channel, username: String, password: String?, database: String?) { | |||
| self.init(endpointInfo: .configureChannel(channel), tls: .disable, username: username, password: password, database: database) | |||
| public init(establishedChannel channel: Channel, tls: PostgresConnection.Configuration.TLS = .disable, username: String, password: String?, database: String?) { | |||
There was a problem hiding this comment.
This is a breaking API change. Would you mind adding an additional initializer? This initializer should then call the new one that you just created.
Do you feel comfortable to add a unit test using a NIOAsyncTestingChannel? An example can be found here:
https://github.com/vapor/postgres-nio/blob/f2a6394a2e7157d547727b975fc0328b92f89fb1/Tests/PostgresNIOTests/New/PostgresConnectionTests.swift#L41C1-L82C1
There was a problem hiding this comment.
I probably won't have time to do the test right now, as I need to fix the wrong channel handler order first for TLS 🤭
There was a problem hiding this comment.
I need to fix the wrong channel handler order first for TLS
Not sure I follow here.
When connecting through an established channel (e.g. an SSH tunnel) it can still be useful to allow postgres to connect using TLS. This PR exposes the tls parameter on the connection method for using an established channel.