From 932770ccec2e354d699915453af1d17d851c8d2f Mon Sep 17 00:00:00 2001 From: Dylan Gormley Date: Tue, 25 Aug 2026 12:17:28 -0500 Subject: [PATCH] feat: add readiness checks --- dtcli/cli.py | 3 +- dtcli/doctor.py | 181 +++++++++++++++++++++++++++++++++++++++++++ tests/test_doctor.py | 154 ++++++++++++++++++++++++++++++++++++ 3 files changed, 337 insertions(+), 1 deletion(-) create mode 100644 dtcli/doctor.py create mode 100644 tests/test_doctor.py diff --git a/dtcli/cli.py b/dtcli/cli.py index 9aaea1c..cab4f61 100644 --- a/dtcli/cli.py +++ b/dtcli/cli.py @@ -6,7 +6,7 @@ from click_aliasing import ClickAliasedGroup from rich import console, pretty -from dtcli import clear, config, ls, ps, pull, scout, unregistered +from dtcli import clear, config, doctor, ls, ps, pull, scout, unregistered from dtcli.utilities import utilities pretty.install() @@ -42,6 +42,7 @@ def version(): cli.add_command(clear.clear) cli.add_command(config.config) +cli.add_command(doctor.doctor) cli.add_command(ls.list, aliases=["ls"]) cli.add_command(ps.ps) cli.add_command(pull.pull) diff --git a/dtcli/doctor.py b/dtcli/doctor.py new file mode 100644 index 0000000..f9a4e12 --- /dev/null +++ b/dtcli/doctor.py @@ -0,0 +1,181 @@ +"""Datatrail readiness checks.""" + +import json +from datetime import datetime, timezone +from pathlib import Path +from typing import Any, Dict, Optional, Tuple +from urllib.parse import urlparse + +import click +import requests +import yaml +from OpenSSL import crypto # type: ignore + +from dtcli.config import CONFIG + +REQUEST_TIMEOUT = 10 +SERVICE_URLS = { + "minoc": "https://ws-uv.canfar.net/minoc/capabilities", + "luskan": "https://ws-uv.canfar.net/luskan/capabilities", +} + + +def _result(ok: bool, message: str) -> Dict[str, Any]: + """Create one check result.""" + return {"ok": ok, "message": message} + + +def _load_config() -> Optional[Dict[str, Any]]: + """Load the configuration without printing its contents.""" + try: + with open(CONFIG) as stream: + config = yaml.safe_load(stream) + except (OSError, UnicodeError, yaml.YAMLError): + return None + return config if isinstance(config, dict) else None + + +def _check_config() -> Tuple[Dict[str, Any], Optional[Dict[str, Any]]]: + """Load and validate the configuration.""" + config = _load_config() + if config is None: + return _result(False, "Configuration could not be loaded."), None + + server = config.get("server") + certificate = config.get("vospace_certfile") + site = config.get("site") + root_mounts = config.get("root_mounts") + parsed = urlparse(server) if isinstance(server, str) else None + valid_server = bool(parsed and parsed.scheme in ("http", "https") and parsed.netloc) + valid_mount = ( + isinstance(site, str) + and isinstance(root_mounts, dict) + and isinstance(root_mounts.get(site), str) + ) + if not valid_server or not isinstance(certificate, str) or not valid_mount: + return _result(False, "Configuration is missing required values."), None + return _result(True, "Configuration is ready."), config + + +def _check_server(server: str) -> Dict[str, Any]: + """Check the central server and response shape.""" + try: + response = requests.get( + server.rstrip("/") + "/query/dataset/scopes", + timeout=REQUEST_TIMEOUT, + ) + except requests.RequestException: + return _result(False, "Datatrail server request failed.") + if not 200 <= response.status_code < 300: + return _result(False, f"Datatrail server returned HTTP {response.status_code}.") + try: + scopes = response.json() + except (requests.JSONDecodeError, ValueError): + return _result(False, "Datatrail server returned invalid JSON.") + if not isinstance(scopes, list) or not all( + isinstance(scope, str) for scope in scopes + ): + return _result(False, "Datatrail server returned an invalid scope list.") + return _result(True, "Datatrail server is ready.") + + +def _certificate_time(value: Optional[bytes]) -> Optional[datetime]: + """Parse an X509 certificate timestamp.""" + if value is None: + return None + try: + return datetime.strptime(value.decode("ascii"), "%Y%m%d%H%M%SZ").replace( + tzinfo=timezone.utc + ) + except (UnicodeDecodeError, ValueError): + return None + + +def _check_certificate(certfile: str) -> Dict[str, Any]: + """Check that the configured certificate is current.""" + try: + pem = Path(certfile).read_bytes() + except OSError: + return _result(False, "CANFAR certificate could not be read.") + try: + certificate = crypto.load_certificate(crypto.FILETYPE_PEM, pem) + except crypto.Error: + return _result(False, "CANFAR certificate is not valid PEM.") + + not_before = _certificate_time(certificate.get_notBefore()) + not_after = _certificate_time(certificate.get_notAfter()) + now = datetime.now(timezone.utc) + if not_before is None or not_after is None: + return _result(False, "CANFAR certificate dates are invalid.") + if now < not_before: + return _result(False, "CANFAR certificate is not valid yet.") + if now >= not_after: + return _result(False, "CANFAR certificate is expired.") + return _result(True, "CANFAR certificate is valid.") + + +def _check_service(name: str, url: str, certfile: str) -> Dict[str, Any]: + """Check one authenticated CANFAR service.""" + try: + response = requests.get( + url, + cert=certfile, + allow_redirects=True, + timeout=REQUEST_TIMEOUT, + ) + except requests.RequestException: + return _result(False, f"{name} request failed.") + if not 200 <= response.status_code < 300: + return _result(False, f"{name} returned HTTP {response.status_code}.") + if not isinstance(response.headers.get("x-vo-authenticated"), str): + return _result(False, f"{name} did not authenticate the certificate.") + return _result(True, f"{name} is ready.") + + +def run_checks() -> Dict[str, Any]: + """Run all readiness checks.""" + config_check, config = _check_config() + checks = {"config": config_check} + if config is None: + message = "Not checked because configuration failed." + checks.update( + { + "server": _result(False, message), + "certificate": _result(False, message), + "minoc": _result(False, message), + "luskan": _result(False, message), + } + ) + return {"ok": False, "checks": checks} + + checks["server"] = _check_server(config["server"]) + checks["certificate"] = _check_certificate(config["vospace_certfile"]) + if checks["certificate"]["ok"]: + for name, url in SERVICE_URLS.items(): + checks[name] = _check_service(name, url, config["vospace_certfile"]) + else: + message = "Not checked because the certificate failed." + checks["minoc"] = _result(False, message) + checks["luskan"] = _result(False, message) + return {"ok": all(check["ok"] for check in checks.values()), "checks": checks} + + +def _show_report(report: Dict[str, Any]) -> None: + """Print readiness results.""" + for name, check in report["checks"].items(): + status = "OK" if check["ok"] else "FAILED" + click.echo(f"{name}: {status} - {check['message']}") + + +@click.command(name="doctor", help="Check Datatrail readiness.") +@click.option("--json", "output_json", is_flag=True, help="Output as JSON.") +@click.pass_context +def doctor(ctx: click.Context, output_json: bool) -> None: + """Check configuration and service readiness.""" + report = run_checks() + if output_json: + click.echo(json.dumps(report, indent=2)) + else: + _show_report(report) + if not report["ok"]: + ctx.exit(1) diff --git a/tests/test_doctor.py b/tests/test_doctor.py new file mode 100644 index 0000000..a8c9977 --- /dev/null +++ b/tests/test_doctor.py @@ -0,0 +1,154 @@ +"""Tests for readiness checks.""" + +import json +from datetime import datetime, timedelta, timezone +from pathlib import Path + +import requests +from click.testing import CliRunner + +from dtcli import doctor +from dtcli.cli import cli + + +class FakeCertificate: + """Certificate with controlled validity dates.""" + + def __init__(self, not_before: datetime, not_after: datetime): + """Store certificate dates.""" + self.not_before = not_before + self.not_after = not_after + + def get_notBefore(self) -> bytes: + """Return the start date as an X509 timestamp.""" + return self.not_before.strftime("%Y%m%d%H%M%SZ").encode("ascii") + + def get_notAfter(self) -> bytes: + """Return the end date as an X509 timestamp.""" + return self.not_after.strftime("%Y%m%d%H%M%SZ").encode("ascii") + + +class FakeResponse: + """Small requests response substitute.""" + + def __init__(self, status_code=200, payload=None, headers=None): + """Store response fields.""" + self.status_code = status_code + self.payload = payload + self.headers = headers or {} + + def json(self): + """Return the configured JSON payload.""" + return self.payload + + +def _config(certfile: Path): + """Create a valid test configuration.""" + return { + "server": "https://example.invalid/datatrail", + "vospace_certfile": str(certfile), + "site": "local", + "root_mounts": {"local": "./"}, + } + + +def test_run_checks_ready(monkeypatch, tmp_path: Path) -> None: + """Report success when every dependency is ready.""" + certfile = tmp_path / "cert.pem" + certfile.write_text("certificate") + now = datetime.now(timezone.utc) + certificate = FakeCertificate(now - timedelta(days=1), now + timedelta(days=1)) + monkeypatch.setattr(doctor, "_load_config", lambda: _config(certfile)) + monkeypatch.setattr( + doctor.crypto, "load_certificate", lambda file_type, pem: certificate + ) + + def fake_get(url, **kwargs): + """Return valid server and service responses.""" + if url.endswith("/query/dataset/scopes"): + return FakeResponse(payload=["test.scope"]) + return FakeResponse(headers={"x-vo-authenticated": "user"}) + + monkeypatch.setattr(doctor.requests, "get", fake_get) + + report = doctor.run_checks() + + assert report["ok"] is True + assert list(report["checks"]) == [ + "config", + "server", + "certificate", + "minoc", + "luskan", + ] + assert all(check["ok"] for check in report["checks"].values()) + + +def test_certificate_expired(monkeypatch, tmp_path: Path) -> None: + """Reject an expired certificate without showing its contents.""" + certfile = tmp_path / "cert.pem" + certfile.write_text("private-value") + now = datetime.now(timezone.utc) + certificate = FakeCertificate(now - timedelta(days=2), now - timedelta(days=1)) + monkeypatch.setattr( + doctor.crypto, "load_certificate", lambda file_type, pem: certificate + ) + + result = doctor._check_certificate(str(certfile)) + + assert result == {"ok": False, "message": "CANFAR certificate is expired."} + assert "private-value" not in result["message"] + + +def test_server_requires_scope_list(monkeypatch) -> None: + """Reject an unexpected central server response.""" + monkeypatch.setattr( + doctor.requests, + "get", + lambda url, **kwargs: FakeResponse(payload={"scopes": ["test.scope"]}), + ) + + result = doctor._check_server("https://example.invalid/datatrail") + + assert result["ok"] is False + assert result["message"] == "Datatrail server returned an invalid scope list." + + +def test_service_requires_authentication_header(monkeypatch) -> None: + """Reject a service response without authenticated identity.""" + monkeypatch.setattr(doctor.requests, "get", lambda url, **kwargs: FakeResponse()) + + result = doctor._check_service("minoc", "https://example.invalid", "cert.pem") + + assert result["ok"] is False + assert result["message"] == "minoc did not authenticate the certificate." + + +def test_doctor_json_hides_request_details(monkeypatch, tmp_path: Path) -> None: + """Keep configured credentials and request errors out of JSON output.""" + certfile = tmp_path / "cert.pem" + certfile.write_text("certificate") + now = datetime.now(timezone.utc) + certificate = FakeCertificate(now - timedelta(days=1), now + timedelta(days=1)) + config = _config(certfile) + config["server"] = "https://user:secret@example.invalid/datatrail" + monkeypatch.setattr("dtcli.cli.check_version", lambda: None) + monkeypatch.setattr(doctor, "_load_config", lambda: config) + monkeypatch.setattr( + doctor.crypto, "load_certificate", lambda file_type, pem: certificate + ) + + def fail_request(url, **kwargs): + """Raise an error containing sensitive request details.""" + raise requests.ConnectionError(url) + + monkeypatch.setattr(doctor.requests, "get", fail_request) + + result = CliRunner().invoke(cli, ["doctor", "--json"]) + + assert result.exit_code == 1 + report = json.loads(result.output) + assert report["ok"] is False + assert report["checks"]["server"]["ok"] is False + assert "secret" not in result.output + assert "user:" not in result.output