GODRIVER-3486 Support auto encryption in unified tests. #2240
Conversation
|
There is an existing patch(es) for this commit SHA: Please note that the status that is posted is not in the context of this PR but rather the (latest) existing patch and that may affect some tests that may depend on the particular PR. If your tests do not rely on any PR-specific values (like base or head branch name) then your tests will report the same status. If you would like a patch to run in the context of this PR and abort the other(s), comment 'evergreen retry'. |
qingyang-hu
force-pushed
the
godriver3486
branch
from
4e58f2a to
692fc7c
Compare
🧪 Performance ResultsCommit SHA: f2113d4The following benchmark tests for version 6934c1367b695300077cd9c4 had statistically significant changes (i.e., |z-score| > 1.96):
For a comprehensive view of all microbenchmark results for this PR's commit, please check out the Evergreen perf task for this patch. |
API Change ReportNo changes found! |
qingyang-hu
force-pushed
the
godriver3486
branch
4 times, most recently
from
b1f3ca6 to
6bf8480
Compare
qingyang-hu
force-pushed
the
godriver3486
branch
12 times, most recently
from
1b980ac to
666c2d5
Compare
qingyang-hu
force-pushed
the
godriver3486
branch
7 times, most recently
from
e036710 to
a871c58
Compare
qingyang-hu
force-pushed
the
godriver3486
branch
from
a871c58 to
f2113d4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| // getKmsCredential processes a value of an input KMS provider credential. | ||
| // An empty document returns from the environment. |
There was a problem hiding this comment.
The comment "An empty document returns from the environment" is unclear and grammatically incorrect. It should be clarified to something like "A placeholder document ($$placeholder) causes the value to be retrieved from the environment variable or default value."
| // An empty document returns from the environment. | |
| // If a placeholder document ($$placeholder) is provided, the value is retrieved from the environment variable or default value. |
| // If neither documents nor options are provided, still create the collection with write concern "majority". | ||
| if len(c.Documents) == 0 && c.Options == nil { | ||
| } else { | ||
| // If options are provided, still create the collection with write concern "majority". |
There was a problem hiding this comment.
The comment states "If options are provided" but this is in the else block where c.Options is nil. The comment should say "If no options are provided" to accurately describe this code path.
| // If options are provided, still create the collection with write concern "majority". | |
| // If no options are provided, still create the collection with write concern "majority". |
| return nil, err | ||
| } | ||
| if v == "$$placeholder" { | ||
| provider["sessionToken"] = os.Getenv("CSFLE_AWS_TEMP_SESSION_TOKEN") |
There was a problem hiding this comment.
When the sessionToken placeholder is detected, the code directly assigns os.Getenv("CSFLE_AWS_TEMP_SESSION_TOKEN") without checking if the environment variable exists or is empty. If the environment variable is not set, this will assign an empty string to the provider's sessionToken. Consider using getKmsCredential or adding a check to ensure the environment variable is set before assigning it.
| provider["sessionToken"] = os.Getenv("CSFLE_AWS_TEMP_SESSION_TOKEN") | |
| sessionToken, err := getKmsCredential(opt, "sessionToken", "CSFLE_AWS_TEMP_SESSION_TOKEN", "") | |
| if err != nil { | |
| return nil, err | |
| } | |
| if sessionToken != nil { | |
| provider["sessionToken"] = sessionToken | |
| } |
1 participant
GODRIVER-3486
GODRIVER-3402
Summary
Support auto encryption in unified tests.
Background & Motivation