-
Notifications
You must be signed in to change notification settings - Fork 64
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
PYTHON-3942 Enable AzureKMS through AWS Secrets Manager #354
Conversation
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.
Updated scripts pass with C driver.
Removal of orphaned roles is much appreciated. LGTM.
|
||
import boto3 | ||
import botocore.exceptions | ||
|
||
AWS_ROLE_ARN = "arn:aws:iam::857654397073:role/drivers-test-secrets-role" |
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 we set this in an environment variable? Not a huge fan of setting even non-secret credentials inside a script.
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.
There are other places we hard-code ARNs or links in this repo. The intent here was that it should be entirely self-contained without drivers needing to manually update EVG project config, which is part of the reason for having this script in general.
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.
I'd prefer we migrate away form hard-coding ARNs in general, but that's a different discussion entirely. In the context of the existing repo, makes sense.
Key Vault Crypto User
role as part of teardown, ifAZUREKMS_SCOPE
is set. We were adding orphaned roles to the key vault.