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

Adding namespace support for Snowflake #5486

Merged
merged 11 commits into from
Nov 14, 2024

Conversation

galvana
Copy link
Contributor

@galvana galvana commented Nov 12, 2024

Closes LA-120

Description Of Changes

Updates the SnowflakeConnector to use a namespaced query config. This allows queries to specify the correct database name and schema without relying on these values from the connectionconfig.

Code Changes

  • Added the SnowflakeNamespaceMeta schema (database_name and schema)
  • Updated SnowflakeConnector and SnowflakeQueryConfig
  • Updated the Snowflake tests to run access and erasures with
    • Connection-level dataset name and schema (previous functionality)
    • Dataset-level dataset name and schema (new functionality)

Steps to Confirm

Load datasets

  1. Start the Fides webserver, Admin UI, and Privacy Center.
  2. Navigate to Data inventory > Manage datasets and click on Add dataset. Upload these datasets snowflake_example_test_dataset.txt and snowflake_example_test_dataset_with_namespace.txt.

Configure integration

  1. Navigate to Data inventory > Add systems and click on Add a system.
  2. Name it Snowflake and hit Save.
  3. Navigate to the Integrations tab and select Snowflake from the drop down. Add the credentials from 1password (it's ok to skip Private key, Passphrase, and Role).
  4. Test the integration.
  5. Link the snowflake_example_test_dataset to the integration.

Run tests

  1. Submit an access request via the Privacy Center for identity [email protected]. Approve the request and verify it is successful. This is the existing functionality.
  2. Navigate back to the Snowflake integration and delete Database and Schema, hit Save. Submit another privacy request, it should fail this time.
  3. Unlink the snowflake_example_test_dataset and link snowflake_example_test_dataset_with_namespace, hit Save. Submit another request, the request should use the namespace values and succeed.

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
  • Followup issues:
    • Followup issues created (include link)
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:

Copy link

vercel bot commented Nov 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
fides-plus-nightly ⬜️ Ignored (Inspect) Visit Preview Nov 14, 2024 7:32pm

Copy link

cypress bot commented Nov 12, 2024

fides    Run #11008

Run Properties:  status check passed Passed #11008  •  git commit 0f77bdd5a8 ℹ️: Merge 649e6f0f047418813371c37e2b8604517bd5fdda into 52ac3301bef835ab4671d258eb80...
Project fides
Branch Review refs/pull/5486/merge
Run status status check passed Passed #11008
Run duration 00m 37s
Commit git commit 0f77bdd5a8 ℹ️: Merge 649e6f0f047418813371c37e2b8604517bd5fdda into 52ac3301bef835ab4671d258eb80...
Committer Adrian Galvan
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 4
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

Copy link

codecov bot commented Nov 12, 2024

Codecov Report

Attention: Patch coverage is 52.00000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 85.18%. Comparing base (d14d031) to head (48b63eb).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/fides/api/service/connectors/query_config.py 23.07% 10 Missing ⚠️
src/fides/api/service/connectors/sql_connector.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5486      +/-   ##
==========================================
- Coverage   85.20%   85.18%   -0.03%     
==========================================
  Files         386      387       +1     
  Lines       24247    24265      +18     
  Branches     2644     2646       +2     
==========================================
+ Hits        20660    20669       +9     
- Misses       3033     3042       +9     
  Partials      554      554              

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

@galvana galvana added the run unsafe ci checks Runs fides-related CI checks that require sensitive credentials label Nov 12, 2024
Copy link
Contributor

@eastandwestwind eastandwestwind left a comment

Choose a reason for hiding this comment

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

Overall looks good, but wanted to call out 2 areas:

  1. Docs- are there any current customer-facing docs around options for yaml structure re namespaces? Can we update docs to say that we currently support this for Snowflake and BigQuery?
  2. I know this is a draft PR, but calling out to remember to write specific QA steps in the description 👍

@galvana galvana marked this pull request as ready for review November 13, 2024 22:43
@eastandwestwind
Copy link
Contributor

Can you add a Changelog row as well @galvana?

@galvana galvana merged commit 77e87be into main Nov 14, 2024
14 checks passed
@galvana galvana deleted the LA-120-add-namespace-support-for-snowflake branch November 14, 2024 19:33
Copy link

cypress bot commented Nov 14, 2024

fides    Run #11009

Run Properties:  status check passed Passed #11009  •  git commit 77e87be5fe: Adding namespace support for Snowflake (#5486)
Project fides
Branch Review main
Run status status check passed Passed #11009
Run duration 00m 38s
Commit git commit 77e87be5fe: Adding namespace support for Snowflake (#5486)
Committer Adrian Galvan
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 4
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run unsafe ci checks Runs fides-related CI checks that require sensitive credentials
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants