Skip to content

Conversation

@aljones15
Copy link
Contributor

@aljones15 aljones15 commented Sep 2, 2022

  1. Removes the roles check for ./lib/authn.js
  2. Throws if the server version is less than 3.6.0 (this was tested and was the lowest mongo node 4 goes)
  3. Ups the default server version to 4.4

TODO:

  • Test with mongodb server v3
  • Test with mongodb server v2

Node Driver compatibility matrix is here: https://www.mongodb.com/docs/drivers/node/current/compatibility/#mongodb-compatibility

3.6 seems to be the lowest.

@aljones15 aljones15 self-assigned this Sep 2, 2022
@codecov-commenter
Copy link

codecov-commenter commented Sep 2, 2022

Codecov Report

Merging #94 (d64005d) into mongo-driver-4-rc (1b4c0e6) will decrease coverage by 0.41%.
The diff coverage is 30.76%.

@@                  Coverage Diff                  @@
##           mongo-driver-4-rc      #94      +/-   ##
=====================================================
- Coverage              87.08%   86.66%   -0.42%     
=====================================================
  Files                      8        8              
  Lines                    813      810       -3     
=====================================================
- Hits                     708      702       -6     
- Misses                   105      108       +3     
Impacted Files Coverage Δ
lib/authn.js 75.00% <25.00%> (-2.05%) ⬇️
lib/config.js 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@aljones15 aljones15 marked this pull request as ready for review October 26, 2022 19:01
'NotSupportedError', {
url: urls.sanitize(config.url),
version,
required: '>= 3.6.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't a different version being required by code changes elsewhere in this PR?

Copy link
Contributor Author

@aljones15 aljones15 Dec 1, 2022

Choose a reason for hiding this comment

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

The driver only goes down to 3.6 now: https://www.mongodb.com/docs/drivers/node/current/compatibility/#mongodb-compatibility

So 3.6 is the hard limit on server version.

You can see the 2 checks are here:

function _checkServerVersion({serverInfo, config}) {
// check that server version is supported
const {version} = serverInfo;
logger.info('connected to database', {
url: urls.sanitize(config.url),
version,
});
// if the server is not at least 3.6.0 throw
// as mongo node driver 4 only supports 3.6.0 and higher
if(!semver.gte(version, '3.6.0')) {
throw new BedrockError(
'Unsupported database version.',
'NotSupportedError', {
url: urls.sanitize(config.url),
version,
required: '>= 3.6.0'
});
}
if(config.requirements.serverVersion &&
!semver.satisfies(version, config.requirements.serverVersion)) {
throw new BedrockError(
'Unsupported database version.',
'NotSupportedError', {
url: urls.sanitize(config.url),
version,
required: config.requirements.serverVersion
});
}
}

config.mongodb.requirements = {};
// server version requirement with server-style string
config.mongodb.requirements.serverVersion = '>=4.2';
config.mongodb.requirements.serverVersion = '>=4.4';
Copy link
Contributor

Choose a reason for hiding this comment

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

Is 4.4 a requirement? We do still have some clusters on Atlas that are 4.2.x.

Copy link
Contributor Author

@aljones15 aljones15 Dec 1, 2022

Choose a reason for hiding this comment

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

Ok so 4.4 is the recommended min version, you can set the serverVersion as low as 3.6. See the compatibility matrix here: https://www.mongodb.com/docs/drivers/node/current/compatibility/#mongodb-compatibility

You can see the two checks here:

function _checkServerVersion({serverInfo, config}) {
// check that server version is supported
const {version} = serverInfo;
logger.info('connected to database', {
url: urls.sanitize(config.url),
version,
});
// if the server is not at least 3.6.0 throw
// as mongo node driver 4 only supports 3.6.0 and higher
if(!semver.gte(version, '3.6.0')) {
throw new BedrockError(
'Unsupported database version.',
'NotSupportedError', {
url: urls.sanitize(config.url),
version,
required: '>= 3.6.0'
});
}
if(config.requirements.serverVersion &&
!semver.satisfies(version, config.requirements.serverVersion)) {
throw new BedrockError(
'Unsupported database version.',
'NotSupportedError', {
url: urls.sanitize(config.url),
version,
required: config.requirements.serverVersion
});
}
}

@aljones15 aljones15 requested a review from mattcollier December 1, 2022 20:52
@dlongley
Copy link
Member

dlongley commented Mar 5, 2025

Closing in favor of #107.

@dlongley dlongley closed this Mar 5, 2025
@dlongley dlongley deleted the mongo-4-no-roles-check branch March 5, 2025 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants