-
Notifications
You must be signed in to change notification settings - Fork 94
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
MONGOCRYPT-768 refactor to support multiple schemas #953
MONGOCRYPT-768 refactor to support multiple schemas #953
Conversation
To manage schemas for an auto encryption operation. Supports more than one schema.
The schema broker requires the name to identify the collection in the result. The server replies with this field, but some mock data did not include it.
Catches a regression found when adding the schema broker
To match name from collinfo
07505cc
to
bf688f7
Compare
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]>
Co-authored-by: Ezra Chung <[email protected]>
Co-authored-by: Ezra Chung <[email protected]>
An aggregate with $lookup may have an encryptedFields on a collection referenced in the pipeline.
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.
Further testing motivated additional changes that seemed appropriate for this PR:
prefer "JSON Schema" to refer to the concept, and "jsonSchema" to refer to the field. Co-authored-by: Ezra Chung <[email protected]>
Related to MONGOCRYPT-774. The shared library was previously referred to as "csfle" but has since been renamed to "crypt_shared".
Handle the special case first, then the default. Also test explain with crypt_shared.
Bypassing the create command does not need a schema. `_finalize` checks if any schemas have QE.
`_fle2_collect_keys_for_compaction` is called twice. On the first call, if there is no associated schema, do nothing.
Co-authored-by: Ezra Chung <[email protected]>
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.
A few suggestions remaining; otherwise, LGTM.
src/mc-schema-broker.c
Outdated
// "explain": { "find": "to-mongocryptd" }, | ||
// "encryptionInformation": {} | ||
// } | ||
BSON_ASSERT(bson_iter_init_find(&iter, cmd, "explain")); |
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.
This should probably be demoted to a runtime error rather than an assertion (otherwise the "expected 'explain' to be a document" error below could also be asserted).
Co-authored-by: Ezra Chung <[email protected]>
Summary
Add
mc_schema_broker_t
to manage schemas for an auto encryption operation.Tested with this libmongocrypt: https://spruce.mongodb.com/version/67abfe5152bbba0007c15d63
Tested with the C driver pointing to this branch: https://spruce.mongodb.com/version/67ac860a932f4c0007855354
Background & Motivation
No meaningful changes to behavior are expected. This refactor is intended to help support $lookup in MONGOCRYPT-741. Supporting $lookup requires multiple schemas in one operation.
mc_schema_broker_t
supports multiple schemas to the same database.The term schema refers to either a JSON Schema (for CSFLE) or an encryptedFields document (for QE).
SPM-2472 adds a new field to mongocryptd/crypt_shared:
csfleEncryptionSchemas
. This is to support passing multiple CSFLE schemas.encryptionInformation
already supports passing multiple QE schemas. The syntax forcsfleEncryptionSchemas
is documented in Technical Design: Support $lookup in CSFLE and QE.Test runner improvements
Additional test runner improvements were made to help test
mc_schema_broker_t
.Wildcard test selection
Support trailing wildcard in test selection to ease running related tests:
TMP_BSONF
Several tests use the following pattern to declare BSON inline:
Using
BSON_STR
has the advantage of improved formatting and syntax highlighting. However, a%s
is formatted to% s
:This PR adds
TMP_BSONF
which supportsMC_BSON
andMC_STR
formatting tokens to insert abson_t*
or C string:TEST_FILE_AS_BSON
A new
TEST_FILE_AS_BSON
is added to load a file as abson_t*
rather than amongocrypt_binary_t*
. This was convenient for testingmc_schema_broker_t
which acceptsbson_t*
in the interface.