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

Unify ops/ctl web applications #1016

Merged
merged 70 commits into from
Aug 31, 2022
Merged

Conversation

ThomasLaPiana
Copy link
Contributor

@ThomasLaPiana ThomasLaPiana commented Aug 24, 2022

Closes #1011

Code Changes

  • do another merge of fidesops/main
  • move all of the required ops_main.py logic into main.py
  • fix policy & policy webhook endpoint clashes
  • fix user duplicated endpoints
  • dedupe health endpoint
  • get all of the tests passing
  • copy over all of the required secrets for GH actions
  • deduplicate anything that makes sense to deduplicate related to server infra/configuration

Steps to Confirm

  • tests pass 🚀

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation Updated:
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

Description Of Changes

These changes are the lynchpin of the Unified Fides ™️ project, as this PR fully merges both applications into a single webserver. Further work will be done down the line to continue deduplicating certain code and cleaning things up, however with this change ctl & ops will share a single webserver + database

This PR makes some choices around which pieces to take from which server implementation for merging into a single main.py file, so any reviews should mostly focus on the main.py changes

dependabot bot and others added 28 commits August 16, 2022 10:50
…1014)

Bumps [next-auth](https://github.com/nextauthjs/next-auth) from 4.9.0 to 4.10.3.
- [Release notes](https://github.com/nextauthjs/next-auth/releases)
- [Changelog](https://github.com/nextauthjs/next-auth/blob/main/CHANGELOG.md)
- [Commits](https://github.com/nextauthjs/next-auth/compare/[email protected]@v4.10.3)

---
updated-dependencies:
- dependency-name: next-auth
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Sean Preston <[email protected]>
* Bump fideslib to fix docs auth issue

* Update CHANGELOG

Co-authored-by: Paul Sanders <[email protected]>
* Email config mvp crud / db layer

* adds name to email config model

* remove unintended changes

* gets POC working

* removes org name config var, updates crud endpoints to better handle supporting only 1 config, formatting

* updates postman collection, adds to changelog, bumps downrev on migration

* sort

* formatting

* use correct response model

* Delete base.py

* update migration annotation
* Starting point for SaaS connector templates

* Fix imports from restructuring.

* Get happy path working for instantiate connector from template endpoint.

* Remove updating connector instances for now - out of scope.

* Test nonexistent templates, secrets validation, instance key / fides key already exists.

* Create DatasetConfigs and ConnectionConfigs instead of create_or_update in the template endpoint.  Don't save ConnectionConfig until secrets are validated.

* Add the other saas connectors to the registry and update their configs and datasets with instance_fides_key.

- Fix datadog yaml so it can be included in the saas connector registry. There was an error in how the saas config was formatted.

* Update the fides_keys in the existing saas configs and dataset yamls to have brackets around the "instance_fides_key" to indicate these will be replaced.

Update the fides_key definition to allow "<instance_fides_key>" with brackets specifically to pass validation.

* Fix a side effect on a separate endpoint that returns the types of secrets that should be supplied for a given connector.  Use the saas config type instead of the fides key for the model title. Add test verifying that fides key /instance key validation works as expected.

* - Update CHANGELOG
- Add new endpoint to postman collection
- Add drafts doc.
- Update old response body in docs for connection types.

* Replace the <instance_fides_key> with a properly formatted fides_key in the saas fixtures.

* If DatasetConfig creation fails, delete the recently created ConnectionConfig.

* Address some of the saas integration tests where I've changed the fides_key.

* Fix typos.

* Fix typo.

* Fix unrelated bug where hubspot dataset has new datacategories with user-* data categories after the fideslang update, so they would show up if the user picked a "user" data category.

* Respond to CR.

Co-authored-by: Dawn Pattison <[email protected]>
… of both the connection config and the dataset. (#1105)
* Don't create a new engine as part of running the health checks and share a single engine across the application, including for the health checks.  Currently we're using the default pool_size and max_overflow.

* Update changelog.

* Fix that health checks are still supposed to run, even if the database is disabled.

* Need to yield instead -  'generator' object has no attribute 'query'
* Escape redis user and password

* Update CHANGELOG

Co-authored-by: Paul Sanders <[email protected]>
* pass in analytics id env to worker

* changelog
* Add an endpoint to verify a user's identity before queuing the privacy request provided it doesn't need separate manual approval by a system admin.

- Add a new PrivacyRequest.identity_verified_at timestamp
- Add a new PrivacyRequestStatus - "identity_unverified".
- Add methods to cache the verification code in Redis for comparison with a default ttl of 10 minutes

* - Fix linting/copy-paste errors.
- Update changelog.
- Add endpoint to postman

* Add new keys to response bodies.

* Instead of using a new VerificationCode schema, use the SubjectIdentityVerificationBodyParams that already exists.

* Revert "Instead of using a new VerificationCode schema, use the SubjectIdentityVerificationBodyParams that already exists."

This reverts commit 40fcf6d119135d08a6d3ecfc40c5d73846bf2205.
Bumps [hashicorp/vault-action](https://github.com/hashicorp/vault-action) from 2.4.1 to 2.4.2.
- [Release notes](https://github.com/hashicorp/vault-action/releases)
- [Changelog](https://github.com/hashicorp/vault-action/blob/main/CHANGELOG.md)
- [Commits](hashicorp/vault-action@v2.4.1...v2.4.2)

---
updated-dependencies:
- dependency-name: hashicorp/vault-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [fastapi[all]](https://github.com/tiangolo/fastapi) from 0.79.0 to 0.79.1.
- [Release notes](https://github.com/tiangolo/fastapi/releases)
- [Commits](fastapi/fastapi@0.79.0...0.79.1)

---
updated-dependencies:
- dependency-name: fastapi[all]
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [types-urllib3](https://github.com/python/typeshed) from 1.26.22 to 1.26.23.
- [Release notes](https://github.com/python/typeshed/releases)
- [Commits](https://github.com/python/typeshed/commits)

---
updated-dependencies:
- dependency-name: types-urllib3
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [nox](https://github.com/wntrblm/nox) from 2022.1.7 to 2022.8.7.
- [Release notes](https://github.com/wntrblm/nox/releases)
- [Changelog](https://github.com/wntrblm/nox/blob/main/CHANGELOG.md)
- [Commits](wntrblm/nox@2022.1.7...2022.8.7)

---
updated-dependencies:
- dependency-name: nox
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [faker](https://github.com/joke2k/faker) from 14.0.0 to 14.1.0.
- [Release notes](https://github.com/joke2k/faker/releases)
- [Changelog](https://github.com/joke2k/faker/blob/master/CHANGELOG.md)
- [Commits](joke2k/faker@v14.0.0...v14.1.0)

---
updated-dependencies:
- dependency-name: faker
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [sqlalchemy-redshift](https://github.com/sqlalchemy-redshift/sqlalchemy-redshift) from 0.8.10 to 0.8.11.
- [Release notes](https://github.com/sqlalchemy-redshift/sqlalchemy-redshift/releases)
- [Changelog](https://github.com/sqlalchemy-redshift/sqlalchemy-redshift/blob/main/CHANGES.rst)
- [Commits](sqlalchemy-redshift/sqlalchemy-redshift@0.8.10...0.8.11)

---
updated-dependencies:
- dependency-name: sqlalchemy-redshift
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Add initial POC for dynamic routing

* fix a couple lints

* Fix mypy lint

* making pylint happy

* Remove log

* Fix another pylint issue

* Add docstring

* Update index if check

* Handle nested nextjs routes

* Update changelog
* Sass Connecter feature development

761 - Add a Connection - SaaS connector's configuration parameters
984 - Saas Connector configuration - left navigation to toggle between connection params and dataset config
985 - SaaS Connector Configuration - Testing a Connection

* Skipping unit test temporarily

* Update flags.json file

By default, turning off the createNewConnection flag which is still under development.

* Updated CHANGELOG.md file

* Resolved ESLint issues

* Resolved npm build issue

* Resolved npm build issue

* Updated Saas connector configuration

* Updated Saas connector configuration

* Removed unnecessary import statement

* add new privilege for creating SaaS connectors to user management interface

* add connections read as a privilege

* Applied code review feedback

* Updated the CSS visibility of the CircleHelpIcon component

* Added toast success when a user creates a Saas config

* Resolved ESLint issue

Co-authored-by: Sean Preston <[email protected]>
* 1128-Add Retry button back into the subject request detail view

* Updated CHANGELOG.md file

* provide a way to give invited users the resume permission

Co-authored-by: Sean Preston <[email protected]>
* Set local_host to None for non-endpoint analytics calls.  These are logging various tasks coming out of celery.

* Update Changelog.
* Add email_templates module

* run isort

* Add unit tests

* Update ttl calculation

* Add ttl minutes test

* fix lint issues

* fix pylint issue

* fix pylint issue

* fix isort

* Update template constant

* Update changelog

* fix lints

* Add jinja to requirements.txt

* update templates directory

* update unit test

* Update imports

* fix issue template path

* Add templates to manifest

Co-authored-by: Paul Sanders <[email protected]>
* Fix typo in `derived_identity`

* Update CHANGELOG.md

* Missed PR Link

* replaced `<>` with `{}`

In the live version of the docs, the `<>`s were being stripped from our code example titles. I've replaced them with `{}` to align with some of the other pages I've seen.
* If identity verification required, send email to the user with the verification code.

* Adjust the identity_verification_required autouse fixture, and add an autouse override for just the tests where we want to turn on identity verification.

* Add starting docs and updating the changelog.

Start with identity_verification_required set to False for now until all the related pieces are in.

* Update some of the docstrings.

* Add unverified status color in the FE.

* Add new privacy request status to types and constants.

* Restore trailing comma.

* Update identity_verification_required to subject_identity_verification_required for clarity.

* Adjust email_body_params to accommodate new template.

Co-authored-by: Sean Preston <[email protected]>
* Make connection type search case-insensitive.

* Update changelog.
* Add a new Pii class and use it to wrap arguments not already wrapped with NonPii in those logs that are currently using %-style formatting.

* Switch logging formatting to %-style instead of f-string.

* Continue to address lingering f string instances, and wrap some arguments in Pii, such as raw exceptions.

* Remove NotPii class and update tests.

* Adjust errors made in %-style conversion.

* Remove accidental Pii on print statements, update some PII wrappings.

* Adjust string formatting of newly added log.

* Update Changelog.

* Fix missed closing curly brackets.

* Remove missed curly brackets.
@ThomasLaPiana ThomasLaPiana self-assigned this Aug 24, 2022
@ThomasLaPiana
Copy link
Contributor Author

the final test errors here are in the request_runner_service_test.py file, not sure what's breaking here so I'm going through and trying to carefully discern why some tests in that file are passing while others are failing

@ThomasLaPiana
Copy link
Contributor Author

ThomasLaPiana commented Aug 30, 2022

so looking at the test failures, it doesn't make much sense...I've printed out the values for the failing postgres test and they are as follows:

The customer table after deletion:
[(2, 'Jill Customer'), (3, 'Jane Customer'), (1, None)]
Looking for customer_id: 1
In row: (2, 'Jill Customer')
Looking for customer_id: 1
In row: (3, 'Jane Customer')
Looking for customer_id: 1
In row: (1, None)

However the expected match of 1 in (1, None) isn't happening oddly enough

As can be seen, the id absolutely exists in the row, but for some reason it isn't matching. Seems like good news as this seems to suggest there is no larger issue here other than maybe a small code fix in the test itself

@ThomasLaPiana
Copy link
Contributor Author

I fixed the failing tests but I don't quite understand why they broke in the first place @sanders41

The solution was that the comparison wasn't working, but it seemed valid to me 🤔

This is what I used that works:
if customer_id == row.id:

This is what was there previously that didn't work but I'm not sure why:
if customer_id in row:

@ThomasLaPiana
Copy link
Contributor Author

all non external/saas tests are passing!

next step is to copy over all of the secrets that ops needs

@ThomasLaPiana ThomasLaPiana added 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 Aug 30, 2022
@ThomasLaPiana
Copy link
Contributor Author

@sanders41 @seanpreston can you help me get the secrets copied over? I couldn't find them in 1pass under the same name, and those are the last tests left to check

@sanders41
Copy link
Contributor

Very weird on the comprehension check. Every reason I can think of that would make if customer_id in row: fail would also make if customer_id == row.id: fail. Really if customer_id == row.id: is more strict so if it works lets go with it.

@eastandwestwind can you help with the secrets? I think these will be the vault secrets.

@eastandwestwind
Copy link
Contributor

eastandwestwind commented Aug 30, 2022

@ThomasLaPiana we shouldn't need to copy over vault secrets into GH secrets, since ops will only look in Vault for secrets at this point. Btw, you might not have proper access to view the fidesops secrets in 1pass (message me to get more details on which folder to look for), but @daveqnet could help you out with access.

@ThomasLaPiana
Copy link
Contributor Author

@ThomasLaPiana we shouldn't need to copy over vault secrets into GH secrets, since ops will only look in Vault for secrets at this point. Btw, you might not have proper access to view the fidesops secrets in 1pass (message me to get more details on which folder to look for), but @daveqnet could help you out with access.

this would be for:

  1. The vault secrets aren't exactly specified in 1pass, so I'm not sure what to add to the repo
  2. the external integration tests (redshift, bigquery, snowflake) still appear to use non-vault secrets

@ThomasLaPiana ThomasLaPiana added 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 Aug 31, 2022
@ThomasLaPiana ThomasLaPiana added 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 Aug 31, 2022
@ThomasLaPiana ThomasLaPiana added 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 Aug 31, 2022
@ThomasLaPiana ThomasLaPiana added 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 Aug 31, 2022
@ThomasLaPiana ThomasLaPiana marked this pull request as ready for review August 31, 2022 08:26
@ThomasLaPiana ThomasLaPiana requested review from a team August 31, 2022 08:26
@ThomasLaPiana
Copy link
Contributor Author

State of the PR:

  • Ops External Integration tests have 2 failing Snowflake tests
  • Ops External SaaS tests have 2 failing Hubspot tests

I'm going to merge now, as these tests shouldn't hold up this PR or the rest of the work. They can be fixed in a smaller PR

@ThomasLaPiana ThomasLaPiana merged commit 8733454 into unified-fides Aug 31, 2022
@ThomasLaPiana ThomasLaPiana deleted the ThomasLaPiana-unify-webapps branch August 31, 2022 08:28
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.

Merge server apps on the unified-fides branch
8 participants