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

[CT-2860] Add IAM Authentication to dbt-postgres #8187

Conversation

christopherscholz
Copy link

@christopherscholz christopherscholz commented Jul 22, 2023

resolves dbt-labs/dbt-postgres#14
docs #3798

Problem

It is not possible to use AWS IAM Authentication with dbt-postgres.

Solution

This adds IAM Authentication to dbt-postgres in order to connect to Amazon RDS Aurora PostgreSQL and Amazon RDS for PostgreSQL.
It does it in the same fashion as dbt-redshift. It is refactoring the connection function by providing it through a connection factory.

The following profile configurations change

  • password becomes optional
  • method is added: default database; options: database, iam
  • iam_profile is added: default None, used to overwrite default AWS IAM profile
  • region is added: default None, used to overwrite default AWS Region

If iam is chosen as method, then before connecting to the PostgreSQL db using psycopg2.connect, it would

  • open boto3 session
  • create rds client
  • get the auth token by calling generate_db_auth_token and pass it to kwargs as password

if method is not set or set to database, the current password connection is used

Checklist

  • I have read the contributing guide and understand what's expected of me
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX

@cla-bot cla-bot bot added the cla:yes label Jul 22, 2023
@christopherscholz christopherscholz changed the title Add IAM Authentication to dbt-postgres [CT-2860] Add IAM Authentication to dbt-postgres Jul 22, 2023
@christopherscholz christopherscholz force-pushed the feature-add-iam-authentication-to-dbt-postgres branch from 66d8c77 to 7a66fe3 Compare July 22, 2023 22:43
@christopherscholz christopherscholz force-pushed the feature-add-iam-authentication-to-dbt-postgres branch from 7a66fe3 to 81ba4a6 Compare July 23, 2023 16:13
@christopherscholz christopherscholz marked this pull request as ready for review July 23, 2023 16:14
@christopherscholz christopherscholz requested review from a team as code owners July 23, 2023 16:14
@christopherscholz christopherscholz requested review from mikealfare, emmyoop and aranke and removed request for a team July 23, 2023 16:14
@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.02%. Comparing base (518eb73) to head (81ba4a6).
Report is 377 commits behind head on main.

❗ Current head 81ba4a6 differs from pull request most recent head 8917638. Consider uploading reports for the commit 8917638 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8187      +/-   ##
==========================================
- Coverage   86.22%   83.02%   -3.21%     
==========================================
  Files         174      174              
  Lines       25515    25498      -17     
==========================================
- Hits        22001    21169     -832     
- Misses       3514     4329     +815     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@christopherscholz christopherscholz force-pushed the feature-add-iam-authentication-to-dbt-postgres branch 2 times, most recently from 81ba4a6 to 532d392 Compare July 28, 2023 22:53
@christopherscholz
Copy link
Author

I have fixed the tests. Also tested it against a real Amazon RDS for PostgreSQL using IAM Auth, which worked for me without any issue.

@christopherscholz christopherscholz force-pushed the feature-add-iam-authentication-to-dbt-postgres branch 3 times, most recently from 46274f0 to 30de338 Compare July 29, 2023 21:36
@christopherscholz
Copy link
Author

christopherscholz commented Jul 29, 2023

I have changed the Credentials validation flow a bit. My goal was to keep it as restrictive as before without changing too much code.

The classmethod PostgresCredentials.validate (hologram.JsonSchemaMixin.validate) validates a Dict against the class.

I have changed it, so that the dataclass PostgresCredentials is a union of all specific credential methods. But instead of only validate against this one, I have changed the validate classmethod to

  1. validate against itself as before
  2. if no error is risen then check which authentication method is chosen
  3. validate also against the extenden PostgresCredentials dataclass for the specific method

This way password is still required for database authentication and at the same time is forbidden for IAM authentication.

@christopherscholz christopherscholz force-pushed the feature-add-iam-authentication-to-dbt-postgres branch from 30de338 to 6760e82 Compare July 29, 2023 22:47
@christopherscholz christopherscholz force-pushed the feature-add-iam-authentication-to-dbt-postgres branch from 6760e82 to 8917638 Compare July 29, 2023 22:58
@dbeatty10 dbeatty10 added the Team:Adapters Issues designated for the adapter area of the code label Jul 30, 2023
Copy link
Contributor

This PR has been marked as Stale because it has been open with no activity as of late. If you would like the PR to remain open, please comment on the PR or else it will be closed in 7 days.

@github-actions github-actions bot added the stale Issues that have gone stale label Jan 27, 2024
@github-actions github-actions bot removed the stale Issues that have gone stale label Jan 28, 2024
@dbeatty10 dbeatty10 added the community This PR is from a community member label Mar 22, 2024
@dbeatty10 dbeatty10 added dbt-postgres Needs migration to dbt-postgres repo enhancement New feature or request labels Apr 9, 2024
@dbeatty10
Copy link
Contributor

Thanks for taking the time to open this PR @christopherscholz! Since opening, we've decoupled dbt Adapters from dbt Core, and this code now lives in a separate repo: dbt-postgres.

A consequence of the decoupling is that PR can't be merged anymore as is, so we're closing it. For more context see #9171.

The linked issue has already been transferred. Please don't hesitate to re-open your proposed code changes within this PR in the dbt-postgres repo.

@dbeatty10 dbeatty10 closed this Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes community This PR is from a community member dbt-postgres Needs migration to dbt-postgres repo enhancement New feature or request Team:Adapters Issues designated for the adapter area of the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-2860] [Feature] Add AWS IAM Authentication to dbt-postgres
2 participants