Developer
f921524d37
New canonical openspec/specs/service-instance-scoping/spec.md (21 reqs PI-101..121). Change-side delta + sync-report. web-ui/prometheus-charting/ service-storage/service-credential-testing canonicals untouched.
277 lines
16 KiB
Markdown
277 lines
16 KiB
Markdown
# Service Instance Scoping
|
|
|
|
> Domain: `service-instance-scoping` · **Canonical specification.** Created by `per-instance-hook-scoping`.
|
|
>
|
|
> This is the merged end-state of Manage's per-instance hook-scoping fix: when multiple instances of
|
|
> Alertmanager, Prometheus, or the backups service are configured, the service-type tabs (`AlertsTab`,
|
|
> `MetricsTab`, `JobsTab`) and their backing hooks/fetch functions MUST scope to the instance whose
|
|
> page the operator is viewing — not whichever instance resolves first globally. New parameters are
|
|
> optional everywhere, preserving backward compatibility for all pre-existing callers, and dashboard
|
|
> widgets (which resolve service via a separate `useWidgetData` path) are explicitly untouched. This
|
|
> spec is acceptance-focused and verifiable; it deliberately does not prescribe implementation.
|
|
|
|
## Purpose
|
|
|
|
Define WHAT must be true of Manage's observability and backup service-type tab/hook scoping after the
|
|
`per-instance-hook-scoping` change: the frontend TanStack Query hooks (`useAlertmanagerAlerts`,
|
|
`useAlertmanagerStatus`, `usePrometheusStatus`, `useBackupJobs`, `useBackupRuns`, `useBackupAlerts`)
|
|
MUST accept an optional `serviceId`, fold it into their `queryKey` (so per-instance caches never
|
|
collide), and pass it to their fetch functions; the API client functions MUST append a `service_id`
|
|
query param when provided; the backup endpoints and `SettingsStore` query methods MUST filter by
|
|
`service_id` when a non-empty value is given and return all rows when omitted; the three tabs MUST
|
|
pass their received `instance.id` into the scoped hooks (and drop stale TODO comments); the instance
|
|
switcher MUST re-scope on switch. `usePrometheusTargets` and `useMonitoringMachines` stay global by
|
|
design, and every new parameter is optional with a default that preserves today's behavior. This spec
|
|
is acceptance-focused and verifiable; it deliberately does not prescribe implementation.
|
|
|
|
## Requirements
|
|
|
|
### Requirement: PI-101 — useAlertmanagerAlerts accepts serviceId
|
|
|
|
The hook `useAlertmanagerAlerts` MUST accept an optional `serviceId?: string` argument. When provided, the hook MUST (a) include the `serviceId` in its TanStack Query `queryKey` (so caches do not collide across instances), and (b) pass the `serviceId` to its underlying fetch function. When omitted, the hook MUST behave identically to today (global/first-configured resolution).
|
|
|
|
#### Scenario: per-instance cache key
|
|
|
|
- GIVEN two configured Alertmanager instances A and B
|
|
- WHEN `useAlertmanagerAlerts("A")` and `useAlertmanagerAlerts("B")` are both mounted
|
|
- THEN their `queryKey`s MUST differ (the `serviceId` appears in the key)
|
|
- AND a cache hit for A MUST NOT be served for B (no stale cross-instance data)
|
|
|
|
#### Scenario: omitted serviceId is backward-compatible
|
|
|
|
- GIVEN a caller that does not pass `serviceId`
|
|
- WHEN the hook runs
|
|
- THEN its behavior MUST match the pre-change hook (queryKey shape equivalent, fetch path identical)
|
|
|
|
### Requirement: PI-102 — useAlertmanagerStatus accepts serviceId
|
|
|
|
The hook `useAlertmanagerStatus` MUST accept an optional `serviceId?: string`, include it in its `queryKey`, and pass it to its fetch function. Omitting it MUST preserve today's behavior.
|
|
|
|
#### Scenario: per-instance cache key
|
|
|
|
- GIVEN instances A and B
|
|
- WHEN `useAlertmanagerStatus("A")` and `useAlertmanagerStatus("B")` are mounted
|
|
- THEN the `queryKey`s MUST differ and a cache hit for A MUST NOT serve B
|
|
|
|
### Requirement: PI-103 — usePrometheusStatus accepts serviceId
|
|
|
|
The hook `usePrometheusStatus` MUST accept an optional `serviceId?: string`, include it in its `queryKey`, and pass it to its fetch function. Omitting it MUST preserve today's behavior.
|
|
|
|
#### Scenario: per-instance cache key
|
|
|
|
- GIVEN instances A and B
|
|
- WHEN `usePrometheusStatus("A")` and `usePrometheusStatus("B")` are mounted
|
|
- THEN the `queryKey`s MUST differ and a cache hit for A MUST NOT serve B
|
|
|
|
### Requirement: PI-104 — useBackupJobs accepts serviceId
|
|
|
|
The hook `useBackupJobs` MUST accept an optional `serviceId?: string`, include it in its `queryKey`, and pass it to its fetch function. Omitting it MUST preserve today's behavior.
|
|
|
|
#### Scenario: per-instance cache key
|
|
|
|
- GIVEN backup services A and B
|
|
- WHEN `useBackupJobs("A")` and `useBackupJobs("B")` are mounted
|
|
- THEN the `queryKey`s MUST differ and a cache hit for A MUST NOT serve B
|
|
|
|
### Requirement: PI-105 — useBackupRuns accepts serviceId
|
|
|
|
The hook `useBackupRuns(jobId, status, serviceId?)` MUST accept an optional `serviceId?: string` in addition to its existing `jobId` and `status` arguments, include it in its `queryKey`, and pass it to its fetch function. Omitting it MUST preserve today's behavior.
|
|
|
|
#### Scenario: per-instance cache key
|
|
|
|
- GIVEN backup services A and B
|
|
- WHEN `useBackupRuns("job1", "all", "A")` and `useBackupRuns("job1", "all", "B")` are mounted
|
|
- THEN the `queryKey`s MUST differ
|
|
|
|
### Requirement: PI-106 — useBackupAlerts accepts serviceId
|
|
|
|
The hook `useBackupAlerts(jobId, acknowledged, severity, serviceId?)` MUST accept an optional `serviceId?: string` in addition to its existing arguments, include it in its `queryKey`, and pass it to its fetch function. Omitting it MUST preserve today's behavior.
|
|
|
|
#### Scenario: per-instance cache key
|
|
|
|
- GIVEN backup services A and B
|
|
- WHEN `useBackupAlerts("job1", false, "all", "A")` and `useBackupAlerts("job1", false, "all", "B")` are mounted
|
|
- THEN the `queryKey`s MUST differ
|
|
|
|
### Requirement: PI-107 — usePrometheusTargets and useMonitoringMachines stay global
|
|
|
|
The hooks `usePrometheusTargets` and `useMonitoringMachines` MUST NOT gain a `serviceId` parameter.
|
|
|
|
- `usePrometheusTargets` returns Node Exporter scrape targets for *external* Prometheus instances via `http_sd_configs` (cross-instance by design).
|
|
- `useMonitoringMachines` is a global cross-service concept (not per-instance).
|
|
|
|
#### Scenario: global hooks unchanged
|
|
|
|
- GIVEN the change is applied
|
|
- WHEN `usePrometheusTargets()` and `useMonitoringMachines()` signatures are inspected
|
|
- THEN neither accepts a `serviceId` arg and both behave identically to before
|
|
|
|
### Requirement: PI-108 — Observability fetch functions pass service_id query param
|
|
|
|
The fetch functions `fetchAlertmanagerAlerts`, `fetchAlertmanagerStatus`, and `fetchPrometheusStatus` (in `api/client.ts`) MUST each accept an optional `serviceId?: string`. When provided, the function MUST append `service_id=<id>` to the request URL as a query parameter (correctly using `?` for the first param and `&` for subsequent ones). When omitted, the request MUST be identical to today (no `service_id` param sent).
|
|
|
|
#### Scenario: serviceId appended
|
|
|
|
- GIVEN `fetchPrometheusStatus("svc-abc")` is called
|
|
- WHEN the request is issued
|
|
- THEN the URL contains `service_id=svc-abc`
|
|
|
|
#### Scenario: omitted serviceId is a no-op
|
|
|
|
- GIVEN `fetchPrometheusStatus()` is called (no serviceId)
|
|
- WHEN the request is issued
|
|
- THEN the URL MUST NOT contain `service_id`
|
|
|
|
### Requirement: PI-109 — Backup fetch functions pass service_id query param
|
|
|
|
The fetch functions `fetchBackupJobs`, `fetchBackupRuns`, `fetchBackupAlerts`, and `fetchBackupDashboard` (in `api/backups.ts`) MUST each accept an optional `serviceId?: string`. When provided, the function MUST append `service_id=<id>` to the request URL. When omitted, the request MUST be identical to today.
|
|
|
|
#### Scenario: serviceId appended alongside existing params
|
|
|
|
- GIVEN `fetchBackupRuns("job1", "failed", "svc-xyz")` is called
|
|
- WHEN the request is issued
|
|
- THEN the URL contains both `job_id=job1&status=failed` (existing params) AND `service_id=svc-xyz`
|
|
|
|
#### Scenario: omitted serviceId is a no-op
|
|
|
|
- GIVEN `fetchBackupJobs()` is called (no serviceId)
|
|
- WHEN the request is issued
|
|
- THEN the URL MUST NOT contain `service_id`
|
|
|
|
### Requirement: PI-110 — Backup endpoints accept optional service_id
|
|
|
|
The endpoints `get_backup_jobs`, `get_backup_runs`, `get_backup_alerts`, and `get_backup_dashboard` in `routers/backups.py` MUST each accept an optional `service_id: str | None = None` query parameter and thread it into the corresponding `SettingsStore` query method.
|
|
|
|
#### Scenario: service_id passed through to store
|
|
|
|
- GIVEN a request `GET /api/backups/jobs?service_id=svc-xyz`
|
|
- WHEN the endpoint runs
|
|
- THEN `store.list_backup_jobs` MUST be called with `service_id="svc-xyz"`
|
|
|
|
### Requirement: PI-111 — SettingsStore backup queries filter by service_id when provided
|
|
|
|
The methods `list_backup_jobs`, `list_backup_runs`, and `list_backup_alerts` in `services/settings_store.py` MUST each accept an optional `service_id: str | None` parameter. When `service_id` is a non-null, non-empty string, the query MUST include a `WHERE service_id = ?` clause (or equivalent scoping) returning only rows attributed to that service. When `service_id` is `None` or empty, the query MUST return ALL rows (no filter) — preserving backward-compatibility for existing callers.
|
|
|
|
The `backup_jobs` and `backup_runs` tables already carry `service_id` attribution (from the `services-as-hub-ia` Slice 3 work); this requirement adds the filter, not the column.
|
|
|
|
#### Scenario: filtered to one service
|
|
|
|
- GIVEN the store holds jobs for services A and B
|
|
- WHEN `list_backup_jobs(service_id="A")` is called
|
|
- THEN only service A's jobs MUST be returned
|
|
|
|
#### Scenario: no filter returns all
|
|
|
|
- GIVEN the store holds jobs for services A and B
|
|
- WHEN `list_backup_jobs(service_id=None)` is called
|
|
- THEN jobs for BOTH services MUST be returned (backward-compat)
|
|
|
|
#### Scenario: empty string is treated as no filter
|
|
|
|
- GIVEN the store holds jobs for services A and B
|
|
- WHEN `list_backup_jobs(service_id="")` is called
|
|
- THEN jobs for BOTH services MUST be returned (defensive against falsy strings)
|
|
|
|
### Requirement: PI-112 — Alertmanager/Prometheus status endpoints already accept service_id
|
|
|
|
The endpoints `get_alertmanager_alerts`, `get_alertmanager_status`, and `get_prometheus_status` in `routers/monitoring.py` ALREADY accept a `service_id: str | None = None` query parameter and resolve via `resolve_service_record(store, "<type>", service_id)`. No backend change is required for these. This requirement exists to document the asymmetry: the frontend wiring change for these hooks is necessary (PI-108 + PI-101..103), but the backend endpoint work is zero.
|
|
|
|
#### Scenario: existing endpoint behavior confirmed
|
|
|
|
- GIVEN the change is applied
|
|
- WHEN `get_alertmanager_alerts(service_id="svc-a")` is called
|
|
- THEN it MUST resolve to the `svc-a` Alertmanager instance (pre-existing behavior, unchanged)
|
|
|
|
### Requirement: PI-113 — AlertsTab passes instance.id
|
|
|
|
The component `pages/service-tabs/AlertsTab.tsx` MUST call `useAlertmanagerAlerts(instance.id)` and `useAlertmanagerStatus(instance.id)` (passing the `instance.id` it already receives as a prop). The existing TODO comment about "hooks don't accept a service_id yet" MUST be removed.
|
|
|
|
#### Scenario: data is instance-scoped
|
|
|
|
- GIVEN two Alertmanager instances A and B are configured
|
|
- WHEN the operator navigates to instance A's service page
|
|
- THEN AlertsTab MUST show alerts sourced from instance A (not B, not first-resolved)
|
|
|
|
### Requirement: PI-114 — MetricsTab passes instance.id
|
|
|
|
The component `pages/service-tabs/MetricsTab.tsx` MUST call `usePrometheusStatus(instance.id)` (passing the `instance.id` it already receives as a prop). The existing TODO comment MUST be removed. (`usePrometheusTargets` stays global — PI-107.)
|
|
|
|
#### Scenario: data is instance-scoped
|
|
|
|
- GIVEN two Prometheus instances A and B are configured
|
|
- WHEN the operator navigates to instance A's service page
|
|
- THEN MetricsTab MUST show status for instance A (not B, not first-resolved)
|
|
|
|
### Requirement: PI-115 — JobsTab passes instance.id
|
|
|
|
The component `pages/service-tabs/JobsTab.tsx` MUST call `useBackupJobs(instance.id)`, `useBackupRuns(<jobId>, <status>, instance.id)`, and `useBackupAlerts(<jobId>, <acknowledged>, <severity>, instance.id)`. The existing TODO comment MUST be removed.
|
|
|
|
#### Scenario: data is instance-scoped
|
|
|
|
- GIVEN two backups service instances A and B are configured
|
|
- WHEN the operator navigates to instance A's service page
|
|
- THEN JobsTab MUST show jobs/runs/alerts for service A only
|
|
|
|
### Requirement: PI-116 — Instance switcher re-scopes tab data
|
|
|
|
The `ServicePage` instance switcher (present when `enabledSiblings.length > 1`) changes the active instance. When the operator switches from instance A to instance B, the three tabs MUST re-fetch and re-render with instance B's data. (This falls out naturally from passing `instance.id` into the hook `queryKey`s — switching the prop changes the key, triggering a fresh fetch — but it is verified explicitly because it's the user-visible correctness goal.)
|
|
|
|
#### Scenario: switch re-scopes
|
|
|
|
- GIVEN the operator is viewing instance A's MetricsTab (data from A)
|
|
- WHEN the operator uses the instance switcher to select instance B
|
|
- THEN the tab MUST re-render with instance B's status (not A's cached data)
|
|
|
|
### Requirement: PI-117 — Dashboard widgets render identically before/after
|
|
|
|
Dashboard widgets (`AlertmanagerAlertsWidget`, `BackupsWidget`, `PrometheusMetricWidget`/`Metric*`, etc.) resolve their service via the widget-instance's `service_id` binding through `useWidgetData` — a different code path from the hooks changed here. They MUST render identically before and after this change. The widget component tests MUST remain unchanged and green.
|
|
|
|
#### Scenario: widget path unaffected
|
|
|
|
- GIVEN the change is applied
|
|
- WHEN a dashboard widget fetches its data
|
|
- THEN the fetch path (`useWidgetData(widget.id, …)`) MUST be unchanged; no `useAlertmanager*` / `useBackup*` hook is on the widget data path
|
|
|
|
### Requirement: PI-118 — All new params are optional; omitting equals today's behavior
|
|
|
|
Every new parameter added by this change (hook `serviceId?: string`, fetch-function `serviceId?: string`, endpoint `service_id: str | None = None`, store-method `service_id: str | None = None`) MUST be optional with a default that preserves today's behavior. No existing caller is forced to pass the new argument. Existing callers that omit it MUST see identical results to the pre-change code.
|
|
|
|
#### Scenario: existing callers unbroken
|
|
|
|
- GIVEN any pre-existing caller of `useBackupJobs()`, `fetchBackupRuns()`, `GET /api/backups/jobs`, or `store.list_backup_jobs()`
|
|
- WHEN the change is applied and the caller is run without modification
|
|
- THEN the behavior MUST be identical to before (no error, same data returned)
|
|
|
|
### Requirement: PI-119 — Backend tests + lint green
|
|
|
|
`PYTHONPATH=src python3 -m pytest -q` and `PYTHONPATH=src python3 -m ruff check src tests` from `backend/` MUST pass. New backend tests MUST cover: `list_backup_*` filtering by `service_id` (filtered, unfiltered, empty-string); backup endpoint threading `service_id` into the store.
|
|
|
|
#### Scenario: backup filter tests exist
|
|
|
|
- GIVEN the change is applied
|
|
- WHEN `pytest tests/test_backups.py` (or equivalent) runs
|
|
- THEN it MUST include a test asserting `service_id="A"` returns only A's rows AND `service_id=None` returns all rows
|
|
|
|
### Requirement: PI-120 — Frontend typechecks, builds, and lints
|
|
|
|
`npm run build` (which runs `tsc -b` + `vite build`) and `npm run lint` from `frontend/` MUST pass (0 errors; pre-existing warnings are acceptable).
|
|
|
|
### Requirement: PI-121 — Hook + tab tests cover the new serviceId wiring
|
|
|
|
The frontend test suite MUST include:
|
|
|
|
- A hook test asserting the `serviceId` appears in the `queryKey` (per-instance cache isolation) for at least one representative hook (e.g. `useBackupJobs`).
|
|
- A tab test asserting `instance.id` is passed through to the hook (for at least one of AlertsTab / MetricsTab / JobsTab).
|
|
|
|
#### Scenario: per-instance queryKey test exists
|
|
|
|
- GIVEN the change is applied
|
|
- WHEN the test suite runs
|
|
- THEN a test MUST assert that calling the hook with different `serviceId` values produces different `queryKey`s
|
|
|
|
#### Scenario: tab passes instance.id test exists
|
|
|
|
- GIVEN the change is applied
|
|
- WHEN the test suite runs
|
|
- THEN a test MUST assert that at least one tab component passes its received `instance.id` to the scoped hook
|