diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f39a6bf1b..31ed9e17ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ 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) * 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. @@ -40,6 +41,7 @@ The types of changes are: * Initial configuration wizard UI view * Refactored step & form results management to use Redux Toolkit slice. * Remove the `obscure` requirement from the `generate` endpoint [#819](https://github.com/ethyca/fides/pull/819) +* Remove the `obscure` requirement from the `generate` endpoint [#819](https://github.com/ethyca/fides/pull/819) ### Docs diff --git a/docker-compose.yml b/docker-compose.yml index 3741e3f89d..dbb7a16173 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -29,6 +29,7 @@ services: FIDESCTL_TEST_MODE: "True" FIDESCTL__CLI__SERVER_HOST: "fidesctl" FIDESCTL__CLI__SERVER_PORT: "8080" + FIDESCTL__CLI__ANALYTICS_ID: ${FIDESCTL__CLI__ANALYTICS_ID} FIDESCTL__API__DATABASE_HOST: "fidesctl-db" fidesctl-ui: diff --git a/requirements.txt b/requirements.txt index 31c247d75c..b9eecc6cc5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,6 +8,7 @@ fastapi==0.77.1 fideslang==0.9.0 fideslib==2.2.2 fideslog==1.1.5 +GitPython==3.1 loguru>=0.5,<0.6 openpyxl==3.0.9 pandas==1.4 diff --git a/src/fidesctl/cli/commands/core.py b/src/fidesctl/cli/commands/core.py index 4911b05f0f..1ba68988dd 100644 --- a/src/fidesctl/cli/commands/core.py +++ b/src/fidesctl/cli/commands/core.py @@ -7,12 +7,13 @@ manifests_dir_argument, verbose_flag, ) -from fidesctl.cli.utils import pretty_echo, print_divider, with_analytics +from fidesctl.cli.utils import echo_red, pretty_echo, print_divider, with_analytics from fidesctl.core import apply as _apply 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.utils import git_is_dirty @click.command() @@ -141,11 +142,18 @@ 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. + + Aborts the sync 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 _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!" + ) + raise SystemExit(1) _sync.sync( url=config.cli.server_url, manifests_dir=manifests_dir, diff --git a/src/fidesctl/cli/utils.py b/src/fidesctl/cli/utils.py index ca5332e0a5..bbd3bb6614 100644 --- a/src/fidesctl/cli/utils.py +++ b/src/fidesctl/cli/utils.py @@ -101,11 +101,18 @@ def check_and_update_analytics_config(ctx: click.Context, config_path: str) -> N user={"analytics_opt_out": ctx.obj["CONFIG"].user.analytics_opt_out} ) - if ctx.obj["CONFIG"].user.analytics_opt_out is False and get_config_from_file( + is_analytics_opt_out = ctx.obj["CONFIG"].user.analytics_opt_out + is_analytics_opt_out_config_empty = get_config_from_file( config_path, "cli", "analytics_id", - ) in ("", None): + ) in ("", None) + is_analytics_opt_out_env_var_set = getenv("FIDESCTL__CLI__ANALYTICS_ID") + if ( + not is_analytics_opt_out + and is_analytics_opt_out_config_empty + and not is_analytics_opt_out_env_var_set + ): config_updates.update(cli={"analytics_id": ctx.obj["CONFIG"].cli.analytics_id}) if len(config_updates) > 0: diff --git a/src/fidesctl/core/utils.py b/src/fidesctl/core/utils.py index b018b701b0..fced99c31d 100644 --- a/src/fidesctl/core/utils.py +++ b/src/fidesctl/core/utils.py @@ -12,6 +12,7 @@ import sqlalchemy from fideslang.models import DatasetField, FidesModel from fideslang.validation import FidesValidationError +from git.repo import Repo from sqlalchemy.engine import Engine logger = logging.getLogger("server_api") @@ -117,3 +118,17 @@ def sanitize_fides_key(proposed_fides_key: str) -> str: """ sanitized_fides_key = re.sub(r"[^a-zA-Z0-9_.-]", "_", proposed_fides_key) return sanitized_fides_key + + +def git_is_dirty(dir_to_check: str = ".") -> bool: + """ + Checks to see if the local repo has unstaged changes. + Can also specify a directory to check. + """ + repo = Repo() + git_session = repo.git() + + dirty_phrases = ["Changes not staged for commit:", "Untracked files:"] + git_status = git_session.status(dir_to_check).split("\n") + is_dirty = any(phrase in git_status for phrase in dirty_phrases) + return is_dirty diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 75ef7e8b00..312955e363 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -1,11 +1,11 @@ # pylint: disable=missing-docstring, redefined-outer-name import os from pathlib import PosixPath -from shutil import copytree from typing import Generator import pytest from click.testing import CliRunner +from git.repo import Repo from py._path.local import LocalPath from fidesctl.cli import cli @@ -102,13 +102,19 @@ def test_dry_diff_apply(test_config_path: str, test_cli_runner: CliRunner) -> No def test_sync( test_config_path: str, test_cli_runner: CliRunner, tmp_path: PosixPath ) -> None: - copytree("demo_resources", tmp_path, dirs_exist_ok=True) - result = test_cli_runner.invoke( - cli, ["-f", test_config_path, "sync", str(tmp_path)] - ) + """ + 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, "sync", test_dir]) print(result.output) assert result.exit_code == 0 + git_session = Repo().git() + git_session.checkout("HEAD", test_dir) + @pytest.mark.integration def test_audit(test_config_path: str, test_cli_runner: CliRunner) -> None: diff --git a/tests/core/test_utils.py b/tests/core/test_utils.py index 3934897519..877c7f4a48 100644 --- a/tests/core/test_utils.py +++ b/tests/core/test_utils.py @@ -1,4 +1,5 @@ # pylint: disable=missing-docstring, redefined-outer-name +import os from pathlib import PosixPath from typing import Generator @@ -90,3 +91,25 @@ def test_sanitize_fides_key(fides_key: str, sanitized_fides_key: str) -> None: ) def test_check_fides_key(fides_key: str, sanitized_fides_key: str) -> None: assert sanitized_fides_key == utils.check_fides_key(fides_key) + + +@pytest.mark.unit +class TestGitIsDirty: + """ + These tests can't use the standard pytest tmpdir + because the files need to be within the git repo + to be properly tested. + + They will therefore also break if the real dir + used for testing is deleted. + """ + + def test_not_dirty(self) -> None: + assert not utils.git_is_dirty("tests/data/example_sql/") + + def test_new_file_is_dirty(self) -> None: + test_file = "tests/data/example_sql/new_file.txt" + with open(test_file, "w") as file: + file.write("test file") + assert utils.git_is_dirty() + os.remove(test_file)