GODRIVER-3567 Update the Retrieve interface.#2481
Open
qingyang-hu wants to merge 1 commit into
Open
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
godriver3567
branch
from
9a0ac7f to
d35eac3
Compare
Contributor
🧪 Performance ResultsCommit SHA: d35eac3The following benchmark tests for version 6a563c3d883db50007a6bc60 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. |
Contributor
API Change Report./v2/x/mongo/drivercompatible changesAWSCredentialsProvider: added |
qingyang-hu
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the driver’s AWS/Azure credential plumbing to support context-aware credential retrieval, adds an AWS credentials provider hook on driver.Cred, and refactors the internal AWS auth flow to use the v4 signer directly.
Changes:
- Add
AWSCredentialsProvidersupport tox/mongo/driver.Credand wire it into MongoDB-AWS authentication. - Refactor internal AWS/Azure credential providers to use
Retrieve(ctx)/Get(ctx)and move expiration tracking intocredentials.Value. - Update internal credentials caching/chain logic and adjust related tests.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| x/mongo/driver/driver.go | Adds AWSCredentialsProvider to Cred and defines the provider interface. |
| x/mongo/driver/auth/mongodbaws.go | Wires optional custom AWS provider into the provider chain and uses v4 signer path. |
| x/mongo/driver/auth/creds/credscaching_test.go | Updates AWS credential caching test expectations for invalid/expired credentials. |
| x/mongo/driver/auth/creds/azurecreds.go | Switches Azure credential retrieval to Credentials.Get(ctx). |
| x/mongo/driver/auth/creds/awscreds.go | Switches AWS credential retrieval to Credentials.Get(ctx). |
| x/mongo/driver/auth/aws_sasl_adapter.go | Refactors SASL client to sign requests via v4 signer instead of the old conversation wrapper. |
| internal/credproviders/static_provider.go | Updates static provider to implement Retrieve(context.Context). |
| internal/credproviders/imds_provider.go | Refactors Azure IMDS provider to return Value expiration via fields instead of internal state. |
| internal/credproviders/env_provider.go | Refactors env provider to context signature and uses CanExpire/Expires semantics for invalid creds. |
| internal/credproviders/ecs_provider.go | Refactors ECS provider to context signature and uses CanExpire/Expires in returned Value. |
| internal/credproviders/ec2_provider.go | Refactors EC2 provider to context signature and uses CanExpire/Expires in returned Value. |
| internal/credproviders/assume_role_provider.go | Refactors assume-role provider to context signature and uses CanExpire/Expires in returned Value. |
| internal/aws/signer/v4/v4.go | Updates signer to call Credentials.Get(ctx) and adjusts documentation. |
| internal/aws/credentials/credentials.go | Redesigns credentials caching around Value.Expired() and atomic storage; removes provider-side IsExpired. |
| internal/aws/credentials/credentials_test.go | Updates credential tests for the new API and concurrency behavior. |
| internal/aws/credentials/chain_provider.go | Updates chain provider to call Retrieve(ctx) and validate returned values. |
| internal/aws/credentials/chain_provider_test.go | Updates chain provider tests for new API/behavior. |
Comments suppressed due to low confidence (1)
x/mongo/driver/auth/aws_sasl_adapter.go:56
- awsSaslAdapter.Next ignores the provided context, and finalMsg creates an http.Request without attaching the SASL context. Because the v4 signer fetches credentials using req.Context(), credential retrieval and signing cannot be canceled when the auth context is canceled.
Thread ctx through step/finalMsg and use http.NewRequestWithContext (or req.WithContext) so signer.Sign and credentials.Get observe cancellation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+87
to
90
| // AWSCredentialsProvider is the interface used to retrieve AWS credentials. | ||
| type AWSCredentialsProvider interface { | ||
| Retrieve(ctx context.Context) (credentials.Value, error) | ||
| } |
Comment on lines
+153
to
+156
| val := v.(*Value) | ||
| if val == nil || !val.HasKeys() { | ||
| return Value{}, false | ||
| } |
Comment on lines
+100
to
+103
| // Sign will set the request's Body to be the `body` parameter passed in. If an | ||
| // empty body parameter passed to Sign, the request's Body field will be set to | ||
| // nil. Its important to note that this functionality will not change the | ||
| // request's ContentLength of the request. |
Comment on lines
91
to
93
| // Validates that a single call to Retrieve is shared between two calls to Get | ||
| time.Sleep(10 * time.Millisecond) | ||
| stub.done <- struct{}{} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GODRIVER-3567
Summary
Update the
Retrieveinterface:Background & Motivation
ext/awsauthsubmodule will be added in the following PR.