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

Check for uncommitted files before running sync #869

Merged
merged 15 commits into from
Jul 12, 2022
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,19 @@ The types of changes are:
## [Unreleased](https://github.com/ethyca/fides/compare/1.7.0...main)

### Added

* Add datasets via YAML in the UI [#708](https://github.com/ethyca/fides/issues/708)
* Add delete confirmation when deleting a field or collection from a dataset [808](https://github.com/ethyca/fides/issues/808)
* Add ability to delete datasets from the UI
* Initial configuration wizard UI view
* System scanning step: AWS credentials form and initial `generate` API usage.
* 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)

### Changed

* Updated the `datamap` endpoint to return human-readable column names as the first response item [#779](https://github.com/ethyca/fides/pull/779)
* Initial configuration wizard UI view
* Refactored step & form results management to use Redux Toolkit slice.
Expand All @@ -39,6 +42,7 @@ The types of changes are:
* Fixed a build issue causing an `unknown` version of `fidesctl` to be installed in published Docker images [#836](https://github.com/ethyca/fides/pull/836)

### Changed

* Remove the `obscure` requirement from the `generate` endpoint [#819](https://github.com/ethyca/fides/pull/819)

## [1.7.0](https://github.com/ethyca/fides/compare/1.6.1...1.7.0) - 2022-06-23
Expand Down Expand Up @@ -97,7 +101,7 @@ The types of changes are:

* Replaced all references to `make` with `nox` [#547](https://github.com/ethyca/fides/pull/547)
* Removed config/schemas page [#613](https://github.com/ethyca/fides/issues/613)
* Dataset UI and config wizard docs added (https://github.com/ethyca/fides/pull/697)
* Dataset UI and config wizard docs added (<https://github.com/ethyca/fides/pull/697>)
* The fides README now walks through generating a datamap [#746](https://github.com/ethyca/fides/pull/746)

### Fixed
Expand Down
1 change: 1 addition & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ cryptography>=36
deepdiff==5.8.0
fideslang==0.9.0
fideslog==1.1.5
GitPython==3.1
loguru>=0.5,<0.6
openpyxl==3.0.9
pandas==1.4
Expand Down
10 changes: 9 additions & 1 deletion src/fidesctl/cli/commands/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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!"
ThomasLaPiana marked this conversation as resolved.
Show resolved Hide resolved
)
raise SystemExit(1)
_sync.sync(
url=config.cli.server_url,
manifests_dir=manifests_dir,
Expand Down
11 changes: 9 additions & 2 deletions src/fidesctl/cli/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
15 changes: 15 additions & 0 deletions src/fidesctl/core/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -119,3 +120,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
16 changes: 11 additions & 5 deletions tests/cli/test_cli.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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:
Expand Down
23 changes: 23 additions & 0 deletions tests/core/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# pylint: disable=missing-docstring, redefined-outer-name
import os
from pathlib import PosixPath
from typing import Generator

Expand Down Expand Up @@ -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)