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-1420 Cache AWS credentials received from endpoints #905

Merged
merged 5 commits into from
Jun 27, 2023

Conversation

isabelatkinson
Copy link
Contributor

No description provided.

@@ -496,3 +557,45 @@ impl ServerFirst {
}
}
}

#[cfg(test)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this module to avoid making the mutex and various fields pub(crate) just for the sake of testing.

@@ -1066,8 +1066,8 @@ tasks:
- func: "run aws auth test with assume role credentials"
- func: "run aws auth test with aws credentials as environment variables"
- func: "run aws auth test with aws credentials and session token as environment variables"
- func: "run aws auth test with aws EC2 credentials"
- func: "run aws ECS auth test"
# - func: "run aws auth test with aws EC2 credentials"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(see #904)

@isabelatkinson isabelatkinson marked this pull request as ready for review June 27, 2023 17:02
@isabelatkinson
Copy link
Contributor Author

I'm fixing up the MSRV failure but this is ready for review otherwise.

@isabelatkinson isabelatkinson requested a review from abr-egn June 27, 2023 17:02
Copy link
Contributor

@abr-egn abr-egn left a comment

Choose a reason for hiding this comment

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

LGTM!

#[cfg_attr(feature = "async-std-runtime", async_std::test)]
async fn credential_caching() {
// This test should only be run when authenticating using AWS endpoints.
if var("SKIP_CREDENTIAL_CACHING_TESTS").is_ok() {
Copy link
Contributor

Choose a reason for hiding this comment

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

It might make the evergreen config a little less noisy if this were an enable rather than a skip. That's just a vague inclination, though, so feel free to disregard if you feel otherwise :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about doing that originally (agreed this is more noisy), but I'd prefer for these tests to be opt-out rather than opt-in so that we don't forget to enable them if we add a new mechanism using an endpoint in the future.

@isabelatkinson isabelatkinson merged commit 79fa2e1 into mongodb:main Jun 27, 2023
@isabelatkinson isabelatkinson deleted the cache-aws-credentials branch June 28, 2023 17:49
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