-
Notifications
You must be signed in to change notification settings - Fork 132
MQE-1918 MFTF AWS Secrets Manager - Local Use #543
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
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.
Currently, if the credentials to the secret manager are not set correctly, the tests end up throwing exception like this -
MFTF uses Credential Storage in the following precedence: .credentials file, HashiCorp Vault and AWS Secret Manager. You need to configure at least one to use _CREDS in tests. And make sure key/value exists.
Not sure why it doesn't throw this error instead - Unable to create AWS Secret Manager client
eg: CREDENTIAL_AWS_SECRET_MANAGER_PROFILE
=asdf (not existant)
CREDENTIAL_AWS_SECRET_MANAGER_REGION
=us-east-2 (non existant)
.../FunctionalTestingFramework/DataGenerator/Handlers/SecretStorage/AwsSecretManagerStorage.php
Outdated
Show resolved
Hide resolved
.../FunctionalTestingFramework/DataGenerator/Handlers/SecretStorage/AwsSecretManagerStorage.php
Outdated
Show resolved
Hide resolved
.../FunctionalTestingFramework/DataGenerator/Handlers/SecretStorage/AwsSecretManagerStorage.php
Outdated
Show resolved
Hide resolved
src/Magento/FunctionalTestingFramework/DataGenerator/Handlers/CredentialStore.php
Show resolved
Hide resolved
One more thing, There is no error if you use non-existing profile when create AWS client. There will be error when access the secret. Thus the generic error message. |
Since the CI/CD story may end up changing the order will it be better to modify the exception message to - |
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.
Implementation looks good. Needs branch update + a build trigger just to triple check
aa287d2
to
fd9e721
Compare
This is incorporated in PR # 554 |
Description
Fixed Issues (if relevant)
Contribution checklist