252 saas connector sendgrid #883

ghost · 2022-07-15T16:54:48Z

Purpose

Purpose of this PR is to add Sendgrid connector Contacts access and erasure endpoints/

Changes

Added config for Sendgrid Contact Access and Erasure Endpoints
Added related dataset and configuration
Added associated tests and fixtures to validate above functionality

Checklist

Ticket

Fixes #

…se of ignore_errors enhancement

… remove DELETE endpoint per PR comments

…in tests

adamsachs · 2022-07-15T19:38:05Z

just leaving a note here for some context on why we used a delete endpoint even though initially we had defined an update endpoint -- we wanted to get sendgrid removal functionality in place without requiring #458 to be completed. therefore, we just switched to delete so then we could remove the contact and implicitly remove it for the various mailing lists its a part of.

(this is based off of some discussion with @galvana)

adamsachs · 2022-07-15T19:40:10Z

also noting - before merging, we need to:

update github secrets to include salseforce credentials
once that's done, add the Run Unsafe PR Checks label so that we test the external integration

adamsachs · 2022-07-18T14:09:18Z

tests/integration_tests/saas/test_sendgrid_task.py

+    )
+    assert erasure == {"sendgrid_connector_example:contacts": 1}
+
+    config.execution.MASKING_STRICT = False


what's the purpose of this? perhaps what you intend to do is reset the MASKING_STRICT var back to whatever its value was before it's explicitly set it to False in line 127 above? you'd then need to temporarily store the previous value in a local variable before you set it in line 127.

adamsachs · 2022-07-18T14:14:01Z

tests/integration_tests/saas/test_sendgrid_task.py

+        {"email": sendgrid_erasure_identity_email},
+        get_cached_data_for_erasures(privacy_request.id),
+    )
+    assert erasure == {"sendgrid_connector_example:contacts": 1}


do we know how long deletion takes? if it's a matter of seconds (rather than minutes/hours), then it's probably worth us invoking another poll_for_existence here to confirm that the the contact does not exist anymore, i.e. confirming that our deletion was really successful in the external system.

you'd be able to invoke it similarly to how it's being invoked in sendgrid_fixtures.py now, but just passing False for the existence_desired argument and updating the error_message appropriately. (you'd also probably want to update the _contact_exists method in that module to no longer have a leading _ since it'd not solely be for internal module use.

adamsachs

this generally looks good to me, a few minor comments/things to clarify. they may require some minor changes.

adamsachs · 2022-07-18T18:23:33Z

thanks for the updates to add poll_for_existence into the test! i do think there's a slight problem in how it's being invoked, though.

we should also be sure the tests are still passing with the addition of this polling. can you make sure to run the sendgrid integration tests locally before the next commit?

adamsachs · 2022-07-18T19:31:34Z

@HamzaWaseemOnBench this is looking good! i think the code is good to go, but a couple of minor maintenance things we need to take care of:

looks like some merge conflicts now that need to be resolved (hopefully quick; let me know if you need any help)
can you update CHANGELOG.md file
i think we should get the sendgrid credentials added to GH secrets so that we can apply the Run Unsafe PR Checks label to this PR and run the external integration tests successfully

…ops into 252-saas-connector-sendgrid

adamsachs · 2022-07-19T14:20:57Z

CHANGELOG.md

@@ -132,6 +132,7 @@ The types of changes are:
 * Adds endpoint analytics events [#622](https://github.com/ethyca/fidesops/pull/622)
 * Sample dataset for Salesforce with access configuration [#676](https://github.com/ethyca/fidesops/pull/676)
 * Sample dataset and access configuration for Zendesk (ticket endpoints) [#677](https://github.com/ethyca/fidesops/pull/677)
+* Sample dataset for Sendgrid with access configuration [#883](https://github.com/ethyca/fidesops/pull/883)


@HamzaWaseemOnBench please follow the instructions provided in the PR template that describe how to update the CHANGELOG.md file. there are a few things you need to do here:

merge in main so the most recent CHANGELOG.md file is being appended to. this is to avoid merge conflicts in the file.

your description for this feature needs to be added in the Unreleased section, since you are not adding this feature to the 1.6.0 release, but you're adding it to the next upcoming release. You can also refer to the Keep a Changelog documentation for more information about how to update the changelog.

the description you gave here doesn't really capture the full scope of your changes - i think a more accurate description would read something like: "Access and erasure support for Sendgrid contacts endpoint"

…ctor-sendgrid

adamsachs · 2022-07-19T14:41:21Z

CHANGELOG.md

@@ -17,6 +17,7 @@ The types of changes are:
 * `Security` in case of vulnerabilities.

 ## [Unreleased](https://github.com/ethyca/fidesops/compare/1.6.3...main)
+* Access and erasure support for Sendgrid contacts endpoint [#883](https://github.com/ethyca/fidesops/pull/)


@HamzaWaseemOnBench this needs to be under the Added subsection

…ggestion

adamsachs · 2022-07-19T15:57:06Z

all looks good, thanks for all the updates. noting that the mssql integration test failing is almost certainly not due to changes in this branch, but instead due to #813

* initial sendgrid saas connector integration. access only, contacts only * erasure (update) support for sendgrid * remove unused imports * update sendgrid test fixture to expect 404 response status code because of ignore_errors enhancement * Fixing import order and cleaning up the retry logic for consistency * add sendgrid env var support to makefile and unsafe_pr_checks config. remove DELETE endpoint per PR comments * Added delete endpoint for contacts * Fixing data_path for contacts endpoint * Reverting search query to improve performance and avoid server timeouts * Updated delete endpoint request, used request instead of SaaSRequest in tests * updated imports after check suggestion * Updated code after review * Removed unused variables, imports * Restoring Makefile * Fixed import cryptographic_util error * Misc fixes * Updated Changelog file * Updated Changelog for unreleased section and pulled main * Updated Changelog and added Sendgrid in unreleased section * Updated Changelog and added Sendgrid in unreleased section with link * Updated Changelog and added Sendgrid in added section after Adam's suggestion Co-authored-by: Adam Sachs <adam@Adams-MacBook-Pro.local> Co-authored-by: Adam Sachs <adam@Adams-MBP.attlocal.net> Co-authored-by: Adrian Galvan <adriang430@gmail.com> Co-authored-by: Hamza W <hamza@Hamzas-MacBook-Pro.local> Co-authored-by: Adrian Galvan <adrian@ethyca.com>

Adam Sachs and others added 15 commits May 2, 2022 15:39

initial sendgrid saas connector integration. access only, contacts only

1c1fb89

erasure (update) support for sendgrid

03a9b6e

remove unused imports

f600992

Merge branch 'main' into 252-saas-connector-sendgrid

1e2e82d

Merge branch 'main' into 252-saas-connector-sendgrid

1c2eab2

update sendgrid test fixture to expect 404 response status code becau…

ecf689e

…se of ignore_errors enhancement

Fixing import order and cleaning up the retry logic for consistency

284c878

add sendgrid env var support to makefile and unsafe_pr_checks config.…

5b41555

… remove DELETE endpoint per PR comments

Merge branch 'main' into 252-saas-connector-sendgrid

e7d8549

Added delete endpoint for contacts

fcb05fb

Fixing data_path for contacts endpoint

3b063a0

Main branch pull and some related configuration changes added

4573de4

Reverting search query to improve performance and avoid server timeouts

2e2c19e

Updated delete endpoint request, used request instead of SaaSRequest …

61ff754

…in tests

updated imports after check suggestion

230c7e6

ghost requested a review from adamsachs July 15, 2022 16:54

adamsachs reviewed Jul 18, 2022

View reviewed changes

adamsachs suggested changes Jul 18, 2022

View reviewed changes

Updated code after review

c31637f

ghost linked an issue Jul 18, 2022 that may be closed by this pull request

Sendgrid - follow-up integration support - "Lists" endpoint #455

Closed

Removed unused variables, imports

f5a6cd0

galvana added 2 commits July 18, 2022 13:04

Merging in main

6cb18ae

Restoring Makefile

c01e13c

galvana added the run unsafe ci checks Triggers running of unsafe CI checks label Jul 18, 2022

Fixed import cryptographic_util error

f1a2bb1

galvana added run unsafe ci checks Triggers running of unsafe CI checks and removed run unsafe ci checks Triggers running of unsafe CI checks labels Jul 18, 2022

Misc fixes

a2d99cf

galvana added dependabot Pull requests from Dependabot run unsafe ci checks Triggers running of unsafe CI checks and removed run unsafe ci checks Triggers running of unsafe CI checks dependabot Pull requests from Dependabot labels Jul 18, 2022

Hamza W added 2 commits July 19, 2022 18:42

Updated Changelog file

f00990f

Merge branch '252-saas-connector-sendgrid' of github.com:ethyca/fides…

7f47682

…ops into 252-saas-connector-sendgrid

adamsachs reviewed Jul 19, 2022

View reviewed changes

Hamza W added 4 commits July 19, 2022 19:34

Merge branch 'main' of github.com:ethyca/fidesops into 252-saas-conne…

4e1df74

…ctor-sendgrid

Updated Changelog for unreleased section and pulled main

227239f

Updated Changelog and added Sendgrid in unreleased section

2368e2d

Updated Changelog and added Sendgrid in unreleased section with link

ba650ac

adamsachs reviewed Jul 19, 2022

View reviewed changes

Updated Changelog and added Sendgrid in added section after Adam's su…

a392415

…ggestion

adamsachs approved these changes Jul 19, 2022

View reviewed changes

ghost merged commit a73e138 into main Jul 19, 2022

ghost deleted the 252-saas-connector-sendgrid branch July 19, 2022 16:03

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

252 saas connector sendgrid #883

252 saas connector sendgrid #883

ghost commented Jul 15, 2022 •

edited by adamsachs

Loading

adamsachs commented Jul 15, 2022

adamsachs commented Jul 15, 2022

adamsachs Jul 18, 2022

adamsachs Jul 18, 2022

adamsachs left a comment

adamsachs commented Jul 18, 2022

adamsachs commented Jul 18, 2022

adamsachs Jul 19, 2022 •

edited

Loading

adamsachs Jul 19, 2022

adamsachs commented Jul 19, 2022

252 saas connector sendgrid #883

252 saas connector sendgrid #883

Conversation

ghost commented Jul 15, 2022 • edited by adamsachs Loading

Purpose

Changes

Checklist

Ticket

adamsachs commented Jul 15, 2022

adamsachs commented Jul 15, 2022

adamsachs Jul 18, 2022

Choose a reason for hiding this comment

adamsachs Jul 18, 2022

Choose a reason for hiding this comment

adamsachs left a comment

Choose a reason for hiding this comment

adamsachs commented Jul 18, 2022

adamsachs commented Jul 18, 2022

adamsachs Jul 19, 2022 • edited Loading

Choose a reason for hiding this comment

adamsachs Jul 19, 2022

Choose a reason for hiding this comment

adamsachs commented Jul 19, 2022

ghost commented Jul 15, 2022 •

edited by adamsachs

Loading

adamsachs Jul 19, 2022 •

edited

Loading