-
Notifications
You must be signed in to change notification settings - Fork 543
[CXX-2546] Create Encrypted Collection #959
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
[CXX-2546] Create Encrypted Collection #959
Conversation
Seeing failures on some replica_set configurations. Very mysterious |
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.
LGTM
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.
LGTM with consideration to remove an overload and suggested simplifications.
bsoncxx::document::value& out_options, | ||
const std::string& kms_provider, | ||
const stdx::optional<bsoncxx::document::view>& masterkey); | ||
|
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.
I think there is no use case to call this overload with a nullopt
for masterkey
.
Consider combining the two overloads:
/**
* @brief Create a collection with client-side-encryption enabled, automatically filling any
* datakeys for encrypted fields.
*
* @param db The database in which the collection will be created
* @param coll_name The name of the new collection
* @param options The options for creating the collection. @see database::create_collection
* @param out_options Output parameter to receive the generated collection options.
* @param kms_provider The KMS provider to use when creating data encryption keys for the
* collection's encrypted fields
* @param masterkey If non-null, specify the masterkey to be used when creating data keys in the
* collection.
* @return collection A handle to the newly created collection
*/
collection create_encrypted_collection(
const database& db,
const std::string& coll_name,
const bsoncxx::document::view& options,
bsoncxx::document::value& out_options,
const std::string& kms_provider,
const stdx::optional<bsoncxx::document::view> masterkey = stdx::nullopt);
Or alternatively: make masterkey
non-optional in this overload.
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 is more due to my aversion to defaulted arguments. I don't know if there's precedent in the codebase. Combining them will result in a mostly-equivalent API. I'll combine them for now.
bson_t* opt_mkey_ptr = nullptr; | ||
scoped_bson_t opt_mkey; | ||
if (masterkey) { | ||
bson_init_static(opt_mkey.bson_for_init(), masterkey->data(), masterkey->length()); |
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.
bson_init_static(opt_mkey.bson_for_init(), masterkey->data(), masterkey->length()); | |
opt_mkey.init_from_static(*masterkey); |
scoped_bson_t coll_opts; | ||
bson_init_static(coll_opts.bson_for_init(), opts.data(), opts.length()); |
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.
scoped_bson_t coll_opts; | |
bson_init_static(coll_opts.bson_for_init(), opts.data(), opts.length()); | |
scoped_bson_t coll_opts(opts); |
Refer: CXX-2546
This changeset introduces the create encrypted collection convenience API, as outlined in DRIVERS-2312. This is used to create data encryption keys on-the-fly when creating a new encrypted collection. The C++ API is mostly just a wrapper around the underlying C API, with a few convenience overloads for optional parameters.
The error-initiating code may be questionable. It has been a while since I've worked in the C++ codebase.