[Unified Fides Resources] Update Test Env Setup and Quickstart #2225

pattisdr · 2023-01-13T00:36:50Z

Closes N/A

Code Changes

Adjust test env setup and quickstart to call a different set of endpoints when creating datasetconfigs (upsert the ctl dataset first and then link to the datasetconfig) instead of the old endpoint (patch a datasetconfig)

Steps to Confirm

Run nox -s test_env
Verify everything came up as expected
nox -s clean
Run nox -s quickstart - run quickstart through to the end

Pre-Merge Checklist

All CI Pipelines Succeeded
Documentation:
- documentation complete, PR opened in fidesdocs
- documentation issue created in fidesdocs
Issue Requirements are Met
Relevant Follow-Up Issues Created
Update CHANGELOG.md

Description Of Changes

Get tests passing on unified-fides-resources branch after update with main.

Also: Remove remaining uses of ops dataset endpoints in fides that we'd like to eventually deprecate (not a blocker for unified-fides-resources): deprecation will happen as a follow-up in the future: #2092. These endpoints still work, but are riskier because a dataset is upserted in the background, and we don't want people to unintentionally override their datasets.

Instead of a patch to /connection/connection_key/dataset do a post to /dataset/upsert and then a patch to /connection/connection_key/datasetconfig in both the test env setup and the quickstart

- Instead of a patch to /connection/connection_key/dataset do a post to /dataset/upsert and then a patch to /connection/connection_key/datasetconfig. - Fix logging issue.

pattisdr · 2023-01-13T02:10:48Z

~~@ThomasLaPiana wanted to see if you had any ideas here. The unified-fides-resources feature branch has been consistently failing the fides_db_scan checks with:~~

sqlalchemy.exc.OperationalError: (psycopg2.OperationalError) connection to server at "fides-db" (172.18.0.3), port 5432 failed: FATAL:  database "fides_test" does not exist`.

~~Both Allison and I have run into this on respective PR's against unified-fides-resources. I've been updating the branch regularly with main, where it doesn't seem to be failing.~~

I wonder if it's related to populating the application db versus the test db.

EDIT: Nevermind Thomas, I spotted it, this branch was trying to connect to the test db not the application db after all for some reason.

…es" should be scanning the application db not the test db.

pattisdr · 2023-01-13T16:53:39Z

.fides/fides.toml

@@ -7,7 +7,7 @@ db = "fides"
 test_db = "fides_test"

 [credentials]
-app_postgres = {connection_string="postgresql+psycopg2://postgres:fides@fides-db:5432/fides_test"}
+app_postgres = {connection_string="postgresql+psycopg2://postgres:fides@fides-db:5432/fides"}


I'm not sure why I made this change on this branch, that was a mistake - this should be looking at the running application db not the test db, and it was why we've had a failing test on this branch for awhile.

pattisdr · 2023-01-13T16:54:20Z

scripts/quickstart.py

@@ -324,7 +344,7 @@ def create_dsr_policy(key: str):
    if response.ok:
        policies = (response.json())["succeeded"]
        if len(policies) > 0:
-            logger.info("Created fidesops policy with key=%s via {url}", key)
+            logger.info("Created fidesops policy with key={} via {}", key, url)


This had been causing the quickstart to break FYI, not sure if people are using the quickstart

I believe @seanpreston said it was still being used for manual testing. If that's the case I think we should also be running it during CI checks (another ticket for another time)

…aset key with new saas fixtures.

… sub categories to avoid conflict when resolving data categories.

pattisdr · 2023-01-17T14:43:56Z

@allisonking the main reason I put up this PR was to get relevant passing on unified-fides-resources after I got it up to date with main. It fixes that failing fides db scan issue we were both running into, and gets some of the newly added saas connectors adjusted so they work with unified-fides-resources. The remaining failing tests are unrelated to this work.

allisonking

🚀

allisonking · 2023-01-17T15:12:02Z

data/saas/dataset/recharge_dataset.yml

@@ -91,7 +91,6 @@ dataset:
            fidesops_meta:
              data_type: string
          - name: shopify_customer_id
-            data_categories: [system.operations]


how come this had to be deleted?

Ah sorry, with the unified-fides-resources merge, the ops side has a constraint where you can't put data categories at the "object" level. If you have nested data, data categories need to be on the sub-fields. This is because DSR's use the data categories to figure out which fields to send to the user. If the top level field and sub field have different data categories that conflict, we could get in a state where we don't know which data we should send.

pattisdr · 2023-01-17T15:51:34Z

Thanks for the review @allisonking

Update setup/dataset and quickstart to use the new dataset endpoints.

1f618a2

- Instead of a patch to /connection/connection_key/dataset do a post to /dataset/upsert and then a patch to /connection/connection_key/datasetconfig. - Fix logging issue.

pattisdr added Unified Fides Resources run unsafe ci checks Runs fides-related CI checks that require sensitive credentials and removed run unsafe ci checks Runs fides-related CI checks that require sensitive credentials labels Jan 13, 2023

pattisdr mentioned this pull request Jan 13, 2023

Deprecate patch_datasets and patch_yaml_datasets endpoints #2092

Closed

pattisdr marked this pull request as ready for review January 13, 2023 14:40

pattisdr self-assigned this Jan 13, 2023

Fix rogue change - "fides scan dataset db --credentials-id app_postgr…

2570413

…es" should be scanning the application db not the test db.

pattisdr commented Jan 13, 2023

View reviewed changes

pattisdr added 2 commits January 13, 2023 22:07

Remove instances of trying to create DatasetConfigs with obsolete dat…

27a4c1f

…aset key with new saas fixtures.

Remove data category from object level field - should be specified on…

2ae2488

… sub categories to avoid conflict when resolving data categories.

pattisdr requested a review from allisonking January 17, 2023 14:41

pattisdr requested a review from TheAndrewJackson January 17, 2023 14:52

allisonking approved these changes Jan 17, 2023

View reviewed changes

pattisdr merged commit 5306c2d into unified-fides-resources Jan 17, 2023

pattisdr deleted the unified-fides-resources-setup-and-quickstart branch January 17, 2023 15:52

pattisdr mentioned this pull request Jan 17, 2023

Unified Fides Resources Feature #2254

Merged

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Unified Fides Resources] Update Test Env Setup and Quickstart #2225

[Unified Fides Resources] Update Test Env Setup and Quickstart #2225

pattisdr commented Jan 13, 2023 •

edited

Loading

pattisdr commented Jan 13, 2023 •

edited

Loading

pattisdr Jan 13, 2023

pattisdr Jan 13, 2023

ThomasLaPiana Jan 17, 2023

pattisdr commented Jan 17, 2023 •

edited

Loading

allisonking left a comment

allisonking Jan 17, 2023

pattisdr Jan 17, 2023

pattisdr commented Jan 17, 2023

[Unified Fides Resources] Update Test Env Setup and Quickstart #2225

[Unified Fides Resources] Update Test Env Setup and Quickstart #2225

Conversation

pattisdr commented Jan 13, 2023 • edited Loading

Code Changes

Steps to Confirm

Pre-Merge Checklist

Description Of Changes

pattisdr commented Jan 13, 2023 • edited Loading

pattisdr Jan 13, 2023

Choose a reason for hiding this comment

pattisdr Jan 13, 2023

Choose a reason for hiding this comment

ThomasLaPiana Jan 17, 2023

Choose a reason for hiding this comment

pattisdr commented Jan 17, 2023 • edited Loading

allisonking left a comment

Choose a reason for hiding this comment

allisonking Jan 17, 2023

Choose a reason for hiding this comment

pattisdr Jan 17, 2023

Choose a reason for hiding this comment

pattisdr commented Jan 17, 2023

pattisdr commented Jan 13, 2023 •

edited

Loading

pattisdr commented Jan 13, 2023 •

edited

Loading

pattisdr commented Jan 17, 2023 •

edited

Loading