Skip to content
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

feat(NODE-3467): implement srvMaxHosts, srvServiceName options #3031

Merged
merged 30 commits into from
Nov 16, 2021
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
ce4d7e4
feat(NODE-3467): implement srvMaxHosts, srvServiceName, and rescanSrv…
nbbeeken Nov 8, 2021
6321246
fix: unit tests
nbbeeken Nov 9, 2021
52b3377
fix: integration tests
nbbeeken Nov 9, 2021
676f8f3
wip
nbbeeken Nov 9, 2021
7879773
test: fix up shuffle unit tests
nbbeeken Nov 9, 2021
1d09e18
fix: shuffle tests round 2 undo srv event saving
nbbeeken Nov 10, 2021
f7c304d
Apply suggestions from code review
nbbeeken Nov 10, 2021
eca1ee4
test: remove dupe test
nbbeeken Nov 10, 2021
471dc61
fix: remove unused equals method
nbbeeken Nov 10, 2021
0b35275
docs: improve limit description
nbbeeken Nov 10, 2021
5daa3ce
fix: lint
nbbeeken Nov 10, 2021
382ad4d
fix: remove rescan option and drop TXT record option logic
nbbeeken Nov 10, 2021
852fca3
fix: permit new options only on srv connection strings
nbbeeken Nov 10, 2021
28567f8
fix: address comments, fix option parsing errors, test naming
nbbeeken Nov 11, 2021
00a39d1
fix: LB connection string assertion
nbbeeken Nov 11, 2021
12a72ab
feat: super algorithm enhancements O(-1) speeds
nbbeeken Nov 11, 2021
5fa6550
fix: shuffle lowerBound logic, test for srvServiceName length error
nbbeeken Nov 12, 2021
537ec45
or -> nor
nbbeeken Nov 12, 2021
0db7bb8
fix: address comments except for connection_string tests
nbbeeken Nov 15, 2021
42a0942
fix: whoops broke host gathering, fixed now
nbbeeken Nov 15, 2021
0db9494
suggestions!
nbbeeken Nov 15, 2021
60e3c75
move tests into correct places, update initial seed list testing WIP
nbbeeken Nov 15, 2021
34bc0d4
add ticket mention
nbbeeken Nov 15, 2021
beb74c5
clarify records
nbbeeken Nov 15, 2021
0e45319
remove comment
nbbeeken Nov 15, 2021
d9e8f28
prevent mutation
nbbeeken Nov 15, 2021
eec372e
test: add object option test, clean up assertions
nbbeeken Nov 16, 2021
7465308
call makeStubs first in test 13
nbbeeken Nov 16, 2021
ad521aa
fix: check for nullish srvMaxHosts
nbbeeken Nov 16, 2021
09b30d6
fix: increase the input size
nbbeeken Nov 16, 2021
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
2 changes: 1 addition & 1 deletion .mocharc.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
"ts"
],
"require": [
"ts-node/register",
"source-map-support/register",
"ts-node/register",
"test/tools/runner/chai-addons",
"test/tools/runner/circular-dep-hack"
],
Expand Down
57 changes: 48 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
"@types/node": "^16.10.3",
"@types/saslprep": "^1.0.1",
"@types/semver": "^7.3.8",
"@types/sinon": "^10.0.6",
"@types/whatwg-url": "^8.2.1",
"@typescript-eslint/eslint-plugin": "^4.33.0",
"@typescript-eslint/parser": "^4.33.0",
Expand All @@ -69,7 +70,7 @@
"prettier": "^2.4.1",
"rimraf": "^3.0.2",
"semver": "^7.3.5",
"sinon": "^11.1.2",
"sinon": "^12.0.1",
"sinon-chai": "^3.7.0",
"source-map-support": "^0.5.20",
"standard-version": "^9.3.1",
Expand Down
89 changes: 71 additions & 18 deletions src/connection_string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export function resolveSRVRecord(options: MongoOptions, callback: Callback<HostA

// Resolve the SRV record and use the result as the list of hosts to connect to.
const lookupAddress = options.srvHost;
dns.resolveSrv(`_mongodb._tcp.${lookupAddress}`, (err, addresses) => {
dns.resolveSrv(`_${options.srvServiceName}._tcp.${lookupAddress}`, (err, addresses) => {
if (err) return callback(err);

if (addresses.length === 0) {
Expand Down Expand Up @@ -116,14 +116,14 @@ export function resolveSRVRecord(options: MongoOptions, callback: Callback<HostA
);
}

if (VALID_TXT_RECORDS.some(option => txtRecordOptions.get(option) === '')) {
return callback(new MongoParseError('Cannot have empty URI params in DNS TXT Record'));
}

const source = txtRecordOptions.get('authSource') ?? undefined;
const replicaSet = txtRecordOptions.get('replicaSet') ?? undefined;
const loadBalanced = txtRecordOptions.get('loadBalanced') ?? undefined;

if (source === '' || replicaSet === '') {
return callback(new MongoParseError('Cannot have empty URI params in DNS TXT Record'));
}

if (!options.userSpecifiedAuthSource && source) {
options.credentials = MongoCredentials.merge(options.credentials, { source });
}
Expand All @@ -136,6 +136,10 @@ export function resolveSRVRecord(options: MongoOptions, callback: Callback<HostA
options.loadBalanced = true;
}

if (options.replicaSet && options.srvMaxHosts > 0) {
return callback(new MongoParseError('Cannot combine replicaSet option with srvMaxHosts'));
}

const lbError = validateLoadBalancedOptions(hostAddresses, options);
if (lbError) {
return callback(lbError);
Expand Down Expand Up @@ -251,13 +255,6 @@ export function parseOptions(

const mongoOptions = Object.create(null);
mongoOptions.hosts = isSRV ? [] : hosts.map(HostAddress.fromString);
if (isSRV) {
// SRV Record is resolved upon connecting
mongoOptions.srvHost = hosts[0];
if (!url.searchParams.has('tls') && !url.searchParams.has('ssl')) {
options.tls = true;
}
}

const urlOptions = new CaseInsensitiveMap();

Expand Down Expand Up @@ -289,12 +286,6 @@ export function parseOptions(
throw new MongoAPIError('URI cannot contain options with no value');
}

if (key.toLowerCase() === 'serverapi') {
throw new MongoParseError(
'URI cannot contain `serverApi`, it can only be passed to the client'
);
}

if (key.toLowerCase() === 'authsource' && urlOptions.has('authSource')) {
// If authSource is an explicit key in the urlOptions we need to remove the implicit dbName
urlOptions.delete('authSource');
dariakp marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -309,10 +300,58 @@ export function parseOptions(
Object.entries(options).filter(([, v]) => v != null)
);

// Validate options that can only be provided by one of uri or object

if (urlOptions.has('serverApi')) {
throw new MongoParseError(
'URI cannot contain `serverApi`, it can only be passed to the client'
);
}

if (objectOptions.has('loadBalanced')) {
throw new MongoParseError('loadBalanced is only a valid option in the URI');
}

// SRV connection string validations

const nonZeroSrvMaxHosts =
objectOptions.get('srvMaxHosts') > 0 || urlOptions.get('srvMaxHosts') > 0;
dariakp marked this conversation as resolved.
Show resolved Hide resolved

const srvSpecificOptionsSpecified =
objectOptions.has('srvMaxHosts') ||
urlOptions.has('srvMaxHosts') ||
objectOptions.has('srvServiceName') ||
urlOptions.has('srvServiceName');

if (isSRV) {
dariakp marked this conversation as resolved.
Show resolved Hide resolved
// SRV Record is resolved upon connecting
mongoOptions.srvHost = hosts[0];
// SRV turns on TLS by default, but users can override and turn it off
const noUserSpecifiedTLS = !objectOptions.has('tls') && !urlOptions.has('tls');
const noUserSpecifiedSSL = !objectOptions.has('ssl') && !urlOptions.has('ssl');
if (noUserSpecifiedTLS && noUserSpecifiedSSL) {
objectOptions.set('tls', true);
objectOptions.set('ssl', true);
}
} else {
if (srvSpecificOptionsSpecified) {
throw new MongoParseError(
'Cannot use maxSrvHosts nor srvServiceName with a non-srv connection string'
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
);
}
}

if (nonZeroSrvMaxHosts) {
if (urlOptions.get('loadBalanced') === true) {
throw new MongoParseError('Cannot use srvMaxHosts option with loadBalanced');
dariakp marked this conversation as resolved.
Show resolved Hide resolved
}
if (urlOptions.has('replicaSet') || objectOptions.has('replicaSet')) {
throw new MongoParseError('Cannot use srvMaxHosts option with replicaSet');
}
}

// All option collection

const allOptions = new CaseInsensitiveMap();

const allKeys = new Set<string>([
Expand Down Expand Up @@ -360,6 +399,8 @@ export function parseOptions(
);
}

// Option parsing and setting

for (const [key, descriptor] of Object.entries(OPTIONS)) {
const values = allOptions.get(key);
if (!values || values.length === 0) continue;
Expand Down Expand Up @@ -439,6 +480,10 @@ function validateLoadBalancedOptions(
if (mongoOptions.directConnection) {
return new MongoParseError(LB_DIRECT_CONNECTION_ERROR);
}

if (mongoOptions.srvHost && mongoOptions.srvMaxHosts > 0) {
return new MongoParseError('Cannot limit srv hosts with loadBalanced enabled');
}
}
}

Expand Down Expand Up @@ -924,6 +969,14 @@ export const OPTIONS = {
default: 0,
type: 'uint'
},
srvMaxHosts: {
type: 'uint',
default: 0
},
srvServiceName: {
type: 'string',
default: 'mongodb'
dariakp marked this conversation as resolved.
Show resolved Hide resolved
},
ssl: {
target: 'tls',
type: 'boolean'
Expand Down
12 changes: 12 additions & 0 deletions src/mongo_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,16 @@ export interface MongoClientOptions extends BSONSerializeOptions, SupportedNodeC
compressors?: CompressorName[] | string;
/** An integer that specifies the compression level if using zlib for network compression. */
zlibCompressionLevel?: 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | undefined;
/** The maximum number of hosts to connect to when using an srv connection string, a setting of `0` means unlimited hosts */
srvMaxHosts?: number;
/**
* Modifies the srv URI to look like:
*
* `_{srvServiceName}._tcp.{hostname}.{domainname}`
*
* Querying this DNS URI is expected to respond with SRV records
*/
srvServiceName?: string;
/** The maximum number of connections in the connection pool. */
maxPoolSize?: number;
/** The minimum number of connections in the connection pool. */
Expand Down Expand Up @@ -643,6 +653,8 @@ export interface MongoOptions
| 'retryWrites'
| 'serverSelectionTimeoutMS'
| 'socketTimeoutMS'
| 'srvMaxHosts'
| 'srvServiceName'
| 'tlsAllowInvalidCertificates'
| 'tlsAllowInvalidHostnames'
| 'tlsInsecure'
Expand Down
10 changes: 8 additions & 2 deletions src/operations/connect.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { MongoRuntimeError, MongoInvalidArgumentError } from '../error';
import { Topology, TOPOLOGY_EVENTS } from '../sdam/topology';
import { resolveSRVRecord } from '../connection_string';
import type { Callback } from '../utils';
import { Callback, shuffle } from '../utils';
import type { MongoClient, MongoOptions } from '../mongo_client';
import { CMAP_EVENTS } from '../cmap/connection_pool';
import { APM_EVENTS } from '../cmap/connection';
Expand Down Expand Up @@ -51,7 +51,13 @@ export function connect(
if (typeof options.srvHost === 'string') {
return resolveSRVRecord(options, (err, hosts) => {
if (err || !hosts) return callback(err);
for (const [index, host] of hosts.entries()) {

const selectedHosts =
options.srvMaxHosts === 0 || options.srvMaxHosts >= hosts.length
? hosts
: shuffle(hosts, options.srvMaxHosts);

for (const [index, host] of selectedHosts.entries()) {
options.hosts[index] = host;
}

Expand Down
Loading