MONGOCRYPT-723 support $lookup
#954
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Require opting in to enable multi-collection support. Require drivers to signal that the new protocol for MONGOCRYPT_CTX_NEED_MONGO_COLLINFO is implemented.
Used to implement opt-in check
Useful to further configure `mongocrypt_t` before test
Test $lookup is supported. Test $unionWith and $facet are parsed. Test opt-in is required to support multiple collections. Test mongocryptd/crypt_shared version is checked.
To fix observed test failures on Evergreen
eramongodb
requested changes
Feb 18, 2025
Co-authored-by: Ezra Chung <[email protected]>
Co-authored-by: Ezra Chung <[email protected]>
Co-authored-by: Ezra Chung <[email protected]>
Co-authored-by: Ezra Chung <[email protected]>
eramongodb
reviewed
Feb 18, 2025
Co-authored-by: Ezra Chung <[email protected]>
Co-authored-by: Ezra Chung <[email protected]>
eramongodb
approved these changes
Feb 18, 2025
mdb-ad
approved these changes
Feb 20, 2025
mdbmes
reviewed
Feb 21, 2025
| } tester_mongocrypt_flags; | ||
|
|
||
| /* Arbitrary max of 2048 instances of temporary test data. Increase as needed. | ||
| /* Arbitrary max of 2148 instances of temporary test data. Increase as needed. | ||
| * TODO(MONGOCRYPT-775) increasing further (e.g. 3000+) causes a segfault on Windows test runs. Revise. |
There was a problem hiding this comment.
Putting "static" on the "tester" definition in main() should fix this, it's a pretty large thing to keep on the stack.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Support encrypting "aggregate" commands with
$lookupTested with libmongocrypt patch: https://spruce.mongodb.com/version/67af62d2eff4dc0007490c8f
Tested with C driver patch pointing to this branch: https://spruce.mongodb.com/version/67af5960bad7a30007a37b90
Background & Motivation
Parsing
Enough of the aggregate pipeline is parsed to find occurrences of
$lookup.$lookupcan be nested within$lookup,$facet, and$unionWith.mongocryptd/crypt_shared return an error for
$unionWithand$facet:Aggregation stage $unionWith is not allowed or supported with automatic encryption.. Regardless, this PR parses within$unionWithand$facetfor completeness.Parsing in libmongocrypt may require future changes. If a future server version extends the aggregate pipeline, libmongocrypt may not find all
$lookupstages. However, if libmongocrypt does not pass a schema for a needed collection, mongocryptd/crypt_shared errors (e.g.Missing encryption schema for namespace: db.c2) rather than assumes the collection has no schema. This was decided acceptable in Technical Design: Support $lookup in CSFLE and QE:mongocryptd/crypt_shared version check
A version check is added to ensure mongocryptd/crypt_shared supports multiple schemas. This is intended to improve the error that would otherwise be returned from a too-old mongocryptd/crypt_shared. Testing mongocryptd 8.0 gets the following errors:
jsonSchema or encryptionInformation is requiredwhen passing multiple CSFLE schemas.Exactly one namespace is supported with encryptionInformationwhen passing multiple QE schemas.Instead, libmongocrypt returns an error like the following:
Encrypting 'aggregate' requires multiple schemas. Detected mongocryptd with wire version 17, but need 26. Upgrade mongocryptd to 8.1 or newer.when using mongocryptdEncrypting 'aggregate' requires multiple schemas. Detected crypt_shared with version 8.0.4, but need 8.1. Upgrade crypt_shared to 8.1 or newer.when using crypt_sharedOpt-in to support multi-collection commands
The state
MONGOCRYPT_CTX_NEED_MONGO_COLLINFOrequests the driver sendlistCollectionsto check for server-side schemas. Before this PR, this state only expected at most one result fromlistCollections. Quoting integrating.md:To support
$lookup, drivers are now expected to return all results fromlistCollections.libmongocrypt cannot distinguish between "did not pass all results" (following old protocol) and "server did not have results". libmongocrypt applies empty schemas to collections that have no known schema. If a driver upgrades libmongocrypt and does not implement the new protocol, an empty schema might be applied to a collection that really has a server-side schema.
To ensure drivers upgrading libmongocrypt follow the new protocol, drivers must call
mongocrypt_setopt_enable_multiple_collinfoto enable support for multi-collection commands. Without opting-in, libmongocrypt returns an error if a command requires multiple collections. This is to avoid the risk that a driver upgrades libmongocrypt without implementing the new protocol.Increasing
TEST_DATA_COUNTIncreasing
TEST_DATA_COUNTwas done to support the new tests. Increasing too high resulted in a segfault on Windows (too large stack frame?). Filed MONGOCRYPT-775 to investigate separately (not strictly needed for this PR).