diff --git a/CHANGELOG.md b/CHANGELOG.md index 3205275..0e1b4c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,41 @@ # Changelog +## v1.5.0 + +### Added +- 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. +- Support for Configurable Discovery: + - `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. + - `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. + +### Changed +- A declined confirmation prompt now exits 10 (`cancelled`) instead of 1, + so a decision is not reported as a failure. Ctrl-C still exits 1. +- `dm run start --json` and `dm run retry --json` only emit the run id. + `"status": "queued"` is no longer returned. + +### Fixed +- `dm rulesets generate`, `dm connections update --password`, and the + deprecated `dm system import` no longer fail. +- File errors now name the file instead of printing a traceback. +- Unhandled server and network errors now abort with a code, not a traceback. + ## v1.4.0 ### Added diff --git a/README.md b/README.md index 07fe69f..8cf88ca 100644 --- a/README.md +++ b/README.md @@ -138,17 +138,16 @@ dm connections delete # Delete a connection ### Rulesets ```console -dm rulesets list # List all rulesets -dm rulesets list --type file # Filter by type -dm rulesets get # Show ruleset details +dm rulesets list [--type database|file] # List all rulesets, or filter by type +dm rulesets get [--type database|file] # Show details; --type disambiguates same-name rulesets dm rulesets get --yaml # Print raw YAML only -dm rulesets get --type file # Disambiguate same-name rulesets -dm rulesets create --name --file rules.yaml # Create/update (type auto-detected from YAML) -dm rulesets create --name --file rules.yaml --type file # Force a type -dm rulesets delete [--type file|database] # Delete a ruleset +dm rulesets create --name --file rules.yaml # Create/update (type read from the existing ruleset) +dm rulesets create --name --file rules.yaml [--type database|file] # Force a type +dm rulesets delete [--type database|file] # 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 --type database|file # 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 +163,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 ``` @@ -216,11 +216,42 @@ 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 --json # {"id": } +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": } +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|file] [--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 -f cfg.yaml [--type database|file] # Create/update from YAML +dm discover configs delete [--type database|file] # Delete a config +dm discover configs validate -f cfg.yaml --type database|file # Validate against server (YAML under 60 KiB) +dm discover configs status [--type database|file] # Validation status; poll after creating YAML of 60 KiB+ +``` + +#### Discovery config libraries + +The same library can be imported by both database and file discovery configs. + +```console +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 @@ -308,6 +339,8 @@ empty on failure): | 7 | auth_failed | credentials rejected by server | | 8 | conflict | operation rejected by server state | | 9 | transport_error | network or TLS failure | +| 10 | cancelled | you answered no to a confirmation prompt | +| 11 | forbidden | user lacks permission for the operation | Exit codes are stable across minor versions. The `error.code` string in the JSON envelope mirrors these names. diff --git a/claude-skills/datamasque-cli/skills/datamasque-cli/SKILL.md b/claude-skills/datamasque-cli/skills/datamasque-cli/SKILL.md index ad70cda..712704d 100644 --- a/claude-skills/datamasque-cli/skills/datamasque-cli/SKILL.md +++ b/claude-skills/datamasque-cli/skills/datamasque-cli/SKILL.md @@ -23,9 +23,11 @@ In agent mode — auto-detected when stdout is not a TTY, `AI_AGENT` is set, or `error.code` is the stable identifier; branch on it rather than the message. The set is `not_found`, `invalid_input`, `ambiguous`, `auth_required`, -`auth_failed`, `conflict`, `transport_error`, `error`. Exit code is non-zero -on any error; exit 2 specifically means a CLI usage error (unknown flag, -missing argument) from typer. +`auth_failed`, `conflict`, `transport_error`, `cancelled`, `forbidden`, +`error`. Exit code is non-zero on any error; exit 2 specifically means a CLI +usage error (unknown flag, missing argument) from typer, exit 10 means the user +declined a confirmation prompt, and exit 11 means the logged-in user lacks +permission. `DM_OUTPUT=table` forces human-readable output. @@ -55,7 +57,7 @@ Pass repeated `--options key=value` for server-side knobs - **Ruleset namespaces.** `database` and `file` rulesets share a name namespace, so `customers` can exist in both. `dm run start` reads the source connection's type and picks the matching ruleset automatically. - For `get` / `create` / `delete`, pass `--type file|database` only when + For `get` / `create` / `delete`, pass `--type database|file` only when two rows share the name and you need to disambiguate. - **File masking needs a destination.** Database masking is in-place; @@ -89,6 +91,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/pyproject.toml b/pyproject.toml index b4dfa47..7948c2f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "datamasque-cli" -version = "1.4.0" +version = "1.5.0" description = "Official command-line interface for the DataMasque data-masking platform." authors = [ { name = "DataMasque Ltd" }, @@ -12,7 +12,8 @@ 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", + "pydantic>=2.5,<3", ] classifiers = [ "Development Status :: 4 - Beta", @@ -37,7 +38,7 @@ Repository = "https://github.com/datamasque/datamasque-cli" Issues = "https://github.com/datamasque/datamasque-cli/issues" [project.scripts] -dm = "datamasque_cli.main:app" +dm = "datamasque_cli.main:main" [dependency-groups] dev = [ diff --git a/src/datamasque_cli/client.py b/src/datamasque_cli/client.py index 99394cd..8cc9d19 100644 --- a/src/datamasque_cli/client.py +++ b/src/datamasque_cli/client.py @@ -14,7 +14,7 @@ from datamasque.client.models.ifm import DataMasqueIfmInstanceConfig from datamasque_cli.config import Config, Profile, load_config -from datamasque_cli.output import ErrorCode, abort +from datamasque_cli.errors import ErrorCode, abort ENV_URL = "DATAMASQUE_URL" ENV_USERNAME = "DATAMASQUE_USERNAME" diff --git a/src/datamasque_cli/commands/auth.py b/src/datamasque_cli/commands/auth.py index 2d08535..13ac7f3 100644 --- a/src/datamasque_cli/commands/auth.py +++ b/src/datamasque_cli/commands/auth.py @@ -6,7 +6,8 @@ from datamasque_cli.client import get_client, profile_from_env from datamasque_cli.config import DEFAULT_PROFILE, Profile, load_config, save_config -from datamasque_cli.output import ErrorCode, abort, print_info, print_success, print_table +from datamasque_cli.errors import ErrorCode, abort +from datamasque_cli.output import print_info, print_success, print_table # `login` and `status` handle connection errors locally # because they need softer behaviour than `get_client`'s hard abort: diff --git a/src/datamasque_cli/commands/connections.py b/src/datamasque_cli/commands/connections.py index 1957e61..b7d590e 100644 --- a/src/datamasque_cli/commands/connections.py +++ b/src/datamasque_cli/commands/connections.py @@ -2,12 +2,12 @@ from __future__ import annotations -import json from enum import StrEnum from pathlib import Path import typer from datamasque.client import DataMasqueClient +from datamasque.client.exceptions import DataMasqueApiError from datamasque.client.models.connection import ( AzureConnectionConfig, ConnectionConfig, @@ -22,7 +22,9 @@ ) from datamasque_cli.client import get_client -from datamasque_cli.output import ErrorCode, abort, print_success, redact_sensitive_fields, render_output +from datamasque_cli.errors import ErrorCode, abort, abort_api_error, confirm_or_abort +from datamasque_cli.fileio import read_json_object_or_abort +from datamasque_cli.output import print_success, redact_sensitive_fields, render_output class ConnectionType(StrEnum): @@ -205,8 +207,11 @@ def create_connection( def _create_from_file(client: DataMasqueClient, file: Path) -> None: """Create a connection from a JSON file.""" - data = json.loads(file.read_text()) - conn_type = _parse_connection_type(data.pop("type", "database")) + data = read_json_object_or_abort(file) + raw_type = data.pop("type", "database") + if not isinstance(raw_type, str): + abort(f'{file}: "type" must be a string.', code=ErrorCode.INVALID_INPUT) + conn_type = _parse_connection_type(raw_type) # Convert db_type string to enum for database connections. if conn_type is ConnectionType.DATABASE and "database_type" in data: @@ -298,7 +303,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 +355,12 @@ 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) + # `dbpassword` is the server's field name for the database password on connection PATCH. + 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)}.") @@ -363,7 +376,7 @@ def delete_connection( abort(f"Connection '{name}' not found.", code=ErrorCode.NOT_FOUND) if not is_confirmed: - typer.confirm(f"Delete connection '{name}'?", abort=True) + confirm_or_abort(f"Delete connection '{name}'?") client.delete_connection_by_name_if_exists(name) print_success(f"Connection '{name}' deleted.") diff --git a/src/datamasque_cli/commands/discovery.py b/src/datamasque_cli/commands/discovery.py index 2aa6914..add4a99 100644 --- a/src/datamasque_cli/commands/discovery.py +++ b/src/datamasque_cli/commands/discovery.py @@ -3,17 +3,71 @@ from __future__ import annotations import json +from http import HTTPStatus from pathlib import Path import typer from datamasque.client import DataMasqueClient, RunId +from datamasque.client.exceptions import ( + DataMasqueApiError, + DiscoveryConfigNotFoundError, + InvalidDiscoveryConfigError, +) 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.client.models.status import MaskingRunStatus from datamasque_cli.client import get_client -from datamasque_cli.output import ErrorCode, abort, print_json, print_success, render_output, should_emit_json +from datamasque_cli.commands import discovery_config_libraries, discovery_configs +from datamasque_cli.errors import ( + ErrorCode, + abort, + abort_api_error, + abort_if_not_found, + require_id_or_abort, +) +from datamasque_cli.fileio import write_bytes_or_abort, write_text_or_abort +from datamasque_cli.output import 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 _run_status_or_abort_if_absent(client: DataMasqueClient, run_id: int) -> MaskingRunStatus | None: + """Return the run's status, `None` when it cannot be read, and abort when the run does not exist.""" + try: + return client.get_run_info(RunId(run_id)).status + except DataMasqueApiError as exc: + abort_if_not_found(exc, f"Run {run_id}") + return None + + +def _abort_if_run_output_missing( + client: DataMasqueClient, + exc: DataMasqueApiError, + run_id: int, + output_label: str, +) -> None: + """Explain a missing run output from the run's own state.""" + if exc.response.status_code not in (HTTPStatus.NOT_FOUND, HTTPStatus.BAD_REQUEST): + return + + status = _run_status_or_abort_if_absent(client, run_id) + if status is not None and not status.is_in_final_state: + abort( + f"No {output_label} available for run {run_id} yet; the run is {status.value}.", + code=ErrorCode.NOT_FOUND, + hint=f"Check progress with `dm run status {run_id}`.", + ) + + abort_api_error(f"No {output_label} available for run {run_id}", exc) def _write_or_echo(content: str, output: Path | None, success_label: str) -> None: @@ -21,7 +75,7 @@ def _write_or_echo(content: str, output: Path | None, success_label: str) -> Non if output is None: typer.echo(content) return - output.write_text(content) + write_text_or_abort(output, content) print_success(f"{success_label} written to {output}") @@ -33,10 +87,37 @@ 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`. + + Config names are unique per type, so name plus type identifies at most one config. + """ + match = client.get_discovery_config_by_name(name, expected_type) + if match is not None: + return require_id_or_abort(match.id, f"discovery config '{name}'") + + other_type = ( + DiscoveryConfigType.file if expected_type is DiscoveryConfigType.database else DiscoveryConfigType.database + ) + if client.get_discovery_config_by_name(name, other_type) is not None: + abort( + f"Discovery config '{name}' exists as {other_type.value}, " + 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) + + @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"), + is_json: bool = typer.Option(False, "--json", help="Output as JSON"), ) -> None: """Start a schema-discovery run on a connection. @@ -47,12 +128,73 @@ 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) + 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) + config_source = f"config '{config}'" + else: + request = SchemaDiscoveryRequest(connection=ConnectionId(conn_id)) + run_id = client.start_schema_discovery_run(request) + config_source = "default discovery" + except DiscoveryConfigNotFoundError as exc: + abort(str(exc), code=ErrorCode.NOT_FOUND) + except InvalidDiscoveryConfigError as exc: + abort(str(exc), code=ErrorCode.INVALID_INPUT) + 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}'. " + f"Schema discovery run {run_id} started for connection '{connection}' ({config_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)}) + + +@app.command("file") +def start_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"), + is_json: bool = typer.Option(False, "--json", help="Output as JSON"), +) -> 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) + + 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) + config_source = f"config '{config}'" + else: + request = FileDataDiscoveryRequest(connection=ConnectionId(conn_id)) + run_id = client.start_file_data_discovery_run(request) + config_source = "default discovery" + except DiscoveryConfigNotFoundError as exc: + abort(str(exc), code=ErrorCode.NOT_FOUND) + except InvalidDiscoveryConfigError as exc: + abort(str(exc), code=ErrorCode.INVALID_INPUT) + 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}' ({config_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)}) @app.command("schema-results") @@ -68,7 +210,11 @@ 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(client, exc, run_id, "schema discovery results") + abort_api_error(f"Failed to list schema discovery results for run {run_id}", exc) data = [ { @@ -77,8 +223,11 @@ 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 "", + "safe_data_preview": ( + r.data.safe_data_preview.model_dump(mode="json") if r.data.safe_data_preview else None + ), } for r in results ] @@ -98,7 +247,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(client, exc, run_id, "sensitive data discovery report") + abort_api_error(f"Failed to download sensitive data discovery report for run {run_id}", exc) _write_or_echo(report, output, "SDD report") @@ -115,7 +268,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(client, exc, run_id, "database discovery report") + abort_api_error(f"Failed to download database discovery report for run {run_id}", exc) if isinstance(report, bytes): if output is None: @@ -125,7 +282,7 @@ def db_discovery_report( hint=f"Re-run with -o .zip (e.g. -o discovery_report_{run_id}.zip).", ) target = output if output.suffix.lower() == ".zip" else output.parent / (output.name + ".zip") - target.write_bytes(report) + write_bytes_or_abort(target, report) print_success(f"Database discovery report (split, zip) written to {target}") return @@ -141,14 +298,52 @@ def file_discovery_report( ) -> None: """Download file discovery report for a run.""" client = get_client(profile) - report = client.get_file_data_discovery_report(RunId(run_id)) + try: + report = client.get_file_data_discovery_report(RunId(run_id)) + except DataMasqueApiError as exc: + _abort_if_run_output_missing(client, exc, run_id, "file discovery report") + abort_api_error(f"Failed to download file discovery report for run {run_id}", exc) + serialised_report = [result.model_dump(mode="json") for result in report] if output is not None: - output.write_text(json.dumps(report, indent=2, default=str)) + write_text_or_abort(output, json.dumps(serialised_report, 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(serialised_report) + 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") +def download_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) + try: + snapshot = client.get_discovery_run_config_snapshot_yaml(RunId(run_id)) + except DataMasqueApiError as exc: + _abort_if_run_output_missing(client, exc, run_id, "discovery config snapshot") + abort_api_error(f"Failed to download discovery config snapshot for run {run_id}", exc) + _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..1370687 --- /dev/null +++ b/src/datamasque_cli/commands/discovery_config_libraries.py @@ -0,0 +1,200 @@ +"""Discovery config library management commands (configurable discovery).""" + +from __future__ import annotations + +import uuid +from pathlib import Path + +import typer +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.errors import ErrorCode, abort, abort_api_error, confirm_or_abort +from datamasque_cli.fileio import read_text_or_abort +from datamasque_cli.output import print_success, print_warning, render_output + +app = typer.Typer(help="Manage discovery config libraries (configurable discovery).", no_args_is_help=True) + + +def _format_library_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 + + +@app.command("list") +def list_libraries( + 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() + + data = [ + { + "id": lib.id, + "namespace": lib.namespace or "", + "name": lib.name, + "valid": lib.is_valid.value if lib.is_valid else "unknown", + "used_by": lib.usage_count, + } + for lib in libraries + ] + + render_output( + data, + is_json=is_json, + columns=["id", "namespace", "name", "valid", "used_by"], + title="Discovery Config Libraries", + ) + + +@app.command("get") +def get_library( + 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_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) + lib = client.get_discovery_config_library_by_name(name, namespace) + + if lib is None: + abort( + f"Discovery config library '{_format_library_label(name, namespace)}' not found.", + code=ErrorCode.NOT_FOUND, + ) + + if is_yaml: + typer.echo(lib.yaml) + return + + data: dict[str, object] = { + "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: {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), + 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.""" + yaml_content = read_text_or_abort(file) + + client = get_client(profile) + library = DiscoveryConfigLibrary(name=name, namespace=namespace, yaml=yaml_content) + client.create_or_update_discovery_config_library(library) + print_success(f"Discovery config library '{_format_library_label(name, namespace)}' created/updated.") + + +@app.command("delete") +def delete_library( + name: str = typer.Argument(help="Library name to delete"), + 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. + """ + label = _format_library_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: + confirm_or_abort(f"Delete discovery config library '{label}'?") + + 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.", + ) + + 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), + profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), +) -> None: + """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 = read_text_or_abort(file) + temp_name = f"__dm_cli_validate_{uuid.uuid4().hex}" + + client = get_client(profile) + library = DiscoveryConfigLibrary(name=temp_name, yaml=yaml_content) + + try: + created = client.create_discovery_config_library(library) + except DataMasqueApiError as exc: + abort_api_error(f'Validation of discovery config library "{file.name}" failed', exc) + + 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") +def show_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.""" + client = get_client(profile) + lib = client.get_discovery_config_library_by_name(name, namespace) + + if lib is None: + abort( + f"Discovery config library '{_format_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}") diff --git a/src/datamasque_cli/commands/discovery_configs.py b/src/datamasque_cli/commands/discovery_configs.py new file mode 100644 index 0000000..b21915c --- /dev/null +++ b/src/datamasque_cli/commands/discovery_configs.py @@ -0,0 +1,280 @@ +"""Discovery config management commands (configurable discovery).""" + +from __future__ import annotations + +import uuid +from pathlib import Path + +import typer +from datamasque.client import DataMasqueClient +from datamasque.client.exceptions import DataMasqueApiError +from datamasque.client.models.discovery_config import DiscoveryConfig, DiscoveryConfigType +from datamasque.client.models.status import ValidationErrorDetails, ValidationStatus + +from datamasque_cli.client import get_client +from datamasque_cli.errors import ( + ErrorCode, + abort, + abort_api_error, + abort_if_invalid, + confirm_or_abort, + require_id_or_abort, +) +from datamasque_cli.fileio import ( + abort_if_too_large_for_sync_validation, + read_text_or_abort, + write_text_or_abort, +) +from datamasque_cli.output import print_info, print_success, print_warning, 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 _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: + 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 database|file to disambiguate.", + ) + return matches[0] + + +@app.command("list") +def list_configs( + 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: + """List all discovery configs.""" + client = get_client(profile) + configs = client.list_discovery_configs() + + if config_type is not None: + configs = [c for c in configs if c.config_type is config_type] + + 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: 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) + match = _collapse_to_one_or_abort(_find_by_name(client, name, config_type), name) + + config_id = require_id_or_abort(match.id, f"discovery config '{name}'") + full = client.get_discovery_config(config_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 get_default_config( + 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) + # `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": config_type.value}) + yaml_content = response.content.decode("utf-8") + + if output is not None: + write_text_or_abort(output, yaml_content) + print_success(f"Default {config_type.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: DiscoveryConfigType | 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) + + if config_type is not None: + resolved_type = config_type + elif len(existing) == 1: + resolved_type = existing[0].config_type + print_info(f"Updating existing {resolved_type.value}-type discovery config '{name}'.") + elif not existing: + abort( + f"No discovery config named '{name}' exists.", + code=ErrorCode.NOT_FOUND, + hint="Pass --type database|file 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 database|file to pick which one to update.", + ) + + yaml_content = read_text_or_abort(file) + + config = DiscoveryConfig(name=name, yaml=yaml_content, config_type=resolved_type) + client.create_or_update_discovery_config(config) + print_success(f"Discovery config '{name}' ({resolved_type.value}) created/updated.") + + +@app.command("delete") +def delete_config( + name: str = typer.Argument(help="Discovery config name to delete"), + 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) + match = _collapse_to_one_or_abort(_find_by_name(client, name, config_type), name) + config_id = require_id_or_abort(match.id, f"discovery config '{name}'") + + if not is_confirmed: + confirm_or_abort(f"Delete discovery config '{name}' ({match.config_type.value})?") + + client.delete_discovery_config_by_id_if_exists(config_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: 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. + + Creates a temporary config to trigger server-side validation, + then deletes it. Reports any validation errors. + + Note that configs over 60 KiB validate asynchronously and cannot be validated here. + """ + yaml_content = read_text_or_abort(file) + abort_if_too_large_for_sync_validation( + yaml_content, + file, + 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=temp_name, yaml=yaml_content, config_type=config_type) + + try: + created = client.create_discovery_config(config) + except DataMasqueApiError as exc: + abort_api_error(f'Validation of discovery config "{file.name}" failed', exc) + + 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") +def show_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.") diff --git a/src/datamasque_cli/commands/files.py b/src/datamasque_cli/commands/files.py index 00842fe..03bbd10 100644 --- a/src/datamasque_cli/commands/files.py +++ b/src/datamasque_cli/commands/files.py @@ -8,7 +8,8 @@ from datamasque.client.models.files import DataMasqueFile, SnowflakeKeyFile from datamasque_cli.client import get_client -from datamasque_cli.output import ErrorCode, abort, print_success, render_output +from datamasque_cli.errors import ErrorCode, abort, confirm_or_abort +from datamasque_cli.output import print_success, render_output app = typer.Typer(help="Manage uploaded files (Oracle wallets, Snowflake keys).", no_args_is_help=True) @@ -57,7 +58,7 @@ def delete_file( abort(f"File '{name}' ({file_type}) not found.", code=ErrorCode.NOT_FOUND) if not is_confirmed: - typer.confirm(f"Delete file '{name}' ({file_type})?", abort=True) + confirm_or_abort(f"Delete file '{name}' ({file_type})?") client.delete_file_if_exists(match) print_success(f"File '{name}' deleted.") diff --git a/src/datamasque_cli/commands/ifm.py b/src/datamasque_cli/commands/ifm.py index 1511fc1..0963d99 100644 --- a/src/datamasque_cli/commands/ifm.py +++ b/src/datamasque_cli/commands/ifm.py @@ -11,7 +11,7 @@ import sys from enum import StrEnum from pathlib import Path -from typing import Any, NoReturn +from typing import Any import typer from datamasque.client.exceptions import DataMasqueApiError @@ -23,79 +23,13 @@ ) from datamasque_cli.client import get_ifm_client -from datamasque_cli.output import ErrorCode, abort, print_error, print_json, print_success, render_output +from datamasque_cli.errors import ErrorCode, abort, abort_api_error, confirm_or_abort +from datamasque_cli.fileio import read_text_or_abort +from datamasque_cli.output import print_error, print_json, print_success, render_output app = typer.Typer(help="Manage in-flight-masking (IFM) ruleset plans and execute masks.", no_args_is_help=True) -# IFM service maps HTTP statuses to the CLI's stable `ErrorCode` taxonomy so -# agents and scripts get the right exit code (see "Exit codes" in `README.md`). -# Anything not listed falls through to `ErrorCode.ERROR` (exit 1). -_STATUS_TO_ERROR_CODE: dict[int, ErrorCode] = { - 400: ErrorCode.INVALID_INPUT, - 404: ErrorCode.NOT_FOUND, - 409: ErrorCode.CONFLICT, - 422: ErrorCode.INVALID_INPUT, -} - - -def _format_pydantic_errors(errors: list[Any]) -> str: - """Flatten FastAPI's `detail` list (Pydantic `e.errors()`) into a readable string. - - Each entry looks like `{"loc": [...], "msg": "...", "type": "..."}`; - we render `field.path: message` per entry, joined with `; `. - Entries that don't match the shape fall back to `str(entry)`. - """ - parts: list[str] = [] - for entry in errors: - if isinstance(entry, dict) and "msg" in entry: - loc = entry.get("loc") or [] - location = ".".join(str(part) for part in loc if part != "body") if isinstance(loc, (list, tuple)) else "" - parts.append(f"{location}: {entry['msg']}" if location else str(entry["msg"])) - else: - parts.append(str(entry)) - return "; ".join(parts) - - -def _server_error_detail(exc: DataMasqueApiError) -> str | None: - """Pull a human-readable error string from the IFM response body, if present. - - The IFM service returns `{"error": "..."}`; - FastAPI validation errors come back as `{"detail": ...}`, - where `detail` is either a string or a list of Pydantic error dicts (422s). - Falls through to `None` if the body is missing or not parseable. - """ - try: - body = exc.response.json() - except (ValueError, AttributeError): - return None - if isinstance(body, dict): - error = body.get("error") - if isinstance(error, str): - return error - if "detail" in body: - detail = body["detail"] - if isinstance(detail, str): - return detail - if isinstance(detail, list): - return _format_pydantic_errors(detail) - return str(detail) - return None - - -def _abort_api_error(prefix: str, exc: DataMasqueApiError) -> NoReturn: - """Map an `DataMasqueApiError` to the right `ErrorCode` and surface the body. - - The default `str(exc)` only includes the HTTP status, - so the actual server message is hidden without this. - """ - status_code = getattr(exc.response, "status_code", None) - code = _STATUS_TO_ERROR_CODE.get(status_code, ErrorCode.ERROR) if isinstance(status_code, int) else ErrorCode.ERROR - detail = _server_error_detail(exc) - message = f"{prefix}: {detail}" if detail else f"{prefix}: {exc}" - abort(message, code=code) - - class LogLevel(StrEnum): DEBUG = "DEBUG" INFO = "INFO" @@ -117,11 +51,7 @@ def _load_mask_input(data: str) -> list[Any]: if data == "-": raw = sys.stdin.read() else: - try: - raw = Path(data).read_text() - except OSError as exc: - code = ErrorCode.NOT_FOUND if isinstance(exc, FileNotFoundError) else ErrorCode.INVALID_INPUT - abort(f"Could not read mask input file '{data}': {exc.strerror or exc}", code=code) + raw = read_text_or_abort(Path(data)) try: parsed = json.loads(raw) @@ -143,7 +73,7 @@ def list_plans( try: plans = client.list_ruleset_plans() except DataMasqueApiError as exc: - _abort_api_error("Failed to list IFM ruleset plans", exc) + abort_api_error("Failed to list IFM ruleset plans", exc) data = [ { @@ -176,7 +106,7 @@ def get_plan( try: plan = client.get_ruleset_plan(name) except DataMasqueApiError as exc: - _abort_api_error(f"Failed to get IFM ruleset plan '{name}'", exc) + abort_api_error(f"Failed to get IFM ruleset plan '{name}'", exc) if is_yaml: if plan.ruleset_yaml is None: @@ -217,13 +147,13 @@ def create_plan( client = get_ifm_client(profile) request = RulesetPlanCreateRequest( name=name, - ruleset_yaml=file.read_text(), + ruleset_yaml=read_text_or_abort(file), options=_options_from_flags(enabled, log_level), ) try: created = client.create_ruleset_plan(request) except DataMasqueApiError as exc: - _abort_api_error("Failed to create IFM ruleset plan", exc) + abort_api_error("Failed to create IFM ruleset plan", exc) print_success(f"IFM ruleset plan '{created.name}' created (serial {created.serial}).") if created.url: @@ -249,13 +179,13 @@ def update_plan( client = get_ifm_client(profile) request = RulesetPlanPartialUpdateRequest( - ruleset_yaml=file.read_text() if file is not None else None, + ruleset_yaml=read_text_or_abort(file) if file is not None else None, options=_options_from_flags(enabled, log_level), ) try: updated = client.patch_ruleset_plan(name, request) except DataMasqueApiError as exc: - _abort_api_error(f"Failed to update IFM ruleset plan '{name}'", exc) + abort_api_error(f"Failed to update IFM ruleset plan '{name}'", exc) print_success(f"IFM ruleset plan '{name}' updated (serial {updated.serial}).") @@ -268,13 +198,13 @@ def delete_plan( ) -> None: """Delete an IFM ruleset plan.""" if not is_confirmed: - typer.confirm(f"Delete IFM ruleset plan '{name}'?", abort=True) + confirm_or_abort(f"Delete IFM ruleset plan '{name}'?") client = get_ifm_client(profile) try: client.delete_ruleset_plan(name) except DataMasqueApiError as exc: - _abort_api_error(f"Failed to delete IFM ruleset plan '{name}'", exc) + abort_api_error(f"Failed to delete IFM ruleset plan '{name}'", exc) print_success(f"IFM ruleset plan '{name}' deleted.") @@ -317,7 +247,7 @@ def mask( try: result = client.mask(name, request) except DataMasqueApiError as exc: - _abort_api_error("Mask request failed", exc) + abort_api_error("Mask request failed", exc) if not result.success: print_error("Mask failed.") @@ -342,7 +272,7 @@ def verify_token( try: info = client.verify_token() except DataMasqueApiError as exc: - _abort_api_error("Failed to verify IFM token", exc) + abort_api_error("Failed to verify IFM token", exc) if is_json: print_json({"scopes": info.scopes}) return diff --git a/src/datamasque_cli/commands/ruleset_libraries.py b/src/datamasque_cli/commands/ruleset_libraries.py index 675b73b..d43190b 100644 --- a/src/datamasque_cli/commands/ruleset_libraries.py +++ b/src/datamasque_cli/commands/ruleset_libraries.py @@ -5,10 +5,14 @@ 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, print_success, render_output +from datamasque_cli.errors import ErrorCode, abort, abort_api_error, abort_if_invalid, confirm_or_abort +from datamasque_cli.fileio import read_text_or_abort +from datamasque_cli.output import print_info, print_success, render_output, should_emit_json app = typer.Typer(help="Manage ruleset libraries.", no_args_is_help=True) @@ -74,7 +78,7 @@ def create_library( profile: str | None = typer.Option(None, "--profile", "-p", help="Profile to use"), ) -> None: """Create or update a ruleset library from a YAML file.""" - yaml_content = file.read_text() + yaml_content = read_text_or_abort(file) client = get_client(profile) library = RulesetLibrary(name=name, namespace=namespace, yaml=yaml_content) @@ -98,9 +102,17 @@ def delete_library( abort(f"Library '{label}' not found.", code=ErrorCode.NOT_FOUND) if not is_confirmed: - typer.confirm(f"Delete library '{label}'?", abort=True) + confirm_or_abort(f"Delete library '{label}'?") + + 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.", + ) - client.delete_ruleset_library_by_name_if_exists(name, namespace, force=force) print_success(f"Library '{label}' deleted.") @@ -114,19 +126,56 @@ 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) + try: + validated = client.validate_ruleset_library(lib.id) + except DataMasqueApiError as exc: + abort_api_error(f"Failed to validate library '{label}'", exc) + 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}") +@app.command("status") +def show_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.") + + @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 80100e1..095f0fa 100644 --- a/src/datamasque_cli/commands/rulesets.py +++ b/src/datamasque_cli/commands/rulesets.py @@ -2,7 +2,6 @@ from __future__ import annotations -import json import uuid from pathlib import Path @@ -10,10 +9,28 @@ 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, abort, print_error, print_info, print_success, print_warning, render_output +from datamasque_cli.errors import ( + ErrorCode, + abort, + abort_api_error, + abort_if_invalid, + confirm_or_abort, + require_id_or_abort, +) +from datamasque_cli.fileio import ( + abort_if_too_large_for_sync_validation, + read_json_object_or_abort, + read_text_or_abort, + write_bytes_or_abort, + write_text_or_abort, +) +from datamasque_cli.output import print_info, print_success, print_warning, render_output, should_emit_json app = typer.Typer(help="Manage masking rulesets.", no_args_is_help=True) @@ -30,8 +47,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: @@ -39,14 +56,14 @@ def _pick_single(matches: list[Ruleset], name: str) -> Ruleset: abort( f"Multiple rulesets named '{name}':\n {options}", code=ErrorCode.AMBIGUOUS, - hint="Pass --type file|database to disambiguate.", + hint="Pass --type database|file to disambiguate.", ) return matches[0] @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: @@ -55,8 +72,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 = [ { @@ -73,15 +89,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. @@ -105,7 +122,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", @@ -124,10 +141,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}'.") @@ -135,17 +151,17 @@ def create_ruleset( abort( f"No ruleset named '{name}' exists.", code=ErrorCode.NOT_FOUND, - hint="Pass --type file|database to create a new one.", + hint="Pass --type database|file to create a new one.", ) else: options = ", ".join(r.ruleset_type.value for r in existing) abort( f"Multiple rulesets named '{name}' ({options}).", code=ErrorCode.AMBIGUOUS, - hint="Pass --type file|database to pick which one to update.", + hint="Pass --type database|file to pick which one to update.", ) - yaml_content = file.read_text() + yaml_content = read_text_or_abort(file) ruleset = Ruleset(name=name, yaml=yaml_content, ruleset_type=rs_type) client.create_or_update_ruleset(ruleset) print_success(f"Ruleset '{name}' ({rs_type.value}) created/updated.") @@ -154,27 +170,28 @@ 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) + ruleset_id = require_id_or_abort(match.id, f"ruleset '{name}'") if not is_confirmed: - typer.confirm(f"Delete ruleset '{name}' ({match.ruleset_type.value})?", abort=True) + confirm_or_abort(f"Delete ruleset '{name}' ({match.ruleset_type.value})?") - assert match.id is not None # Populated by list_rulesets - client.delete_ruleset_by_id_if_exists(match.id) + client.delete_ruleset_by_id_if_exists(ruleset_id) print_success(f"Ruleset '{name}' ({match.ruleset_type.value}) deleted.") @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", @@ -186,25 +203,33 @@ 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) + yaml_content = read_text_or_abort(file) + abort_if_too_large_for_sync_validation( + yaml_content, + file, + 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) except DataMasqueApiError as exc: - print_error(f"Validation failed: {exc}") - raise SystemExit(1) from None + abort_api_error(f"Validation of ruleset '{file.name}' failed", exc) # `try/finally` so a Ctrl-C or unexpected exception between create and # delete still cleans up the temp ruleset on the server. try: - 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: @@ -226,7 +251,7 @@ def export_bundle( client = get_client(profile) # `export_configuration` is not yet wrapped in datamasque-python; hit the endpoint directly. response = client.make_request("GET", "/api/export/v1/") - output_path.write_bytes(response.content) + write_bytes_or_abort(output_path, response.content) print_success(f"Bundle exported to {output_path}") @@ -252,7 +277,7 @@ def import_bundle( pass `--overwrite-*` flags to replace existing entries. """ if not is_confirmed: - typer.confirm("This will modify rulesets, libraries, and seed files. Continue?", abort=True) + confirm_or_abort("This will modify rulesets, libraries, and seed files. Continue?") client = get_client(profile) # `/api/import/v1/` expects a multipart `zip_archive` upload plus three @@ -287,6 +312,36 @@ def import_bundle( render_output(summary, is_json=False, title="Import summary") +@app.command("status") +def show_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.") + + @app.command("generate") def generate_ruleset( request_file: Path = typer.Option( @@ -301,15 +356,18 @@ 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 = read_json_object_or_abort(request_file) - 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) + write_text_or_abort(output, yaml_content) print_success(f"Generated ruleset written to {output}") else: typer.echo(yaml_content) diff --git a/src/datamasque_cli/commands/runs.py b/src/datamasque_cli/commands/runs.py index dc124aa..56304c5 100644 --- a/src/datamasque_cli/commands/runs.py +++ b/src/datamasque_cli/commands/runs.py @@ -10,14 +10,14 @@ import typer from datamasque.client import DataMasqueClient, RunId -from datamasque.client.exceptions import DataMasqueApiError, RunNotCancellableError +from datamasque.client.exceptions import DataMasqueApiError, InvalidLibraryError, RunNotCancellableError from datamasque.client.models.connection import ConnectionConfig from datamasque.client.models.runs import MaskingRunOptions, MaskingRunRequest, RunInfo from datamasque_cli.client import get_client +from datamasque_cli.errors import ErrorCode, abort, abort_api_error, abort_if_not_found +from datamasque_cli.fileio import write_text_or_abort from datamasque_cli.output import ( - ErrorCode, - abort, console, print_error, print_json, @@ -60,6 +60,35 @@ def _format_run_dict(run_data: dict[str, object], *, is_styled: bool = False) -> } +def _start_run_or_abort(client: DataMasqueClient, request: MaskingRunRequest) -> RunId: + """Start a masking run, or abort with the server's reason for refusing it.""" + try: + return client.start_masking_run(request) + except InvalidLibraryError as exc: + abort(str(exc), code=ErrorCode.INVALID_INPUT) + except DataMasqueApiError as exc: + abort_api_error("Failed to start the run", exc) + + +def _get_run_or_abort(client: DataMasqueClient, run_id: int) -> RunInfo: + """Return the run, or abort when the server rejects the lookup.""" + try: + return client.get_run_info(RunId(run_id)) + except DataMasqueApiError as exc: + abort_if_not_found(exc, f"Run {run_id}") + abort_api_error(f"Could not read run {run_id}", exc) + + +def _get_run_log_or_abort(client: DataMasqueClient, run_id: int) -> str: + """Return the run's log, or abort when the server rejects the request.""" + try: + log: str = client.get_run_log(RunId(run_id)) + except DataMasqueApiError as exc: + abort_if_not_found(exc, f"Run {run_id}") + abort_api_error(f"Could not read the log for run {run_id}", exc) + return log + + def _resolve_connection(client: DataMasqueClient, name_or_id: str) -> ConnectionConfig: """Return the connection matching `name_or_id`, preferring name.""" connections = client.list_connections() @@ -209,12 +238,12 @@ def start_run( destination_connection=destination_id, options=MaskingRunOptions.model_validate(_parse_options(options)), ) - run_id = client.start_masking_run(run_request) + run_id = _start_run_or_abort(client, run_request) print_success(f"Run {run_id} started ({run_name}).") if is_background: if should_emit_json(is_json): - print_json({"id": int(run_id), "status": "queued"}) + print_json({"id": int(run_id)}) return _wait_for_run(client, run_id, is_json=is_json, ruleset=ruleset, connection=connection) @@ -228,7 +257,7 @@ def run_status( ) -> None: """Get status of a masking run.""" client = get_client(profile) - run = client.get_run_info(RunId(run_id)) + run = _get_run_or_abort(client, run_id) # Skip rich styling tags when output will be JSON — otherwise they'd appear # as literal "[status.finished]finished[/status.finished]" in the dump. is_styled = not should_emit_json(is_json) @@ -256,7 +285,10 @@ def list_runs( if limit is not None: params.append(f"limit={limit}") query = f"?{'&'.join(params)}" if params else "" - response = client.make_request("GET", f"/api/runs/{query}") + try: + response = client.make_request("GET", f"/api/runs/{query}") + except DataMasqueApiError as exc: + abort_api_error("Failed to list runs", exc) body = response.json() # The API may return a paginated envelope or a flat list depending on version. @@ -293,7 +325,7 @@ def run_logs( raw_mode = should_emit_json(is_json) if not follow: - log = client.get_run_log(RunId(run_id)) + log = _get_run_log_or_abort(client, run_id) if raw_mode: typer.echo(log) else: @@ -302,7 +334,7 @@ def run_logs( printed = 0 while True: - log = client.get_run_log(RunId(run_id)) + log = _get_run_log_or_abort(client, run_id) # Defend against server-side log rotation shrinking the buffer: # reset the cursor rather than slicing past the end. printed = min(printed, len(log)) @@ -314,7 +346,7 @@ def run_logs( _print_pretty_logs(chunk) printed = len(log) - info = client.get_run_info(RunId(run_id)) + info = _get_run_or_abort(client, run_id) if info.status.is_in_final_state: return time.sleep(_POLL_INTERVAL_SECONDS) @@ -331,6 +363,9 @@ def cancel_run( client.cancel_run(RunId(run_id)) except RunNotCancellableError as exc: abort(str(exc), code=ErrorCode.CONFLICT) + except DataMasqueApiError as exc: + abort_if_not_found(exc, f"Run {run_id}") + abort_api_error(f"Could not cancel run {run_id}", exc) print_success(f"Run {run_id} cancellation requested.") @@ -358,12 +393,12 @@ def run_report( f"Check status with `dm run status {run_id}`." ), ) - raise + abort_api_error(f"Failed to download report for run {run_id}", exc) if output is None: typer.echo(report) else: - output.write_text(report) + write_text_or_abort(output, report) print_success(f"Run report written to {output}") @@ -391,7 +426,7 @@ def retry_run( connection or ruleset since then are picked up automatically. """ client = get_client(profile) - original = client.get_run_info(RunId(run_id)) + original = _get_run_or_abort(client, run_id) source_id = original.source_connection.id ruleset_id = original.ruleset @@ -416,12 +451,12 @@ def retry_run( destination_connection=str(destination_id) if destination_id else None, options=MaskingRunOptions.model_validate(options), ) - new_run_id = client.start_masking_run(run_request) + new_run_id = _start_run_or_abort(client, run_request) print_success(f"Run {new_run_id} started (retry of {run_id}, {run_name}).") if is_background: if should_emit_json(is_json): - print_json({"id": int(new_run_id), "status": "queued"}) + print_json({"id": int(new_run_id)}) return _wait_for_run(client, new_run_id, is_json=is_json) @@ -455,7 +490,7 @@ def _wait_for_run( with console.status(f"Waiting for run {run_id}...") as spinner: while True: - run = client.get_run_info(run_id) + run = _get_run_or_abort(client, run_id) if run.status.is_in_final_state: break spinner.update(f"Run {run_id}: {run.status.value}") diff --git a/src/datamasque_cli/commands/seeds.py b/src/datamasque_cli/commands/seeds.py index 1999a05..3f07b59 100644 --- a/src/datamasque_cli/commands/seeds.py +++ b/src/datamasque_cli/commands/seeds.py @@ -8,7 +8,8 @@ from datamasque.client.models.files import SeedFile from datamasque_cli.client import get_client -from datamasque_cli.output import ErrorCode, abort, print_success, render_output +from datamasque_cli.errors import ErrorCode, abort, confirm_or_abort +from datamasque_cli.output import print_success, render_output app = typer.Typer(help="Manage seed files.", no_args_is_help=True) @@ -50,7 +51,7 @@ def delete_seed( abort(f"Seed file '{filename}' not found.", code=ErrorCode.NOT_FOUND) if not is_confirmed: - typer.confirm(f"Delete seed file '{filename}'?", abort=True) + confirm_or_abort(f"Delete seed file '{filename}'?") client.delete_file_if_exists(match) print_success(f"Seed file '{filename}' deleted.") diff --git a/src/datamasque_cli/commands/system.py b/src/datamasque_cli/commands/system.py index 6e39e85..5eb527c 100644 --- a/src/datamasque_cli/commands/system.py +++ b/src/datamasque_cli/commands/system.py @@ -10,15 +10,8 @@ from datamasque_cli.client import get_client, get_unauthenticated_client from datamasque_cli.commands.rulesets import export_bundle, import_bundle -from datamasque_cli.output import ( - ErrorCode, - abort, - print_json, - print_success, - print_warning, - render_output, - should_emit_json, -) +from datamasque_cli.errors import ErrorCode, abort, abort_api_error +from datamasque_cli.output import print_json, print_success, print_warning, render_output, should_emit_json app = typer.Typer(help="System administration commands.", no_args_is_help=True) @@ -96,7 +89,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") @@ -134,7 +134,7 @@ def admin_install( code=ErrorCode.CONFLICT, hint="Use `dm auth login` to sign in as an existing user.", ) - raise + abort_api_error("Admin install failed", e) print_success(f"Admin user '{username}' created.") diff --git a/src/datamasque_cli/commands/users.py b/src/datamasque_cli/commands/users.py index 833186e..ce5ff8f 100644 --- a/src/datamasque_cli/commands/users.py +++ b/src/datamasque_cli/commands/users.py @@ -6,7 +6,8 @@ from datamasque.client.models.user import User, UserRole from datamasque_cli.client import get_client -from datamasque_cli.output import ErrorCode, abort, print_success, render_output +from datamasque_cli.errors import ErrorCode, abort, confirm_or_abort +from datamasque_cli.output import print_success, render_output app = typer.Typer(help="Manage users.", no_args_is_help=True) @@ -63,7 +64,7 @@ def delete_user( abort(f"User '{username}' not found.", code=ErrorCode.NOT_FOUND) if not is_confirmed: - typer.confirm(f"Delete user '{username}'?", abort=True) + confirm_or_abort(f"Delete user '{username}'?") client.delete_user_by_username_if_exists(username) print_success(f"User '{username}' deleted.") diff --git a/src/datamasque_cli/errors.py b/src/datamasque_cli/errors.py new file mode 100644 index 0000000..e3c834b --- /dev/null +++ b/src/datamasque_cli/errors.py @@ -0,0 +1,194 @@ +from __future__ import annotations + +import json +from collections.abc import Mapping +from enum import IntEnum, StrEnum +from http import HTTPStatus +from typing import Any, NoReturn, TypeVar + +import typer +from datamasque.client.exceptions import DataMasqueApiError +from datamasque.client.models.discovery_config import DiscoveryConfigId +from datamasque.client.models.discovery_config_library import DiscoveryConfigLibraryId +from datamasque.client.models.ruleset import RulesetId +from datamasque.client.models.ruleset_library import RulesetLibraryId +from datamasque.client.models.status import ValidationErrorDetails, ValidationStatus +from pydantic import BaseModel, ConfigDict, JsonValue + +from datamasque_cli.output import console, is_agent_context, print_error + +AnyId = TypeVar("AnyId", DiscoveryConfigId, DiscoveryConfigLibraryId, RulesetId, RulesetLibraryId) + + +class ErrorCode(StrEnum): + """Stable, machine-readable error categories.""" + + ERROR = "error" + NOT_FOUND = "not_found" + INVALID_INPUT = "invalid_input" + AMBIGUOUS = "ambiguous" + AUTH_REQUIRED = "auth_required" + AUTH_FAILED = "auth_failed" + CONFLICT = "conflict" + TRANSPORT_ERROR = "transport_error" + CANCELLED = "cancelled" + FORBIDDEN = "forbidden" + + +class ExitCode(IntEnum): + """Every process exit status the CLI can return.""" + + OK = 0 + ERROR = 1 + USAGE_ERROR = 2 + NOT_FOUND = 3 + INVALID_INPUT = 4 + AMBIGUOUS = 5 + AUTH_REQUIRED = 6 + AUTH_FAILED = 7 + CONFLICT = 8 + TRANSPORT_ERROR = 9 + CANCELLED = 10 + FORBIDDEN = 11 + + +# Stable across minor versions so agents can branch on them. `OK` and `USAGE_ERROR` +# are absent because `abort()` never produces them; typer returns 2 by itself. +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, + ErrorCode.CANCELLED: ExitCode.CANCELLED, + ErrorCode.FORBIDDEN: ExitCode.FORBIDDEN, +} + + +def abort(message: str, *, code: ErrorCode = ErrorCode.ERROR, hint: str | None = None) -> NoReturn: + """Print an error and exit with the exit code mapped to `code`. + + In agent mode, emits a structured error envelope to stderr: + {"error": {"code": "...", "message": "...", "hint": "..."}} + In human mode, prints a red 'Error: …' line plus an optional hint. + + `code` is an `ErrorCode` member (StrEnum), so it serializes directly + into the envelope's `error.code` field as the underlying string. + """ + if is_agent_context(): + envelope: dict[str, Any] = {"error": {"code": code, "message": message}} + if hint: + envelope["error"]["hint"] = hint + typer.echo(json.dumps(envelope), err=True) + else: + print_error(message) + if hint: + console.print(f"[dim]Hint: {hint}[/dim]") + raise SystemExit(EXIT_CODE_BY_ERROR[code]) + + +def confirm_or_abort(message: str) -> None: + """Ask `message`, and abort with `cancelled` when the answer is no.""" + if typer.confirm(message): + return + abort("Cancelled.", code=ErrorCode.CANCELLED) + + +def require_id_or_abort(id_value: AnyId | None, subject: str) -> AnyId: + """Return `subject`'s id, or abort when it is absent.""" + if id_value is None: + abort(f"Server returned {subject} without an id.", code=ErrorCode.ERROR) + return id_value + + +class _ValidationEntry(BaseModel): + """One entry in a validation error list.""" + + model_config = ConfigDict(extra="ignore") + + loc: list[str | int] = [] + msg: str + + +class _ErrorBody(BaseModel): + """The shapes a DataMasque error response body takes.""" + + model_config = ConfigDict(extra="allow") + + detail: str | list[_ValidationEntry] | None = None + error: str | None = None + + +def _format_validation_errors(errors: list[_ValidationEntry]) -> str: + """Render each entry as `field.path: message`, joined with `; `.""" + parts = [] + for error in errors: + location = ".".join(str(part) for part in error.loc if part != "body") + parts.append(f"{location}: {error.msg}" if location else error.msg) + return "; ".join(parts) + + +def _format_field_errors(fields: Mapping[str, JsonValue]) -> str | None: + """Render each field error as `field: message`, joined with `; `.""" + parts = [ + str(message) if field == "non_field_errors" else f"{field}: {message}" + for field, messages in fields.items() + if isinstance(messages, list) + for message in messages + ] + return "; ".join(parts) or None + + +def extract_server_error_reason(exc: DataMasqueApiError) -> str | None: + """Return the reason the server gave for a failed request, or `None` when there is none.""" + try: + body = _ErrorBody.model_validate(exc.response.json()) + except ValueError: + return None + if isinstance(body.detail, str): + return body.detail + if body.detail: + return _format_validation_errors(body.detail) + return body.error or _format_field_errors(body.model_extra or {}) + + +# The `ErrorCode` each HTTP status means. Anything unlisted is `ERROR`. +_ERROR_CODE_BY_STATUS: Mapping[int, ErrorCode] = { + HTTPStatus.BAD_REQUEST: ErrorCode.INVALID_INPUT, + HTTPStatus.UNAUTHORIZED: ErrorCode.AUTH_FAILED, + HTTPStatus.FORBIDDEN: ErrorCode.FORBIDDEN, + HTTPStatus.NOT_FOUND: ErrorCode.NOT_FOUND, + HTTPStatus.CONFLICT: ErrorCode.CONFLICT, + HTTPStatus.UNPROCESSABLE_ENTITY: ErrorCode.INVALID_INPUT, +} + + +def abort_api_error(prefix: str, exc: DataMasqueApiError, *, conflict_hint: str | None = None) -> NoReturn: + """Abort with DataMasque's explanation of a failed request.""" + reason = extract_server_error_reason(exc) or str(exc) + code = _ERROR_CODE_BY_STATUS.get(exc.response.status_code, ErrorCode.ERROR) + hint = "Run `dm auth login` to sign in again." if code is ErrorCode.AUTH_FAILED else None + + if code is ErrorCode.CONFLICT: + # Prefixing a conflict repeats what the reason already says. + abort(reason, code=code, hint=conflict_hint) + abort(f"{prefix}: {reason}", code=code, hint=hint) + + +def abort_if_not_found(exc: DataMasqueApiError, subject: str) -> None: + """Abort when the server says `subject` does not exist.""" + if exc.response.status_code == HTTPStatus.NOT_FOUND: + abort(f"{subject} not found.", code=ErrorCode.NOT_FOUND) + + +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/src/datamasque_cli/fileio.py b/src/datamasque_cli/fileio.py new file mode 100644 index 0000000..a442fe3 --- /dev/null +++ b/src/datamasque_cli/fileio.py @@ -0,0 +1,73 @@ +"""Reading and writing the files a user passes on the command line. + +Every read and write goes through here, so a bad path, a bad encoding, or bad +content ends the command with a coded error naming the file, not a traceback. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +from pydantic import JsonValue + +from datamasque_cli.errors import ErrorCode, abort + +# Mirrors the server's limit: at or above this, validation is queued and returns no verdict. +MAX_SYNC_VALIDATION_KIB = 60 +_BYTES_PER_KIB = 1024 + + +def abort_if_too_large_for_sync_validation( + yaml_content: str, file: Path, *, create_command: str, status_command: str +) -> None: + """Abort when `yaml_content` is too large for the server to validate synchronously.""" + size = len(yaml_content.encode("utf-8")) + if size < MAX_SYNC_VALIDATION_KIB * _BYTES_PER_KIB: + return + abort( + f"{file} is {size // _BYTES_PER_KIB} KiB; " + f"validation for YAML of {MAX_SYNC_VALIDATION_KIB} KiB or larger runs asynchronously.", + code=ErrorCode.INVALID_INPUT, + hint=f"Create it with `{create_command}`, then check `{status_command}` until validation finishes.", + ) + + +def read_text_or_abort(file: Path) -> str: + """Read `file` as UTF-8, and abort when it cannot be read or decoded.""" + try: + content = file.read_text(encoding="utf-8") + except UnicodeDecodeError: + abort(f"{file} is not valid UTF-8.", code=ErrorCode.INVALID_INPUT, hint="Re-save the file as UTF-8.") + except OSError as exc: + code = ErrorCode.NOT_FOUND if isinstance(exc, FileNotFoundError) else ErrorCode.INVALID_INPUT + abort(f"Could not read {file}: {exc.strerror or exc}", code=code) + + if not content.strip(): + abort(f"{file} is empty.", code=ErrorCode.INVALID_INPUT) + return content + + +def read_json_object_or_abort(file: Path) -> dict[str, JsonValue]: + """Read `file` as a UTF-8 JSON object, and abort when it cannot be read or parsed.""" + content = read_text_or_abort(file) + try: + parsed: JsonValue = json.loads(content) + except json.JSONDecodeError as exc: + abort(f"{file} is not valid JSON: {exc}", code=ErrorCode.INVALID_INPUT) + if not isinstance(parsed, dict): + abort(f"{file} must contain a JSON object.", code=ErrorCode.INVALID_INPUT) + return parsed + + +def write_bytes_or_abort(path: Path, content: bytes) -> None: + """Write `content` to `path`, and abort when the path cannot be written.""" + try: + path.write_bytes(content) + except OSError as exc: + abort(f"Could not write {path}: {exc.strerror or exc}", code=ErrorCode.INVALID_INPUT) + + +def write_text_or_abort(path: Path, content: str) -> None: + """Write `content` to `path` as UTF-8, and abort when the path cannot be written.""" + write_bytes_or_abort(path, content.encode("utf-8")) diff --git a/src/datamasque_cli/main.py b/src/datamasque_cli/main.py index 8efdbe3..8222ebc 100644 --- a/src/datamasque_cli/main.py +++ b/src/datamasque_cli/main.py @@ -9,11 +9,15 @@ 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 datamasque.client.exceptions import ( + DataMasqueApiError, + DataMasqueException, + DataMasqueTransportError, +) from rich.console import Console from typer.main import get_command @@ -30,7 +34,17 @@ system, users, ) +from datamasque_cli.errors import ErrorCode, abort, abort_api_error from datamasque_cli.output import print_json, should_emit_json, stdout_console +from datamasque_cli.protocols import ( + Argument, + ArgumentEntry, + CommandEntry, + CompactEntry, + Group, + Option, + OptionEntry, +) app = typer.Typer( name="dm", @@ -60,42 +74,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 isinstance(param, 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 isinstance(param, 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 +121,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}) @@ -130,5 +140,16 @@ def catalog( stdout_console.print(f" [bold]{item['path']:<{width}}[/bold] [dim]{item['help']}[/dim]") +def main() -> None: + try: + app() + except DataMasqueApiError as exc: + abort_api_error("Request failed", exc) + except DataMasqueTransportError as exc: + abort(str(exc), code=ErrorCode.TRANSPORT_ERROR) + except DataMasqueException as exc: + abort(str(exc), code=ErrorCode.ERROR) + + if __name__ == "__main__": - app() + main() diff --git a/src/datamasque_cli/output.py b/src/datamasque_cli/output.py index 58dafaa..d52f8d4 100644 --- a/src/datamasque_cli/output.py +++ b/src/datamasque_cli/output.py @@ -14,8 +14,7 @@ import json import os import sys -from enum import StrEnum -from typing import Any, NoReturn +from typing import Any import typer from rich.console import Console @@ -46,40 +45,6 @@ _REDACTED = "" -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. - """ - - ERROR = "error" - NOT_FOUND = "not_found" - INVALID_INPUT = "invalid_input" - AMBIGUOUS = "ambiguous" - AUTH_REQUIRED = "auth_required" - AUTH_FAILED = "auth_failed" - CONFLICT = "conflict" - 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, -} - - def redact_sensitive_fields(data: dict[str, Any]) -> dict[str, Any]: """Return a copy of `data` with values of sensitive-named keys replaced by ``. @@ -226,25 +191,3 @@ def render_output( print_kv(data, title=title) else: typer.echo(data) - - -def abort(message: str, *, code: ErrorCode = ErrorCode.ERROR, hint: str | None = None) -> NoReturn: - """Print an error and exit with the exit code mapped to `code`. - - In agent mode, emits a structured error envelope to stderr: - {"error": {"code": "...", "message": "...", "hint": "..."}} - In human mode, prints a red 'Error: …' line plus an optional hint. - - `code` is an `ErrorCode` member (StrEnum), so it serializes directly - into the envelope's `error.code` field as the underlying string. - """ - if is_agent_context(): - envelope: dict[str, Any] = {"error": {"code": code, "message": message}} - if hint: - envelope["error"]["hint"] = hint - typer.echo(json.dumps(envelope), err=True) - else: - print_error(message) - if hint: - console.print(f"[dim]Hint: {hint}[/dim]") - raise SystemExit(EXIT_CODES[code]) diff --git a/src/datamasque_cli/protocols.py b/src/datamasque_cli/protocols.py new file mode 100644 index 0000000..00bae4a --- /dev/null +++ b/src/datamasque_cli/protocols.py @@ -0,0 +1,69 @@ +from __future__ import annotations + +from typing import Protocol, TypedDict, runtime_checkable + + +class Param(Protocol): + """The attributes every parameter has.""" + + name: str | None + required: bool + + +@runtime_checkable +class Option(Param, Protocol): + """A parameter passed by flag.""" + + opts: list[str] + help: str | None + is_flag: bool + + +@runtime_checkable +class Argument(Param, Protocol): + """A positional parameter.""" + + +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_auth.py b/tests/commands/test_auth.py index 102dff0..fbdbd9d 100644 --- a/tests/commands/test_auth.py +++ b/tests/commands/test_auth.py @@ -5,6 +5,7 @@ from typer.testing import CliRunner from datamasque_cli.config import Config, Profile +from datamasque_cli.errors import ExitCode from datamasque_cli.main import app from tests.conftest import make_config @@ -65,26 +66,26 @@ def test_login_writes_to_named_profile( def test_login_rejects_url_without_scheme(_mock_load: MagicMock, mock_save: MagicMock, runner: CliRunner) -> None: result = runner.invoke(app, ["auth", "login"], input="localhost\n") - assert result.exit_code != 0 + assert result.exit_code == ExitCode.INVALID_INPUT assert "http://" in result.stderr mock_save.assert_not_called() def test_login_rejects_url_flag(runner: CliRunner) -> None: result = runner.invoke(app, ["auth", "login", "--url", "https://x"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.USAGE_ERROR assert "no such option" in result.stderr.lower() def test_login_rejects_username_flag(runner: CliRunner) -> None: result = runner.invoke(app, ["auth", "login", "--username", "admin"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.USAGE_ERROR assert "no such option" in result.stderr.lower() def test_login_rejects_password_flag(runner: CliRunner) -> None: result = runner.invoke(app, ["auth", "login", "--password", "secret"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.USAGE_ERROR assert "no such option" in result.stderr.lower() @@ -106,7 +107,7 @@ def test_logout_falls_back_to_remaining_profile(mock_load: MagicMock, mock_save: @patch(f"{MODULE}.load_config", return_value=Config()) def test_logout_nonexistent_profile_aborts(_mock_load: MagicMock, _mock_save: MagicMock, runner: CliRunner) -> None: result = runner.invoke(app, ["auth", "logout", "--profile", "nope"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.NOT_FOUND # -- use ------------------------------------------------------------------- @@ -127,4 +128,4 @@ def test_use_profile_switches_active(mock_load: MagicMock, mock_save: MagicMock, @patch(f"{MODULE}.load_config", return_value=Config()) def test_use_profile_nonexistent_aborts(_mock_load: MagicMock, _mock_save: MagicMock, runner: CliRunner) -> None: result = runner.invoke(app, ["auth", "use", "nonexistent"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.NOT_FOUND 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_connections.py b/tests/commands/test_connections.py index e0c001b..bb70e2b 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, @@ -14,6 +15,7 @@ from typer.testing import CliRunner from datamasque_cli.commands.connections import _format_role +from datamasque_cli.errors import ExitCode from datamasque_cli.main import app MODULE = "datamasque_cli.commands.connections" @@ -137,7 +139,7 @@ def test_create_connection_mounted_share(mock_get_client: MagicMock, runner: Cli def test_create_connection_missing_name_aborts(mock_get_client: MagicMock, runner: CliRunner) -> None: mock_get_client.return_value = MagicMock() result = runner.invoke(app, ["connections", "create", "--type", "database"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.INVALID_INPUT @patch(f"{MODULE}.get_client") @@ -251,7 +253,7 @@ def test_delete_connection_aborts_when_missing( result = runner.invoke(app, ["connections", "delete", "no_such_conn", "--yes"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.NOT_FOUND mock_client.delete_connection_by_name_if_exists.assert_not_called() @@ -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.INVALID_INPUT + 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 @@ -281,7 +299,7 @@ def test_test_connection_aborts_when_missing( result = runner.invoke(app, ["connections", "test", "no_such_conn"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.NOT_FOUND mock_client.make_request.assert_not_called() @@ -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"}, ) @@ -314,7 +332,7 @@ def test_update_connection_aborts_without_any_fields( result = runner.invoke(app, ["connections", "update", "my_conn"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.INVALID_INPUT mock_client.make_request.assert_not_called() @@ -326,7 +344,7 @@ def test_update_connection_aborts_when_missing( result = runner.invoke(app, ["connections", "update", "no_such_conn", "--password", "x"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.NOT_FOUND mock_client.make_request.assert_not_called() diff --git a/tests/commands/test_discovery.py b/tests/commands/test_discovery.py index bb76651..4800d98 100644 --- a/tests/commands/test_discovery.py +++ b/tests/commands/test_discovery.py @@ -1,16 +1,60 @@ from __future__ import annotations +import json +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, + DiscoveryConfigNotFoundError, + InvalidDiscoveryConfigError, +) +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 datamasque.client.models.status import MaskingRunStatus from typer.testing import CliRunner +from datamasque_cli.errors import ExitCode from datamasque_cli.main import app MODULE = "datamasque_cli.commands.discovery" +def _make_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 _make_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() @@ -72,21 +116,163 @@ 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 +def _make_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=_make_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 = _make_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 + 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 = _make_file_report() + + result = runner.invoke(app, ["discover", "file-report", "7"]) assert result.exit_code == 0 - assert '"file": "a"' in out.read_text() + assert "phone" in result.stdout + assert "data.csv" in result.stdout + assert "safe_data_preview" not in result.stdout + + +# -- missing run output ---------------------------------------------------- + + +_OUTPUT_COMMANDS = [ + (["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"), + (["discover", "file-report", "42"], "get_file_data_discovery_report", 404, "file discovery report"), +] + + +def _client_missing_output(client_method: str, status: int, detail: str) -> MagicMock: + client = MagicMock() + response = MagicMock(status_code=status) + response.json.return_value = {"detail": detail} + getattr(client, client_method).side_effect = DataMasqueApiError(f"{status}", response=response) + return client + + +def _run_in_status(status: MaskingRunStatus) -> MagicMock: + run = MagicMock() + run.status = status + return run + + +@pytest.mark.parametrize(("command", "client_method", "status", "expected"), _OUTPUT_COMMANDS) +@patch(f"{MODULE}.get_client") +def test_output_missing_while_run_is_unfinished_says_to_wait( + mock_get_client: MagicMock, + runner: CliRunner, + command: list[str], + client_method: str, + status: int, + expected: str, +) -> None: + client = _client_missing_output(client_method, status, "not ready") + client.get_run_info.return_value = _run_in_status(MaskingRunStatus.running) + mock_get_client.return_value = client + + 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 yet; the run is running." in stderr + assert "dm run status 42" in stderr + + +@pytest.mark.parametrize(("command", "client_method", "status", "expected"), _OUTPUT_COMMANDS) +@patch(f"{MODULE}.get_client") +def test_output_missing_on_finished_run_reports_the_server_reason( + mock_get_client: MagicMock, + runner: CliRunner, + command: list[str], + client_method: str, + status: int, + expected: str, +) -> None: + client = _client_missing_output(client_method, status, "Schema discovery has not been run on this connection.") + client.get_run_info.return_value = _run_in_status(MaskingRunStatus.finished) + mock_get_client.return_value = client + + result = runner.invoke(app, command) + + expected_code = ExitCode.INVALID_INPUT if status == 400 else ExitCode.NOT_FOUND + assert result.exit_code == expected_code + stderr = " ".join(result.stderr.split()) + assert f"No {expected} available for run 42: Schema discovery has not been run on this connection." in stderr + assert "dm run status 42" not in stderr + + +@pytest.mark.parametrize(("command", "client_method", "status", "expected"), _OUTPUT_COMMANDS) +@patch(f"{MODULE}.get_client") +def test_output_missing_for_unknown_run_is_not_found( + mock_get_client: MagicMock, + runner: CliRunner, + command: list[str], + client_method: str, + status: int, + expected: str, +) -> None: + client = _client_missing_output(client_method, status, "not ready") + client.get_run_info.side_effect = DataMasqueApiError("404", response=MagicMock(status_code=404)) + mock_get_client.return_value = client + + result = runner.invoke(app, command) + + assert result.exit_code == ExitCode.NOT_FOUND + assert "Run 42 not found." in " ".join(result.stderr.split()) + + +@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 + response = MagicMock(status_code=500) + response.json.return_value = {"detail": "Report generation crashed."} + client.get_sdd_report.side_effect = DataMasqueApiError("500", response=response) + + result = runner.invoke(app, ["discover", "sdd-report", "42"]) + + assert result.exit_code == ExitCode.ERROR + assert "Report generation crashed." in " ".join(result.stderr.split()) + assert "Traceback" not in result.stderr # -- schema discovery trigger --------------------------------------------- @@ -110,6 +296,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} + + +@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.INVALID_INPUT + 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() @@ -124,6 +339,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( @@ -138,6 +354,7 @@ def test_schema_results_lists_with_flattened_rows(mock_get_client: MagicMock, ru SimpleNamespace(label="PII"), ], constraint="Primary", + safe_data_preview=None, ), ), ] @@ -150,3 +367,248 @@ 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="", + safe_data_preview=None, + ), + ), + SimpleNamespace( + id=2, + column="notes", + table="users", + schema_name="public", + data=SimpleNamespace( + data_type="text", + discovery_matches=[SimpleNamespace(label=None)], + constraint="", + safe_data_preview=None, + ), + ), + ] + + 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"] == "-" + + +@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=_make_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 ---------------------------------- + + +def _fake_config_lookup(**ids_by_type: str) -> Callable[[str, DiscoveryConfigType], SimpleNamespace | None]: + """Return a `get_discovery_config_by_name` side effect that knows only the given types.""" + + def lookup(name: str, config_type: DiscoveryConfigType) -> SimpleNamespace | None: + config_id = ids_by_type.get(config_type.value) + if config_id is None: + return None + return SimpleNamespace(id=config_id, name=name, config_type=config_type) + + return lookup + + +@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.get_discovery_config_by_name.side_effect = _fake_config_lookup(database="cfg-1") + 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.get_discovery_config_by_name.side_effect = _fake_config_lookup(file="cfg-2") + + result = runner.invoke(app, ["discover", "schema", "my_db", "--config", "docs"]) + + assert result.exit_code == ExitCode.INVALID_INPUT + assert "exists as file" in " ".join(result.stderr.split()) + client.start_schema_discovery_run_from_config.assert_not_called() + + +@patch(f"{MODULE}.get_client") +def test_schema_config_not_found_aborts(mock_get_client: MagicMock, runner: CliRunner) -> None: + """Neither type holds the name, so this is not-found rather than a type mismatch.""" + client = MagicMock() + mock_get_client.return_value = client + client.list_connections.return_value = [SimpleNamespace(id="abc-123", name="my_db", mask_type="database")] + client.get_discovery_config_by_name.side_effect = _fake_config_lookup() + + result = runner.invoke(app, ["discover", "schema", "my_db", "--config", "nope"]) + + assert result.exit_code == ExitCode.NOT_FOUND + 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.get_discovery_config_by_name.side_effect = _fake_config_lookup(file="cfg-3") + 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" + + +_CONFIG_RUN_STARTS = [ + ( + ["discover", "schema", "my_db", "--config", "emp"], + SimpleNamespace(id="abc-123", name="my_db", mask_type="database"), + {"database": "cfg-1"}, + "start_schema_discovery_run_from_config", + ), + ( + ["discover", "file", "my_files", "--config", "docs"], + SimpleNamespace(id="fs-1", name="my_files", mask_type="file"), + {"file": "cfg-3"}, + "start_file_data_discovery_run_from_config", + ), +] + + +@pytest.mark.parametrize(("command", "connection", "config_ids", "start_method"), _CONFIG_RUN_STARTS) +@pytest.mark.parametrize( + ("error", "expected_code"), + [ + (DiscoveryConfigNotFoundError, ExitCode.NOT_FOUND), + (InvalidDiscoveryConfigError, ExitCode.INVALID_INPUT), + ], +) +@patch(f"{MODULE}.get_client") +def test_config_run_start_error_keeps_its_own_code( + mock_get_client: MagicMock, + runner: CliRunner, + error: type[DataMasqueApiError], + expected_code: ExitCode, + command: list[str], + connection: SimpleNamespace, + config_ids: dict[str, str], + start_method: str, +) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_connections.return_value = [connection] + client.get_discovery_config_by_name.side_effect = _fake_config_lookup(**config_ids) + getattr(client, start_method).side_effect = error( + "run failed to start: the config is unusable", response=MagicMock(status_code=400) + ) + + result = runner.invoke(app, command) + + assert result.exit_code == expected_code + assert "the config is unusable" in " ".join(result.stderr.split()) + + +@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} + + +@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..fd8d374 --- /dev/null +++ b/tests/commands/test_discovery_config_libraries.py @@ -0,0 +1,291 @@ +from __future__ import annotations + +from http import HTTPStatus +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.status import ValidationStatus +from typer.testing import CliRunner + +from datamasque_cli.errors import ExitCode +from datamasque_cli.main import app + +MODULE = "datamasque_cli.commands.discovery_config_libraries" + + +def _make_library( + name: str, + namespace: str = "", + library_id: str = "lib-uuid", + is_valid: ValidationStatus | None = ValidationStatus.valid, + usage_count: int = 0, + yaml: str | None = None, + validation_error: str | None = None, +) -> SimpleNamespace: + """A discovery config library as the server returns it, carrying its validation status.""" + return SimpleNamespace( + id=library_id, + name=name, + namespace=namespace, + is_valid=is_valid, + validation_error=validation_error, + usage_count=usage_count, + created=None, + modified=None, + yaml=yaml, + ) + + +@patch(f"{MODULE}.get_client") +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 = [ + _make_library("finance", namespace="org", usage_count=3), + ] + + result = runner.invoke(app, ["discover", "libraries", "list", "--json"]) + + 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.get_discovery_config_library_by_name.return_value = _make_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_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.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_by_name.assert_called_once_with("finance", "") + + +@patch(f"{MODULE}.get_client") +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" + lib.write_text("labels: []\n") + + result = runner.invoke( + app, + ["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.get_discovery_config_library_by_name.return_value = _make_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_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 = _make_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 = _make_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") +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.return_value = _make_library("finance", is_valid=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_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_library.side_effect = DataMasqueApiError( + "API request failed with status 400", response=response + ) + lib = tmp_path / "lib.yaml" + lib.write_text("labels: []\n") + + result = runner.invoke(app, ["discover", "libraries", "validate", "-f", str(lib)]) + + assert result.exit_code == ExitCode.INVALID_INPUT + assert "config_yaml: invalid" in " ".join(result.stderr.split()) + client.delete_discovery_config_library_by_id_if_exists.assert_not_called() + + +@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.create_discovery_config_library.return_value = _make_library( + "finance", 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)]) + + 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.return_value = _make_library("finance", is_valid=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 + + +@pytest.mark.parametrize( + ("is_valid", "validation_error"), + [ + (ValidationStatus.valid, None), + (ValidationStatus.invalid, "duplicate label 'email'"), + ], + ids=["valid", "invalid"], +) +@patch(f"{MODULE}.get_client") +def test_status_reports_state( + mock_get_client: MagicMock, + runner: CliRunner, + is_valid: ValidationStatus, + validation_error: str | None, +) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.get_discovery_config_library_by_name.return_value = _make_library( + "finance", is_valid=is_valid, validation_error=validation_error + ) + + result = runner.invoke(app, ["discover", "libraries", "status", "finance", "--json"]) + + assert result.exit_code == ExitCode.OK + assert f'"status": "{is_valid.value}"' in result.stdout + if validation_error: + assert validation_error in result.stdout diff --git a/tests/commands/test_discovery_configs.py b/tests/commands/test_discovery_configs.py new file mode 100644 index 0000000..e990f7f --- /dev/null +++ b/tests/commands/test_discovery_configs.py @@ -0,0 +1,314 @@ +from __future__ import annotations + +from http import HTTPStatus +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.discovery_config import DiscoveryConfigType +from datamasque.client.models.status import ValidationStatus +from typer.testing import CliRunner + +from datamasque_cli.errors import ExitCode +from datamasque_cli.main import app + +MODULE = "datamasque_cli.commands.discovery_configs" + + +def _make_discovery_config( + name: str, + config_type: DiscoveryConfigType = DiscoveryConfigType.database, + config_id: str = "cfg-uuid", + is_valid: ValidationStatus | None = ValidationStatus.valid, + yaml: str | None = None, + validation_error: str | None = None, +) -> SimpleNamespace: + """A discovery config as the server returns it, carrying its validation status.""" + return SimpleNamespace( + id=config_id, + name=name, + config_type=config_type, + is_valid=is_valid, + validation_error=validation_error, + validation_error_details=[], + created=None, + modified=None, + yaml=yaml, + ) + + +@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_ERROR + 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() + mock_get_client.return_value = client + client.list_discovery_configs.return_value = [ + _make_discovery_config("emp", DiscoveryConfigType.database), + _make_discovery_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 = [_make_discovery_config("emp")] + client.get_discovery_config.return_value = _make_discovery_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 = [ + _make_discovery_config("shared", DiscoveryConfigType.database, config_id="a"), + _make_discovery_config("shared", DiscoveryConfigType.file, config_id="b"), + ] + + result = runner.invoke(app, ["discover", "configs", "get", "shared"]) + + assert result.exit_code == ExitCode.AMBIGUOUS + 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 = [ + _make_discovery_config("shared", DiscoveryConfigType.database, config_id="a"), + _make_discovery_config("shared", DiscoveryConfigType.file, config_id="b"), + ] + client.get_discovery_config.return_value = _make_discovery_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: 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 == ExitCode.NOT_FOUND + 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: Path) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_configs.return_value = [_make_discovery_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 = [_make_discovery_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 == ExitCode.NOT_FOUND + 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: Path) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.create_discovery_config.return_value = _make_discovery_config("emp", is_valid=ValidationStatus.valid) + 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 + 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: Path) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.create_discovery_config.return_value = _make_discovery_config( + "emp", 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 == 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.INVALID_INPUT + 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.return_value = _make_discovery_config("emp", is_valid=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: 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() + + +@pytest.mark.parametrize( + ("is_valid", "validation_error"), + [ + (ValidationStatus.valid, None), + (ValidationStatus.invalid, "unknown label 'foo'"), + (ValidationStatus.in_progress, None), + ], + ids=["valid", "invalid", "in_progress"], +) +@patch(f"{MODULE}.get_client") +def test_status_reports_state( + mock_get_client: MagicMock, + runner: CliRunner, + is_valid: ValidationStatus, + validation_error: str | None, +) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_discovery_configs.return_value = [ + _make_discovery_config("emp", is_valid=is_valid, validation_error=validation_error) + ] + + result = runner.invoke(app, ["discover", "configs", "status", "emp", "--json"]) + + assert result.exit_code == ExitCode.OK + assert f'"status": "{is_valid.value}"' in result.stdout + if validation_error: + assert validation_error in result.stdout diff --git a/tests/commands/test_files.py b/tests/commands/test_files.py index 08de9f8..cdfd7d5 100644 --- a/tests/commands/test_files.py +++ b/tests/commands/test_files.py @@ -6,6 +6,7 @@ from datamasque.client.models.files import SnowflakeKeyFile from typer.testing import CliRunner +from datamasque_cli.errors import ExitCode from datamasque_cli.main import app MODULE = "datamasque_cli.commands.files" @@ -32,11 +33,11 @@ def test_delete_file_aborts_when_missing(mock_get_client: MagicMock, runner: Cli result = runner.invoke(app, ["files", "delete", "snowflake-key", "nope", "--yes"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.NOT_FOUND client.delete_file_if_exists.assert_not_called() @patch(f"{MODULE}.get_client") def test_delete_file_rejects_unknown_type(mock_get_client: MagicMock, runner: CliRunner) -> None: result = runner.invoke(app, ["files", "delete", "not-a-type", "x", "--yes"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.USAGE_ERROR diff --git a/tests/commands/test_ifm.py b/tests/commands/test_ifm.py index 6081bc1..14fc313 100644 --- a/tests/commands/test_ifm.py +++ b/tests/commands/test_ifm.py @@ -6,9 +6,11 @@ from types import SimpleNamespace from unittest.mock import MagicMock, patch +import pytest from datamasque.client.exceptions import DataMasqueApiError from typer.testing import CliRunner +from datamasque_cli.errors import ErrorCode, ExitCode from datamasque_cli.main import app MODULE = "datamasque_cli.commands.ifm" @@ -171,7 +173,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() @@ -193,7 +195,7 @@ def test_delete_without_confirmation_aborts(mock_get_client: MagicMock, runner: result = runner.invoke(app, ["ifm", "delete", "p1"], input="n\n") - assert result.exit_code != 0 + assert result.exit_code == ExitCode.CANCELLED client.delete_ruleset_plan.assert_not_called() @@ -236,7 +238,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 @@ -251,12 +253,15 @@ def test_mask_rejects_non_list_input(mock_get_client: MagicMock, runner: CliRunn result = runner.invoke(app, ["ifm", "mask", "p1", "--data", str(data_file)]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.INVALID_INPUT client.mask.assert_not_called() @patch(f"{MODULE}.get_ifm_client") -def test_mask_aborts_when_data_file_missing(mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path) -> None: +def test_mask_aborts_when_data_file_missing( + mock_get_client: MagicMock, runner: CliRunner, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setenv("DM_OUTPUT", "json") client = MagicMock() mock_get_client.return_value = client @@ -264,9 +269,10 @@ def test_mask_aborts_when_data_file_missing(mock_get_client: MagicMock, runner: result = runner.invoke(app, ["ifm", "mask", "p1", "--data", str(missing)]) - assert result.exit_code != 0 - assert "Could not read mask input file" in result.stderr - assert "Traceback" not in result.stderr + assert result.exit_code == ExitCode.NOT_FOUND + payload = json.loads(result.stderr) + assert payload["error"]["code"] == ErrorCode.NOT_FOUND + assert payload["error"]["message"].startswith(f"Could not read {missing}: ") client.mask.assert_not_called() @@ -323,7 +329,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 +341,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 +353,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 +369,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 +388,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 +407,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 +423,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 +439,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 +458,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 +470,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 +486,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 +545,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 @@ -573,5 +579,5 @@ def test_create_rejects_invalid_log_level(mock_get_client: MagicMock, runner: Cl ["ifm", "create", "--name", "smoke", "--file", str(yaml_file), "--log-level", "TRACE"], ) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.USAGE_ERROR client.create_ruleset_plan.assert_not_called() diff --git a/tests/commands/test_ruleset_libraries.py b/tests/commands/test_ruleset_libraries.py index 522de9f..1003548 100644 --- a/tests/commands/test_ruleset_libraries.py +++ b/tests/commands/test_ruleset_libraries.py @@ -1,15 +1,65 @@ from __future__ import annotations +from http import HTTPStatus from types import SimpleNamespace from unittest.mock import MagicMock, patch +import pytest +from datamasque.client.exceptions import DataMasqueApiError +from datamasque.client.models.status import ValidationErrorDetails, ValidationStatus from typer.testing import CliRunner +from datamasque_cli.errors import ExitCode from datamasque_cli.main import app MODULE = "datamasque_cli.commands.ruleset_libraries" +def _make_ruleset_library( + name: str, + namespace: str = "", + library_id: str = "lib-uuid", + is_valid: ValidationStatus | None = None, + validation_errors: list[ValidationErrorDetails] | None = None, +) -> SimpleNamespace: + """A ruleset library as the server returns it, carrying its validation status.""" + return SimpleNamespace( + id=library_id, + name=name, + namespace=namespace, + is_valid=is_valid, + validation_errors=validation_errors or [], + ) + + +@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 = _make_ruleset_library("lib") + 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 + + @patch(f"{MODULE}.get_client") def test_delete_library_aborts_when_missing(mock_get_client: MagicMock, runner: CliRunner) -> None: client = MagicMock() @@ -18,7 +68,7 @@ def test_delete_library_aborts_when_missing(mock_get_client: MagicMock, runner: result = runner.invoke(app, ["libraries", "delete", "nope", "--yes"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.NOT_FOUND client.delete_ruleset_library_by_name_if_exists.assert_not_called() @@ -38,13 +88,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 = _make_ruleset_library("my-lib") + client.validate_ruleset_library.return_value = _make_ruleset_library("my-lib", is_valid=ValidationStatus.valid) result = runner.invoke(app, ["libraries", "validate", "my-lib"]) @@ -61,5 +106,74 @@ def test_validate_library_aborts_when_missing(mock_get_client: MagicMock, runner result = runner.invoke(app, ["libraries", "validate", "missing"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.NOT_FOUND 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 = _make_ruleset_library("my-lib") + client.validate_ruleset_library.return_value = _make_ruleset_library( + "my-lib", + is_valid=ValidationStatus.invalid, + validation_errors=[ + ValidationErrorDetails(message="unknown mask type 'from_nowhere'", line_number=3), + ValidationErrorDetails(message="duplicate anchor 'email'", line_number=None), + ], + ) + + result = runner.invoke(app, ["libraries", "validate", "my-lib"]) + + 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 + + +@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 = _make_ruleset_library("my-lib") + client.validate_ruleset_library.return_value = _make_ruleset_library( + "my-lib", is_valid=ValidationStatus.in_progress + ) + + result = runner.invoke(app, ["libraries", "validate", "my-lib"]) + + assert result.exit_code == 0 + assert "in_progress" in result.stderr + + +@pytest.mark.parametrize( + ("is_valid", "errors"), + [ + (ValidationStatus.valid, []), + ( + ValidationStatus.invalid, + [ValidationErrorDetails(message="Unknown mask `nope`.")], + ), + ], + ids=["valid", "invalid"], +) +@patch(f"{MODULE}.get_client") +def test_status_reports_state( + mock_get_client: MagicMock, + runner: CliRunner, + is_valid: ValidationStatus, + errors: list[ValidationErrorDetails], +) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.get_ruleset_library_by_name.return_value = _make_ruleset_library( + "lib", is_valid=is_valid, validation_errors=errors + ) + + result = runner.invoke(app, ["libraries", "status", "lib", "--json"]) + + assert result.exit_code == ExitCode.OK + assert f'"status": "{is_valid.value}"' in result.stdout + for error in errors: + assert error.message in result.stdout diff --git a/tests/commands/test_rulesets.py b/tests/commands/test_rulesets.py index 677f99f..8ba2fb1 100644 --- a/tests/commands/test_rulesets.py +++ b/tests/commands/test_rulesets.py @@ -4,10 +4,13 @@ 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 ValidationErrorDetails, ValidationStatus from typer.testing import CliRunner +from datamasque_cli.errors import ExitCode from datamasque_cli.main import app MODULE = "datamasque_cli.commands.rulesets" @@ -17,6 +20,30 @@ def _ruleset(id_: int, name: str, rs_type: RulesetType) -> SimpleNamespace: return SimpleNamespace(id=id_, name=name, ruleset_type=rs_type, yaml="") +def _make_ruleset_with_status( + is_valid: ValidationStatus | None, + validation_errors: list[ValidationErrorDetails] | None = None, + id_: int = 99, +) -> SimpleNamespace: + """A ruleset as the server returns it, carrying its validation status.""" + return SimpleNamespace( + id=id_, + name="demo", + ruleset_type=RulesetType.database, + is_valid=is_valid, + validation_errors=validation_errors or [], + ) + + +@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_ERROR + assert "is not one of" in result.output + mock_get_client.assert_not_called() + + # -- create (type resolution via server lookup) ---------------------------- @@ -33,7 +60,7 @@ def test_create_requires_type_when_ruleset_is_new( result = runner.invoke(app, ["rulesets", "create", "--name", "demo", "--file", str(yaml_file)]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.NOT_FOUND client.create_or_update_ruleset.assert_not_called() @@ -69,7 +96,7 @@ def test_create_requires_type_when_both_namespaces_hold_same_name( result = runner.invoke(app, ["rulesets", "create", "--name", "demo", "--file", str(yaml_file)]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.AMBIGUOUS client.create_or_update_ruleset.assert_not_called() @@ -128,7 +155,7 @@ def test_get_aborts_when_multiple_same_name_without_type(mock_get_client: MagicM ] result = runner.invoke(app, ["rulesets", "get", "demo"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.AMBIGUOUS @patch(f"{MODULE}.get_client") @@ -181,7 +208,7 @@ def test_delete_aborts_when_ambiguous(mock_get_client: MagicMock, runner: CliRun result = runner.invoke(app, ["rulesets", "delete", "demo", "--yes"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.AMBIGUOUS client.delete_ruleset_by_id_if_exists.assert_not_called() @@ -193,7 +220,7 @@ def test_delete_aborts_when_missing(mock_get_client: MagicMock, runner: CliRunne result = runner.invoke(app, ["rulesets", "delete", "demo", "--yes"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.NOT_FOUND client.delete_ruleset_by_id_if_exists.assert_not_called() @@ -229,7 +256,7 @@ def test_validate_requires_type_flag(mock_get_client: MagicMock, runner: CliRunn result = runner.invoke(app, ["rulesets", "validate", "--file", str(yaml_file)]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.USAGE_ERROR client.create_or_update_ruleset.assert_not_called() @@ -239,12 +266,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.return_value = _make_ruleset_with_status(ValidationStatus.valid) yaml_file = tmp_path / "rs.yaml" yaml_file.write_text("tasks:\n - type: mask_table\n") @@ -268,12 +290,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.return_value = _make_ruleset_with_status(ValidationStatus.valid) yaml_file = tmp_path / "rs.yaml" yaml_file.write_text("tasks:\n - type: mask_table\n") @@ -287,12 +304,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.return_value = _make_ruleset_with_status(ValidationStatus.valid) client.delete_ruleset_by_id_if_exists.side_effect = DataMasqueApiError("boom", response=MagicMock()) yaml_file = tmp_path / "rs.yaml" @@ -304,6 +316,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.return_value = _make_ruleset_with_status( + ValidationStatus.invalid, + [ + ValidationErrorDetails(message="unknown mask type 'from_nowhere'", line_number=7), + ValidationErrorDetails(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 == 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 + 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.return_value = _make_ruleset_with_status(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 ---------------------------------------- @@ -333,7 +389,7 @@ def test_import_bundle_requires_confirmation(mock_get_client: MagicMock, runner: result = runner.invoke(app, ["rulesets", "import-bundle", "--file", str(bundle)], input="n\n") - assert result.exit_code != 0 + assert result.exit_code == ExitCode.CANCELLED client.make_request.assert_not_called() @@ -390,3 +446,52 @@ 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 ---------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("is_valid", "errors"), + [ + (ValidationStatus.valid, []), + ( + ValidationStatus.invalid, + [ValidationErrorDetails(message="Missing `key` in `tasks`.", line_number=3)], + ), + (ValidationStatus.in_progress, []), + ], + ids=["valid", "invalid", "in_progress"], +) +@patch(f"{MODULE}.get_client") +def test_status_reports_state( + mock_get_client: MagicMock, + runner: CliRunner, + is_valid: ValidationStatus, + errors: list[ValidationErrorDetails], +) -> None: + client = MagicMock() + mock_get_client.return_value = client + client.list_rulesets.return_value = [_make_ruleset_with_status(is_valid, errors)] + + result = runner.invoke(app, ["rulesets", "status", "demo", "--json"]) + + assert result.exit_code == ExitCode.OK + assert f'"status": "{is_valid.value}"' in result.stdout + for error in errors: + assert error.message in result.stdout diff --git a/tests/commands/test_runs.py b/tests/commands/test_runs.py index 4d254aa..6fa8a0c 100644 --- a/tests/commands/test_runs.py +++ b/tests/commands/test_runs.py @@ -23,6 +23,7 @@ _resolve_connection_id, _resolve_ruleset_id, ) +from datamasque_cli.errors import ExitCode from datamasque_cli.main import app MODULE = "datamasque_cli.commands.runs" @@ -195,7 +196,7 @@ def test_start_run_aborts_on_destination_type_mismatch(mock_get_client: MagicMoc result = runner.invoke(app, ["run", "start", "-c", "db_src", "-r", "demo", "-d", "files_dst"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.INVALID_INPUT client.start_masking_run.assert_not_called() @@ -286,7 +287,7 @@ def test_start_run_aborts_when_file_source_has_no_destination(mock_get_client: M result = runner.invoke(app, ["run", "start", "-c", "files_src", "-r", "demo"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.INVALID_INPUT assert "destination" in result.stderr.lower() client.start_masking_run.assert_not_called() @@ -378,7 +379,7 @@ def test_retry_run_aborts_when_original_missing_ruleset(mock_get_client: MagicMo result = runner.invoke(app, ["run", "retry", "200"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.INVALID_INPUT client.start_masking_run.assert_not_called() @@ -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 ---------------------------------------------------- @@ -537,6 +538,75 @@ def test_run_report_aborts_with_friendly_message_on_404( result = runner.invoke(app, ["run", "report", "42"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.NOT_FOUND assert "No report available for run 42" in result.stderr assert "dm run status 42" in result.stderr + + +@patch(f"{MODULE}.get_client") +def test_run_report_reports_server_reason_on_other_errors( + mock_get_client: MagicMock, mock_client: MagicMock, runner: CliRunner +) -> None: + """Only a 404 means "no report yet" — other failures must show the server's reason.""" + mock_get_client.return_value = mock_client + response = MagicMock(status_code=500) + response.json.return_value = {"detail": "Report storage is unavailable."} + mock_client.get_run_report.side_effect = DataMasqueApiError("boom", response=response) + + result = runner.invoke(app, ["run", "report", "42"]) + + assert result.exit_code == ExitCode.ERROR + assert "Report storage is unavailable." in " ".join(result.stderr.split()) + assert "No report available" not in result.stderr + assert "Traceback" not in result.stderr + + +_UNKNOWN_RUN_COMMANDS = [ + (["run", "status", "42"], "get_run_info"), + (["run", "logs", "42"], "get_run_log"), + (["run", "cancel", "42"], "cancel_run"), + (["run", "retry", "42"], "get_run_info"), + (["run", "wait", "42"], "get_run_info"), +] + + +@pytest.mark.parametrize(("command", "client_method"), _UNKNOWN_RUN_COMMANDS) +@patch(f"{MODULE}.get_client") +def test_unknown_run_is_not_found( + mock_get_client: MagicMock, + mock_client: MagicMock, + runner: CliRunner, + command: list[str], + client_method: str, +) -> None: + mock_get_client.return_value = mock_client + response = MagicMock(status_code=404) + response.json.return_value = {"detail": "The requested run was not found."} + getattr(mock_client, client_method).side_effect = DataMasqueApiError("404", response=response) + + result = runner.invoke(app, command) + + assert result.exit_code == ExitCode.NOT_FOUND + assert "Run 42 not found." in " ".join(result.stderr.split()) + + +@pytest.mark.parametrize(("command", "client_method"), _UNKNOWN_RUN_COMMANDS) +@patch(f"{MODULE}.get_client") +def test_other_run_request_failures_report_the_server_reason( + mock_get_client: MagicMock, + mock_client: MagicMock, + runner: CliRunner, + command: list[str], + client_method: str, +) -> None: + mock_get_client.return_value = mock_client + response = MagicMock(status_code=500) + response.json.return_value = {"detail": "Run storage is unavailable."} + getattr(mock_client, client_method).side_effect = DataMasqueApiError("boom", response=response) + + result = runner.invoke(app, command) + + assert result.exit_code == ExitCode.ERROR + assert "Run storage is unavailable." in " ".join(result.stderr.split()) + assert "not found" not in result.stderr + assert "Traceback" not in result.stderr diff --git a/tests/commands/test_seeds.py b/tests/commands/test_seeds.py index f73ba0b..6f1e0c3 100644 --- a/tests/commands/test_seeds.py +++ b/tests/commands/test_seeds.py @@ -6,6 +6,7 @@ from datamasque.client.models.files import SeedFile from typer.testing import CliRunner +from datamasque_cli.errors import ExitCode from datamasque_cli.main import app MODULE = "datamasque_cli.commands.seeds" @@ -19,7 +20,7 @@ def test_delete_seed_aborts_when_missing(mock_get_client: MagicMock, runner: Cli result = runner.invoke(app, ["seeds", "delete", "nope.csv", "--yes"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.NOT_FOUND client.delete_file_if_exists.assert_not_called() diff --git a/tests/commands/test_system.py b/tests/commands/test_system.py index a952cda..b82fb69 100644 --- a/tests/commands/test_system.py +++ b/tests/commands/test_system.py @@ -8,6 +8,7 @@ from datamasque.client.models.license import LicenseInfo, SwitchableLicenseMetadata from typer.testing import CliRunner +from datamasque_cli.errors import ExitCode from datamasque_cli.main import app 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 @@ -171,8 +172,10 @@ def test_admin_install_does_not_swallow_non_401_errors(mock_get_unauth: MagicMoc ], ) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.INVALID_INPUT assert "already complete" not in result.stderr + assert "400 Bad Request" in result.stderr + assert "Traceback" not in result.stderr @patch(f"{MODULE}.get_unauthenticated_client") diff --git a/tests/commands/test_users.py b/tests/commands/test_users.py index f843bd7..51442e9 100644 --- a/tests/commands/test_users.py +++ b/tests/commands/test_users.py @@ -5,6 +5,7 @@ from typer.testing import CliRunner +from datamasque_cli.errors import ExitCode from datamasque_cli.main import app MODULE = "datamasque_cli.commands.users" @@ -30,5 +31,5 @@ def test_delete_user_aborts_when_missing(mock_get_client: MagicMock, runner: Cli result = runner.invoke(app, ["users", "delete", "nope", "--yes"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.NOT_FOUND client.delete_user_by_username_if_exists.assert_not_called() diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 024f1bd..6344b93 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -13,6 +13,8 @@ from datamasque_cli.main import app _TERMINAL_STATUSES = frozenset({"finished", "finished_with_warnings", "failed", "cancelled"}) +_DISCOVERY_RULESET_PREFIX = "$auto_" +_SCHEMA_DISCOVERY_RULESET = "$auto_schema_discovery" @pytest.fixture(scope="session", autouse=True) @@ -165,3 +167,162 @@ def db_yaml(tmp_path: Path) -> Path: " value: redacted@example.com\n" ) return path + + +DISCOVERY_TEST_NAMESPACE = "dm_int_ns" + + +def create_discovery_config(runner: CliRunner, name: str, config_type: str, yaml_file: Path) -> None: + """Create a discovery config, and fail the test when the CLI rejects it.""" + result = runner.invoke( + app, + ["discover", "configs", "create", "--name", name, "--type", config_type, "-f", str(yaml_file)], + ) + assert result.exit_code == 0, f"could not create {config_type} config '{name}': {result.stdout}{result.stderr}" + + +def create_discovery_config_library(runner: CliRunner, name: str, yaml_file: Path, namespace: str = "") -> None: + """Create a discovery config library, and fail the test when the CLI rejects it.""" + result = runner.invoke( + app, + ["discover", "libraries", "create", "--name", name, "--namespace", namespace, "-f", str(yaml_file)], + ) + label = f"{namespace}/{name}" if namespace else name + assert result.exit_code == 0, f"could not create library '{label}': {result.stdout}{result.stderr}" + + +@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): + args = ["discover", "libraries", "delete", name, "--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 invalid_ruleset_yaml(tmp_path: Path) -> Path: + """YAML the ruleset parser rejects.""" + path = tmp_path / "invalid_ruleset.yaml" + path.write_text("version: '1.0'\ntasks:\n - type: not_a_real_task\n") + return path + + +@pytest.fixture() +def ruleset_library_name(runner: CliRunner) -> Iterator[str]: + name = f"dm_int_{uuid.uuid4().hex[:8]}" + yield name + runner.invoke(app, ["libraries", "delete", name, "--yes", "--force"]) + + +@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 finished_non_schema_run(runner: CliRunner) -> int: + """Id of a finished run that schema discovery never ran on.""" + result = runner.invoke(app, ["run", "list", "--limit", "100", "--json"]) + if result.exit_code != 0: + pytest.skip("Could not list runs") + runs = json.loads(result.stdout) + scanned = {r["source"] for r in runs if str(r["ruleset"]).startswith(_SCHEMA_DISCOVERY_RULESET)} + finished = [ + r + for r in runs + if r["status"] in _TERMINAL_STATUSES + and r["ruleset"] + and not str(r["ruleset"]).startswith(_DISCOVERY_RULESET_PREFIX) + and r["source"] not in scanned + ] + if not finished: + pytest.skip("No finished masking run on a connection without schema discovery") + return int(finished[0]["id"]) + + +@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) + + +@pytest.fixture() +def file_source_connection(runner: CliRunner) -> str: + """Name of a file-type source connection.""" + override = os.environ.get("DM_TEST_FILE_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 file source") + match = _pick_by_role(json.loads(result.stdout), {"source", "source+destination"}, ("MountedShare", "Azure", "S3")) + if not match: + pytest.skip("No file-type source connection on this instance; set DM_TEST_FILE_CONN to override") + return match diff --git a/tests/integration/test_connections.py b/tests/integration/test_connections.py index 4423f6e..b536c6c 100644 --- a/tests/integration/test_connections.py +++ b/tests/integration/test_connections.py @@ -5,6 +5,7 @@ import pytest from typer.testing import CliRunner +from datamasque_cli.errors import ExitCode from datamasque_cli.main import app pytestmark = pytest.mark.integration @@ -13,7 +14,7 @@ def test_connection_test_aborts_on_missing_connection(runner: CliRunner) -> None: result = runner.invoke(app, ["connections", "test", "dm_int_never_exists_xyz"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.NOT_FOUND assert "not found" in result.stderr.lower() @@ -47,5 +48,5 @@ def test_connection_update_aborts_with_no_fields(runner: CliRunner, connection_n result = runner.invoke(app, ["connections", "update", connection_name]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.INVALID_INPUT assert "at least one field" in result.stderr.lower() diff --git a/tests/integration/test_delete_safety.py b/tests/integration/test_delete_safety.py index 4330a78..8f4b306 100644 --- a/tests/integration/test_delete_safety.py +++ b/tests/integration/test_delete_safety.py @@ -5,6 +5,7 @@ import pytest from typer.testing import CliRunner +from datamasque_cli.errors import ExitCode from datamasque_cli.main import app pytestmark = pytest.mark.integration @@ -16,5 +17,5 @@ def test_delete_nonexistent_aborts_not_found(runner: CliRunner, resource: str) - result = runner.invoke(app, [resource, "delete", missing, "--yes"]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.NOT_FOUND assert "not found" in result.stderr.lower() diff --git a/tests/integration/test_discovery.py b/tests/integration/test_discovery.py new file mode 100644 index 0000000..628bdf6 --- /dev/null +++ b/tests/integration/test_discovery.py @@ -0,0 +1,112 @@ +from __future__ import annotations + +import re +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +from datamasque_cli.errors import ExitCode +from datamasque_cli.main import app +from tests.integration.conftest import create_discovery_config + +pytestmark = pytest.mark.integration + + +def test_schema_config_type_mismatch_aborts( + runner: CliRunner, + any_connection: str, + discovery_config_name: str, + file_discovery_config: Path, +) -> None: + create_discovery_config(runner, discovery_config_name, "file", file_discovery_config) + + result = runner.invoke(app, ["discover", "schema", any_connection, "--config", discovery_config_name]) + assert result.exit_code == ExitCode.INVALID_INPUT + 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: + create_discovery_config(runner, discovery_config_name, "database", db_discovery_config) + + result = runner.invoke(app, ["discover", "file", any_connection, "--config", discovery_config_name]) + 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 == ExitCode.NOT_FOUND + + +def test_file_run_from_config_and_snapshot( + runner: CliRunner, + file_source_connection: str, + discovery_config_name: str, + file_discovery_config: Path, + tmp_path: Path, +) -> None: + create_discovery_config(runner, discovery_config_name, "file", file_discovery_config) + + start = runner.invoke(app, ["discover", "file", file_source_connection, "--config", discovery_config_name]) + if start.exit_code != 0: + pytest.skip(f"Could not start file discovery on '{file_source_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() + + +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_discovery_config(runner, discovery_config_name, "database", db_discovery_config) + + 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() + + +def test_schema_results_on_a_non_schema_run_reports_the_server_reason( + runner: CliRunner, finished_non_schema_run: int +) -> None: + result = runner.invoke(app, ["discover", "schema-results", str(finished_non_schema_run)]) + + assert result.exit_code == ExitCode.INVALID_INPUT + stderr = " ".join(result.stderr.split()) + assert f"No schema discovery results available for run {finished_non_schema_run}:" in stderr + assert "dm run status" not in stderr + + +def test_schema_results_on_an_unknown_run_is_not_found(runner: CliRunner) -> None: + result = runner.invoke(app, ["discover", "schema-results", "999999999"]) + + assert result.exit_code == ExitCode.NOT_FOUND + assert "Run 999999999 not found." in " ".join(result.stderr.split()) diff --git a/tests/integration/test_discovery_configs.py b/tests/integration/test_discovery_configs.py new file mode 100644 index 0000000..eccb101 --- /dev/null +++ b/tests/integration/test_discovery_configs.py @@ -0,0 +1,255 @@ +from __future__ import annotations + +import json +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +from datamasque_cli.errors import ExitCode +from datamasque_cli.main import app +from tests.integration.conftest import ( + DISCOVERY_TEST_NAMESPACE, + create_discovery_config, + create_discovery_config_library, +) + +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_discovery_config(runner, discovery_config_name, "database", db_discovery_config) + + 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 == ExitCode.NOT_FOUND + + +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 == ExitCode.INVALID_INPUT + 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: + create_discovery_config(runner, discovery_config_name, "database", db_discovery_config) + create_discovery_config(runner, discovery_config_name, "file", file_discovery_config) + + 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: + create_discovery_config(runner, discovery_config_name, "database", db_discovery_config) + create_discovery_config(runner, discovery_config_name, "file", file_discovery_config) + + result = runner.invoke( + app, ["discover", "configs", "create", "--name", discovery_config_name, "-f", str(db_discovery_config)] + ) + + assert result.exit_code == ExitCode.AMBIGUOUS + 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 == ExitCode.NOT_FOUND + + +# --- discovery config libraries ---------------------------------------------- + + +def test_library_create_get_delete_lifecycle( + runner: CliRunner, + discovery_library_name: str, + discovery_library_yaml: Path, +) -> None: + create_discovery_config_library(runner, discovery_library_name, discovery_library_yaml) + + 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 == ExitCode.NOT_FOUND + + +def test_library_namespace_is_isolated( + runner: CliRunner, + discovery_library_name: str, + discovery_library_yaml: Path, +) -> None: + create_discovery_config_library( + runner, discovery_library_name, discovery_library_yaml, namespace=DISCOVERY_TEST_NAMESPACE + ) + + 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 == 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)]) + assert result.exit_code == ExitCode.INVALID_INPUT + + +def test_library_delete_refuses_while_imported_then_force_succeeds( + runner: CliRunner, + discovery_library_name: str, + discovery_config_name: str, + tmp_path: Path, +) -> None: + library = tmp_path / "importable.yaml" + library.write_text( + "labels:\n" + " - name: dm_int_label\n" + " description: Integration test label\n" + " categories: [PII]\n" + "metadata_rules:\n" + " - name: DM Int Rule\n" + " label: dm_int_label\n" + " column:\n" + " type: regex\n" + " pattern: dm_int_column\n" + ) + create_discovery_config_library(runner, discovery_library_name, library, namespace=DISCOVERY_TEST_NAMESPACE) + + ref = f"{DISCOVERY_TEST_NAMESPACE}/{discovery_library_name}" + config = tmp_path / "importing.yaml" + config.write_text( + f"imports:\n" + f"- {ref}\n" + f"labels:\n" + f' $ref: "{ref}#labels"\n' + f"metadata_rules:\n" + f' $ref: "{ref}#metadata_rules"\n' + f"idd_rules: []\n" + f"files:\n" + f" include:\n" + f' - "*.csv"\n' + ) + create_discovery_config(runner, discovery_config_name, "file", config) + + delete = [ + "discover", + "libraries", + "delete", + discovery_library_name, + "--namespace", + DISCOVERY_TEST_NAMESPACE, + "--yes", + ] + + conflict = runner.invoke(app, delete) + assert conflict.exit_code == ExitCode.CONFLICT + assert "--force" in " ".join(conflict.stderr.split()) + + forced = runner.invoke(app, [*delete, "--force"]) + assert forced.exit_code == ExitCode.OK + + status = runner.invoke(app, ["discover", "configs", "status", discovery_config_name, "--type", "file", "--json"]) + assert json.loads(status.stdout)["status"] == "invalid" + + +# --- status ------------------------------------------------------------------ + + +@pytest.mark.parametrize( + ("is_valid_yaml", "expected_status"), + [ + (True, "valid"), + (False, "invalid"), + ], + ids=["valid", "invalid"], +) +def test_config_status_reports_stored_validation_state( + runner: CliRunner, + discovery_config_name: str, + db_discovery_config: Path, + invalid_discovery_yaml: Path, + is_valid_yaml: bool, + expected_status: str, +) -> None: + source = db_discovery_config if is_valid_yaml else invalid_discovery_yaml + create_discovery_config(runner, discovery_config_name, "database", source) + + result = runner.invoke(app, ["discover", "configs", "status", discovery_config_name, "--json"]) + + assert result.exit_code == ExitCode.OK + body = json.loads(result.stdout) + assert body["status"] == expected_status + if not is_valid_yaml: + assert body["validation_error"] + + +@pytest.mark.parametrize( + ("is_valid_yaml", "expected_status"), + [ + (True, "valid"), + (False, "invalid"), + ], + ids=["valid", "invalid"], +) +def test_library_status( + runner: CliRunner, + discovery_library_name: str, + discovery_library_yaml: Path, + invalid_discovery_yaml: Path, + is_valid_yaml: bool, + expected_status: str, +) -> None: + source = discovery_library_yaml if is_valid_yaml else invalid_discovery_yaml + create_discovery_config_library(runner, discovery_library_name, source) + + result = runner.invoke(app, ["discover", "libraries", "status", discovery_library_name, "--json"]) + + assert result.exit_code == ExitCode.OK + body = json.loads(result.stdout) + assert body["status"] == expected_status + if not is_valid_yaml: + assert body["validation_error"] diff --git a/tests/integration/test_rulesets.py b/tests/integration/test_rulesets.py index 7d94933..7015eb6 100644 --- a/tests/integration/test_rulesets.py +++ b/tests/integration/test_rulesets.py @@ -1,10 +1,12 @@ from __future__ import annotations +import json from pathlib import Path import pytest from typer.testing import CliRunner +from datamasque_cli.errors import ExitCode from datamasque_cli.main import app pytestmark = pytest.mark.integration @@ -41,7 +43,7 @@ def test_create_without_type_aborts_when_name_is_ambiguous( result = runner.invoke(app, ["rulesets", "create", "--name", ruleset_name, "--file", str(file_yaml)]) - assert result.exit_code != 0 + assert result.exit_code == ExitCode.AMBIGUOUS assert "Multiple rulesets" in result.stderr @@ -79,6 +81,58 @@ def test_delete_with_type_leaves_other_namespace_intact( file_gone = runner.invoke(app, ["rulesets", "get", ruleset_name, "--type", "file", "--yaml"]) db_still = runner.invoke(app, ["rulesets", "get", ruleset_name, "--type", "database", "--yaml"]) - assert file_gone.exit_code != 0 + assert file_gone.exit_code == ExitCode.NOT_FOUND assert db_still.exit_code == 0 assert "mask_table" in db_still.stdout + + +# --- status ------------------------------------------------------------------ + + +@pytest.mark.parametrize( + ("is_valid_yaml", "expected_status"), + [ + (True, "valid"), + (False, "invalid"), + ], + ids=["valid", "invalid"], +) +def test_ruleset_status( + runner: CliRunner, + ruleset_name: str, + db_yaml: Path, + invalid_ruleset_yaml: Path, + is_valid_yaml: bool, + expected_status: str, +) -> None: + source = db_yaml if is_valid_yaml else invalid_ruleset_yaml + create = runner.invoke( + app, ["rulesets", "create", "--name", ruleset_name, "--file", str(source), "--type", "database"] + ) + assert create.exit_code == 0, create.stdout + + result = runner.invoke(app, ["rulesets", "status", ruleset_name, "--type", "database", "--json"]) + + assert result.exit_code == ExitCode.OK + body = json.loads(result.stdout) + assert body["status"] == expected_status + if not is_valid_yaml: + assert body["errors"] + + +def test_ruleset_library_status_reports_invalid( + runner: CliRunner, + ruleset_library_name: str, + invalid_ruleset_yaml: Path, +) -> None: + create = runner.invoke( + app, ["libraries", "create", "--name", ruleset_library_name, "--file", str(invalid_ruleset_yaml)] + ) + assert create.exit_code == 0, create.stdout + + result = runner.invoke(app, ["libraries", "status", ruleset_library_name, "--json"]) + + assert result.exit_code == ExitCode.OK + body = json.loads(result.stdout) + assert body["status"] == "invalid" + assert body["errors"] diff --git a/tests/integration/test_runs.py b/tests/integration/test_runs.py index 0d8ce9a..b922e04 100644 --- a/tests/integration/test_runs.py +++ b/tests/integration/test_runs.py @@ -6,6 +6,7 @@ import pytest from typer.testing import CliRunner +from datamasque_cli.errors import ExitCode from datamasque_cli.main import app from tests.integration.conftest import wait_for_run @@ -112,3 +113,32 @@ def test_run_start_passes_options_end_to_end( assert result.exit_code == 0, f"run start --options failed: {result.stdout}" run_id = int(json.loads(result.stdout)["id"]) assert wait_for_run(runner, run_id) in {"finished", "finished_with_warnings"} + + +def test_run_start_with_invalid_ruleset_reports_the_server_reason( + runner: CliRunner, + ruleset_name: str, + invalid_ruleset_yaml: Path, + file_connection_pair: tuple[str, str], +) -> None: + source, destination = file_connection_pair + create = runner.invoke( + app, ["rulesets", "create", "--name", ruleset_name, "--file", str(invalid_ruleset_yaml), "--type", "file"] + ) + assert create.exit_code == 0, create.stdout + + result = runner.invoke(app, ["run", "start", "-c", source, "-r", ruleset_name, "-d", destination, "--background"]) + + assert result.exit_code == ExitCode.INVALID_INPUT + stderr = " ".join(result.stderr.split()) + assert "ruleset: Cannot start run" in stderr + assert "Traceback" not in result.stderr + + +@pytest.mark.parametrize("command", ["status", "logs", "cancel", "retry"]) +def test_unknown_run_is_not_found(runner: CliRunner, command: str) -> None: + unknown_run_id = 999999999 + result = runner.invoke(app, ["run", command, str(unknown_run_id)]) + + assert result.exit_code == ExitCode.NOT_FOUND + assert f"Run {unknown_run_id} not found." in " ".join(result.stderr.split()) diff --git a/tests/test_main.py b/tests/test_main.py new file mode 100644 index 0000000..e96de3b --- /dev/null +++ b/tests/test_main.py @@ -0,0 +1,117 @@ +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +import pytest +from datamasque.client.exceptions import ( + DataMasqueApiError, + DataMasqueNotReadyError, + DataMasqueTransportError, +) + +from datamasque_cli.errors import ExitCode +from datamasque_cli.main import main + +MODULE = "datamasque_cli.main" + + +@patch(f"{MODULE}.app") +def test_unhandled_api_error_aborts_cleanly(mock_app: MagicMock, capsys: pytest.CaptureFixture[str]) -> None: + response = MagicMock(status_code=500) + response.json.return_value = {"detail": "Report storage is unavailable."} + mock_app.side_effect = DataMasqueApiError("boom", response=response) + + with pytest.raises(SystemExit) as exc_info: + main() + + assert exc_info.value.code == ExitCode.ERROR + assert "Report storage is unavailable." in " ".join(capsys.readouterr().err.split()) + + +@patch(f"{MODULE}.app") +def test_unhandled_api_conflict_keeps_its_code(mock_app: MagicMock) -> None: + response = MagicMock(status_code=409) + response.json.return_value = {"detail": "Already running."} + mock_app.side_effect = DataMasqueApiError("boom", response=response) + + with pytest.raises(SystemExit) as exc_info: + main() + + assert exc_info.value.code == ExitCode.CONFLICT + + +@pytest.mark.parametrize( + ("status", "expected_exit"), + [ + (401, ExitCode.AUTH_FAILED), + (403, ExitCode.FORBIDDEN), + (404, ExitCode.NOT_FOUND), + ], +) +@patch(f"{MODULE}.app") +def test_unhandled_api_error_maps_status_to_its_code(mock_app: MagicMock, status: int, expected_exit: ExitCode) -> None: + response = MagicMock(status_code=status) + response.json.return_value = {"detail": "The requested run was not found."} + mock_app.side_effect = DataMasqueApiError("boom", response=response) + + with pytest.raises(SystemExit) as exc_info: + main() + + assert exc_info.value.code == expected_exit + + +@patch(f"{MODULE}.app") +def test_auth_failure_hints_at_login(mock_app: MagicMock, capsys: pytest.CaptureFixture[str]) -> None: + response = MagicMock(status_code=401) + response.json.return_value = {"detail": "Token is invalid or expired."} + mock_app.side_effect = DataMasqueApiError("boom", response=response) + + with pytest.raises(SystemExit): + main() + + assert "dm auth login" in " ".join(capsys.readouterr().err.split()) + + +@patch(f"{MODULE}.app") +def test_transport_error_aborts_with_transport_code(mock_app: MagicMock) -> None: + mock_app.side_effect = DataMasqueTransportError("Connection reset by peer") + + with pytest.raises(SystemExit) as exc_info: + main() + + assert exc_info.value.code == ExitCode.TRANSPORT_ERROR + + +@patch(f"{MODULE}.app") +def test_other_datamasque_errors_abort_as_unclassified(mock_app: MagicMock) -> None: + """The base-class clause catches the rest of the exception family.""" + mock_app.side_effect = DataMasqueNotReadyError("Server is starting up") + + with pytest.raises(SystemExit) as exc_info: + main() + + assert exc_info.value.code == ExitCode.ERROR + + +@patch(f"{MODULE}.app") +def test_exit_status_passes_through(mock_app: MagicMock) -> None: + """Typer signals success, and `abort()` signals failure, by raising `SystemExit`. + + Both must survive the wrapper, which they do because `SystemExit` inherits + from `BaseException` rather than `Exception`. + """ + mock_app.side_effect = SystemExit(ExitCode.NOT_FOUND) + + with pytest.raises(SystemExit) as exc_info: + main() + + assert exc_info.value.code == ExitCode.NOT_FOUND + + +@patch(f"{MODULE}.app") +def test_unrelated_exceptions_are_not_swallowed(mock_app: MagicMock) -> None: + """Only the DataMasque family is translated; a bug in our own code still raises.""" + mock_app.side_effect = ValueError("a genuine bug") + + with pytest.raises(ValueError, match="a genuine bug"): + main() diff --git a/tests/test_output.py b/tests/test_output.py index 961eb97..d722a87 100644 --- a/tests/test_output.py +++ b/tests/test_output.py @@ -1,13 +1,28 @@ from __future__ import annotations import json +from pathlib import Path +from unittest.mock import MagicMock import pytest +from datamasque.client.exceptions import DataMasqueApiError +from datamasque.client.models.ruleset import RulesetId -from datamasque_cli.output import ( - EXIT_CODES, +from datamasque_cli.errors import ( + EXIT_CODE_BY_ERROR, ErrorCode, + ExitCode, abort, + extract_server_error_reason, + require_id_or_abort, +) +from datamasque_cli.fileio import ( + read_json_object_or_abort, + read_text_or_abort, + write_bytes_or_abort, + write_text_or_abort, +) +from datamasque_cli.output import ( is_agent_context, print_json, print_success, @@ -176,6 +191,8 @@ def test_abort_human_mode_prints_red_error(monkeypatch: pytest.MonkeyPatch, caps (ErrorCode.AUTH_FAILED, 7), (ErrorCode.CONFLICT, 8), (ErrorCode.TRANSPORT_ERROR, 9), + (ErrorCode.CANCELLED, 10), + (ErrorCode.FORBIDDEN, 11), ], ) def test_abort_maps_code_to_documented_exit_code(code: ErrorCode, expected_exit: int) -> None: @@ -186,8 +203,20 @@ 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_require_id_returns_the_id_when_present() -> None: + assert require_id_or_abort(RulesetId("rs-1"), "ruleset 'payroll'") == "rs-1" + + +def test_require_id_aborts_when_the_server_omitted_it(capsys: pytest.CaptureFixture[str]) -> None: + with pytest.raises(SystemExit) as exc_info: + require_id_or_abort(None, "ruleset 'payroll'") + + assert exc_info.value.code == ExitCode.ERROR + assert "ruleset 'payroll' without an id" in _unwrapped(capsys.readouterr().err) def test_print_success_suppressed_in_agent_mode( @@ -198,3 +227,164 @@ def test_print_success_suppressed_in_agent_mode( captured = capsys.readouterr() assert captured.err == "" assert captured.out == "" + + +# -- file helpers ---------------------------------------------------------- + + +def _unwrapped(text: str) -> str: + """Rejoin a message Rich broke across lines to fit the terminal.""" + return " ".join(text.split()) + + +def _without_whitespace(text: str) -> str: + """Drop every space, for paths Rich may have split mid-token.""" + return "".join(text.split()) + + +def test_read_text_returns_utf8_content(tmp_path: Path) -> None: + file = tmp_path / "rules.yaml" + file.write_text("name: café\n", encoding="utf-8") + + assert read_text_or_abort(file) == "name: café\n" + + +def test_read_text_rejects_other_encodings(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + file = tmp_path / "latin1.yaml" + file.write_bytes("name: café\n".encode("latin-1")) + + with pytest.raises(SystemExit) as exc_info: + read_text_or_abort(file) + + assert exc_info.value.code == ExitCode.INVALID_INPUT + assert "is not valid UTF-8" in _unwrapped(capsys.readouterr().err) + + +@pytest.mark.parametrize("content", ["", "\n", " \n\t\n"]) +def test_read_text_rejects_empty_content(tmp_path: Path, capsys: pytest.CaptureFixture[str], content: str) -> None: + file = tmp_path / "empty.yaml" + file.write_text(content, encoding="utf-8") + + with pytest.raises(SystemExit) as exc_info: + read_text_or_abort(file) + + assert exc_info.value.code == ExitCode.INVALID_INPUT + assert "is empty" in _unwrapped(capsys.readouterr().err) + + +def test_read_text_missing_file_is_not_found(tmp_path: Path) -> None: + with pytest.raises(SystemExit) as exc_info: + read_text_or_abort(tmp_path / "absent.yaml") + + assert exc_info.value.code == ExitCode.NOT_FOUND + + +def test_read_text_directory_is_invalid_input(tmp_path: Path) -> None: + with pytest.raises(SystemExit) as exc_info: + read_text_or_abort(tmp_path) + + assert exc_info.value.code == ExitCode.INVALID_INPUT + + +def test_read_errors_name_the_file(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + file = tmp_path / "absent.yaml" + + with pytest.raises(SystemExit): + read_text_or_abort(file) + + assert _without_whitespace(str(file)) in _without_whitespace(capsys.readouterr().err) + + +def test_read_json_object_parses_content(tmp_path: Path) -> None: + file = tmp_path / "request.json" + file.write_text('{"connection": "abc"}', encoding="utf-8") + + assert read_json_object_or_abort(file) == {"connection": "abc"} + + +def test_read_json_object_rejects_malformed_content(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + file = tmp_path / "broken.json" + file.write_text('{"connection": ', encoding="utf-8") + + with pytest.raises(SystemExit) as exc_info: + read_json_object_or_abort(file) + + assert exc_info.value.code == ExitCode.INVALID_INPUT + assert "is not valid JSON" in _unwrapped(capsys.readouterr().err) + + +@pytest.mark.parametrize("content", ['["a", "b"]', '"just a string"', "42", "null"]) +def test_read_json_object_rejects_non_objects(tmp_path: Path, capsys: pytest.CaptureFixture[str], content: str) -> None: + """Valid JSON that is not an object would crash the callers, which index into it.""" + file = tmp_path / "array.json" + file.write_text(content, encoding="utf-8") + + with pytest.raises(SystemExit) as exc_info: + read_json_object_or_abort(file) + + assert exc_info.value.code == ExitCode.INVALID_INPUT + assert "must contain a JSON object" in _unwrapped(capsys.readouterr().err) + + +def test_write_text_round_trips_utf8(tmp_path: Path) -> None: + file = tmp_path / "out.yaml" + + write_text_or_abort(file, "name: café\n") + + assert file.read_bytes() == "name: café\n".encode() + + +def test_write_text_to_missing_directory_aborts(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + target = tmp_path / "no_such_dir" / "out.yaml" + + with pytest.raises(SystemExit) as exc_info: + write_text_or_abort(target, "content") + + assert exc_info.value.code == ExitCode.INVALID_INPUT + assert _without_whitespace(str(target)) in _without_whitespace(capsys.readouterr().err) + + +def test_write_bytes_to_a_directory_aborts(tmp_path: Path) -> None: + with pytest.raises(SystemExit) as exc_info: + write_bytes_or_abort(tmp_path, b"PK\x03\x04") + + assert exc_info.value.code == ExitCode.INVALID_INPUT + + +# -- server error reasons -------------------------------------------------- + + +def _api_error(body: object) -> DataMasqueApiError: + response = MagicMock(status_code=400) + response.json.return_value = body + return DataMasqueApiError("boom", response=response) + + +@pytest.mark.parametrize( + ("body", "expected"), + [ + ({"detail": "The requested run was not found."}, "The requested run was not found."), + ({"detail": [{"loc": ["body", "name"], "msg": "field required"}]}, "name: field required"), + ({"error": "boom"}, "boom"), + ({"ruleset": ['Ruleset "x" is invalid.']}, 'ruleset: Ruleset "x" is invalid.'), + ({"non_field_errors": ["Exactly one option must be provided."]}, "Exactly one option must be provided."), + ({"host": ["required"], "port": ["not an int"]}, "host: required; port: not an int"), + ], + ids=["detail", "detail_list", "error", "field", "non_field", "multiple_fields"], +) +def test_server_error_reason_reads_every_body_shape(body: object, expected: str) -> None: + assert extract_server_error_reason(_api_error(body)) == expected + + +@pytest.mark.parametrize( + "body", [{}, {"id": 5, "name": "x"}, [], "plain text"], ids=["empty", "no_errors", "list", "str"] +) +def test_server_error_reason_is_none_without_one(body: object) -> None: + assert extract_server_error_reason(_api_error(body)) is None + + +def test_server_error_reason_is_none_when_body_is_not_json() -> None: + response = MagicMock(status_code=404) + response.json.side_effect = ValueError("no json") + + assert extract_server_error_reason(DataMasqueApiError("404", response=response)) is None diff --git a/uv.lock b/uv.lock index 13b056f..c254fc4 100644 --- a/uv.lock +++ b/uv.lock @@ -141,10 +141,11 @@ wheels = [ [[package]] name = "datamasque-cli" -version = "1.4.0" +version = "1.5.0" source = { editable = "." } dependencies = [ { name = "datamasque-python" }, + { name = "pydantic" }, { name = "tomli-w" }, { name = "typer" }, ] @@ -159,7 +160,8 @@ dev = [ [package.metadata] requires-dist = [ - { name = "datamasque-python", specifier = ">=1.0.0,<2" }, + { name = "datamasque-python", specifier = ">=1.2.2,<2" }, + { name = "pydantic", specifier = ">=2.5,<3" }, { name = "tomli-w", specifier = ">=1.0.0" }, { name = "typer", specifier = ">=0.15.0" }, ] @@ -174,15 +176,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]]