Skip to content

CDRIVER-4563 do not create or drop eccCollection #1232

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

Merged
merged 10 commits into from
Apr 21, 2023

Conversation

kevinAlbs
Copy link
Collaborator

@kevinAlbs kevinAlbs commented Apr 5, 2023

Summary

  • Do not create or drop eccCollection.
  • Update fle2v2 spec tests.
  • Return error if attempting to create a QE collection with server version < 7.0.0 (maxWireVersion < 21)

Verified with this patch build.

Background & Motivation

See DRIVERS-2524 for additional background behind these changes.

@kevinAlbs kevinAlbs force-pushed the D2524 branch 2 times, most recently from 3846611 to 18abd11 Compare April 10, 2023 17:40
@kevinAlbs kevinAlbs marked this pull request as ready for review April 12, 2023 15:23
Copy link
Contributor

@eramongodb eramongodb left a comment

Choose a reason for hiding this comment

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

Minor feedback; otherwise, LGTM.

Comment on lines 1123 to 1124
"Driver support of Queryable Encryption is incompatible "
"with server. Upgrade server to use Queryable Encryption.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest including both the detected max wire version and the required server version in the error message.

@@ -310,6 +310,8 @@ _mongoc_wire_version_to_server_version (int32_t version)
return "5.3";
case 17:
return "6.0";
case 21:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
case 21:
case WIRE_VERSION_7_0:

May be worth considering replacing literals here with their macro equivalents.

Copy link
Contributor

@vector-of-bool vector-of-bool left a comment

Choose a reason for hiding this comment

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

LGTM

@kevinAlbs kevinAlbs merged commit cf45741 into mongodb:master Apr 21, 2023
create_encField_state_collection (
database, encryptedFields, name, "ecoc", error);
if (!state_collections_ok) {
// Failed to create one or more state collections
goto fail;
}

// Check the wire version to ensure server is 7.0.0 or newer.
Copy link
Member

Choose a reason for hiding this comment

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

This wire version check should be performed before creating the QEv2 state collections. I opened CDRIVER-4653 to track this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants