-
Notifications
You must be signed in to change notification settings - Fork 245
DRIVERS-3227 test text indexes with auto encryption #1823
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
Conversation
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.
Two small comments. Passing in Node: https://parsley.mongodb.com/evergreen/mongo_node_driver_next_rhel8_custom_dependency_tests_run_custom_csfle_tests_latest_patch_acd86250216d0edf0fbc4a1893701a2df038d9bb_6883ddf0c6cd1900073e32b9_25_07_25_19_41_51/0/task?bookmarks=0,1811,1983&selectedLineRange=L1811-L1826
source/client-side-encryption/tests/unified/QE-Text-substringPreview.yml
Outdated
Show resolved
Hide resolved
source/client-side-encryption/tests/unified/QE-Text-substringPreview.yml
Show resolved
Hide resolved
description: QE-Text-cleanupStructuredEncryptionData | ||
schemaVersion: "1.23" | ||
runOnRequirements: | ||
# Requires libmongocrypt 1.15.0 for SPM-4158. |
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.
What would you think about making libmongocrypt version a runOnRequirement? This would be useful in Node and maybe other drivers who have bindings separate from their driver code. We actually do run our latest driver test suite against an older version of our bindings, so this would be necessary in Node (if it isn't codified into the UTR, we'll just have to skip it manually).
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 like the idea. Added a minLibmongocryptVersion
constraint. I expect this can also benefit backporting tests to release branches using an older libmongocrypt.
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.
We probably want to adopt these changes in #1784. I added a comment to that PR about these UTR changes.
Does not make any changes yet.
add alternative form of `csfle` to support a `minLibmongocryptVersion`
To fix `no schema with key or ref` error from AJV with tests using draft2019 spec.
Co-authored-by: Bailey Pearson <[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.
Assigned |
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.
Changes to schema extraction LGTM
exit 1 | ||
fi | ||
|
||
if ! ajvCheck=$(ajv --spec="$spec" -s "source/unified-test-format/schema-$schemaVersion.json" -d "$testFile"); then |
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.
Noted that this pertains to https://ajv.js.org/packages/ajv-cli.html#json-schema-language-and-version
Did something in this PR require introduction of the --spec
option for ajv-cli
? It looks like the schema URL was changed in db69351 when deprecated
options were introduced.
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.
Adding schema 1.25 led to the discovery of an existing issue using ajv
to validate with schema 1.24:
% ajv test -s ./source/unified-test-format/schema-1.24.json -d ./source/unified-test-format/tests/valid-pass/poc-crud.json --valid
schema ./source/unified-test-format/schema-1.24.json is invalid
error: no schema with key or ref "https://json-schema.org/draft/2019-09/schema#"
I expect this was undiscovered since there are no tests currently using the 1.24 format. Quoting #1809:
JSON Schema draft 2019-09 adds a new "deprecated" property
Adding the --spec
argument appears to fix. ajv help
notes draft7
(used in spec 1.23 and older) is the default.
|
||
- `minLibmongocryptVersion`: Optional string. The minimum libmongocrypt (inclusive) required to successfully run the | ||
tests. If this field is omitted, there is no lower bound on the required libmongocrypt version. The format of this | ||
string is defined in [Version String](#version-string). |
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 seems fine.
runOnRequirement-csfle-type.yml
remains as-is, because you're making the type of csfle
more permissive, but I think you can add a new test for your restriction on the object value's structure.
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.
Added invalid
tests for minLibmongocryptVersion
.
Summary
minLibmongocryptVersion
.ajv
validation with schema version 1.24+.To run tests:
Tests were run in the C driver in mongodb/mongo-c-driver#2069
Background & Motivation
Tests are intended to serve as simple end-to-end tests to ensure drivers have upgraded libmongocrypt and work as expected. Tests are not intended to exhaustively test behavior of text queries.
This PR does not update EncryptOpts for explicit encryption. This is expected in DRIVERS-3213.
libmongocrypt 1.15.0
libmongocrypt 1.15.0 is released. Binaries can be obtained from the
Files
tab from upload tasks on the 1.15.0 tagged commit.libmongocrypt 1.14.0 includes most of the text index support but excludes support for cleanup/compact commands (MONGOCRYPT-810 + MONGOCRYPT-811). Tests are added for
compactStructuredEncryptionData
andcleanupStructuredEncryptionData
to ensure drivers have updated libmongocrypt to get latest changes from SPM-4158. Attempting to run these tests on libmongocrypt 1.14.0 results in a mismatch failure:minLibmongocryptVersion
runOnRequirements
are extended to add aminLibmongocryptVersion
constraint. This is intended to help drivers skip tests when using older libmongocrypt versions (see comment).This is implemented as an alternative form of the
csfle
constraint, summarized as:This required a new Unified Test Format schema version 1.25. This led to the discovery of an existing issue using
ajv
to validate with schema 1.24:I expect this was undiscovered since there are no tests currently using the 1.24 format. Quoting #1809:
Explicit
--spec
arguments are added toajv
invocations to fix.ajv help
notesdraft7
(used in spec 1.23 and older) is the default.Please complete the following before merging:
[ ] Test these changes against all server versions and topologies (including standalone, replica set, and shardedC does not-yet test sharded with In-Use Encryption.clusters).