-
Notifications
You must be signed in to change notification settings - Fork 456
CDRIVER-4548 Support ENVIRONMENT:azure for MONGODB-OIDC.
#2166
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
Support a custom token resource, optional timeout, and optional client_id.
And move check that both env+user callbacks are not set from auth to cache (to simplify).
The tasks already define their `run_on`.
To avoid backslashes with multi-line commands.
Ninja uses parallel by default.
Working directory is set by Evergreen task
Already set at the project level.
Saves a small amount of clean-up if topology creation fails before.
eramongodb
left a comment
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.
Minor feedback remaining; otherwise, LGTM.
| tar -czf test-libmongoc.tar.gz "${files[@]}" | ||
| echo "Creating test-libmongoc tarball ... end" | ||
|
|
||
| cat <<EOT > oidc-remote-test-expansion.yml |
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.
| cat <<EOT > oidc-remote-test-expansion.yml | |
| cat <<EOT >oidc-remote-test-expansion.yml |
Formatting.
| bash_exec( | ||
| command_type=EvgCommandType.SETUP, | ||
| include_expansions_in_env=['AWS_ACCESS_KEY_ID', 'AWS_SECRET_ACCESS_KEY', 'AWS_SESSION_TOKEN'], | ||
| env={"AZUREOIDC_VMNAME_PREFIX": "CDRIVER"}, |
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.
| env={"AZUREOIDC_VMNAME_PREFIX": "CDRIVER"}, | |
| env={'AZUREOIDC_VMNAME_PREFIX': 'CDRIVER'}, |
Formatting.
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.
Formatted file with Ruff formatter.
| bash_exec( | ||
| command_type=EvgCommandType.SETUP, | ||
| script='./drivers-evergreen-tools/.evergreen/auth_oidc/azure/delete-vm.sh', | ||
| ) |
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.
| ) | |
| ), |
Formatting.
| const mlib_duration remaining = mlib_duration((*timeout_us, us), minus, (bson_get_monotonic_time(), us)); | ||
| timer = mlib_expires_after(remaining); |
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.
| const mlib_duration remaining = mlib_duration((*timeout_us, us), minus, (bson_get_monotonic_time(), us)); | |
| timer = mlib_expires_after(remaining); | |
| timer = mlib_expires_at((mlib_time_point){.time_since_monotonic_start = mlib_duration(*timeout_us, us)}); |
timeout_us is a time point (in microseconds). bson_get_monotonic_time() calls mlib_now(). I do not think there is a need to go through a duration computation before calling mlib_expires_after() (which calls mlib_now() again).
Note: internal code need not follow public API documentation's instructions to compare against bson_get_monotonic_time().
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.
Good point. Added comment above call to mongoc_oidc_callback_params_set_timeout that bson_get_monotonic_time() calls mlib_now().
connorsmacd
left a comment
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.
LGTM aside from nitpicks.
| for (char const *i = (char *)str; *i; i++) { | ||
| if (needs_percent_encoding((unsigned char)*i)) { | ||
| encoded_len += 3u; |
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.
| for (char const *i = (char *)str; *i; i++) { | |
| if (needs_percent_encoding((unsigned char)*i)) { | |
| encoded_len += 3u; | |
| static const size_t percent_encoded_char_len = 3u; | |
| for (char const *i = (char *)str; *i; i++) { | |
| if (needs_percent_encoding((unsigned char)*i)) { | |
| encoded_len += percent_encoded_char_len; |
Suggest using a constant here since the meaning of 3u wasn't immediately obvious to me.
| int req = bson_snprintf(o, 4, "%%%02X", (unsigned char)*i); | ||
| // Expect no truncation. | ||
| BSON_ASSERT(req == 3); | ||
| o += 3u; |
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.
| o += 3u; | |
| o += percent_encoded_char_len; |
Suggested in review. Not automatically added by Ruff.
Summary
Support
ENVIRONMENT:azureforMONGODB-OIDC.mcd-azure.hto support Azure IMDS requests for OIDC. This internal header previously only supported Azure IMDS requests to Azure Key Vault for In-Use Encryption.Patch build: https://spruce.mongodb.com/version/6904afbc379d3a0007e1ac83
Testing
Evergreen testing is described in the drivers-evergreen-tools README.md. Testing follows a similar pattern to the
testazurekms-task: buildtest-libmongocon an Evergreen host, create a remote Azure VM with a matching OS, copy the binary, run the test.oidc-compile-azure.shincludes a temporary workaround to install UV (as was done in #2163).