CDRIVER-4365 support encryptedFields in collection create and drop #983
Conversation
add encrypted_fields_map to auto_encryption_opts get encrypted fields from map or server add _mongoc_iter_document_as_bson
The name for FLE 2.0 is not yet decided. Leave docs brief for now.
kevinAlbs
changed the title
| mongoc_collection_t * | ||
| mongoc_database_create_collection (mongoc_database_t *database, | ||
| static mongoc_collection_t * | ||
| create_collection (mongoc_database_t *database, | ||
| const char *name, | ||
| const bson_t *opts, | ||
| bson_error_t *error) |
There was a problem hiding this comment.
Ensure you're running a formatting pass over the sources before merging, as later unrelated changes to the same file might re-format the previously-unformatted code and glutter the diff. I use VSCode "Format on save" to ensure I never forget and don't need to bother fiddling with whitespace. Although this often causes headaches when an unrelated change in the file will rewrite some other lines that hadn't been given a formatting pass.
We could also take advantage of .gitblameignore to do a formatting pass on all-the-things (and also maybe increase our column limit? 👀)
| escCollection = create_collection (database, escName, NULL /* opts */, error); | ||
| if (!escCollection) { | ||
| goto fail; | ||
| } |
There was a problem hiding this comment.
This pair of repeated ops could be made into a separate function to reduce redundancy and help clarity, as then:
bool okay =
(escCollection = _create_encField_state_collection(database, encryptedFields, name, "esc", error)
&& (eccCollection = _create_encField_state_collection(database, encryptedFields, name, "ecc", error)
&& (ecocCollection = _create_encField_state_collection(database, encryptedFields, name, "ecoc", error);
if (!okay) {
// Failed to create one or more field collections
goto fail;
}| } | ||
| bson_destroy (create_indexes); | ||
| bson_free (index_name); | ||
| bson_destroy (keys); |
There was a problem hiding this comment.
These cleanups are the same. I would try:
ok = mongoc_database_write_command_with_opts(...);
// [cleanup]No goto needed since the return value is effectively stored in ok
| bson_init (encryptedFields); | ||
|
|
||
| if (bson_empty0 (efMap)) { | ||
| return true; |
There was a problem hiding this comment.
I'd recommend comments on reasons for early returns and goto fails. What conditions are being satisfied or violated? e.g. "Empty efMap will have no encrypted fields", "No efMap entry for this database+collection", or "The efMap entry should always be a document", to help a future reader quickly understand the purpose of a branch or jump.
| return false; | ||
| } | ||
|
|
||
| bson_free (ns); |
There was a problem hiding this comment.
ns can be freed earlier (after the init_find branch).
Summary
mongoc_auto_encryption_opts_set_encrypted_fields_mapencryptedFieldsrequired behavior to create and drop collection helpers._mongoc_iter_document_as_bsonutility to convert abson_iter_ttobson_t.Background & Motivation
Please see mongodb/specifications#1183 for the specification changes.
FLE 2.0 requires a non-standalone server topology. Tests need to apply write concern majority on key vault operations, as noted in the specification test README
That is because the key vault is queried with readConcern "majority". If keys are inserted without writeConcern "majority", some tests fail to find the key if it the insert is not majority committed before it is queried.
Because the name of FLE 2.0 is still being decided, the documentation for
mongoc_auto_encryption_opts_set_encrypted_fields_mapis brief.