Skip to content
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

RUST-1442 On-demand Azure KMS credentials #872

Merged
merged 15 commits into from
May 12, 2023

Conversation

abr-egn
Copy link
Contributor

@abr-egn abr-egn commented May 9, 2023

RUST-1442

This adds support for fetching on-demand csfle credentials from the Azure IMDS service. Notably, this requires an HTTP fetch, so this updates the internal HttpClient to not be aws auth only; this also means that azure auth has the same restriction as aws auth (i.e. tokio), and is behind a feature flag.

This PR includes the unit tests; integration tests to come in a follow-up PR.

@abr-egn abr-egn force-pushed the RUST-1442/azure-kms branch from ed874e7 to 342a09e Compare May 9, 2023 18:02
@abr-egn abr-egn marked this pull request as ready for review May 9, 2023 20:55
@abr-egn abr-egn requested a review from isabelatkinson May 9, 2023 20:55
@@ -65,6 +65,9 @@ bson-uuid-1 = ["bson/uuid-1"]
# This can only be used with the tokio-runtime feature flag.
aws-auth = ["reqwest"]

# Enable support for on-demand Azure KMS credentials.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a note here that this can only be used with tokio (similar to above)? It's a bummer that we need to add a new feature flag here; I'll also need to do so for the GCP KMS work. We should try to consolidate these in 3.0.0; we could unify them under the reqwest feature created by the optional dependency, or possibly make them simpler if we remove support for tokio.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, done. And yeah, agreed that it's not a great situation w.r.t. features and dependencies.

&format!("invalid `expires_in` response field: {}", e),
)
})?;
#[allow(clippy::redundant_clone)]
Copy link
Contributor

Choose a reason for hiding this comment

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

why is a redundant clone needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The #[cfg(test)] line below needs server_response, but without the clone this line is a partial move.

@abr-egn abr-egn requested a review from isabelatkinson May 12, 2023 15:24
@abr-egn abr-egn merged commit 6a6c309 into mongodb:main May 12, 2023
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.

2 participants