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

Add support for impersonating a service account with BigQuery #2677

Merged
merged 6 commits into from
Aug 4, 2020

Conversation

bbhoss
Copy link
Contributor

@bbhoss bbhoss commented Aug 1, 2020

resolves #2672

Description

See #2672. Opening up this draft as I hope to get some early feedback. Tests and docs are the main TODOs, I still need to figure out how the testing works.

Checklist

  • I have signed the CLA
  • 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
  • I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

@cla-bot
Copy link

cla-bot bot commented Aug 1, 2020

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Preston Marshall.
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@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@bbhoss
Copy link
Contributor Author

bbhoss commented Aug 1, 2020

I added a test but it doesn't seem very useful. Not really sure the existing tests are setup in a way to easily test this without major mocking, I don't see any other authentication tests. Let me know what else is needed.

@bbhoss bbhoss marked this pull request as ready for review August 1, 2020 19:42
@jtcohen6
Copy link
Contributor

jtcohen6 commented Aug 2, 2020

@bbhoss You're right, in general we don't have as much testing for non-default authentication methods (Snowflake SSO being the big exception). In this specific case, since CircleCI runs our integration tests against BigQuery using a service account (and it doesn't sound like they support oauth), I don't know if we have a better option.

I'm still going to test this locally by oauth'ing into our sandboxed BigQuery account and impersonating Circle's service account.

@bbhoss
Copy link
Contributor Author

bbhoss commented Aug 3, 2020

@jtcohen6 added docs here in case you need a walkthrough for testing locally. Let me know if there's anything else I can do :)

Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for writing up those docs.

We don't have rigorous tests for this, but the code change is quite self-contained. As long as it worked for you in your project and for me in a dummy project, I feel good about merging this in (tomorrow)

CHANGELOG.md Show resolved Hide resolved
Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing!

@jtcohen6 jtcohen6 merged commit 36fda28 into dbt-labs:dev/marian-anderson Aug 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow impersonation of Google Cloud service accounts with BigQuery
2 participants