SWIFT-1277, SWIFT-1263, SWIFT-1507 Sync latest initial DNS seedlist discovery tests (#748)

Author: kmahar

Sources/MongoSwift/MongoClient.swift

@@ -77,6 +77,14 @@ public struct MongoClientOptions: CodingStrategyProvider {
     /// seconds (30000 ms).
     public var serverSelectionTimeoutMS: Int?
 
+    /// The maximum number of SRV results to randomly select when initially populating the seedlist or, during SRV
+    /// polling, adding new hosts to the topology.
+    public var srvMaxHosts: Int?
+
+    /// The service name to use for SRV lookup. Must be a valid SRV service name according to RFC 6335.
+    /// -SeeAlso: https://datatracker.ietf.org/doc/html/rfc6335#section-5.1
+    public var srvServiceName: String?
+
     /**
      * `MongoSwift.MongoClient` provides an asynchronous API by running all blocking operations off of their
      * originating threads in a thread pool. `MongoSwiftSync.MongoClient` is implemented as a wrapper of the async
@@ -158,6 +166,8 @@ public struct MongoClientOptions: CodingStrategyProvider {
         retryWrites: Bool? = nil,
         serverAPI: MongoServerAPI? = nil,
         serverSelectionTimeoutMS: Int? = nil,
+        srvMaxHosts: Int? = nil,
+        srvServiceName: String? = nil,
         threadPoolSize: Int? = nil,
         tls: Bool? = nil,
         tlsAllowInvalidCertificates: Bool? = nil,
@@ -187,6 +197,8 @@ public struct MongoClientOptions: CodingStrategyProvider {
         self.retryWrites = retryWrites
         self.retryReads = retryReads
         self.serverAPI = serverAPI
+        self.srvMaxHosts = srvMaxHosts
+        self.srvServiceName = srvServiceName
         self.serverSelectionTimeoutMS = serverSelectionTimeoutMS
         self.threadPoolSize = threadPoolSize
         self.tls = tls

Sources/MongoSwift/MongoConnectionString.swift

@@ -604,6 +604,12 @@ public struct MongoConnectionString: Codable, LosslessStringConvertible {
         if let serverSelectionTimeoutMS = options.serverSelectionTimeoutMS {
             self.serverSelectionTimeoutMS = serverSelectionTimeoutMS
         }
+        if let srvMaxHosts = options.srvMaxHosts {
+            self.srvMaxHosts = srvMaxHosts
+        }
+        if let srvServiceName = options.srvServiceName {
+            self.srvServiceName = srvServiceName
+        }
         if let tls = options.tls {
             self.tls = tls
         }

Tests/MongoSwiftTests/DNSSeedlistTests.swift

@@ -10,9 +10,9 @@ struct DNSSeedlistTestCase: Decodable {
     /// A mongodb+srv connection string.
     let uri: String
     /// The expected set of initial seeds discovered from the SRV record.
-    let seeds: [String]
+    let seeds: [String]?
     /// The discovered topology's list of hosts once SDAM completes a scan.
-    let hosts: [ServerAddress]
+    let hosts: [ServerAddress]?
     /// The parsed connection string options as discovered from URI and TXT records.
     let options: BSONDocument?
     /// Additional options present in the connection string URI such as Userinfo (as user and password), and Auth
@@ -22,9 +22,13 @@ struct DNSSeedlistTestCase: Decodable {
     let error: Bool?
     /// A comment to indicate why a test would fail.
     let comment: String?
+    /// The expected number of initial seeds discovered from the SRV record.
+    let numSeeds: Int?
+    /// The expected number of hosts discovered once SDAM completes a scan.
+    let numHosts: Int?
 
     private enum CodingKeys: String, CodingKey {
-        case uri, seeds, hosts, options, parsedOptions = "parsed_options", error, comment
+        case uri, seeds, hosts, options, parsedOptions = "parsed_options", error, comment, numSeeds, numHosts
     }
 }
 
@@ -88,13 +92,23 @@ final class DNSSeedlistTests: MongoSwiftTestCase {
         try runDNSSeedlistTests(tests)
     }
 
+    func testInitialDNSSeedlistDiscoverySharded() throws {
+        guard MongoSwiftTestCase.topologyType == .sharded else {
+            print("Skipping test because of unsupported topology type \(MongoSwiftTestCase.topologyType)")
+            return
+        }
+
+        let tests = try retrieveSpecTestFiles(
+            specName: "initial-dns-seedlist-discovery",
+            subdirectory: "sharded",
+            asType: DNSSeedlistTestCase.self
+        )
+
+        try runDNSSeedlistTests(tests)
+    }
+
     func runDNSSeedlistTests(_ tests: [(String, DNSSeedlistTestCase)]) throws {
         for (fileName, testCase) in tests {
-            // TODO: SWIFT-1455: unskip these test
-            if fileName == "loadBalanced-no-results.json" || fileName == "loadBalanced-true-multiple-hosts.json" {
-                continue
-            }
-
             // this particular test case requires SSL is disabled. see DRIVERS-1324.
             let requiresTLS = fileName != "txt-record-with-overridden-ssl-option.json"
 
@@ -118,8 +132,7 @@ final class DNSSeedlistTests: MongoSwiftTestCase {
                 try self.withTestClient(testCase.uri, options: opts) { client in
                     client.addSDAMEventHandler(topologyWatcher)
 
-                    // try selecting a server to trigger SDAM
-                    _ = try client.connectionPool.selectServer(forWrites: false)
+                    _ = try client.db("admin").runCommand(["ping": 1]).wait()
 
                     guard testCase.error != true else {
                         XCTFail("Expected error for test case \(testCase.comment ?? ""), got none")
@@ -130,8 +143,13 @@ final class DNSSeedlistTests: MongoSwiftTestCase {
                     // eventually matches the list of hosts."
                     // This needs to be done before the client leaves scope to ensure the SDAM machinery
                     // keeps running.
-                    expect(topologyWatcher.getLastDescription()?.servers.map { $0.address })
-                        .toEventually(equal(testCase.hosts), timeout: 5)
+                    if let expectedHosts = testCase.hosts {
+                        expect(topologyWatcher.getLastDescription()?.servers.map { $0.address })
+                            .toEventually(equal(expectedHosts), timeout: 5)
+                    } else if let expectedNumHosts = testCase.numHosts {
+                        expect(topologyWatcher.getLastDescription()?.servers)
+                            .toEventually(haveCount(expectedNumHosts), timeout: 5)
+                    }
 
                     // "You MUST verify that each of the values of the Connection String Options under options match the
                     // Client's parsed value for that option."

Tests/MongoSwiftTests/MongoConnectionStringTests.swift

@@ -661,4 +661,42 @@ final class ConnectionStringTests: MongoSwiftTestCase {
         connString7.credential?.source = nil
         expect(connString7.description).toNot(contain("authsource"))
     }
+
+    func testSrvMaxHosts() throws {
+        var connString1 = try MongoConnectionString(string: "mongodb+srv://test1.test.build.10gen.cc/?srvMaxHosts=1")
+        expect(connString1.srvMaxHosts).to(equal(1))
+
+        // applyOptions should properly set and override original value
+        let opts1 = MongoClientOptions(srvMaxHosts: 2)
+        try connString1.applyOptions(opts1)
+        expect(connString1.srvMaxHosts).to(equal(2))
+
+        // invalid value of option
+        let opts2 = MongoClientOptions(srvMaxHosts: -1)
+        expect(try connString1.applyOptions(opts2)).to(throwError(errorType: MongoError.InvalidArgumentError.self))
+
+        var connString2 = try MongoConnectionString(string: "mongodb://localhost:27017")
+        // option is only valid with SRV URIs
+        expect(try connString2.applyOptions(opts1)).to(throwError(errorType: MongoError.InvalidArgumentError.self))
+    }
+
+    func testSrvServiceName() throws {
+        var connString1 = try MongoConnectionString(
+            string: "mongodb+srv://test1.test.build.10gen.cc/?srvServiceName=customname"
+        )
+        expect(connString1.srvServiceName).to(equal("customname"))
+
+        // applyOptions should properly set and override original value
+        let opts1 = MongoClientOptions(srvServiceName: "newcustomname")
+        try connString1.applyOptions(opts1)
+        expect(connString1.srvServiceName).to(equal("newcustomname"))
+
+        // invalid value of option
+        let opts2 = MongoClientOptions(srvServiceName: "-invalid-")
+        expect(try connString1.applyOptions(opts2)).to(throwError(errorType: MongoError.InvalidArgumentError.self))
+
+        var connString2 = try MongoConnectionString(string: "mongodb://localhost:27017")
+        // option is only valid with SRV URIs
+        expect(try connString2.applyOptions(opts1)).to(throwError(errorType: MongoError.InvalidArgumentError.self))
+    }
 }

Tests/Specs/initial-dns-seedlist-discovery/initial-dns-seedlist-discovery.rst

@@ -10,8 +10,8 @@ Initial DNS Seedlist Discovery
 :Authors: Derick Rethans
 :Status: Draft
 :Type: Standards
-:Last Modified: 2019-03-07
-:Version: 1.3.2
+:Last Modified: 2021-10-14
+:Version: 1.6.0
 :Spec Lead: Matt Broadstone
 :Advisory Group: \A. Jesse Jiryu Davis
 :Approver(s): Bernie Hackett, David Golden, Jeff Yemin, Matt Broadstone, A. Jesse Jiryu Davis
@@ -48,6 +48,9 @@ interpreted as described in `RFC 2119 <https://www.ietf.org/rfc/rfc2119.txt>`_.
 Specification
 =============
 
+Connection String Format
+------------------------
+
 The connection string parser in the driver is extended with a new protocol
 ``mongodb+srv`` as a logical pre-processing step before it considers the
 connection string and SDAM specifications. In this protocol, the comma
@@ -61,33 +64,59 @@ format is::
 specification following the ``Host Information``. This includes the ``Auth
 database`` and ``Connection Options``.
 
-Seedlist Discovery
-------------------
 
-In this preprocessing step, the driver will query the DNS server for SRV
-records on ``{hostname}.{domainname}``, prefixed with ``_mongodb._tcp.``:
-``_mongodb._tcp.{hostname}.{domainname}``. This DNS query is expected to
-respond with one or more SRV records. From the DNS result, the driver now MUST
-behave the same as if an ``mongodb://`` URI was provided with all the host
-names and port numbers that were returned as part of the DNS SRV query result.
+MongoClient Configuration
+-------------------------
 
-The priority and weight fields in returned SRV records MUST be ignored.
+srvMaxHosts
+~~~~~~~~~~~
 
-If ``mongodb+srv`` is used, a driver MUST implicitly also enable TLS. Clients
-can turn this off by passing ``ssl=false`` in either the Connection String,
-or options passed in as parameters in code to the MongoClient constructor (or
-equivalent API for each driver), but not through a TXT record (discussed in
-the next section).
+This option is used to limit the number of mongos connections that may be
+created for sharded topologies. This option limits the number of SRV records
+used to populate the seedlist during initial discovery, as well as the number of
+additional hosts that may be added during
+`SRV polling <../polling-srv-records-for-mongos-discovery/polling-srv-records-for-mongos-discovery.rst>`_.
+This option requires a non-negative integer and defaults to zero (i.e. no
+limit). This option MUST only be configurable at the level of a ``MongoClient``.
 
-A driver MUST verify that in addition to the ``{hostname}``, the
-``{domainname}`` consists of at least two parts: the domain name, and a TLD.
-Drivers MUST raise an error and MUST NOT contact the DNS server to obtain SRV
-(or TXT records) if the full URI does not consists of at least three parts.
 
-A driver MUST verify that the host names returned through SRV records have the
-same parent ``{domainname}``. Drivers MUST raise an error and MUST NOT
-initiate a connection to any returned host name which does not share the same
-``{domainname}``.
+srvServiceName
+~~~~~~~~~~~~~~
+
+This option specifies a valid SRV service name according to
+`RFC 6335 <https://datatracker.ietf.org/doc/html/rfc6335#section-5.1>`_, with
+the exception that it may exceed 15 characters as long as the 63rd (62nd with
+prepended underscore) character DNS query limit is not surpassed. This option
+requires a string value and defaults to "mongodb". This option MUST only be
+configurable at the level of a ``MongoClient``.
+
+
+URI Validation
+~~~~~~~~~~~~~~
+
+The driver MUST report an error if either the ``srvServiceName`` or
+``srvMaxHosts`` URI options are specified with a non-SRV URI (i.e. scheme other
+than ``mongodb+srv``). The driver MUST allow specifying the ``srvServiceName``
+and ``srvMaxHosts`` URI options with an SRV URI (i.e. ``mongodb+srv`` scheme).
+
+If ``srvMaxHosts`` is a positive integer, the driver MUST throw an error in the
+following cases:
+
+- The connection string contains a ``replicaSet`` option.
+- The connection string contains a ``loadBalanced`` option with a value of
+  ``true``.
+
+When validating URI options, the driver MUST first do the SRV and TXT lookup and
+then perform the validation. For drivers that do SRV lookup asynchronously this
+may result in a ``MongoClient`` being instantiated but erroring later during
+operation execution.
+
+
+Seedlist Discovery
+------------------
+
+Validation Before Querying DNS
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 It is an error to specify a port in a connection string with the
 ``mongodb+srv`` protocol, and the driver MUST raise a parse error and MUST NOT
@@ -97,14 +126,54 @@ It is an error to specify more than one host name in a connection string with
 the ``mongodb+srv`` protocol, and the driver MUST raise a parse error and MUST
 NOT do DNS resolution or contact hosts.
 
-The driver MUST NOT attempt to connect to any hosts until the DNS query has
-returned its results.
+A driver MUST verify that in addition to the ``{hostname}``, the
+``{domainname}`` consists of at least two parts: the domain name, and a TLD.
+Drivers MUST raise an error and MUST NOT contact the DNS server to obtain SRV
+(or TXT records) if the full URI does not consist of at least three parts.
+
+If ``mongodb+srv`` is used, a driver MUST implicitly also enable TLS. Clients
+can turn this off by passing ``tls=false`` in either the Connection String,
+or options passed in as parameters in code to the MongoClient constructor (or
+equivalent API for each driver), but not through a TXT record (discussed in a
+later section).
+
+
+Querying DNS
+~~~~~~~~~~~~
+
+In this preprocessing step, the driver will query the DNS server for SRV records
+on ``{hostname}.{domainname}``, prefixed with the SRV service name and protocol.
+The SRV service name is provided in the ``srvServiceName`` URI option and
+defaults to ``mongodb``. The protocol is always ``tcp``. After prefixing, the
+URI should look like: ``_{srvServiceName}._tcp.{hostname}.{domainname}``. This
+DNS query is expected to respond with one or more SRV records.
+
+The priority and weight fields in returned SRV records MUST be ignored.
 
 If the DNS result returns no SRV records, or no records at all, or a DNS error
 happens, an error MUST be raised indicating that the URI could not be used to
 find hostnames. The error SHALL include the reason why they could not be
 found.
 
+A driver MUST verify that the host names returned through SRV records have the
+same parent ``{domainname}``. Drivers MUST raise an error and MUST NOT
+initiate a connection to any returned host name which does not share the same
+``{domainname}``.
+
+The driver MUST NOT attempt to connect to any hosts until the DNS query has
+returned its results.
+
+If ``srvMaxHosts`` is zero or greater than or equal to the number of hosts in
+the DNS result, the driver MUST populate the seedlist with all hosts.
+
+If ``srvMaxHosts`` is greater than zero and less than the number of hosts in the
+DNS result, the driver MUST randomly select that many hosts and use them to
+populate the seedlist. Drivers SHOULD use the `Fisher-Yates shuffle`_ for
+randomization.
+
+.. _`Fisher-Yates shuffle`: https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle#The_modern_algorithm
+
+
 Default Connection String Options
 ---------------------------------
 
@@ -122,8 +191,8 @@ raise an error when multiple TXT records are encountered.
 Information returned within a TXT record is a simple URI string, just like
 the ``{options}`` in a connection string.
 
-A Client MUST only support the ``authSource`` and ``replicaSet`` options
-through a TXT record, and MUST raise an error if any other option is
+A Client MUST only support the ``authSource``, ``replicaSet``, and ``loadBalanced``
+options through a TXT record, and MUST raise an error if any other option is
 encountered. Although using ``mongodb+srv://`` implicitly enables TLS, a
 Client MUST NOT allow the ``ssl`` option to be set through a TXT record
 option.
@@ -274,6 +343,18 @@ SRV records.
 ChangeLog
 =========
 
+2021-10-14 - 1.6.0
+    Add ``srvMaxHosts`` MongoClient option and restructure Seedlist Discovery
+    section. Improve documentation for the ``srvServiceName`` MongoClient
+    option and add a new URI Validation section.
+
+2021-09-15 - 1.5.0
+    Clarify that service name only defaults to ``mongodb``, and should be
+    defined by the ``srvServiceName`` URI option.
+
+2021-04-15 - 1.4.0
+    Adding in behaviour for load balancer mode.
+
 2019-03-07 - 1.3.2
     Clarify that CNAME is not supported
 

Tests/Specs/initial-dns-seedlist-discovery/tests/load-balanced/loadBalanced-directConnection.json

@@ -1,10 +1,10 @@
 {
-  "uri": "mongodb+srv://test20.test.build.10gen.cc/?directConnection=false",
+  "uri": "mongodb+srv://test24.test.build.10gen.cc/?directConnection=false",
   "seeds": [
-    "localhost.test.build.10gen.cc:27017"
+    "localhost.test.build.10gen.cc:8000"
   ],
   "hosts": [
-    "localhost.test.build.10gen.cc:27017"
+    "localhost.test.build.10gen.cc:8000"
   ],
   "options": {
     "loadBalanced": true,

Tests/Specs/initial-dns-seedlist-discovery/tests/load-balanced/loadBalanced-replicaSet-errors.json

@@ -1,5 +1,5 @@
 {
-  "uri": "mongodb+srv://test20.test.build.10gen.cc/?replicaSet=replset",
+  "uri": "mongodb+srv://test24.test.build.10gen.cc/?replicaSet=replset",
   "seeds": [],
   "hosts": [],
   "error": true,

Tests/Specs/initial-dns-seedlist-discovery/tests/load-balanced/loadBalanced-true-txt.json

@@ -1,10 +1,10 @@
 {
-  "uri": "mongodb+srv://test20.test.build.10gen.cc/",
+  "uri": "mongodb+srv://test24.test.build.10gen.cc/",
   "seeds": [
-    "localhost.test.build.10gen.cc:27017"
+    "localhost.test.build.10gen.cc:8000"
   ],
   "hosts": [
-    "localhost.test.build.10gen.cc:27017"
+    "localhost.test.build.10gen.cc:8000"
   ],
   "options": {
     "loadBalanced": true,