Skip to content
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

feat: add support iam role support for warehouse destinations #2496

Merged
merged 12 commits into from Sep 30, 2022

Conversation

koladilip
Copy link
Contributor

@koladilip koladilip commented Sep 28, 2022

Description

As part of AWS FTR we need to support IAM roles instead of access keys for all AWS interactions. We already supported for file and streaming destinations using common utils. Now I am doing the same for warehouse destinations where there is dependency on S3 for Snowflake, deltalake, Redshift and using common utils we can automatically enable support for role based authentication.

In that process I have update glue client creation to use the same common utils to support role based auth.

Notion Ticket

https://www.notion.so/rudderstacks/AWS-FTR-for-Redshift-e7dc80ee847a49d6b7bb7fc03aeeffc2

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

@koladilip koladilip changed the title Feat.add roles support for redshift feat: add roles support for redshift Sep 28, 2022
@koladilip koladilip changed the title feat: add roles support for redshift add support iam role support for warehouse destinations Sep 28, 2022
@koladilip koladilip changed the title add support iam role support for warehouse destinations feat: add support iam role support for warehouse destinations Sep 28, 2022
@codecov
Copy link

codecov bot commented Sep 28, 2022

Codecov Report

Base: 41.35% // Head: 41.47% // Increases project coverage by +0.11% 🎉

Coverage data is based on head (c6c5f74) compared to base (85b0342).
Patch coverage: 53.75% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2496      +/-   ##
==========================================
+ Coverage   41.35%   41.47%   +0.11%     
==========================================
  Files         175      175              
  Lines       36266    36288      +22     
==========================================
+ Hits        14998    15050      +52     
+ Misses      20333    20303      -30     
  Partials      935      935              
Impacted Files Coverage Δ
...connection-tester/destination_connection_tester.go 15.23% <0.00%> (-0.15%) ⬇️
warehouse/clickhouse/clickhouse.go 0.00% <0.00%> (ø)
warehouse/deltalake/deltalake.go 0.00% <0.00%> (ø)
warehouse/mssql/mssql.go 0.46% <0.00%> (-0.01%) ⬇️
warehouse/postgres/postgres.go 0.00% <0.00%> (ø)
warehouse/redshift/redshift.go 0.00% <0.00%> (ø)
warehouse/slave.go 1.25% <0.00%> (-0.01%) ⬇️
warehouse/snowflake/snowflake.go 0.00% <0.00%> (ø)
warehouse/upload.go 6.49% <0.00%> (ø)
warehouse/utils/utils.go 60.60% <43.47%> (-0.19%) ⬇️
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@lvrach
Copy link
Member

lvrach commented Sep 28, 2022

@koladilip can you provide additional details and context regarding the changes in this PR?

@koladilip
Copy link
Contributor Author

koladilip commented Sep 29, 2022

@koladilip can you provide additional details and context regarding the changes in this PR?

@lvrach I have updated the issue description, let me know if you need more details are needed.

@koladilip koladilip requested review from achettyiitr and cisse21 and removed request for dhawal1248 September 29, 2022 17:57
@koladilip
Copy link
Contributor Author

Tested with redshift and glue, it is working fine.

utils/misc/misc.go Outdated Show resolved Hide resolved
warehouse/utils/utils.go Outdated Show resolved Hide resolved
koladilip and others added 2 commits September 30, 2022 13:09
Co-authored-by: Francesco Casula <fracasula@users.noreply.github.com>
Co-authored-by: Francesco Casula <fracasula@users.noreply.github.com>
@koladilip koladilip merged commit 867123a into master Sep 30, 2022
This was referenced Sep 30, 2022
@github-actions github-actions bot deleted the feat.add-roles-support-for-redshift branch December 1, 2022 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants