fix: add option to use private key for authentication #671
Conversation
|
Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA. In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR. CLA has not been signed by users: @jschenz-chwy |
jschenz-chwy
changed the title
|
I thought I had already signed the contributor license agreement, but that might have been under my personal account. I just submitted the form for my work account. |
|
Just FYI, multiple people are waiting for astronomer/astronomer-cosmos#267 which, as far as I understand it, is blocked by this PR. Is there anything we can do to help review this change? |
|
+1, this would be great to get in! Happy to help however I can |
|
Agreed! Would love to get this functionality, also happy to assist wherever I can |
|
@dbeatty10 also happy to help getting this PR merged |
Fleid
added
ready_for_review
|
As this is not covered with any unit/functional tests has anyone done any manual validation that this will work? |
|
@colin-rogers-dbt I did run it in my own k8s environment, and it worked as expected. Not sure if anyone else following this has done something similar, but it would be good to have more than just my experience to share. |
|
@jschenz-chwy I just updated the checklist within the PR description: 
We'll definitely need public-facing documentation for PEM vs. DER private keys, could you use the following link to open an issue and describe the content that should go in the docs? Even better if you're willing to open a PR to update the docs too! @colin-rogers-dbt or @mikealfare can share what kind of testing they want for this PR. |
@colin-rogers-dbt, I don't even see where we're testing the current connection method in our functional tests, do you? I think this is just a miss and manual verification might be sufficient. If we wanted to automate it, we'd need to add a few secrets to the repo before CI would pass. |
@mikealfare Agreed, I'll open an issue to add these tests, in the meantime I'm comfortable moving forward since this has been manually tested |
|
I have opened a PR for the doc change here: dbt-labs/docs.getdbt.com#3786 |
|
beat me to it, thanks @jschenz-chwy |
## What are you changing in this pull request and why? <!--- Describe your changes and why you're making them. If linked to an open issue or a pull request on dbt Core, then link to them here! To learn more about the writing conventions used in the dbt Labs docs, see the [Content style guide](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/content-style-guide.md). --> This change describes the option of using a plain-text PEM format private key for authentication with Snowflake added in dbt-labs/dbt-snowflake#671. ## Checklist <!-- Uncomment if you're publishing docs for a prerelease version of dbt (delete if not applicable): - [ ] Add versioning components, as described in [Versioning Docs](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/single-sourcing-content.md#versioning-entire-pages) - [ ] Add a note to the prerelease version [Migration Guide](https://github.com/dbt-labs/docs.getdbt.com/tree/current/website/docs/guides/migration/versions) --> - [X] Review the [Content style guide](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/content-style-guide.md) and [About versioning](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/single-sourcing-content.md#adding-a-new-version) so my content adheres to these guidelines. - [X] Add a checklist item for anything that needs to happen before this PR is merged, such as "needs technical review" or "change base branch." Checklist: - [x] The PR dbt-labs/dbt-snowflake#671 needs to be merged - [ ] This doc will need to be updated with the dbt-snowflake version that supports the PEM key option.
* adding option for pem formatted private key * adding changie --------- Co-authored-by: Doug Beatty <[email protected]>
resolves #619
Description
This PR adds the option of using a pem private key.
Checklist
changie newto create a changelog entry