spec(service-credential-tester): add spec (21 reqs CT-101..121)
Per-type test routines for 7 remote types (qbittorrent, prometheus via Grafana gateway, alertmanager, jellyfin, authentik, ssh_tasks, nextcloud) + backups (no test). Endpoint POST /api/services/test, no persistence, friendly error translation (resolves qBit login #3 at UI layer). Frontend Test button + gate Create/Save on pass with Save-anyway override. Stale-proposal correction: jellyseerr is not in the registry (merged into jellyfin).
This commit is contained in:
@@ -0,0 +1,222 @@
|
||||
# SDD Spec: Service Credential Tester
|
||||
|
||||
**Change:** `service-credential-tester`
|
||||
**Phase:** spec
|
||||
**Date:** 2026-07-09
|
||||
|
||||
This spec defines the acceptance requirements for the change. Requirements are testable and derived from `proposal.md` §6 (success criteria) and §8 (resolved questions). Prefix `CT-` for credential-tester.
|
||||
|
||||
## Requirement categories
|
||||
|
||||
1. Endpoint shape, validation, dispatch
|
||||
2. Per-type test routines
|
||||
3. No-persistence + security
|
||||
4. Frontend test UI
|
||||
5. Tests + gate greenness
|
||||
|
||||
---
|
||||
|
||||
## 1. Endpoint shape, validation, dispatch
|
||||
|
||||
### Requirement: CT-101 — Test endpoint accepts unsaved service input and returns a structured result
|
||||
|
||||
The backend MUST expose `POST /api/services/test` accepting a `ServiceInstanceInput` body (`service_type`, `name`, `config`, `secrets`, `enabled`) — NOT a service id, because the tester operates on unsaved form values. On success the endpoint MUST return HTTP 200 with a body of shape `{ "ok": bool, "detail": str, "evidence": str | null }`. The endpoint MUST be auth-gated identically to every other `/api/services/*` endpoint (JWT/API-key).
|
||||
|
||||
#### Scenario: successful connection returns ok with evidence
|
||||
|
||||
- GIVEN a valid `ServiceInstanceInput` for a reachable service
|
||||
- WHEN `POST /api/services/test` is called with that body
|
||||
- THEN the response is HTTP 200 with `{ "ok": true, "detail": <human summary>, "evidence": <proof string> }`
|
||||
|
||||
#### Scenario: failed connection returns ok=false with actionable detail
|
||||
|
||||
- GIVEN a valid input whose target is unreachable or rejects auth
|
||||
- WHEN the endpoint is called
|
||||
- THEN the response is HTTP 200 with `{ "ok": false, "detail": <specific failure message>, "evidence": null }`
|
||||
|
||||
### Requirement: CT-102 — Validation runs before the test and rejects malformed config with 422
|
||||
|
||||
The endpoint MUST reuse the existing `_validate_input` helper (from `routers/services.py`) to validate `service_type`, `config` (via the definition's `config_model`), and secret-key names BEFORE any network probe. Malformed input MUST raise HTTP 422 with the same detail format as `POST /api/services/instances`, so the existing validation-error surfacing (the `#1` fix in `ServicesPage.tsx`) covers the test endpoint too.
|
||||
|
||||
#### Scenario: schema-less URL rejected before any network call
|
||||
|
||||
- GIVEN a `qbittorrent` input whose `base_url` lacks the `http://` schema
|
||||
- WHEN the endpoint is called
|
||||
- THEN the response is HTTP 422 with a detail naming the schema requirement
|
||||
- AND no network call is made to qBittorrent
|
||||
|
||||
#### Scenario: unknown service type rejected
|
||||
|
||||
- GIVEN an input with `service_type = "nonexistent"`
|
||||
- WHEN the endpoint is called
|
||||
- THEN the response is HTTP 422 with "Unknown service type"
|
||||
|
||||
### Requirement: CT-103 — Closed per-type dispatch mirrors SERVICE_DEFINITIONS
|
||||
|
||||
The endpoint MUST dispatch to a per-type test routine via the service definition (a new optional `test_callable` field on `ServiceDefinition`, defaulting to `None`). The dispatch is closed and compile-time — no runtime plugin loading, no arbitrary callable execution. A service type whose definition has `test_callable = None` (e.g. `backups`, which has no remote connection) MUST return `{ "ok": true, "detail": "No connection test for this service type", "evidence": null }` without any network call.
|
||||
|
||||
#### Scenario: backups type returns ok without a network probe
|
||||
|
||||
- GIVEN a `backups` service input
|
||||
- WHEN the endpoint is called
|
||||
- THEN the response is `{ "ok": true, "detail": "No connection test for this service type", "evidence": null }`
|
||||
- AND no network request is issued
|
||||
|
||||
#### Scenario: unknown widget kind is not involved
|
||||
|
||||
- GIVEN a `prometheus` input
|
||||
- WHEN the endpoint is called
|
||||
- THEN the dispatch keys off `service_type` only (not `widget_kind`); the test is per-service-instance, not per-widget
|
||||
|
||||
---
|
||||
|
||||
## 2. Per-type test routines
|
||||
|
||||
Each routine lives alongside its integration module (e.g. `integrations/qbittorrent.py` gains `test_connection(config, secrets) -> TestResult`), constructs the appropriate client with a short timeout (≤10s), attempts a minimal read-only probe, catches exceptions, and translates them to human-friendly messages via a shared error-translation helper. The helper maps common failure modes — connection refused, DNS failure, SSL/cert error, HTTP 401/403 (auth), timeout — to actionable strings, modeled on the `test_machine_ssh` message-translation style.
|
||||
|
||||
### Requirement: CT-104 — qBittorrent test logs in and probes maindata; surfaces auth failures specifically
|
||||
|
||||
The `qbittorrent` test routine MUST construct a `QbittorrentClient` from the config + decrypted secrets, call login (`POST /api/v2/auth/login`), then probe `GET /api/v2/sync/maindata` (or `/app/version`). On success it MUST return `{ "ok": true, "evidence": <qBittorrent version or "connected"> }`. When qBittorrent's login returns anything other than `"Ok."` (e.g. the literal `"Fails."`), the routine MUST return `{ "ok": false, "detail": "Authentication failed — qBittorrent rejected the credentials." }` — surfacing the specific auth failure at the UI layer, not just in backend logs.
|
||||
|
||||
#### Scenario: correct credentials succeed with version evidence
|
||||
|
||||
- GIVEN a qBittorrent instance reachable at the configured URL with valid username/password
|
||||
- WHEN the test routine runs
|
||||
- THEN it returns `{ "ok": true, "detail": "Connected to qBittorrent.", "evidence": <version> }`
|
||||
|
||||
#### Scenario: wrong password surfaces a specific auth message
|
||||
|
||||
- GIVEN a qBittorrent instance that returns `"Fails."` on login
|
||||
- WHEN the test routine runs
|
||||
- THEN it returns `{ "ok": false, "detail": "Authentication failed — qBittorrent rejected the credentials.", "evidence": null }`
|
||||
|
||||
### Requirement: CT-105 — Prometheus test queries the Grafana gateway with expr "up"
|
||||
|
||||
The `prometheus` test routine MUST issue `POST {grafana_url}/api/ds/query` carrying `Authorization: Bearer {grafana_api_key}` and a `queries[0]` entry keyed by the configured `datasource_uid` with `expr: "up"`. There MUST be no direct Prometheus HTTP call (the `prometheus` service sources through Grafana per `grafana-metric-gateway`). On success it returns `{ "ok": true, "evidence": "Gateway reachable; datasource responded." }`. On HTTP 401/403 it returns an auth-specific message; on connection error it returns an unreachable message.
|
||||
|
||||
#### Scenario: gateway reachable returns ok
|
||||
|
||||
- GIVEN a prometheus service with valid Grafana gateway credentials
|
||||
- WHEN the test routine runs
|
||||
- THEN it returns `{ "ok": true, "detail": "Grafana gateway reachable.", "evidence": "Gateway reachable; datasource responded." }`
|
||||
|
||||
#### Scenario: wrong API key surfaces auth failure
|
||||
|
||||
- GIVEN a prometheus service whose `grafana_api_key` is invalid
|
||||
- WHEN the test routine runs
|
||||
- THEN it returns `{ "ok": false, "detail": <message mentioning authentication/authorization>, "evidence": null }`
|
||||
|
||||
### Requirement: CT-106 — Alertmanager test probes /api/v2/alerts or /api/v2/status
|
||||
|
||||
The `alertmanager` test routine MUST issue an authenticated GET (using the optional `api_key` secret when set) to `/api/v2/alerts` (or `/api/v2/status`). On success it returns `{ "ok": true, "evidence": <cluster version> }`. On failure it translates the error per the shared helper.
|
||||
|
||||
### Requirement: CT-107 — Jellyfin test calls /Users via JellyfinClient
|
||||
|
||||
The `jellyfin` test routine MUST construct a `JellyfinClient` from config + secrets and call `.users()`. On success it returns `{ "ok": true, "evidence": "<N> users" }`. On failure it translates the error.
|
||||
|
||||
### Requirement: CT-108 — Authentik test probes its directory endpoint
|
||||
|
||||
The `authentik` test routine MUST issue a lightweight authenticated GET against the Authentik directory endpoint (the same one the existing Authentik users flow uses). On success it returns `{ "ok": true, "evidence": <instance slug or "connected"> }`. On failure it translates the error.
|
||||
|
||||
### Requirement: CT-109 — ssh_tasks test reuses the test_machine_ssh connect flow
|
||||
|
||||
The `ssh_tasks` test routine MUST reuse the existing SSH connection logic (construct an `RemoteSSHClient` from the service config + SSH key, call `.connect()`, translate banner/auth errors per the established `test_machine_ssh` patterns). On success it returns `{ "ok": true, "evidence": "Connected to <host>:<port>" }`. The known-host recording behavior (first successful connect records the host key) is preserved.
|
||||
|
||||
### Requirement: CT-110 — Nextcloud test probes /status.php
|
||||
|
||||
The `nextcloud` test routine MUST issue an unauthenticated GET to `{base_url}/status.php`. On success it returns `{ "ok": true, "evidence": <Nextcloud version> }`. On failure it translates the error.
|
||||
|
||||
### Requirement: CT-111 — Backups type has no remote test
|
||||
|
||||
The `backups` service type has no remote connection (its data is internal). Its definition MUST set `test_callable = None`, and the endpoint returns `{ "ok": true, "detail": "No connection test for this service type" }` (CT-103).
|
||||
|
||||
---
|
||||
|
||||
## 3. No-persistence + security
|
||||
|
||||
### Requirement: CT-112 — The test endpoint MUST NOT persist any service row or secret
|
||||
|
||||
The test endpoint MUST NOT call `store.upsert_service`, `store.update_setting`, or any other persistence method. It accepts plaintext secrets in the request body (over TLS, identical to the existing create endpoint), probes the target, and discards the secrets. A test MUST verify this by calling `POST /api/services/test` and asserting the service count in the store is unchanged afterward.
|
||||
|
||||
#### Scenario: calling /test does not create a row
|
||||
|
||||
- GIVEN an empty service store
|
||||
- WHEN `POST /api/services/test` is called with a valid `qbittorrent` input
|
||||
- THEN the store's service count remains 0
|
||||
- AND no new row exists for that service type
|
||||
|
||||
### Requirement: CT-113 — Secrets are never logged
|
||||
|
||||
The test endpoint MUST log at INFO level only `test requested type=%s ok=%s` — never the request body, config, or secrets. The log call MUST pass through `sanitize_log_extra` (the existing helper) so any accidental inclusion of secret-named keys is scrubbed.
|
||||
|
||||
#### Scenario: logs contain no secret values
|
||||
|
||||
- GIVEN a test call carrying `secrets: { "password": "hunter2" }`
|
||||
- WHEN the endpoint runs
|
||||
- THEN no log line contains "hunter2"
|
||||
- AND the INFO log line reads `test requested type=qbittorrent ok=false` (or similar), with no secret values
|
||||
|
||||
---
|
||||
|
||||
## 4. Frontend test UI
|
||||
|
||||
### Requirement: CT-114 — useTestServiceInstance mutation hook + testServiceInstance API client function
|
||||
|
||||
`frontend/src/api/services.ts` MUST gain `testServiceInstance(input: ServiceInstanceInput): Promise<TestResult>` posting to `/api/services/test`. `frontend/src/hooks/useServices.ts` MUST gain `useTestServiceInstance()` returning a TanStack mutation whose `mutateAsync` resolves to `{ ok, detail, evidence }`.
|
||||
|
||||
### Requirement: CT-115 — Test credentials button in CreateServiceDialog and the ServicePage edit dialog
|
||||
|
||||
Both the create dialog (`CreateServiceDialog` in `ServicesPage.tsx`) and the edit dialog on `ServicePage.tsx` MUST render a "Test credentials" button below the config/secret form fields. Clicking it fires the `useTestServiceInstance` mutation with the current draft values. While pending, the button shows a "Testing…" state and is disabled.
|
||||
|
||||
### Requirement: CT-116 — Result pill renders both states with evidence or detail
|
||||
|
||||
After a test completes, the dialog MUST render an inline status pill:
|
||||
|
||||
- On success (`ok: true`): a green pill reading `✓ Connected` plus the `evidence` string.
|
||||
- On failure (`ok: false`): a red/destructive pill reading `✗ <detail>`.
|
||||
|
||||
The pill is rendered in the same dialog, below the Test button, above the footer.
|
||||
|
||||
### Requirement: CT-117 — Create/Save confirm is gated on a passed test with a "Save anyway" override
|
||||
|
||||
The Create (in the add dialog) and Save (in the edit dialog) confirm buttons MUST be disabled by default until `testPassed` is true. A "Save anyway" checkbox MUST appear that, when checked, re-enables the confirm button — allowing an operator to pre-configure a service that is not yet online. The default state of the checkbox is unchecked (gating is on).
|
||||
|
||||
### Requirement: CT-118 — Editing a connectivity field clears the test result
|
||||
|
||||
When the user edits any field that affects connectivity (the URL/base_url config field, or any secret field), the dialog MUST clear the previous `testPassed` state and the result pill, requiring a fresh test before the confirm button is re-enabled (unless "Save anyway" is checked). This prevents a stale green result from masking a typo correction.
|
||||
|
||||
---
|
||||
|
||||
## 5. Tests + gate greenness
|
||||
|
||||
### Requirement: CT-119 — Backend tests cover routines, dispatch, validation, and no-persistence
|
||||
|
||||
`pytest` from `backend/` MUST pass, including new tests for:
|
||||
|
||||
- Each type's `test_connection` routine (mocked client) returning the expected `{ok, detail, evidence}` shape for both success and at least one failure case.
|
||||
- The `POST /api/services/test` endpoint dispatching correctly per type.
|
||||
- Validation running before the test (422 on malformed config, no network call).
|
||||
- No-persistence: calling `/test` leaves the store unchanged.
|
||||
|
||||
### Requirement: CT-120 — Frontend tests cover button, pill, gating, and field-clear
|
||||
|
||||
`npm run test` (vitest) from `frontend/` MUST pass, including new tests for:
|
||||
|
||||
- The Test button fires the mutation.
|
||||
- The result pill renders both the green (ok) and red (failed) states.
|
||||
- The Create/Save confirm button is disabled until the test passes (and re-enabled by "Save anyway").
|
||||
- Editing a connectivity field clears `testPassed`.
|
||||
|
||||
### Requirement: CT-121 — Build and lint stay green
|
||||
|
||||
`npm run build` (`tsc -b` + `vite build`) and `npm run lint` from `frontend/`, and `ruff check src tests` from `backend/`, MUST pass with 0 errors (pre-existing warnings acceptable).
|
||||
|
||||
---
|
||||
|
||||
## Notes for downstream phases
|
||||
|
||||
- **Slice plan (≤400 lines each):** S1 = backend endpoint + `test_callable` field + per-type routines (7 types) + shared error-translation helper + backend tests; S2 = frontend API client + hook + Test button + result pill + gating + Save anyway + field-clear + frontend tests.
|
||||
- **The `#1` validation-surfacing fix (commit `493c0e1`) is a prerequisite** — it ensures the test endpoint's 422 validation errors render in the dialog. No action needed; already shipped.
|
||||
- **Stale-proposal correction:** the proposal mentions `jellyseerr` as a distinct service type. The active registry has no `jellyseerr` entry (merged into Jellyfin config by `services-as-hub-ia`). This spec covers the 8 active types: alertmanager, authentik, backups, jellyfin, nextcloud, prometheus, qbittorrent, ssh_tasks. Jellyseerr is dropped.
|
||||
- **TestResult dataclass:** `integrations/base.py` gains a small `TestResult` dataclass (`{ok: bool, detail: str, evidence: str | None}`) and `ServiceDefinition` gains an optional `test_callable: Callable[[dict, dict], TestResult] | None = None` field (placed last, after the existing fields, to satisfy dataclass field-ordering).
|
||||
- **Error translation:** the shared helper covers refused, DNS, SSL, 401/403, timeout; type-specific messages (e.g. qBit "Fails.") are handled inside each routine.
|
||||
Reference in New Issue
Block a user