Skip to content

Commit 68074f9

Browse files
authored
test(NODE-4152): make connect auth check configurable (#3202)
1 parent 0ec03bf commit 68074f9

File tree

11 files changed

+182
-167
lines changed

11 files changed

+182
-167
lines changed

.evergreen/config.yml

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@ stepback: true
22
command_type: system
33
exec_timeout_secs: 1200
44
timeout:
5-
- command: shell.exec
5+
- command: subprocess.exec
66
params:
7-
script: |
8-
ls -la
7+
binary: ls
8+
args:
9+
- '-la'
910
functions:
1011
fetch source:
1112
- command: git.get_project
@@ -215,6 +216,7 @@ functions:
215216
bash ${PROJECT_DIRECTORY}/.evergreen/run-tests.sh
216217
run lint checks:
217218
- command: subprocess.exec
219+
type: test
218220
params:
219221
working_dir: src
220222
timeout_secs: 60
@@ -225,6 +227,7 @@ functions:
225227
- ${PROJECT_DIRECTORY}/.evergreen/run-lint-checks.sh
226228
run unit tests:
227229
- command: subprocess.exec
230+
type: test
228231
params:
229232
working_dir: src
230233
timeout_secs: 60
@@ -235,6 +238,7 @@ functions:
235238
- ${PROJECT_DIRECTORY}/.evergreen/run-unit-tests.sh
236239
run typescript next:
237240
- command: subprocess.exec
241+
type: test
238242
params:
239243
working_dir: src
240244
timeout_secs: 60
@@ -246,6 +250,7 @@ functions:
246250
- ${PROJECT_DIRECTORY}/.evergreen/run-typescript.sh
247251
run typescript oldest:
248252
- command: subprocess.exec
253+
type: test
249254
params:
250255
working_dir: src
251256
timeout_secs: 60
@@ -258,6 +263,7 @@ functions:
258263
- ${PROJECT_DIRECTORY}/.evergreen/run-typescript.sh
259264
run typescript current:
260265
- command: subprocess.exec
266+
type: test
261267
params:
262268
working_dir: src
263269
timeout_secs: 60

.evergreen/config.yml.in

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,11 @@ exec_timeout_secs: 1200
1515

1616
# What to do when evergreen hits the timeout (`post:` tasks are run automatically)
1717
timeout:
18-
- command: shell.exec
18+
- command: subprocess.exec
1919
params:
20-
script: |
21-
ls -la
20+
binary: ls
21+
args:
22+
- "-la"
2223

2324
functions:
2425
"fetch source":
@@ -240,6 +241,7 @@ functions:
240241

241242
"run lint checks":
242243
- command: subprocess.exec
244+
type: test
243245
params:
244246
working_dir: "src"
245247
timeout_secs: 60
@@ -251,6 +253,7 @@ functions:
251253

252254
"run unit tests":
253255
- command: subprocess.exec
256+
type: test
254257
params:
255258
working_dir: "src"
256259
timeout_secs: 60
@@ -262,6 +265,7 @@ functions:
262265

263266
"run typescript next":
264267
- command: subprocess.exec
268+
type: test
265269
params:
266270
working_dir: "src"
267271
timeout_secs: 60
@@ -274,6 +278,7 @@ functions:
274278

275279
"run typescript oldest":
276280
- command: subprocess.exec
281+
type: test
277282
params:
278283
working_dir: "src"
279284
timeout_secs: 60
@@ -287,6 +292,7 @@ functions:
287292

288293
"run typescript current":
289294
- command: subprocess.exec
295+
type: test
290296
params:
291297
working_dir: "src"
292298
timeout_secs: 60

src/connection_string.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,14 @@ export function parseOptions(
263263
const { hosts, isSRV } = url;
264264

265265
const mongoOptions = Object.create(null);
266+
267+
// Feature flags
268+
for (const flag of Object.getOwnPropertySymbols(options)) {
269+
if (FEATURE_FLAGS.has(flag)) {
270+
mongoOptions[flag] = options[flag];
271+
}
272+
}
273+
266274
mongoOptions.hosts = isSRV ? [] : hosts.map(HostAddress.fromString);
267275

268276
const urlOptions = new CaseInsensitiveMap<any[]>();
@@ -1241,3 +1249,9 @@ export const DEFAULT_OPTIONS = new CaseInsensitiveMap(
12411249
.filter(([, descriptor]) => descriptor.default != null)
12421250
.map(([k, d]) => [k, d.default])
12431251
);
1252+
1253+
/**
1254+
* Set of permitted feature flags
1255+
* @internal
1256+
*/
1257+
export const FEATURE_FLAGS = new Set([Symbol.for('@@mdb.skipPingOnConnect')]);

src/mongo_client.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,9 @@ export interface MongoClientOptions extends BSONSerializeOptions, SupportedNodeC
254254
srvPoller?: SrvPoller;
255255
/** @internal */
256256
connectionType?: typeof Connection;
257+
258+
/** @internal */
259+
[featureFlag: symbol]: any;
257260
}
258261

259262
/** @public */
@@ -696,8 +699,6 @@ export interface MongoOptions
696699
*/
697700
tls: boolean;
698701

699-
/**
700-
* Turn these options into a reusable connection URI
701-
*/
702-
toURI(): string;
702+
/** @internal */
703+
[featureFlag: symbol]: any;
703704
}

src/operations/execute_operation.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ export function executeOperation<
9595
return maybePromise(callback, callback => {
9696
let topology: Topology;
9797
try {
98+
// TODO(NODE-4151): Use skipPingOnConnect and call connect here to make client.connect optional
9899
topology = getTopology(topologyProvider);
99100
} catch (error) {
100101
return callback(error);

src/sdam/topology.ts

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { deserialize, serialize } from '../bson';
44
import type { MongoCredentials } from '../cmap/auth/mongo_credentials';
55
import type { ConnectionEvents, DestroyOptions } from '../cmap/connection';
66
import type { CloseOptions, ConnectionPoolEvents } from '../cmap/connection_pool';
7-
import { DEFAULT_OPTIONS } from '../connection_string';
7+
import { DEFAULT_OPTIONS, FEATURE_FLAGS } from '../connection_string';
88
import {
99
CLOSE,
1010
CONNECT,
@@ -153,6 +153,8 @@ export interface TopologyOptions extends BSONSerializeOptions, ServerOptions {
153153
metadata: ClientMetadata;
154154
/** MongoDB server API version */
155155
serverApi?: ServerApi;
156+
/** @internal */
157+
[featureFlag: symbol]: any;
156158
}
157159

158160
/** @public */
@@ -248,22 +250,8 @@ export class Topology extends TypedEventEmitter<TopologyEvents> {
248250
// Options should only be undefined in tests, MongoClient will always have defined options
249251
options = options ?? {
250252
hosts: [HostAddress.fromString('localhost:27017')],
251-
retryReads: DEFAULT_OPTIONS.get('retryReads'),
252-
retryWrites: DEFAULT_OPTIONS.get('retryWrites'),
253-
serverSelectionTimeoutMS: DEFAULT_OPTIONS.get('serverSelectionTimeoutMS'),
254-
directConnection: DEFAULT_OPTIONS.get('directConnection'),
255-
loadBalanced: DEFAULT_OPTIONS.get('loadBalanced'),
256-
metadata: DEFAULT_OPTIONS.get('metadata'),
257-
monitorCommands: DEFAULT_OPTIONS.get('monitorCommands'),
258-
tls: DEFAULT_OPTIONS.get('tls'),
259-
maxPoolSize: DEFAULT_OPTIONS.get('maxPoolSize'),
260-
minPoolSize: DEFAULT_OPTIONS.get('minPoolSize'),
261-
waitQueueTimeoutMS: DEFAULT_OPTIONS.get('waitQueueTimeoutMS'),
262-
connectionType: DEFAULT_OPTIONS.get('connectionType'),
263-
connectTimeoutMS: DEFAULT_OPTIONS.get('connectTimeoutMS'),
264-
maxIdleTimeMS: DEFAULT_OPTIONS.get('maxIdleTimeMS'),
265-
heartbeatFrequencyMS: DEFAULT_OPTIONS.get('heartbeatFrequencyMS'),
266-
minHeartbeatFrequencyMS: DEFAULT_OPTIONS.get('minHeartbeatFrequencyMS')
253+
...Object.fromEntries(DEFAULT_OPTIONS.entries()),
254+
...Object.fromEntries(FEATURE_FLAGS.entries())
267255
};
268256

269257
if (typeof seeds === 'string') {
@@ -333,7 +321,6 @@ export class Topology extends TypedEventEmitter<TopologyEvents> {
333321

334322
// timer management
335323
connectionTimers: new Set<NodeJS.Timeout>(),
336-
337324
detectShardedTopology: ev => this.detectShardedTopology(ev),
338325
detectSrvRecords: ev => this.detectSrvRecords(ev)
339326
};
@@ -462,7 +449,8 @@ export class Topology extends TypedEventEmitter<TopologyEvents> {
462449
}
463450

464451
// TODO: NODE-2471
465-
if (server && this.s.credentials) {
452+
const skipPingOnConnect = this.s.options[Symbol.for('@@mdb.skipPingOnConnect')] === true;
453+
if (!skipPingOnConnect && server && this.s.credentials) {
466454
server.command(ns('admin.$cmd'), { ping: 1 }, {}, err => {
467455
if (err) {
468456
typeof callback === 'function' ? callback(err) : this.emit(Topology.ERROR, err);
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { expect } from 'chai';
2+
3+
describe('Feature Flags', () => {
4+
describe('@@mdb.skipPingOnConnect', () => {
5+
beforeEach(function () {
6+
if (process.env.AUTH !== 'auth') {
7+
this.currentTest.skipReason = 'ping count relies on auth to be enabled';
8+
this.skip();
9+
}
10+
});
11+
12+
const tests = [
13+
// only skipInitiaPing=true will have no events upon connect
14+
{ description: 'should skip ping command when set to true', value: true, expectEvents: 0 },
15+
{
16+
description: 'should not skip ping command when set to false',
17+
value: false,
18+
expectEvents: 1
19+
},
20+
{ description: 'should not skip ping command when unset', value: undefined, expectEvents: 1 }
21+
];
22+
for (const { description, value, expectEvents } of tests) {
23+
it(description, async function () {
24+
const options =
25+
value === undefined ? {} : { [Symbol.for('@@mdb.skipPingOnConnect')]: value };
26+
const client = this.configuration.newClient({}, { ...options, monitorCommands: true });
27+
const events = [];
28+
client.on('commandStarted', event => events.push(event));
29+
30+
try {
31+
await client.connect();
32+
} finally {
33+
await client.close();
34+
}
35+
36+
expect(events).to.have.lengthOf(expectEvents);
37+
if (expectEvents > 1) {
38+
for (const event of events) {
39+
expect(event).to.have.property('commandName', 'ping');
40+
}
41+
}
42+
});
43+
}
44+
});
45+
});

test/tools/spec-runner/index.js

Lines changed: 9 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -339,43 +339,24 @@ const CMAP_EVENTS = new Set([
339339
'connectionPoolCleared'
340340
]);
341341

342-
let displayCommands = false;
343342
function runTestSuiteTest(configuration, spec, context) {
344343
context.commandEvents = [];
345-
const clientOptions = translateClientOptions(
346-
Object.assign(
347-
{
348-
heartbeatFrequencyMS: 100,
349-
minHeartbeatFrequencyMS: 100,
350-
monitorCommands: true
351-
},
352-
spec.clientOptions
353-
)
354-
);
344+
const clientOptions = translateClientOptions({
345+
heartbeatFrequencyMS: 100,
346+
minHeartbeatFrequencyMS: 100,
347+
monitorCommands: true,
348+
...spec.clientOptions,
349+
[Symbol.for('@@mdb.skipPingOnConnect')]: true
350+
});
355351

356352
const url = resolveConnectionString(configuration, spec, context);
357353
const client = configuration.newClient(url, clientOptions);
358354
CMAP_EVENTS.forEach(eventName => client.on(eventName, event => context.cmapEvents.push(event)));
359355
SDAM_EVENTS.forEach(eventName => client.on(eventName, event => context.sdamEvents.push(event)));
360356

361-
let skippedInitialPing = false;
362357
client.on('commandStarted', event => {
363-
if (IGNORED_COMMANDS.has(event.commandName)) {
364-
return;
365-
}
366-
367-
// If credentials were provided, then the Topology sends an initial `ping` command
368-
// that we want to skip
369-
if (event.commandName === 'ping' && client.topology.s.credentials && !skippedInitialPing) {
370-
skippedInitialPing = true;
371-
return;
372-
}
373-
374-
context.commandEvents.push(event);
375-
376-
// very useful for debugging
377-
if (displayCommands) {
378-
// console.dir(event, { depth: 5 });
358+
if (!IGNORED_COMMANDS.has(event.commandName)) {
359+
context.commandEvents.push(event);
379360
}
380361
});
381362

@@ -406,8 +387,6 @@ function runTestSuiteTest(configuration, spec, context) {
406387
// ignore
407388
}
408389
}
409-
// enable to see useful APM debug information at the time of actual test run
410-
// displayCommands = true;
411390

412391
const operationContext = {
413392
client,

0 commit comments

Comments
 (0)