From 4b45721079cefb9e6a90540645a1552d23c5f892 Mon Sep 17 00:00:00 2001 From: Peter <101368063+ClassicMMT@users.noreply.github.com> Date: Fri, 17 Jul 2026 10:32:35 +1200 Subject: [PATCH 1/6] chore: Update to datamasque-python==1.2.2 --- CHANGELOG.md | 11 +++ pyproject.toml | 2 +- src/datamasque_cli/commands/discovery.py | 2 +- .../commands/ruleset_libraries.py | 8 +- src/datamasque_cli/commands/rulesets.py | 12 ++- src/datamasque_cli/output.py | 11 +++ tests/commands/test_discovery.py | 41 ++++++++++ tests/commands/test_ruleset_libraries.py | 51 ++++++++++-- tests/commands/test_rulesets.py | 82 +++++++++++++++---- uv.lock | 8 +- 10 files changed, 193 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3205275..4e7602d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## v1.5.0 + +### Added +- Support for datamasque-python 1.1.8. + - `dm discover schema-results` handles matches with no label. + - Validation errors are now printed. + - `dm rulesets validate` and `dm libraries validate` now fail (return 4) + on invalid rulesets/libraries. + - `dm discover db-report` writes a zip archive returned for large reports to + `--output`, aborting with a hint rather than dumping binary data to stdout. + ## v1.4.0 ### Added diff --git a/pyproject.toml b/pyproject.toml index b4dfa47..3f6859a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,7 +12,7 @@ requires-python = ">=3.11" dependencies = [ "typer>=0.15.0", "tomli-w>=1.0.0", - "datamasque-python>=1.0.0,<2", + "datamasque-python>=1.2.2,<2", ] classifiers = [ "Development Status :: 4 - Beta", diff --git a/src/datamasque_cli/commands/discovery.py b/src/datamasque_cli/commands/discovery.py index 2aa6914..c38e10a 100644 --- a/src/datamasque_cli/commands/discovery.py +++ b/src/datamasque_cli/commands/discovery.py @@ -77,7 +77,7 @@ def schema_results( "table": r.table, "column": r.column, "data_type": r.data.data_type or "", - "matches": ", ".join(m.label for m in r.data.discovery_matches) or "-", + "matches": ", ".join(m.label for m in r.data.discovery_matches if m.label) or "-", "constraint": r.data.constraint or "", } for r in results diff --git a/src/datamasque_cli/commands/ruleset_libraries.py b/src/datamasque_cli/commands/ruleset_libraries.py index 675b73b..b34e960 100644 --- a/src/datamasque_cli/commands/ruleset_libraries.py +++ b/src/datamasque_cli/commands/ruleset_libraries.py @@ -8,7 +8,7 @@ from datamasque.client.models.ruleset_library import RulesetLibrary from datamasque_cli.client import get_client -from datamasque_cli.output import ErrorCode, abort, print_success, render_output +from datamasque_cli.output import ErrorCode, abort, abort_if_invalid, print_success, render_output app = typer.Typer(help="Manage ruleset libraries.", no_args_is_help=True) @@ -114,16 +114,18 @@ def validate_library( Triggers a server-side validation pass on an existing library and reports the result. """ + label = f"{namespace}/{name}" if namespace else name + client = get_client(profile) lib = client.get_ruleset_library_by_name(name, namespace) if lib is None: - label = f"{namespace}/{name}" if namespace else name abort(f"Library '{label}' not found.", code=ErrorCode.NOT_FOUND) validated = client.validate_ruleset_library(lib.id) + abort_if_invalid(f"Library '{label}'", validated.is_valid, validated.validation_errors) + status = validated.is_valid.value if validated.is_valid else "unknown" - label = f"{namespace}/{name}" if namespace else name print_success(f"Library '{label}' validation status: {status}") diff --git a/src/datamasque_cli/commands/rulesets.py b/src/datamasque_cli/commands/rulesets.py index 80100e1..df38f7f 100644 --- a/src/datamasque_cli/commands/rulesets.py +++ b/src/datamasque_cli/commands/rulesets.py @@ -13,7 +13,16 @@ from datamasque.client.models.ruleset import Ruleset, RulesetType from datamasque_cli.client import get_client -from datamasque_cli.output import ErrorCode, abort, print_error, print_info, print_success, print_warning, render_output +from datamasque_cli.output import ( + ErrorCode, + abort, + abort_if_invalid, + print_error, + print_info, + print_success, + print_warning, + render_output, +) app = typer.Typer(help="Manage masking rulesets.", no_args_is_help=True) @@ -204,6 +213,7 @@ def validate_ruleset( # `try/finally` so a Ctrl-C or unexpected exception between create and # delete still cleans up the temp ruleset on the server. try: + abort_if_invalid(f"Ruleset '{file.name}' ({rs_type.value})", created.is_valid, created.validation_errors) print_success(f"Ruleset '{file.name}' ({rs_type.value}) is valid.") finally: if created.id is not None: diff --git a/src/datamasque_cli/output.py b/src/datamasque_cli/output.py index 58dafaa..5a45819 100644 --- a/src/datamasque_cli/output.py +++ b/src/datamasque_cli/output.py @@ -18,6 +18,7 @@ from typing import Any, NoReturn import typer +from datamasque.client.models.status import ValidationErrorDetails, ValidationStatus from rich.console import Console from rich.table import Table from rich.text import Text @@ -248,3 +249,13 @@ def abort(message: str, *, code: ErrorCode = ErrorCode.ERROR, hint: str | None = if hint: console.print(f"[dim]Hint: {hint}[/dim]") raise SystemExit(EXIT_CODES[code]) + + +def abort_if_invalid(subject: str, is_valid: ValidationStatus | None, errors: list[ValidationErrorDetails]) -> None: + """Print each server-side validation error for `subject` and exit, if it failed validation.""" + if is_valid is not ValidationStatus.invalid and not errors: + return + for error in errors: + location = f" (line {error.line_number})" if error.line_number is not None else "" + print_error(f"{error.message}{location}") + abort(f"{subject} is invalid.", code=ErrorCode.INVALID_INPUT) diff --git a/tests/commands/test_discovery.py b/tests/commands/test_discovery.py index bb76651..d68fa07 100644 --- a/tests/commands/test_discovery.py +++ b/tests/commands/test_discovery.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json from pathlib import Path from types import SimpleNamespace from unittest.mock import MagicMock, patch @@ -150,3 +151,43 @@ def test_schema_results_lists_with_flattened_rows(mock_get_client: MagicMock, ru assert '"EMAIL_ADDRESS"' in result.stdout assert '"US_SSN, PII"' in result.stdout assert '"Primary"' in result.stdout + + +@patch(f"{MODULE}.get_client") +def test_schema_results_skips_unlabelled_matches(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_schema_discovery_results.return_value = [ + SimpleNamespace( + id=1, + column="email", + table="users", + schema_name="public", + data=SimpleNamespace( + data_type="varchar", + discovery_matches=[ + SimpleNamespace(label="EMAIL_ADDRESS"), + SimpleNamespace(label=None), + ], + constraint="", + ), + ), + SimpleNamespace( + id=2, + column="notes", + table="users", + schema_name="public", + data=SimpleNamespace( + data_type="text", + discovery_matches=[SimpleNamespace(label=None)], + constraint="", + ), + ), + ] + + result = runner.invoke(app, ["discover", "schema-results", "42", "--json"]) + + assert result.exit_code == 0 + rows = json.loads(result.stdout) + assert rows[0]["matches"] == "EMAIL_ADDRESS" + assert rows[1]["matches"] == "-" diff --git a/tests/commands/test_ruleset_libraries.py b/tests/commands/test_ruleset_libraries.py index 522de9f..ecacdd1 100644 --- a/tests/commands/test_ruleset_libraries.py +++ b/tests/commands/test_ruleset_libraries.py @@ -3,6 +3,7 @@ from types import SimpleNamespace from unittest.mock import MagicMock, patch +from datamasque.client.models.status import ValidationStatus from typer.testing import CliRunner from datamasque_cli.main import app @@ -10,6 +11,13 @@ MODULE = "datamasque_cli.commands.ruleset_libraries" +def _validated_library( + is_valid: ValidationStatus | None, + validation_errors: list[SimpleNamespace] | None = None, +) -> SimpleNamespace: + return SimpleNamespace(id="lib-uuid", is_valid=is_valid, validation_errors=validation_errors or []) + + @patch(f"{MODULE}.get_client") def test_delete_library_aborts_when_missing(mock_get_client: MagicMock, runner: CliRunner) -> None: client = MagicMock() @@ -38,13 +46,8 @@ def test_delete_library_proceeds_when_present(mock_get_client: MagicMock, runner def test_validate_library_reports_status(mock_get_client: MagicMock, runner: CliRunner) -> None: client = MagicMock() mock_get_client.return_value = client - original = MagicMock() - original.id = "lib-uuid" - client.get_ruleset_library_by_name.return_value = original - - validated = MagicMock() - validated.is_valid = MagicMock(value="valid") - client.validate_ruleset_library.return_value = validated + client.get_ruleset_library_by_name.return_value = SimpleNamespace(id="lib-uuid", name="my-lib", namespace="") + client.validate_ruleset_library.return_value = _validated_library(ValidationStatus.valid) result = runner.invoke(app, ["libraries", "validate", "my-lib"]) @@ -63,3 +66,37 @@ def test_validate_library_aborts_when_missing(mock_get_client: MagicMock, runner assert result.exit_code != 0 client.validate_ruleset_library.assert_not_called() + + +@patch(f"{MODULE}.get_client") +def test_validate_library_invalid_prints_errors_and_exits_4(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.get_ruleset_library_by_name.return_value = SimpleNamespace(id="lib-uuid", name="my-lib", namespace="") + client.validate_ruleset_library.return_value = _validated_library( + ValidationStatus.invalid, + [ + SimpleNamespace(message="unknown mask type 'from_nowhere'", line_number=3), + SimpleNamespace(message="duplicate anchor 'email'", line_number=None), + ], + ) + + result = runner.invoke(app, ["libraries", "validate", "my-lib"]) + + assert result.exit_code == 4 # invalid_input + assert "unknown mask type 'from_nowhere'" in result.stderr + assert "line 3" in result.stderr + assert "duplicate anchor 'email'" in result.stderr + + +@patch(f"{MODULE}.get_client") +def test_validate_library_nonterminal_status_passes_through(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.get_ruleset_library_by_name.return_value = SimpleNamespace(id="lib-uuid", name="my-lib", namespace="") + client.validate_ruleset_library.return_value = _validated_library(ValidationStatus.in_progress) + + result = runner.invoke(app, ["libraries", "validate", "my-lib"]) + + assert result.exit_code == 0 + assert "in_progress" in result.stderr diff --git a/tests/commands/test_rulesets.py b/tests/commands/test_rulesets.py index 677f99f..fc418ee 100644 --- a/tests/commands/test_rulesets.py +++ b/tests/commands/test_rulesets.py @@ -1,11 +1,14 @@ from __future__ import annotations +from collections.abc import Callable from pathlib import Path from types import SimpleNamespace from unittest.mock import MagicMock, patch +import pytest from datamasque.client.exceptions import DataMasqueApiError from datamasque.client.models.ruleset import RulesetType +from datamasque.client.models.status import ValidationStatus from typer.testing import CliRunner from datamasque_cli.main import app @@ -17,6 +20,20 @@ def _ruleset(id_: int, name: str, rs_type: RulesetType) -> SimpleNamespace: return SimpleNamespace(id=id_, name=name, ruleset_type=rs_type, yaml="") +def _create_returning( + is_valid: ValidationStatus | None, + validation_errors: list[SimpleNamespace] | None = None, +) -> Callable[[object], object]: + + def fake_create(rs: object) -> object: + rs.id = 99 # type: ignore[attr-defined] + rs.is_valid = is_valid # type: ignore[attr-defined] + rs.validation_errors = validation_errors or [] # type: ignore[attr-defined] + return rs + + return fake_create + + # -- create (type resolution via server lookup) ---------------------------- @@ -239,12 +256,7 @@ def test_validate_uses_unique_temp_name_and_cleans_by_id( ) -> None: client = MagicMock() mock_get_client.return_value = client - - def fake_create(rs: object) -> object: - rs.id = 99 # type: ignore[attr-defined] - return rs - - client.create_or_update_ruleset.side_effect = fake_create + client.create_or_update_ruleset.side_effect = _create_returning(ValidationStatus.valid) yaml_file = tmp_path / "rs.yaml" yaml_file.write_text("tasks:\n - type: mask_table\n") @@ -268,12 +280,7 @@ def test_validate_cleans_up_when_print_success_interrupted( """`try/finally` guarantees the temp ruleset is deleted even if a later step raises.""" client = MagicMock() mock_get_client.return_value = client - - def fake_create(rs: object) -> object: - rs.id = 99 # type: ignore[attr-defined] - return rs - - client.create_or_update_ruleset.side_effect = fake_create + client.create_or_update_ruleset.side_effect = _create_returning(ValidationStatus.valid) yaml_file = tmp_path / "rs.yaml" yaml_file.write_text("tasks:\n - type: mask_table\n") @@ -287,12 +294,7 @@ def fake_create(rs: object) -> object: def test_validate_warns_when_cleanup_fails(mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path) -> None: client = MagicMock() mock_get_client.return_value = client - - def fake_create(rs: object) -> object: - rs.id = 99 # type: ignore[attr-defined] - return rs - - client.create_or_update_ruleset.side_effect = fake_create + client.create_or_update_ruleset.side_effect = _create_returning(ValidationStatus.valid) client.delete_ruleset_by_id_if_exists.side_effect = DataMasqueApiError("boom", response=MagicMock()) yaml_file = tmp_path / "rs.yaml" @@ -304,6 +306,50 @@ def fake_create(rs: object) -> object: assert "left on server" in result.stderr +@patch(f"{MODULE}.get_client") +def test_validate_sync_invalid_prints_errors_and_cleans_up( + mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path +) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.create_or_update_ruleset.side_effect = _create_returning( + ValidationStatus.invalid, + [ + SimpleNamespace(message="unknown mask type 'from_nowhere'", line_number=7), + SimpleNamespace(message="tasks must not be empty", line_number=None), + ], + ) + + yaml_file = tmp_path / "rs.yaml" + yaml_file.write_text("tasks: []\n") + + result = runner.invoke(app, ["rulesets", "validate", "--file", str(yaml_file), "--type", "database"]) + + assert result.exit_code == 4 # invalid_input + assert "unknown mask type 'from_nowhere'" in result.stderr + assert "line 7" in result.stderr + assert "tasks must not be empty" in result.stderr + client.delete_ruleset_by_id_if_exists.assert_called_once_with(99) + + +@pytest.mark.parametrize("initial_status", [None, ValidationStatus.in_progress]) +@patch(f"{MODULE}.get_client") +def test_validate_nonterminal_status_reports_valid( + mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path, initial_status: ValidationStatus | None +) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.create_or_update_ruleset.side_effect = _create_returning(initial_status) + + yaml_file = tmp_path / "rs.yaml" + yaml_file.write_text("tasks:\n - type: mask_table\n") + + result = runner.invoke(app, ["rulesets", "validate", "--file", str(yaml_file), "--type", "database"]) + + assert result.exit_code == 0 + client.delete_ruleset_by_id_if_exists.assert_called_once_with(99) + + # -- export-bundle / import-bundle ---------------------------------------- diff --git a/uv.lock b/uv.lock index 13b056f..68a8441 100644 --- a/uv.lock +++ b/uv.lock @@ -159,7 +159,7 @@ dev = [ [package.metadata] requires-dist = [ - { name = "datamasque-python", specifier = ">=1.0.0,<2" }, + { name = "datamasque-python", specifier = ">=1.2.2,<2" }, { name = "tomli-w", specifier = ">=1.0.0" }, { name = "typer", specifier = ">=0.15.0" }, ] @@ -174,15 +174,15 @@ dev = [ [[package]] name = "datamasque-python" -version = "1.0.4" +version = "1.2.2" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "pydantic" }, { name = "requests" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/e0/52/1acd8c73b15e07c417a7a90060facc15b8823d5cf3207c6a51a8f9510be6/datamasque_python-1.0.4.tar.gz", hash = "sha256:45d1020364e16cd8200b972960f2bf72a683a2633cacde0c0d3eb8b00a80191a", size = 164063, upload-time = "2026-06-09T06:35:57.719Z" } +sdist = { url = "https://files.pythonhosted.org/packages/be/c0/70ecb82cd6f3672bd1a582ff422a2edbb9cf26dc861bacf688df6c489a86/datamasque_python-1.2.2.tar.gz", hash = "sha256:72244e318d32871a7b4a22da5b88592979c3ec0f38d323b91001ff7335eda006", size = 206865, upload-time = "2026-07-31T01:19:05.916Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/50/8e/7323a24cd06116cd2b1fcb3a664df86660fc11dbc49c470afc91d2744819/datamasque_python-1.0.4-py3-none-any.whl", hash = "sha256:893eb5e63814d2862d3f2d5d0b26850cb7367f54cd315bb7e2716ed4cdd69cd3", size = 50839, upload-time = "2026-06-09T06:35:56.328Z" }, + { url = "https://files.pythonhosted.org/packages/b8/4f/472bfaed98a67bdf25ffd5a9590c6c5b7ca379c50aea494bb53496acc4ee/datamasque_python-1.2.2-py3-none-any.whl", hash = "sha256:d324f71108b179422613535111dbfbefad53a8ed4e388bcdfe61da041d051f8b", size = 76315, upload-time = "2026-07-31T01:19:04.628Z" }, ] [[package]] From 5a0d608316a8a2afa5c2e4f43a25b3380dee9749 Mon Sep 17 00:00:00 2001 From: Peter <101368063+ClassicMMT@users.noreply.github.com> Date: Fri, 17 Jul 2026 14:33:01 +1200 Subject: [PATCH 2/6] feat: Add support for configurable discovery --- CHANGELOG.md | 9 + README.md | 35 +- src/datamasque_cli/commands/discovery.py | 100 +++++- .../commands/discovery_config_libraries.py | 217 ++++++++++++ .../commands/discovery_configs.py | 213 +++++++++++ tests/commands/test_catalog.py | 6 + tests/commands/test_discovery.py | 90 +++++ .../test_discovery_config_libraries.py | 119 +++++++ tests/commands/test_discovery_configs.py | 197 +++++++++++ tests/integration/conftest.py | 90 +++++ tests/integration/test_discovery.py | 333 ++++++++++++++++++ 11 files changed, 1400 insertions(+), 9 deletions(-) create mode 100644 src/datamasque_cli/commands/discovery_config_libraries.py create mode 100644 src/datamasque_cli/commands/discovery_configs.py create mode 100644 tests/commands/test_discovery_config_libraries.py create mode 100644 tests/commands/test_discovery_configs.py create mode 100644 tests/integration/test_discovery.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e7602d..67ed5c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,15 @@ on invalid rulesets/libraries. - `dm discover db-report` writes a zip archive returned for large reports to `--output`, aborting with a hint rather than dumping binary data to stdout. +- Support for Configurable Discovery: + - `dm discover configs` — list, get, defaults, create, delete, and validate + discovery configs (`database` or `file`). + - `dm discover libraries` — list, get, create, delete, and validate discovery + config libraries. + - `dm discover schema --config ` and `dm discover file + [--config ]` start discovery runs with or without a specific config. + - `dm discover config-snapshot ` downloads the discovery config a run + actually used. ## v1.4.0 diff --git a/README.md b/README.md index 07fe69f..bc9c940 100644 --- a/README.md +++ b/README.md @@ -216,11 +216,36 @@ dm users delete # Delete a user ### Discovery ```console -dm discover schema # Start a schema-discovery run -dm discover schema-results # List schema-discovery results once the run finishes -dm discover sdd-report # Sensitive data discovery report -dm discover db-report # Database discovery CSV -dm discover file-report # File discovery report +dm discover schema # Schema discovery (built-in keyword-driven) +dm discover schema --config # Schema discovery from a saved database config +dm discover schema-results # List schema-discovery results once the run finishes +dm discover file # File data discovery (built-in keyword-driven) +dm discover file --config # File data discovery from a saved file config +dm discover sdd-report # Sensitive data discovery report +dm discover db-report # Database discovery CSV +dm discover file-report # File discovery report +dm discover config-snapshot -o used.yaml # Download the discovery config a run actually used +``` + +#### Discovery configs + +```console +dm discover configs list [--type database|file] # List configs +dm discover configs get [--type database] [--yaml] # Show details or raw YAML +dm discover configs defaults [--type database|file] -o cfg.yaml # Built-in default as a starting point +dm discover configs create --name --type database -f cfg.yaml # Create/update from YAML +dm discover configs delete [--type database] # Delete a config +dm discover configs validate -f cfg.yaml --type database # Validate a YAML file against the server +``` + +#### Discovery config libraries + +```console +dm discover libraries list [--type database|file] +dm discover libraries get [--type database] [--namespace org] [--yaml] +dm discover libraries create --name --type database --namespace org -f lib.yaml +dm discover libraries delete [--type database] [--namespace org] [--force] # --force if imported by configs +dm discover libraries validate -f lib.yaml --type database ``` ### Seeds diff --git a/src/datamasque_cli/commands/discovery.py b/src/datamasque_cli/commands/discovery.py index c38e10a..6daf571 100644 --- a/src/datamasque_cli/commands/discovery.py +++ b/src/datamasque_cli/commands/discovery.py @@ -8,12 +8,21 @@ import typer from datamasque.client import DataMasqueClient, RunId from datamasque.client.models.connection import ConnectionId -from datamasque.client.models.discovery import SchemaDiscoveryRequest +from datamasque.client.models.discovery import ( + FileDataDiscoveryFromConfigRequest, + FileDataDiscoveryRequest, + SchemaDiscoveryFromConfigRequest, + SchemaDiscoveryRequest, +) +from datamasque.client.models.discovery_config import DiscoveryConfigId, DiscoveryConfigType from datamasque_cli.client import get_client +from datamasque_cli.commands import discovery_config_libraries, discovery_configs from datamasque_cli.output import ErrorCode, abort, print_json, print_success, render_output, should_emit_json app = typer.Typer(help="Data discovery operations.", no_args_is_help=True) +app.add_typer(discovery_configs.app, name="configs") +app.add_typer(discovery_config_libraries.app, name="libraries") def _write_or_echo(content: str, output: Path | None, success_label: str) -> None: @@ -33,9 +42,40 @@ def _resolve_connection_id(client: DataMasqueClient, name_or_id: str) -> str: return str(match.id) +def _resolve_discovery_config_id( + client: DataMasqueClient, name: str, expected_type: DiscoveryConfigType +) -> DiscoveryConfigId: + """Resolve a discovery config name to its UUID, requiring it to be of `expected_type`.""" + named = [c for c in client.list_discovery_configs() if c.name == name] + matches = [c for c in named if c.config_type is expected_type] + + if not matches: + if named: + existing = ", ".join(c.config_type.value for c in named) + abort( + f"Discovery config '{name}' exists as {existing}, " + f"but {expected_type.value} discovery needs a {expected_type.value} config.", + code=ErrorCode.INVALID_INPUT, + ) + abort(f"Discovery config '{name}' not found.", code=ErrorCode.NOT_FOUND) + if len(matches) > 1: + options = "\n ".join(f"id={c.id}" for c in matches) + abort( + f"Multiple {expected_type.value} discovery configs named '{name}':\n {options}", + code=ErrorCode.AMBIGUOUS, + ) + + config_id = matches[0].id + assert config_id is not None + return config_id + + @app.command("schema") def schema_discovery( connection: str = typer.Argument(help="Connection name or ID"), + config: str | None = typer.Option( + None, "--config", "-c", help="Run with a saved database discovery config (configurable discovery)" + ), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), ) -> None: """Start a schema-discovery run on a connection. @@ -47,14 +87,54 @@ def schema_discovery( client = get_client(profile) conn_id = _resolve_connection_id(client, connection) - request = SchemaDiscoveryRequest(connection=ConnectionId(conn_id)) - run_id = client.start_schema_discovery_run(request) + if config is not None: + config_id = _resolve_discovery_config_id(client, config, DiscoveryConfigType.database) + from_config = SchemaDiscoveryFromConfigRequest(connection=ConnectionId(conn_id), discovery_config=config_id) + run_id = client.start_schema_discovery_run_from_config(from_config) + source = f"config '{config}'" + else: + request = SchemaDiscoveryRequest(connection=ConnectionId(conn_id)) + run_id = client.start_schema_discovery_run(request) + source = "default discovery" + print_success( - f"Schema discovery run {run_id} started for connection '{connection}'. " + f"Schema discovery run {run_id} started for connection '{connection}' ({source}). " f"Once finished, list results with: dm discover schema-results {run_id}" ) +@app.command("file") +def file_discovery( + connection: str = typer.Argument(help="Connection name or ID"), + config: str | None = typer.Option( + None, "--config", "-c", help="Run with a saved file discovery config (configurable discovery)" + ), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), +) -> None: + """Start a file-data-discovery run on a file connection. + + Once finished, download the report with `dm discover file-report ` + (poll with `dm run status `). + """ + client = get_client(profile) + conn_id = _resolve_connection_id(client, connection) + + if config is not None: + config_id = _resolve_discovery_config_id(client, config, DiscoveryConfigType.file) + from_config = FileDataDiscoveryFromConfigRequest(connection=ConnectionId(conn_id), discovery_config=config_id) + run_id = client.start_file_data_discovery_run_from_config(from_config) + source = f"config '{config}'" + else: + request = FileDataDiscoveryRequest(connection=ConnectionId(conn_id)) + run_id = client.start_file_data_discovery_run(request) + source = "default discovery" + + print_success( + f"File data discovery run {run_id} started for connection '{connection}' ({source}). " + f"Once finished, download the report with: dm discover file-report {run_id}" + ) + + @app.command("schema-results") def schema_results( run_id: int = typer.Argument(help="Schema discovery run ID"), @@ -152,3 +232,15 @@ def file_discovery_report( print_json(report) else: render_output(report, is_json=False, title=f"File Discovery: Run {run_id}") + + +@app.command("config-snapshot") +def config_snapshot( + run_id: int = typer.Argument(help="Discovery run ID"), + output: Path | None = typer.Option(None, "--output", "-o", help="Write YAML to this path"), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), +) -> None: + """Download the discovery config a run used (the run's snapshot).""" + client = get_client(profile) + snapshot = client.get_discovery_run_config_snapshot_yaml(RunId(run_id)) + _write_or_echo(snapshot, output, "Discovery config snapshot") diff --git a/src/datamasque_cli/commands/discovery_config_libraries.py b/src/datamasque_cli/commands/discovery_config_libraries.py new file mode 100644 index 0000000..d375f65 --- /dev/null +++ b/src/datamasque_cli/commands/discovery_config_libraries.py @@ -0,0 +1,217 @@ +"""Discovery config library management commands (configurable discovery).""" + +from __future__ import annotations + +from pathlib import Path + +import typer +from datamasque.client import DataMasqueClient +from datamasque.client.models.discovery_config import DiscoveryConfigType +from datamasque.client.models.discovery_config_library import DiscoveryConfigLibrary +from datamasque.client.models.status import ValidationStatus + +from datamasque_cli.client import get_client +from datamasque_cli.output import ErrorCode, abort, print_info, print_success, render_output + +app = typer.Typer(help="Manage discovery config libraries (configurable discovery).", no_args_is_help=True) + + +def _label(name: str, namespace: str) -> str: + """Render a library's display label as `namespace/name`, or bare `name` in the default namespace.""" + return f"{namespace}/{name}" if namespace else name + + +def _find_by_name( + client: DataMasqueClient, + name: str, + config_type: DiscoveryConfigType | None = None, + namespace: str | None = None, +) -> list[DiscoveryConfigLibrary]: + """Return all libraries matching `name`, optionally narrowed by `namespace` and `config_type`.""" + matches = [lib for lib in client.list_discovery_config_libraries() if lib.name == name] + if namespace is not None: + matches = [lib for lib in matches if lib.namespace == namespace] + if config_type is not None: + matches = [lib for lib in matches if lib.config_type is config_type] + return matches + + +def _pick_single(matches: list[DiscoveryConfigLibrary], name: str) -> DiscoveryConfigLibrary: + """Return the sole match or abort with a disambiguation message.""" + if not matches: + abort(f"Discovery config library '{name}' not found.", code=ErrorCode.NOT_FOUND) + if len(matches) > 1: + options = "\n ".join( + f"id={lib.id} namespace={lib.namespace or '(default)'} type={lib.config_type.value}" for lib in matches + ) + abort( + f"Multiple discovery config libraries named '{name}':\n {options}", + code=ErrorCode.AMBIGUOUS, + hint="Pass --type file|database and/or --namespace to disambiguate.", + ) + return matches[0] + + +@app.command("list") +def list_libraries( + config_type: str | None = typer.Option(None, "--type", "-t", help="Filter by type: database or file"), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), + is_json: bool = typer.Option(False, "--json", help="Output as JSON"), +) -> None: + """List all discovery config libraries.""" + client = get_client(profile) + libraries = client.list_discovery_config_libraries() + + if config_type is not None: + wanted = DiscoveryConfigType(config_type) + libraries = [lib for lib in libraries if lib.config_type is wanted] + + data = [ + { + "id": lib.id, + "namespace": lib.namespace or "", + "name": lib.name, + "type": lib.config_type.value, + "valid": lib.is_valid.value if lib.is_valid else "unknown", + } + for lib in libraries + ] + + render_output( + data, + is_json=is_json, + columns=["id", "namespace", "name", "type", "valid"], + title="Discovery Config Libraries", + ) + + +@app.command("get") +def get_library( + name: str = typer.Argument(help="Library name"), + config_type: str | None = typer.Option(None, "--type", "-t", help="Required when two libraries share a name"), + namespace: str = typer.Option("", "--namespace", "-n", help="Library namespace"), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), + is_yaml: bool = typer.Option(False, "--yaml", help="Output raw YAML content only"), + is_json: bool = typer.Option(False, "--json", help="Output as JSON"), +) -> None: + """Show a discovery config library's details or YAML content.""" + client = get_client(profile) + wanted = DiscoveryConfigType(config_type) if config_type is not None else None + match = _pick_single(_find_by_name(client, name, wanted, namespace), name) + + # `list_discovery_config_libraries` omits the YAML body; fetch the single library for it. + assert match.id is not None + full = client.get_discovery_config_library(match.id) + + if is_yaml: + typer.echo(full.yaml) + return + + data: dict[str, object] = { + "id": full.id, + "namespace": full.namespace, + "name": full.name, + "type": full.config_type.value, + "valid": full.is_valid.value if full.is_valid else "unknown", + "created": full.created, + "modified": full.modified, + } + render_output(data, is_json=is_json, title=f"Discovery Config Library: {full.name}") + + +@app.command("create") +def create_library( + name: str = typer.Option(..., help="Library name"), + file: Path = typer.Option(..., "--file", "-f", help="Path to YAML library file", exists=True, readable=True), + config_type: str | None = typer.Option( + None, + "--type", + "-t", + help=( + "Config type: database or file. " + "Required when the library does not yet exist; defaults to the existing type on updates." + ), + ), + namespace: str = typer.Option("", "--namespace", "-n", help="Library namespace"), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), +) -> None: + """Create or update a discovery config library from a YAML file.""" + client = get_client(profile) + existing = _find_by_name(client, name, namespace=namespace) + explicit = DiscoveryConfigType(config_type) if config_type is not None else None + + if explicit is not None: + lib_type = explicit + elif len(existing) == 1: + lib_type = existing[0].config_type + print_info(f"Updating existing {lib_type.value}-type library '{_label(name, namespace)}'.") + elif not existing: + abort( + f"No discovery config library named '{_label(name, namespace)}' exists.", + code=ErrorCode.NOT_FOUND, + hint="Pass --type file|database to create a new one.", + ) + else: + options = ", ".join(lib.config_type.value for lib in existing) + abort( + f"Multiple libraries named '{_label(name, namespace)}' ({options}).", + code=ErrorCode.AMBIGUOUS, + hint="Pass --type file|database to pick which one to update.", + ) + + yaml_content = file.read_text() + library = DiscoveryConfigLibrary(name=name, namespace=namespace, yaml=yaml_content, config_type=lib_type) + client.create_or_update_discovery_config_library(library) + print_success(f"Discovery config library '{_label(name, namespace)}' ({lib_type.value}) created/updated.") + + +@app.command("delete") +def delete_library( + name: str = typer.Argument(help="Library name to delete"), + config_type: str | None = typer.Option(None, "--type", "-t", help="Required when two libraries share a name"), + namespace: str = typer.Option("", "--namespace", "-n", help="Library namespace"), + force: bool = typer.Option(False, "--force", help="Force delete even if imported by discovery configs"), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), + is_confirmed: bool = typer.Option(False, "--yes", "-y", help="Skip confirmation"), +) -> None: + """Delete a discovery config library by name. + + If the library is imported by any discovery configs, + the server rejects the delete unless --force is passed. + """ + client = get_client(profile) + wanted = DiscoveryConfigType(config_type) if config_type is not None else None + match = _pick_single(_find_by_name(client, name, wanted, namespace), name) + label = _label(name, namespace) + + if not is_confirmed: + typer.confirm(f"Delete discovery config library '{label}' ({match.config_type.value})?", abort=True) + + assert match.id is not None + client.delete_discovery_config_library_by_id_if_exists(match.id, force=force) + print_success(f"Discovery config library '{label}' deleted.") + + +@app.command("validate") +def validate_library( + file: Path = typer.Option(..., "--file", "-f", help="Path to YAML library file", exists=True, readable=True), + config_type: str = typer.Option(..., "--type", "-t", help="Config type: database or file"), + namespace: str = typer.Option("", "--namespace", "-n", help="Library namespace"), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), +) -> None: + """Validate a discovery config library YAML file against the DataMasque server.""" + yaml_content = file.read_text() + lib_type = DiscoveryConfigType(config_type) + + client = get_client(profile) + library = DiscoveryConfigLibrary(name=file.stem, namespace=namespace, yaml=yaml_content, config_type=lib_type) + validated = client.validate_discovery_config_library(library) + + if validated.is_valid is ValidationStatus.invalid: + abort( + f'Discovery config library "{file.name}" is invalid: {validated.validation_error}', + code=ErrorCode.INVALID_INPUT, + ) + + status = validated.is_valid.value if validated.is_valid else "unknown" + print_success(f'Discovery config library "{file.name}" validation status: {status}') diff --git a/src/datamasque_cli/commands/discovery_configs.py b/src/datamasque_cli/commands/discovery_configs.py new file mode 100644 index 0000000..1f26939 --- /dev/null +++ b/src/datamasque_cli/commands/discovery_configs.py @@ -0,0 +1,213 @@ +"""Discovery config management commands (configurable discovery).""" + +from __future__ import annotations + +from pathlib import Path + +import typer +from datamasque.client import DataMasqueClient +from datamasque.client.models.discovery_config import DiscoveryConfig, DiscoveryConfigType +from datamasque.client.models.status import ValidationStatus + +from datamasque_cli.client import get_client +from datamasque_cli.output import ErrorCode, abort, print_info, print_success, render_output + +app = typer.Typer(help="Manage discovery configs (configurable discovery).", no_args_is_help=True) + + +def _find_by_name( + client: DataMasqueClient, + name: str, + config_type: DiscoveryConfigType | None = None, +) -> list[DiscoveryConfig]: + """Return all discovery configs matching `name`, optionally narrowed by `config_type`.""" + matches = [c for c in client.list_discovery_configs() if c.name == name] + if config_type is not None: + matches = [c for c in matches if c.config_type is config_type] + return matches + + +def _pick_single(matches: list[DiscoveryConfig], name: str) -> DiscoveryConfig: + """Return the sole match or abort with a disambiguation message.""" + if not matches: + abort(f"Discovery config '{name}' not found.", code=ErrorCode.NOT_FOUND) + if len(matches) > 1: + options = "\n ".join(f"id={c.id} type={c.config_type.value}" for c in matches) + abort( + f"Multiple discovery configs named '{name}':\n {options}", + code=ErrorCode.AMBIGUOUS, + hint="Pass --type file|database to disambiguate.", + ) + return matches[0] + + +@app.command("list") +def list_configs( + config_type: str | None = typer.Option(None, "--type", "-t", help="Filter by type: database or file"), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), + is_json: bool = typer.Option(False, "--json", help="Output as JSON"), +) -> None: + """List all discovery configs.""" + client = get_client(profile) + configs = client.list_discovery_configs() + + if config_type is not None: + wanted = DiscoveryConfigType(config_type) + configs = [c for c in configs if c.config_type is wanted] + + data = [ + { + "id": c.id, + "name": c.name, + "type": c.config_type.value, + "valid": c.is_valid.value if c.is_valid else "unknown", + } + for c in configs + ] + + render_output(data, is_json=is_json, columns=["id", "name", "type", "valid"], title="Discovery Configs") + + +@app.command("get") +def get_config( + name: str = typer.Argument(help="Discovery config name"), + config_type: str | None = typer.Option(None, "--type", "-t", help="Required when two configs share a name"), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), + is_yaml: bool = typer.Option(False, "--yaml", help="Output raw YAML content only"), + is_json: bool = typer.Option(False, "--json", help="Output as JSON"), +) -> None: + """Show a discovery config's details or YAML content.""" + client = get_client(profile) + wanted = DiscoveryConfigType(config_type) if config_type is not None else None + match = _pick_single(_find_by_name(client, name, wanted), name) + + assert match.id is not None + full = client.get_discovery_config(match.id) + + if is_yaml: + typer.echo(full.yaml) + return + + data: dict[str, object] = { + "id": full.id, + "name": full.name, + "type": full.config_type.value, + "valid": full.is_valid.value if full.is_valid else "unknown", + "created": full.created, + "modified": full.modified, + } + render_output(data, is_json=is_json, title=f"Discovery Config: {full.name}") + + +@app.command("defaults") +def config_defaults( + config_type: str = typer.Option("database", "--type", "-t", help="Config type: database or file"), + output: Path | None = typer.Option(None, "--output", "-o", help="Write YAML to this path"), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), +) -> None: + """Print the server's built-in default discovery config as YAML.""" + client = get_client(profile) + wanted = DiscoveryConfigType(config_type) + # `get_default_discovery_config_yaml` takes no config type, so call `make_request` to pass one. + response = client.make_request("GET", "/api/discovery/configs/defaults/", params={"config_type": wanted.value}) + yaml_content = response.content.decode("utf-8") + + if output is not None: + output.write_text(yaml_content) + print_success(f"Default {wanted.value} discovery config written to {output}") + return + + typer.echo(yaml_content) + + +@app.command("create") +def create_config( + name: str = typer.Option(..., help="Discovery config name"), + file: Path = typer.Option(..., "--file", "-f", help="Path to YAML config file", exists=True, readable=True), + config_type: str | None = typer.Option( + None, + "--type", + "-t", + help=( + "Config type: database or file. " + "Required when the config does not yet exist; defaults to the existing type on updates." + ), + ), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), +) -> None: + """Create or update a discovery config from a YAML file. + + A brand-new config needs --type because there is no stored row to copy the + type from; an update defaults to whatever the existing row is stored as. + """ + client = get_client(profile) + existing = _find_by_name(client, name) + explicit = DiscoveryConfigType(config_type) if config_type is not None else None + + if explicit is not None: + cfg_type = explicit + elif len(existing) == 1: + cfg_type = existing[0].config_type + print_info(f"Updating existing {cfg_type.value}-type discovery config '{name}'.") + elif not existing: + abort( + f"No discovery config named '{name}' exists.", + code=ErrorCode.NOT_FOUND, + hint="Pass --type file|database to create a new one.", + ) + else: + options = ", ".join(c.config_type.value for c in existing) + abort( + f"Multiple discovery configs named '{name}' ({options}).", + code=ErrorCode.AMBIGUOUS, + hint="Pass --type file|database to pick which one to update.", + ) + + yaml_content = file.read_text() + config = DiscoveryConfig(name=name, yaml=yaml_content, config_type=cfg_type) + client.create_or_update_discovery_config(config) + print_success(f"Discovery config '{name}' ({cfg_type.value}) created/updated.") + + +@app.command("delete") +def delete_config( + name: str = typer.Argument(help="Discovery config name to delete"), + config_type: str | None = typer.Option(None, "--type", "-t", help="Required when two configs share a name"), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), + is_confirmed: bool = typer.Option(False, "--yes", "-y", help="Skip confirmation"), +) -> None: + """Delete a discovery config by name.""" + client = get_client(profile) + wanted = DiscoveryConfigType(config_type) if config_type is not None else None + match = _pick_single(_find_by_name(client, name, wanted), name) + + if not is_confirmed: + typer.confirm(f"Delete discovery config '{name}' ({match.config_type.value})?", abort=True) + + assert match.id is not None + client.delete_discovery_config_by_id_if_exists(match.id) + print_success(f"Discovery config '{name}' ({match.config_type.value}) deleted.") + + +@app.command("validate") +def validate_config( + file: Path = typer.Option(..., "--file", "-f", help="Path to YAML config file", exists=True, readable=True), + config_type: str = typer.Option(..., "--type", "-t", help="Config type: database or file"), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), +) -> None: + """Validate a discovery config YAML file against the DataMasque server.""" + yaml_content = file.read_text() + cfg_type = DiscoveryConfigType(config_type) + + client = get_client(profile) + config = DiscoveryConfig(name=file.stem, yaml=yaml_content, config_type=cfg_type) + validated = client.validate_discovery_config(config) + + if validated.is_valid is ValidationStatus.invalid: + abort( + f'Discovery config "{file.name}" is invalid: {validated.validation_error}', + code=ErrorCode.INVALID_INPUT, + ) + + status = validated.is_valid.value if validated.is_valid else "unknown" + print_success(f'Discovery config "{file.name}" validation status: {status}') diff --git a/tests/commands/test_catalog.py b/tests/commands/test_catalog.py index 3083057..3ae1e37 100644 --- a/tests/commands/test_catalog.py +++ b/tests/commands/test_catalog.py @@ -22,6 +22,12 @@ def test_catalog_compact_json_lists_every_subcommand(monkeypatch: pytest.MonkeyP assert "run start" in paths assert "auth login" in paths + # Nested discovery-config groups surface as `discover ` paths. + assert "discover schema" in paths + assert "discover configs list" in paths + assert "discover libraries create" in paths + assert "discover config-snapshot" in paths + def test_catalog_full_includes_options(monkeypatch: pytest.MonkeyPatch, runner: CliRunner) -> None: monkeypatch.setenv("DM_OUTPUT", "json") diff --git a/tests/commands/test_discovery.py b/tests/commands/test_discovery.py index d68fa07..68fb69f 100644 --- a/tests/commands/test_discovery.py +++ b/tests/commands/test_discovery.py @@ -5,6 +5,7 @@ from types import SimpleNamespace from unittest.mock import MagicMock, patch +from datamasque.client.models.discovery_config import DiscoveryConfigType from typer.testing import CliRunner from datamasque_cli.main import app @@ -191,3 +192,92 @@ def test_schema_results_skips_unlabelled_matches(mock_get_client: MagicMock, run rows = json.loads(result.stdout) assert rows[0]["matches"] == "EMAIL_ADDRESS" assert rows[1]["matches"] == "-" + + +# -- configurable-discovery run triggers ---------------------------------- + + +@patch(f"{MODULE}.get_client") +def test_schema_with_config_runs_from_saved_config(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_connections.return_value = [SimpleNamespace(id="abc-123", name="my_db", mask_type="database")] + client.list_discovery_configs.return_value = [ + SimpleNamespace(id="cfg-1", name="emp", config_type=DiscoveryConfigType.database), + ] + client.start_schema_discovery_run_from_config.return_value = 77 + + result = runner.invoke(app, ["discover", "schema", "my_db", "--config", "emp"]) + + assert result.exit_code == 0 + client.start_schema_discovery_run.assert_not_called() + (call,) = client.start_schema_discovery_run_from_config.call_args_list + (request,) = call.args + assert request.connection == "abc-123" + assert request.discovery_config == "cfg-1" + assert "dm discover schema-results 77" in result.stderr + + +@patch(f"{MODULE}.get_client") +def test_schema_config_wrong_type_aborts(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_connections.return_value = [SimpleNamespace(id="abc-123", name="my_db", mask_type="database")] + client.list_discovery_configs.return_value = [ + SimpleNamespace(id="cfg-2", name="docs", config_type=DiscoveryConfigType.file), + ] + + result = runner.invoke(app, ["discover", "schema", "my_db", "--config", "docs"]) + + assert result.exit_code == 4 # invalid_input + client.start_schema_discovery_run_from_config.assert_not_called() + + +@patch(f"{MODULE}.get_client") +def test_file_without_config_runs_keyword_discovery(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_connections.return_value = [SimpleNamespace(id="fs-1", name="my_files", mask_type="file")] + client.start_file_data_discovery_run.return_value = 88 + + result = runner.invoke(app, ["discover", "file", "my_files"]) + + assert result.exit_code == 0 + client.start_file_data_discovery_run_from_config.assert_not_called() + (call,) = client.start_file_data_discovery_run.call_args_list + (request,) = call.args + assert request.connection == "fs-1" + assert "dm discover file-report 88" in result.stderr + + +@patch(f"{MODULE}.get_client") +def test_file_with_config_runs_from_saved_config(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_connections.return_value = [SimpleNamespace(id="fs-1", name="my_files", mask_type="file")] + client.list_discovery_configs.return_value = [ + SimpleNamespace(id="cfg-3", name="docs", config_type=DiscoveryConfigType.file), + ] + client.start_file_data_discovery_run_from_config.return_value = 89 + + result = runner.invoke(app, ["discover", "file", "my_files", "--config", "docs"]) + + assert result.exit_code == 0 + client.start_file_data_discovery_run.assert_not_called() + (call,) = client.start_file_data_discovery_run_from_config.call_args_list + (request,) = call.args + assert request.discovery_config == "cfg-3" + + +@patch(f"{MODULE}.get_client") +def test_config_snapshot_writes_to_output(mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.get_discovery_run_config_snapshot_yaml.return_value = "# provenance\nlabels: []\n" + + out = tmp_path / "used.yaml" + result = runner.invoke(app, ["discover", "config-snapshot", "42", "--output", str(out)]) + + assert result.exit_code == 0 + assert out.read_text() == "# provenance\nlabels: []\n" + client.get_discovery_run_config_snapshot_yaml.assert_called_once_with(42) diff --git a/tests/commands/test_discovery_config_libraries.py b/tests/commands/test_discovery_config_libraries.py new file mode 100644 index 0000000..ceb2dc7 --- /dev/null +++ b/tests/commands/test_discovery_config_libraries.py @@ -0,0 +1,119 @@ +from __future__ import annotations + +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from datamasque.client.models.discovery_config import DiscoveryConfigType +from datamasque.client.models.status import ValidationStatus +from typer.testing import CliRunner + +from datamasque_cli.main import app + +MODULE = "datamasque_cli.commands.discovery_config_libraries" + + +def _library( + name: str, + config_type: DiscoveryConfigType = DiscoveryConfigType.database, + namespace: str = "", + library_id: str = "lib-uuid", + is_valid: ValidationStatus | None = ValidationStatus.valid, + yaml: str | None = None, +) -> SimpleNamespace: + return SimpleNamespace( + id=library_id, + name=name, + namespace=namespace, + config_type=config_type, + is_valid=is_valid, + validation_error=None, + created=None, + modified=None, + yaml=yaml, + ) + + +@patch(f"{MODULE}.get_client") +def test_list_shows_namespace_and_type(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_config_libraries.return_value = [ + _library("finance", namespace="org"), + ] + + result = runner.invoke(app, ["discover", "libraries", "list", "--json"]) + + assert result.exit_code == 0 + assert '"finance"' in result.stdout + assert '"org"' in result.stdout + + +@patch(f"{MODULE}.get_client") +def test_get_yaml_fetches_full_library(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_config_libraries.return_value = [_library("finance", namespace="org")] + client.get_discovery_config_library.return_value = _library("finance", namespace="org", yaml="labels: []\n") + + result = runner.invoke(app, ["discover", "libraries", "get", "finance", "--namespace", "org", "--yaml"]) + + assert result.exit_code == 0 + assert "labels: []" in result.stdout + client.get_discovery_config_library.assert_called_once_with("lib-uuid") + + +@patch(f"{MODULE}.get_client") +def test_get_namespace_scopes_lookup(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_config_libraries.return_value = [_library("finance", namespace="org")] + + result = runner.invoke(app, ["discover", "libraries", "get", "finance"]) + + assert result.exit_code == 3 + client.get_discovery_config_library.assert_not_called() + + +@patch(f"{MODULE}.get_client") +def test_create_new_requires_type(mock_get_client: MagicMock, runner: CliRunner, tmp_path) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_config_libraries.return_value = [] + lib = tmp_path / "lib.yaml" + lib.write_text("labels: []\n") + + result = runner.invoke( + app, + ["discover", "libraries", "create", "--name", "finance", "-n", "org", "-f", str(lib), "--type", "database"], + ) + + assert result.exit_code == 0 + client.create_or_update_discovery_config_library.assert_called_once() + + +@patch(f"{MODULE}.get_client") +def test_delete_force_passes_through(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_config_libraries.return_value = [_library("finance", namespace="org")] + + result = runner.invoke(app, ["discover", "libraries", "delete", "finance", "-n", "org", "--force", "--yes"]) + + assert result.exit_code == 0 + client.delete_discovery_config_library_by_id_if_exists.assert_called_once_with("lib-uuid", force=True) + + +@patch(f"{MODULE}.get_client") +def test_validate_invalid_exits_4(mock_get_client: MagicMock, runner: CliRunner, tmp_path) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.validate_discovery_config_library.return_value = SimpleNamespace( + is_valid=ValidationStatus.invalid, validation_error="duplicate label 'email'" + ) + lib = tmp_path / "lib.yaml" + lib.write_text("labels: []\n") + + result = runner.invoke(app, ["discover", "libraries", "validate", "-f", str(lib), "--type", "database"]) + + assert result.exit_code == 4 + assert "duplicate label 'email'" in result.stderr diff --git a/tests/commands/test_discovery_configs.py b/tests/commands/test_discovery_configs.py new file mode 100644 index 0000000..509c98e --- /dev/null +++ b/tests/commands/test_discovery_configs.py @@ -0,0 +1,197 @@ +from __future__ import annotations + +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from datamasque.client.models.discovery_config import DiscoveryConfigType +from datamasque.client.models.status import ValidationStatus +from typer.testing import CliRunner + +from datamasque_cli.main import app + +MODULE = "datamasque_cli.commands.discovery_configs" + + +def _config( + name: str, + config_type: DiscoveryConfigType = DiscoveryConfigType.database, + config_id: str = "cfg-uuid", + is_valid: ValidationStatus | None = ValidationStatus.valid, + yaml: str | None = None, +) -> SimpleNamespace: + return SimpleNamespace( + id=config_id, + name=name, + config_type=config_type, + is_valid=is_valid, + validation_error=None, + created=None, + modified=None, + yaml=yaml, + ) + + +@patch(f"{MODULE}.get_client") +def test_list_filters_by_type(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_configs.return_value = [ + _config("emp", DiscoveryConfigType.database), + _config("docs", DiscoveryConfigType.file), + ] + + result = runner.invoke(app, ["discover", "configs", "list", "--type", "file"]) + + assert result.exit_code == 0 + assert "docs" in result.stdout + assert "emp" not in result.stdout + + +@patch(f"{MODULE}.get_client") +def test_get_yaml_fetches_full_config(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_configs.return_value = [_config("emp")] + client.get_discovery_config.return_value = _config("emp", yaml="labels: []\n") + + result = runner.invoke(app, ["discover", "configs", "get", "emp", "--yaml"]) + + assert result.exit_code == 0 + assert "labels: []" in result.stdout + client.get_discovery_config.assert_called_once_with("cfg-uuid") + + +@patch(f"{MODULE}.get_client") +def test_get_ambiguous_name_aborts(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_configs.return_value = [ + _config("shared", DiscoveryConfigType.database, config_id="a"), + _config("shared", DiscoveryConfigType.file, config_id="b"), + ] + + result = runner.invoke(app, ["discover", "configs", "get", "shared"]) + + assert result.exit_code == 5 + client.get_discovery_config.assert_not_called() + + +@patch(f"{MODULE}.get_client") +def test_get_ambiguous_resolved_by_type(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_configs.return_value = [ + _config("shared", DiscoveryConfigType.database, config_id="a"), + _config("shared", DiscoveryConfigType.file, config_id="b"), + ] + client.get_discovery_config.return_value = _config("shared", DiscoveryConfigType.file, config_id="b") + + result = runner.invoke(app, ["discover", "configs", "get", "shared", "--type", "file"]) + + assert result.exit_code == 0 + client.get_discovery_config.assert_called_once_with("b") + + +@patch(f"{MODULE}.get_client") +def test_defaults_requests_typed_default(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.make_request.return_value = SimpleNamespace(content=b"labels: []\n") + + result = runner.invoke(app, ["discover", "configs", "defaults", "--type", "file"]) + + assert result.exit_code == 0 + assert "labels: []" in result.stdout + client.make_request.assert_called_once_with( + "GET", "/api/discovery/configs/defaults/", params={"config_type": "file"} + ) + + +@patch(f"{MODULE}.get_client") +def test_create_new_requires_type(mock_get_client: MagicMock, runner: CliRunner, tmp_path) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_configs.return_value = [] + cfg = tmp_path / "cfg.yaml" + cfg.write_text("labels: []\n") + + missing_type = runner.invoke(app, ["discover", "configs", "create", "--name", "emp", "-f", str(cfg)]) + assert missing_type.exit_code == 3 + client.create_or_update_discovery_config.assert_not_called() + + with_type = runner.invoke( + app, ["discover", "configs", "create", "--name", "emp", "-f", str(cfg), "--type", "database"] + ) + assert with_type.exit_code == 0 + client.create_or_update_discovery_config.assert_called_once() + + +@patch(f"{MODULE}.get_client") +def test_create_update_defaults_to_existing_type(mock_get_client: MagicMock, runner: CliRunner, tmp_path) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_configs.return_value = [_config("emp", DiscoveryConfigType.database)] + cfg = tmp_path / "cfg.yaml" + cfg.write_text("labels: []\n") + + result = runner.invoke(app, ["discover", "configs", "create", "--name", "emp", "-f", str(cfg)]) + + assert result.exit_code == 0 + client.create_or_update_discovery_config.assert_called_once() + + +@patch(f"{MODULE}.get_client") +def test_delete_proceeds_when_present(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_configs.return_value = [_config("emp")] + + result = runner.invoke(app, ["discover", "configs", "delete", "emp", "--yes"]) + + assert result.exit_code == 0 + client.delete_discovery_config_by_id_if_exists.assert_called_once_with("cfg-uuid") + + +@patch(f"{MODULE}.get_client") +def test_delete_aborts_when_missing(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_configs.return_value = [] + + result = runner.invoke(app, ["discover", "configs", "delete", "nope", "--yes"]) + + assert result.exit_code == 3 + client.delete_discovery_config_by_id_if_exists.assert_not_called() + + +@patch(f"{MODULE}.get_client") +def test_validate_reports_valid(mock_get_client: MagicMock, runner: CliRunner, tmp_path) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.validate_discovery_config.return_value = SimpleNamespace( + is_valid=ValidationStatus.valid, validation_error=None + ) + cfg = tmp_path / "cfg.yaml" + cfg.write_text("labels: []\n") + + result = runner.invoke(app, ["discover", "configs", "validate", "-f", str(cfg), "--type", "database"]) + + assert result.exit_code == 0 + assert "valid" in result.stderr + client.validate_discovery_config.assert_called_once() + + +@patch(f"{MODULE}.get_client") +def test_validate_invalid_exits_4(mock_get_client: MagicMock, runner: CliRunner, tmp_path) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.validate_discovery_config.return_value = SimpleNamespace( + is_valid=ValidationStatus.invalid, validation_error="unknown label 'foo'" + ) + cfg = tmp_path / "cfg.yaml" + cfg.write_text("labels: []\n") + + result = runner.invoke(app, ["discover", "configs", "validate", "-f", str(cfg), "--type", "database"]) + + assert result.exit_code == 4 + assert "unknown label 'foo'" in result.stderr diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 024f1bd..7ed9c71 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -165,3 +165,93 @@ def db_yaml(tmp_path: Path) -> Path: " value: redacted@example.com\n" ) return path + + +DISCOVERY_TEST_NAMESPACE = "dm_int_ns" + + +@pytest.fixture() +def discovery_config_name(runner: CliRunner) -> Iterator[str]: + name = f"dm_int_{uuid.uuid4().hex[:8]}" + yield name + for config_type in ("file", "database"): + runner.invoke(app, ["discover", "configs", "delete", name, "--type", config_type, "--yes"]) + + +@pytest.fixture() +def discovery_library_name(runner: CliRunner) -> Iterator[str]: + name = f"dm_int_{uuid.uuid4().hex[:8]}" + yield name + for namespace in ("", DISCOVERY_TEST_NAMESPACE): + for config_type in ("file", "database"): + args = ["discover", "libraries", "delete", name, "--type", config_type, "--yes", "--force"] + if namespace: + args += ["--namespace", namespace] + runner.invoke(app, args) + + +@pytest.fixture() +def db_discovery_config(runner: CliRunner, tmp_path: Path) -> Path: + """The server's built-in database discovery config.""" + path = tmp_path / "db_config.yaml" + result = runner.invoke(app, ["discover", "configs", "defaults", "--type", "database", "-o", str(path)]) + if result.exit_code != 0 or not path.exists(): + pytest.skip("Could not fetch the default database discovery config from the instance") + return path + + +@pytest.fixture() +def file_discovery_config(runner: CliRunner, tmp_path: Path) -> Path: + """The server's built-in file discovery config.""" + path = tmp_path / "file_config.yaml" + result = runner.invoke(app, ["discover", "configs", "defaults", "--type", "file", "-o", str(path)]) + if result.exit_code != 0 or not path.exists(): + pytest.skip("Could not fetch the default file discovery config from the instance") + return path + + +@pytest.fixture() +def discovery_library_yaml(tmp_path: Path) -> Path: + """Minimal valid discovery config library.""" + path = tmp_path / "library.yaml" + path.write_text("labels: []\nmetadata_rules: []\nidd_rules: []\n") + return path + + +@pytest.fixture() +def invalid_discovery_yaml(tmp_path: Path) -> Path: + """YAML the discovery parser rejects.""" + path = tmp_path / "invalid.yaml" + path.write_text("this: is\nnot: a valid discovery config\ngarbage: true\n") + return path + + +@pytest.fixture() +def any_connection(runner: CliRunner) -> str: + """Name of any connection on the instance.""" + result = runner.invoke(app, ["connections", "list", "--json"]) + if result.exit_code != 0: + pytest.skip("Could not list connections") + conns = json.loads(result.stdout) + if not conns: + pytest.skip("No connections on this instance") + return str(conns[0]["name"]) + + +@pytest.fixture() +def database_connection(runner: CliRunner) -> str: + """Name of a database-type source connection.""" + override = os.environ.get("DM_TEST_DB_CONN") + if override: + return override + result = runner.invoke(app, ["connections", "list", "--json"]) + if result.exit_code != 0: + pytest.skip("Could not list connections to find a database source") + conns = json.loads(result.stdout) + match = next( + (c["name"] for c in conns if c["type"] == "Database" and c["role"] in {"source", "source+destination"}), + None, + ) + if not match: + pytest.skip("No database-type source connection on this instance; set DM_TEST_DB_CONN to override") + return str(match) diff --git a/tests/integration/test_discovery.py b/tests/integration/test_discovery.py new file mode 100644 index 0000000..0b13cc3 --- /dev/null +++ b/tests/integration/test_discovery.py @@ -0,0 +1,333 @@ +"""Live-instance tests for configurable discovery.""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +from datamasque_cli.main import app +from tests.integration.conftest import DISCOVERY_TEST_NAMESPACE + +pytestmark = pytest.mark.integration + + +# --- discovery configs ------------------------------------------------------- + + +def test_config_create_get_delete_lifecycle( + runner: CliRunner, + discovery_config_name: str, + db_discovery_config: Path, +) -> None: + create = runner.invoke( + app, + [ + "discover", + "configs", + "create", + "--name", + discovery_config_name, + "--type", + "database", + "-f", + str(db_discovery_config), + ], + ) + assert create.exit_code == 0, create.stdout + + get_yaml = runner.invoke(app, ["discover", "configs", "get", discovery_config_name, "--yaml"]) + assert get_yaml.exit_code == 0 + assert "labels:" in get_yaml.stdout + + listing = runner.invoke(app, ["discover", "configs", "list"]) + assert discovery_config_name in listing.stdout + + delete = runner.invoke(app, ["discover", "configs", "delete", discovery_config_name, "--yes"]) + assert delete.exit_code == 0 + + gone = runner.invoke(app, ["discover", "configs", "get", discovery_config_name]) + assert gone.exit_code == 3 + + +def test_config_validate_accepts_default_config(runner: CliRunner, db_discovery_config: Path) -> None: + result = runner.invoke( + app, ["discover", "configs", "validate", "-f", str(db_discovery_config), "--type", "database"] + ) + assert result.exit_code == 0, result.stdout + + +def test_config_validate_rejects_invalid_yaml(runner: CliRunner, invalid_discovery_yaml: Path) -> None: + result = runner.invoke( + app, ["discover", "configs", "validate", "-f", str(invalid_discovery_yaml), "--type", "database"] + ) + assert result.exit_code == 4 + assert "invalid" in result.stderr.lower() + + +def test_config_same_name_coexists_across_types( + runner: CliRunner, + discovery_config_name: str, + db_discovery_config: Path, + file_discovery_config: Path, +) -> None: + db = runner.invoke( + app, + [ + "discover", + "configs", + "create", + "--name", + discovery_config_name, + "--type", + "database", + "-f", + str(db_discovery_config), + ], + ) + file = runner.invoke( + app, + [ + "discover", + "configs", + "create", + "--name", + discovery_config_name, + "--type", + "file", + "-f", + str(file_discovery_config), + ], + ) + assert db.exit_code == 0, db.stdout + assert file.exit_code == 0, file.stdout + + listing = runner.invoke(app, ["discover", "configs", "list"]) + matches = [line for line in listing.stdout.splitlines() if discovery_config_name in line] + assert len(matches) == 2 + + +def test_config_create_without_type_aborts_when_ambiguous( + runner: CliRunner, + discovery_config_name: str, + db_discovery_config: Path, + file_discovery_config: Path, +) -> None: + runner.invoke( + app, + [ + "discover", + "configs", + "create", + "--name", + discovery_config_name, + "--type", + "database", + "-f", + str(db_discovery_config), + ], + ) + runner.invoke( + app, + [ + "discover", + "configs", + "create", + "--name", + discovery_config_name, + "--type", + "file", + "-f", + str(file_discovery_config), + ], + ) + + result = runner.invoke( + app, ["discover", "configs", "create", "--name", discovery_config_name, "-f", str(db_discovery_config)] + ) + + assert result.exit_code != 0 + assert "Multiple discovery configs" in result.stderr + + +def test_config_get_missing_is_not_found(runner: CliRunner) -> None: + result = runner.invoke(app, ["discover", "configs", "get", "dm_int_does_not_exist"]) + assert result.exit_code == 3 + + +# --- discovery config libraries ---------------------------------------------- + + +def test_library_create_get_delete_lifecycle( + runner: CliRunner, + discovery_library_name: str, + discovery_library_yaml: Path, +) -> None: + create = runner.invoke( + app, + [ + "discover", + "libraries", + "create", + "--name", + discovery_library_name, + "--type", + "database", + "-f", + str(discovery_library_yaml), + ], + ) + assert create.exit_code == 0, create.stdout + + get_yaml = runner.invoke(app, ["discover", "libraries", "get", discovery_library_name, "--yaml"]) + assert get_yaml.exit_code == 0 + + listing = runner.invoke(app, ["discover", "libraries", "list"]) + assert discovery_library_name in listing.stdout + + delete = runner.invoke(app, ["discover", "libraries", "delete", discovery_library_name, "--yes"]) + assert delete.exit_code == 0 + + gone = runner.invoke(app, ["discover", "libraries", "get", discovery_library_name]) + assert gone.exit_code == 3 + + +def test_library_namespace_is_isolated( + runner: CliRunner, + discovery_library_name: str, + discovery_library_yaml: Path, +) -> None: + created = runner.invoke( + app, + [ + "discover", + "libraries", + "create", + "--name", + discovery_library_name, + "--type", + "database", + "--namespace", + DISCOVERY_TEST_NAMESPACE, + "-f", + str(discovery_library_yaml), + ], + ) + assert created.exit_code == 0, created.stdout + + in_namespace = runner.invoke( + app, ["discover", "libraries", "get", discovery_library_name, "--namespace", DISCOVERY_TEST_NAMESPACE] + ) + assert in_namespace.exit_code == 0 + + default_namespace = runner.invoke(app, ["discover", "libraries", "get", discovery_library_name]) + assert default_namespace.exit_code == 3 + + +def test_library_validate_rejects_invalid_yaml(runner: CliRunner, invalid_discovery_yaml: Path) -> None: + result = runner.invoke( + app, ["discover", "libraries", "validate", "-f", str(invalid_discovery_yaml), "--type", "database"] + ) + assert result.exit_code == 4 + + +# --- `--config` resolution guards (abort before any run starts) -------------- + + +def test_schema_config_type_mismatch_aborts( + runner: CliRunner, + any_connection: str, + discovery_config_name: str, + file_discovery_config: Path, +) -> None: + runner.invoke( + app, + [ + "discover", + "configs", + "create", + "--name", + discovery_config_name, + "--type", + "file", + "-f", + str(file_discovery_config), + ], + ) + result = runner.invoke(app, ["discover", "schema", any_connection, "--config", discovery_config_name]) + assert result.exit_code == 4 + assert "database config" in result.stderr + + +def test_file_config_type_mismatch_aborts( + runner: CliRunner, + any_connection: str, + discovery_config_name: str, + db_discovery_config: Path, +) -> None: + runner.invoke( + app, + [ + "discover", + "configs", + "create", + "--name", + discovery_config_name, + "--type", + "database", + "-f", + str(db_discovery_config), + ], + ) + result = runner.invoke(app, ["discover", "file", any_connection, "--config", discovery_config_name]) + assert result.exit_code == 4 + assert "file config" in result.stderr + + +def test_schema_config_not_found_aborts(runner: CliRunner, any_connection: str) -> None: + result = runner.invoke(app, ["discover", "schema", any_connection, "--config", "dm_int_no_such_config"]) + assert result.exit_code == 3 + + +# --- run from config + config snapshot (env-gated) --------------------------- + + +def test_schema_run_from_config_and_snapshot( + runner: CliRunner, + database_connection: str, + discovery_config_name: str, + db_discovery_config: Path, + tmp_path: Path, +) -> None: + create = runner.invoke( + app, + [ + "discover", + "configs", + "create", + "--name", + discovery_config_name, + "--type", + "database", + "-f", + str(db_discovery_config), + ], + ) + assert create.exit_code == 0, create.stdout + + start = runner.invoke(app, ["discover", "schema", database_connection, "--config", discovery_config_name]) + if start.exit_code != 0: + pytest.skip(f"Could not start schema discovery on '{database_connection}': {start.stdout}{start.stderr}") + + output = " ".join(start.stderr.split()) + assert f"config '{discovery_config_name}'" in output + match = re.search(r"run (\d+)", output) + assert match, f"no run id in output: {output}" + run_id = match.group(1) + + snapshot = tmp_path / "snapshot.yaml" + snap_result = runner.invoke(app, ["discover", "config-snapshot", run_id, "-o", str(snapshot)]) + assert snap_result.exit_code == 0, snap_result.stdout + assert snapshot.exists() and snapshot.read_text().strip() From eef74a5fe0ff043da11ecbb0951594221678889f Mon Sep 17 00:00:00 2001 From: Peter <101368063+ClassicMMT@users.noreply.github.com> Date: Mon, 20 Jul 2026 11:21:55 +1200 Subject: [PATCH 3/6] feat: add safe data preview support --- CHANGELOG.md | 2 + .../skills/datamasque-cli/SKILL.md | 8 ++ src/datamasque_cli/commands/discovery.py | 29 ++++- tests/commands/test_discovery.py | 105 +++++++++++++++++- 4 files changed, 137 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67ed5c6..c44b3f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ [--config ]` start discovery runs with or without a specific config. - `dm discover config-snapshot ` downloads the discovery config a run actually used. +- Safe Data Preview: `dm discover schema-results` and `dm discover file-report` + include `safe_data_preview` in their `--json` output. ## v1.4.0 diff --git a/claude-skills/datamasque-cli/skills/datamasque-cli/SKILL.md b/claude-skills/datamasque-cli/skills/datamasque-cli/SKILL.md index ad70cda..afd74e6 100644 --- a/claude-skills/datamasque-cli/skills/datamasque-cli/SKILL.md +++ b/claude-skills/datamasque-cli/skills/datamasque-cli/SKILL.md @@ -89,6 +89,14 @@ Pass repeated `--options key=value` for server-side knobs then fetch results with `dm discover schema-results ` / `sdd-report` / `db-report` / `file-report`. +- **Configurable discovery and Safe Data Preview.** Save a discovery config + with `dm discover configs create` (start from `dm discover configs defaults`), + then run `dm discover schema --config `. When the config + enables in-data discovery with safe data preview, `dm discover schema-results + --json` carries a `safe_data_preview` per column — value distributions, + patterns, and cardinality worth reading before choosing masks. It is JSON-only; + `file-report --json` exposes the same per locator. + - **`dm rulesets validate --file --type `** runs server-side validation without committing the ruleset. Use this before `create` when you want a clean failure mode for bad YAML. diff --git a/src/datamasque_cli/commands/discovery.py b/src/datamasque_cli/commands/discovery.py index 6daf571..975eaab 100644 --- a/src/datamasque_cli/commands/discovery.py +++ b/src/datamasque_cli/commands/discovery.py @@ -159,6 +159,9 @@ def schema_results( "data_type": r.data.data_type or "", "matches": ", ".join(m.label for m in r.data.discovery_matches if m.label) or "-", "constraint": r.data.constraint or "", + "safe_data_preview": ( + r.data.safe_data_preview.model_dump(mode="json") if r.data.safe_data_preview else None + ), } for r in results ] @@ -222,16 +225,34 @@ def file_discovery_report( """Download file discovery report for a run.""" client = get_client(profile) report = client.get_file_data_discovery_report(RunId(run_id)) + full = [result.model_dump(mode="json") for result in report] if output is not None: - output.write_text(json.dumps(report, indent=2, default=str)) + output.write_text(json.dumps(full, indent=2, default=str)) print_success(f"File discovery report written to {output}") return if should_emit_json(is_json): - print_json(report) - else: - render_output(report, is_json=False, title=f"File Discovery: Run {run_id}") + print_json(full) + return + + rows = [ + { + "id": result.id, + "files": ", ".join(f.path for f in result.files), + "locator": locator.locator, + "matches": ", ".join(m.label for m in locator.matches if m.label) or "-", + "data_types": ", ".join(locator.data_types) or "-", + } + for result in report + for locator in result.results + ] + render_output( + rows, + is_json=False, + columns=["id", "files", "locator", "matches", "data_types"], + title=f"File Discovery: Run {run_id}", + ) @app.command("config-snapshot") diff --git a/tests/commands/test_discovery.py b/tests/commands/test_discovery.py index 68fb69f..b877f07 100644 --- a/tests/commands/test_discovery.py +++ b/tests/commands/test_discovery.py @@ -5,7 +5,22 @@ from types import SimpleNamespace from unittest.mock import MagicMock, patch +from datamasque.client.models.discovery import ( + FileDiscoveryFile, + FileDiscoveryLocatorResult, + FileDiscoveryResult, +) from datamasque.client.models.discovery_config import DiscoveryConfigType +from datamasque.client.models.runs import RunConnectionRef +from datamasque.client.models.safe_data_preview import ( + CommonStatistics, + LengthsStatistics, + NumericPreview, + NumericStatistics, + NumericSummaries, + StringPreview, + StringStatistics, +) from typer.testing import CliRunner from datamasque_cli.main import app @@ -13,6 +28,24 @@ MODULE = "datamasque_cli.commands.discovery" +def _string_preview() -> StringPreview: + return StringPreview( + statistics_common=CommonStatistics(count_row=100, count_null=0, count_distinct=76), + statistics_kind=StringStatistics( + lengths=LengthsStatistics(min=8, max=30, mean=13.4, median=13.0, most_common=[]), + ), + ) + + +def _numeric_preview() -> NumericPreview: + return NumericPreview( + statistics_common=CommonStatistics(count_row=500, count_null=0, count_distinct=500), + statistics_kind=NumericStatistics( + summaries=NumericSummaries(mean=1.9e8, q1=9e7, q2=2.15e8, q3=2.7e8, p5=4.6e7, p95=2.78e8), + ), + ) + + @patch(f"{MODULE}.get_client") def test_sdd_report_writes_to_output_file(mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path) -> None: client = MagicMock() @@ -78,17 +111,48 @@ def test_db_report_split_without_output_aborts(mock_get_client: MagicMock, runne assert "-o" in result.stderr +def _file_report() -> list[FileDiscoveryResult]: + return [ + FileDiscoveryResult( + id=7, + connection=RunConnectionRef(id="c1", name="myinput"), + file_type="csv", + files=[FileDiscoveryFile(path="data.csv", file_type="csv")], + results=[ + FileDiscoveryLocatorResult( + locator="phone", matches=[], data_types=["int"], safe_data_preview=_numeric_preview() + ), + ], + ), + ] + + @patch(f"{MODULE}.get_client") def test_file_report_writes_json_to_output(mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path) -> None: client = MagicMock() mock_get_client.return_value = client - client.get_file_data_discovery_report.return_value = [{"file": "a"}] + client.get_file_data_discovery_report.return_value = _file_report() out = tmp_path / "file.json" - result = runner.invoke(app, ["discover", "file-report", "42", "--output", str(out)]) + result = runner.invoke(app, ["discover", "file-report", "7", "--output", str(out)]) assert result.exit_code == 0 - assert '"file": "a"' in out.read_text() + payload = json.loads(out.read_text()) + assert payload[0]["results"][0]["safe_data_preview"]["kind"] == "numeric" + + +@patch(f"{MODULE}.get_client") +def test_file_report_table_lists_locators(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.get_file_data_discovery_report.return_value = _file_report() + + result = runner.invoke(app, ["discover", "file-report", "7"]) + + assert result.exit_code == 0 + assert "phone" in result.stdout + assert "data.csv" in result.stdout + assert "safe_data_preview" not in result.stdout # -- schema discovery trigger --------------------------------------------- @@ -126,6 +190,7 @@ def test_schema_results_lists_with_flattened_rows(mock_get_client: MagicMock, ru data_type="varchar", discovery_matches=[SimpleNamespace(label="EMAIL_ADDRESS")], constraint="", + safe_data_preview=None, ), ), SimpleNamespace( @@ -140,6 +205,7 @@ def test_schema_results_lists_with_flattened_rows(mock_get_client: MagicMock, ru SimpleNamespace(label="PII"), ], constraint="Primary", + safe_data_preview=None, ), ), ] @@ -171,6 +237,7 @@ def test_schema_results_skips_unlabelled_matches(mock_get_client: MagicMock, run SimpleNamespace(label=None), ], constraint="", + safe_data_preview=None, ), ), SimpleNamespace( @@ -182,6 +249,7 @@ def test_schema_results_skips_unlabelled_matches(mock_get_client: MagicMock, run data_type="text", discovery_matches=[SimpleNamespace(label=None)], constraint="", + safe_data_preview=None, ), ), ] @@ -194,6 +262,37 @@ def test_schema_results_skips_unlabelled_matches(mock_get_client: MagicMock, run assert rows[1]["matches"] == "-" +@patch(f"{MODULE}.get_client") +def test_schema_results_includes_safe_data_preview_in_json(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_schema_discovery_results.return_value = [ + SimpleNamespace( + id=1, + column="author", + table="books", + schema_name="public", + data=SimpleNamespace( + data_type="varchar", + discovery_matches=[SimpleNamespace(label="name")], + constraint="", + safe_data_preview=_string_preview(), + ), + ), + ] + + result = runner.invoke(app, ["discover", "schema-results", "42", "--json"]) + + assert result.exit_code == 0 + rows = json.loads(result.stdout) + assert rows[0]["safe_data_preview"]["kind"] == "string" + assert rows[0]["safe_data_preview"]["statistics_kind"]["lengths"]["max"] == 30 + + table = runner.invoke(app, ["discover", "schema-results", "42"]) + assert table.exit_code == 0 + assert "safe_data_preview" not in table.stdout + + # -- configurable-discovery run triggers ---------------------------------- From 79c0e3c65c65ec01b1b2c65e24e77948f7d80dac Mon Sep 17 00:00:00 2001 From: Peter <101368063+ClassicMMT@users.noreply.github.com> Date: Tue, 21 Jul 2026 11:36:59 +1200 Subject: [PATCH 4/6] style: Move error codes into an enum --- src/datamasque_cli/output.py | 44 ++++++++++++------- tests/commands/test_discovery.py | 5 ++- .../test_discovery_config_libraries.py | 5 ++- tests/commands/test_discovery_configs.py | 9 ++-- tests/commands/test_ifm.py | 29 ++++++------ tests/commands/test_ruleset_libraries.py | 3 +- tests/commands/test_rulesets.py | 3 +- tests/commands/test_runs.py | 3 +- tests/commands/test_system.py | 3 +- tests/integration/test_discovery.py | 19 ++++---- tests/test_output.py | 6 +-- 11 files changed, 74 insertions(+), 55 deletions(-) diff --git a/src/datamasque_cli/output.py b/src/datamasque_cli/output.py index 5a45819..21567dd 100644 --- a/src/datamasque_cli/output.py +++ b/src/datamasque_cli/output.py @@ -14,7 +14,7 @@ import json import os import sys -from enum import StrEnum +from enum import IntEnum, StrEnum from typing import Any, NoReturn import typer @@ -51,8 +51,7 @@ class ErrorCode(StrEnum): """Stable, machine-readable error categories. StrEnum members are str subclasses, so the value flows directly into - the JSON envelope's `error.code` field via `json.dumps`. Pair with - `EXIT_CODES` to map to a process exit status. + the JSON envelope's `error.code` field via `json.dumps`. """ ERROR = "error" @@ -65,19 +64,30 @@ class ErrorCode(StrEnum): TRANSPORT_ERROR = "transport_error" -# Exit-code taxonomy for `abort()`. Stable across minor versions — agents can -# branch on these to decide whether to retry, prompt the user, or give up. -# `2` is intentionally skipped because typer/click already uses it for CLI -# usage errors (unknown flag, missing required arg). -EXIT_CODES: dict[ErrorCode, int] = { - ErrorCode.ERROR: 1, - ErrorCode.NOT_FOUND: 3, - ErrorCode.INVALID_INPUT: 4, - ErrorCode.AMBIGUOUS: 5, - ErrorCode.AUTH_REQUIRED: 6, - ErrorCode.AUTH_FAILED: 7, - ErrorCode.CONFLICT: 8, - ErrorCode.TRANSPORT_ERROR: 9, +class ExitCode(IntEnum): + """Every process exit status the CLI can return.""" + + OK = 0 + ERROR = 1 + USAGE = 2 + NOT_FOUND = 3 + INVALID_INPUT = 4 + AMBIGUOUS = 5 + AUTH_REQUIRED = 6 + AUTH_FAILED = 7 + CONFLICT = 8 + TRANSPORT_ERROR = 9 + + +EXIT_CODE_BY_ERROR: dict[ErrorCode, ExitCode] = { + ErrorCode.ERROR: ExitCode.ERROR, + ErrorCode.NOT_FOUND: ExitCode.NOT_FOUND, + ErrorCode.INVALID_INPUT: ExitCode.INVALID_INPUT, + ErrorCode.AMBIGUOUS: ExitCode.AMBIGUOUS, + ErrorCode.AUTH_REQUIRED: ExitCode.AUTH_REQUIRED, + ErrorCode.AUTH_FAILED: ExitCode.AUTH_FAILED, + ErrorCode.CONFLICT: ExitCode.CONFLICT, + ErrorCode.TRANSPORT_ERROR: ExitCode.TRANSPORT_ERROR, } @@ -248,7 +258,7 @@ def abort(message: str, *, code: ErrorCode = ErrorCode.ERROR, hint: str | None = print_error(message) if hint: console.print(f"[dim]Hint: {hint}[/dim]") - raise SystemExit(EXIT_CODES[code]) + raise SystemExit(EXIT_CODE_BY_ERROR[code]) def abort_if_invalid(subject: str, is_valid: ValidationStatus | None, errors: list[ValidationErrorDetails]) -> None: diff --git a/tests/commands/test_discovery.py b/tests/commands/test_discovery.py index b877f07..7142fff 100644 --- a/tests/commands/test_discovery.py +++ b/tests/commands/test_discovery.py @@ -24,6 +24,7 @@ from typer.testing import CliRunner from datamasque_cli.main import app +from datamasque_cli.output import ExitCode MODULE = "datamasque_cli.commands.discovery" @@ -107,7 +108,7 @@ def test_db_report_split_without_output_aborts(mock_get_client: MagicMock, runne result = runner.invoke(app, ["discover", "db-report", "42"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.INVALID_INPUT assert "-o" in result.stderr @@ -328,7 +329,7 @@ def test_schema_config_wrong_type_aborts(mock_get_client: MagicMock, runner: Cli result = runner.invoke(app, ["discover", "schema", "my_db", "--config", "docs"]) - assert result.exit_code == 4 # invalid_input + assert result.exit_code == ExitCode.INVALID_INPUT client.start_schema_discovery_run_from_config.assert_not_called() diff --git a/tests/commands/test_discovery_config_libraries.py b/tests/commands/test_discovery_config_libraries.py index ceb2dc7..ebb65f5 100644 --- a/tests/commands/test_discovery_config_libraries.py +++ b/tests/commands/test_discovery_config_libraries.py @@ -8,6 +8,7 @@ from typer.testing import CliRunner from datamasque_cli.main import app +from datamasque_cli.output import ExitCode MODULE = "datamasque_cli.commands.discovery_config_libraries" @@ -70,7 +71,7 @@ def test_get_namespace_scopes_lookup(mock_get_client: MagicMock, runner: CliRunn result = runner.invoke(app, ["discover", "libraries", "get", "finance"]) - assert result.exit_code == 3 + assert result.exit_code == ExitCode.NOT_FOUND client.get_discovery_config_library.assert_not_called() @@ -115,5 +116,5 @@ def test_validate_invalid_exits_4(mock_get_client: MagicMock, runner: CliRunner, result = runner.invoke(app, ["discover", "libraries", "validate", "-f", str(lib), "--type", "database"]) - assert result.exit_code == 4 + assert result.exit_code == ExitCode.INVALID_INPUT assert "duplicate label 'email'" in result.stderr diff --git a/tests/commands/test_discovery_configs.py b/tests/commands/test_discovery_configs.py index 509c98e..5266ff4 100644 --- a/tests/commands/test_discovery_configs.py +++ b/tests/commands/test_discovery_configs.py @@ -8,6 +8,7 @@ from typer.testing import CliRunner from datamasque_cli.main import app +from datamasque_cli.output import ExitCode MODULE = "datamasque_cli.commands.discovery_configs" @@ -72,7 +73,7 @@ def test_get_ambiguous_name_aborts(mock_get_client: MagicMock, runner: CliRunner result = runner.invoke(app, ["discover", "configs", "get", "shared"]) - assert result.exit_code == 5 + assert result.exit_code == ExitCode.AMBIGUOUS client.get_discovery_config.assert_not_called() @@ -116,7 +117,7 @@ def test_create_new_requires_type(mock_get_client: MagicMock, runner: CliRunner, cfg.write_text("labels: []\n") missing_type = runner.invoke(app, ["discover", "configs", "create", "--name", "emp", "-f", str(cfg)]) - assert missing_type.exit_code == 3 + assert missing_type.exit_code == ExitCode.NOT_FOUND client.create_or_update_discovery_config.assert_not_called() with_type = runner.invoke( @@ -160,7 +161,7 @@ def test_delete_aborts_when_missing(mock_get_client: MagicMock, runner: CliRunne result = runner.invoke(app, ["discover", "configs", "delete", "nope", "--yes"]) - assert result.exit_code == 3 + assert result.exit_code == ExitCode.NOT_FOUND client.delete_discovery_config_by_id_if_exists.assert_not_called() @@ -193,5 +194,5 @@ def test_validate_invalid_exits_4(mock_get_client: MagicMock, runner: CliRunner, result = runner.invoke(app, ["discover", "configs", "validate", "-f", str(cfg), "--type", "database"]) - assert result.exit_code == 4 + assert result.exit_code == ExitCode.INVALID_INPUT assert "unknown label 'foo'" in result.stderr diff --git a/tests/commands/test_ifm.py b/tests/commands/test_ifm.py index 6081bc1..d9eb481 100644 --- a/tests/commands/test_ifm.py +++ b/tests/commands/test_ifm.py @@ -10,6 +10,7 @@ from typer.testing import CliRunner from datamasque_cli.main import app +from datamasque_cli.output import ExitCode MODULE = "datamasque_cli.commands.ifm" @@ -171,7 +172,7 @@ def test_update_aborts_when_no_fields_provided(mock_get_client: MagicMock, runne result = runner.invoke(app, ["ifm", "update", "p1"]) - assert result.exit_code == 4 + assert result.exit_code == ExitCode.INVALID_INPUT client.patch_ruleset_plan.assert_not_called() @@ -236,7 +237,7 @@ def test_mask_soft_failure_exits_nonzero_and_logs( result = runner.invoke(app, ["ifm", "mask", "p1", "--data", str(data_file)]) - assert result.exit_code == 1 + assert result.exit_code == ExitCode.ERROR assert "Mask failed." in result.stderr assert "bad input" in result.stderr @@ -323,7 +324,7 @@ def test_list_aborts_on_api_error(mock_get_client: MagicMock, runner: CliRunner) result = runner.invoke(app, ["ifm", "list"]) - assert result.exit_code == 1 + assert result.exit_code == ExitCode.ERROR assert "Failed to list IFM ruleset plans" in result.stderr @@ -335,7 +336,7 @@ def test_get_aborts_on_api_error(mock_get_client: MagicMock, runner: CliRunner) result = runner.invoke(app, ["ifm", "get", "p1"]) - assert result.exit_code == 1 + assert result.exit_code == ExitCode.ERROR assert "Failed to get IFM ruleset plan 'p1'" in result.stderr @@ -347,7 +348,7 @@ def test_verify_token_aborts_on_api_error(mock_get_client: MagicMock, runner: Cl result = runner.invoke(app, ["ifm", "verify-token"]) - assert result.exit_code == 1 + assert result.exit_code == ExitCode.ERROR assert "Failed to verify IFM token" in result.stderr @@ -363,7 +364,7 @@ def test_get_404_exits_with_not_found_code(mock_get_client: MagicMock, runner: C result = runner.invoke(app, ["ifm", "get", "p1"]) - assert result.exit_code == 3 + assert result.exit_code == ExitCode.NOT_FOUND assert "Ruleset plan 'p1' not found." in result.stderr @@ -382,7 +383,7 @@ def test_create_400_surfaces_server_error_body(mock_get_client: MagicMock, runne result = runner.invoke(app, ["ifm", "create", "--name", "smoke", "--file", str(yaml_file)]) - assert result.exit_code == 4 + assert result.exit_code == ExitCode.INVALID_INPUT assert "unknown mask type 'from_invalid'" in _flat(result.stderr) @@ -401,7 +402,7 @@ def test_mask_400_surfaces_server_error_body(mock_get_client: MagicMock, runner: result = runner.invoke(app, ["ifm", "mask", "p1", "--data", str(data_file), "--run-secret", "short"]) - assert result.exit_code == 4 + assert result.exit_code == ExitCode.INVALID_INPUT assert "Run secret length must be at least 20 characters." in _flat(result.stderr) @@ -417,7 +418,7 @@ def test_update_404_exits_with_not_found_code(mock_get_client: MagicMock, runner result = runner.invoke(app, ["ifm", "update", "p1", "--enabled"]) - assert result.exit_code == 3 + assert result.exit_code == ExitCode.NOT_FOUND assert "Ruleset plan 'p1' not found." in result.stderr @@ -433,7 +434,7 @@ def test_delete_404_exits_with_not_found_code(mock_get_client: MagicMock, runner result = runner.invoke(app, ["ifm", "delete", "p1", "--yes"]) - assert result.exit_code == 3 + assert result.exit_code == ExitCode.NOT_FOUND assert "Ruleset plan 'p1' not found." in result.stderr @@ -452,7 +453,7 @@ def test_create_409_exits_with_conflict_code(mock_get_client: MagicMock, runner: result = runner.invoke(app, ["ifm", "create", "--name", "smoke", "--file", str(yaml_file)]) - assert result.exit_code == 8 + assert result.exit_code == ExitCode.CONFLICT assert "already exists" in _flat(result.stderr) @@ -464,7 +465,7 @@ def test_get_404_falls_back_when_body_not_json(mock_get_client: MagicMock, runne result = runner.invoke(app, ["ifm", "get", "p1"]) - assert result.exit_code == 3 + assert result.exit_code == ExitCode.NOT_FOUND assert "Failed to get IFM ruleset plan 'p1'" in result.stderr @@ -480,7 +481,7 @@ def test_get_extracts_fastapi_detail_field(mock_get_client: MagicMock, runner: C result = runner.invoke(app, ["ifm", "get", "p1"]) - assert result.exit_code == 4 + assert result.exit_code == ExitCode.INVALID_INPUT assert "validation failed on field 'name'" in result.stderr @@ -539,7 +540,7 @@ def test_get_formats_pydantic_422_detail_list(mock_get_client: MagicMock, runner result = runner.invoke(app, ["ifm", "get", "p1"]) - assert result.exit_code == 4 + assert result.exit_code == ExitCode.INVALID_INPUT flat = _flat(result.stderr) assert "name: field required" in flat assert "options.log_level: invalid choice" in flat diff --git a/tests/commands/test_ruleset_libraries.py b/tests/commands/test_ruleset_libraries.py index ecacdd1..93d67ef 100644 --- a/tests/commands/test_ruleset_libraries.py +++ b/tests/commands/test_ruleset_libraries.py @@ -7,6 +7,7 @@ from typer.testing import CliRunner from datamasque_cli.main import app +from datamasque_cli.output import ExitCode MODULE = "datamasque_cli.commands.ruleset_libraries" @@ -83,7 +84,7 @@ def test_validate_library_invalid_prints_errors_and_exits_4(mock_get_client: Mag result = runner.invoke(app, ["libraries", "validate", "my-lib"]) - assert result.exit_code == 4 # invalid_input + assert result.exit_code == ExitCode.INVALID_INPUT assert "unknown mask type 'from_nowhere'" in result.stderr assert "line 3" in result.stderr assert "duplicate anchor 'email'" in result.stderr diff --git a/tests/commands/test_rulesets.py b/tests/commands/test_rulesets.py index fc418ee..0f4359d 100644 --- a/tests/commands/test_rulesets.py +++ b/tests/commands/test_rulesets.py @@ -12,6 +12,7 @@ from typer.testing import CliRunner from datamasque_cli.main import app +from datamasque_cli.output import ExitCode MODULE = "datamasque_cli.commands.rulesets" @@ -325,7 +326,7 @@ def test_validate_sync_invalid_prints_errors_and_cleans_up( result = runner.invoke(app, ["rulesets", "validate", "--file", str(yaml_file), "--type", "database"]) - assert result.exit_code == 4 # invalid_input + assert result.exit_code == ExitCode.INVALID_INPUT assert "unknown mask type 'from_nowhere'" in result.stderr assert "line 7" in result.stderr assert "tasks must not be empty" in result.stderr diff --git a/tests/commands/test_runs.py b/tests/commands/test_runs.py index 4d254aa..f6f33fb 100644 --- a/tests/commands/test_runs.py +++ b/tests/commands/test_runs.py @@ -24,6 +24,7 @@ _resolve_ruleset_id, ) from datamasque_cli.main import app +from datamasque_cli.output import ExitCode MODULE = "datamasque_cli.commands.runs" @@ -460,7 +461,7 @@ def test_wait_run_failure_exits_1(mock_get_client: MagicMock, _mock_time: MagicM client.get_run_info.return_value = _run_info(id=1, status="failed") result = runner.invoke(app, ["run", "wait", "1"]) - assert result.exit_code == 1 + assert result.exit_code == ExitCode.ERROR # -- _print_pretty_logs ---------------------------------------------------- diff --git a/tests/commands/test_system.py b/tests/commands/test_system.py index a952cda..159333c 100644 --- a/tests/commands/test_system.py +++ b/tests/commands/test_system.py @@ -9,6 +9,7 @@ from typer.testing import CliRunner from datamasque_cli.main import app +from datamasque_cli.output import ExitCode MODULE = "datamasque_cli.commands.system" @@ -143,7 +144,7 @@ def test_admin_install_translates_401_into_conflict(mock_get_unauth: MagicMock, ], ) - assert result.exit_code == 8 # ErrorCode.CONFLICT + assert result.exit_code == ExitCode.CONFLICT assert "already complete" in result.stderr assert "dm auth login" in result.stderr diff --git a/tests/integration/test_discovery.py b/tests/integration/test_discovery.py index 0b13cc3..213e7f6 100644 --- a/tests/integration/test_discovery.py +++ b/tests/integration/test_discovery.py @@ -9,6 +9,7 @@ from typer.testing import CliRunner from datamasque_cli.main import app +from datamasque_cli.output import ExitCode from tests.integration.conftest import DISCOVERY_TEST_NAMESPACE pytestmark = pytest.mark.integration @@ -49,7 +50,7 @@ def test_config_create_get_delete_lifecycle( assert delete.exit_code == 0 gone = runner.invoke(app, ["discover", "configs", "get", discovery_config_name]) - assert gone.exit_code == 3 + assert gone.exit_code == ExitCode.NOT_FOUND def test_config_validate_accepts_default_config(runner: CliRunner, db_discovery_config: Path) -> None: @@ -63,7 +64,7 @@ def test_config_validate_rejects_invalid_yaml(runner: CliRunner, invalid_discove result = runner.invoke( app, ["discover", "configs", "validate", "-f", str(invalid_discovery_yaml), "--type", "database"] ) - assert result.exit_code == 4 + assert result.exit_code == ExitCode.INVALID_INPUT assert "invalid" in result.stderr.lower() @@ -154,7 +155,7 @@ def test_config_create_without_type_aborts_when_ambiguous( def test_config_get_missing_is_not_found(runner: CliRunner) -> None: result = runner.invoke(app, ["discover", "configs", "get", "dm_int_does_not_exist"]) - assert result.exit_code == 3 + assert result.exit_code == ExitCode.NOT_FOUND # --- discovery config libraries ---------------------------------------------- @@ -191,7 +192,7 @@ def test_library_create_get_delete_lifecycle( assert delete.exit_code == 0 gone = runner.invoke(app, ["discover", "libraries", "get", discovery_library_name]) - assert gone.exit_code == 3 + assert gone.exit_code == ExitCode.NOT_FOUND def test_library_namespace_is_isolated( @@ -223,14 +224,14 @@ def test_library_namespace_is_isolated( assert in_namespace.exit_code == 0 default_namespace = runner.invoke(app, ["discover", "libraries", "get", discovery_library_name]) - assert default_namespace.exit_code == 3 + assert default_namespace.exit_code == ExitCode.NOT_FOUND def test_library_validate_rejects_invalid_yaml(runner: CliRunner, invalid_discovery_yaml: Path) -> None: result = runner.invoke( app, ["discover", "libraries", "validate", "-f", str(invalid_discovery_yaml), "--type", "database"] ) - assert result.exit_code == 4 + assert result.exit_code == ExitCode.INVALID_INPUT # --- `--config` resolution guards (abort before any run starts) -------------- @@ -257,7 +258,7 @@ def test_schema_config_type_mismatch_aborts( ], ) result = runner.invoke(app, ["discover", "schema", any_connection, "--config", discovery_config_name]) - assert result.exit_code == 4 + assert result.exit_code == ExitCode.INVALID_INPUT assert "database config" in result.stderr @@ -282,13 +283,13 @@ def test_file_config_type_mismatch_aborts( ], ) result = runner.invoke(app, ["discover", "file", any_connection, "--config", discovery_config_name]) - assert result.exit_code == 4 + assert result.exit_code == ExitCode.INVALID_INPUT assert "file config" in result.stderr def test_schema_config_not_found_aborts(runner: CliRunner, any_connection: str) -> None: result = runner.invoke(app, ["discover", "schema", any_connection, "--config", "dm_int_no_such_config"]) - assert result.exit_code == 3 + assert result.exit_code == ExitCode.NOT_FOUND # --- run from config + config snapshot (env-gated) --------------------------- diff --git a/tests/test_output.py b/tests/test_output.py index 961eb97..f639302 100644 --- a/tests/test_output.py +++ b/tests/test_output.py @@ -5,7 +5,7 @@ import pytest from datamasque_cli.output import ( - EXIT_CODES, + EXIT_CODE_BY_ERROR, ErrorCode, abort, is_agent_context, @@ -186,8 +186,8 @@ def test_abort_maps_code_to_documented_exit_code(code: ErrorCode, expected_exit: def test_exit_code_table_covers_every_error_code() -> None: # Guard: every ErrorCode member must have an exit-code mapping. This trips - # if a new ErrorCode is added without updating EXIT_CODES. - assert set(EXIT_CODES.keys()) == set(ErrorCode) + # if a new ErrorCode is added without updating EXIT_CODE_BY_ERROR. + assert set(EXIT_CODE_BY_ERROR.keys()) == set(ErrorCode) def test_print_success_suppressed_in_agent_mode( From ecc96859e920f05a94440b730aa663c827519171 Mon Sep 17 00:00:00 2001 From: Peter <101368063+ClassicMMT@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:26:55 +1200 Subject: [PATCH 5/6] feat: Add validation status commands - Add support for datamasque-python 1.2.1 - Add a status command to rulesets, libraries, discover configs, and discover libraries - Refuse YAML of 60 KiB+ in rulesets/discover configs validate - Rework discover libraries for updated library model - Fix rulesets generate and connections update --password - Drop click dependency - Update changelog - Report server reason when library deletion is rejected --- CHANGELOG.md | 24 ++- README.md | 22 ++- src/datamasque_cli/commands/connections.py | 21 ++- src/datamasque_cli/commands/discovery.py | 61 ++++-- .../commands/discovery_config_libraries.py | 176 +++++++----------- .../commands/discovery_configs.py | 118 ++++++++---- .../commands/ruleset_libraries.py | 58 +++++- src/datamasque_cli/commands/rulesets.py | 99 +++++++--- src/datamasque_cli/commands/system.py | 9 +- src/datamasque_cli/main.py | 62 +++--- src/datamasque_cli/output.py | 30 +++ src/datamasque_cli/protocols.py | 59 ++++++ tests/commands/test_connections.py | 20 +- tests/commands/test_discovery.py | 43 +++++ .../test_discovery_config_libraries.py | 121 ++++++++++-- tests/commands/test_discovery_configs.py | 64 ++++++- tests/commands/test_ruleset_libraries.py | 63 ++++++- tests/commands/test_rulesets.py | 72 ++++++- tests/integration/conftest.py | 9 +- tests/integration/test_discovery.py | 8 +- 20 files changed, 869 insertions(+), 270 deletions(-) create mode 100644 src/datamasque_cli/protocols.py diff --git a/CHANGELOG.md b/CHANGELOG.md index c44b3f9..f4b9b60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,24 +3,28 @@ ## v1.5.0 ### Added -- Support for datamasque-python 1.1.8. +- Support for datamasque-python 1.2.1. - `dm discover schema-results` handles matches with no label. - - Validation errors are now printed. - - `dm rulesets validate` and `dm libraries validate` now fail (return 4) - on invalid rulesets/libraries. - - `dm discover db-report` writes a zip archive returned for large reports to - `--output`, aborting with a hint rather than dumping binary data to stdout. + - `dm rulesets validate` and `dm libraries validate` print validation + errors for invalid YAML. - Support for Configurable Discovery: - - `dm discover configs` — list, get, defaults, create, delete, and validate - discovery configs (`database` or `file`). - - `dm discover libraries` — list, get, create, delete, and validate discovery - config libraries. + - `dm discover configs` — list, get, defaults, create, delete, validate, + and status for discovery configs (`database` or `file`). + - `dm discover libraries` — list, get, create, delete, validate, and status + for discovery config libraries (untyped; shared by both config types). - `dm discover schema --config ` and `dm discover file [--config ]` start discovery runs with or without a specific config. - `dm discover config-snapshot ` downloads the discovery config a run actually used. +- `dm rulesets status` and `dm libraries status` — show a stored ruleset's or + library's validation state and errors. +- `dm rulesets validate` and `dm discover configs validate` refuse YAML of + 60 KiB or larger, which the server validates asynchronously; create it and + poll `status` instead. - Safe Data Preview: `dm discover schema-results` and `dm discover file-report` include `safe_data_preview` in their `--json` output. +- `dm rulesets generate`, `dm connections update --password`, and the + deprecated `dm system import` no longer fail. ## v1.4.0 diff --git a/README.md b/README.md index bc9c940..06a94ac 100644 --- a/README.md +++ b/README.md @@ -148,7 +148,8 @@ dm rulesets create --name --file rules.yaml --type file # Force a type dm rulesets delete [--type file|database] # Delete a ruleset dm rulesets generate --file request.json # Generate from schema dm rulesets generate --file req.json -o out.yaml # Generate to file -dm rulesets validate --file rules.yaml # Validate against server +dm rulesets validate --file rules.yaml # Validate against server (YAML under 60 KiB) +dm rulesets status # Validation status; poll after creating YAML of 60 KiB+ dm rulesets export-bundle -o bundle.zip # Export rulesets + libraries + seeds dm rulesets import-bundle --file bundle.zip # Import a previously exported bundle dm rulesets import-bundle -f bundle.zip --overwrite-rulesets --overwrite-libraries # Replace existing entries @@ -164,6 +165,7 @@ dm libraries create --name --file lib.yaml # Create/update from file dm libraries create --name --file lib.yaml --namespace pii # With namespace dm libraries delete # Delete a library dm libraries validate # Re-validate against current server schema +dm libraries status # Validation status; poll after creating YAML of 60 KiB+ dm libraries usage # Show rulesets using it ``` @@ -218,9 +220,11 @@ dm users delete # Delete a user ```console dm discover schema # Schema discovery (built-in keyword-driven) dm discover schema --config # Schema discovery from a saved database config +dm discover schema --json # {"id": , "status": "queued"} dm discover schema-results # List schema-discovery results once the run finishes dm discover file # File data discovery (built-in keyword-driven) dm discover file --config # File data discovery from a saved file config +dm discover file --json # {"id": , "status": "queued"} dm discover sdd-report # Sensitive data discovery report dm discover db-report # Database discovery CSV dm discover file-report # File discovery report @@ -235,17 +239,21 @@ dm discover configs get [--type database] [--yaml] # Show detail dm discover configs defaults [--type database|file] -o cfg.yaml # Built-in default as a starting point dm discover configs create --name --type database -f cfg.yaml # Create/update from YAML dm discover configs delete [--type database] # Delete a config -dm discover configs validate -f cfg.yaml --type database # Validate a YAML file against the server +dm discover configs validate -f cfg.yaml --type database # Validate against server (YAML under 60 KiB) +dm discover configs status [--type database] # Validation status; poll after creating YAML of 60 KiB+ ``` #### Discovery config libraries +Libraries are untyped — the same library can be imported by both database and file discovery configs. + ```console -dm discover libraries list [--type database|file] -dm discover libraries get [--type database] [--namespace org] [--yaml] -dm discover libraries create --name --type database --namespace org -f lib.yaml -dm discover libraries delete [--type database] [--namespace org] [--force] # --force if imported by configs -dm discover libraries validate -f lib.yaml --type database +dm discover libraries list +dm discover libraries get [--namespace org] [--yaml] +dm discover libraries create --name --namespace org -f lib.yaml +dm discover libraries delete [--namespace org] [--force] # --force if imported by configs +dm discover libraries validate -f lib.yaml +dm discover libraries status [--namespace org] ``` ### Seeds diff --git a/src/datamasque_cli/commands/connections.py b/src/datamasque_cli/commands/connections.py index 1957e61..1196594 100644 --- a/src/datamasque_cli/commands/connections.py +++ b/src/datamasque_cli/commands/connections.py @@ -8,6 +8,7 @@ import typer from datamasque.client import DataMasqueClient +from datamasque.client.exceptions import DataMasqueApiError from datamasque.client.models.connection import ( AzureConnectionConfig, ConnectionConfig, @@ -22,7 +23,14 @@ ) from datamasque_cli.client import get_client -from datamasque_cli.output import ErrorCode, abort, print_success, redact_sensitive_fields, render_output +from datamasque_cli.output import ( + ErrorCode, + abort, + abort_api_error, + print_success, + redact_sensitive_fields, + render_output, +) class ConnectionType(StrEnum): @@ -298,7 +306,10 @@ def test_connection( if match is None: abort(f"Connection '{name}' not found.", code=ErrorCode.NOT_FOUND) - response = client.make_request("POST", f"/api/connections/{match.id}/test/", data={}) + try: + response = client.make_request("POST", f"/api/connections/{match.id}/test/", data={}) + except DataMasqueApiError as exc: + abort_api_error(f"Connection '{match.name}' is not reachable", exc) body = response.json() if response.content else {} warning = body.get("message") if isinstance(body, dict) else None @@ -347,7 +358,11 @@ def update_connection( if not updates: abort("Pass at least one field to update (e.g. --password, --host).", code=ErrorCode.INVALID_INPUT) - client.make_request("PATCH", f"/api/connections/{match.id}/", data=updates) + payload = dict(updates) + if "password" in payload: + payload["dbpassword"] = payload.pop("password") + + client.make_request("PATCH", f"/api/connections/{match.id}/", data=payload) print_success(f"Connection '{match.name}' updated: {', '.join(updates)}.") diff --git a/src/datamasque_cli/commands/discovery.py b/src/datamasque_cli/commands/discovery.py index 975eaab..5ec64fd 100644 --- a/src/datamasque_cli/commands/discovery.py +++ b/src/datamasque_cli/commands/discovery.py @@ -7,6 +7,7 @@ import typer from datamasque.client import DataMasqueClient, RunId +from datamasque.client.exceptions import DataMasqueApiError from datamasque.client.models.connection import ConnectionId from datamasque.client.models.discovery import ( FileDataDiscoveryFromConfigRequest, @@ -18,7 +19,15 @@ from datamasque_cli.client import get_client from datamasque_cli.commands import discovery_config_libraries, discovery_configs -from datamasque_cli.output import ErrorCode, abort, print_json, print_success, render_output, should_emit_json +from datamasque_cli.output import ( + ErrorCode, + abort, + abort_api_error, + print_json, + print_success, + render_output, + should_emit_json, +) app = typer.Typer(help="Data discovery operations.", no_args_is_help=True) app.add_typer(discovery_configs.app, name="configs") @@ -77,6 +86,7 @@ def schema_discovery( None, "--config", "-c", help="Run with a saved database discovery config (configurable discovery)" ), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), + is_json: bool = typer.Option(False, "--json", help="Output as JSON"), ) -> None: """Start a schema-discovery run on a connection. @@ -87,20 +97,25 @@ def schema_discovery( client = get_client(profile) conn_id = _resolve_connection_id(client, connection) - if config is not None: - config_id = _resolve_discovery_config_id(client, config, DiscoveryConfigType.database) - from_config = SchemaDiscoveryFromConfigRequest(connection=ConnectionId(conn_id), discovery_config=config_id) - run_id = client.start_schema_discovery_run_from_config(from_config) - source = f"config '{config}'" - else: - request = SchemaDiscoveryRequest(connection=ConnectionId(conn_id)) - run_id = client.start_schema_discovery_run(request) - source = "default discovery" + try: + if config is not None: + config_id = _resolve_discovery_config_id(client, config, DiscoveryConfigType.database) + from_config = SchemaDiscoveryFromConfigRequest(connection=ConnectionId(conn_id), discovery_config=config_id) + run_id = client.start_schema_discovery_run_from_config(from_config) + source = f"config '{config}'" + else: + request = SchemaDiscoveryRequest(connection=ConnectionId(conn_id)) + run_id = client.start_schema_discovery_run(request) + source = "default discovery" + except DataMasqueApiError as exc: + abort_api_error(f"Failed to start schema discovery on '{connection}'", exc) print_success( f"Schema discovery run {run_id} started for connection '{connection}' ({source}). " f"Once finished, list results with: dm discover schema-results {run_id}" ) + if should_emit_json(is_json): + print_json({"id": int(run_id), "status": "queued"}) @app.command("file") @@ -110,6 +125,7 @@ def file_discovery( None, "--config", "-c", help="Run with a saved file discovery config (configurable discovery)" ), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), + is_json: bool = typer.Option(False, "--json", help="Output as JSON"), ) -> None: """Start a file-data-discovery run on a file connection. @@ -119,20 +135,27 @@ def file_discovery( client = get_client(profile) conn_id = _resolve_connection_id(client, connection) - if config is not None: - config_id = _resolve_discovery_config_id(client, config, DiscoveryConfigType.file) - from_config = FileDataDiscoveryFromConfigRequest(connection=ConnectionId(conn_id), discovery_config=config_id) - run_id = client.start_file_data_discovery_run_from_config(from_config) - source = f"config '{config}'" - else: - request = FileDataDiscoveryRequest(connection=ConnectionId(conn_id)) - run_id = client.start_file_data_discovery_run(request) - source = "default discovery" + try: + if config is not None: + config_id = _resolve_discovery_config_id(client, config, DiscoveryConfigType.file) + from_config = FileDataDiscoveryFromConfigRequest( + connection=ConnectionId(conn_id), discovery_config=config_id + ) + run_id = client.start_file_data_discovery_run_from_config(from_config) + source = f"config '{config}'" + else: + request = FileDataDiscoveryRequest(connection=ConnectionId(conn_id)) + run_id = client.start_file_data_discovery_run(request) + source = "default discovery" + except DataMasqueApiError as exc: + abort_api_error(f"Failed to start file data discovery on '{connection}'", exc) print_success( f"File data discovery run {run_id} started for connection '{connection}' ({source}). " f"Once finished, download the report with: dm discover file-report {run_id}" ) + if should_emit_json(is_json): + print_json({"id": int(run_id), "status": "queued"}) @app.command("schema-results") diff --git a/src/datamasque_cli/commands/discovery_config_libraries.py b/src/datamasque_cli/commands/discovery_config_libraries.py index d375f65..8a93462 100644 --- a/src/datamasque_cli/commands/discovery_config_libraries.py +++ b/src/datamasque_cli/commands/discovery_config_libraries.py @@ -5,13 +5,12 @@ from pathlib import Path import typer -from datamasque.client import DataMasqueClient -from datamasque.client.models.discovery_config import DiscoveryConfigType +from datamasque.client.exceptions import DataMasqueApiError, DataMasqueArgumentError from datamasque.client.models.discovery_config_library import DiscoveryConfigLibrary from datamasque.client.models.status import ValidationStatus from datamasque_cli.client import get_client -from datamasque_cli.output import ErrorCode, abort, print_info, print_success, render_output +from datamasque_cli.output import ErrorCode, ExitCode, abort, abort_api_error, print_success, render_output app = typer.Typer(help="Manage discovery config libraries (configurable discovery).", no_args_is_help=True) @@ -21,40 +20,8 @@ def _label(name: str, namespace: str) -> str: return f"{namespace}/{name}" if namespace else name -def _find_by_name( - client: DataMasqueClient, - name: str, - config_type: DiscoveryConfigType | None = None, - namespace: str | None = None, -) -> list[DiscoveryConfigLibrary]: - """Return all libraries matching `name`, optionally narrowed by `namespace` and `config_type`.""" - matches = [lib for lib in client.list_discovery_config_libraries() if lib.name == name] - if namespace is not None: - matches = [lib for lib in matches if lib.namespace == namespace] - if config_type is not None: - matches = [lib for lib in matches if lib.config_type is config_type] - return matches - - -def _pick_single(matches: list[DiscoveryConfigLibrary], name: str) -> DiscoveryConfigLibrary: - """Return the sole match or abort with a disambiguation message.""" - if not matches: - abort(f"Discovery config library '{name}' not found.", code=ErrorCode.NOT_FOUND) - if len(matches) > 1: - options = "\n ".join( - f"id={lib.id} namespace={lib.namespace or '(default)'} type={lib.config_type.value}" for lib in matches - ) - abort( - f"Multiple discovery config libraries named '{name}':\n {options}", - code=ErrorCode.AMBIGUOUS, - hint="Pass --type file|database and/or --namespace to disambiguate.", - ) - return matches[0] - - @app.command("list") def list_libraries( - config_type: str | None = typer.Option(None, "--type", "-t", help="Filter by type: database or file"), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), is_json: bool = typer.Option(False, "--json", help="Output as JSON"), ) -> None: @@ -62,17 +29,13 @@ def list_libraries( client = get_client(profile) libraries = client.list_discovery_config_libraries() - if config_type is not None: - wanted = DiscoveryConfigType(config_type) - libraries = [lib for lib in libraries if lib.config_type is wanted] - data = [ { "id": lib.id, "namespace": lib.namespace or "", "name": lib.name, - "type": lib.config_type.value, "valid": lib.is_valid.value if lib.is_valid else "unknown", + "used_by": lib.usage_count, } for lib in libraries ] @@ -80,7 +43,7 @@ def list_libraries( render_output( data, is_json=is_json, - columns=["id", "namespace", "name", "type", "valid"], + columns=["id", "namespace", "name", "valid", "used_by"], title="Discovery Config Libraries", ) @@ -88,7 +51,6 @@ def list_libraries( @app.command("get") def get_library( name: str = typer.Argument(help="Library name"), - config_type: str | None = typer.Option(None, "--type", "-t", help="Required when two libraries share a name"), namespace: str = typer.Option("", "--namespace", "-n", help="Library namespace"), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), is_yaml: bool = typer.Option(False, "--yaml", help="Output raw YAML content only"), @@ -96,79 +58,47 @@ def get_library( ) -> None: """Show a discovery config library's details or YAML content.""" client = get_client(profile) - wanted = DiscoveryConfigType(config_type) if config_type is not None else None - match = _pick_single(_find_by_name(client, name, wanted, namespace), name) + lib = client.get_discovery_config_library_by_name(name, namespace) - # `list_discovery_config_libraries` omits the YAML body; fetch the single library for it. - assert match.id is not None - full = client.get_discovery_config_library(match.id) + if lib is None: + abort(f"Discovery config library '{_label(name, namespace)}' not found.", code=ErrorCode.NOT_FOUND) if is_yaml: - typer.echo(full.yaml) + typer.echo(lib.yaml) return data: dict[str, object] = { - "id": full.id, - "namespace": full.namespace, - "name": full.name, - "type": full.config_type.value, - "valid": full.is_valid.value if full.is_valid else "unknown", - "created": full.created, - "modified": full.modified, + "id": lib.id, + "namespace": lib.namespace, + "name": lib.name, + "valid": lib.is_valid.value if lib.is_valid else "unknown", + "used_by": lib.usage_count, + "created": lib.created, + "modified": lib.modified, } - render_output(data, is_json=is_json, title=f"Discovery Config Library: {full.name}") + render_output(data, is_json=is_json, title=f"Discovery Config Library: {lib.name}") @app.command("create") def create_library( name: str = typer.Option(..., help="Library name"), file: Path = typer.Option(..., "--file", "-f", help="Path to YAML library file", exists=True, readable=True), - config_type: str | None = typer.Option( - None, - "--type", - "-t", - help=( - "Config type: database or file. " - "Required when the library does not yet exist; defaults to the existing type on updates." - ), - ), namespace: str = typer.Option("", "--namespace", "-n", help="Library namespace"), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), ) -> None: """Create or update a discovery config library from a YAML file.""" client = get_client(profile) - existing = _find_by_name(client, name, namespace=namespace) - explicit = DiscoveryConfigType(config_type) if config_type is not None else None - - if explicit is not None: - lib_type = explicit - elif len(existing) == 1: - lib_type = existing[0].config_type - print_info(f"Updating existing {lib_type.value}-type library '{_label(name, namespace)}'.") - elif not existing: - abort( - f"No discovery config library named '{_label(name, namespace)}' exists.", - code=ErrorCode.NOT_FOUND, - hint="Pass --type file|database to create a new one.", - ) - else: - options = ", ".join(lib.config_type.value for lib in existing) - abort( - f"Multiple libraries named '{_label(name, namespace)}' ({options}).", - code=ErrorCode.AMBIGUOUS, - hint="Pass --type file|database to pick which one to update.", - ) - - yaml_content = file.read_text() - library = DiscoveryConfigLibrary(name=name, namespace=namespace, yaml=yaml_content, config_type=lib_type) - client.create_or_update_discovery_config_library(library) - print_success(f"Discovery config library '{_label(name, namespace)}' ({lib_type.value}) created/updated.") + library = DiscoveryConfigLibrary(name=name, namespace=namespace, yaml=file.read_text()) + try: + client.create_or_update_discovery_config_library(library) + except DataMasqueArgumentError: + abort(f"{file} contains no YAML content.", code=ErrorCode.INVALID_INPUT) + print_success(f"Discovery config library '{_label(name, namespace)}' created/updated.") @app.command("delete") def delete_library( name: str = typer.Argument(help="Library name to delete"), - config_type: str | None = typer.Option(None, "--type", "-t", help="Required when two libraries share a name"), namespace: str = typer.Option("", "--namespace", "-n", help="Library namespace"), force: bool = typer.Option(False, "--force", help="Force delete even if imported by discovery configs"), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), @@ -179,33 +109,39 @@ def delete_library( If the library is imported by any discovery configs, the server rejects the delete unless --force is passed. """ - client = get_client(profile) - wanted = DiscoveryConfigType(config_type) if config_type is not None else None - match = _pick_single(_find_by_name(client, name, wanted, namespace), name) label = _label(name, namespace) + client = get_client(profile) + if client.get_discovery_config_library_by_name(name, namespace) is None: + abort(f"Discovery config library '{label}' not found.", code=ErrorCode.NOT_FOUND) + if not is_confirmed: - typer.confirm(f"Delete discovery config library '{label}' ({match.config_type.value})?", abort=True) + typer.confirm(f"Delete discovery config library '{label}'?", abort=True) + + try: + client.delete_discovery_config_library_by_name_if_exists(name, namespace, force=force) + except DataMasqueApiError as exc: + abort_api_error( + f"Failed to delete discovery config library '{label}'", + exc, + conflict_hint="Re-run with --force to delete it and mark the dependent configs invalid.", + ) - assert match.id is not None - client.delete_discovery_config_library_by_id_if_exists(match.id, force=force) print_success(f"Discovery config library '{label}' deleted.") @app.command("validate") def validate_library( file: Path = typer.Option(..., "--file", "-f", help="Path to YAML library file", exists=True, readable=True), - config_type: str = typer.Option(..., "--type", "-t", help="Config type: database or file"), - namespace: str = typer.Option("", "--namespace", "-n", help="Library namespace"), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), ) -> None: """Validate a discovery config library YAML file against the DataMasque server.""" - yaml_content = file.read_text() - lib_type = DiscoveryConfigType(config_type) - client = get_client(profile) - library = DiscoveryConfigLibrary(name=file.stem, namespace=namespace, yaml=yaml_content, config_type=lib_type) - validated = client.validate_discovery_config_library(library) + library = DiscoveryConfigLibrary(name=file.stem, yaml=file.read_text()) + try: + validated = client.validate_discovery_config_library(library) + except DataMasqueArgumentError: + abort(f"{file} contains no YAML content.", code=ErrorCode.INVALID_INPUT) if validated.is_valid is ValidationStatus.invalid: abort( @@ -215,3 +151,33 @@ def validate_library( status = validated.is_valid.value if validated.is_valid else "unknown" print_success(f'Discovery config library "{file.name}" validation status: {status}') + + +@app.command("status") +def library_status( + name: str = typer.Argument(help="Library name"), + namespace: str = typer.Option("", "--namespace", "-n", help="Library namespace"), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), + is_json: bool = typer.Option(False, "--json", help="Output as JSON"), +) -> None: + """Show a discovery config library's validation status. + + Exits 0 when valid, 4 when invalid. + """ + client = get_client(profile) + lib = client.get_discovery_config_library_by_name(name, namespace) + + if lib is None: + abort(f"Discovery config library '{_label(name, namespace)}' not found.", code=ErrorCode.NOT_FOUND) + + status = lib.is_valid.value if lib.is_valid else "unknown" + data: dict[str, object] = { + "namespace": lib.namespace, + "name": lib.name, + "status": status, + "validation_error": lib.validation_error, + } + render_output(data, is_json=is_json, title=f"Discovery Config Library: {lib.name}") + + if lib.is_valid is ValidationStatus.invalid: + raise SystemExit(ExitCode.INVALID_INPUT) diff --git a/src/datamasque_cli/commands/discovery_configs.py b/src/datamasque_cli/commands/discovery_configs.py index 1f26939..863f06d 100644 --- a/src/datamasque_cli/commands/discovery_configs.py +++ b/src/datamasque_cli/commands/discovery_configs.py @@ -6,11 +6,21 @@ import typer from datamasque.client import DataMasqueClient +from datamasque.client.exceptions import DataMasqueArgumentError from datamasque.client.models.discovery_config import DiscoveryConfig, DiscoveryConfigType -from datamasque.client.models.status import ValidationStatus +from datamasque.client.models.status import ValidationErrorDetails, ValidationStatus from datamasque_cli.client import get_client -from datamasque_cli.output import ErrorCode, abort, print_info, print_success, render_output +from datamasque_cli.output import ( + ErrorCode, + ExitCode, + abort, + abort_if_async_validation, + abort_if_invalid, + print_info, + print_success, + render_output, +) app = typer.Typer(help="Manage discovery configs (configurable discovery).", no_args_is_help=True) @@ -27,8 +37,8 @@ def _find_by_name( return matches -def _pick_single(matches: list[DiscoveryConfig], name: str) -> DiscoveryConfig: - """Return the sole match or abort with a disambiguation message.""" +def _collapse_to_one_or_abort(matches: list[DiscoveryConfig], name: str) -> DiscoveryConfig: + """Return the single discovery config matching `name`, or abort asking for `--type`.""" if not matches: abort(f"Discovery config '{name}' not found.", code=ErrorCode.NOT_FOUND) if len(matches) > 1: @@ -43,7 +53,9 @@ def _pick_single(matches: list[DiscoveryConfig], name: str) -> DiscoveryConfig: @app.command("list") def list_configs( - config_type: str | None = typer.Option(None, "--type", "-t", help="Filter by type: database or file"), + config_type: DiscoveryConfigType | None = typer.Option( + None, "--type", "-t", help="Filter by type: database or file" + ), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), is_json: bool = typer.Option(False, "--json", help="Output as JSON"), ) -> None: @@ -52,8 +64,7 @@ def list_configs( configs = client.list_discovery_configs() if config_type is not None: - wanted = DiscoveryConfigType(config_type) - configs = [c for c in configs if c.config_type is wanted] + configs = [c for c in configs if c.config_type is config_type] data = [ { @@ -71,15 +82,16 @@ def list_configs( @app.command("get") def get_config( name: str = typer.Argument(help="Discovery config name"), - config_type: str | None = typer.Option(None, "--type", "-t", help="Required when two configs share a name"), + config_type: DiscoveryConfigType | None = typer.Option( + None, "--type", "-t", help="Required when two configs share a name" + ), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), is_yaml: bool = typer.Option(False, "--yaml", help="Output raw YAML content only"), is_json: bool = typer.Option(False, "--json", help="Output as JSON"), ) -> None: """Show a discovery config's details or YAML content.""" client = get_client(profile) - wanted = DiscoveryConfigType(config_type) if config_type is not None else None - match = _pick_single(_find_by_name(client, name, wanted), name) + match = _collapse_to_one_or_abort(_find_by_name(client, name, config_type), name) assert match.id is not None full = client.get_discovery_config(match.id) @@ -101,20 +113,21 @@ def get_config( @app.command("defaults") def config_defaults( - config_type: str = typer.Option("database", "--type", "-t", help="Config type: database or file"), + config_type: DiscoveryConfigType = typer.Option( + DiscoveryConfigType.database, "--type", "-t", help="Config type: database or file" + ), output: Path | None = typer.Option(None, "--output", "-o", help="Write YAML to this path"), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), ) -> None: """Print the server's built-in default discovery config as YAML.""" client = get_client(profile) - wanted = DiscoveryConfigType(config_type) # `get_default_discovery_config_yaml` takes no config type, so call `make_request` to pass one. - response = client.make_request("GET", "/api/discovery/configs/defaults/", params={"config_type": wanted.value}) + response = client.make_request("GET", "/api/discovery/configs/defaults/", params={"config_type": config_type.value}) yaml_content = response.content.decode("utf-8") if output is not None: output.write_text(yaml_content) - print_success(f"Default {wanted.value} discovery config written to {output}") + print_success(f"Default {config_type.value} discovery config written to {output}") return typer.echo(yaml_content) @@ -124,7 +137,7 @@ def config_defaults( def create_config( name: str = typer.Option(..., help="Discovery config name"), file: Path = typer.Option(..., "--file", "-f", help="Path to YAML config file", exists=True, readable=True), - config_type: str | None = typer.Option( + config_type: DiscoveryConfigType | None = typer.Option( None, "--type", "-t", @@ -142,10 +155,9 @@ def create_config( """ client = get_client(profile) existing = _find_by_name(client, name) - explicit = DiscoveryConfigType(config_type) if config_type is not None else None - if explicit is not None: - cfg_type = explicit + if config_type is not None: + cfg_type = config_type elif len(existing) == 1: cfg_type = existing[0].config_type print_info(f"Updating existing {cfg_type.value}-type discovery config '{name}'.") @@ -165,21 +177,25 @@ def create_config( yaml_content = file.read_text() config = DiscoveryConfig(name=name, yaml=yaml_content, config_type=cfg_type) - client.create_or_update_discovery_config(config) + try: + client.create_or_update_discovery_config(config) + except DataMasqueArgumentError: + abort(f"{file} contains no YAML content.", code=ErrorCode.INVALID_INPUT) print_success(f"Discovery config '{name}' ({cfg_type.value}) created/updated.") @app.command("delete") def delete_config( name: str = typer.Argument(help="Discovery config name to delete"), - config_type: str | None = typer.Option(None, "--type", "-t", help="Required when two configs share a name"), + config_type: DiscoveryConfigType | None = typer.Option( + None, "--type", "-t", help="Required when two configs share a name" + ), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), is_confirmed: bool = typer.Option(False, "--yes", "-y", help="Skip confirmation"), ) -> None: """Delete a discovery config by name.""" client = get_client(profile) - wanted = DiscoveryConfigType(config_type) if config_type is not None else None - match = _pick_single(_find_by_name(client, name, wanted), name) + match = _collapse_to_one_or_abort(_find_by_name(client, name, config_type), name) if not is_confirmed: typer.confirm(f"Delete discovery config '{name}' ({match.config_type.value})?", abort=True) @@ -192,22 +208,60 @@ def delete_config( @app.command("validate") def validate_config( file: Path = typer.Option(..., "--file", "-f", help="Path to YAML config file", exists=True, readable=True), - config_type: str = typer.Option(..., "--type", "-t", help="Config type: database or file"), + config_type: DiscoveryConfigType = typer.Option(..., "--type", "-t", help="Config type: database or file"), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), ) -> None: - """Validate a discovery config YAML file against the DataMasque server.""" + """Validate a discovery config YAML file against the DataMasque server. + + Note that configs over 60 KB validate asynchronously and cannot be validated here. + """ yaml_content = file.read_text() - cfg_type = DiscoveryConfigType(config_type) + abort_if_async_validation( + yaml_content, + subject=f'Discovery config "{file.name}"', + create_command=f"dm discover configs create --name --type {config_type.value} -f {file}", + status_command="dm discover configs status ", + ) client = get_client(profile) - config = DiscoveryConfig(name=file.stem, yaml=yaml_content, config_type=cfg_type) - validated = client.validate_discovery_config(config) + config = DiscoveryConfig(name=file.stem, yaml=yaml_content, config_type=config_type) + try: + validated = client.validate_discovery_config(config) + except DataMasqueArgumentError: + abort(f"{file} contains no YAML content.", code=ErrorCode.INVALID_INPUT) - if validated.is_valid is ValidationStatus.invalid: - abort( - f'Discovery config "{file.name}" is invalid: {validated.validation_error}', - code=ErrorCode.INVALID_INPUT, - ) + errors = validated.validation_error_details + if not errors and validated.validation_error: + errors = [ValidationErrorDetails(message=validated.validation_error)] + abort_if_invalid(f'Discovery config "{file.name}"', validated.is_valid, errors) status = validated.is_valid.value if validated.is_valid else "unknown" print_success(f'Discovery config "{file.name}" validation status: {status}') + + +@app.command("status") +def config_status( + name: str = typer.Argument(help="Discovery config name"), + config_type: DiscoveryConfigType | None = typer.Option( + None, "--type", "-t", help="Required when two configs share a name" + ), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), + is_json: bool = typer.Option(False, "--json", help="Output as JSON"), +) -> None: + """Show a discovery config's validation status.""" + client = get_client(profile) + match = _collapse_to_one_or_abort(_find_by_name(client, name, config_type), name) + + status = match.is_valid.value if match.is_valid else "unknown" + data: dict[str, object] = { + "name": match.name, + "type": match.config_type.value, + "status": status, + "validation_error": match.validation_error, + } + render_output(data, is_json=is_json, title=f"Discovery Config: {match.name}") + + if match.is_valid is ValidationStatus.in_progress: + print_info("Still validating — run this command again shortly.") + if match.is_valid is ValidationStatus.invalid: + raise SystemExit(ExitCode.INVALID_INPUT) diff --git a/src/datamasque_cli/commands/ruleset_libraries.py b/src/datamasque_cli/commands/ruleset_libraries.py index b34e960..56e5cda 100644 --- a/src/datamasque_cli/commands/ruleset_libraries.py +++ b/src/datamasque_cli/commands/ruleset_libraries.py @@ -5,10 +5,22 @@ from pathlib import Path import typer +from datamasque.client.exceptions import DataMasqueApiError from datamasque.client.models.ruleset_library import RulesetLibrary +from datamasque.client.models.status import ValidationStatus from datamasque_cli.client import get_client -from datamasque_cli.output import ErrorCode, abort, abort_if_invalid, print_success, render_output +from datamasque_cli.output import ( + ErrorCode, + ExitCode, + abort, + abort_api_error, + abort_if_invalid, + print_info, + print_success, + render_output, + should_emit_json, +) app = typer.Typer(help="Manage ruleset libraries.", no_args_is_help=True) @@ -100,7 +112,15 @@ def delete_library( if not is_confirmed: typer.confirm(f"Delete library '{label}'?", abort=True) - client.delete_ruleset_library_by_name_if_exists(name, namespace, force=force) + try: + client.delete_ruleset_library_by_name_if_exists(name, namespace, force=force) + except DataMasqueApiError as exc: + abort_api_error( + f"Failed to delete library '{label}'", + exc, + conflict_hint="Re-run with --force to delete it and flag the dependent rulesets for revalidation.", + ) + print_success(f"Library '{label}' deleted.") @@ -129,6 +149,40 @@ def validate_library( print_success(f"Library '{label}' validation status: {status}") +@app.command("status") +def library_status( + name: str = typer.Argument(help="Library name"), + namespace: str = typer.Option("", "--namespace", "-n", help="Library namespace"), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), + is_json: bool = typer.Option(False, "--json", help="Output as JSON"), +) -> None: + """Show a ruleset library's validation status.""" + client = get_client(profile) + lib = client.get_ruleset_library_by_name(name, namespace) + + if lib is None: + label = f"{namespace}/{name}" if namespace else name + abort(f"Library '{label}' not found.", code=ErrorCode.NOT_FOUND) + + status = lib.is_valid.value if lib.is_valid else "unknown" + errors = lib.validation_errors or [] + data: dict[str, object] = { + "namespace": lib.namespace, + "name": lib.name, + "status": status, + } + if should_emit_json(is_json): + data["errors"] = [error.model_dump(mode="json") for error in errors] + else: + data["errors"] = "; ".join(error.message for error in errors) + render_output(data, is_json=is_json, title=f"Library: {lib.name}") + + if lib.is_valid is ValidationStatus.in_progress: + print_info("Still validating — run this command again shortly.") + if lib.is_valid is ValidationStatus.invalid: + raise SystemExit(ExitCode.INVALID_INPUT) + + @app.command("usage") def library_usage( name: str = typer.Argument(help="Library name"), diff --git a/src/datamasque_cli/commands/rulesets.py b/src/datamasque_cli/commands/rulesets.py index df38f7f..ce6dc8b 100644 --- a/src/datamasque_cli/commands/rulesets.py +++ b/src/datamasque_cli/commands/rulesets.py @@ -10,18 +10,24 @@ from datamasque.client import DataMasqueClient from datamasque.client.base import UploadFile from datamasque.client.exceptions import DataMasqueApiError +from datamasque.client.models.discovery import FileRulesetGenerationRequest, RulesetGenerationRequest from datamasque.client.models.ruleset import Ruleset, RulesetType +from datamasque.client.models.status import ValidationStatus +from pydantic import ValidationError from datamasque_cli.client import get_client from datamasque_cli.output import ( ErrorCode, + ExitCode, abort, + abort_if_async_validation, abort_if_invalid, print_error, print_info, print_success, print_warning, render_output, + should_emit_json, ) app = typer.Typer(help="Manage masking rulesets.", no_args_is_help=True) @@ -39,8 +45,8 @@ def _find_by_name( return matches -def _pick_single(matches: list[Ruleset], name: str) -> Ruleset: - """Return the sole match or abort with a disambiguation message.""" +def _collapse_to_one_or_abort(matches: list[Ruleset], name: str) -> Ruleset: + """Return the single ruleset matching `name`, or abort asking for `--type`.""" if not matches: abort(f"Ruleset '{name}' not found.", code=ErrorCode.NOT_FOUND) if len(matches) > 1: @@ -55,7 +61,7 @@ def _pick_single(matches: list[Ruleset], name: str) -> Ruleset: @app.command("list") def list_rulesets( - ruleset_type: str | None = typer.Option(None, "--type", "-t", help="Filter by type: database or file"), + ruleset_type: RulesetType | None = typer.Option(None, "--type", "-t", help="Filter by type: database or file"), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), is_json: bool = typer.Option(False, "--json", help="Output as JSON"), ) -> None: @@ -64,8 +70,7 @@ def list_rulesets( rulesets = client.list_rulesets() if ruleset_type is not None: - wanted = RulesetType(ruleset_type) - rulesets = [rs for rs in rulesets if rs.ruleset_type == wanted] + rulesets = [rs for rs in rulesets if rs.ruleset_type == ruleset_type] data = [ { @@ -82,15 +87,16 @@ def list_rulesets( @app.command("get") def get_ruleset( name: str = typer.Argument(help="Ruleset name"), - ruleset_type: str | None = typer.Option(None, "--type", "-t", help="Required when two rulesets share a name"), + ruleset_type: RulesetType | None = typer.Option( + None, "--type", "-t", help="Required when two rulesets share a name" + ), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), is_yaml: bool = typer.Option(False, "--yaml", help="Output raw YAML content only"), is_json: bool = typer.Option(False, "--json", help="Output as JSON"), ) -> None: """Show a ruleset's details or YAML content.""" client = get_client(profile) - wanted = RulesetType(ruleset_type) if ruleset_type is not None else None - match = _pick_single(_find_by_name(client, name, wanted), name) + match = _collapse_to_one_or_abort(_find_by_name(client, name, ruleset_type), name) # `list_rulesets` omits the YAML body for performance; fetch the single ruleset # to populate `yaml` via the Ruleset pydantic model's `config_yaml` alias. @@ -114,7 +120,7 @@ def get_ruleset( def create_ruleset( name: str = typer.Option(..., help="Ruleset name"), file: Path = typer.Option(..., "--file", "-f", help="Path to YAML ruleset file", exists=True, readable=True), - ruleset_type: str | None = typer.Option( + ruleset_type: RulesetType | None = typer.Option( None, "--type", "-t", @@ -133,10 +139,9 @@ def create_ruleset( """ client = get_client(profile) existing = _find_by_name(client, name) - explicit = RulesetType(ruleset_type) if ruleset_type is not None else None - if explicit is not None: - rs_type = explicit + if ruleset_type is not None: + rs_type = ruleset_type elif len(existing) == 1: rs_type = existing[0].ruleset_type print_info(f"Updating existing {rs_type.value}-type ruleset '{name}'.") @@ -163,14 +168,15 @@ def create_ruleset( @app.command("delete") def delete_ruleset( name: str = typer.Argument(help="Ruleset name to delete"), - ruleset_type: str | None = typer.Option(None, "--type", "-t", help="Required when two rulesets share a name"), + ruleset_type: RulesetType | None = typer.Option( + None, "--type", "-t", help="Required when two rulesets share a name" + ), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), is_confirmed: bool = typer.Option(False, "--yes", "-y", help="Skip confirmation"), ) -> None: """Delete a ruleset by name.""" client = get_client(profile) - wanted = RulesetType(ruleset_type) if ruleset_type is not None else None - match = _pick_single(_find_by_name(client, name, wanted), name) + match = _collapse_to_one_or_abort(_find_by_name(client, name, ruleset_type), name) if not is_confirmed: typer.confirm(f"Delete ruleset '{name}' ({match.ruleset_type.value})?", abort=True) @@ -183,7 +189,7 @@ def delete_ruleset( @app.command("validate") def validate_ruleset( file: Path = typer.Option(..., "--file", "-f", help="Path to YAML ruleset file", exists=True, readable=True), - ruleset_type: str = typer.Option( + ruleset_type: RulesetType = typer.Option( ..., "--type", "-t", @@ -195,14 +201,21 @@ def validate_ruleset( Creates a temporary ruleset to trigger server-side validation, then deletes it. Reports any validation errors. + + Note that rulesets over 60 KiB validate asynchronously and cannot be validated here. """ yaml_content = file.read_text() - rs_type = RulesetType(ruleset_type) + abort_if_async_validation( + yaml_content, + subject=f"Ruleset '{file.name}'", + create_command=f"dm rulesets create --name --type {ruleset_type.value} -f {file}", + status_command="dm rulesets status ", + ) # `uuid` guards against collisions between concurrent `validate` runs. temp_name = f"__dm_cli_validate_{uuid.uuid4().hex}" client = get_client(profile) - ruleset = Ruleset(name=temp_name, yaml=yaml_content, ruleset_type=rs_type) + ruleset = Ruleset(name=temp_name, yaml=yaml_content, ruleset_type=ruleset_type) try: created = client.create_or_update_ruleset(ruleset) @@ -213,8 +226,9 @@ def validate_ruleset( # `try/finally` so a Ctrl-C or unexpected exception between create and # delete still cleans up the temp ruleset on the server. try: - abort_if_invalid(f"Ruleset '{file.name}' ({rs_type.value})", created.is_valid, created.validation_errors) - print_success(f"Ruleset '{file.name}' ({rs_type.value}) is valid.") + abort_if_invalid(f"Ruleset '{file.name}' ({ruleset_type.value})", created.is_valid, created.validation_errors) + status = created.is_valid.value if created.is_valid else "unknown" + print_success(f"Ruleset '{file.name}' ({ruleset_type.value}) validation status: {status}") finally: if created.id is not None: try: @@ -297,6 +311,38 @@ def import_bundle( render_output(summary, is_json=False, title="Import summary") +@app.command("status") +def ruleset_status( + name: str = typer.Argument(help="Ruleset name"), + ruleset_type: RulesetType | None = typer.Option( + None, "--type", "-t", help="Required when two rulesets share a name" + ), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), + is_json: bool = typer.Option(False, "--json", help="Output as JSON"), +) -> None: + """Show a ruleset's validation status.""" + client = get_client(profile) + match = _collapse_to_one_or_abort(_find_by_name(client, name, ruleset_type), name) + + status = match.is_valid.value if match.is_valid else "unknown" + errors = match.validation_errors or [] + data: dict[str, object] = { + "name": match.name, + "type": match.ruleset_type.value, + "status": status, + } + if should_emit_json(is_json): + data["errors"] = [error.model_dump(mode="json") for error in errors] + else: + data["errors"] = "; ".join(error.message for error in errors) + render_output(data, is_json=is_json, title=f"Ruleset: {match.name}") + + if match.is_valid is ValidationStatus.in_progress: + print_info("Still validating — run this command again shortly.") + if match.is_valid is ValidationStatus.invalid: + raise SystemExit(ExitCode.INVALID_INPUT) + + @app.command("generate") def generate_ruleset( request_file: Path = typer.Option( @@ -311,12 +357,15 @@ def generate_ruleset( The request JSON format matches the DataMasque API's /api/generate-ruleset/v2/ endpoint. """ client = get_client(profile) - generation_request = json.loads(request_file.read_text()) + raw_request = json.loads(request_file.read_text()) - if is_file_ruleset: - yaml_content = client.generate_file_ruleset(generation_request) - else: - yaml_content = client.generate_ruleset(generation_request) + try: + if is_file_ruleset: + yaml_content = client.generate_file_ruleset(FileRulesetGenerationRequest.model_validate(raw_request)) + else: + yaml_content = client.generate_ruleset(RulesetGenerationRequest.model_validate(raw_request)) + except ValidationError as exc: + abort(f"Invalid generation request in {request_file}: {exc}", code=ErrorCode.INVALID_INPUT) if output is not None: output.write_text(yaml_content) diff --git a/src/datamasque_cli/commands/system.py b/src/datamasque_cli/commands/system.py index 6e39e85..6b61557 100644 --- a/src/datamasque_cli/commands/system.py +++ b/src/datamasque_cli/commands/system.py @@ -96,7 +96,14 @@ def import_config( ) -> None: """Deprecated alias for `dm rulesets import-bundle`.""" print_warning("`dm system import` is deprecated; use `dm rulesets import-bundle` instead.") - import_bundle(file=file, profile=profile, is_confirmed=is_confirmed) + import_bundle( + file=file, + overwrite_rulesets=False, + overwrite_libraries=False, + overwrite_seeds=False, + profile=profile, + is_confirmed=is_confirmed, + ) @app.command("upload-licence") diff --git a/src/datamasque_cli/main.py b/src/datamasque_cli/main.py index 8efdbe3..c2f3586 100644 --- a/src/datamasque_cli/main.py +++ b/src/datamasque_cli/main.py @@ -9,10 +9,9 @@ from __future__ import annotations +from collections.abc import Sequence from importlib.metadata import version as pkg_version -from typing import Any -import click import typer from rich.console import Console from typer.main import get_command @@ -31,6 +30,7 @@ users, ) from datamasque_cli.output import print_json, should_emit_json, stdout_console +from datamasque_cli.protocols import ArgumentEntry, CommandEntry, CompactEntry, Group, OptionEntry app = typer.Typer( name="dm", @@ -60,42 +60,38 @@ def version() -> None: typer.echo(f"v{pkg_version('datamasque-cli')}") -def _walk_commands(group: click.Group, path_prefix: str = "") -> list[dict[str, Any]]: - """Walk a click group recursively and yield one entry per leaf command. - - Each entry has `path` (space-separated), `help` (first sentence of the - docstring), and `options` (a flat list of flags + arguments). - """ - items: list[dict[str, Any]] = [] +def walk_commands(group: Group, path_prefix: str = "") -> list[CommandEntry]: + """Walk a command group recursively and yield one entry per leaf command.""" + items: list[CommandEntry] = [] for name, cmd in sorted(group.commands.items()): if cmd.hidden: continue path = f"{path_prefix} {name}".strip() - if isinstance(cmd, click.Group): - items.extend(_walk_commands(cmd, path)) + if isinstance(cmd, Group): + items.extend(walk_commands(cmd, path)) continue - options: list[dict[str, Any]] = [] + options: list[OptionEntry | ArgumentEntry] = [] for param in cmd.params: - if isinstance(param, click.Option): + if param.param_type_name == "option": options.append( - { - "flags": list(param.opts), - "help": param.help or "", - "required": param.required, - "is_flag": param.is_flag, - } + OptionEntry( + flags=list(param.opts), + help=param.help or "", + required=param.required, + is_flag=param.is_flag, + ) ) - elif isinstance(param, click.Argument): + elif param.param_type_name == "argument": options.append( - { - "name": param.name, - "required": param.required, - "is_argument": True, - } + ArgumentEntry( + name=param.name, + required=param.required, + is_argument=True, + ) ) # Take only the first paragraph of help text — keeps the catalog dense. help_text = (cmd.help or "").strip().split("\n\n", 1)[0].replace("\n", " ") - items.append({"path": path, "help": help_text, "options": options}) + items.append(CommandEntry(path=path, help=help_text, options=options)) return items @@ -111,13 +107,13 @@ def catalog( Designed to be called once at session start so an agent can introspect every available subcommand without parsing per-command --help screens. """ - click_app = get_command(app) - if not isinstance(click_app, click.Group): - # Defensive — a Typer app with subcommands always materialises as a Group. - raise RuntimeError("Root command is not a click Group; cannot walk catalog.") - items = _walk_commands(click_app) - if is_compact: - items = [{"path": item["path"], "help": item["help"]} for item in items] + root = get_command(app) + if not isinstance(root, Group): + raise RuntimeError("Root command is not a command group; cannot walk catalog.") + commands = walk_commands(root) + items: Sequence[CompactEntry] = ( + [CompactEntry(path=command["path"], help=command["help"]) for command in commands] if is_compact else commands + ) if should_emit_json(is_json): print_json({"commands": items}) diff --git a/src/datamasque_cli/output.py b/src/datamasque_cli/output.py index 21567dd..0c0318e 100644 --- a/src/datamasque_cli/output.py +++ b/src/datamasque_cli/output.py @@ -15,9 +15,11 @@ import os import sys from enum import IntEnum, StrEnum +from http import HTTPStatus from typing import Any, NoReturn import typer +from datamasque.client.exceptions import DataMasqueApiError from datamasque.client.models.status import ValidationErrorDetails, ValidationStatus from rich.console import Console from rich.table import Table @@ -261,6 +263,20 @@ def abort(message: str, *, code: ErrorCode = ErrorCode.ERROR, hint: str | None = raise SystemExit(EXIT_CODE_BY_ERROR[code]) +def abort_api_error(prefix: str, exc: DataMasqueApiError, *, conflict_hint: str | None = None) -> NoReturn: + """Abort with the admin server's own explanation of a failed request.""" + try: + body = exc.response.json() + except ValueError: + body = None + detail = body.get("detail") if isinstance(body, dict) else None + reason = detail if isinstance(detail, str) else str(exc) + + if exc.response.status_code == HTTPStatus.CONFLICT: + abort(reason, code=ErrorCode.CONFLICT, hint=conflict_hint) + abort(f"{prefix}: {reason}", code=ErrorCode.ERROR) + + def abort_if_invalid(subject: str, is_valid: ValidationStatus | None, errors: list[ValidationErrorDetails]) -> None: """Print each server-side validation error for `subject` and exit, if it failed validation.""" if is_valid is not ValidationStatus.invalid and not errors: @@ -269,3 +285,17 @@ def abort_if_invalid(subject: str, is_valid: ValidationStatus | None, errors: li location = f" (line {error.line_number})" if error.line_number is not None else "" print_error(f"{error.message}{location}") abort(f"{subject} is invalid.", code=ErrorCode.INVALID_INPUT) + + +def abort_if_async_validation(yaml_content: str, *, subject: str, create_command: str, status_command: str) -> None: + """Abort when `yaml_content` is too large for the server to validate synchronously.""" + kib = 1024 + max_sync_kib = 60 + size = len(yaml_content.encode("utf-8")) + if size < max_sync_kib * kib: + return + abort( + f"{subject} is {size // kib} KiB; validation for YAML of {max_sync_kib} KiB or larger runs asynchronously.", + code=ErrorCode.INVALID_INPUT, + hint=f"Create it with `{create_command}`, then check `{status_command}` until validation finishes.", + ) diff --git a/src/datamasque_cli/protocols.py b/src/datamasque_cli/protocols.py new file mode 100644 index 0000000..3687cd2 --- /dev/null +++ b/src/datamasque_cli/protocols.py @@ -0,0 +1,59 @@ +from __future__ import annotations + +from typing import Protocol, TypedDict, runtime_checkable + + +class Param(Protocol): + """The parameter attributes the catalog reads.""" + + name: str | None + param_type_name: str + opts: list[str] + required: bool + help: str | None + is_flag: bool + + +class Command(Protocol): + """The command attributes the catalog reads.""" + + hidden: bool + help: str | None + params: list[Param] + + +@runtime_checkable +class Group(Command, Protocol): + """A command that holds subcommands.""" + + commands: dict[str, Command] + + +class OptionEntry(TypedDict): + """A catalog entry for one of a command's options.""" + + flags: list[str] + help: str + required: bool + is_flag: bool + + +class ArgumentEntry(TypedDict): + """A catalog entry for one of a command's positional arguments.""" + + name: str | None + required: bool + is_argument: bool + + +class CompactEntry(TypedDict): + """A catalog entry as `--compact` emits it, with options dropped.""" + + path: str + help: str + + +class CommandEntry(CompactEntry): + """A catalog entry for a single leaf command.""" + + options: list[OptionEntry | ArgumentEntry] diff --git a/tests/commands/test_connections.py b/tests/commands/test_connections.py index e0c001b..af3592d 100644 --- a/tests/commands/test_connections.py +++ b/tests/commands/test_connections.py @@ -4,6 +4,7 @@ from unittest.mock import MagicMock, patch import pytest +from datamasque.client.exceptions import DataMasqueApiError from datamasque.client.models.connection import ( DatabaseConnectionConfig, DatabaseType, @@ -15,6 +16,7 @@ from datamasque_cli.commands.connections import _format_role from datamasque_cli.main import app +from datamasque_cli.output import ExitCode MODULE = "datamasque_cli.commands.connections" @@ -273,6 +275,22 @@ def test_test_connection_posts_to_test_endpoint( mock_client.make_request.assert_called_once_with("POST", "/api/connections/1/test/", data={}) +@patch(f"{MODULE}.get_client") +def test_test_connection_reports_unreachable_target( + mock_get_client: MagicMock, mock_client: MagicMock, runner: CliRunner +) -> None: + mock_get_client.return_value = mock_client + mock_response = MagicMock(status_code=400) + mock_response.json.return_value = {"detail": 'DNS lookup for "postgres-dev" failed.'} + mock_client.make_request.side_effect = DataMasqueApiError("boom", response=mock_response) + + result = runner.invoke(app, ["connections", "test", "my_conn"]) + + assert result.exit_code == ExitCode.ERROR + assert 'DNS lookup for "postgres-dev" failed.' in " ".join(result.stderr.split()) + assert "Traceback" not in result.stderr + + @patch(f"{MODULE}.get_client") def test_test_connection_aborts_when_missing( mock_get_client: MagicMock, mock_client: MagicMock, runner: CliRunner @@ -302,7 +320,7 @@ def test_update_connection_patches_changed_fields( mock_client.make_request.assert_called_once_with( "PATCH", "/api/connections/1/", - data={"host": "db2.example.com", "password": "new-pw"}, + data={"host": "db2.example.com", "dbpassword": "new-pw"}, ) diff --git a/tests/commands/test_discovery.py b/tests/commands/test_discovery.py index 7142fff..890991b 100644 --- a/tests/commands/test_discovery.py +++ b/tests/commands/test_discovery.py @@ -5,6 +5,7 @@ from types import SimpleNamespace from unittest.mock import MagicMock, patch +from datamasque.client.exceptions import DataMasqueApiError from datamasque.client.models.discovery import ( FileDiscoveryFile, FileDiscoveryLocatorResult, @@ -177,6 +178,35 @@ def test_schema_starts_discovery_run_and_points_at_results(mock_get_client: Magi assert "dm discover schema-results 99" in result.stderr +@patch(f"{MODULE}.get_client") +def test_schema_emits_run_id_as_json(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_connections.return_value = [SimpleNamespace(id="abc-123", name="my_db", mask_type="database")] + client.start_schema_discovery_run.return_value = 99 + + result = runner.invoke(app, ["discover", "schema", "my_db", "--json"]) + + assert result.exit_code == 0 + assert json.loads(result.stdout) == {"id": 99, "status": "queued"} + + +@patch(f"{MODULE}.get_client") +def test_file_start_failure_reports_server_detail(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_connections.return_value = [SimpleNamespace(id="fs-1", name="my_files", mask_type="file")] + response = MagicMock(status_code=400) + response.json.return_value = {"detail": "Simultaneous runs on the same connection are not allowed."} + client.start_file_data_discovery_run.side_effect = DataMasqueApiError("boom", response=response) + + result = runner.invoke(app, ["discover", "file", "my_files"]) + + assert result.exit_code == ExitCode.ERROR + assert "Simultaneous runs on the same connection are not allowed." in " ".join(result.stderr.split()) + assert "Traceback" not in result.stderr + + @patch(f"{MODULE}.get_client") def test_schema_results_lists_with_flattened_rows(mock_get_client: MagicMock, runner: CliRunner) -> None: client = MagicMock() @@ -369,6 +399,19 @@ def test_file_with_config_runs_from_saved_config(mock_get_client: MagicMock, run assert request.discovery_config == "cfg-3" +@patch(f"{MODULE}.get_client") +def test_file_emits_run_id_as_json(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_connections.return_value = [SimpleNamespace(id="fs-1", name="my_files", mask_type="file")] + client.start_file_data_discovery_run.return_value = 88 + + result = runner.invoke(app, ["discover", "file", "my_files", "--json"]) + + assert result.exit_code == 0 + assert json.loads(result.stdout) == {"id": 88, "status": "queued"} + + @patch(f"{MODULE}.get_client") def test_config_snapshot_writes_to_output(mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path) -> None: client = MagicMock() diff --git a/tests/commands/test_discovery_config_libraries.py b/tests/commands/test_discovery_config_libraries.py index ebb65f5..dd6ce56 100644 --- a/tests/commands/test_discovery_config_libraries.py +++ b/tests/commands/test_discovery_config_libraries.py @@ -1,9 +1,10 @@ from __future__ import annotations +from http import HTTPStatus from types import SimpleNamespace from unittest.mock import MagicMock, patch -from datamasque.client.models.discovery_config import DiscoveryConfigType +from datamasque.client.exceptions import DataMasqueApiError from datamasque.client.models.status import ValidationStatus from typer.testing import CliRunner @@ -15,19 +16,19 @@ def _library( name: str, - config_type: DiscoveryConfigType = DiscoveryConfigType.database, namespace: str = "", library_id: str = "lib-uuid", is_valid: ValidationStatus | None = ValidationStatus.valid, + usage_count: int = 0, yaml: str | None = None, ) -> SimpleNamespace: return SimpleNamespace( id=library_id, name=name, namespace=namespace, - config_type=config_type, is_valid=is_valid, validation_error=None, + usage_count=usage_count, created=None, modified=None, yaml=yaml, @@ -35,11 +36,11 @@ def _library( @patch(f"{MODULE}.get_client") -def test_list_shows_namespace_and_type(mock_get_client: MagicMock, runner: CliRunner) -> None: +def test_list_shows_namespace_and_usage(mock_get_client: MagicMock, runner: CliRunner) -> None: client = MagicMock() mock_get_client.return_value = client client.list_discovery_config_libraries.return_value = [ - _library("finance", namespace="org"), + _library("finance", namespace="org", usage_count=3), ] result = runner.invoke(app, ["discover", "libraries", "list", "--json"]) @@ -47,61 +48,123 @@ def test_list_shows_namespace_and_type(mock_get_client: MagicMock, runner: CliRu assert result.exit_code == 0 assert '"finance"' in result.stdout assert '"org"' in result.stdout + assert '"used_by": 3' in result.stdout @patch(f"{MODULE}.get_client") def test_get_yaml_fetches_full_library(mock_get_client: MagicMock, runner: CliRunner) -> None: client = MagicMock() mock_get_client.return_value = client - client.list_discovery_config_libraries.return_value = [_library("finance", namespace="org")] - client.get_discovery_config_library.return_value = _library("finance", namespace="org", yaml="labels: []\n") + client.get_discovery_config_library_by_name.return_value = _library("finance", namespace="org", yaml="labels: []\n") result = runner.invoke(app, ["discover", "libraries", "get", "finance", "--namespace", "org", "--yaml"]) assert result.exit_code == 0 assert "labels: []" in result.stdout - client.get_discovery_config_library.assert_called_once_with("lib-uuid") + client.get_discovery_config_library_by_name.assert_called_once_with("finance", "org") @patch(f"{MODULE}.get_client") def test_get_namespace_scopes_lookup(mock_get_client: MagicMock, runner: CliRunner) -> None: client = MagicMock() mock_get_client.return_value = client - client.list_discovery_config_libraries.return_value = [_library("finance", namespace="org")] + client.get_discovery_config_library_by_name.return_value = None result = runner.invoke(app, ["discover", "libraries", "get", "finance"]) assert result.exit_code == ExitCode.NOT_FOUND - client.get_discovery_config_library.assert_not_called() + client.get_discovery_config_library_by_name.assert_called_once_with("finance", "") @patch(f"{MODULE}.get_client") -def test_create_new_requires_type(mock_get_client: MagicMock, runner: CliRunner, tmp_path) -> None: +def test_create_posts_library(mock_get_client: MagicMock, runner: CliRunner, tmp_path) -> None: client = MagicMock() mock_get_client.return_value = client - client.list_discovery_config_libraries.return_value = [] lib = tmp_path / "lib.yaml" lib.write_text("labels: []\n") result = runner.invoke( app, - ["discover", "libraries", "create", "--name", "finance", "-n", "org", "-f", str(lib), "--type", "database"], + ["discover", "libraries", "create", "--name", "finance", "-n", "org", "-f", str(lib)], ) assert result.exit_code == 0 client.create_or_update_discovery_config_library.assert_called_once() + created = client.create_or_update_discovery_config_library.call_args.args[0] + assert created.name == "finance" + assert created.namespace == "org" @patch(f"{MODULE}.get_client") def test_delete_force_passes_through(mock_get_client: MagicMock, runner: CliRunner) -> None: client = MagicMock() mock_get_client.return_value = client - client.list_discovery_config_libraries.return_value = [_library("finance", namespace="org")] + client.get_discovery_config_library_by_name.return_value = _library("finance", namespace="org") result = runner.invoke(app, ["discover", "libraries", "delete", "finance", "-n", "org", "--force", "--yes"]) assert result.exit_code == 0 - client.delete_discovery_config_library_by_id_if_exists.assert_called_once_with("lib-uuid", force=True) + client.delete_discovery_config_library_by_name_if_exists.assert_called_once_with("finance", "org", force=True) + + +@patch(f"{MODULE}.get_client") +def test_delete_in_use_reports_server_reason(mock_get_client: MagicMock, runner: CliRunner) -> None: + """A 409 must surface the server's explanation, not a traceback. + + The server refuses to delete a library that active configs still import, and + its `detail` names the count. Without handling, the raised `DataMasqueApiError` + escapes as an unhandled exception and the user only sees a stack trace. + """ + client = MagicMock() + mock_get_client.return_value = client + client.get_discovery_config_library_by_name.return_value = _library("finance") + response = MagicMock() + response.status_code = HTTPStatus.CONFLICT + response.json.return_value = { + "detail": 'Cannot delete library "finance": used by 2 active config(s)', + "configs": [{"id": "cfg-1", "name": "employees"}], + } + client.delete_discovery_config_library_by_name_if_exists.side_effect = DataMasqueApiError( + "API request to https://dm/api/discovery/config-libraries/lib-uuid/ failed with status 409", + response=response, + ) + + result = runner.invoke(app, ["discover", "libraries", "delete", "finance", "--yes"]) + + assert result.exit_code == ExitCode.CONFLICT + assert "used by 2 active config(s)" in result.stderr + assert "--force" in result.stderr + + +@patch(f"{MODULE}.get_client") +def test_delete_other_api_error_is_generic_failure(mock_get_client: MagicMock, runner: CliRunner) -> None: + """Non-409 API failures still abort cleanly rather than raising.""" + client = MagicMock() + mock_get_client.return_value = client + client.get_discovery_config_library_by_name.return_value = _library("finance") + response = MagicMock() + response.status_code = HTTPStatus.INTERNAL_SERVER_ERROR + response.json.side_effect = ValueError("no body") + client.delete_discovery_config_library_by_name_if_exists.side_effect = DataMasqueApiError( + "API request failed with status 500", response=response + ) + + result = runner.invoke(app, ["discover", "libraries", "delete", "finance", "--yes"]) + + assert result.exit_code == ExitCode.ERROR + assert "Failed to delete discovery config library 'finance'" in result.stderr + + +@patch(f"{MODULE}.get_client") +def test_delete_missing_is_not_found(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.get_discovery_config_library_by_name.return_value = None + + result = runner.invoke(app, ["discover", "libraries", "delete", "finance", "--yes"]) + + assert result.exit_code == ExitCode.NOT_FOUND + client.delete_discovery_config_library_by_name_if_exists.assert_not_called() @patch(f"{MODULE}.get_client") @@ -114,7 +177,33 @@ def test_validate_invalid_exits_4(mock_get_client: MagicMock, runner: CliRunner, lib = tmp_path / "lib.yaml" lib.write_text("labels: []\n") - result = runner.invoke(app, ["discover", "libraries", "validate", "-f", str(lib), "--type", "database"]) + result = runner.invoke(app, ["discover", "libraries", "validate", "-f", str(lib)]) assert result.exit_code == ExitCode.INVALID_INPUT assert "duplicate label 'email'" in result.stderr + + +@patch(f"{MODULE}.get_client") +def test_status_valid_exits_0(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.get_discovery_config_library_by_name.return_value = _library("finance", namespace="org") + + result = runner.invoke(app, ["discover", "libraries", "status", "finance", "-n", "org", "--json"]) + + assert result.exit_code == 0 + assert '"status": "valid"' in result.stdout + + +@patch(f"{MODULE}.get_client") +def test_status_invalid_exits_4(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + library = _library("finance", is_valid=ValidationStatus.invalid) + library.validation_error = "duplicate label 'email'" + client.get_discovery_config_library_by_name.return_value = library + + result = runner.invoke(app, ["discover", "libraries", "status", "finance", "--json"]) + + assert result.exit_code == ExitCode.INVALID_INPUT + assert "duplicate label 'email'" in result.stdout diff --git a/tests/commands/test_discovery_configs.py b/tests/commands/test_discovery_configs.py index 5266ff4..78b44a1 100644 --- a/tests/commands/test_discovery_configs.py +++ b/tests/commands/test_discovery_configs.py @@ -32,6 +32,15 @@ def _config( ) +@patch(f"{MODULE}.get_client") +def test_unknown_type_is_rejected_before_any_request(mock_get_client: MagicMock, runner: CliRunner) -> None: + result = runner.invoke(app, ["discover", "configs", "list", "--type", "banana"]) + + assert result.exit_code == ExitCode.USAGE + assert "is not one of" in result.output + mock_get_client.assert_not_called() + + @patch(f"{MODULE}.get_client") def test_list_filters_by_type(mock_get_client: MagicMock, runner: CliRunner) -> None: client = MagicMock() @@ -170,7 +179,7 @@ def test_validate_reports_valid(mock_get_client: MagicMock, runner: CliRunner, t client = MagicMock() mock_get_client.return_value = client client.validate_discovery_config.return_value = SimpleNamespace( - is_valid=ValidationStatus.valid, validation_error=None + is_valid=ValidationStatus.valid, validation_error=None, validation_error_details=[] ) cfg = tmp_path / "cfg.yaml" cfg.write_text("labels: []\n") @@ -187,7 +196,7 @@ def test_validate_invalid_exits_4(mock_get_client: MagicMock, runner: CliRunner, client = MagicMock() mock_get_client.return_value = client client.validate_discovery_config.return_value = SimpleNamespace( - is_valid=ValidationStatus.invalid, validation_error="unknown label 'foo'" + is_valid=ValidationStatus.invalid, validation_error="unknown label 'foo'", validation_error_details=[] ) cfg = tmp_path / "cfg.yaml" cfg.write_text("labels: []\n") @@ -196,3 +205,54 @@ def test_validate_invalid_exits_4(mock_get_client: MagicMock, runner: CliRunner, assert result.exit_code == ExitCode.INVALID_INPUT assert "unknown label 'foo'" in result.stderr + + +@patch(f"{MODULE}.get_client") +def test_validate_oversize_aborts_before_any_request(mock_get_client: MagicMock, runner: CliRunner, tmp_path) -> None: + cfg = tmp_path / "big.yaml" + cfg.write_text("# padding\n" * 7000) + + result = runner.invoke(app, ["discover", "configs", "validate", "-f", str(cfg), "--type", "database"]) + + assert result.exit_code == ExitCode.INVALID_INPUT + assert "asynchronously" in result.stderr + assert "dm discover configs status" in result.stderr + mock_get_client.assert_not_called() + + +@patch(f"{MODULE}.get_client") +def test_status_valid_exits_0(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_configs.return_value = [_config("emp")] + + result = runner.invoke(app, ["discover", "configs", "status", "emp", "--json"]) + + assert result.exit_code == 0 + assert '"status": "valid"' in result.stdout + + +@patch(f"{MODULE}.get_client") +def test_status_invalid_exits_4(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + config = _config("emp", is_valid=ValidationStatus.invalid) + config.validation_error = "unknown label 'foo'" + client.list_discovery_configs.return_value = [config] + + result = runner.invoke(app, ["discover", "configs", "status", "emp", "--json"]) + + assert result.exit_code == ExitCode.INVALID_INPUT + assert "unknown label 'foo'" in result.stdout + + +@patch(f"{MODULE}.get_client") +def test_status_in_progress_exits_0(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_configs.return_value = [_config("emp", is_valid=ValidationStatus.in_progress)] + + result = runner.invoke(app, ["discover", "configs", "status", "emp", "--json"]) + + assert result.exit_code == 0 + assert '"status": "in_progress"' in result.stdout diff --git a/tests/commands/test_ruleset_libraries.py b/tests/commands/test_ruleset_libraries.py index 93d67ef..9cdcb4f 100644 --- a/tests/commands/test_ruleset_libraries.py +++ b/tests/commands/test_ruleset_libraries.py @@ -1,9 +1,11 @@ from __future__ import annotations +from http import HTTPStatus from types import SimpleNamespace from unittest.mock import MagicMock, patch -from datamasque.client.models.status import ValidationStatus +from datamasque.client.exceptions import DataMasqueApiError +from datamasque.client.models.status import ValidationErrorDetails, ValidationStatus from typer.testing import CliRunner from datamasque_cli.main import app @@ -12,6 +14,34 @@ MODULE = "datamasque_cli.commands.ruleset_libraries" +@patch(f"{MODULE}.get_client") +def test_delete_in_use_reports_server_reason(mock_get_client: MagicMock, runner: CliRunner) -> None: + """A 409 must surface the server's explanation, not a traceback. + + The server refuses to delete a library that active rulesets still import, and + its `detail` names the count. Without handling, the raised `DataMasqueApiError` + escapes as an unhandled exception and the user only sees a stack trace. + """ + client = MagicMock() + mock_get_client.return_value = client + client.get_ruleset_library_by_name.return_value = SimpleNamespace(id="lib-uuid", name="lib", namespace="") + response = MagicMock() + response.status_code = HTTPStatus.CONFLICT + response.json.return_value = { + "detail": 'Cannot delete library "lib": used by 3 active ruleset(s)', + "rulesets": [{"id": "rs-1", "name": "customers"}], + } + client.delete_ruleset_library_by_name_if_exists.side_effect = DataMasqueApiError( + "API request to https://dm/api/ruleset-libraries/lib-uuid/ failed with status 409", response=response + ) + + result = runner.invoke(app, ["libraries", "delete", "lib", "--yes"]) + + assert result.exit_code == ExitCode.CONFLICT + assert "used by 3 active ruleset(s)" in result.stderr + assert "--force" in result.stderr + + def _validated_library( is_valid: ValidationStatus | None, validation_errors: list[SimpleNamespace] | None = None, @@ -101,3 +131,34 @@ def test_validate_library_nonterminal_status_passes_through(mock_get_client: Mag assert result.exit_code == 0 assert "in_progress" in result.stderr + + +@patch(f"{MODULE}.get_client") +def test_status_valid_exits_0(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.get_ruleset_library_by_name.return_value = SimpleNamespace( + namespace="", name="lib", is_valid=ValidationStatus.valid, validation_errors=[] + ) + + result = runner.invoke(app, ["libraries", "status", "lib", "--json"]) + + assert result.exit_code == 0 + assert '"status": "valid"' in result.stdout + + +@patch(f"{MODULE}.get_client") +def test_status_invalid_exits_4(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.get_ruleset_library_by_name.return_value = SimpleNamespace( + namespace="", + name="lib", + is_valid=ValidationStatus.invalid, + validation_errors=[ValidationErrorDetails(message="Unknown mask `nope`.")], + ) + + result = runner.invoke(app, ["libraries", "status", "lib", "--json"]) + + assert result.exit_code == ExitCode.INVALID_INPUT + assert "Unknown mask `nope`." in result.stdout diff --git a/tests/commands/test_rulesets.py b/tests/commands/test_rulesets.py index 0f4359d..5e880f9 100644 --- a/tests/commands/test_rulesets.py +++ b/tests/commands/test_rulesets.py @@ -8,7 +8,7 @@ import pytest from datamasque.client.exceptions import DataMasqueApiError from datamasque.client.models.ruleset import RulesetType -from datamasque.client.models.status import ValidationStatus +from datamasque.client.models.status import ValidationErrorDetails, ValidationStatus from typer.testing import CliRunner from datamasque_cli.main import app @@ -35,6 +35,15 @@ def fake_create(rs: object) -> object: return fake_create +@patch(f"{MODULE}.get_client") +def test_unknown_type_is_rejected_before_any_request(mock_get_client: MagicMock, runner: CliRunner) -> None: + result = runner.invoke(app, ["rulesets", "list", "--type", "banana"]) + + assert result.exit_code == ExitCode.USAGE + assert "is not one of" in result.output + mock_get_client.assert_not_called() + + # -- create (type resolution via server lookup) ---------------------------- @@ -437,3 +446,64 @@ def test_system_export_alias_warns_and_delegates(mock_get_client: MagicMock, run assert result.exit_code == 0 assert "deprecated" in result.stderr.lower() assert output.read_bytes() == b"zip-body" + + +@patch(f"{MODULE}.get_client") +def test_validate_oversize_aborts_before_any_request( + mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path +) -> None: + yaml_file = tmp_path / "big.yaml" + yaml_file.write_text("# padding\n" * 7000) + + result = runner.invoke(app, ["rulesets", "validate", "--file", str(yaml_file), "--type", "database"]) + + assert result.exit_code == ExitCode.INVALID_INPUT + assert "asynchronously" in result.stderr + assert "dm rulesets status" in result.stderr + mock_get_client.assert_not_called() + + +# -- status ---------------------------------------------------------------- + + +def _listed_ruleset(is_valid: ValidationStatus, errors: list[ValidationErrorDetails] | None = None) -> SimpleNamespace: + return SimpleNamespace( + id=1, name="demo", ruleset_type=RulesetType.database, is_valid=is_valid, validation_errors=errors or [] + ) + + +@patch(f"{MODULE}.get_client") +def test_status_valid_exits_0(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_rulesets.return_value = [_listed_ruleset(ValidationStatus.valid)] + + result = runner.invoke(app, ["rulesets", "status", "demo", "--json"]) + + assert result.exit_code == 0 + assert '"status": "valid"' in result.stdout + + +@patch(f"{MODULE}.get_client") +def test_status_invalid_exits_4_with_errors(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + errors = [ValidationErrorDetails(message="Missing `key` in `tasks`.", line_number=3)] + client.list_rulesets.return_value = [_listed_ruleset(ValidationStatus.invalid, errors)] + + result = runner.invoke(app, ["rulesets", "status", "demo", "--json"]) + + assert result.exit_code == ExitCode.INVALID_INPUT + assert "Missing `key` in `tasks`." in result.stdout + + +@patch(f"{MODULE}.get_client") +def test_status_in_progress_exits_0(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_rulesets.return_value = [_listed_ruleset(ValidationStatus.in_progress)] + + result = runner.invoke(app, ["rulesets", "status", "demo", "--json"]) + + assert result.exit_code == 0 + assert '"status": "in_progress"' in result.stdout diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 7ed9c71..5e51a79 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -183,11 +183,10 @@ def discovery_library_name(runner: CliRunner) -> Iterator[str]: name = f"dm_int_{uuid.uuid4().hex[:8]}" yield name for namespace in ("", DISCOVERY_TEST_NAMESPACE): - for config_type in ("file", "database"): - args = ["discover", "libraries", "delete", name, "--type", config_type, "--yes", "--force"] - if namespace: - args += ["--namespace", namespace] - runner.invoke(app, args) + args = ["discover", "libraries", "delete", name, "--yes", "--force"] + if namespace: + args += ["--namespace", namespace] + runner.invoke(app, args) @pytest.fixture() diff --git a/tests/integration/test_discovery.py b/tests/integration/test_discovery.py index 213e7f6..27a9f33 100644 --- a/tests/integration/test_discovery.py +++ b/tests/integration/test_discovery.py @@ -174,8 +174,6 @@ def test_library_create_get_delete_lifecycle( "create", "--name", discovery_library_name, - "--type", - "database", "-f", str(discovery_library_yaml), ], @@ -208,8 +206,6 @@ def test_library_namespace_is_isolated( "create", "--name", discovery_library_name, - "--type", - "database", "--namespace", DISCOVERY_TEST_NAMESPACE, "-f", @@ -228,9 +224,7 @@ def test_library_namespace_is_isolated( def test_library_validate_rejects_invalid_yaml(runner: CliRunner, invalid_discovery_yaml: Path) -> None: - result = runner.invoke( - app, ["discover", "libraries", "validate", "-f", str(invalid_discovery_yaml), "--type", "database"] - ) + result = runner.invoke(app, ["discover", "libraries", "validate", "-f", str(invalid_discovery_yaml)]) assert result.exit_code == ExitCode.INVALID_INPUT From fdb585240e4c70730903fef9286410bf79154ae0 Mon Sep 17 00:00:00 2001 From: Peter <101368063+ClassicMMT@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:35:16 +1200 Subject: [PATCH 6/6] refactor: from dm-python - Add validation to discover configs/libraries - Abort on empty YAML directly - Abort with not-found when a run has no discovery output --- CHANGELOG.md | 2 +- src/datamasque_cli/commands/discovery.py | 46 +++++++- .../commands/discovery_config_libraries.py | 66 ++++++++---- .../commands/discovery_configs.py | 48 ++++++--- src/datamasque_cli/output.py | 8 ++ tests/commands/test_discovery.py | 52 +++++++++ .../test_discovery_config_libraries.py | 76 ++++++++++++- tests/commands/test_discovery_configs.py | 100 +++++++++++++++--- 8 files changed, 341 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4b9b60..e76ae1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## v1.5.0 ### Added -- Support for datamasque-python 1.2.1. +- Support for datamasque-python 1.2.2. - `dm discover schema-results` handles matches with no label. - `dm rulesets validate` and `dm libraries validate` print validation errors for invalid YAML. diff --git a/src/datamasque_cli/commands/discovery.py b/src/datamasque_cli/commands/discovery.py index 5ec64fd..bbbc93c 100644 --- a/src/datamasque_cli/commands/discovery.py +++ b/src/datamasque_cli/commands/discovery.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +from http import HTTPStatus from pathlib import Path import typer @@ -34,6 +35,25 @@ app.add_typer(discovery_config_libraries.app, name="libraries") +def _abort_if_run_output_missing( + exc: DataMasqueApiError, + run_id: int, + output_label: str, + missing_statuses: tuple[HTTPStatus, ...] = (HTTPStatus.NOT_FOUND,), +) -> None: + """Turn the error a run without `output_label` returns into a not-found envelope.""" + if exc.response is None or exc.response.status_code not in missing_statuses: + return + abort( + f"No {output_label} available for run {run_id}.", + code=ErrorCode.NOT_FOUND, + hint=( + f"Discovery output is written once the run reaches a final state. " + f"Check status with `dm run status {run_id}`." + ), + ) + + def _write_or_echo(content: str, output: Path | None, success_label: str) -> None: """Write `content` to `output` when given, otherwise echo to stdout.""" if output is None: @@ -171,7 +191,13 @@ def schema_results( output reflects what discovery actually found. """ client = get_client(profile) - results = client.list_schema_discovery_results(RunId(run_id)) + try: + results = client.list_schema_discovery_results(RunId(run_id)) + except DataMasqueApiError as exc: + _abort_if_run_output_missing( + exc, run_id, "schema discovery results", (HTTPStatus.NOT_FOUND, HTTPStatus.BAD_REQUEST) + ) + raise data = [ { @@ -204,7 +230,11 @@ def sdd_report( ) -> None: """Download sensitive data discovery report for a run.""" client = get_client(profile) - report = client.get_sdd_report(RunId(run_id)) + try: + report = client.get_sdd_report(RunId(run_id)) + except DataMasqueApiError as exc: + _abort_if_run_output_missing(exc, run_id, "sensitive data discovery report") + raise _write_or_echo(report, output, "SDD report") @@ -221,7 +251,11 @@ def db_discovery_report( requires `-o`, since a zip can't be streamed to stdout. """ client = get_client(profile) - report = client.get_db_discovery_result_report(RunId(run_id)) + try: + report = client.get_db_discovery_result_report(RunId(run_id)) + except DataMasqueApiError as exc: + _abort_if_run_output_missing(exc, run_id, "database discovery report") + raise if isinstance(report, bytes): if output is None: @@ -286,5 +320,9 @@ def config_snapshot( ) -> None: """Download the discovery config a run used (the run's snapshot).""" client = get_client(profile) - snapshot = client.get_discovery_run_config_snapshot_yaml(RunId(run_id)) + try: + snapshot = client.get_discovery_run_config_snapshot_yaml(RunId(run_id)) + except DataMasqueApiError as exc: + _abort_if_run_output_missing(exc, run_id, "discovery config snapshot") + raise _write_or_echo(snapshot, output, "Discovery config snapshot") diff --git a/src/datamasque_cli/commands/discovery_config_libraries.py b/src/datamasque_cli/commands/discovery_config_libraries.py index 8a93462..cff44c7 100644 --- a/src/datamasque_cli/commands/discovery_config_libraries.py +++ b/src/datamasque_cli/commands/discovery_config_libraries.py @@ -2,15 +2,25 @@ from __future__ import annotations +import uuid from pathlib import Path import typer -from datamasque.client.exceptions import DataMasqueApiError, DataMasqueArgumentError +from datamasque.client.exceptions import DataMasqueApiError from datamasque.client.models.discovery_config_library import DiscoveryConfigLibrary from datamasque.client.models.status import ValidationStatus from datamasque_cli.client import get_client -from datamasque_cli.output import ErrorCode, ExitCode, abort, abort_api_error, print_success, render_output +from datamasque_cli.output import ( + ErrorCode, + ExitCode, + abort, + abort_api_error, + abort_if_empty, + print_success, + print_warning, + render_output, +) app = typer.Typer(help="Manage discovery config libraries (configurable discovery).", no_args_is_help=True) @@ -87,12 +97,12 @@ def create_library( profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), ) -> None: """Create or update a discovery config library from a YAML file.""" + yaml_content = file.read_text() + abort_if_empty(yaml_content, file) + client = get_client(profile) - library = DiscoveryConfigLibrary(name=name, namespace=namespace, yaml=file.read_text()) - try: - client.create_or_update_discovery_config_library(library) - except DataMasqueArgumentError: - abort(f"{file} contains no YAML content.", code=ErrorCode.INVALID_INPUT) + library = DiscoveryConfigLibrary(name=name, namespace=namespace, yaml=yaml_content) + client.create_or_update_discovery_config_library(library) print_success(f"Discovery config library '{_label(name, namespace)}' created/updated.") @@ -135,22 +145,38 @@ def validate_library( file: Path = typer.Option(..., "--file", "-f", help="Path to YAML library file", exists=True, readable=True), profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), ) -> None: - """Validate a discovery config library YAML file against the DataMasque server.""" + """Validate a discovery config library YAML file against the DataMasque server. + + Creates a temporary library to trigger server-side validation, + then deletes it. Reports any validation errors. + """ + yaml_content = file.read_text() + abort_if_empty(yaml_content, file) + temp_name = f"__dm_cli_validate_{uuid.uuid4().hex}" + client = get_client(profile) - library = DiscoveryConfigLibrary(name=file.stem, yaml=file.read_text()) + library = DiscoveryConfigLibrary(name=temp_name, yaml=yaml_content) + try: - validated = client.validate_discovery_config_library(library) - except DataMasqueArgumentError: - abort(f"{file} contains no YAML content.", code=ErrorCode.INVALID_INPUT) - - if validated.is_valid is ValidationStatus.invalid: - abort( - f'Discovery config library "{file.name}" is invalid: {validated.validation_error}', - code=ErrorCode.INVALID_INPUT, - ) + created = client.create_discovery_config_library(library) + except DataMasqueApiError as exc: + abort_api_error(f'Validation of discovery config library "{file.name}" failed', exc) - status = validated.is_valid.value if validated.is_valid else "unknown" - print_success(f'Discovery config library "{file.name}" validation status: {status}') + try: + if created.is_valid is ValidationStatus.invalid: + abort( + f'Discovery config library "{file.name}" is invalid: {created.validation_error}', + code=ErrorCode.INVALID_INPUT, + ) + + status = created.is_valid.value if created.is_valid else "unknown" + print_success(f'Discovery config library "{file.name}" validation status: {status}') + finally: + if created.id is not None: + try: + client.delete_discovery_config_library_by_id_if_exists(created.id) + except DataMasqueApiError as exc: + print_warning(f"Validation library '{temp_name}' left on server; delete manually. Reason: {exc}") @app.command("status") diff --git a/src/datamasque_cli/commands/discovery_configs.py b/src/datamasque_cli/commands/discovery_configs.py index 863f06d..8509735 100644 --- a/src/datamasque_cli/commands/discovery_configs.py +++ b/src/datamasque_cli/commands/discovery_configs.py @@ -2,11 +2,12 @@ from __future__ import annotations +import uuid from pathlib import Path import typer from datamasque.client import DataMasqueClient -from datamasque.client.exceptions import DataMasqueArgumentError +from datamasque.client.exceptions import DataMasqueApiError from datamasque.client.models.discovery_config import DiscoveryConfig, DiscoveryConfigType from datamasque.client.models.status import ValidationErrorDetails, ValidationStatus @@ -15,10 +16,13 @@ ErrorCode, ExitCode, abort, + abort_api_error, abort_if_async_validation, + abort_if_empty, abort_if_invalid, print_info, print_success, + print_warning, render_output, ) @@ -176,11 +180,10 @@ def create_config( ) yaml_content = file.read_text() + abort_if_empty(yaml_content, file) + config = DiscoveryConfig(name=name, yaml=yaml_content, config_type=cfg_type) - try: - client.create_or_update_discovery_config(config) - except DataMasqueArgumentError: - abort(f"{file} contains no YAML content.", code=ErrorCode.INVALID_INPUT) + client.create_or_update_discovery_config(config) print_success(f"Discovery config '{name}' ({cfg_type.value}) created/updated.") @@ -213,30 +216,43 @@ def validate_config( ) -> None: """Validate a discovery config YAML file against the DataMasque server. + Creates a temporary config to trigger server-side validation, + then deletes it. Reports any validation errors. + Note that configs over 60 KB validate asynchronously and cannot be validated here. """ yaml_content = file.read_text() + abort_if_empty(yaml_content, file) abort_if_async_validation( yaml_content, subject=f'Discovery config "{file.name}"', create_command=f"dm discover configs create --name --type {config_type.value} -f {file}", status_command="dm discover configs status ", ) + temp_name = f"__dm_cli_validate_{uuid.uuid4().hex}" client = get_client(profile) - config = DiscoveryConfig(name=file.stem, yaml=yaml_content, config_type=config_type) - try: - validated = client.validate_discovery_config(config) - except DataMasqueArgumentError: - abort(f"{file} contains no YAML content.", code=ErrorCode.INVALID_INPUT) + config = DiscoveryConfig(name=temp_name, yaml=yaml_content, config_type=config_type) - errors = validated.validation_error_details - if not errors and validated.validation_error: - errors = [ValidationErrorDetails(message=validated.validation_error)] - abort_if_invalid(f'Discovery config "{file.name}"', validated.is_valid, errors) + try: + created = client.create_discovery_config(config) + except DataMasqueApiError as exc: + abort_api_error(f'Validation of discovery config "{file.name}" failed', exc) - status = validated.is_valid.value if validated.is_valid else "unknown" - print_success(f'Discovery config "{file.name}" validation status: {status}') + try: + errors = created.validation_error_details + if not errors and created.validation_error: + errors = [ValidationErrorDetails(message=created.validation_error)] + abort_if_invalid(f'Discovery config "{file.name}"', created.is_valid, errors) + + status = created.is_valid.value if created.is_valid else "unknown" + print_success(f'Discovery config "{file.name}" validation status: {status}') + finally: + if created.id is not None: + try: + client.delete_discovery_config_by_id_if_exists(created.id) + except DataMasqueApiError as exc: + print_warning(f"Validation config '{temp_name}' left on server; delete manually. Reason: {exc}") @app.command("status") diff --git a/src/datamasque_cli/output.py b/src/datamasque_cli/output.py index 0c0318e..6381ba6 100644 --- a/src/datamasque_cli/output.py +++ b/src/datamasque_cli/output.py @@ -16,6 +16,7 @@ import sys from enum import IntEnum, StrEnum from http import HTTPStatus +from pathlib import Path from typing import Any, NoReturn import typer @@ -287,6 +288,13 @@ def abort_if_invalid(subject: str, is_valid: ValidationStatus | None, errors: li abort(f"{subject} is invalid.", code=ErrorCode.INVALID_INPUT) +def abort_if_empty(yaml_content: str, file: Path) -> None: + """Abort when `file` holds no YAML for the server to act on.""" + if yaml_content: + return + abort(f"{file} contains no YAML content.", code=ErrorCode.INVALID_INPUT) + + def abort_if_async_validation(yaml_content: str, *, subject: str, create_command: str, status_command: str) -> None: """Abort when `yaml_content` is too large for the server to validate synchronously.""" kib = 1024 diff --git a/tests/commands/test_discovery.py b/tests/commands/test_discovery.py index 890991b..7025eff 100644 --- a/tests/commands/test_discovery.py +++ b/tests/commands/test_discovery.py @@ -5,6 +5,7 @@ from types import SimpleNamespace from unittest.mock import MagicMock, patch +import pytest from datamasque.client.exceptions import DataMasqueApiError from datamasque.client.models.discovery import ( FileDiscoveryFile, @@ -157,6 +158,57 @@ def test_file_report_table_lists_locators(mock_get_client: MagicMock, runner: Cl assert "safe_data_preview" not in result.stdout +# -- missing run output ---------------------------------------------------- + + +@pytest.mark.parametrize( + ("command", "client_method", "status", "expected"), + [ + (["discover", "sdd-report", "42"], "get_sdd_report", 404, "sensitive data discovery report"), + (["discover", "db-report", "42"], "get_db_discovery_result_report", 404, "database discovery report"), + ( + ["discover", "config-snapshot", "42"], + "get_discovery_run_config_snapshot_yaml", + 404, + "discovery config snapshot", + ), + (["discover", "schema-results", "42"], "list_schema_discovery_results", 400, "schema discovery results"), + ], +) +@patch(f"{MODULE}.get_client") +def test_missing_run_output_aborts_not_found( + mock_get_client: MagicMock, + runner: CliRunner, + command: list[str], + client_method: str, + status: int, + expected: str, +) -> None: + client = MagicMock() + mock_get_client.return_value = client + getattr(client, client_method).side_effect = DataMasqueApiError( + f"{status}", response=SimpleNamespace(status_code=status) + ) + + result = runner.invoke(app, command) + + assert result.exit_code == ExitCode.NOT_FOUND + stderr = " ".join(result.stderr.split()) + assert f"No {expected} available for run 42" in stderr + assert "dm run status 42" in stderr + + +@patch(f"{MODULE}.get_client") +def test_unexpected_api_error_is_not_swallowed(mock_get_client: MagicMock, runner: CliRunner) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.get_sdd_report.side_effect = DataMasqueApiError("500", response=SimpleNamespace(status_code=500)) + + result = runner.invoke(app, ["discover", "sdd-report", "42"]) + + assert result.exit_code != ExitCode.NOT_FOUND + + # -- schema discovery trigger --------------------------------------------- diff --git a/tests/commands/test_discovery_config_libraries.py b/tests/commands/test_discovery_config_libraries.py index dd6ce56..883b4b3 100644 --- a/tests/commands/test_discovery_config_libraries.py +++ b/tests/commands/test_discovery_config_libraries.py @@ -1,10 +1,13 @@ from __future__ import annotations +from collections.abc import Callable from http import HTTPStatus +from pathlib import Path from types import SimpleNamespace from unittest.mock import MagicMock, patch from datamasque.client.exceptions import DataMasqueApiError +from datamasque.client.models.discovery_config_library import DiscoveryConfigLibrary, DiscoveryConfigLibraryId from datamasque.client.models.status import ValidationStatus from typer.testing import CliRunner @@ -35,6 +38,20 @@ def _library( ) +def _create_returning( + is_valid: ValidationStatus | None, + validation_error: str | None = None, +) -> Callable[[DiscoveryConfigLibrary], DiscoveryConfigLibrary]: + + def fake_create(library: DiscoveryConfigLibrary) -> DiscoveryConfigLibrary: + library.id = DiscoveryConfigLibraryId("lib-uuid") + library.is_valid = is_valid + library.validation_error = validation_error + return library + + return fake_create + + @patch(f"{MODULE}.get_client") def test_list_shows_namespace_and_usage(mock_get_client: MagicMock, runner: CliRunner) -> None: client = MagicMock() @@ -77,7 +94,7 @@ def test_get_namespace_scopes_lookup(mock_get_client: MagicMock, runner: CliRunn @patch(f"{MODULE}.get_client") -def test_create_posts_library(mock_get_client: MagicMock, runner: CliRunner, tmp_path) -> None: +def test_create_posts_library(mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path) -> None: client = MagicMock() mock_get_client.return_value = client lib = tmp_path / "lib.yaml" @@ -168,11 +185,42 @@ def test_delete_missing_is_not_found(mock_get_client: MagicMock, runner: CliRunn @patch(f"{MODULE}.get_client") -def test_validate_invalid_exits_4(mock_get_client: MagicMock, runner: CliRunner, tmp_path) -> None: +def test_validate_empty_file_aborts_before_any_request( + mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path +) -> None: + lib = tmp_path / "empty.yaml" + lib.write_text("") + + result = runner.invoke(app, ["discover", "libraries", "validate", "-f", str(lib)]) + + assert result.exit_code == ExitCode.INVALID_INPUT + mock_get_client.assert_not_called() + + +@patch(f"{MODULE}.get_client") +def test_validate_reports_valid(mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.create_discovery_config_library.side_effect = _create_returning(ValidationStatus.valid) + lib = tmp_path / "lib.yaml" + lib.write_text("labels: []\n") + + result = runner.invoke(app, ["discover", "libraries", "validate", "-f", str(lib)]) + + assert result.exit_code == 0 + assert "valid" in result.stderr + created = client.create_discovery_config_library.call_args.args[0] + assert created.name.startswith("__dm_cli_validate_") + assert created.yaml == "labels: []\n" + client.delete_discovery_config_library_by_id_if_exists.assert_called_once_with("lib-uuid") + + +@patch(f"{MODULE}.get_client") +def test_validate_invalid_exits_4(mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path) -> None: client = MagicMock() mock_get_client.return_value = client - client.validate_discovery_config_library.return_value = SimpleNamespace( - is_valid=ValidationStatus.invalid, validation_error="duplicate label 'email'" + client.create_discovery_config_library.side_effect = _create_returning( + ValidationStatus.invalid, validation_error="duplicate label 'email'" ) lib = tmp_path / "lib.yaml" lib.write_text("labels: []\n") @@ -181,6 +229,26 @@ def test_validate_invalid_exits_4(mock_get_client: MagicMock, runner: CliRunner, assert result.exit_code == ExitCode.INVALID_INPUT assert "duplicate label 'email'" in result.stderr + client.delete_discovery_config_library_by_id_if_exists.assert_called_once_with("lib-uuid") + + +@patch(f"{MODULE}.get_client") +def test_validate_warns_when_temp_library_cleanup_fails( + mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path +) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.create_discovery_config_library.side_effect = _create_returning(ValidationStatus.valid) + client.delete_discovery_config_library_by_id_if_exists.side_effect = DataMasqueApiError( + "boom", response=MagicMock() + ) + lib = tmp_path / "lib.yaml" + lib.write_text("labels: []\n") + + result = runner.invoke(app, ["discover", "libraries", "validate", "-f", str(lib)]) + + assert result.exit_code == 0 + assert "left on server" in result.stderr @patch(f"{MODULE}.get_client") diff --git a/tests/commands/test_discovery_configs.py b/tests/commands/test_discovery_configs.py index 78b44a1..32fd3ce 100644 --- a/tests/commands/test_discovery_configs.py +++ b/tests/commands/test_discovery_configs.py @@ -1,9 +1,13 @@ from __future__ import annotations +from collections.abc import Callable +from http import HTTPStatus +from pathlib import Path from types import SimpleNamespace from unittest.mock import MagicMock, patch -from datamasque.client.models.discovery_config import DiscoveryConfigType +from datamasque.client.exceptions import DataMasqueApiError +from datamasque.client.models.discovery_config import DiscoveryConfig, DiscoveryConfigId, DiscoveryConfigType from datamasque.client.models.status import ValidationStatus from typer.testing import CliRunner @@ -32,6 +36,21 @@ def _config( ) +def _create_returning( + is_valid: ValidationStatus | None, + validation_error: str | None = None, +) -> Callable[[DiscoveryConfig], DiscoveryConfig]: + + def fake_create(config: DiscoveryConfig) -> DiscoveryConfig: + config.id = DiscoveryConfigId("cfg-uuid") + config.is_valid = is_valid + config.validation_error = validation_error + config.validation_error_details = [] + return config + + return fake_create + + @patch(f"{MODULE}.get_client") def test_unknown_type_is_rejected_before_any_request(mock_get_client: MagicMock, runner: CliRunner) -> None: result = runner.invoke(app, ["discover", "configs", "list", "--type", "banana"]) @@ -118,7 +137,7 @@ def test_defaults_requests_typed_default(mock_get_client: MagicMock, runner: Cli @patch(f"{MODULE}.get_client") -def test_create_new_requires_type(mock_get_client: MagicMock, runner: CliRunner, tmp_path) -> None: +def test_create_new_requires_type(mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path) -> None: client = MagicMock() mock_get_client.return_value = client client.list_discovery_configs.return_value = [] @@ -137,7 +156,7 @@ def test_create_new_requires_type(mock_get_client: MagicMock, runner: CliRunner, @patch(f"{MODULE}.get_client") -def test_create_update_defaults_to_existing_type(mock_get_client: MagicMock, runner: CliRunner, tmp_path) -> None: +def test_create_update_defaults_to_existing_type(mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path) -> None: client = MagicMock() mock_get_client.return_value = client client.list_discovery_configs.return_value = [_config("emp", DiscoveryConfigType.database)] @@ -175,12 +194,10 @@ def test_delete_aborts_when_missing(mock_get_client: MagicMock, runner: CliRunne @patch(f"{MODULE}.get_client") -def test_validate_reports_valid(mock_get_client: MagicMock, runner: CliRunner, tmp_path) -> None: +def test_validate_reports_valid(mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path) -> None: client = MagicMock() mock_get_client.return_value = client - client.validate_discovery_config.return_value = SimpleNamespace( - is_valid=ValidationStatus.valid, validation_error=None, validation_error_details=[] - ) + client.create_discovery_config.side_effect = _create_returning(ValidationStatus.valid) cfg = tmp_path / "cfg.yaml" cfg.write_text("labels: []\n") @@ -188,15 +205,19 @@ def test_validate_reports_valid(mock_get_client: MagicMock, runner: CliRunner, t assert result.exit_code == 0 assert "valid" in result.stderr - client.validate_discovery_config.assert_called_once() + created = client.create_discovery_config.call_args.args[0] + assert created.name.startswith("__dm_cli_validate_") + assert created.yaml == "labels: []\n" + assert created.config_type is DiscoveryConfigType.database + client.delete_discovery_config_by_id_if_exists.assert_called_once_with("cfg-uuid") @patch(f"{MODULE}.get_client") -def test_validate_invalid_exits_4(mock_get_client: MagicMock, runner: CliRunner, tmp_path) -> None: +def test_validate_invalid_exits_4(mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path) -> None: client = MagicMock() mock_get_client.return_value = client - client.validate_discovery_config.return_value = SimpleNamespace( - is_valid=ValidationStatus.invalid, validation_error="unknown label 'foo'", validation_error_details=[] + client.create_discovery_config.side_effect = _create_returning( + ValidationStatus.invalid, validation_error="unknown label 'foo'" ) cfg = tmp_path / "cfg.yaml" cfg.write_text("labels: []\n") @@ -205,10 +226,65 @@ def test_validate_invalid_exits_4(mock_get_client: MagicMock, runner: CliRunner, assert result.exit_code == ExitCode.INVALID_INPUT assert "unknown label 'foo'" in result.stderr + client.delete_discovery_config_by_id_if_exists.assert_called_once_with("cfg-uuid") + + +@patch(f"{MODULE}.get_client") +def test_validate_rejected_create_aborts_without_delete( + mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path +) -> None: + client = MagicMock() + mock_get_client.return_value = client + response = MagicMock() + response.status_code = HTTPStatus.BAD_REQUEST + response.json.return_value = {"detail": "config_yaml: invalid"} + client.create_discovery_config.side_effect = DataMasqueApiError( + "API request failed with status 400", response=response + ) + cfg = tmp_path / "cfg.yaml" + cfg.write_text("labels: []\n") + + result = runner.invoke(app, ["discover", "configs", "validate", "-f", str(cfg), "--type", "database"]) + + assert result.exit_code == ExitCode.ERROR + assert "config_yaml: invalid" in result.stderr + client.delete_discovery_config_by_id_if_exists.assert_not_called() + + +@patch(f"{MODULE}.get_client") +def test_validate_warns_when_temp_config_cleanup_fails( + mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path +) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.create_discovery_config.side_effect = _create_returning(ValidationStatus.valid) + client.delete_discovery_config_by_id_if_exists.side_effect = DataMasqueApiError("boom", response=MagicMock()) + cfg = tmp_path / "cfg.yaml" + cfg.write_text("labels: []\n") + + result = runner.invoke(app, ["discover", "configs", "validate", "-f", str(cfg), "--type", "database"]) + + assert result.exit_code == 0 + assert "left on server" in result.stderr + + +@patch(f"{MODULE}.get_client") +def test_validate_empty_file_aborts_before_any_request( + mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path +) -> None: + cfg = tmp_path / "empty.yaml" + cfg.write_text("") + + result = runner.invoke(app, ["discover", "configs", "validate", "-f", str(cfg), "--type", "database"]) + + assert result.exit_code == ExitCode.INVALID_INPUT + mock_get_client.assert_not_called() @patch(f"{MODULE}.get_client") -def test_validate_oversize_aborts_before_any_request(mock_get_client: MagicMock, runner: CliRunner, tmp_path) -> None: +def test_validate_oversize_aborts_before_any_request( + mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path +) -> None: cfg = tmp_path / "big.yaml" cfg.write_text("# padding\n" * 7000)