hashicorp / go-secure-stdlib

Mozilla Public License 2.0
64 stars 24 forks source link

awsutil: ensure GenerateCredentialChain checks envs #80

Closed fairclothjm closed 1 year ago

fairclothjm commented 1 year ago

https://github.com/hashicorp/go-secure-stdlib/pull/57 caused a regression in Vault. In that PR the AWS environment variable checks in GenerateCredentialChain were moved to NewCredentialsConfig for AWS_ROLE_ARN, AWS_WEB_IDENTITY_TOKEN_FILE, and AWS_ROLE_SESSION_NAME. This caused users of the awsutil library that call GenerateCredentialConfig to fail to set the above values if they are depending on env variables to be read.

Another solution could be to find all users of go-secure-stdlib’s awsutil and ensure env vars are properly read and set in the config before calling GenerateCredentialConfig like in https://github.com/hashicorp/vault/pull/21930. However, I think a this PR is a better resolution. With this change we shouldn't have to make any changes anywhere else.

This change was tested with the following steps:

Additionally, this has been tested against the Boundary E2E tests by @ddebko. Thanks!

Closes https://github.com/hashicorp/go-secure-stdlib/issues/71

fairclothjm commented 1 year ago

might be nice to add some unit tests though so we don't get another regression? This is a pretty long function with lots of edge cases.

@swenson Agreed! I will address adding some tests in another PR if you are ok with that?