From 60b9cf3e438b7b45d1ba21bb334c47bda5928c75 Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 12 Jul 2022 12:55:36 -0700 Subject: [PATCH 01/13] Add a placeholder change for this PR --- src/fidesctl/cli/commands/core.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/fidesctl/cli/commands/core.py b/src/fidesctl/cli/commands/core.py index 1ba68988ddb..03ca3253943 100644 --- a/src/fidesctl/cli/commands/core.py +++ b/src/fidesctl/cli/commands/core.py @@ -142,6 +142,9 @@ def parse(ctx: click.Context, manifests_dir: str, verbose: bool = False) -> None def sync(ctx: click.Context, manifests_dir: str) -> None: """ Update local resource files by their fides_key to match their server versions. + + Alternatively, with the "--all" flag all resources from the server will be pulled + down into local files. Aborts the sync if there are unstaged or untracked files in the manifests dir. """ From e1f7d272439f9d171f1e1df25eebbd4dca002363 Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 12 Jul 2022 15:05:45 -0700 Subject: [PATCH 02/13] add the `sync_all` flag to the "sync" command and module --- src/fidesctl/cli/commands/core.py | 11 +++++++-- src/fidesctl/core/sync.py | 37 ++++++++++++++++++++++++++----- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/fidesctl/cli/commands/core.py b/src/fidesctl/cli/commands/core.py index 03ca3253943..24be41d7cef 100644 --- a/src/fidesctl/cli/commands/core.py +++ b/src/fidesctl/cli/commands/core.py @@ -138,11 +138,17 @@ def parse(ctx: click.Context, manifests_dir: str, verbose: bool = False) -> None @click.command() @click.pass_context @manifests_dir_argument +@click.option( + "--sync-all", + "-a", + is_flag=True, + help="Pulls all resources from the server, regardless of if they already exist locally.", +) @with_analytics -def sync(ctx: click.Context, manifests_dir: str) -> None: +def sync(ctx: click.Context, manifests_dir: str, sync_all: bool = False) -> None: """ Update local resource files by their fides_key to match their server versions. - + Alternatively, with the "--all" flag all resources from the server will be pulled down into local files. @@ -161,4 +167,5 @@ def sync(ctx: click.Context, manifests_dir: str) -> None: url=config.cli.server_url, manifests_dir=manifests_dir, headers=config.user.request_headers, + sync_all=sync_all, ) diff --git a/src/fidesctl/core/sync.py b/src/fidesctl/core/sync.py index 98563141158..ae642a3c6a8 100644 --- a/src/fidesctl/core/sync.py +++ b/src/fidesctl/core/sync.py @@ -9,14 +9,14 @@ from fidesctl.core.utils import get_manifest_list -def sync(manifests_dir: str, url: str, headers: Dict[str, str]) -> None: +def sync_existing_resources( + manifests_dir: str, url: str, headers: Dict[str, str] +) -> bool: """ - If a resource in a local file has a matching resource on the server, - write out the server version into the local file. + Update all of the pre-existing local resources to match their + state on the server. """ - manifest_path_list = get_manifest_list(manifests_dir) - print_divider() for manifest_path in manifest_path_list: print(f"Syncing file: '{manifest_path}'...") @@ -48,4 +48,31 @@ def sync(manifests_dir: str, url: str, headers: Dict[str, str]) -> None: echo_green(f"Updated manifest file written out to: '{manifest_path}'") print_divider() + return True + + +def sync_missing_resources( + manifests_dir: str, url: str, headers: Dict[str, str] +) -> bool: + """ + Write resources out locally that currently only exist on the server. + """ + return True + + +def sync( + manifests_dir: str, url: str, headers: Dict[str, str], sync_all: bool = False +) -> None: + """ + If a resource in a local file has a matching resource on the server, + write out the server version into the local file. + + If the 'all' flag is passed, additionally pull all other server resources + into local files as well. + """ + sync_existing_resources(manifests_dir, url, headers) + + if sync_all: + sync_missing_resources(manifests_dir, url, headers) + echo_green("Sync complete.") From 8d75499a06770e0c5894b167eecc33533e62c897 Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 12 Jul 2022 15:35:35 -0700 Subject: [PATCH 03/13] add the crucial piece we've been missing this whole time --- src/fidesctl/cli/commands/util.py | 9 +++++++-- src/fidesctl/cli/utils.py | 9 +++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/fidesctl/cli/commands/util.py b/src/fidesctl/cli/commands/util.py index b12a2805a93..85c80d1bdba 100644 --- a/src/fidesctl/cli/commands/util.py +++ b/src/fidesctl/cli/commands/util.py @@ -7,7 +7,12 @@ from fideslog.sdk.python.utils import OPT_OUT_COPY, OPT_OUT_PROMPT import fidesctl -from fidesctl.cli.utils import check_server, send_init_analytics, with_analytics +from fidesctl.cli.utils import ( + FIDESCTL_ASCII_ART, + check_server, + send_init_analytics, + with_analytics, +) from fidesctl.core.utils import echo_green @@ -45,7 +50,7 @@ def init(ctx: click.Context, fides_directory_location: str) -> None: "cli": {"server_protocol", "server_host", "server_port", "analytics_id"}, "user": {"analytics_opt_out"}, } - + click.echo(FIDESCTL_ASCII_ART) click.echo("Initializing Fidesctl...") separate() diff --git a/src/fidesctl/cli/utils.py b/src/fidesctl/cli/utils.py index bbd3bb66146..c3384cd3010 100644 --- a/src/fidesctl/cli/utils.py +++ b/src/fidesctl/cli/utils.py @@ -33,6 +33,15 @@ from fidesctl.core.config.utils import get_config_from_file, update_config_file from fidesctl.core.utils import API_PREFIX, check_response, echo_green, echo_red +FIDESCTL_ASCII_ART = """ +███████╗██╗██████╗ ███████╗███████╗ ██████╗████████╗██╗ +██╔════╝██║██╔══██╗██╔════╝██╔════╝██╔════╝╚══██╔══╝██║ +█████╗ ██║██║ ██║█████╗ ███████╗██║ ██║ ██║ +██╔══╝ ██║██║ ██║██╔══╝ ╚════██║██║ ██║ ██║ +██║ ██║██████╔╝███████╗███████║╚██████╗ ██║ ███████╗ +╚═╝ ╚═╝╚═════╝ ╚══════╝╚══════╝ ╚═════╝ ╚═╝ ╚══════╝ +""" + def check_server(cli_version: str, server_url: str, quiet: bool = False) -> None: """Runs a health check and a version check against the server.""" From af1d0b7944791e745a9649471c276c853d0f7772 Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 12 Jul 2022 15:39:30 -0700 Subject: [PATCH 04/13] store the keys that get updated and exclude them from the missing resource sync --- src/fidesctl/core/sync.py | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/src/fidesctl/core/sync.py b/src/fidesctl/core/sync.py index ae642a3c6a8..5a5537f9fc6 100644 --- a/src/fidesctl/core/sync.py +++ b/src/fidesctl/core/sync.py @@ -1,22 +1,25 @@ """This module handles the logic for syncing remote resource versions into their local file.""" -from typing import Dict +from typing import Dict, List import yaml from fideslang.manifests import load_yaml_into_dict from fidesctl.cli.utils import echo_green, print_divider -from fidesctl.core.api_helpers import get_server_resource +from fidesctl.core.api_helpers import get_server_resource, list_server_resources from fidesctl.core.utils import get_manifest_list def sync_existing_resources( manifests_dir: str, url: str, headers: Dict[str, str] -) -> bool: +) -> List[str]: """ Update all of the pre-existing local resources to match their state on the server. """ manifest_path_list = get_manifest_list(manifests_dir) + # Store and return the keys of resources that get synced here. + existing_keys: List[str] = [] + print_divider() for manifest_path in manifest_path_list: print(f"Syncing file: '{manifest_path}'...") @@ -29,6 +32,8 @@ def sync_existing_resources( for resource in resource_list: fides_key = resource["fides_key"] + existing_keys.append(fides_key) + server_resource = get_server_resource( url, resource_type, fides_key, headers, raw=True ) @@ -48,20 +53,31 @@ def sync_existing_resources( echo_green(f"Updated manifest file written out to: '{manifest_path}'") print_divider() - return True + return existing_keys def sync_missing_resources( - manifests_dir: str, url: str, headers: Dict[str, str] + manifest_path: str, url: str, headers: Dict[str, str], existing_keys: List[str] ) -> bool: """ - Write resources out locally that currently only exist on the server. + Writes all "system", "dataset" and "policy" resources out locally + that currently only exist on the server. """ + + print(existing_keys) + manifest_file = + resources_to_sync = ["system", "dataset", "policies"] + resource_dict = { + resource: list_server_resources(url, headers, resource, existing_keys) + for resource in resources_to_sync + } + + # Write out the resources in a file return True def sync( - manifests_dir: str, url: str, headers: Dict[str, str], sync_all: bool = False + manifests_dir: str, manifest_path: str, url: str, headers: Dict[str, str], sync_all: bool = False ) -> None: """ If a resource in a local file has a matching resource on the server, @@ -70,9 +86,9 @@ def sync( If the 'all' flag is passed, additionally pull all other server resources into local files as well. """ - sync_existing_resources(manifests_dir, url, headers) + existing_keys = sync_existing_resources(manifests_dir, url, headers) if sync_all: - sync_missing_resources(manifests_dir, url, headers) + sync_missing_resources(manifest_path, url, headers, existing_keys) echo_green("Sync complete.") From 5c39ac537f1bdc4dec80ec8eeed7f6fe6e054aa0 Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 12 Jul 2022 18:45:25 -0700 Subject: [PATCH 05/13] add a "raw" parameter to list_server_resources to get back a raw dict instead of a parsed model --- src/fidesctl/cli/commands/core.py | 12 +- src/fidesctl/core/api_helpers.py | 22 ++- src/fidesctl/core/sync.py | 30 ++-- test_manifest.yml | 290 ++++++++++++++++++++++++++++++ 4 files changed, 331 insertions(+), 23 deletions(-) create mode 100644 test_manifest.yml diff --git a/src/fidesctl/cli/commands/core.py b/src/fidesctl/cli/commands/core.py index 24be41d7cef..2bf72f8d6ee 100644 --- a/src/fidesctl/cli/commands/core.py +++ b/src/fidesctl/cli/commands/core.py @@ -139,13 +139,13 @@ def parse(ctx: click.Context, manifests_dir: str, verbose: bool = False) -> None @click.pass_context @manifests_dir_argument @click.option( - "--sync-all", - "-a", - is_flag=True, - help="Pulls all resources from the server, regardless of if they already exist locally.", + "--sync-new", + "-s", + default="new_manifests.yml", + help="Pulls all new resources from the server into this file.", ) @with_analytics -def sync(ctx: click.Context, manifests_dir: str, sync_all: bool = False) -> None: +def sync(ctx: click.Context, manifests_dir: str, sync_new: str) -> None: """ Update local resource files by their fides_key to match their server versions. @@ -167,5 +167,5 @@ def sync(ctx: click.Context, manifests_dir: str, sync_all: bool = False) -> None url=config.cli.server_url, manifests_dir=manifests_dir, headers=config.user.request_headers, - sync_all=sync_all, + sync_new=sync_new, ) diff --git a/src/fidesctl/core/api_helpers.py b/src/fidesctl/core/api_helpers.py index e4c47be96f6..4c9b258f720 100644 --- a/src/fidesctl/core/api_helpers.py +++ b/src/fidesctl/core/api_helpers.py @@ -82,24 +82,32 @@ def list_server_resources( headers: Dict[str, str], resource_type: str, exclude_keys: List[str], -) -> List[FidesModel]: + raw: bool = False, +) -> Optional[Union[List[FidesModel], List[Dict]]]: """ Get a list of resources from the server and return them as parsed objects. Returns an empty list if no resources are found or if the API returns an error. """ response: Response = api.ls(url=url, resource_type=resource_type, headers=headers) - server_resources: List[FidesModel] = ( + server_resources = ( [ - parse_dict( - resource_type=resource_type, - resource=resource, - from_server=True, - ) + resource for resource in response.json() if isinstance(resource, dict) and resource["fides_key"] not in exclude_keys ] if response.status_code >= 200 and response.status_code <= 299 else [] ) + + if not raw and server_resources: + server_resources = [ + parse_dict( + resource_type=resource_type, + resource=resource_dict, + from_server=True, + ) + for resource_dict in server_resources + ] + return server_resources diff --git a/src/fidesctl/core/sync.py b/src/fidesctl/core/sync.py index 5a5537f9fc6..32146b3a71e 100644 --- a/src/fidesctl/core/sync.py +++ b/src/fidesctl/core/sync.py @@ -2,11 +2,21 @@ from typing import Dict, List import yaml -from fideslang.manifests import load_yaml_into_dict from fidesctl.cli.utils import echo_green, print_divider from fidesctl.core.api_helpers import get_server_resource, list_server_resources from fidesctl.core.utils import get_manifest_list +from fideslang.manifests import load_yaml_into_dict +from fideslang import FidesModel + + +def write_manifest_file(manifest_path: str, manifest: Dict) -> None: + """ + Write a manifest file out. + """ + with open(manifest_path, "w") as manifest_file: + yaml.dump(manifest, manifest_file, sort_keys=False, indent=2) + echo_green(f"Updated manifest file written out to: '{manifest_path}'") def sync_existing_resources( @@ -47,10 +57,7 @@ def sync_existing_resources( updated_resource_list.append(resource) updated_manifest[resource_type] = updated_resource_list - - with open(manifest_path, "w") as manifest_file: - yaml.dump(updated_manifest, manifest_file, sort_keys=False, indent=2) - echo_green(f"Updated manifest file written out to: '{manifest_path}'") + write_manifest_file(manifest_path, updated_manifest) print_divider() return existing_keys @@ -65,19 +72,22 @@ def sync_missing_resources( """ print(existing_keys) - manifest_file = resources_to_sync = ["system", "dataset", "policies"] - resource_dict = { + resource_manifest: Dict[str, List[FidesModel]] = { resource: list_server_resources(url, headers, resource, existing_keys) for resource in resources_to_sync } # Write out the resources in a file + write_manifest_file(manifest_path, resource_manifest) return True def sync( - manifests_dir: str, manifest_path: str, url: str, headers: Dict[str, str], sync_all: bool = False + manifests_dir: str, + url: str, + headers: Dict[str, str], + sync_new: str, ) -> None: """ If a resource in a local file has a matching resource on the server, @@ -88,7 +98,7 @@ def sync( """ existing_keys = sync_existing_resources(manifests_dir, url, headers) - if sync_all: - sync_missing_resources(manifest_path, url, headers, existing_keys) + if sync_new: + sync_missing_resources(sync_new, url, headers, existing_keys) echo_green("Sync complete.") diff --git a/test_manifest.yml b/test_manifest.yml new file mode 100644 index 00000000000..59248673549 --- /dev/null +++ b/test_manifest.yml @@ -0,0 +1,290 @@ +system: +- !!python/object:fideslang.models.System + __dict__: + fides_key: demo_analytics_system + organization_fides_key: default_organization + name: Demo Analytics System + description: A system used for analyzing customer behaviour. + registry_id: null + meta: null + fidesctl_meta: null + system_type: Service + data_responsibility_title: Controller + privacy_declarations: + - !!python/object:fideslang.models.PrivacyDeclaration + __dict__: + name: Analyze customer behaviour for improvements. + data_categories: + - user.provided.identifiable.contact + - user.derived.identifiable.device.cookie_id + data_use: improve.system + data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified + data_subjects: + - customer + dataset_references: + - demo_users_dataset + __fields_set__: !!set + data_qualifier: null + name: null + data_subjects: null + data_categories: null + dataset_references: null + data_use: null + __private_attribute_values__: {} + system_dependencies: null + joint_controller: null + third_country_transfers: + - USA + - CAN + administrating_department: Engineering + data_protection_impact_assessment: !!python/object:fideslang.models.DataProtectionImpactAssessment + __dict__: + is_required: true + progress: Complete + link: !!python/object/new:pydantic.networks.AnyUrl + args: + - https://example.org/analytics_system_data_protection_impact_assessment + state: !!python/tuple + - null + - scheme: https + user: null + password: null + host: example.org + tld: org + host_type: domain + port: null + path: /analytics_system_data_protection_impact_assessment + query: null + fragment: null + __fields_set__: !!set + link: null + progress: null + is_required: null + __private_attribute_values__: {} + __fields_set__: !!set + name: null + fidesctl_meta: null + organization_fides_key: null + fides_key: null + system_dependencies: null + privacy_declarations: null + third_country_transfers: null + data_protection_impact_assessment: null + system_type: null + meta: null + description: null + joint_controller: null + data_responsibility_title: null + administrating_department: null + registry_id: null + __private_attribute_values__: {} +- !!python/object:fideslang.models.System + __dict__: + fides_key: demo_marketing_system + organization_fides_key: default_organization + name: Demo Marketing System + description: Collect data about our users for marketing. + registry_id: null + meta: null + fidesctl_meta: null + system_type: Service + data_responsibility_title: Processor + privacy_declarations: + - !!python/object:fideslang.models.PrivacyDeclaration + __dict__: + name: Collect data for marketing + data_categories: + - user.derived.identifiable.device.cookie_id + data_use: advertising + data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified + data_subjects: + - customer + dataset_references: null + __fields_set__: !!set + data_qualifier: null + name: null + data_subjects: null + data_categories: null + dataset_references: null + data_use: null + __private_attribute_values__: {} + system_dependencies: null + joint_controller: null + third_country_transfers: null + administrating_department: Marketing + data_protection_impact_assessment: !!python/object:fideslang.models.DataProtectionImpactAssessment + __dict__: + is_required: false + progress: null + link: null + __fields_set__: !!set + link: null + progress: null + is_required: null + __private_attribute_values__: {} + __fields_set__: !!set + name: null + fidesctl_meta: null + organization_fides_key: null + fides_key: null + system_dependencies: null + privacy_declarations: null + third_country_transfers: null + data_protection_impact_assessment: null + system_type: null + meta: null + description: null + joint_controller: null + data_responsibility_title: null + administrating_department: null + registry_id: null + __private_attribute_values__: {} +dataset: +- !!python/object:fideslang.models.Dataset + __dict__: + fides_key: demo_users_dataset + organization_fides_key: default_organization + name: Demo Users Dataset + description: Data collected about users for our analytics system. + meta: null + data_categories: [] + data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified + fidesctl_meta: null + joint_controller: null + retention: 30 days after account deletion + third_country_transfers: + - GBR + - CAN + collections: + - !!python/object:fideslang.models.DatasetCollection + __dict__: + name: users + description: User information + data_categories: [] + data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified + retention: null + fields: + - !!python/object:fideslang.models.DatasetField + __dict__: + name: created_at + description: User's creation timestamp + data_categories: + - system.operations + data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified + retention: null + fields: null + __fields_set__: !!set + data_qualifier: null + name: null + retention: null + data_categories: null + fields: null + description: null + __private_attribute_values__: {} + - !!python/object:fideslang.models.DatasetField + __dict__: + name: email + description: User's Email + data_categories: + - user.provided.identifiable.contact.email + data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified + retention: Account termination + fields: null + __fields_set__: !!set + data_qualifier: null + name: null + retention: null + data_categories: null + fields: null + description: null + __private_attribute_values__: {} + - !!python/object:fideslang.models.DatasetField + __dict__: + name: first_name + description: User's first name + data_categories: + - user.provided.identifiable.name + data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified + retention: Account termination + fields: null + __fields_set__: !!set + data_qualifier: null + name: null + retention: null + data_categories: null + fields: null + description: null + __private_attribute_values__: {} + - !!python/object:fideslang.models.DatasetField + __dict__: + name: food_preference + description: User's favorite food + data_categories: [] + data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified + retention: null + fields: null + __fields_set__: !!set + data_qualifier: null + name: null + retention: null + data_categories: null + fields: null + description: null + __private_attribute_values__: {} + - !!python/object:fideslang.models.DatasetField + __dict__: + name: state + description: User's State + data_categories: + - user.provided.identifiable.contact.state + data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified + retention: null + fields: null + __fields_set__: !!set + data_qualifier: null + name: null + retention: null + data_categories: null + fields: null + description: null + __private_attribute_values__: {} + - !!python/object:fideslang.models.DatasetField + __dict__: + name: uuid + description: User's unique ID + data_categories: + - user.derived.identifiable.unique_id + data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified + retention: null + fields: null + __fields_set__: !!set + data_qualifier: null + name: null + retention: null + data_categories: null + fields: null + description: null + __private_attribute_values__: {} + __fields_set__: !!set + data_qualifier: null + name: null + retention: null + data_categories: null + fields: null + description: null + __private_attribute_values__: {} + __fields_set__: !!set + data_qualifier: null + name: null + retention: null + data_categories: null + fidesctl_meta: null + organization_fides_key: null + fides_key: null + collections: null + third_country_transfers: null + meta: null + description: null + joint_controller: null + __private_attribute_values__: {} +policies: [] From d16a2ce11e333859ce8fcea7ff256e759ef8586e Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 12 Jul 2022 19:03:46 -0700 Subject: [PATCH 06/13] sync_missing_resources works as intended --- src/fidesctl/cli/commands/core.py | 6 +- src/fidesctl/core/sync.py | 19 +- test_manifest.yml | 290 ------------------------------ 3 files changed, 17 insertions(+), 298 deletions(-) delete mode 100644 test_manifest.yml diff --git a/src/fidesctl/cli/commands/core.py b/src/fidesctl/cli/commands/core.py index 2bf72f8d6ee..e338ac72bb1 100644 --- a/src/fidesctl/cli/commands/core.py +++ b/src/fidesctl/cli/commands/core.py @@ -1,4 +1,6 @@ """Contains all of the core CLI commands for Fidesctl.""" +from typing import Optional + import click from fidesctl.cli.options import ( @@ -141,11 +143,11 @@ def parse(ctx: click.Context, manifests_dir: str, verbose: bool = False) -> None @click.option( "--sync-new", "-s", - default="new_manifests.yml", + default=None, help="Pulls all new resources from the server into this file.", ) @with_analytics -def sync(ctx: click.Context, manifests_dir: str, sync_new: str) -> None: +def sync(ctx: click.Context, manifests_dir: str, sync_new: Optional[str]) -> None: """ Update local resource files by their fides_key to match their server versions. diff --git a/src/fidesctl/core/sync.py b/src/fidesctl/core/sync.py index 32146b3a71e..9d686e9f8cc 100644 --- a/src/fidesctl/core/sync.py +++ b/src/fidesctl/core/sync.py @@ -1,5 +1,5 @@ """This module handles the logic for syncing remote resource versions into their local file.""" -from typing import Dict, List +from typing import Dict, List, Optional import yaml @@ -71,15 +71,22 @@ def sync_missing_resources( that currently only exist on the server. """ - print(existing_keys) + print(f"Writing out new resources to file: '{manifest_path}'...") resources_to_sync = ["system", "dataset", "policies"] - resource_manifest: Dict[str, List[FidesModel]] = { - resource: list_server_resources(url, headers, resource, existing_keys) + resource_manifest = { + resource: list_server_resources( + url=url, + headers=headers, + resource_type=resource, + exclude_keys=existing_keys, + raw=True, + ) for resource in resources_to_sync } # Write out the resources in a file write_manifest_file(manifest_path, resource_manifest) + print_divider() return True @@ -87,11 +94,11 @@ def sync( manifests_dir: str, url: str, headers: Dict[str, str], - sync_new: str, + sync_new: Optional[str], ) -> None: """ If a resource in a local file has a matching resource on the server, - write out the server version into the local file. + write the server version into the local file. If the 'all' flag is passed, additionally pull all other server resources into local files as well. diff --git a/test_manifest.yml b/test_manifest.yml deleted file mode 100644 index 59248673549..00000000000 --- a/test_manifest.yml +++ /dev/null @@ -1,290 +0,0 @@ -system: -- !!python/object:fideslang.models.System - __dict__: - fides_key: demo_analytics_system - organization_fides_key: default_organization - name: Demo Analytics System - description: A system used for analyzing customer behaviour. - registry_id: null - meta: null - fidesctl_meta: null - system_type: Service - data_responsibility_title: Controller - privacy_declarations: - - !!python/object:fideslang.models.PrivacyDeclaration - __dict__: - name: Analyze customer behaviour for improvements. - data_categories: - - user.provided.identifiable.contact - - user.derived.identifiable.device.cookie_id - data_use: improve.system - data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified - data_subjects: - - customer - dataset_references: - - demo_users_dataset - __fields_set__: !!set - data_qualifier: null - name: null - data_subjects: null - data_categories: null - dataset_references: null - data_use: null - __private_attribute_values__: {} - system_dependencies: null - joint_controller: null - third_country_transfers: - - USA - - CAN - administrating_department: Engineering - data_protection_impact_assessment: !!python/object:fideslang.models.DataProtectionImpactAssessment - __dict__: - is_required: true - progress: Complete - link: !!python/object/new:pydantic.networks.AnyUrl - args: - - https://example.org/analytics_system_data_protection_impact_assessment - state: !!python/tuple - - null - - scheme: https - user: null - password: null - host: example.org - tld: org - host_type: domain - port: null - path: /analytics_system_data_protection_impact_assessment - query: null - fragment: null - __fields_set__: !!set - link: null - progress: null - is_required: null - __private_attribute_values__: {} - __fields_set__: !!set - name: null - fidesctl_meta: null - organization_fides_key: null - fides_key: null - system_dependencies: null - privacy_declarations: null - third_country_transfers: null - data_protection_impact_assessment: null - system_type: null - meta: null - description: null - joint_controller: null - data_responsibility_title: null - administrating_department: null - registry_id: null - __private_attribute_values__: {} -- !!python/object:fideslang.models.System - __dict__: - fides_key: demo_marketing_system - organization_fides_key: default_organization - name: Demo Marketing System - description: Collect data about our users for marketing. - registry_id: null - meta: null - fidesctl_meta: null - system_type: Service - data_responsibility_title: Processor - privacy_declarations: - - !!python/object:fideslang.models.PrivacyDeclaration - __dict__: - name: Collect data for marketing - data_categories: - - user.derived.identifiable.device.cookie_id - data_use: advertising - data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified - data_subjects: - - customer - dataset_references: null - __fields_set__: !!set - data_qualifier: null - name: null - data_subjects: null - data_categories: null - dataset_references: null - data_use: null - __private_attribute_values__: {} - system_dependencies: null - joint_controller: null - third_country_transfers: null - administrating_department: Marketing - data_protection_impact_assessment: !!python/object:fideslang.models.DataProtectionImpactAssessment - __dict__: - is_required: false - progress: null - link: null - __fields_set__: !!set - link: null - progress: null - is_required: null - __private_attribute_values__: {} - __fields_set__: !!set - name: null - fidesctl_meta: null - organization_fides_key: null - fides_key: null - system_dependencies: null - privacy_declarations: null - third_country_transfers: null - data_protection_impact_assessment: null - system_type: null - meta: null - description: null - joint_controller: null - data_responsibility_title: null - administrating_department: null - registry_id: null - __private_attribute_values__: {} -dataset: -- !!python/object:fideslang.models.Dataset - __dict__: - fides_key: demo_users_dataset - organization_fides_key: default_organization - name: Demo Users Dataset - description: Data collected about users for our analytics system. - meta: null - data_categories: [] - data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified - fidesctl_meta: null - joint_controller: null - retention: 30 days after account deletion - third_country_transfers: - - GBR - - CAN - collections: - - !!python/object:fideslang.models.DatasetCollection - __dict__: - name: users - description: User information - data_categories: [] - data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified - retention: null - fields: - - !!python/object:fideslang.models.DatasetField - __dict__: - name: created_at - description: User's creation timestamp - data_categories: - - system.operations - data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified - retention: null - fields: null - __fields_set__: !!set - data_qualifier: null - name: null - retention: null - data_categories: null - fields: null - description: null - __private_attribute_values__: {} - - !!python/object:fideslang.models.DatasetField - __dict__: - name: email - description: User's Email - data_categories: - - user.provided.identifiable.contact.email - data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified - retention: Account termination - fields: null - __fields_set__: !!set - data_qualifier: null - name: null - retention: null - data_categories: null - fields: null - description: null - __private_attribute_values__: {} - - !!python/object:fideslang.models.DatasetField - __dict__: - name: first_name - description: User's first name - data_categories: - - user.provided.identifiable.name - data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified - retention: Account termination - fields: null - __fields_set__: !!set - data_qualifier: null - name: null - retention: null - data_categories: null - fields: null - description: null - __private_attribute_values__: {} - - !!python/object:fideslang.models.DatasetField - __dict__: - name: food_preference - description: User's favorite food - data_categories: [] - data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified - retention: null - fields: null - __fields_set__: !!set - data_qualifier: null - name: null - retention: null - data_categories: null - fields: null - description: null - __private_attribute_values__: {} - - !!python/object:fideslang.models.DatasetField - __dict__: - name: state - description: User's State - data_categories: - - user.provided.identifiable.contact.state - data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified - retention: null - fields: null - __fields_set__: !!set - data_qualifier: null - name: null - retention: null - data_categories: null - fields: null - description: null - __private_attribute_values__: {} - - !!python/object:fideslang.models.DatasetField - __dict__: - name: uuid - description: User's unique ID - data_categories: - - user.derived.identifiable.unique_id - data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified - retention: null - fields: null - __fields_set__: !!set - data_qualifier: null - name: null - retention: null - data_categories: null - fields: null - description: null - __private_attribute_values__: {} - __fields_set__: !!set - data_qualifier: null - name: null - retention: null - data_categories: null - fields: null - description: null - __private_attribute_values__: {} - __fields_set__: !!set - data_qualifier: null - name: null - retention: null - data_categories: null - fidesctl_meta: null - organization_fides_key: null - fides_key: null - collections: null - third_country_transfers: null - meta: null - description: null - joint_controller: null - __private_attribute_values__: {} -policies: [] From aa1ebd7b39c6ede68226cc9065ae26f8db012001 Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 12 Jul 2022 19:23:02 -0700 Subject: [PATCH 07/13] fix linting issues --- src/fidesctl/core/sync.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/fidesctl/core/sync.py b/src/fidesctl/core/sync.py index 9d686e9f8cc..4ba08862b8e 100644 --- a/src/fidesctl/core/sync.py +++ b/src/fidesctl/core/sync.py @@ -2,12 +2,11 @@ from typing import Dict, List, Optional import yaml +from fideslang.manifests import load_yaml_into_dict from fidesctl.cli.utils import echo_green, print_divider from fidesctl.core.api_helpers import get_server_resource, list_server_resources from fidesctl.core.utils import get_manifest_list -from fideslang.manifests import load_yaml_into_dict -from fideslang import FidesModel def write_manifest_file(manifest_path: str, manifest: Dict) -> None: From 37cf73448c463c5c0fc9fc03b963cc9f05521b1a Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 14 Jul 2022 16:21:53 -0700 Subject: [PATCH 08/13] rename everything to "pull" --- src/fidesctl/cli/__init__.py | 4 ++-- src/fidesctl/cli/commands/core.py | 22 ++++++++--------- src/fidesctl/core/{sync.py => pull.py} | 29 +++++++++++++---------- tests/cli/test_cli.py | 4 ++-- tests/core/{test_sync.py => test_pull.py} | 2 +- 5 files changed, 33 insertions(+), 28 deletions(-) rename src/fidesctl/core/{sync.py => pull.py} (83%) rename tests/core/{test_sync.py => test_pull.py} (62%) diff --git a/src/fidesctl/cli/__init__.py b/src/fidesctl/cli/__init__.py index 6343c3504e4..2a68caaa0d0 100644 --- a/src/fidesctl/cli/__init__.py +++ b/src/fidesctl/cli/__init__.py @@ -11,7 +11,7 @@ from fidesctl.core.config import get_config from .commands.annotate import annotate -from .commands.core import apply, evaluate, parse, sync +from .commands.core import apply, evaluate, parse, pull from .commands.crud import delete, get, ls from .commands.db import database from .commands.export import export @@ -34,7 +34,7 @@ get, ls, status, - sync, + pull, ] API_COMMAND_DICT = {command.name or str(command): command for command in API_COMMANDS} ALL_COMMANDS = API_COMMANDS + LOCAL_COMMANDS diff --git a/src/fidesctl/cli/commands/core.py b/src/fidesctl/cli/commands/core.py index e338ac72bb1..01ca21d8d59 100644 --- a/src/fidesctl/cli/commands/core.py +++ b/src/fidesctl/cli/commands/core.py @@ -14,7 +14,7 @@ from fidesctl.core import audit as _audit from fidesctl.core import evaluate as _evaluate from fidesctl.core import parse as _parse -from fidesctl.core import sync as _sync +from fidesctl.core import pull as _pull from fidesctl.core.utils import git_is_dirty @@ -141,33 +141,33 @@ def parse(ctx: click.Context, manifests_dir: str, verbose: bool = False) -> None @click.pass_context @manifests_dir_argument @click.option( - "--sync-new", - "-s", + "--all-resources", + "-a", default=None, - help="Pulls all new resources from the server into this file.", + help="Pulls all locally missing resources from the server into this file.", ) @with_analytics -def sync(ctx: click.Context, manifests_dir: str, sync_new: Optional[str]) -> None: +def pull(ctx: click.Context, manifests_dir: str, all_resources: Optional[str]) -> None: """ Update local resource files by their fides_key to match their server versions. Alternatively, with the "--all" flag all resources from the server will be pulled - down into local files. + down into a local file. - Aborts the sync if there are unstaged or untracked files in the manifests dir. + The pull is aborted if there are unstaged or untracked files in the manifests dir. """ config = ctx.obj["CONFIG"] - # Do this to validate the manifests since they won't get parsed during the sync process + # Do this to validate the manifests since they won't get parsed during the pull process _parse.parse(manifests_dir) if git_is_dirty(manifests_dir): echo_red( - f"There are unstaged changes in your manifest directory: '{manifests_dir}' \nAborting sync!" + f"There are unstaged changes in your manifest directory: '{manifests_dir}' \nAborting pull!" ) raise SystemExit(1) - _sync.sync( + _pull.pull( url=config.cli.server_url, manifests_dir=manifests_dir, headers=config.user.request_headers, - sync_new=sync_new, + all_resources=all_resources, ) diff --git a/src/fidesctl/core/sync.py b/src/fidesctl/core/pull.py similarity index 83% rename from src/fidesctl/core/sync.py rename to src/fidesctl/core/pull.py index 4ba08862b8e..9b63878d0e2 100644 --- a/src/fidesctl/core/sync.py +++ b/src/fidesctl/core/pull.py @@ -18,7 +18,7 @@ def write_manifest_file(manifest_path: str, manifest: Dict) -> None: echo_green(f"Updated manifest file written out to: '{manifest_path}'") -def sync_existing_resources( +def pull_existing_resources( manifests_dir: str, url: str, headers: Dict[str, str] ) -> List[str]: """ @@ -26,12 +26,12 @@ def sync_existing_resources( state on the server. """ manifest_path_list = get_manifest_list(manifests_dir) - # Store and return the keys of resources that get synced here. + # Store and return the keys of resources that get pulled here. existing_keys: List[str] = [] print_divider() for manifest_path in manifest_path_list: - print(f"Syncing file: '{manifest_path}'...") + print(f"Pulling file: '{manifest_path}'...") manifest = load_yaml_into_dict(manifest_path) updated_manifest = {} @@ -62,7 +62,7 @@ def sync_existing_resources( return existing_keys -def sync_missing_resources( +def pull_missing_resources( manifest_path: str, url: str, headers: Dict[str, str], existing_keys: List[str] ) -> bool: """ @@ -71,7 +71,7 @@ def sync_missing_resources( """ print(f"Writing out new resources to file: '{manifest_path}'...") - resources_to_sync = ["system", "dataset", "policies"] + resources_to_pull = ["system", "dataset", "policies"] resource_manifest = { resource: list_server_resources( url=url, @@ -80,7 +80,7 @@ def sync_missing_resources( exclude_keys=existing_keys, raw=True, ) - for resource in resources_to_sync + for resource in resources_to_pull } # Write out the resources in a file @@ -89,11 +89,11 @@ def sync_missing_resources( return True -def sync( +def pull( manifests_dir: str, url: str, headers: Dict[str, str], - sync_new: Optional[str], + all_resources: Optional[str], ) -> None: """ If a resource in a local file has a matching resource on the server, @@ -102,9 +102,14 @@ def sync( If the 'all' flag is passed, additionally pull all other server resources into local files as well. """ - existing_keys = sync_existing_resources(manifests_dir, url, headers) + existing_keys = pull_existing_resources(manifests_dir, url, headers) - if sync_new: - sync_missing_resources(sync_new, url, headers, existing_keys) + if all_resources: + pull_missing_resources( + manifest_path=all_resources, + url=url, + headers=headers, + existing_keys=existing_keys, + ) - echo_green("Sync complete.") + echo_green("Pull complete.") diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 312955e363d..2475287dd91 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -99,7 +99,7 @@ def test_dry_diff_apply(test_config_path: str, test_cli_runner: CliRunner) -> No @pytest.mark.integration -def test_sync( +def test_pull( test_config_path: str, test_cli_runner: CliRunner, tmp_path: PosixPath ) -> None: """ @@ -108,7 +108,7 @@ def test_sync( and then reset. """ test_dir = "demo_resources/" - result = test_cli_runner.invoke(cli, ["-f", test_config_path, "sync", test_dir]) + result = test_cli_runner.invoke(cli, ["-f", test_config_path, "pull", test_dir]) print(result.output) assert result.exit_code == 0 diff --git a/tests/core/test_sync.py b/tests/core/test_pull.py similarity index 62% rename from tests/core/test_sync.py rename to tests/core/test_pull.py index 73979aab247..2b00187981e 100644 --- a/tests/core/test_sync.py +++ b/tests/core/test_pull.py @@ -1,3 +1,3 @@ -def test_sync() -> None: +def test_pull() -> None: """Placeholder test.""" assert 1 From 14d559d697dad5eb0062f49ab3230cb9804339a6 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 14 Jul 2022 18:13:02 -0700 Subject: [PATCH 09/13] update the resource files due to changes on main --- .fides/dataset.yml | 61 +++++++++++++++++++++++----------------------- .fides/policy.yml | 2 ++ 2 files changed, 33 insertions(+), 30 deletions(-) diff --git a/.fides/dataset.yml b/.fides/dataset.yml index 9ee660f7966..c0c9e6d6a80 100644 --- a/.fides/dataset.yml +++ b/.fides/dataset.yml @@ -1,6 +1,7 @@ dataset: - fides_key: public organization_fides_key: default_organization + tags: null name: public description: The dataset responsible for storing all of the data related to a fidesctl instance. @@ -81,14 +82,14 @@ dataset: data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: updated_at + - name: tags description: null data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: tags + - name: updated_at description: null data_categories: - system.operations @@ -150,14 +151,14 @@ dataset: data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: updated_at + - name: tags description: null data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: tags + - name: updated_at description: null data_categories: - system.operations @@ -228,14 +229,14 @@ dataset: data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: updated_at + - name: tags description: null data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: tags + - name: updated_at description: null data_categories: - system.operations @@ -333,14 +334,14 @@ dataset: data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: updated_at + - name: tags description: null data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: tags + - name: updated_at description: null data_categories: - system.operations @@ -468,22 +469,22 @@ dataset: data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: third_country_transfers + - name: tags description: null data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: updated_at - description: The timestamp of when the row was last updated + - name: third_country_transfers + description: null data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: tags - description: null + - name: updated_at + description: The timestamp of when the row was last updated data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified @@ -644,15 +645,15 @@ dataset: data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: updated_at - description: The timestamp of when the row was last updated + - name: tags + description: null data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: tags - description: null + - name: updated_at + description: The timestamp of when the row was last updated data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified @@ -713,15 +714,15 @@ dataset: data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: updated_at - description: The timestamp of when the row was last updated + - name: tags + description: null data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: tags - description: null + - name: updated_at + description: The timestamp of when the row was last updated data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified @@ -775,15 +776,15 @@ dataset: data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: updated_at - description: The timestamp of when the row was last updated + - name: tags + description: null data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: tags - description: null + - name: updated_at + description: The timestamp of when the row was last updated data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified @@ -909,22 +910,22 @@ dataset: data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: third_country_transfers + - name: tags description: null data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: updated_at - description: The timestamp of when the row was last updated + - name: third_country_transfers + description: null data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified retention: null fields: null - - name: tags - description: null + - name: updated_at + description: The timestamp of when the row was last updated data_categories: - system.operations data_qualifier: aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified diff --git a/.fides/policy.yml b/.fides/policy.yml index a3b6616d1fd..545f099d4cd 100644 --- a/.fides/policy.yml +++ b/.fides/policy.yml @@ -1,6 +1,7 @@ policy: - fides_key: fidesctl_policy organization_fides_key: default_organization + tags: null name: Fidesctl Policy description: The main privacy policy for Fidesctl. rules: @@ -20,6 +21,7 @@ policy: data_qualifier: aggregated - fides_key: data_sharing_policy organization_fides_key: default_organization + tags: null name: Data Sharing description: The privacy policy that governs sharing of data with third parties. rules: From 4b13761e73a5115a0ded4756b35c16ef850fc698 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 14 Jul 2022 18:30:44 -0700 Subject: [PATCH 10/13] update tests, add git_reset as a general fixture --- tests/cli/test_cli.py | 51 +++++++++++++++++++++++++++++------------ tests/conftest.py | 11 +++++++++ tests/core/test_pull.py | 11 +++++++-- 3 files changed, 56 insertions(+), 17 deletions(-) diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index ebe8905aa1f..25f34bdc957 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -99,21 +99,42 @@ def test_dry_diff_apply(test_config_path: str, test_cli_runner: CliRunner) -> No @pytest.mark.integration -def test_pull( - test_config_path: str, test_cli_runner: CliRunner, tmp_path: PosixPath -) -> None: - """ - Due to the fact that this command checks the real git status, a pytest - tmp_dir can't be used. Consequently a real directory must be tested against - and then reset. - """ - test_dir = "demo_resources/" - result = test_cli_runner.invoke(cli, ["-f", test_config_path, "pull", test_dir]) - print(result.output) - assert result.exit_code == 0 - - git_session = Repo().git() - git_session.checkout("HEAD", test_dir) +class TestPull: + def test_pull( + self, + test_config_path: str, + test_cli_runner: CliRunner, + tmp_path: PosixPath, + git_reset: None, + ) -> None: + """ + Due to the fact that this command checks the real git status, a pytest + tmp_dir can't be used. Consequently a real directory must be tested against + and then reset. + """ + test_dir = "demo_resources/" + result = test_cli_runner.invoke(cli, ["-f", test_config_path, "pull", test_dir]) + print(result.output) + assert result.exit_code == 0 + + def test_pull_all( + self, + test_config_path: str, + test_cli_runner: CliRunner, + tmp_path: PosixPath, + git_reset: None, + ) -> None: + """ + Due to the fact that this command checks the real git status, a pytest + tmp_dir can't be used. Consequently a real directory must be tested against + and then reset. + """ + test_dir = "demo_resources/" + result = test_cli_runner.invoke( + cli, ["-f", test_config_path, "pull", test_dir, "-a", "test_resources.yml"] + ) + print(result.output) + assert result.exit_code == 0 @pytest.mark.integration diff --git a/tests/conftest.py b/tests/conftest.py index 52cc10b388d..37477fa4070 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,6 +5,7 @@ import pytest import yaml +from git.repo import Repo from fideslang import models from starlette.testclient import TestClient @@ -16,6 +17,16 @@ TEST_INVALID_CONFIG_PATH = "tests/test_invalid_config.toml" +@pytest.fixture(scope="function") +def git_reset() -> None: + """This fixture is used to reset the repo files to HEAD.""" + + # Yield here so that the reset happens after the tests run + yield + git_session = Repo().git() + git_session.checkout("HEAD", test_dir) + + @pytest.fixture(scope="session") def test_config_path() -> Generator: yield TEST_CONFIG_PATH diff --git a/tests/core/test_pull.py b/tests/core/test_pull.py index 2b00187981e..c6fd3dc7acb 100644 --- a/tests/core/test_pull.py +++ b/tests/core/test_pull.py @@ -1,3 +1,10 @@ -def test_pull() -> None: +import pytest + +from fidesctl.core.config import FidesctlConfig +from fidesctl.core.pull import pull_existing_resources + + +@pytest.mark.unit +def test_pull_existing_resources(test_config: FidesctlConfig) -> None: """Placeholder test.""" - assert 1 + pull_existing_resources("demo_resources/", test_config.cli.server_url, test_config.user.request_headers) From 9ad3a7ec6f18e8502e2d3f535ded04d581eb26fb Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 14 Jul 2022 18:52:29 -0700 Subject: [PATCH 11/13] fix tests --- src/fidesctl/core/pull.py | 2 +- tests/cli/test_cli.py | 30 ++++++++++++++++++++++-------- tests/conftest.py | 10 ---------- tests/core/test_pull.py | 15 ++++++++++++++- 4 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/fidesctl/core/pull.py b/src/fidesctl/core/pull.py index 9b63878d0e2..e09a029d57e 100644 --- a/src/fidesctl/core/pull.py +++ b/src/fidesctl/core/pull.py @@ -71,7 +71,7 @@ def pull_missing_resources( """ print(f"Writing out new resources to file: '{manifest_path}'...") - resources_to_pull = ["system", "dataset", "policies"] + resources_to_pull = ["system", "dataset", "policy"] resource_manifest = { resource: list_server_resources( url=url, diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 25f34bdc957..86c7eb56487 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -1,6 +1,5 @@ # pylint: disable=missing-docstring, redefined-outer-name import os -from pathlib import PosixPath from typing import Generator import pytest @@ -13,6 +12,13 @@ OKTA_URL = "https://dev-78908748.okta.com" +def git_reset(change_dir: str) -> None: + """This fixture is used to reset the repo files to HEAD.""" + + git_session = Repo().git() + git_session.checkout("HEAD", change_dir) + + @pytest.fixture() def test_cli_runner() -> Generator: runner = CliRunner() @@ -104,16 +110,15 @@ def test_pull( self, test_config_path: str, test_cli_runner: CliRunner, - tmp_path: PosixPath, - git_reset: None, ) -> None: """ Due to the fact that this command checks the real git status, a pytest tmp_dir can't be used. Consequently a real directory must be tested against and then reset. """ - test_dir = "demo_resources/" + test_dir = ".fides/" result = test_cli_runner.invoke(cli, ["-f", test_config_path, "pull", test_dir]) + git_reset(test_dir) print(result.output) assert result.exit_code == 0 @@ -121,18 +126,27 @@ def test_pull_all( self, test_config_path: str, test_cli_runner: CliRunner, - tmp_path: PosixPath, - git_reset: None, ) -> None: """ Due to the fact that this command checks the real git status, a pytest tmp_dir can't be used. Consequently a real directory must be tested against and then reset. """ - test_dir = "demo_resources/" + test_dir = ".fides/" + test_file = ".fides/test_resources.yml" result = test_cli_runner.invoke( - cli, ["-f", test_config_path, "pull", test_dir, "-a", "test_resources.yml"] + cli, + [ + "-f", + test_config_path, + "pull", + test_dir, + "-a", + ".fides/test_resources.yml", + ], ) + git_reset(test_dir) + os.remove(test_file) print(result.output) assert result.exit_code == 0 diff --git a/tests/conftest.py b/tests/conftest.py index 37477fa4070..f67c5c6b749 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -17,16 +17,6 @@ TEST_INVALID_CONFIG_PATH = "tests/test_invalid_config.toml" -@pytest.fixture(scope="function") -def git_reset() -> None: - """This fixture is used to reset the repo files to HEAD.""" - - # Yield here so that the reset happens after the tests run - yield - git_session = Repo().git() - git_session.checkout("HEAD", test_dir) - - @pytest.fixture(scope="session") def test_config_path() -> Generator: yield TEST_CONFIG_PATH diff --git a/tests/core/test_pull.py b/tests/core/test_pull.py index c6fd3dc7acb..b7aa522bbfa 100644 --- a/tests/core/test_pull.py +++ b/tests/core/test_pull.py @@ -1,10 +1,23 @@ import pytest +from git.repo import Repo from fidesctl.core.config import FidesctlConfig from fidesctl.core.pull import pull_existing_resources +def git_reset(change_dir: str) -> None: + """This fixture is used to reset the repo files to HEAD.""" + + git_session = Repo().git() + git_session.checkout("HEAD", change_dir) + + @pytest.mark.unit def test_pull_existing_resources(test_config: FidesctlConfig) -> None: """Placeholder test.""" - pull_existing_resources("demo_resources/", test_config.cli.server_url, test_config.user.request_headers) + test_dir = ".fides/" + existing_keys = pull_existing_resources( + test_dir, test_config.cli.server_url, test_config.user.request_headers + ) + git_reset(test_dir) + assert len(existing_keys) > 1 From 8cb74f009354904d93cba2c9a8351a1e66d581a5 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 14 Jul 2022 18:53:38 -0700 Subject: [PATCH 12/13] fix conftest linting error --- tests/conftest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index f67c5c6b749..52cc10b388d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,7 +5,6 @@ import pytest import yaml -from git.repo import Repo from fideslang import models from starlette.testclient import TestClient From d21bf162959da7b5d6452738b369460783e38392 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 14 Jul 2022 18:56:44 -0700 Subject: [PATCH 13/13] update the changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b4b35164d0..9464ba28411 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,8 @@ The types of changes are: * System scanning results: AWS systems are stored and can be selected for review * Added Cypress for testing [713](https://github.com/ethyca/fides/pull/833) * CustomInput type "password" with show/hide icon. -* Sync CLI command now checks for untracked/unstaged files in the manifests dir [#869](https://github.com/ethyca/fides/pull/869) +* Pull CLI command now checks for untracked/unstaged files in the manifests dir [#869](https://github.com/ethyca/fides/pull/869) +* Pull CLI command has a flag to pull missing files from the server [#895](https://github.com/ethyca/fides/pull/895) * Add Okta support to the `/generate` endpoint [#842](https://github.com/ethyca/fides/pull/842) * Add db support to `/generate` endpoint [849](https://github.com/ethyca/fides/pull/849) * Added OpenAPI TypeScript client generation for the UI app. See the [README](/clients/admin-ui/src/types/api/README.md) for more details.