adding IAM role authentication support for redshift

[abbywh]

Committer: Abby Whittier [email protected]

resolves #623
docs dbt-labs/docs.getdbt.com/#

Problem

dbt-redshift currently does not natively support the redshift authentication method get-cluster-credentials-with-iam. A dbt customer using IAM roles as db users currently has no way to authenticate as those users.

Solution

There is now a new redshift connection method to authenticate as an IAM role, called IAMR. This strongly resembles the classic IAM authentication, except the user field is unnecessary.

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]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Whittier.
This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email [email protected]
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[abbywh]

Is this being tested now? Any chance we can reopen for tracking purposes?

[cla-bot]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Whittier.
This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email [email protected]
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[cla-bot]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Whittier.
This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email [email protected]
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[colin-rogers-dbt]

@abbywh looks like there's something wrong with your git setup and is throwing off our CLA check

[abbywh]

@colin-rogers-dbt do you have any documentation for your bot? I can explain what happened. I ammended the commit to remove my private email address in favor of my public, and you can see my email in the commit, but it's still blocking. I would happily parse through how to fix it (maybe a git filter repo?) if I can find some documentation around it.

[abbywh]

for anyone digging this up, make sure you update the config as given by the bot, then do git amend --reset-author as opposed to git amend --author="Author Name <[email protected]>".

[dataders]

@jiezhen-chen @soksamnanglim @sathiish-kumar y'all wanna take a look here as well?

[jiezhen-chen]

Instead of introducing a new IAMR auth method, would yall consider having it under IAM? Looks like the behavior of IAM and IAMR are the same except user is now optional, can we optionally pass in self.credential.user only when it is provided in the yml file?

[abbywh]

@jiezhen-chen i also considered that option. would be happy to change it to that, but i was worried about muddling the error cases for the iam authentication.

for example, consider i'm a user that wants to use IAM auth and omits the user field. the error i would get would be something like: Error: IAM role xyz does not have permission GetClusterCredentialWithIam on cluster 123. This might incorrectly lead a user to add that permission. We also couldn't catch that error and throw a different message, because then we wouldn't cover the error case where a user actually intends to use iamr and had that permission misconfigured.

I think this is mostly a stylistic choice that should be left to the maintainers. But we should add a note about that error message to the documentation if we choose to combine the routes. Just let me know which you would prefer knowing this and I will implement it 😃

[jiezhen-chen]

@abbywh Having a IAMR auth method makes sense. I think that if you want to use GetClusterCredentialsWithIAM v2 where IAM role is used, you'd also need to set group_federation to true, according to this issue. I don't see this added as a parameter passed into the connect() method. Is the current IAMR connect() method able to connect using GetClusterCredentialsWithIAM v2 without needing to set group_federation to true?

[abbywh]

yes you are correct there was a mistake in my testing setup. I will make the changes to the tests that should catch this + that code change.

[misteliy]

Hi all, I would like to understand the progress of this issue since we would like to switch completely to IAM (not specifying user explicitly but rather through IAM) - thus IAMR would be great to have as soon as possible.

[abbywh]

+1. This not being deployed is still causing big issues for us.

[joaobernardopa]

Hello everyone, this would actually be a game changer for us as in our current setup we still need to create all external schemas...

[saratvysyaraju]

Hey All,

Any updates on when this feature may get shipped? Our team is currently waiting for this to begin adopting the IAM Auth

[mikealfare]

@abbywh I'll be looking at this PR over the next week or so, with the intent of merging it within the next two weeks. Given the duration for which this has been open, I wanted to check with you to see if you're available to make updates, if needed. I don't mind taking it over, and I would do so in a way that you maintain credit for your contributions either way. But we prefer to offer the contributor the opportunity to complete the PR if so desired.

[mikealfare]

I pushed the merge conflict resolution so that the branch can pull updates from main. It's a large update because this PR pre-dates the core/adapter de-coupling work; I think my IDE said 94 files or so.

[mikealfare]

I left a few comments, mostly around aesthetics and issues arising from the de-coupling work.

The one thing that we'll need to add is an integration test verifying that we can connect with an IAM role. I need to setup some infra for that test to run in CI. In the meantime, we need to do a few things:

• add REDSHIFT_TEST_IAM_ROLE and REDSHIFT_TEST_IAM_PROFILE to ./test.env.example here
• pull these new values into the test fixtures in ./tests/conftest.py here
• create a test module for the new tests: ./tests/functional/adapter/test_auth_methods.py
• create a test in the new module that connects via IAM role and runs a basic query to demonstrate access

The test should probably remove things like user for a positive test. And it's probably worth adding a negative test as well, by removing role.

[mikealfare]

In the interest of time, I will take over this PR in #781 and close this one.