-
Notifications
You must be signed in to change notification settings - Fork 2
feat(doctor): add readiness checks #182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
|
Comment on lines
+38
to
+57
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be broken up: the validation of the config file should exist in config.py. Moulding that into the shape that doctor needs can remain here. |
||
|
|
||
|
|
||
| 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, | ||
| ) | ||
|
Comment on lines
+63
to
+66
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a health check endpoint that might be better for this, it returns the following json: ❯ http get https://frb.chimenet.ca/datatrail/health/check
╭────────────────────────┬────────────────────────────────────────────╮
│ status │ ok │
│ │ ╭──────────┬─────────────────────────────╮ │
│ checks │ │ │ ╭──────────────────┬──────╮ │ │
│ │ │ database │ │ status │ ok │ │ │
│ │ │ │ │ response_time_ms │ 7.58 │ │ │
│ │ │ │ ╰──────────────────┴──────╯ │ │
│ │ │ │ ╭──────────────────┬──────╮ │ │
│ │ │ api │ │ status │ ok │ │ │
│ │ │ │ │ response_time_ms │ 8.25 │ │ │
│ │ │ │ ╰──────────────────┴──────╯ │ │
│ │ ╰──────────┴─────────────────────────────╯ │
│ timestamp │ 2026-08-31T19:57:21.140372+00:00 │
│ total_response_time_ms │ 8.25 │
╰────────────────────────┴────────────────────────────────────────────╯ |
||
| 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) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A function to read the config already exists in config.py, it is called procure. It returns the entire dictionary, or if a key is given, the value of that key only. The specific exception checks can be added there.