datamill-co / target-redshift

A Singer.io Target for Redshift
MIT License
23 stars 17 forks source link

Allow STS token to be used as a config variable #31

Closed Limess closed 4 years ago

Limess commented 5 years ago

This allows a workaround for temporary credential usage, allowing workarounds when using roles.

The example use case here is using Apache Airflow: I wish to use the built in credentials providers which airflow uses - to do this I'm using IAM roles which are assumed by boto3 automatically, and then taking the access key/secret key/sts token and providing them to libraries which need them.

It'd be equally useful to allow an AWS profile or role to be used in future, specifically to separate the role used by Redshift COPY and the S3 credentials to do the multipart upload.

AlexanderMann commented 5 years ago

@Limess I don't have any problem with this change overall. The work done here is pretty thorough. Have you run this code at all in your setup to confirm it works? Also, the sts setup, does that still require the token and secret config vars?

The big concern I have with all of the myriad of connection options for AWS is always trying to setup tests to make sure all of this stuff works. Each one is a unicorn. So if you can verify this is working for your setup, then that would alleviate some concern.

AlexanderMann commented 5 years ago

@awm33 I'm game for this contribution, do you have any thoughts?

awm33 commented 5 years ago

@AlexanderMann I'm good. I don't think it replaces the need or closes the issue for role support, but gets a step closer.

Limess commented 5 years ago

I've tested this with our setup - installing it using a git pip url - and it functions as expected. I agree that there's a myriad of ways to use AWS credentials and various combinations which are then incompatible (e.g role + token, secret + STS token and token but no secret), but I believe that generally the user will be aware of the requirements based on their choice of authentication method, so it's potentially not necessary to setup a test suite for each combination.

The token and secret are both still mandatory when using an STS token.

AlexanderMann commented 5 years ago

@Limess there are some other changes I think which are coming down the pipe so I'd expect this to land in a release late this week/early next week. Thanks for the help!

AlexanderMann commented 4 years ago

@Limess dunno how this went missing for so long, but the release is going up today. Sorry about that!