-
Notifications
You must be signed in to change notification settings - Fork 1
Add URLEndpointListener QE tests and TLS support in iOS TestServer #384
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
Changes from 19 commits
0c4641b
e808a27
73afb3b
3569675
8fed5b8
edbedae
a92a751
7733116
4cf6a57
aa18542
d19e200
4334ff5
7a7d04c
715d404
6e82c53
f0769a7
02356af
f79c858
ff7d8fb
1317e69
e60bfe8
2dec1b6
6606b2d
d5633c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,7 +86,7 @@ class DatabaseManager { | |
| } | ||
| } | ||
|
|
||
| public func startListener(dbName: String, collections: [String], port: UInt16?, disableTLS: Bool = false) throws -> UUID { | ||
| public func startListener(dbName: String, collections: [String], port: UInt16?, disableTLS: Bool = false, identity:ContentTypes.TLSIdentityData?) throws -> UUID { | ||
|
barkha06 marked this conversation as resolved.
Outdated
|
||
| var collectionsArr: [Collection] = [] | ||
|
|
||
| guard let database = databases[dbName] | ||
|
|
@@ -107,7 +107,27 @@ class DatabaseManager { | |
| var listenerConfig = URLEndpointListenerConfiguration(collections: collectionsArr) | ||
| listenerConfig.port = port | ||
| listenerConfig.disableTLS = disableTLS | ||
|
|
||
| if !listenerConfig.disableTLS { | ||
| let label = "ios-p2p-\(dbName)" | ||
| let importedIdentity: TLSIdentity | ||
|
|
||
| do { | ||
| if let id = identity { | ||
| importedIdentity = try DatabaseManager.createTLSIdentity( for: id, label:label) | ||
| } else { | ||
| guard let existingIdentity = try TLSIdentity.identity(withLabel: label) else { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let do not do this logic to avoid using any stale identity which may cause a test failure that is difficult to figure out. So if the created identity is nil, we can throw the error right away.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can do something like this listenerConfig.disableTLS = disableTLS
if let id = identity {
guard let tlsIdentity = try DatabaseManager.createTLSIdentity( for: id, label:label) else {
throw TestServerError.badRequest("Failed to import TLS identity")
}
listenerConfig.tlsIdentity = tlsIdentity
}
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will fail if a replicator is created twice with the same label, as is done in one of the tests. I had requested something like this to indicate passing nil is the way to say "use the previous existing identity" |
||
| throw TestServerError.badRequest( | ||
| "TLS enabled but no existing TLS identity found for label \(label)" | ||
| ) | ||
| } | ||
| importedIdentity = existingIdentity | ||
| } | ||
| } catch { | ||
| throw TestServerError.badRequest("Failed to import TLS identity") | ||
| } | ||
|
|
||
| listenerConfig.tlsIdentity = importedIdentity | ||
| } | ||
|
barkha06 marked this conversation as resolved.
|
||
| let listener = URLEndpointListener(config: listenerConfig) | ||
|
|
||
| let listenerID = UUID() | ||
|
|
@@ -298,8 +318,8 @@ class DatabaseManager { | |
|
|
||
| public func startMultipeerReplicator(config: ContentTypes.MultipeerReplicatorConfiguration) throws -> UUID { | ||
| Log.log(level: .debug, message: "Starting Multipeer Replicator") | ||
|
|
||
| let identity = try DatabaseManager.multipeerReplicatorIdentity(for: config) | ||
| let label = "ios-multipeer-\(config.peerGroupID)" | ||
| let identity = try DatabaseManager.createTLSIdentity(for: config.identity, label:label) | ||
|
|
||
| let authenticator = try DatabaseManager.multipeerAuthenticator(for: config.authenticator) | ||
|
|
||
|
|
@@ -710,15 +730,14 @@ class DatabaseManager { | |
| } | ||
| } | ||
|
|
||
| private static func multipeerReplicatorIdentity(for config: ContentTypes.MultipeerReplicatorConfiguration) throws -> TLSIdentity { | ||
| let label = "ios-multipeer-\(config.peerGroupID)" | ||
| private static func createTLSIdentity(for identityData: ContentTypes.TLSIdentityData, label: String) throws -> TLSIdentity { | ||
|
|
||
| guard let data = Data(base64Encoded: config.identity.data) else { | ||
| throw TestServerError.badRequest("Invalid multipeer replictor's identity data") | ||
| guard let data = Data(base64Encoded: identityData.data) else { | ||
| throw TestServerError.badRequest("Invalid TLS identity data") | ||
| } | ||
|
|
||
| try TLSIdentity.deleteIdentity(withLabel: label) | ||
| return try TLSIdentity.importIdentity(withData: data, password: config.identity.password, label: label) | ||
| return try TLSIdentity.importIdentity(withData: data, password: identityData.password, label: label) | ||
| } | ||
|
|
||
| private static func multipeerAuthenticator(for config: ContentTypes.MultipeerReplicatorCAAuthenticator?) throws -> MultipeerCertificateAuthenticator { | ||
|
|
||
|
barkha06 marked this conversation as resolved.
|
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 don't see a reason to make this have a default.
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.
This is a required parameter because the replication URL needs to be generated based on the port, so the ty lint check failed since it was marked optional but it needs to have int value later.
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.
It will still have an int value after it is started. I think the ty lint isn't getting all of the information.
couchbase-lite-tests/client/src/cbltest/api/listener.py
Line 78 in d19e200
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 still don't get why the above is not enough to satisfy this situation.
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 am not sure, but here's the failure:
--> /Users/barkha.goyal/Desktop/temp2/couchbase-lite-tests/tests/QE/test_peer_to_peer.py:376:24 | 374 | all_dbs[1], 375 | endpoint=cblpytest.test_servers[0].replication_url( 376 | "db1", listener1.port, tls=True | ^^^^^^^^^^^^^^ Expected int, found Unknown | int | None 377 | ), 378 | replicator_type=replicator_type, | info: Element None of this union is not assignable to int info: Method defined here --> /Users/barkha.goyal/Desktop/temp2/couchbase-lite-tests/client/src/cbltest/api/testserver.py:146:9 | 144 | await self.__request_factory.send_request(self.__index, request) 145 | 146 | def replication_url(self, db_name: str, port: int, tls: bool = False): | ^^^^^^^^^^^^^^^ --------- Parameter declared here 147 | """
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.
One way to avoid would this is to make port a private parameter and define a getter for it, since the getter will always return an int, the ty lint would accept it.