Skip to content

CDRIVER-4584 support Queryable Encryption v2 #1228

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 14 commits into from
Apr 6, 2023
Merged

Conversation

kevinAlbs
Copy link
Collaborator

@kevinAlbs kevinAlbs commented Mar 31, 2023

Summary

  • Resync test changes made in DRIVERS-2435
    • Remove fle2-* tests expecting the QEv1 protocol.
    • Add fle2v2-* tests expecting the QEv2 protocol.
  • Test with libmongocrypt 1.8.0-alpha0
  • Require libmongocrypt 1.8.0 or newer to build C driver
  • Skip Queryable Encryption tests on Serverless until DRIVERS-2589 is resolved.
  • Require server 7.0 to run Queryable Encryption tests.

Changes are tested with this patch build

Background & Motivation

See DRIVERS-2435.

To enable the featureFlagFLE2ProtocolVersion2 until it is enabled by default in SERVER-69563
Syncs to specifications commit 64deb2837a2355f6002775c49b9b6c50c9dc560f
remove unnecessary wire version checks server version is now checked in the function call
@kevinAlbs kevinAlbs marked this pull request as ready for review March 31, 2023 15:22
@kevinAlbs kevinAlbs requested a review from eramongodb April 4, 2023 13:17
Comment on lines 19 to 27
if "procParams" in server:
if "setParameter" in server["procParams"]:
server["procParams"]["setParameter"]["featureFlagFLE2ProtocolVersion2"] = "1"
else:
server["procParams"]["setParameter"] = {
"featureFlagFLE2ProtocolVersion2": "1"
}
return True
return False
Copy link
Contributor

@vector-of-bool vector-of-bool Apr 5, 2023

Choose a reason for hiding this comment

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

Suggested change
if "procParams" in server:
if "setParameter" in server["procParams"]:
server["procParams"]["setParameter"]["featureFlagFLE2ProtocolVersion2"] = "1"
else:
server["procParams"]["setParameter"] = {
"featureFlagFLE2ProtocolVersion2": "1"
}
return True
return False
params = server.get("procParams")
if params is None:
return False
setParams = params.setdefault("setParameter", {})
setParams["featureFlagFLE2ProtocolVersion2"] = "1"
return True

Dict.get returns None if the key is absent. Dict.setdefault(K, V) returns the existing value if present or performs Dict[K] = V, and then returns Dict[V]. dicts are by-reference mutable.

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, although I got distracted by the Python code.

raise Exception(
"Did not add setParameter. Does the orchestration config have `procParams`?"
)
pass
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
pass

Comment on lines 29 to 55
# Rewrite for a server.
if rewrite_server(config):
did_rewrite = True
# Rewrite for each member in a replica set.
if "members" in config:
for server in config["members"]:
if rewrite_server(server):
did_rewrite = True
# Rewrite each shard.
if "shards" in config:
for shard in config["shards"]:
if "shardParams" in shard:
if "members" in shard["shardParams"]:
for server in shard["shardParams"]["members"]:
if rewrite_server (server):
did_rewrite = True
# Rewrite each router.
if "routers" in config:
for router in config["routers"]:
# routers do not use `procParams`. Use setParameter directly.
if "setParameter" in router:
router["setParameter"]["featureFlagFLE2ProtocolVersion2"] = "1"
else:
router["setParameter"] = {
"featureFlagFLE2ProtocolVersion2": "1"
}
did_rewrite = True
Copy link
Contributor

Choose a reason for hiding this comment

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

I got nerd-sniped into flattening this code :)

Suggested change
# Rewrite for a server.
if rewrite_server(config):
did_rewrite = True
# Rewrite for each member in a replica set.
if "members" in config:
for server in config["members"]:
if rewrite_server(server):
did_rewrite = True
# Rewrite each shard.
if "shards" in config:
for shard in config["shards"]:
if "shardParams" in shard:
if "members" in shard["shardParams"]:
for server in shard["shardParams"]["members"]:
if rewrite_server (server):
did_rewrite = True
# Rewrite each router.
if "routers" in config:
for router in config["routers"]:
# routers do not use `procParams`. Use setParameter directly.
if "setParameter" in router:
router["setParameter"]["featureFlagFLE2ProtocolVersion2"] = "1"
else:
router["setParameter"] = {
"featureFlagFLE2ProtocolVersion2": "1"
}
did_rewrite = True
import itertools
# We will rewrite the root config:
root = [config]
# And any top-level members:
root_members = config.get("members", [])
# As well as the members within any defined shards:
shards = config.get("shards", [])
# Get a list of lists of membres:
shard_member_lists = (s.get("shardParams", {}).get("members", []) for s in shards)
# Flatten the list of lists:
shard_members = itertools.chain.from_iterable(shard_member_lists)
# For all members in all groups:
all_members = itertools.chain(root, root_members, shard_members)
# Rewrite them all:
all_rewrites = list(map(rewrite_server, all_members))
# Did we actually rewrite anything?
did_rewrite = any(all_rewrites)
# Rewrite each router.
for rtr in config.get("routers", []):
ps = rtr.setdefault("setParameter", {})
ps["featureFlagFLE2ProtocolVersion2"] = "1"
did_rewrite = True

itertools.chain reference

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh that is really neat. I did not know about itertools.chain.

@kevinAlbs
Copy link
Collaborator Author

The setfle2parameter.py has has been removed now that the feature flag is enabled on servers by default: DRIVERS-2590

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.

3 participants