Resolves issues with static credentials auth and add tests #214
Conversation
| value, i.e. AWS_SECRET_ACCESS_KEY. | ||
| type: string | ||
| default: AWS_SECRET_ACCESS_KEY | ||
| default: $AWS_SECRET_ACCESS_KEY |
There was a problem hiding this comment.
Should the description be updated if $ is required ?
There was a problem hiding this comment.
You are right, I will create a new PR for this.
There was a problem hiding this comment.
this is a breaking change and should have been marked as 6.0, or even 5.2 at the very least, definitely not a patch release.
There was a problem hiding this comment.
@mscrivo I was considering that, but for what I saw this was supposed to be working in this way since some a lot of versions back, and the behavior was broken. If this was working someway with this implementation please let me know and I will update the versioning.
There was a problem hiding this comment.
we had to update all our usages of:
- aws-cli/setup:
aws_access_key_id: KEY
region: REGION
aws_secret_access_key: SECRET
to:
- aws-cli/setup:
aws_access_key_id: $KEY
region: $REGION
aws_secret_access_key: $SECRET
because it broke. Not a huge deal, just surprising to have happen on a patch release.
There was a problem hiding this comment.
got it, as I said, I didn't expect this to be working with only credentials in any scenario, but if it was, you are right, this cannot be a patch.
There was a problem hiding this comment.
I'm going to revert this change in a new patch and release it again in a new minor version. Thanks @mscrivo for notifying that this was actually working.
)" This reverts commit b59332d.
4 participants
The configure script was still managing the static credentials as if they were environment_variable type, and they are string now.
I'm updating the defaults and the script to work as they are supposed to, and adding a new test that validate the setup when using static credentials authentication.