From 39b0b7cc63edc352f78fc5b94b9bef18a9f60d1a Mon Sep 17 00:00:00 2001 From: Adam Sachs Date: Mon, 3 Oct 2022 18:37:51 -0400 Subject: [PATCH 1/4] Remove SaaS type enum and dynamically reference registered types Update tests to dynamically compare results rather than looking for static values. --- .../v1/endpoints/connection_type_endpoints.py | 10 +- .../connection_config.py | 4 +- src/fides/api/ops/schemas/saas/saas_config.py | 24 +- .../test_connection_template_endpoints.py | 234 +++++++++++++----- 4 files changed, 178 insertions(+), 94 deletions(-) diff --git a/src/fides/api/ops/api/v1/endpoints/connection_type_endpoints.py b/src/fides/api/ops/api/v1/endpoints/connection_type_endpoints.py index a6c3555f3b..e080fa6e8c 100644 --- a/src/fides/api/ops/api/v1/endpoints/connection_type_endpoints.py +++ b/src/fides/api/ops/api/v1/endpoints/connection_type_endpoints.py @@ -22,7 +22,7 @@ ConnectionSystemTypeMap, SystemType, ) -from fides.api.ops.schemas.saas.saas_config import SaaSConfig, SaaSType +from fides.api.ops.schemas.saas.saas_config import SaaSConfig from fides.api.ops.service.connectors.saas.connector_registry_service import ( ConnectorRegistry, load_registry, @@ -71,14 +71,14 @@ def is_match(elem: str) -> bool: ] ) if system_type == SystemType.saas or system_type is None: + registry: ConnectorRegistry = load_registry(registry_file) saas_types: List[str] = sorted( [ - saas_type.value - for saas_type in SaaSType - if saas_type != SaaSType.custom and is_match(saas_type.value) + saas_type + for saas_type in registry.connector_types() + if saas_type != "custom" and is_match(saas_type) ] ) - registry: ConnectorRegistry = load_registry(registry_file) for item in saas_types: human_readable_name: str = item diff --git a/src/fides/api/ops/schemas/connection_configuration/connection_config.py b/src/fides/api/ops/schemas/connection_configuration/connection_config.py index a2f2670a29..c686b90976 100644 --- a/src/fides/api/ops/schemas/connection_configuration/connection_config.py +++ b/src/fides/api/ops/schemas/connection_configuration/connection_config.py @@ -8,7 +8,7 @@ from fides.api.ops.schemas.api import BulkResponse, BulkUpdateFailed from fides.api.ops.schemas.connection_configuration import connection_secrets_schemas from fides.api.ops.schemas.dataset import FidesopsDataset -from fides.api.ops.schemas.saas.saas_config import SaaSConfigBase, SaaSType +from fides.api.ops.schemas.saas.saas_config import SaaSConfigBase from fides.api.ops.schemas.shared_schemas import FidesOpsKey @@ -60,7 +60,7 @@ class ConnectionSystemTypeMap(BaseModel): Describes the returned schema for connection types """ - identifier: Union[ConnectionType, SaaSType] + identifier: Union[ConnectionType, str] type: SystemType human_readable: str diff --git a/src/fides/api/ops/schemas/saas/saas_config.py b/src/fides/api/ops/schemas/saas/saas_config.py index ab769cf906..fdef620eab 100644 --- a/src/fides/api/ops/schemas/saas/saas_config.py +++ b/src/fides/api/ops/schemas/saas/saas_config.py @@ -243,28 +243,6 @@ def validate_connector_param(cls, values: Dict[str, Any]) -> Dict[str, Any]: return values -class SaaSType(Enum): - """ - Enum to store saas connection type in Fidesops - """ - - adobe_campaign = "adobe_campaign" - auth0 = "auth0" - shopify = "shopify" - mailchimp = "mailchimp" - hubspot = "hubspot" - outreach = "outreach" - salesforce = "salesforce" - segment = "segment" - sentry = "sentry" - stripe = "stripe" - zendesk = "zendesk" - custom = "custom" - sendgrid = "sendgrid" - datadog = "datadog" - rollbar = "rollbar" - - class SaaSConfigBase(BaseModel): """ Used to store base info for a saas config @@ -272,7 +250,7 @@ class SaaSConfigBase(BaseModel): fides_key: FidesOpsKey name: str - type: SaaSType + type: str @property def fides_key_prop(self) -> FidesOpsKey: diff --git a/tests/ops/api/v1/endpoints/test_connection_template_endpoints.py b/tests/ops/api/v1/endpoints/test_connection_template_endpoints.py index e948d78b5d..c2dcf8673a 100644 --- a/tests/ops/api/v1/endpoints/test_connection_template_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_connection_template_endpoints.py @@ -22,7 +22,11 @@ ) from fides.api.ops.models.datasetconfig import DatasetConfig from fides.api.ops.schemas.connection_configuration.connection_config import SystemType -from fides.api.ops.schemas.saas.saas_config import SaaSType +from fides.api.ops.service.connectors.saas.connector_registry_service import ( + ConnectorRegistry, + load_registry, + registry_file, +) class TestGetConnections: @@ -30,6 +34,10 @@ class TestGetConnections: def url(self, oauth_client: ClientDetail, policy) -> str: return V1_URL_PREFIX + CONNECTION_TYPES + @pytest.fixture(scope="session") + def saas_template_registry(self): + return load_registry(registry_file) + def test_get_connection_types_not_authenticated(self, api_client, url): resp = api_client.get(url, headers={}) assert resp.status_code == 401 @@ -42,23 +50,33 @@ def test_get_connection_types_forbidden( assert resp.status_code == 403 def test_get_connection_types( - self, api_client: TestClient, generate_auth_header, url + self, + api_client: TestClient, + generate_auth_header, + url, + saas_template_registry: ConnectorRegistry, ) -> None: auth_header = generate_auth_header(scopes=[CONNECTION_TYPE_READ]) resp = api_client.get(url, headers=auth_header) data = resp.json()["items"] assert resp.status_code == 200 - assert len(data) == 24 + assert ( + len(data) + == len(ConnectionType) + len(saas_template_registry.connector_types()) - 4 + ) # there are 4 connection types that are not returned by the endpoint assert { "identifier": ConnectionType.postgres.value, "type": SystemType.database.value, "human_readable": "PostgreSQL", } in data + first_saas_type = saas_template_registry.connector_types().pop() assert { - "identifier": SaaSType.stripe.value, + "identifier": first_saas_type, "type": SystemType.saas.value, - "human_readable": "Stripe", + "human_readable": saas_template_registry.get_connector_template( + first_saas_type + ).human_readable, } in data assert "saas" not in [item["identifier"] for item in data] @@ -66,84 +84,160 @@ def test_get_connection_types( assert "custom" not in [item["identifier"] for item in data] assert "manual" not in [item["identifier"] for item in data] - def test_search_connection_types(self, api_client, generate_auth_header, url): + def test_search_connection_types( + self, + api_client, + generate_auth_header, + url, + saas_template_registry: ConnectorRegistry, + ): auth_header = generate_auth_header(scopes=[CONNECTION_TYPE_READ]) - resp = api_client.get(url + "?search=str", headers=auth_header) - assert resp.status_code == 200 - data = resp.json()["items"] - assert len(data) == 1 - assert data[0] == { - "identifier": SaaSType.stripe.value, - "type": SystemType.saas.value, - "human_readable": "Stripe", - } + search = "str" + expected_saas_types = [ + connector_type + for connector_type in saas_template_registry.connector_types() + if search.lower() in connector_type.lower() + ] + expected_saas_data = [ + { + "identifier": saas_type, + "type": SystemType.saas.value, + "human_readable": saas_template_registry.get_connector_template( + saas_type + ).human_readable, + } + for saas_type in expected_saas_types + ] - resp = api_client.get(url + "?search=re", headers=auth_header) + resp = api_client.get(url + f"?search={search}", headers=auth_header) assert resp.status_code == 200 data = resp.json()["items"] - assert len(data) == 3 - assert data == [ - { - "identifier": ConnectionType.postgres.value, - "type": SystemType.database.value, - "human_readable": "PostgreSQL", - }, - { - "identifier": ConnectionType.redshift.value, - "type": SystemType.database.value, - "human_readable": "Amazon Redshift", - }, + + assert len(data) == len(expected_saas_types) + assert data[0] in expected_saas_data + + search = "re" + expected_saas_types = [ + connector_type + for connector_type in saas_template_registry.connector_types() + if search.lower() in connector_type.lower() + ] + expected_saas_data = [ { - "identifier": SaaSType.outreach.value, + "identifier": saas_type, "type": SystemType.saas.value, - "human_readable": "Outreach", - }, + "human_readable": saas_template_registry.get_connector_template( + saas_type + ).human_readable, + } + for saas_type in expected_saas_types ] + resp = api_client.get(url + f"?search={search}", headers=auth_header) + assert resp.status_code == 200 + data = resp.json()["items"] + + # 2 constant non-saas connection types match the search string + assert len(data) == len(expected_saas_types) + 2 + + assert { + "identifier": ConnectionType.postgres.value, + "type": SystemType.database.value, + "human_readable": "PostgreSQL", + } in data + assert { + "identifier": ConnectionType.redshift.value, + "type": SystemType.database.value, + "human_readable": "Amazon Redshift", + } in data + for expected_data in expected_saas_data: + assert expected_data in data + def test_search_connection_types_case_insensitive( - self, api_client, generate_auth_header, url + self, + api_client, + generate_auth_header, + url, + saas_template_registry: ConnectorRegistry, ): auth_header = generate_auth_header(scopes=[CONNECTION_TYPE_READ]) - resp = api_client.get(url + "?search=St", headers=auth_header) + search = "St" + expected_saas_types = [ + connector_type + for connector_type in saas_template_registry.connector_types() + if search.lower() in connector_type.lower() + ] + expected_saas_data = [ + { + "identifier": saas_type, + "type": SystemType.saas.value, + "human_readable": saas_template_registry.get_connector_template( + saas_type + ).human_readable, + } + for saas_type in expected_saas_types + ] + + resp = api_client.get(url + f"?search={search}", headers=auth_header) + assert resp.status_code == 200 data = resp.json()["items"] - assert len(data) == 2 - assert data[0] == { + # 1 constant non-saas connection types match the search string + assert len(data) == len(expected_saas_types) + 1 + assert { "identifier": ConnectionType.postgres.value, "type": SystemType.database.value, "human_readable": "PostgreSQL", - } - assert data[1] == { - "identifier": SaaSType.stripe.value, - "type": SystemType.saas.value, - "human_readable": "Stripe", - } + } in data - resp = api_client.get(url + "?search=Re", headers=auth_header) - assert resp.status_code == 200 - data = resp.json()["items"] - assert len(data) == 3 - assert data == [ - { - "identifier": ConnectionType.postgres.value, - "type": SystemType.database.value, - "human_readable": "PostgreSQL", - }, - { - "identifier": ConnectionType.redshift.value, - "type": SystemType.database.value, - "human_readable": "Amazon Redshift", - }, + for expected_data in expected_saas_data: + assert expected_data in data + + search = "Re" + expected_saas_types = [ + connector_type + for connector_type in saas_template_registry.connector_types() + if search.lower() in connector_type.lower() + ] + expected_saas_data = [ { - "identifier": SaaSType.outreach.value, + "identifier": saas_type, "type": SystemType.saas.value, - "human_readable": "Outreach", - }, + "human_readable": saas_template_registry.get_connector_template( + saas_type + ).human_readable, + } + for saas_type in expected_saas_types ] - def test_search_system_type(self, api_client, generate_auth_header, url): + resp = api_client.get(url + f"?search={search}", headers=auth_header) + assert resp.status_code == 200 + data = resp.json()["items"] + # 2 constant non-saas connection types match the search string + assert len(data) == len(expected_saas_types) + 2 + assert { + "identifier": ConnectionType.postgres.value, + "type": SystemType.database.value, + "human_readable": "PostgreSQL", + } in data + assert { + "identifier": ConnectionType.redshift.value, + "type": SystemType.database.value, + "human_readable": "Amazon Redshift", + } in data + + for expected_data in expected_saas_data: + assert expected_data in data + + def test_search_system_type( + self, + api_client, + generate_auth_header, + url, + saas_template_registry: ConnectorRegistry, + ): auth_header = generate_auth_header(scopes=[CONNECTION_TYPE_READ]) resp = api_client.get(url + "?system_type=nothing", headers=auth_header) @@ -152,7 +246,7 @@ def test_search_system_type(self, api_client, generate_auth_header, url): resp = api_client.get(url + "?system_type=saas", headers=auth_header) assert resp.status_code == 200 data = resp.json()["items"] - assert len(data) == 14 + assert len(data) == len(saas_template_registry.connector_types()) resp = api_client.get(url + "?system_type=database", headers=auth_header) assert resp.status_code == 200 @@ -160,14 +254,26 @@ def test_search_system_type(self, api_client, generate_auth_header, url): assert len(data) == 9 def test_search_system_type_and_connection_type( - self, api_client, generate_auth_header, url + self, + api_client, + generate_auth_header, + url, + saas_template_registry: ConnectorRegistry, ): auth_header = generate_auth_header(scopes=[CONNECTION_TYPE_READ]) - resp = api_client.get(url + "?search=str&system_type=saas", headers=auth_header) + search = "str" + resp = api_client.get( + url + f"?search={search}&system_type=saas", headers=auth_header + ) assert resp.status_code == 200 data = resp.json()["items"] - assert len(data) == 1 + expected_saas_types = [ + connector_type + for connector_type in saas_template_registry.connector_types() + if search.lower() in connector_type.lower() + ] + assert len(data) == len(expected_saas_types) resp = api_client.get( url + "?search=re&system_type=database", headers=auth_header From 2d0e618927cec8dbf83ee93ec90b659d62c3b02e Mon Sep 17 00:00:00 2001 From: Adam Sachs Date: Tue, 4 Oct 2022 09:39:27 -0400 Subject: [PATCH 2/4] Remove references to specific custom SaaSType Since we no longer have an enum for SaaSType, we don't need to have a specific 'custom' type - instead, users can simply create their own type dynamically. There is also now no more invalid 'type' value for saas configs, so we remove the test for that functionality. --- .../api/ops/api/v1/endpoints/connection_type_endpoints.py | 3 +-- tests/ops/models/test_connectionconfig.py | 7 ------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/fides/api/ops/api/v1/endpoints/connection_type_endpoints.py b/src/fides/api/ops/api/v1/endpoints/connection_type_endpoints.py index e080fa6e8c..345c7da3ad 100644 --- a/src/fides/api/ops/api/v1/endpoints/connection_type_endpoints.py +++ b/src/fides/api/ops/api/v1/endpoints/connection_type_endpoints.py @@ -76,7 +76,7 @@ def is_match(elem: str) -> bool: [ saas_type for saas_type in registry.connector_types() - if saas_type != "custom" and is_match(saas_type) + if is_match(saas_type) ] ) @@ -114,7 +114,6 @@ def is_match(elem: str) -> bool: for item in manual_types ] ) - return connection_system_types diff --git a/tests/ops/models/test_connectionconfig.py b/tests/ops/models/test_connectionconfig.py index 1929f6276a..c09bd89568 100644 --- a/tests/ops/models/test_connectionconfig.py +++ b/tests/ops/models/test_connectionconfig.py @@ -140,13 +140,6 @@ def test_default_value_saas_config( connection_config.update_saas_config(db, saas_config=saas_config) assert connection_config.secrets["domain"] == saas_example_secrets["domain"] - def test_default_value_saas_config_invalid_type( - self, db, saas_example_config, saas_example_secrets - ): - saas_example_config["type"] = "invalid" - with pytest.raises(ValueError): - SaaSConfig(**saas_example_config) - def test_connection_type_human_readable(self): for connection in ConnectionType: connection.human_readable # Makes sure all ConnectionTypes have been added to human_readable mapping From 2e29b00247271ce34737921780b4c5c42700526e Mon Sep 17 00:00:00 2001 From: Adam Sachs Date: Tue, 4 Oct 2022 09:42:34 -0400 Subject: [PATCH 3/4] Remove unused import --- src/fides/api/ops/schemas/saas/saas_config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/fides/api/ops/schemas/saas/saas_config.py b/src/fides/api/ops/schemas/saas/saas_config.py index fdef620eab..e62dafbb52 100644 --- a/src/fides/api/ops/schemas/saas/saas_config.py +++ b/src/fides/api/ops/schemas/saas/saas_config.py @@ -1,4 +1,3 @@ -from enum import Enum from typing import Any, Dict, List, Literal, Optional, Set, Union from pydantic import BaseModel, Extra, root_validator, validator From c636a9a1520b9a79d5033823aeb6801d5375caf5 Mon Sep 17 00:00:00 2001 From: Adam Sachs Date: Tue, 4 Oct 2022 10:42:33 -0400 Subject: [PATCH 4/4] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe1b4dd5bc..aa17cb0898 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ The types of changes are: * Changed behavior of `load_default_taxonomy` to append instead of upsert [#1040](https://github.com/ethyca/fides/pull/1040) * Changed behavior of adding privacy declarations to decouple the actions of the "add" and "next" buttons [#1086](https://github.com/ethyca/fides/pull/1086) * Moved system related UI components from the `config-wizard` directory to the `system` directory [#1097](https://github.com/ethyca/fides/pull/1097) +* Updated "type" on SaaS config to be a simple string type, not an enum [#1197](https://github.com/ethyca/fides/pull/1197) ### Developer Experience