Refactor ctl object endpoint generation

[ThomasLaPiana]

Closes #3090

Note to reviewers

This PR is primarily an organizational change, so there is no "new" functionality to test. All tests should pass as they did before.

The only small feature change is the removal of the resource_type param that the ctl endpoints used. They are no longer needed given the new way that these generic endpoints are created.

Code Changes

• convert the current loop into generic, reusable functions
• add a fides.common.utils module to avoid circular dependencies more cleanly
• break out database calls from routes.system into database.system to fix circular dependency
• explicitly create a router for each resources and union them into a single CTL_ROUTER
  • data category
  • data use
  • data subject
  • data qualifier
  • dataset
  • organization
  • policy
  • registry
  • evaluation

Steps to Confirm

• openapi docs still look good
• automated tests pass
• fixes this vuln: https://github.com/ethyca/security-issues/issues/16

Pre-Merge Checklist

• All CI Pipelines Succeeded
• Issue Requirements are Met
• Relevant Follow-Up Issues Created
• Update CHANGELOG.md

Description Of Changes

On top of being difficult to parse/maintain, the for loop that generated the ctl-related object endpoints also had a security vulnerability.

This PR rewrites this loop as a group of discrete functions that are then composed to create each "generic" set of CRUD endpoints. This maintains the DRY aspect of the previous solution while also making it easier when we need to specifically modify a single endpoint.

Along the way, some other improvements became necessary as the slight code reorganization brought some dependency dangers to light. This included a new fides.common.utils file designed to be used across the application and specifically without any reliance on fides itself. This also included refactoring routes.system and moving the database access layer into database instead of in the same routes file, as this also caused a circular dependency

[cypress]

Passing run #2051 ↗︎

0	4	0	0	0
⚠️ You've recorded test results over your free plan limit. Upgrade your plan to view test results.

Details:

Merge d4a3c81 into 5303975...
Project: fides	Commit: 8fcdebbac3 ℹ️
Status: Passed	Duration: 00:45 💡
Started: May 18, 2023 4:55 AM	Ended: May 18, 2023 4:56 AM

_{This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.}

[codecov]

Codecov Report

Patch coverage: 87.37% and project coverage change: -0.02 ⚠️

Comparison is base (5303975) 87.06% compared to head (d4a3c81) 87.05%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3304      +/-   ##
==========================================
- Coverage   87.06%   87.05%   -0.02%     
==========================================
  Files         309      313       +4     
  Lines       18847    18917      +70     
  Branches     2467     2468       +1     
==========================================
+ Hits        16410    16468      +58     
- Misses       1996     2008      +12     
  Partials      441      441              

Impacted Files	Coverage Δ
src/fides/api/ctl/routes/util.py	63.33% <ø> (-3.34%)	⬇️
src/fides/api/ctl/view.py	0.00% <0.00%> (-29.27%)	⬇️
src/fides/core/utils.py	88.23% <14.28%> (+1.66%)	⬆️
src/fides/api/ctl/database/system.py	80.45% <80.45%> (ø)
src/fides/common/utils.py	84.21% <84.21%> (ø)
src/fides/api/ctl/routes/router_factory.py	91.08% <91.08%> (ø)
src/fides/api/app_setup.py	77.17% <100.00%> (-0.49%)	⬇️
src/fides/api/ctl/database/database.py	85.91% <100.00%> (ø)
src/fides/api/ctl/database/seed.py	91.91% <100.00%> (ø)
src/fides/api/ctl/routes/__init__.py	100.00% <100.00%> (ø)
... and 21 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[ThomasLaPiana]

looks like the last major failure here is related to the trailing slashes issue, going to solve like we did before

[ThomasLaPiana]

@daveqnet as this is tied to a security ticket, I'm looking for your verification here as well that it is patched up 🙂

[daveqnet]

@daveqnet as this is tied to a security ticket, I'm looking for your verification here as well that it is patched up 🙂

PR approved from a security perspective, more info here: https://github.com/ethyca/security-issues/issues/16#issuecomment-1550096284

[adamsachs]

@ThomasLaPiana i'm sorry, wasn't able to get to this today - will look at it first thing my tomorrow!

[adamsachs]

thank you @ThomasLaPiana! this is a really nice refactor, IMO it makes this area of the codebase a lot more readable, maintainable and extendable, while still keeping things DRY.

no significant points to address as far as i can tell, i just had some minor nits. i did some smoke testing that the endpoints still work as expected, and paid specific attention to the system API and its use within the admin UI - everything looks good.

of course i'd also like to see if @pattisdr can poke any holes in it!

[pattisdr]

Nice improvement here @ThomasLaPiana, making it easier to start adding one-off changes for specific ctl-resources, while still code-sharing where we can.

[ThomasLaPiana]

@adamsachs @pattisdr thank you both for the thorough review! I'm digging into the fixes and updates now