PROD-2654 - MySQL on RDS as a detection/discovery source

[andres-torres-marroquin]

Closes #PROD-2654

Description Of Changes

Added model for MySQL on RDS.

Code Changes

• Added connection_secrets_rds_mysql.py
• Added migration for the new type on connection

Steps to Confirm

• list any manual steps for reviewers to confirm the changes

Pre-Merge Checklist

• All CI Pipelines Succeeded
• Documentation:
  • documentation complete, PR opened in fidesdocs
  • documentation issue created in fidesdocs
  • if there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
• Issue Requirements are Met
• Relevant Follow-Up Issues Created
• Update CHANGELOG.md
• For API changes, the Postman collection has been updated
• If there are any 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!

[vercel]

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		Sep 24, 2024 4:43pm

[cypress]

fides    Run #10097

Run Properties:   Passed #10097  •  0e82f1b0bd ℹ️: Merge d588db29ab04c027266ff08c0be3a30ab377afc0 into 41094b7274adb21532c23abbd898...

Project	fides
Branch Review	refs/pull/5275/merge
Run status	Passed #10097
Run duration	00m 38s
Commit	0e82f1b0bd ℹ️: Merge d588db29ab04c027266ff08c0be3a30ab377afc0 into 41094b7274adb21532c23abbd898...
Committer	Andres Torres
View all properties for this run ↗︎

---

Test results
Failures	0
Flaky	0
Pending	0
Skipped	0
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 ↗︎

[erosselli]

Also maybe we can add a test on tests/ops/api/v1/endpoints/test_connection_template_endpoints.py::TestGetConnectionSecretSchema to test for the added schema

[erosselli]

LGTM!

[adamsachs]

@andres-torres-marroquin i think we at least need a bit of a stronger type on the cert URL property here, and may need to think a bit more about the security implications - see my comment below

[adamsachs]

knowing what i know about how this field is used downstream in the monitor, and the fact that it's user-input data - i think we need some more validation on this field. at a minimum, we should be using the AnyHttpUrl pydantic class (or even better, using @pattisdr 's AnyHttpUrlString in `fideslang) to put some bounds on this field.

additionally, we should probably perform some more checks on the URL before we fetch it and save it to the filesystem downstream. i'm not exactly sure the best way to check that, but it feels like it'd be wise...

if you feel like it makes more sense to just hardcode the default value here rather than accept user config for it, that may be OK. i know i'd vouched for a configurable field earlier, since i thought the flexibility would be a good fallback in case a customer isn't using this global AWS cert that we expect, but i hadn't really considered the security implications. sorry to go back and forth on this! i still think the flexibility would be nice, but we need to be able to do it in a reasonably secure way, so let's weigh that accordingly.

if you do decide to remove the flexibility, i think it would be good for us to confirm with our prospective customers of this feature whether the global AWS cert we're defaulting to is one that would work for their use case 👍

[andres-torres-marroquin]

I confirmed with Michael that they use the standard certificate bundle.