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

Deprecate connection config fields in UI #3684

Merged
merged 14 commits into from
Jun 29, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ The types of changes are:

### Changed
- Moved GPC preferences slightly earlier in Fides.js lifecycle [#3561](https://github.com/ethyca/fides/pull/3561)
- Remove name and description fields from integration form [#3684](https://github.com/ethyca/fides/pull/3684)
- Update EU PrivacyNoticeRegion codes and allow experience filtering to drop back to country filtering if region not found [#3630](https://github.com/ethyca/fides/pull/3630)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ const initialState: DatastoreConnectionParams = {
};

export type CreateSaasConnectionConfig = {
connectionConfig: CreateSaasConnectionConfigRequest;
connectionConfig: Omit<CreateSaasConnectionConfigRequest, "name">;
systemFidesKey: string;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,8 @@ const createSaasConnector = async (
systemFidesKey: string,
createSaasConnectorFunc: any
) => {
const connectionConfig: CreateSaasConnectionConfigRequest = {
const connectionConfig: Omit<CreateSaasConnectionConfigRequest, "name"> = {
description: values.description,
name: values.name,
instance_key: formatKey(values.instance_key as string),
saas_connector_type: connectionOption.identifier,
secrets: {},
Expand Down Expand Up @@ -91,16 +90,16 @@ export const patchConnectionConfig = async (
? formatKey(values.instance_key as string)
: connectionConfig?.key;

const params1: Omit<ConnectionConfigurationResponse, "created_at"> = {
access: AccessLevel.WRITE,
connection_type: (connectionOption.type === SystemType.SAAS
? connectionOption.type
: connectionOption.identifier) as ConnectionType,
description: values.description,
disabled: false,
key,
name: values.name,
};
const params1: Omit<ConnectionConfigurationResponse, "created_at" | "name"> =
{
access: AccessLevel.WRITE,
connection_type: (connectionOption.type === SystemType.SAAS
? connectionOption.type
: connectionOption.identifier) as ConnectionType,
description: values.description,
disabled: false,
key,
};
const payload = await patchFunc({
systemFidesKey,
connectionConfigs: [params1],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
NumberInput,
NumberInputField,
NumberInputStepper,
Textarea,
Tooltip,
VStack,
} from "@fidesui/react";
Expand Down Expand Up @@ -267,59 +266,6 @@ const ConnectorParametersForm: React.FC<ConnectorParametersFormProps> = ({
{(props: FormikProps<Values>) => (
<Form noValidate>
<VStack align="stretch" gap="16px">
<Field
id="name"
name="name"
validate={(value: string) => validateField("Name", value)}
>
{({ field }: { field: FieldInputProps<string> }) => (
<FormControl
display="flex"
isRequired
isInvalid={props.errors.name && props.touched.name}
>
{getFormLabel("name", "Name")}
<VStack align="flex-start" w="inherit">
<Input
{...field}
autoComplete="off"
autoFocus
color="gray.700"
placeholder={`Enter a friendly name for your new ${
connectionOption!.human_readable
} integration`}
size="sm"
data-testid="input-name"
/>
<FormErrorMessage>{props.errors.name}</FormErrorMessage>
</VStack>
<Flex alignItems="center" h="32px" visibility="hidden">
<CircleHelpIcon marginLeft="8px" />
</Flex>
</FormControl>
)}
</Field>
{/* Description */}
<Field id="description" name="description">
{({ field }: { field: FieldInputProps<string> }) => (
<FormControl display="flex">
{getFormLabel("description", "Description")}
<Textarea
{...field}
color="gray.700"
placeholder={`Enter a description for your new ${
connectionOption!.human_readable
} integration`}
resize="none"
size="sm"
value={field.value || ""}
/>
<Flex alignItems="center" h="32px" visibility="hidden">
<CircleHelpIcon marginLeft="8px" />
</Flex>
</FormControl>
)}
</Field>
{/* Connection Identifier */}
{connectionOption.type !== SystemType.MANUAL ? (
<Field
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
"""make ConnectionConfig.name optional

Revision ID: 7315b9d7fda6
Revises: d2996381c4dd
Create Date: 2023-06-26 20:39:29.953904

"""
import sqlalchemy as sa
from alembic import op

# revision identifiers, used by Alembic.
revision = "7315b9d7fda6"
down_revision = "d2996381c4dd"
branch_labels = None
depends_on = None


def upgrade():
op.alter_column("connectionconfig", "name", nullable=True)

op.drop_index(op.f("ix_connectionconfig_name"), table_name="connectionconfig")


def downgrade():
op.alter_column("connectionconfig", "name", nullable=False)

conn = op.get_bind()
conn.execute(
"CREATE UNIQUE INDEX IF NOT EXISTS ix_connectionconfig_name ON connectionconfig (name);"
galvana marked this conversation as resolved.
Show resolved Hide resolved
)
16 changes: 5 additions & 11 deletions src/fides/api/models/connectionconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
)

from fides.api.common_exceptions import KeyOrNameAlreadyExists
from fides.api.db.base_class import Base, FidesBase, JSONTypeOverride, get_key_from_data
from fides.api.db.base_class import Base, FidesBase, JSONTypeOverride
from fides.api.models.sql_models import System # type: ignore[attr-defined]
from fides.api.schemas.policy import ActionType
from fides.api.schemas.saas.saas_config import SaaSConfig
Expand Down Expand Up @@ -105,7 +105,7 @@ class ConnectionConfig(Base):
Stores credentials to connect fidesops to an engineer's application databases.
"""

name = Column(String, index=True, unique=True, nullable=False)
name = Column(String, nullable=True)
key = Column(String, index=True, unique=True, nullable=False)
description = Column(String, index=True, nullable=True)
connection_type = Column(Enum(ConnectionType), nullable=False)
Expand Down Expand Up @@ -155,10 +155,12 @@ class ConnectionConfig(Base):
)

@property
def system_key(self) -> str:
def system_key(self) -> Optional[str]:
"""Property for caching a system identifier for systems (or connector names as a fallback) for consent reporting"""
if self.system:
return self.system.fides_key
# TODO: Remove this fallback once all connection configs are linked to systems
# This will always be None in the future. `self.system` will always be set.
return self.name
galvana marked this conversation as resolved.
Show resolved Hide resolved

@classmethod
Expand All @@ -169,21 +171,13 @@ def create_without_saving(
# Build properly formatted key/name for ConnectionConfig.
# Borrowed from OrmWrappedFidesBase.create
if hasattr(cls, "key"):
data["key"] = get_key_from_data(data, cls.__name__)
if db.query(cls).filter_by(key=data["key"]).first():
raise KeyOrNameAlreadyExists(
f"Key {data['key']} already exists in {cls.__name__}. Keys will be snake-cased names if not provided. "
f"If you are seeing this error without providing a key, please provide a key or a different name."
""
)

if hasattr(cls, "name"):
TheAndrewJackson marked this conversation as resolved.
Show resolved Hide resolved
data["name"] = data.get("name")
if db.query(cls).filter_by(name=data["name"]).first():
raise KeyOrNameAlreadyExists(
f"Name {data['name']} already exists in {cls.__name__}."
)

# Create
db_obj = cls(**data) # type: ignore
return db_obj
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class CreateConnectionConfiguration(BaseModel):
Note that secrets are *NOT* allowed to be supplied here.
"""

name: str
name: Optional[str]
key: Optional[FidesKey]
connection_type: ConnectionType
access: AccessLevel
Expand Down Expand Up @@ -93,7 +93,7 @@ class ConnectionConfigurationResponse(BaseModel):
Do *NOT* add "secrets" to this schema.
"""

name: str
name: Optional[str]
key: FidesKey
description: Optional[str]
connection_type: ConnectionType
Expand Down Expand Up @@ -128,7 +128,7 @@ class BulkPatchConnectionConfigurationWithSecrets(BulkResponse):
class SaasConnectionTemplateValues(BaseModel):
"""Schema with values to create both a Saas ConnectionConfig and DatasetConfig from a template"""

name: str # For ConnectionConfig
name: Optional[str] # For ConnectionConfig
key: Optional[FidesKey] # For ConnectionConfig
description: Optional[str] # For ConnectionConfig
secrets: connection_secrets_schemas # For ConnectionConfig
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,13 +323,14 @@ def create_connection_config_from_template_no_save(
)

data = {
"name": template_values.name,
"key": template_values.key,
"key": template_values.key if template_values.key else template_values.instance_key,
TheAndrewJackson marked this conversation as resolved.
Show resolved Hide resolved
"description": template_values.description,
"connection_type": ConnectionType.saas,
"access": AccessLevel.write,
"saas_config": config_from_template,
}
if template_values.name:
data["name"] = template_values.name

if system_id:
data["system_id"] = system_id
Expand Down
14 changes: 4 additions & 10 deletions tests/fixtures/saas/connection_template_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ def secondary_sendgrid_instance(db):
connection_config, dataset_config = instantiate_connector(
db,
"sendgrid",
"sendgrid_connection_config_secondary",
"secondary_sendgrid_instance",
"Sendgrid ConnectionConfig description",
secrets,
Expand All @@ -49,7 +48,6 @@ def secondary_mailchimp_instance(db):
"""
connection_config, dataset_config = instantiate_mailchimp(
db,
"mailchimp_connection_config_secondary",
"secondary_mailchimp_instance",
)
yield connection_config, dataset_config
Expand All @@ -67,15 +65,14 @@ def tertiary_mailchimp_instance(db):
"""
connection_config, dataset_config = instantiate_mailchimp(
db,
"mailchimp_connection_config_tertiary",
"tertiary_mailchimp_instance",
)
yield connection_config, dataset_config
dataset_config.delete(db)
connection_config.delete(db)


def instantiate_mailchimp(db, key, fides_key) -> tuple[ConnectionConfig, DatasetConfig]:
def instantiate_mailchimp(db, fides_key) -> tuple[ConnectionConfig, DatasetConfig]:
secrets = {
"domain": "test_mailchimp_domain",
"username": "test_mailchimp_username",
Expand All @@ -84,7 +81,6 @@ def instantiate_mailchimp(db, key, fides_key) -> tuple[ConnectionConfig, Dataset
return instantiate_connector(
db,
"mailchimp",
key,
fides_key,
"Mailchimp ConnectionConfig description",
secrets,
Expand All @@ -94,7 +90,6 @@ def instantiate_mailchimp(db, key, fides_key) -> tuple[ConnectionConfig, Dataset
def instantiate_connector(
db,
connector_type,
key,
fides_key,
description,
secrets,
Expand All @@ -106,15 +101,14 @@ def instantiate_connector(
ConnectorTemplate
] = ConnectorRegistry.get_connector_template(connector_type)
template_vals = SaasConnectionTemplateValues(
name=key,
key=key,
key=fides_key,
description=description,
secrets=secrets,
instance_key=fides_key,
)

connection_config = ConnectionConfig.filter(
db=db, conditions=(ConnectionConfig.key == key)
db=db, conditions=(ConnectionConfig.key == fides_key)
).first()
assert connection_config is None

Expand All @@ -140,7 +134,7 @@ def instantiate_connector(
)

connection_config = ConnectionConfig.filter(
db=db, conditions=(ConnectionConfig.key == key)
db=db, conditions=(ConnectionConfig.key == fides_key)
).first()
assert connection_config is not None
dataset_config = DatasetConfig.filter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,14 @@ def test_patch_http_connection_successful_analytics(
assert call_args[5] is None


def test_patch_connections_no_name(self, api_client, generate_auth_header, url, payload):
auth_header = generate_auth_header(scopes=[CONNECTION_CREATE_OR_UPDATE])
del payload[0]["name"]
response = api_client.patch(url, headers=auth_header, json=payload)
assert 200 == response.status_code
assert response.json()["succeeded"][0]["name"] is None


class TestGetConnections:
@pytest.fixture(scope="function")
def url(self, oauth_client: ClientDetail, policy) -> str:
Expand Down Expand Up @@ -1279,7 +1287,6 @@ def test_delete_saas_connection_config(
connection_config, dataset_config = instantiate_connector(
db,
"sendgrid",
"sendgrid_connection_config_secondary",
"secondary_sendgrid_instance",
"Sendgrid ConnectionConfig description",
secrets,
Expand Down
13 changes: 6 additions & 7 deletions tests/ops/api/v1/endpoints/test_connection_template_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -1143,18 +1143,17 @@ def test_connection_config_name_already_exists(
headers=auth_header,
json=request_body,
)
assert resp.status_code == 400
assert (
f"Name {connection_config.name} already exists in ConnectionConfig"
in resp.json()["detail"]
)
# names don't have to be unique
assert resp.status_code == 200


def test_create_connection_from_template_without_supplying_connection_key(
self, db, generate_auth_header, api_client, base_url
):
auth_header = generate_auth_header(scopes=[SAAS_CONNECTION_INSTANTIATE])
instance_key = "secondary_mailchimp_instance"
request_body = {
"instance_key": "secondary_mailchimp_instance",
"instance_key": instance_key,
"secrets": {
"domain": "test_mailchimp_domain",
"username": "test_mailchimp_username",
Expand All @@ -1181,7 +1180,7 @@ def test_create_connection_from_template_without_supplying_connection_key(
assert connection_config is not None
assert dataset_config is not None

assert connection_config.key == "mailchimp_connector"
assert connection_config.key == instance_key
dataset_config.delete(db)
connection_config.delete(db)

Expand Down
Loading