Skip to content

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.

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

Co-authored-by: vector-of-bool <vectorofbool@gmail.com>
@kevinAlbs kevinAlbs merged commit cf45741 into mongodb:master Apr 21, 2023
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