Use RemoteCredProvider instead of EC2RoleProvider

[jefferai]

This better handles ECS and other future use-cases.

[jefferai]

@joelthompson It's not letting me request a review from you but can you take a look?

[joelthompson]

@jefferai -- this generally seems fine (modulo the compile error from Travis), but I don't have too much experience with the golang SDK. I would suggest updating the documentation to reflect these changes, i.e.:

• In the AWS secret backend, for the region option, mention the environment variable, and updating "Note: the client uses the official AWS SDK and will use environment variable or IAM role-provided credentials if available." to also add ECS task role.
• In the AWS auth backend, where it states, "environment variables, ~/.aws/credentials, IAM instance profile in that order" to add ECS task role
• The S3 storage backend, for the region option, note that it'll use the AWS_REGION environment variable if AWS_DEFAULT_REGION isn't set (and if you're in there, fix the endpoint parameter doc as it has the wrong environment variable)

There's some more cleanup work that could be done to make things more consistent, e.g., move the DynamoDB physical backend to use the credential helper, as it won't pick up creds from ECS tasks, be consistent about using AWS_REGION and AWS_DEFAULT_REGION environment variables, and make AWS_REGION take precedence over AWS_DEFAULT_REGION which is what the golang SDK does -- though that's a small a breaking change. But, I'd suggest just getting this fix in now for the next release, and potentially do the cleanup work later.

[jefferai]

be consistent about using AWS_REGION and AWS_DEFAULT_REGION environment variables, and make AWS_REGION take precedence over AWS_DEFAULT_REGION which is what the golang SDK does

I didn't even see AWS_DEFAULT_REGION in the sdk, I thought that was something Vault-specific.

Anyways, I've updated with your feedback, except for updating the docs. Can you take another look?

[joelthompson]

It might make sense to factor out the region environment variable checking since it's repeated in a few places.

I didn't even see AWS_DEFAULT_REGION in the sdk, I thought that was something Vault-specific.

See, e.g., this

[joelthompson]

Do you also want to check AWS_DEFAULT_REGION here?

[jefferai]

Done!

[joelthompson]

LGTM!

[briankassouf]

Was this incorrect before?

[jefferai]

Yep. Looks like it was copypasta. The code actually uses AWS_S3_ENDPOINT though.