-
Notifications
You must be signed in to change notification settings - Fork 98
PYTHON-3256 Obtain AWS credentials for CSFLE in the same way as for MONGODB-AWS #448
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
juliusgeo
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!
| import copy | ||
|
|
||
| try: | ||
| from pymongo_auth_aws.auth import _aws_temp_credentials |
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.
Could you open a new ticket to make _aws_temp_credentials a public API?
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.
| return | ||
| if not _HAVE_AUTH_AWS: | ||
| raise RuntimeError( | ||
| "MONGODB-AWS authentication requires pymongo-auth-aws: " |
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.
"MONGODB-AWS authentication" is not relevant here. Should say "on demand aws credentials require..."
Should we add pymongo-auth-aws to pymongo[encryption] too?
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.
Opened #452 and updated mongodb/mongo-python-driver#1035
| This is a separate function so it can be overridden in unit tests.""" | ||
| if 'aws' not in kms_providers: | ||
| return | ||
| if len(kms_providers['aws']): |
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.
If we already provided non-empty aws creds to libmongocrypt is this state even possible to reach?
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.
This was in preparation for other types of on-demand credentials.
Re-do of #440 after an accidental merge and revert.