Skip to content

RUST-911: Add srvServiceName URI option #1235

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

Merged
merged 7 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion src/client/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -521,6 +522,9 @@ pub struct ClientOptions {
/// By default, no default database is specified.
pub default_database: Option<String>,

/// Overrides the default "mongodb" service name for SRV lookup in both discovery and polling
pub srv_service_name: Option<String>,

#[builder(setter(skip))]
#[derivative(Debug = "ignore")]
pub(crate) socket_timeout: Option<Duration>,
Expand Down Expand Up @@ -676,6 +680,8 @@ impl Serialize for ClientOptions {
loadbalanced: &'a Option<bool>,

srvmaxhosts: Option<i32>,

srvservicename: &'a Option<String>,
}

let client_options = ClientOptionsHelper {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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<u32>,

/// Overrides the default "mongodb" service name for SRV lookup in both discovery and polling
pub srv_service_name: Option<String>,

wait_queue_timeout: Option<Duration>,
tls_insecure: Option<bool>,

Expand Down Expand Up @@ -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 }
}
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);

Expand Down
1 change: 1 addition & 0 deletions src/client/options/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
}
6 changes: 5 additions & 1 deletion src/sdam/srv_polling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
27 changes: 23 additions & 4 deletions src/srv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -90,14 +94,21 @@ pub(crate) enum DomainMismatch {
#[cfg(feature = "dns-resolver")]
pub(crate) struct SrvResolver {
resolver: crate::runtime::AsyncResolver,
client_options: Option<ClientOptions>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest only storing the service name rather than the entire options struct - it looks like that's the only field being accessed from this

}

#[cfg(feature = "dns-resolver")]
impl SrvResolver {
pub(crate) async fn new(config: Option<ResolverConfig>) -> Result<Self> {
pub(crate) async fn new(
config: Option<ResolverConfig>,
client_options: Option<ClientOptions>,
) -> Result<Self> {
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(
Expand Down Expand Up @@ -149,7 +160,15 @@ impl SrvResolver {
original_hostname: &str,
dm: DomainMismatch,
) -> Result<LookupHosts> {
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)
Expand Down
182 changes: 182 additions & 0 deletions src/test/spec/json/initial-dns-seedlist-discovery/README.md
Original file line number Diff line number Diff line change
@@ -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.
Loading