From b4fd021bec424f7cfd2055ee64c54963bc8f78b0 Mon Sep 17 00:00:00 2001 From: Jada Date: Wed, 30 Oct 2024 14:34:20 -0400 Subject: [PATCH 1/7] RUST-911 Add srvServiceName URI option --- src/client/options.rs | 21 +- src/client/options/parse.rs | 1 + src/sdam/srv_polling.rs | 6 +- src/srv.rs | 27 ++- .../initial-dns-seedlist-discovery/README.md | 182 ++++++++++++++++++ .../initial-dns-seedlist-discovery/README.rst | 162 ---------------- 6 files changed, 231 insertions(+), 168 deletions(-) create mode 100644 src/test/spec/json/initial-dns-seedlist-discovery/README.md delete mode 100644 src/test/spec/json/initial-dns-seedlist-discovery/README.rst diff --git a/src/client/options.rs b/src/client/options.rs index 0740e8e77..1aac79c19 100644 --- a/src/client/options.rs +++ b/src/client/options.rs @@ -89,6 +89,7 @@ const URI_OPTIONS: &[&str] = &[ "waitqueuetimeoutms", "wtimeoutms", "zlibcompressionlevel", + "srvservicename", ]; /// Reserved characters as defined by [Section 2.2 of RFC-3986](https://tools.ietf.org/html/rfc3986#section-2.2). @@ -521,6 +522,9 @@ pub struct ClientOptions { /// By default, no default database is specified. pub default_database: Option, + /// Overrides the default "mongodb" service name for SRV lookup in both discovery and polling + pub srv_service_name: Option, + #[builder(setter(skip))] #[derivative(Debug = "ignore")] pub(crate) socket_timeout: Option, @@ -676,6 +680,8 @@ impl Serialize for ClientOptions { loadbalanced: &'a Option, srvmaxhosts: Option, + + srvservicename: &'a Option, } let client_options = ClientOptionsHelper { @@ -709,6 +715,7 @@ impl Serialize for ClientOptions { .map(|v| v.try_into()) .transpose() .map_err(serde::ser::Error::custom)?, + srvservicename: &self.srv_service_name, }; client_options.serialize(serializer) @@ -865,6 +872,9 @@ pub struct ConnectionString { /// Limit on the number of mongos connections that may be created for sharded topologies. pub srv_max_hosts: Option, + /// Overrides the default "mongodb" service name for SRV lookup in both discovery and polling + pub srv_service_name: Option, + wait_queue_timeout: Option, tls_insecure: Option, @@ -904,7 +914,7 @@ impl HostInfo { Ok(match self { Self::HostIdentifiers(hosts) => ResolvedHostInfo::HostIdentifiers(hosts), Self::DnsRecord(hostname) => { - let mut resolver = SrvResolver::new(resolver_config.clone()).await?; + let mut resolver = SrvResolver::new(resolver_config.clone(), None).await?; let config = resolver.resolve_client_options(&hostname).await?; ResolvedHostInfo::DnsRecord { hostname, config } } @@ -1486,6 +1496,12 @@ impl ConnectionString { ConnectionStringParts::default() }; + if conn_str.srv_service_name.is_some() && !srv { + return Err(Error::invalid_argument( + "srvServiceName cannot be specified with a non-SRV URI", + )); + } + if let Some(srv_max_hosts) = conn_str.srv_max_hosts { if !srv { return Err(Error::invalid_argument( @@ -1976,6 +1992,9 @@ impl ConnectionString { k @ "srvmaxhosts" => { self.srv_max_hosts = Some(get_u32!(value, k)); } + "srvservicename" => { + self.srv_service_name = Some(value.to_string()); + } k @ "tls" | k @ "ssl" => { let tls = get_bool!(value, k); diff --git a/src/client/options/parse.rs b/src/client/options/parse.rs index bf7b22bd9..77681e13c 100644 --- a/src/client/options/parse.rs +++ b/src/client/options/parse.rs @@ -159,6 +159,7 @@ impl ClientOptions { #[cfg(feature = "tracing-unstable")] tracing_max_document_length_bytes: None, srv_max_hosts: conn_str.srv_max_hosts, + srv_service_name: conn_str.srv_service_name, } } } diff --git a/src/sdam/srv_polling.rs b/src/sdam/srv_polling.rs index 21598e5ce..5eca3b545 100644 --- a/src/sdam/srv_polling.rs +++ b/src/sdam/srv_polling.rs @@ -130,7 +130,11 @@ impl SrvPollingMonitor { return Ok(resolver); } - let resolver = SrvResolver::new(self.client_options.resolver_config().cloned()).await?; + let resolver = SrvResolver::new( + self.client_options.resolver_config().cloned(), + Option::Some(self.client_options.clone()), + ) + .await?; // Since the connection was not `Some` above, this will always insert the new connection and // return a reference to it. diff --git a/src/srv.rs b/src/srv.rs index d45863495..8a0fdbf8b 100644 --- a/src/srv.rs +++ b/src/srv.rs @@ -2,7 +2,11 @@ use std::time::Duration; #[cfg(feature = "dns-resolver")] use crate::error::ErrorKind; -use crate::{client::options::ResolverConfig, error::Result, options::ServerAddress}; +use crate::{ + client::options::ResolverConfig, + error::Result, + options::{ClientOptions, ServerAddress}, +}; #[derive(Debug)] pub(crate) struct ResolvedConfig { @@ -90,14 +94,21 @@ pub(crate) enum DomainMismatch { #[cfg(feature = "dns-resolver")] pub(crate) struct SrvResolver { resolver: crate::runtime::AsyncResolver, + client_options: Option, } #[cfg(feature = "dns-resolver")] impl SrvResolver { - pub(crate) async fn new(config: Option) -> Result { + pub(crate) async fn new( + config: Option, + client_options: Option, + ) -> Result { let resolver = crate::runtime::AsyncResolver::new(config.map(|c| c.inner)).await?; - Ok(Self { resolver }) + Ok(Self { + resolver, + client_options, + }) } pub(crate) async fn resolve_client_options( @@ -149,7 +160,15 @@ impl SrvResolver { original_hostname: &str, dm: DomainMismatch, ) -> Result { - let lookup_hostname = format!("_mongodb._tcp.{}", original_hostname); + let default_service_name = "mongodb".to_string(); + let service_name = match &self.client_options { + None => default_service_name, + Some(opts) => opts + .srv_service_name + .clone() + .unwrap_or(default_service_name), + }; + let lookup_hostname = format!("_{}._tcp.{}", service_name, original_hostname); self.get_srv_hosts_unvalidated(&lookup_hostname) .await? .validate(original_hostname, dm) diff --git a/src/test/spec/json/initial-dns-seedlist-discovery/README.md b/src/test/spec/json/initial-dns-seedlist-discovery/README.md new file mode 100644 index 000000000..19e5fdd2e --- /dev/null +++ b/src/test/spec/json/initial-dns-seedlist-discovery/README.md @@ -0,0 +1,182 @@ +# Initial DNS Seedlist Discovery tests + +This directory contains platform-independent tests that drivers can use to prove their conformance to the Initial DNS +Seedlist Discovery spec. + +## Prose Tests + +For the following prose tests, it is assumed drivers are be able to stub DNS results to easily test invalid DNS +resolution results. + +### 1. Allow SRVs with fewer than 3 `.` separated parts + +When running validation on an SRV string before DNS resolution, do not throw a error due to number of SRV parts. + +- `mongodb+srv://localhost` +- `mongodb+srv://mongo.local` + +### 2. Throw when return address does not end with SRV domain + +When given a returned address that does NOT end with the original SRV's domain name, throw a runtime error. + +For this test, run each of the following cases: + +- the SRV `mongodb+srv://localhost` resolving to `localhost.mongodb` +- the SRV `mongodb+srv://mongo.local` resolving to `test_1.evil.local` +- the SRV `mongodb+srv://blogs.mongodb.com` resolving to `blogs.evil.com` + +Remember, the domain of an SRV with one or two `.` separated parts is the SRVs entire hostname. + +### 3. Throw when return address is identical to SRV hostname + +When given a returned address that is identical to the SRV hostname and the SRV hostname has fewer than three `.` +separated parts, throw a runtime error. + +For this test, run each of the following cases: + +- the SRV `mongodb+srv://localhost` resolving to `localhost` +- the SRV `mongodb+srv://mongo.local` resolving to `mongo.local` + +### 4. Throw when return address does not contain `.` separating shared part of domain + +When given a returned address that does NOT share the domain name of the SRV record because it's missing a `.`, throw a +runtime error. + +For this test, run each of the following cases: + +- the SRV `mongodb+srv://localhost` resolving to `test_1.cluster_1localhost` +- the SRV `mongodb+srv://mongo.local` resolving to `test_1.my_hostmongo.local` +- the SRV `mongodb+srv://blogs.mongodb.com` resolving to `cluster.testmongodb.com` + +## Test Setup + +The tests in the `replica-set` directory MUST be executed against a three-node replica set on localhost ports 27017, +27018, and 27019 with replica set name `repl0`. + +The tests in the `load-balanced` directory MUST be executed against a load-balanced sharded cluster with the mongos +servers running on localhost ports 27017 and 27018 and `--loadBalancerPort` 27050 and 27051, respectively (corresponding +to the script in +[drivers-evergreen-tools](https://github.com/mongodb-labs/drivers-evergreen-tools/blob/master/.evergreen/run-load-balancer.sh)). +The load balancers, shard servers, and config servers may run on any open ports. + +The tests in the `sharded` directory MUST be executed against a sharded cluster with the mongos servers running on +localhost ports 27017 and 27018. Shard servers and config servers may run on any open ports. + +In all cases, the clusters MUST be started with SSL enabled. + +To run the tests that accompany this spec, you need to configure the SRV and TXT records with a real name server. The +following records are required for these tests: + +```dns +Record TTL Class Address +localhost.test.build.10gen.cc. 86400 IN A 127.0.0.1 +localhost.sub.test.build.10gen.cc. 86400 IN A 127.0.0.1 + +Record TTL Class Port Target +_mongodb._tcp.test1.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. +_mongodb._tcp.test1.test.build.10gen.cc. 86400 IN SRV 27018 localhost.test.build.10gen.cc. +_mongodb._tcp.test2.test.build.10gen.cc. 86400 IN SRV 27018 localhost.test.build.10gen.cc. +_mongodb._tcp.test2.test.build.10gen.cc. 86400 IN SRV 27019 localhost.test.build.10gen.cc. +_mongodb._tcp.test3.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. +_mongodb._tcp.test5.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. +_mongodb._tcp.test6.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. +_mongodb._tcp.test7.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. +_mongodb._tcp.test8.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. +_mongodb._tcp.test10.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. +_mongodb._tcp.test11.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. +_mongodb._tcp.test12.test.build.10gen.cc. 86400 IN SRV 27017 localhost.build.10gen.cc. +_mongodb._tcp.test13.test.build.10gen.cc. 86400 IN SRV 27017 test.build.10gen.cc. +_mongodb._tcp.test14.test.build.10gen.cc. 86400 IN SRV 27017 localhost.not-test.build.10gen.cc. +_mongodb._tcp.test15.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.not-build.10gen.cc. +_mongodb._tcp.test16.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.not-10gen.cc. +_mongodb._tcp.test17.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.not-cc. +_mongodb._tcp.test18.test.build.10gen.cc. 86400 IN SRV 27017 localhost.sub.test.build.10gen.cc. +_mongodb._tcp.test19.test.build.10gen.cc. 86400 IN SRV 27017 localhost.evil.build.10gen.cc. +_mongodb._tcp.test19.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. +_mongodb._tcp.test20.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. +_mongodb._tcp.test21.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. +_customname._tcp.test22.test.build.10gen.cc 86400 IN SRV 27017 localhost.test.build.10gen.cc. +_mongodb._tcp.test23.test.build.10gen.cc. 86400 IN SRV 8000 localhost.test.build.10gen.cc. +_mongodb._tcp.test24.test.build.10gen.cc. 86400 IN SRV 8000 localhost.test.build.10gen.cc. + +Record TTL Class Text +test5.test.build.10gen.cc. 86400 IN TXT "replicaSet=repl0&authSource=thisDB" +test6.test.build.10gen.cc. 86400 IN TXT "replicaSet=repl0" +test6.test.build.10gen.cc. 86400 IN TXT "authSource=otherDB" +test7.test.build.10gen.cc. 86400 IN TXT "ssl=false" +test8.test.build.10gen.cc. 86400 IN TXT "authSource" +test10.test.build.10gen.cc. 86400 IN TXT "socketTimeoutMS=500" +test11.test.build.10gen.cc. 86400 IN TXT "replicaS" "et=rep" "l0" +test20.test.build.10gen.cc. 86400 IN TXT "loadBalanced=true" +test21.test.build.10gen.cc. 86400 IN TXT "loadBalanced=false" +test24.test.build.10gen.cc. 86400 IN TXT "loadBalanced=true" +``` + +Notes: + +- `test4` is omitted deliberately to test what happens with no SRV record. +- `test9` is missing because it was deleted during the development of the tests. +- The missing `test.` sub-domain in the SRV record target for `test12` is deliberate. +- `test22` is used to test a custom service name (`customname`). +- `test23` and `test24` point to port 8000 (HAProxy) and are used for load-balanced tests. + +In our tests we have used `localhost.test.build.10gen.cc` as the domain, and then configured +`localhost.test.build.10gen.cc` to resolve to 127.0.0.1. + +You need to adapt the records shown above to replace `test.build.10gen.cc` with your own domain name, and update the +"uri" field in the YAML or JSON files in this directory with the actual domain. + +## Test Format and Use + +These YAML and JSON files contain the following fields: + +- `uri`: a `mongodb+srv` connection string +- `seeds`: the expected set of initial seeds discovered from the SRV record +- `numSeeds`: the expected number of initial seeds discovered from the SRV record. This is mainly used to test + `srvMaxHosts`, since randomly selected hosts cannot be deterministically asserted. +- `hosts`: the discovered topology's list of hosts once SDAM completes a scan +- `numHosts`: the expected number of hosts discovered once SDAM completes a scan. This is mainly used to test + `srvMaxHosts`, since randomly selected hosts cannot be deterministically asserted. +- `options`: the parsed [URI options](../../uri-options/uri-options.md) as discovered from the + [Connection String](../../connection-string/connection-string-spec.md)'s "Connection Options" component and SRV + resolution (e.g. TXT records, implicit `tls` default). +- `parsed_options`: additional, parsed options from other + [Connection String](../../connection-string/connection-string-spec.md) components. This is mainly used for asserting + `UserInfo` (as `user` and `password`) and `Auth database` (as `auth_database`). +- `error`: indicates that the parsing of the URI, or the resolving or contents of the SRV or TXT records included + errors. +- `comment`: a comment to indicate why a test would fail. +- `ping`: if false, the test runner should not run a "ping" operation. + +For each YAML file: + +- Create a MongoClient initialized with the `mongodb+srv` connection string. +- Run a "ping" operation unless `ping` is false or `error` is true. + +Assertions: + +- If `seeds` is specified, drivers SHOULD verify that the set of hosts in the client's initial seedlist matches the list + in `seeds`. If `numSeeds` is specified, drivers SHOULD verify that the size of that set matches `numSeeds`. + +- If `hosts` is specified, drivers MUST verify that the set of ServerDescriptions in the client's TopologyDescription + eventually matches the list in `hosts`. If `numHosts` is specified, drivers MUST verify that the size of that set + matches `numHosts`. + +- If `options` is specified, drivers MUST verify each of the values under `options` match the MongoClient's parsed value + for that option. There may be other options parsed by the MongoClient as well, which a test does not verify. + +- If `parsed_options` is specified, drivers MUST verify that each of the values under `parsed_options` match the + MongoClient's parsed value for that option. Supported values include, but are not limited to, `user` and `password` + (parsed from `UserInfo`) and `auth_database` (parsed from `Auth database`). + +- If `error` is specified and `true`, drivers MUST verify that initializing the MongoClient throws an error. If `error` + is not specified or is `false`, both initializing the MongoClient and running a ping operation must succeed without + throwing any errors. + +- If `ping` is not specified or `true`, drivers MUST verify that running a "ping" operation using the initialized + MongoClient succeeds. If `ping` is `false`, drivers MUST NOT run a "ping" operation. + + > **Note:** These tests are expected to be run against MongoDB databases with and without authentication enabled. The + > "ping" operation does not require authentication so should succeed with URIs that contain no userinfo (i.e. no + > username and password). Tests with URIs that contain userinfo always set `ping` to `false` because some drivers will + > fail handshake on a connection if userinfo is provided but incorrect. diff --git a/src/test/spec/json/initial-dns-seedlist-discovery/README.rst b/src/test/spec/json/initial-dns-seedlist-discovery/README.rst deleted file mode 100644 index c1f6c5bb4..000000000 --- a/src/test/spec/json/initial-dns-seedlist-discovery/README.rst +++ /dev/null @@ -1,162 +0,0 @@ -==================================== -Initial DNS Seedlist Discovery tests -==================================== - -This directory contains platform-independent tests that drivers can use -to prove their conformance to the Initial DNS Seedlist Discovery spec. - -Test Setup ----------- - -The tests in the ``replica-set`` directory MUST be executed against a -three-node replica set on localhost ports 27017, 27018, and 27019 with -replica set name ``repl0``. - -The tests in the ``load-balanced`` directory MUST be executed against a -load-balanced sharded cluster with the mongos servers running on localhost ports -27017 and 27018 and ``--loadBalancerPort`` 27050 and 27051, respectively -(corresponding to the script in `drivers-evergreen-tools`_). The load balancers, -shard servers, and config servers may run on any open ports. - -.. _`drivers-evergreen-tools`: https://github.com/mongodb-labs/drivers-evergreen-tools/blob/master/.evergreen/run-load-balancer.sh - -The tests in the ``sharded`` directory MUST be executed against a sharded -cluster with the mongos servers running on localhost ports 27017 and 27018. -Shard servers and config servers may run on any open ports. - -In all cases, the clusters MUST be started with SSL enabled. - -To run the tests that accompany this spec, you need to configure the SRV and -TXT records with a real name server. The following records are required for -these tests:: - - Record TTL Class Address - localhost.test.build.10gen.cc. 86400 IN A 127.0.0.1 - localhost.sub.test.build.10gen.cc. 86400 IN A 127.0.0.1 - - Record TTL Class Port Target - _mongodb._tcp.test1.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. - _mongodb._tcp.test1.test.build.10gen.cc. 86400 IN SRV 27018 localhost.test.build.10gen.cc. - _mongodb._tcp.test2.test.build.10gen.cc. 86400 IN SRV 27018 localhost.test.build.10gen.cc. - _mongodb._tcp.test2.test.build.10gen.cc. 86400 IN SRV 27019 localhost.test.build.10gen.cc. - _mongodb._tcp.test3.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. - _mongodb._tcp.test5.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. - _mongodb._tcp.test6.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. - _mongodb._tcp.test7.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. - _mongodb._tcp.test8.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. - _mongodb._tcp.test10.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. - _mongodb._tcp.test11.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. - _mongodb._tcp.test12.test.build.10gen.cc. 86400 IN SRV 27017 localhost.build.10gen.cc. - _mongodb._tcp.test13.test.build.10gen.cc. 86400 IN SRV 27017 test.build.10gen.cc. - _mongodb._tcp.test14.test.build.10gen.cc. 86400 IN SRV 27017 localhost.not-test.build.10gen.cc. - _mongodb._tcp.test15.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.not-build.10gen.cc. - _mongodb._tcp.test16.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.not-10gen.cc. - _mongodb._tcp.test17.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.not-cc. - _mongodb._tcp.test18.test.build.10gen.cc. 86400 IN SRV 27017 localhost.sub.test.build.10gen.cc. - _mongodb._tcp.test19.test.build.10gen.cc. 86400 IN SRV 27017 localhost.evil.build.10gen.cc. - _mongodb._tcp.test19.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. - _mongodb._tcp.test20.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. - _mongodb._tcp.test21.test.build.10gen.cc. 86400 IN SRV 27017 localhost.test.build.10gen.cc. - _customname._tcp.test22.test.build.10gen.cc 86400 IN SRV 27017 localhost.test.build.10gen.cc. - _mongodb._tcp.test23.test.build.10gen.cc. 86400 IN SRV 8000 localhost.test.build.10gen.cc. - _mongodb._tcp.test24.test.build.10gen.cc. 86400 IN SRV 8000 localhost.test.build.10gen.cc. - - Record TTL Class Text - test5.test.build.10gen.cc. 86400 IN TXT "replicaSet=repl0&authSource=thisDB" - test6.test.build.10gen.cc. 86400 IN TXT "replicaSet=repl0" - test6.test.build.10gen.cc. 86400 IN TXT "authSource=otherDB" - test7.test.build.10gen.cc. 86400 IN TXT "ssl=false" - test8.test.build.10gen.cc. 86400 IN TXT "authSource" - test10.test.build.10gen.cc. 86400 IN TXT "socketTimeoutMS=500" - test11.test.build.10gen.cc. 86400 IN TXT "replicaS" "et=rep" "l0" - test20.test.build.10gen.cc. 86400 IN TXT "loadBalanced=true" - test21.test.build.10gen.cc. 86400 IN TXT "loadBalanced=false" - test24.test.build.10gen.cc. 86400 IN TXT "loadBalanced=true" - -Notes: - -- ``test4`` is omitted deliberately to test what happens with no SRV record. -- ``test9`` is missing because it was deleted during the development of the - tests. -- The missing ``test.`` sub-domain in the SRV record target for ``test12`` is - deliberate. -- ``test22`` is used to test a custom service name (``customname``). -- ``test23`` and ``test24`` point to port 8000 (HAProxy) and are used for - load-balanced tests. - -In our tests we have used ``localhost.test.build.10gen.cc`` as the domain, and -then configured ``localhost.test.build.10gen.cc`` to resolve to 127.0.0.1. - -You need to adapt the records shown above to replace ``test.build.10gen.cc`` -with your own domain name, and update the "uri" field in the YAML or JSON files -in this directory with the actual domain. - -Test Format and Use -------------------- - -These YAML and JSON files contain the following fields: - -- ``uri``: a ``mongodb+srv`` connection string -- ``seeds``: the expected set of initial seeds discovered from the SRV record -- ``numSeeds``: the expected number of initial seeds discovered from the SRV - record. This is mainly used to test ``srvMaxHosts``, since randomly selected - hosts cannot be deterministically asserted. -- ``hosts``: the discovered topology's list of hosts once SDAM completes a scan -- ``numHosts``: the expected number of hosts discovered once SDAM completes a - scan. This is mainly used to test ``srvMaxHosts``, since randomly selected - hosts cannot be deterministically asserted. -- ``options``: the parsed `URI options`_ as discovered from the - `Connection String`_'s "Connection Options" component and SRV resolution - (e.g. TXT records, implicit ``tls`` default). -- ``parsed_options``: additional, parsed options from other `Connection String`_ - components. This is mainly used for asserting ``UserInfo`` (as ``user`` and - ``password``) and ``Auth database`` (as ``auth_database``). -- ``error``: indicates that the parsing of the URI, or the resolving or - contents of the SRV or TXT records included errors. -- ``comment``: a comment to indicate why a test would fail. -- ``ping``: if true, the test runner should run a "ping" operation after - initializing a MongoClient. - -.. _`Connection String`: ../../connection-string/connection-string-spec.rst -.. _`URI options`: ../../uri-options/uri-options.rst - -For each YAML file: - -- Create a MongoClient initialized with the ``mongodb+srv`` - connection string. -- Run run a "ping" operation if ``ping`` in the test file is true. - -Assertions: - -- If ``seeds`` is specified, drivers SHOULD verify that the set of hosts in the - client's initial seedlist matches the list in ``seeds``. If ``numSeeds`` is - specified, drivers SHOULD verify that the size of that set matches - ``numSeeds``. - -- If ``hosts`` is specified, drivers MUST verify that the set of - ServerDescriptions in the client's TopologyDescription eventually matches the - list in ``hosts``. If ``numHosts`` is specified, drivers MUST verify that the - size of that set matches ``numHosts``. - -- If ``options`` is specified, drivers MUST verify each of the values under - ``options`` match the MongoClient's parsed value for that option. There may be - other options parsed by the MongoClient as well, which a test does not verify. - -- If ``parsed_options`` is specified, drivers MUST verify that each of the - values under ``parsed_options`` match the MongoClient's parsed value for that - option. Supported values include, but are not limited to, ``user`` and - ``password`` (parsed from ``UserInfo``) and ``auth_database`` (parsed from - ``Auth database``). - -- If ``error`` is specified and ``true``, drivers MUST verify that initializing - the MongoClient throws an error. If ``error`` is not specified or is - ``false``, both initializing the MongoClient and running a ping operation must - succeed without throwing any errors. - -- If ``ping`` is specified and ``true``, drivers MUST verify that running a - "ping" operation using the initialized MongoClient succeeds. If ``ping`` is - not specified or is ``false``, drivers MUST NOT run a "ping" operation. - **Note:** These tests are expected to be run against MongoDB databases with - and without authentication enabled. The "ping" operation does not require - authentication so should succeed, even though the test URIs do not have - correct authentication information. From ef02be488fe7e401f0e727ee55caa0a989b90d2c Mon Sep 17 00:00:00 2001 From: Jada Date: Wed, 30 Oct 2024 14:56:43 -0400 Subject: [PATCH 2/7] fix format --- src/client/options.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/client/options.rs b/src/client/options.rs index 1aac79c19..0dfeb1f9c 100644 --- a/src/client/options.rs +++ b/src/client/options.rs @@ -1497,9 +1497,9 @@ impl ConnectionString { }; if conn_str.srv_service_name.is_some() && !srv { - return Err(Error::invalid_argument( - "srvServiceName cannot be specified with a non-SRV URI", - )); + return Err(Error::invalid_argument( + "srvServiceName cannot be specified with a non-SRV URI", + )); } if let Some(srv_max_hosts) = conn_str.srv_max_hosts { From 61c4466df5f569a4ee4dbe9caa3a240ef83c6ace Mon Sep 17 00:00:00 2001 From: Jada Date: Thu, 31 Oct 2024 10:57:35 -0400 Subject: [PATCH 3/7] Add prose test and unskip srvServiceName tests --- src/client/options.rs | 9 +++++-- src/client/options/parse.rs | 4 ++- src/client/options/test.rs | 2 -- src/sdam/srv_polling.rs | 2 +- src/sdam/srv_polling/test.rs | 27 +++++++++++++++++++ src/srv.rs | 26 +++++++----------- .../spec/initial_dns_seedlist_discovery.rs | 18 ------------- 7 files changed, 48 insertions(+), 40 deletions(-) diff --git a/src/client/options.rs b/src/client/options.rs index 0dfeb1f9c..c58be4e4c 100644 --- a/src/client/options.rs +++ b/src/client/options.rs @@ -910,11 +910,16 @@ impl Default for HostInfo { } impl HostInfo { - async fn resolve(self, resolver_config: Option) -> Result { + async fn resolve( + self, + resolver_config: Option, + srv_service_name: Option, + ) -> Result { Ok(match self { Self::HostIdentifiers(hosts) => ResolvedHostInfo::HostIdentifiers(hosts), Self::DnsRecord(hostname) => { - let mut resolver = SrvResolver::new(resolver_config.clone(), None).await?; + let mut resolver = + SrvResolver::new(resolver_config.clone(), srv_service_name).await?; let config = resolver.resolve_client_options(&hostname).await?; ResolvedHostInfo::DnsRecord { hostname, config } } diff --git a/src/client/options/parse.rs b/src/client/options/parse.rs index 77681e13c..8aed36ae4 100644 --- a/src/client/options/parse.rs +++ b/src/client/options/parse.rs @@ -24,7 +24,9 @@ impl Action for ParseConnectionString { options.resolver_config.clone_from(&self.resolver_config); } - let resolved = host_info.resolve(self.resolver_config).await?; + let resolved = host_info + .resolve(self.resolver_config, options.srv_service_name.clone()) + .await?; options.hosts = match resolved { ResolvedHostInfo::HostIdentifiers(hosts) => hosts, ResolvedHostInfo::DnsRecord { diff --git a/src/client/options/test.rs b/src/client/options/test.rs index 969638408..863be8f93 100644 --- a/src/client/options/test.rs +++ b/src/client/options/test.rs @@ -22,8 +22,6 @@ static SKIPPED_TESTS: Lazy> = Lazy::new(|| { "maxPoolSize=0 does not error", // TODO RUST-226: unskip this test "Valid tlsCertificateKeyFilePassword is parsed correctly", - // TODO RUST-911: unskip this test - "SRV URI with custom srvServiceName", // TODO RUST-229: unskip the following tests "Single IP literal host without port", "Single IP literal host with port", diff --git a/src/sdam/srv_polling.rs b/src/sdam/srv_polling.rs index 5eca3b545..bd4782000 100644 --- a/src/sdam/srv_polling.rs +++ b/src/sdam/srv_polling.rs @@ -132,7 +132,7 @@ impl SrvPollingMonitor { let resolver = SrvResolver::new( self.client_options.resolver_config().cloned(), - Option::Some(self.client_options.clone()), + self.client_options.srv_service_name.clone(), ) .await?; diff --git a/src/sdam/srv_polling/test.rs b/src/sdam/srv_polling/test.rs index 6b0310d7e..b27c7699c 100644 --- a/src/sdam/srv_polling/test.rs +++ b/src/sdam/srv_polling/test.rs @@ -186,3 +186,30 @@ async fn srv_max_hosts_random() { assert_eq!(2, actual.len()); assert!(actual.contains(&localhost_test_build_10gen(27017))); } + +#[tokio::test] +async fn srv_service_name() { + if get_client_options().await.srv_service_name.is_none() { + log_uncaptured("skipping srv_service_name due to no custom srvServiceName"); + return; + } + let mut options = ClientOptions::new_srv(); + let hosts = vec![ + localhost_test_build_10gen(27019), + localhost_test_build_10gen(27020), + ]; + let rescan_interval = options.original_srv_info.as_ref().cloned().unwrap().min_ttl; + options.hosts.clone_from(&hosts); + options.srv_service_name = Some("customname".to_string()); + options.test_options_mut().mock_lookup_hosts = Some(make_lookup_hosts(vec![ + localhost_test_build_10gen(27019), + localhost_test_build_10gen(27020), + ])); + let mut topology = Topology::new(options).unwrap(); + topology.watch().wait_until_initialized().await; + tokio::time::sleep(rescan_interval * 2).await; + assert_eq!( + hosts.into_iter().collect::>(), + topology.server_addresses() + ); +} diff --git a/src/srv.rs b/src/srv.rs index 8a0fdbf8b..b486414fb 100644 --- a/src/srv.rs +++ b/src/srv.rs @@ -2,11 +2,7 @@ use std::time::Duration; #[cfg(feature = "dns-resolver")] use crate::error::ErrorKind; -use crate::{ - client::options::ResolverConfig, - error::Result, - options::{ClientOptions, ServerAddress}, -}; +use crate::{client::options::ResolverConfig, error::Result, options::ServerAddress}; #[derive(Debug)] pub(crate) struct ResolvedConfig { @@ -94,20 +90,20 @@ pub(crate) enum DomainMismatch { #[cfg(feature = "dns-resolver")] pub(crate) struct SrvResolver { resolver: crate::runtime::AsyncResolver, - client_options: Option, + srv_service_name: Option, } #[cfg(feature = "dns-resolver")] impl SrvResolver { pub(crate) async fn new( config: Option, - client_options: Option, + srv_service_name: Option, ) -> Result { let resolver = crate::runtime::AsyncResolver::new(config.map(|c| c.inner)).await?; Ok(Self { resolver, - client_options, + srv_service_name, }) } @@ -160,15 +156,13 @@ impl SrvResolver { original_hostname: &str, dm: DomainMismatch, ) -> Result { - let default_service_name = "mongodb".to_string(); - let service_name = match &self.client_options { - None => default_service_name, - Some(opts) => opts - .srv_service_name + let lookup_hostname = format!( + "_{}._tcp.{}", + self.srv_service_name .clone() - .unwrap_or(default_service_name), - }; - let lookup_hostname = format!("_{}._tcp.{}", service_name, original_hostname); + .unwrap_or("mongodb".to_string()), + original_hostname + ); self.get_srv_hosts_unvalidated(&lookup_hostname) .await? .validate(original_hostname, dm) diff --git a/src/test/spec/initial_dns_seedlist_discovery.rs b/src/test/spec/initial_dns_seedlist_discovery.rs index b04c1219b..44f1231e9 100644 --- a/src/test/spec/initial_dns_seedlist_discovery.rs +++ b/src/test/spec/initial_dns_seedlist_discovery.rs @@ -62,24 +62,6 @@ struct ParsedOptions { } async fn run_test(mut test_file: TestFile) { - if let Some(ref options) = test_file.options { - // TODO RUST-933: Remove this skip. - let skip = if options.srv_service_name.is_some() { - Some("srvServiceName") - } else { - None - }; - - if let Some(skip) = skip { - log_uncaptured(format!( - "skipping initial_dns_seedlist_discovery test case due to unsupported connection \ - string option: {}", - skip, - )); - return; - } - } - // "encoded-userinfo-and-db.json" specifies a database name with a question mark which is // disallowed on Windows. See // From 2010b9b276cc42667d90e1c87e33524ca0d5731b Mon Sep 17 00:00:00 2001 From: Jada Date: Tue, 5 Nov 2024 13:40:50 -0500 Subject: [PATCH 4/7] use run_test_srv --- src/sdam/srv_polling/test.rs | 35 ++++++++++------------------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/src/sdam/srv_polling/test.rs b/src/sdam/srv_polling/test.rs index b27c7699c..3fd89fc5b 100644 --- a/src/sdam/srv_polling/test.rs +++ b/src/sdam/srv_polling/test.rs @@ -26,26 +26,29 @@ static DEFAULT_HOSTS: Lazy> = Lazy::new(|| { }); async fn run_test(new_hosts: Result>, expected_hosts: HashSet) { - run_test_srv(None, new_hosts, expected_hosts).await + run_test_srv(None, new_hosts, expected_hosts, None).await } async fn run_test_srv( max_hosts: Option, new_hosts: Result>, expected_hosts: HashSet, + srv_service_name: Option, ) { - let actual = run_test_extra(max_hosts, new_hosts).await; + let actual = run_test_extra(max_hosts, new_hosts, srv_service_name).await; assert_eq!(expected_hosts, actual); } async fn run_test_extra( max_hosts: Option, new_hosts: Result>, + srv_service_name: Option ) -> HashSet { let mut options = ClientOptions::new_srv(); options.hosts.clone_from(&DEFAULT_HOSTS); options.test_options_mut().disable_monitoring_threads = true; options.srv_max_hosts = max_hosts; + options.srv_service_name = srv_service_name; let mut topology = Topology::new(options.clone()).unwrap(); topology.watch().wait_until_initialized().await; let mut monitor = @@ -156,8 +159,8 @@ async fn srv_max_hosts_zero() { localhost_test_build_10gen(27020), ]; - run_test_srv(None, Ok(hosts.clone()), hosts.clone().into_iter().collect()).await; - run_test_srv(Some(0), Ok(hosts.clone()), hosts.into_iter().collect()).await; + run_test_srv(None, Ok(hosts.clone()), hosts.clone().into_iter().collect(), None).await; + run_test_srv(Some(0), Ok(hosts.clone()), hosts.into_iter().collect(), None).await; } // SRV polling with srvMaxHosts MongoClient option: All DNS records are selected (srvMaxHosts >= @@ -169,7 +172,7 @@ async fn srv_max_hosts_gt_actual() { localhost_test_build_10gen(27020), ]; - run_test_srv(Some(2), Ok(hosts.clone()), hosts.into_iter().collect()).await; + run_test_srv(Some(2), Ok(hosts.clone()), hosts.into_iter().collect(), None).await; } // SRV polling with srvMaxHosts MongoClient option: New DNS records are randomly selected @@ -182,34 +185,16 @@ async fn srv_max_hosts_random() { localhost_test_build_10gen(27020), ]; - let actual = run_test_extra(Some(2), Ok(hosts)).await; + let actual = run_test_extra(Some(2), Ok(hosts), None).await; assert_eq!(2, actual.len()); assert!(actual.contains(&localhost_test_build_10gen(27017))); } #[tokio::test] async fn srv_service_name() { - if get_client_options().await.srv_service_name.is_none() { - log_uncaptured("skipping srv_service_name due to no custom srvServiceName"); - return; - } - let mut options = ClientOptions::new_srv(); let hosts = vec![ localhost_test_build_10gen(27019), localhost_test_build_10gen(27020), ]; - let rescan_interval = options.original_srv_info.as_ref().cloned().unwrap().min_ttl; - options.hosts.clone_from(&hosts); - options.srv_service_name = Some("customname".to_string()); - options.test_options_mut().mock_lookup_hosts = Some(make_lookup_hosts(vec![ - localhost_test_build_10gen(27019), - localhost_test_build_10gen(27020), - ])); - let mut topology = Topology::new(options).unwrap(); - topology.watch().wait_until_initialized().await; - tokio::time::sleep(rescan_interval * 2).await; - assert_eq!( - hosts.into_iter().collect::>(), - topology.server_addresses() - ); + run_test_srv(None, Ok(hosts.clone()), hosts.into_iter().collect(), Some("customname".to_string())).await; } From fdf235e140c9ad7b2957c8815e9b9403c51c8c3e Mon Sep 17 00:00:00 2001 From: Jada Date: Tue, 5 Nov 2024 15:16:10 -0500 Subject: [PATCH 5/7] fix lint --- src/sdam/srv_polling/test.rs | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/src/sdam/srv_polling/test.rs b/src/sdam/srv_polling/test.rs index 3fd89fc5b..a0fba1404 100644 --- a/src/sdam/srv_polling/test.rs +++ b/src/sdam/srv_polling/test.rs @@ -42,7 +42,7 @@ async fn run_test_srv( async fn run_test_extra( max_hosts: Option, new_hosts: Result>, - srv_service_name: Option + srv_service_name: Option, ) -> HashSet { let mut options = ClientOptions::new_srv(); options.hosts.clone_from(&DEFAULT_HOSTS); @@ -159,8 +159,20 @@ async fn srv_max_hosts_zero() { localhost_test_build_10gen(27020), ]; - run_test_srv(None, Ok(hosts.clone()), hosts.clone().into_iter().collect(), None).await; - run_test_srv(Some(0), Ok(hosts.clone()), hosts.into_iter().collect(), None).await; + run_test_srv( + None, + Ok(hosts.clone()), + hosts.clone().into_iter().collect(), + None, + ) + .await; + run_test_srv( + Some(0), + Ok(hosts.clone()), + hosts.into_iter().collect(), + None, + ) + .await; } // SRV polling with srvMaxHosts MongoClient option: All DNS records are selected (srvMaxHosts >= @@ -172,7 +184,13 @@ async fn srv_max_hosts_gt_actual() { localhost_test_build_10gen(27020), ]; - run_test_srv(Some(2), Ok(hosts.clone()), hosts.into_iter().collect(), None).await; + run_test_srv( + Some(2), + Ok(hosts.clone()), + hosts.into_iter().collect(), + None, + ) + .await; } // SRV polling with srvMaxHosts MongoClient option: New DNS records are randomly selected @@ -196,5 +214,11 @@ async fn srv_service_name() { localhost_test_build_10gen(27019), localhost_test_build_10gen(27020), ]; - run_test_srv(None, Ok(hosts.clone()), hosts.into_iter().collect(), Some("customname".to_string())).await; + run_test_srv( + None, + Ok(hosts.clone()), + hosts.into_iter().collect(), + Some("customname".to_string()), + ) + .await; } From d14b89949d34af8d269ce7b90befdbf0fccf13bb Mon Sep 17 00:00:00 2001 From: Jada Date: Wed, 6 Nov 2024 10:58:11 -0500 Subject: [PATCH 6/7] fix style nit --- src/srv.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/srv.rs b/src/srv.rs index b486414fb..66ae74e89 100644 --- a/src/srv.rs +++ b/src/srv.rs @@ -158,9 +158,7 @@ impl SrvResolver { ) -> Result { let lookup_hostname = format!( "_{}._tcp.{}", - self.srv_service_name - .clone() - .unwrap_or("mongodb".to_string()), + self.srv_service_name.as_deref().unwrap_or("mongodb"), original_hostname ); self.get_srv_hosts_unvalidated(&lookup_hostname) From 7947fbe15f7204fc2472531e0455565f1e7231eb Mon Sep 17 00:00:00 2001 From: Jada Date: Thu, 7 Nov 2024 12:39:06 -0500 Subject: [PATCH 7/7] update test to align more closely with spec description --- src/sdam/srv_polling.rs | 6 +++- src/sdam/srv_polling/test.rs | 62 +++++++++++++++--------------------- 2 files changed, 30 insertions(+), 38 deletions(-) diff --git a/src/sdam/srv_polling.rs b/src/sdam/srv_polling.rs index bd4782000..cbdf71a4f 100644 --- a/src/sdam/srv_polling.rs +++ b/src/sdam/srv_polling.rs @@ -62,7 +62,11 @@ impl SrvPollingMonitor { } fn rescan_interval(&self) -> Duration { - std::cmp::max(self.rescan_interval, MIN_RESCAN_SRV_INTERVAL) + if cfg!(test) { + self.rescan_interval + } else { + std::cmp::max(self.rescan_interval, MIN_RESCAN_SRV_INTERVAL) + } } async fn execute(mut self) { diff --git a/src/sdam/srv_polling/test.rs b/src/sdam/srv_polling/test.rs index a0fba1404..c77d891ab 100644 --- a/src/sdam/srv_polling/test.rs +++ b/src/sdam/srv_polling/test.rs @@ -26,29 +26,26 @@ static DEFAULT_HOSTS: Lazy> = Lazy::new(|| { }); async fn run_test(new_hosts: Result>, expected_hosts: HashSet) { - run_test_srv(None, new_hosts, expected_hosts, None).await + run_test_srv(None, new_hosts, expected_hosts).await } async fn run_test_srv( max_hosts: Option, new_hosts: Result>, expected_hosts: HashSet, - srv_service_name: Option, ) { - let actual = run_test_extra(max_hosts, new_hosts, srv_service_name).await; + let actual = run_test_extra(max_hosts, new_hosts).await; assert_eq!(expected_hosts, actual); } async fn run_test_extra( max_hosts: Option, new_hosts: Result>, - srv_service_name: Option, ) -> HashSet { let mut options = ClientOptions::new_srv(); options.hosts.clone_from(&DEFAULT_HOSTS); options.test_options_mut().disable_monitoring_threads = true; options.srv_max_hosts = max_hosts; - options.srv_service_name = srv_service_name; let mut topology = Topology::new(options.clone()).unwrap(); topology.watch().wait_until_initialized().await; let mut monitor = @@ -159,20 +156,8 @@ async fn srv_max_hosts_zero() { localhost_test_build_10gen(27020), ]; - run_test_srv( - None, - Ok(hosts.clone()), - hosts.clone().into_iter().collect(), - None, - ) - .await; - run_test_srv( - Some(0), - Ok(hosts.clone()), - hosts.into_iter().collect(), - None, - ) - .await; + run_test_srv(None, Ok(hosts.clone()), hosts.clone().into_iter().collect()).await; + run_test_srv(Some(0), Ok(hosts.clone()), hosts.into_iter().collect()).await; } // SRV polling with srvMaxHosts MongoClient option: All DNS records are selected (srvMaxHosts >= @@ -184,13 +169,7 @@ async fn srv_max_hosts_gt_actual() { localhost_test_build_10gen(27020), ]; - run_test_srv( - Some(2), - Ok(hosts.clone()), - hosts.into_iter().collect(), - None, - ) - .await; + run_test_srv(Some(2), Ok(hosts.clone()), hosts.into_iter().collect()).await; } // SRV polling with srvMaxHosts MongoClient option: New DNS records are randomly selected @@ -203,22 +182,31 @@ async fn srv_max_hosts_random() { localhost_test_build_10gen(27020), ]; - let actual = run_test_extra(Some(2), Ok(hosts), None).await; + let actual = run_test_extra(Some(2), Ok(hosts)).await; assert_eq!(2, actual.len()); assert!(actual.contains(&localhost_test_build_10gen(27017))); } #[tokio::test] async fn srv_service_name() { - let hosts = vec![ - localhost_test_build_10gen(27019), - localhost_test_build_10gen(27020), + let rescan_interval = Duration::from_secs(1); + let new_hosts = vec![ + ServerAddress::Tcp { + host: "localhost.test.build.10gen.cc".to_string(), + port: Some(27019), + }, + ServerAddress::Tcp { + host: "localhost.test.build.10gen.cc".to_string(), + port: Some(27020), + }, ]; - run_test_srv( - None, - Ok(hosts.clone()), - hosts.into_iter().collect(), - Some("customname".to_string()), - ) - .await; + let uri = "mongodb+srv://test22.test.build.10gen.cc/?srvServiceName=customname"; + let mut options = ClientOptions::parse(uri).await.unwrap(); + // override the min_ttl to speed up lookup interval + options.original_srv_info.as_mut().unwrap().min_ttl = rescan_interval; + options.test_options_mut().mock_lookup_hosts = Some(make_lookup_hosts(new_hosts.clone())); + let mut topology = Topology::new(options).unwrap(); + topology.watch().wait_until_initialized().await; + tokio::time::sleep(rescan_interval * 2).await; + assert_eq!(topology.server_addresses(), new_hosts.into_iter().collect()); }