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

fix(NODE-2995): Add shared metadata MongoClient #2760

Merged
merged 2 commits into from Mar 29, 2021

Conversation

nbbeeken
Copy link
Contributor

Description

Automatic client side encryption needs to perform metadata look ups
like listCollections. In situations where the connection pool size
is constrained or in full use it can be impossible for an operation
to proceed. Adding a separate client in these situations permits the
metadata look ups to proceed unblocking operations.

What changed?

I added a class to encapsulate the functionality of managing the internal client. We can discuss the details of how we want to manage this, for example if we feel the new class if overkill we can go to keeping things saved directly on the MongoClient. I made the prose tests somewhat programatic, but I still wanted them to be relatively easily debugged so there's some repetition in the design, of course sound off if we want this cleaner.

Are there any files to ignore?

A spec test sync is included here, you can filter out the json and yaml files. An important note I synced with the tests written at the time of the meta client spec work since there's been changes since then relating to AWS temporary credentials. Just calling it out if you notice it doesn't match exactly.

@nbbeeken nbbeeken force-pushed the NODE-2995/CSFLE-meta-client branch from 0f5f2a7 to 3e098ea Compare March 19, 2021 14:57
Automatic client side encryption needs to perform metadata look ups
like listCollections. In situations where the connection pool size
is constrained or in full use it can be impossible for an operation
to proceed. Adding a separate client in these situations permits the
metadata look ups to proceed unblocking operations.
@nbbeeken nbbeeken force-pushed the NODE-2995/CSFLE-meta-client branch from 082fdcb to 45fd876 Compare March 22, 2021 16:55
@nbbeeken
Copy link
Contributor Author

Tests should pass now

Something to note, I made a custom variant for this situation where we need to run tests against changes in mongodb-client-encryption (mongodb/libmongocrypt#168) that aren't yet released. This means merging these changes will break our ubuntu variants where we currently test against a released CSFLE version. I've updated run-tests.sh to pull in the latest, so once we release the fix then those tests will start to pass again. The changes to mongodb-client-encryption aren't required to be in sync with the driver, notably, this line falls back on the client provided in previous/current versions of the driver.

@nbbeeken nbbeeken requested review from durran and emadum March 22, 2021 17:48
@nbbeeken nbbeeken marked this pull request as ready for review March 22, 2021 17:49
Copy link
Contributor

@emadum emadum left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

Copy link
Member

@durran durran left a comment

Choose a reason for hiding this comment

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

LGTM

@nbbeeken nbbeeken merged commit 9256242 into 3.6 Mar 29, 2021
@nbbeeken nbbeeken deleted the NODE-2995/CSFLE-meta-client branch March 29, 2021 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants