From f550abb5ebda38e93dc5fbf9f2d946677dacb468 Mon Sep 17 00:00:00 2001 From: Anemy Date: Tue, 15 Dec 2020 12:15:32 -0500 Subject: [PATCH 1/4] Explicitly pass read preference with db.command commands --- lib/instance-detail-helper.js | 43 +++++++++++++--------- lib/native-client.js | 6 ++- test/instance-detail-helper-mocked.test.js | 38 +++++++++++++++++++ 3 files changed, 69 insertions(+), 18 deletions(-) diff --git a/lib/instance-detail-helper.js b/lib/instance-detail-helper.js index dc8b229f..82d48b9e 100644 --- a/lib/instance-detail-helper.js +++ b/lib/instance-detail-helper.js @@ -20,6 +20,13 @@ const { const debug = require('debug')('mongodb-data-service:instance-detail-helper'); +function getReadPreferenceOptions(db) { + // `db.command` does not use the read preference set on the + // connection, so here we explicitly to specify it in the options. + const readPreference = db.s ? db.s.readPreference : ReadPreference.PRIMARY; + return { readPreference }; +} + /** * aggregates stats across all found databases * @param {Object} results async.auto results @@ -78,7 +85,7 @@ function getBuildInfo(results, done) { }; const adminDb = db.databaseName === 'admin' ? db : db.admin(); - adminDb.command(spec, {}, function(err, res) { + adminDb.command(spec, getReadPreferenceOptions(db), function(err, res) { if (err) { // buildInfo doesn't require any privileges to run, so if it fails, // something really went wrong and we should return the error. @@ -98,7 +105,7 @@ function getCmdLineOpts(results, done) { }; const adminDb = db.databaseName === 'admin' ? db : db.admin(); - adminDb.command(spec, {}, function(err, res) { + adminDb.command(spec, getReadPreferenceOptions(db), function(err, res) { if (err) { debug('getCmdLineOpts failed!', err); return done(null, { errmsg: err.message }); @@ -213,7 +220,7 @@ function getHostInfo(results, done) { }; const adminDb = db.databaseName === 'admin' ? db : db.admin(); - adminDb.command(spec, {}, function(err, res) { + adminDb.command(spec, getReadPreferenceOptions(db), function(err, res) { if (err) { if (isNotAuthorized(err)) { // if the error is that the user is not authorized, silently ignore it @@ -253,12 +260,14 @@ function listDatabases(results, done) { listDatabases: 1 }; + const options = getReadPreferenceOptions(db); + const adminDb = db.databaseName === 'admin' ? db : db.admin(); - adminDb.command(spec, {}, function(err, res) { + adminDb.command(spec, options, function(err, res) { if (err) { if (isNotAuthorized(err)) { // eslint-disable-next-line no-shadow - adminDb.command({connectionStatus: 1, showPrivileges: 1}, {}, function(err, res) { + adminDb.command({connectionStatus: 1, showPrivileges: 1}, options, function(err, res) { if (err) { done(err); return; @@ -343,7 +352,7 @@ function getDatabase(client, db, name, done) { dbStats: 1 }; - const options = {}; + const options = getReadPreferenceOptions(db); debug('running dbStats for `%s`...', name); client.db(name).command(spec, options, function(err, res) { if (err) { @@ -376,8 +385,16 @@ function getDatabases(results, done) { function getUserInfo(results, done) { const db = results.db; + const rp = db.s ? db.s.readPreference : ReadPreference.PRIMARY; + const options = { readPreference: rp }; + // get the user privileges - db.command({ connectionStatus: 1, showPrivileges: true }, {}, function( + db.command({ + connectionStatus: 1, + showPrivileges: true + }, + options, + function( err, res ) { @@ -394,7 +411,7 @@ function getUserInfo(results, done) { } const user = res.authInfo.authenticatedUsers[0]; - db.command({ usersInfo: user, showPrivileges: true }, {}, function( + db.command({ usersInfo: user, showPrivileges: true }, options, function( _err, _res ) { @@ -491,15 +508,7 @@ function getDatabaseCollections(db, done) { const spec = {}; - /** - * @note: Durran: For some reason the listCollections call does not take into - * account the read preference that was set on the db instance - it only looks - * in the passed options: https://github.com/mongodb/node-mongodb-native/blob/2.2/lib/db.js#L671 - */ - const rp = db.s ? db.s.readPreference : ReadPreference.PRIMARY; - const options = { readPreference: rp }; - - db.listCollections(spec, options).toArray(function(err, res) { + db.listCollections(spec, getReadPreferenceOptions(db)).toArray(function(err, res) { if (err) { if (isNotAuthorized(err) || isMongosLocalException(err)) { // if the error is that the user is not authorized, silently ignore it diff --git a/lib/native-client.js b/lib/native-client.js index 235ff202..3c53d039 100644 --- a/lib/native-client.js +++ b/lib/native-client.js @@ -267,7 +267,11 @@ class NativeClient extends EventEmitter { * @param {Function} callback - The callback. */ listDatabases(callback) { - this.database.admin().command({ listDatabases: 1 }, {}, (error, result) => { + this.database.admin().command({ + listDatabases: 1 + }, { + readPreference: this.model.readPreference + }, (error, result) => { if (error) { return callback(this._translateMessage(error)); } diff --git a/test/instance-detail-helper-mocked.test.js b/test/instance-detail-helper-mocked.test.js index 2bbd4e55..d6d236b6 100644 --- a/test/instance-detail-helper-mocked.test.js +++ b/test/instance-detail-helper-mocked.test.js @@ -1,4 +1,5 @@ const assert = require('assert'); +const { ReadPreference } = require('mongodb'); const { getAllowedCollections, getAllowedDatabases, @@ -383,6 +384,43 @@ describe('instance-detail-helper-mocked', function() { done(); }); }); + it('should pass the read preference explicity when it is set', function(done) { + let passedOptions; + results.db = makeMockDB(function(times, command, options, callback) { + if (command.listDatabases) { + passedOptions = options; + return callback(null, { databases: [] }); + } + return callback(null, {}); + }); + results.db.s = { + readPreference: ReadPreference.SECONDARY + }; + + listDatabases(results, function() { + assert.deepEqual(passedOptions, { + readPreference: 'secondary' + }); + done(); + }); + }); + it('defaults to passing the read preference as PRIMARY', function(done) { + let passedOptions; + results.db = makeMockDB(function(times, command, options, callback) { + if (command.listDatabases) { + passedOptions = options; + return callback(null, { databases: [] }); + } + return callback(null, {}); + }); + + listDatabases(results, function() { + assert.deepEqual(passedOptions, { + readPreference: ReadPreference.PRIMARY + }); + done(); + }); + }); it('should pass on other errors from the listDatabases command', function(done) { // instead of the real db handle, pass in the mocked one results.db = makeMockDB( From 9114386362cccc589da5e2603c1ddd975ecb2cb0 Mon Sep 17 00:00:00 2001 From: Anemy Date: Tue, 15 Dec 2020 12:36:34 -0500 Subject: [PATCH 2/4] Remove duplicated read preference option creation --- lib/instance-detail-helper.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/instance-detail-helper.js b/lib/instance-detail-helper.js index 82d48b9e..3d3f8d7d 100644 --- a/lib/instance-detail-helper.js +++ b/lib/instance-detail-helper.js @@ -385,8 +385,7 @@ function getDatabases(results, done) { function getUserInfo(results, done) { const db = results.db; - const rp = db.s ? db.s.readPreference : ReadPreference.PRIMARY; - const options = { readPreference: rp }; + const options = getReadPreferenceOptions(db); // get the user privileges db.command({ From 8c0dba99e1f436c7abf749c2d8555a5881389b2f Mon Sep 17 00:00:00 2001 From: Anemy Date: Tue, 15 Dec 2020 14:29:32 -0500 Subject: [PATCH 3/4] Bump connection model and better peer deps --- package-lock.json | 7 +++---- package.json | 6 +++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/package-lock.json b/package-lock.json index 31bda618..7b5abaed 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4377,9 +4377,9 @@ } }, "mongodb-connection-model": { - "version": "17.3.0", - "resolved": "https://registry.npmjs.org/mongodb-connection-model/-/mongodb-connection-model-17.3.0.tgz", - "integrity": "sha512-loaAlu6xoEyz7th+f4lkFd07BZZawZYl2Q4EvImAp45kZxo8AH9RmuPCCUsxUlcTHP35i5pDHnCVxzAOb+LB3A==", + "version": "18.0.0", + "resolved": "https://registry.npmjs.org/mongodb-connection-model/-/mongodb-connection-model-18.0.0.tgz", + "integrity": "sha512-X5OtFijmCP6HNfsCETEYiBH3M+hvW+p+E7gHqA2CQcVaXaeoojYpX2Nsr7YKwW1oKUbQURfM4szt1Ds9rBAzvw==", "dev": true, "requires": { "ampersand-model": "^8.0.0", @@ -4387,7 +4387,6 @@ "async": "^3.1.0", "debug": "^4.1.1", "lodash": "^4.17.15", - "mongodb": "^3.6.3", "raf": "^3.4.1", "ssh2": "^0.8.7", "storage-mixin": "^3.3.4" diff --git a/package.json b/package.json index b57a5cfc..f54d6872 100644 --- a/package.json +++ b/package.json @@ -20,8 +20,8 @@ "check": "mongodb-js-precommit" }, "peerDependencies": { - "mongodb": "^3.6.3", - "mongodb-connection-model": "^17.3.0" + "mongodb": "3.x", + "mongodb-connection-model": "18.x" }, "dependencies": { "async": "^3.2.0", @@ -41,7 +41,7 @@ "mocha": "^8.2.1", "mock-require": "^3.0.3", "mongodb": "^3.6.3", - "mongodb-connection-model": "^17.3.0", + "mongodb-connection-model": "^18.0.0", "mongodb-js-precommit": "^2.2.1", "mongodb-runner": "^4.8.0", "xvfb-maybe": "^0.2.1" From 93f6bb71de69c823d53bf1dc85f1d69a240cdb1e Mon Sep 17 00:00:00 2001 From: Anemy Date: Tue, 15 Dec 2020 14:33:08 -0500 Subject: [PATCH 4/4] Change connection model peer dep to * --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f54d6872..8d306f05 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ }, "peerDependencies": { "mongodb": "3.x", - "mongodb-connection-model": "18.x" + "mongodb-connection-model": "*" }, "dependencies": { "async": "^3.2.0",