-
Notifications
You must be signed in to change notification settings - Fork 201
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(deps): bump mongosh, driver to latest COMPASS-9056 #6774
base: main
Are you sure you want to change the base?
Changes from all commits
83bd44f
1597ace
05f1d3f
fff9b87
9b661bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -180,11 +180,11 @@ | |
"email": "[email protected]" | ||
}, | ||
"dependencies": { | ||
"@mongosh/node-runtime-worker-thread": "^3.3.0", | ||
"@mongosh/node-runtime-worker-thread": "^3.3.1", | ||
"clipboard": "^2.0.6", | ||
"kerberos": "^2.2.0", | ||
"kerberos": "^2.2.1", | ||
"keytar": "^7.9.0", | ||
"mongodb-client-encryption": "^6.1.0", | ||
"mongodb-client-encryption": "^6.3.0", | ||
"os-dns-native": "^1.2.1", | ||
"system-ca": "^2.0.0" | ||
}, | ||
|
@@ -262,7 +262,7 @@ | |
"hadron-build": "^25.7.4", | ||
"hadron-ipc": "^3.4.4", | ||
"minimatch": "^10.0.1", | ||
"mongodb": "^6.13.1", | ||
"mongodb": "^6.14.1", | ||
"mongodb-build-info": "^1.7.2", | ||
"mongodb-cloud-info": "^2.1.2", | ||
"mongodb-connection-string-url": "^3.0.1", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ import parseNamespace from 'mongodb-ns'; | |
import _ from 'lodash'; | ||
import type { UnboundDataServiceImplLogger } from './logger'; | ||
import { mongoLogId } from './logger'; | ||
import { EJSON } from 'bson'; | ||
|
||
/** | ||
* A list of field paths for a document. | ||
|
@@ -365,6 +366,12 @@ export class CSFLECollectionTrackerImpl implements CSFLECollectionTracker { | |
return info; | ||
} | ||
|
||
*_getCSFLECollectionNames(): Iterable<ReturnType<typeof parseNamespace>> { | ||
for (const ns of this._nsToInfo.keys()) { | ||
yield parseNamespace(ns); | ||
} | ||
} | ||
|
||
updateCollectionInfo( | ||
ns: string, | ||
result: CollectionInfoNameOnly & Partial<CollectionInfo> | ||
|
@@ -402,19 +409,21 @@ export class CSFLECollectionTrackerImpl implements CSFLECollectionTracker { | |
db: (dbName: string) => { | ||
return { | ||
listCollections: (filter: Document, opts: ListCollectionsOptions) => { | ||
// eslint-disable-next-line @typescript-eslint/no-this-alias | ||
const self = this; // there are no async generator arrow functions | ||
return { | ||
toArray: async () => { | ||
[Symbol.asyncIterator]: async function* () { | ||
const collectionInfos = await wrappedClient | ||
.db(dbName) | ||
.listCollections(filter, opts) | ||
.toArray(); | ||
const err = this._checkListCollectionsForLibmongocryptResult( | ||
const err = self._checkListCollectionsForLibmongocryptResult( | ||
dbName, | ||
filter, | ||
(collectionInfos ?? []) as CollectionInfo[] | ||
); | ||
if (err) { | ||
this._logger?.error( | ||
self._logger?.error( | ||
'COMPASS-DATA-SERVICE', | ||
mongoLogId(1_001_000_122), | ||
'CSFLECollectionTracker', | ||
|
@@ -423,7 +432,7 @@ export class CSFLECollectionTrackerImpl implements CSFLECollectionTracker { | |
); | ||
throw err; | ||
} | ||
return collectionInfos; | ||
yield* collectionInfos; | ||
}, | ||
}; | ||
}, | ||
|
@@ -437,53 +446,70 @@ export class CSFLECollectionTrackerImpl implements CSFLECollectionTracker { | |
filter: Document, | ||
collectionInfos: CollectionInfo[] | ||
): Error | undefined { | ||
if (typeof filter?.name !== 'string' || collectionInfos.length > 1) { | ||
// filter is either { name: string } or { name: { $in: string[] } } | ||
if ( | ||
typeof filter?.name !== 'string' && | ||
(!filter?.name || | ||
typeof filter.name !== 'object' || | ||
!Array.isArray(filter.name.$in) || | ||
!(filter.name.$in as unknown[]).every( | ||
(name) => typeof name === 'string' | ||
)) | ||
) { | ||
// This is an assertion more than an actual error condition. | ||
// It ensures that we're only getting listCollections requests | ||
// in the format that we expect them to come in. | ||
return new Error( | ||
`[Compass] Unexpected listCollections request on '${dbName}' with name: '${ | ||
filter?.name as string | ||
}'` | ||
`[Compass] Unexpected listCollections request on '${dbName}' with filter: ${EJSON.stringify( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] Should we include the shape of the filter that we expect? I find it a much nicer DX when I get an error that not only tells me that I did something wrong, but also helps me do the right thing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could, but honestly I don't think that would be worth it, if this actually gets thrown at some point, that means we're going to have to do some digging into the root cause anyway |
||
filter | ||
)}` | ||
); | ||
} | ||
const filterNames: string[] = | ||
typeof filter.name === 'string' ? [filter.name] : filter.name.$in; | ||
|
||
const ns = `${dbName}.${filter.name}`; | ||
const existingInfo = this._getCSFLECollectionInfo(ns); | ||
|
||
if (collectionInfos.length === 0) { | ||
if (existingInfo.serverEnforcedEncryptedFields?.encryptedFields?.length) { | ||
return new Error( | ||
`[Compass] Missing encrypted field information of collection '${ns}'` | ||
); | ||
// First check: All collections for which we had existing encrypted fields | ||
// in the server schema also have a collection schema in the new listCollections response | ||
for (const existingNs of this._getCSFLECollectionNames()) { | ||
if ( | ||
existingNs.database === dbName && | ||
filterNames.includes(existingNs.collection) | ||
) { | ||
const existingInfo = this._getCSFLECollectionInfo(existingNs.ns); | ||
if ( | ||
existingInfo.serverEnforcedEncryptedFields?.encryptedFields?.length && | ||
!collectionInfos.some((info) => info.name === existingNs.collection) | ||
) { | ||
return new Error( | ||
`[Compass] Missing encrypted field information for collection '${existingNs.ns}'` | ||
); | ||
} | ||
} | ||
return; | ||
} | ||
|
||
const [info] = collectionInfos; | ||
if (filter.name !== info.name) { | ||
// Also just a consistency check to make sure that things | ||
// didn't go *terribly* wrong somewhere. | ||
return new Error( | ||
`[Compass] Unexpected listCollections name mismatch: got ${info.name}, expected ${filter.name}` | ||
// Second check: All fields that were previously known to be encrypted | ||
// are still encrypted in the new listCollections response | ||
for (const info of collectionInfos) { | ||
const ns = `${dbName}.${info.name}`; | ||
const existingInfo = this._getCSFLECollectionInfo(ns); | ||
|
||
const newInfo = extractEncryptedFieldsFromListCollectionsResult( | ||
info.options | ||
); | ||
} | ||
const newInfo = extractEncryptedFieldsFromListCollectionsResult( | ||
info.options | ||
); | ||
|
||
for (const expectedEncryptedField of existingInfo | ||
.serverEnforcedEncryptedFields?.encryptedFields ?? []) { | ||
if ( | ||
!newInfo.encryptedFields.some((field) => | ||
_.isEqual(field, expectedEncryptedField) | ||
) | ||
) { | ||
return new Error( | ||
`[Compass] Missing encrypted field '${expectedEncryptedField.join( | ||
'.' | ||
)}' of collection '${ns}' in listCollections result` | ||
); | ||
for (const expectedEncryptedField of existingInfo | ||
.serverEnforcedEncryptedFields?.encryptedFields ?? []) { | ||
if ( | ||
!newInfo.encryptedFields.some((field) => | ||
_.isEqual(field, expectedEncryptedField) | ||
) | ||
) { | ||
return new Error( | ||
`[Compass] Missing encrypted field '${expectedEncryptedField.join( | ||
'.' | ||
)}' of collection '${ns}' in listCollections result` | ||
); | ||
} | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this be documented as a type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but the point here is that the input isn't of a known type ... we could split this out into a separate validation function if you prefer, that takes a generic
unknown
orDocument
input and returns this typeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, I don't have strong feelings about it; leaving it as is is fine also 👍