mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-24 12:42:49 +00:00
415c09dafcd08f78dfd467c6785e0eead8f13a21
5238 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 rcourtman
|
415c09dafc |
Judge resources API freshness by the monitor's thresholds
The resources API rebuilds its registry from the monitor's unified seed, but built it with NewRegistry and plain ingests, so its stale pass used the registry defaults while the monitor judges each source by its configured polling cadence. A Proxmox node polled every two minutes read stale, with warning status, in /api/resources ninety seconds after its poll while the websocket called it online. Presentation coalescing had the same gap in both views. When it merges an agent row the registry kept apart into its Proxmox node by hostname, it picked each reading's source by the default thresholds, so the row showed a silent agent's readings over a poll the registry calls current. The seed now carries the thresholds its registry generation was judged by. The monitor adapter records them per generation: thresholds set since the last rebuild wait for the rebuild that applies them, and the read-state overlays and live supplemental refreshes of a generation are judged by its thresholds, since re-judging a listing cannot clear a stale verdict it already carries. The monitor lists a generation with its thresholds from one registry pointer, and a view built from mock fixtures or a bare snapshot reports none (the defaults it was judged by). The monitor hands seed, freshness and thresholds over from one view, per tenant too. The resources API builds its registry with them (NewRegistryWithStaleThresholds: ingests and stale passes that pass none use them) and keys its cache on its own copy. The coalesce judges metric freshness with metricSourceStale, the rule the stale pass and the merge already share, and both the broadcast and ListForPresentation pass their view's thresholds. The broadcast coalesce also honours the operator's merge exclusions, so the adapter's CoalesceForPresentation takes the listing's thresholds next to them. Patrol's scoped runtime state still rebuilds registries with the defaults. |
||
|
rcourtman
|
d2ac0c5112 |
Remove the summary chart routes no client calls (#2720)
Removes `GET /api/charts/workloads-summary` and `GET
/api/charts/storage-summary`, their frontend clients
(`ChartsAPI.getWorkloadsSummaryCharts`,
`ChartsAPI.getStorageSummaryTrend` and their types), and the backend
code that existed only for them.
**Why they are dead**
- `getWorkloadsSummaryCharts` arrived with the v6 import and was never
called: `git log -S getWorkloadsSummaryCharts -- frontend-modern/src
':!*test*'` lists only that commit, and no other source file ever
referenced the route.
- `getStorageSummaryTrend` lost its last reader when
`storageSummaryTrendCache.ts` was deleted in
|
||
|
pulse-triage[bot]
|
b03965a0d2 | Merge branch 'main' into refactor/drop-summary-chart-routes | ||
|
pulse-triage[bot]
|
0bc18c5ddb | Merge branch 'main' into fix/fresh-install-alert-defaults | ||
|
rcourtman
|
19d8b4f81a |
Resolve saved hosts an operator link joins to the link's primary
After a restart, an agent an operator linked into a live resource, such as an agent inside a vSphere VM, exists only through saved-host continuity until it reports again. Three consumers then disagreed about what a reference to it meant. The read state kept the saved agent as its own offline row and resolved references to it there. The resources API, which seeds a registry from that listing and re-applies the links, folded it into the VM and merged its stale telemetry. Alert intent resolved through the published registry, where the agent is absent, so its agent:<host ID> alerts kept their literal reference. Maintenance or NeverAutoRemediate set on the VM reached neither the agent's alerts nor its Patrol findings in that window, while the API said it applied. An operator link names one identity; telemetry separation is the only reason the saved row stays apart. Link application now holds a saved row beside the link's primary instead of merging it, in every registry. A reference that resolves to the saved row (its canonical ID, agent:<host ID>, its machine ID or an alias it shares with the primary) resolves to the primary, as a folded row's references do, while each row keeps its own telemetry. An in-memory marker on rows continuity introduced rides clones, so the resources API's registry holds them the same way. A saved primary leaves a live member its own identity. Alert intent now resolves references and ancestors through the published registry with saved hosts overlaid, the read state Patrol resolves through. That also lets intent set on an unlinked saved host reach its offline alert. The resolver runs under the alert manager's lock once per metric check, so the overlay is kept per published registry generation, publication builds it before alerts are evaluated, and a build that overlaps a mock-mode switch is discarded. |
||
|
rcourtman
|
b60f441e22 |
Resolve Patrol finding references before reading operator state
Patrol's finding operator-state provider read the store under the finding's exact reference first and resolved it only on a miss. Once an operator link folds an agent into a VM, a finding kept under the agent's ID still found the agent's own row, even a default-valued one, and missed the maintenance window or monitoring mode the operator then set on the merged VM. The resources API and alert intent resolve first, so Patrol disagreed with both. The provider now resolves every reference through the organization monitor's current read state (the published registry with saved hosts overlaid, or the mock estate), which is the listing the resources API seeds from, and reads only an unresolved reference as given. The old miss path built a resources-API registry, which clones the unified listing, for every finding without a row; the read state answers from a registry it keeps. One window still differs: a saved agent linked into a live VM that has not reported since a restart stays its own row in the read state, so its findings read the agent's own state there, as alert intent does. |
||
|
rcourtman
|
7949036657 |
Merge origin/main into fix/link-fold-references
PR 2739 added Resource.linkFolds, a record of every link folded into a resource that rides in-memory clones, which is the same fold set this branch carried as Resource.linkFoldedIDs. Keep one record: drop linkFoldedIDs and index folded IDs from linkFolds when a link applies and when resource seeding copies the record in. Main's unlink test treated a lookup by the folded agent's ID as proof that the agent was still listed. That lookup now answers with the VM holding it, as lookups by source IDs and canonical aliases already do, so the test checks which row answered and pins the VM answer. |
||
|
rcourtman
|
31fcc4f4d6 |
Split operator-linked resources when a merge is reported (#2739)
Stacked on #2725 (Split a manually linked pair when the API unlinks it). Until that merges this PR carries its commit too; only the last commit is new here. ## Problem `POST /api/resources/{id}/report-merge` (the drawer's "Split merged resource" dialog) excluded only the candidate IDs that `ResourceRegistry.SourceTargets` derives with `sourceSpecificID(mergedResource.Type, source, sourceID)`. For a source an operator link folded in, that candidate names neither side of the link: - an agent linked into a VM gets a `vm-` candidate; - an agent linked with a node or a Docker host gets a source-specific candidate, while the folded side's canonical ID derives from its machine ID. No `resource_links` row matched, so even with #2725's one-decision-per-pair store rule the link survived, the pair stayed merged on REST, `/api/state` and the websocket, and the API answered 200 "Merge reported" while the dialog toasted "Resource split applied". Two linked agents share their only source, so the API refused them as "Resource is not merged". Identity merges were already handled: `findMatch` checks exactly the candidate `SourceTargets` reports, and an agent/Docker pair merged by machine identity stays apart through three monitor rebuilds after report-merge. That is now pinned by a test. ## Fix - `applyManualLinks` records each link it applies on the merged resource (`recordManualLinkFold`, unexported `Resource.linkFolds`): the link row's own pair plus the sources the folded side brought, carrying the folded side's earlier folds so each link along a chain keeps its pair. - The record rides in-memory clones. The resources API seeds its registry from the monitor's already-linked listing and never holds the folded row, so this is the only place it can learn the pair. - `HandleReportMerge` excludes the pair of every link whose folded side brought a reported source (`ResourceRegistry.ManualLinkFolds`), alongside the existing identity-match exclusions, deduplicated in either order. #2725's `AddExclusion` then deletes the link row, so the pair splits from the monitor's next rebuild, as with unlink. An applied link also counts as a merge, so two linked agents can be reported. - A pair is recorded once. Record ingest can recreate a folded side whose source mapping points at a holder of another type, then fold it again. - A link joins only the pair its row names. Canonical-ID succession re-keys link endpoints with `UPDATE OR IGNORE` but moves `primary_id` unconditionally. A row whose re-key collided with the successor's own row therefore kept the retired endpoint and took the successor as its primary, and `applyManualLinks` merged the successor through it. No unlink or report-merge of the merged pair removed that row. A primary naming neither side now falls back to the row's first side, which leaves such a row inert. The drawer sends every merged source. Source categories are not attributed to individual contributors, so naming one that a folded side carried undoes that link. A source that arrived along a chain undoes every link on its way to the merged resource. ## Verification - `TestResourceReportMergeSplitsOperatorLinkedPair`: link and report-merge through the API handlers against a `MonitorAdapter` on the same SQLite store, rebuilt three times per step. Cases cover VM/agent, node/agent and agent/Docker pairs linked from either side, plus two linked agents. With #2725's handler, 6 of the 7 cases fail: five with "report-merge left 1 monitor resources", and the agent pair with a 400. The exception is a node linked from its agent, because the node's source-specific candidate happens to be its own ID. - `TestResourceReportMergeSplitsPairBehindASuccessionShadowedRow`, `TestResourceReportMergeSourceFilterSelectsLinks`, `TestResourceReportMergeSplitsIdentityMergedPair`, `TestContract_ResourceReportMergeReplacesOperatorLink` (response shape plus link removal), `TestManualLinkFoldsNameEachLinkPairAlongAChain` (monitor registry and a registry seeded from its listing), `TestManualLinkFoldsRecordEachPairOnceAcrossRecordIngests`, `TestManualLinkFoldRepeatedPairKeepsEarlierSources`, `TestCloneResource_CarriesLinkFolds`, and the `TestResourceAPIReportMergeExcludesRegistryLinkFolds` guardrail. - Negative controls, each failing at the expected assertion: the handler from #2725, the old not-merged gate, the primary honoured outside its row, folds appended without dedupe, a repeated pair keeping only its later sources, `applyManualLinks` without `recordManualLinkFold`, and a clone that shares the fold slice. - Full `./internal/unifiedresources/...`, `./internal/api/resourceapi/...`, `./internal/truenas/...` and `./internal/monitoring/...` pass. |
||
|
rcourtman
|
295edef4d5 |
Bind SAML logins to the browser that started them (#2729)
## Why
With `allowIdpInitiated` off, the default, every SAML login failed.
`SAMLService.ProcessResponse` passed crewjam/saml an empty list of
request IDs, and crewjam refuses any Response whose `InResponseTo` is
not in that list, so both SP-initiated and unsolicited Responses ended
in "Authentication failed". Only providers with IdP-initiated logins
enabled, which skips the check, could sign in.
## What changes
- `/api/saml/{id}/login` records each AuthnRequest ID, on the provider's
service, with the login's `returnTo` and the hash of a random token it
sets as the `HttpOnly`, `SameSite=Lax` cookie `pulse_saml_login`
(`__Host-pulse_saml_login` over HTTPS). A browser that already holds a
token keeps it, so logins started one after another in several tabs all
stay valid. Records last ten minutes, 1,024 per provider, oldest
evicted.
- The IdP posts its Response cross-site, and browsers withhold
`SameSite=Lax` cookies from cross-site POSTs. The ACS answers a
cookieless POST with a small page that re-posts the same fields once
from Pulse's own origin (no form action, so path prefixes survive; CSP
nonce on the script; a no-JS Continue button; a marker field stops a
second repost).
- `ProcessResponse` reads the answered request ID from the canonical
content goxmldsig verified for the signature covering the assertion
crewjam returns: the signed Response (or the Response in a signed
ArtifactResponse), otherwise that assertion's own
`SubjectConfirmationData`. It spends the record only when it is bound to
the presenting browser, and the login returns to the recorded `returnTo`
rather than the posted `RelayState`. A Response for another browser
leaves that browser's record intact.
- The signature verifier refuses signed content carrying a prefixed
attribute or namespace declaration named like an un-namespaced attribute
crewjam reads. encoding/xml fills such a field from any attribute with
that local name, and exclusive canonicalization leaves an unused
declaration out of the signed form. On the base commit, appending
`xmlns:IssueInstant` and `xmlns:NotOnOrAfter` to a two-hour-old signed
Response got it accepted, so with `allowIdpInitiated` on a captured
login could be replayed indefinitely.
- RelayState is now URL-escaped into the redirect, so a `returnTo` with
its own query string reaches the IdP whole.
- With `allowIdpInitiated` on, behaviour is otherwise unchanged: no
cookie or repost needed, posted `RelayState` honoured.
- The SLO LogoutRequest store from #2713 is generalised into a shared
record store; its `consume` semantics are unchanged.
Stacked on #2714 (on #2713, on #2707). Those three conflict with main in
`storage-recovery.md`, where their boundary notes and main's land at the
same anchor, so none can merge on its own. This branch carries a merge
of main that keeps both sets of notes; landing it lands the stack.
## Verification
- New tests in `internal/api/saml_handlers_more_test.go`,
`saml_service_test.go` and `audit_handlers_test.go`: default-settings
login through the repost, multiple tabs, refusal of another browser's,
unknown, unsolicited and replayed Responses (the refused browser's own
login still completes afterwards), the `xmlns:InResponseTo` spoof with
the Response signed and with only the Assertion signed, a
confirmation-less signed assertion spliced ahead of another browser's
assertion, assertion-only signed success, stale IdP-initiated Responses
with spoofed ages, repost escaping and CSP nonce, and the audit event
for a binding refusal.
`TestContract_SAMLManualIDPCertificateVerifiesIdPSignatures` now runs at
default settings instead of enabling IdP-initiated logins.
- Negative controls by `-overlay`: with the attribute refusal disabled
the stale-Response replay is accepted again; with the refusal disabled
and the binding read from crewjam's parsed fields the
`xmlns:InResponseTo` spoof logs in; the round-one binding code accepted
the spliced-assertion case as the other user.
- A real Chromium run against a test IdP on `127.0.0.1` and the SP
handlers on `localhost`, behind `SecurityHeadersWithConfig` in
production CSP mode: the IdP's POST arrived `Sec-Fetch-Site: cross-site`
without the login cookie, the repost arrived `same-origin` with it, and
the login landed signed in at
`/alerts?range=7d&saml=success&tab=history`.
## Known limits
- Records live in memory: a restart, a provider re-initialization, or a
login started on a host that does not share cookies with the configured
public URL leaves the Response unmatched, and the user signs in again.
- Two logins started at the same moment in a browser that holds no token
yet each mint one; the last cookie wins and the other login fails.
- As with OIDC login state, a sustained flood of unauthenticated login
starts can evict pending records.
- On plain HTTP, or HTTPS that Pulse does not recognise behind a proxy,
the cookie has no `__Host-` prefix, so a sibling subdomain could plant
one, the same exposure the session cookie has there.
|
||
|
rcourtman
|
826252d274 |
Stop sending the ignored autonomous_mode chat field (#2730)
## Why
Since
|
||
|
rcourtman
|
b7bdf1b4eb |
Split a manually linked pair when the API unlinks it (#2725)
## Problem
`POST /api/resources/{id}/unlink` answered 200 "Resources unlinked" but
only recorded a `ResourceExclusion`. The manual link row stayed in
`resource_links`, and `ResourceRegistry.applyManualLinks` never consults
exclusions, so the pair stayed merged in the monitor's registry
rebuilds, the resources API and the websocket broadcast. Repro: link a
standalone agent into a VM, unlink it, then `GET /api/resources/<vm>`:
the agent facet is still there and the agent row is still gone.
Separately, the websocket broadcast coalesced host views with
`CoalescePresentationHostResources`, which applies no exclusions, while
REST's `ListForPresentation` applies the registry's, so a host pair the
operator split could be one broadcast row and two REST rows.
No frontend code calls `/api/resources/{id}/link` or `/unlink`
(`/api/agents/agent/unlink` is a different endpoint).
## Semantics
A resource pair carries one operator decision, and the later one
replaces the earlier: a link joins the pair, an exclusion (unlink,
report-merge) keeps it apart.
- Store writes: `AddExclusion` deletes the pair's link rows and
`AddLink` deletes its exclusion rows, in one transaction, matching both
ID orders; each also deletes a row of its own kind that canonical-ID
succession stored in the other order. The in-memory store does the same
and stamps `CreatedAt` like SQLite.
- Store reads: `GetLinks` / `GetExclusions` keep only each pair's latest
decision (`effectiveManualPairDecisions`), so rows written before this
change or rewritten onto one pair by succession resolve by their later
write; a tie keeps the pair apart.
- Registry: `loadOverrides` resolves its two reads together, so an
unlink committed between them cannot load both decisions into one
generation.
- Identity pins: while a Proxmox node was linked to an agent, the merged
resource's pin could record the node's endpoint hostname with the
agent's machine key. After unlink the node completed that key, minted
the agent's ID and folded into it by ID, a path no exclusion check sees.
A pin no longer completes the identity of a record the operator split
from the pinned resource.
- Broadcast: coalesces through `MonitorAdapter.CoalesceForPresentation`
on the read state that listed the rows when its registry is store-backed
(the store's adapter or a host-continuity overlay), else on the resource
store's adapter (the mock view lists through a store-less registry),
sharing `presentationExclusionFilter` with `ListForPresentation`.
In production REST seeds from the monitor's already-linked listing, so
an unlink shows on REST, `/api/state` and the websocket from the
monitor's next registry rebuild, the same generation in which a new link
reaches the broadcast.
## Known limits (unchanged, documented in the contract)
- Report-merge derives candidate IDs from the merged resource's type, so
it does not name the folded side of a cross-type manual link and cannot
undo one; unlink does.
- An agent's own declared node link (`resolveLinkedResource`) still
ignores exclusions.
- Presentation coalescing compares the IDs of the rows being merged, so
a third same-host fragment that merges first can carry an excluded pair
into one row (REST and broadcast now agree on this).
## Verification
- `TestResourceUnlinkSplitsManuallyLinkedPair` (new
`resources_unlink_test.go`) drives link, unlink and relink through the
API handlers against a `MonitorAdapter` on the same SQLite store,
rebuilt twice per step. It failed on main at "after unlink" exactly as
reported.
- `TestUnlinkSplitsHostPairWhosePinTookTheOtherSidesKey`: node/agent
pairs (IP and FQDN endpoints) through three rebuilds per step; both FQDN
cases fail with the pin guard removed.
- `TestManualPairDecisionReplacesThePreviousOne`,
`TestManualPairDecisionReadsResolveStoredConflictsByRecency`,
`TestRegistryResolvesDecisionsAcrossItsTwoReads`,
`TestMonitorAdapterCoalesceForPresentationHonoursExclusions`,
`TestBroadcastPresentationCoalesceHonoursMergeExclusions`,
`TestBroadcastPresentationCoalescePrefersTheListingReadState`.
- Negative controls (each fails with its fix reverted): store write
rule, newest-link-per-pair, registry combined resolution, pin guard,
broadcast exclusion coalesce, broadcast read-state preference,
store-less adapter fallback.
- Full `./internal/unifiedresources/...`,
`./internal/api/resourceapi/...` and `./internal/monitoring/...` pass.
- Live replay on private mock stacks built from main and from this
branch (`POST .../link`, `.../unlink`, `.../link` on a mock VM and a
standalone agent, then `GET /api/resources/{id}` for both). Main: after
unlink the VM still carries the agent facet and the agent row is still
404, unchanged 25 s later. This branch: after unlink the VM has no agent
facet and the agent row is back at once; relink folds it again.
Report-merge and agent-declared node links are left for separate
follow-ups.
|
||
|
rcourtman
|
860b5087d2 |
Merge origin/main into fix/link-fold-references
PR 2719 added per-side link pins beside the fold bookkeeping this branch rewrote; keep both, and replace the contract bullet that said folded IDs do not resolve. |
||
|
rcourtman
|
6dea7101a7 |
Split operator-linked resources when a merge is reported
POST /api/resources/{id}/report-merge excluded only candidate IDs that
SourceTargets derives from the merged resource's type. For a source an
operator link folded in, that candidate names neither side of the link:
an agent linked into a VM got a vm- ID, and an agent linked with a node
or Docker host keyed by its machine ID got a source-specific one. No
link row matched, so the pair stayed merged while the API answered
"Merge reported" and the drawer toasted "Resource split applied". Two
linked agents share their only source and were refused outright as
"Resource is not merged".
applyManualLinks now records each link it applies on the merged
resource: the link row's own pair and the sources the folded side
brought, once per pair, carrying earlier folds so a chain keeps every
pair. The record rides in-memory clones, so the resources API's
registry, seeded from the monitor's already-linked listing, has it
although it never holds the folded row. Report-merge excludes those
pairs for the reported sources, and the store's one-decision-per-pair
rule deletes the link, as unlink does.
A link also joins only the pair its row names now. Canonical-ID
succession moves primary_id even when the endpoint re-key collides
with the successor's own row, and honouring that primary merged the
successor through a row that no unlink or report-merge of the merged
pair removes.
|
||
|
rcourtman
|
39b6007f5d |
Pin each manual-link side from its own identity (#2719)
Stacked on #2715 (its commit is this branch's parent; whichever lands first, the other still merges). ## Problem A manual link merges two resources for display, but `PersistIdentityPins` pinned the merged identity. A link primary was pinned with the folded resource's machine key, and the store's one-owner-per-key rule deleted the folded resource's own pin. For a Proxmox node with a qualified endpoint (`https://pve1.example.test:8006`) linked to a host agent, the node's pin records the endpoint, so the next rebuild completed the agent's machine key from it, minted the agent's machine-derived ID and merged the two by ID before the link applied. With the node as primary, succession then rewrote the link into a self-link on the agent's ID; in either direction, removing the link left the pair merged. Reproduced through `MonitorAdapter.PopulateSnapshotAndSupplemental` with a `MemoryStore` on #2715's head. ## Change - `applyManualLinks` captures each side's own pin before the merge (`recordLinkOwnPin`, first capture per rebuild wins), and `PersistIdentityPins` writes those instead of the merged projection, keeping a pinnable folded side pinned under its own canonical ID. - Link sides write through a new `ResourceStore.ReplaceResourceIdentityPins` (both stores). Like an upsert the row keeps fields the side's own pin leaves empty, but it drops a strong key another resource holds this rebuild, which is how a key an earlier release borrowed from the merged identity leaves the row. - A folded side drops a key a live resource or an earlier folded side already holds instead of trading it every rebuild, and writes no pin when that would cost it its machine evidence (a cluster-only row would be open to cluster-slot succession). - Proxmox auto-links (LinkedAgentID / identity matcher) merge before any manual link applies, so their boot-window pins are unchanged (`TestIssue1753*`, `TestMonitorAdapterRebuildPersistsIdentityPins` pass). Known limits, written into the contract: a node row an earlier release already filled with the agent's key still merges the pair by ID while the node can complete from it; two live resources sharing a strong key still alternate, as on main; evidence a later record pass of the same rebuild merges into an already-joined row reaches neither side's pin. ## Verification - New tests in `registry_merge_policy_test.go` (standalone and clustered node, both link directions, three rebuilds with no steady-state pin writes, then unlink separates the pair; shared-key, kept-field, hostname-guard, machine-loss and borrowed-key cases) and `store_test.go` (replace vs upsert on memory and SQLite). - Each new test fails against #2715's runtime files via `go test -overlay`; each guard rule fails its own test when removed. - `internal/unifiedresources` full package, `internal/monitoring` link/identity tests, `internal/api/resourceapi`, `go build ./...` and `go vet` pass. - Contract: `unified-resources.md` clause 25 and "Operator links reach record-ingested resources". |
||
|
rcourtman
|
d95a316118 |
Stop sending the ignored autonomous_mode chat field
Since
|
||
|
rcourtman
|
077c3a8959 |
Merge origin/main into fix/saml-acs-request-binding
Brings the branch up to the main it lands on; the merge is clean. Contract-Neutral: Merge of origin/main into the SAML stack: main's changes were governed when they landed, and the merge is clean |
||
|
rcourtman
|
d72d355ccf |
Merge origin/main into fix/saml-acs-request-binding
The SAML stack this branch builds on appends its boundary notes to storage-recovery.md at the anchor main used for the linked-agent stale flag note. Both sets of notes are kept, the SAML ones first. Contract-Neutral: Merge of origin/main into the SAML stack: main's changes were governed when they landed, and the only resolution keeps both sets of storage-recovery boundary notes |
||
|
rcourtman
|
58ee64fc1c |
Split a manually linked pair when the API unlinks it
POST /api/resources/{id}/unlink recorded only an exclusion. The link row
stayed, and applyManualLinks never consults exclusions, so the monitor's
rebuilds, the resources API and the websocket kept the pair folded while
the API answered "Resources unlinked".
A pair now carries one operator decision. AddExclusion deletes the pair's
links and AddLink its exclusions, in one transaction and in either ID
order. Reads keep each pair's latest decision, so rows written before
this change, or rewritten onto one pair by canonical-ID succession,
resolve by their later write, and a registry resolves its two reads
together so an unlink landing between them cannot load both.
While a Proxmox node was linked to an agent, the merged resource's
identity pin could take the agent's machine key. After unlink the node
completed that key, minted the agent's ID and folded into it by ID, where
no exclusion check runs. A pin no longer completes a record the operator
split from the pinned resource.
The broadcast coalesced host views without exclusions while REST's
ListForPresentation honoured them, so a split host pair was one websocket
row and two REST rows. The broadcast now coalesces through the read state
that listed the rows, with the same exclusion filter.
|
||
|
rcourtman
|
1a094646d7 |
Let only POST narrate single-resource reports with Pulse Assistant (#2716)
## What
Single-resource report generation (`/api/admin/reports/generate`) now
takes `POST` with a JSON body as its canonical transport, and only
`POST` may narrate a PDF with Pulse Assistant.
- **POST**: the same fields as before (`resourceType`, `resourceId`,
`format`, `metricType`, `start`, `end`, `title`) in a JSON body, decoded
with the `generate-multi` rules (1MB cap, unknown fields and trailing
payload rejected). When Pulse Assistant is configured, a PDF's executive
summary is narrated by the AI provider as before.
- **GET**: still accepts the same fields as query parameters and returns
the same report, but always uses the deterministic summary. The PDF
shows a short note in place of the "Configure Pulse Assistant" tip,
which would be wrong for a tenant that already has the Assistant
configured.
- **GET never resolves the AI service at all.** It reads Patrol findings
only from a tenant AI service that is already running, through a new
`AISettingsHandler.ExistingAIService` lookup. Constructing a tenant
service through `GetAIService` can list provider models or start
background discovery, so a report GET must not trigger it. The router's
reporting wiring moves into `Router.wireReportingAIResolvers` so a test
can drive it.
- **Other methods**: `405` with `Allow: GET, POST`.
- The shared report body decoder now also returns `body_too_large`,
rather than `invalid_body`, when the cap is hit while reading past a
valid value.
- The reporting catalog advertises
`performanceReport.singleResourceMethod: "POST"`, and the settings UI
follows it. A catalog without the field, and the legacy fallback
catalog, keep the GET query transport that older backends accept. Fleet
reports were already `POST`.
- `docs/API.md` and `docs/MSP.md` (and their shipped copies) document
both methods.
## Why
Before this change, a GET for a PDF resolved the tenant's AI narrator.
That made a paid provider call and recorded a usage event in the AI cost
ledger. GET passes the demo-mode guard and the CSRF check, and a
SameSite=Lax session cookie is sent on cross-site top-level GET
navigations. So following a link could spend a signed-in admin's AI
budget.
GET stays available rather than returning `405` because the API
reference and the MSP guide documented it for scripted exports. Breaking
those scripts would add no protection: CSV is never narrated, and GET no
longer reaches the provider.
## Verification
- `TestContract_SingleReportAINarrationRequiresPOST` drives a real
report engine with a counting narrator. GET, HEAD, PUT and DELETE make
zero narrator calls and POST makes one. The GET narrative is heuristic
and carries the note. As a negative control, the same test fails against
the handler on `main` via `go test -overlay` ("GET report called the AI
narrator 1 times, want 0").
- `TestContract_SingleReportGETDoesNotConstructTenantAIService` drives
the router's real resolvers with an uncached tenant: GET constructs no
tenant AI service, while POST resolves one. A negative control that
swaps `ExistingAIService` back to `GetAIService` fails it ("GET report
constructed 1 tenant AI services, want 0").
`TestAISettingsHandler_ExistingAIServiceNeverConstructs` covers the
lookup itself.
- Handler tests cover the POST body rules (empty, unknown field,
trailing value, form-encoded body, fields only in the query, oversized
padding) and the `Allow` header. The catalog contract snapshot includes
the new field.
- Frontend tests cover POST bodies, the GET transport for catalogs that
name GET, parser defaults and validation, and the legacy fallback.
`docsLinks.test.ts` ties the docs to the handler.
- Browser pass on a private stack: real login, dev Pro licence, and
Pulse Assistant pointed at a local stub OpenAI-compatible provider that
logs every call. Checked at 1280×800 and 390×844:
- Generating a single-resource PDF from Settings → Data & Reports sends
one `POST` with a JSON body, downloads the PDF and calls the provider
once. The downloaded PDF carries the AI narrative.
- CSV makes no provider call. The fleet report still posts to
`generate-multi`.
- A top-level browser navigation to the GET URL with the session cookie
downloads the deterministic PDF, with the new note, and makes no
provider call.
- HEAD, PUT, DELETE and PATCH return `405`, and a session POST without
the CSRF header returns `403`.
- The AI usage summary lists `report_narrative` only for the UI POSTs.
- Full `./internal/api/...` and `./pkg/reporting` run on the dev VM:
everything passed apart from two tests that fail there for environment
reasons in code this change does not touch
(`TestServerInfoEndpointReportsDevelopment` needs `.git`; a `configapi`
shell-hangup bootstrap test).
The same class of issue on the Assistant session routes
(`/api/ai/sessions/{id}/summarize` and friends) is handled separately in
#2670.
|
||
|
rcourtman
|
fd36869223 |
Judge source merges by the ingest's stale thresholds (#2717)
Stacked on #2715 (its commit is the parent of this one); either can land first. ## Problem `mergeInto` passed nil thresholds to `mergeMetrics`, so every source merge into an existing row (snapshot and record ingest alike) judged metric freshness by the registry defaults, while the stale pass and manual links used the monitor's configured thresholds. Two visible results: - A Proxmox node polled every two minutes (four-minute threshold) merged with its auto-linked agent showed the silent agent's CPU and memory, while the stale pass called the node's poll current. - A supplemental refresh (`MonitorAdapter.PopulateSupplementalRecords`, and the read-state overlay) of a linked vSphere VM kept the in-guest agent's memory over vSphere's current reading, where a full rebuild from the same records shows vSphere's. ## Change - Snapshot and record ingest hold the caller's thresholds on the registry while they run (the same per-pass pattern as the guest loop's agent index), and `mergeInto` passes them to `mergeMetrics`. The snapshot helpers that reach `mergeInto` keep their signatures. - The stale pass and the metric merge share one threshold rule, `sourceStaleThreshold`, so the merge also honours a sighting's expected update interval (slow Proxmox disk inventory), as the stale pass already did. - Outside mock mode the monitor configures only Proxmox, PBS and PMG, so those are the production sources this changes; mock mode also configures TrueNAS, vSphere and availability. ## Proof - `TestMonitorAdapterSourceMergesUseConfiguredStaleThresholds`: rebuild, supplemental refresh and overlay of a linked vSphere VM, plus a node with an auto-linked agent, through `NewMonitorAdapterWithStaleThresholds`. - `TestSourceMergesJudgeFreshnessByTheIngestStaleThresholds`: snapshot node+agent merge with and without thresholds. - `TestMetricMergeFreshnessMatchesTheStalePass`: the merge and the stale pass agree for default, configured and expected-interval sightings. - Negative control with #2715's `registry.go`: the supplemental, overlay, node+agent, snapshot and expected-interval cases fail; the rebuild case and the defaults case pass on both. - `go test ./internal/unifiedresources/... ./internal/monitoring/... ./internal/api/resourceapi/... ./internal/mock/...` on pulse-dev. |
||
|
rcourtman
|
796a70999b |
Stop tenant incident stores racing on shared ID counters (#2711)
Each tenant monitor owns its own `IncidentStore`, but incident and incident-event IDs came from package-level `int64` counters bumped with `++` under nothing but the calling store's mutex. Two tenants recording alerts at once raced on them. `go test -race` failed `TestLeavingMockModeClearsEveryRunningTenantMonitor` on #2698 ([run](https://github.com/rcourtman/Pulse/actions/runs/37652317098/job/112919080928)) at `incidents.go:1753`. The remediation record counter in `remediation.go` had the same shape, and `startPatrolForContext` creates one remediation log per org. **Fix** - All six `internal/ai/memory` ID generators (incident, incident event, remediation record, memory, incident memory, pattern memory) now mint through one atomic helper, `nextSequencedID` in `ids.go`. - The numeric suffix no longer wraps at 1000. Even with atomic counters, a generator that minted more than a thousand IDs in one wall-clock second repeated an ID. That matters because `RecordNote` finds an incident by ID and the incident query deduplicates merged events by ID. The `<prefix><YYYYMMDDhhmmss>-<n>` layout is unchanged. Only `n` can now exceed three digits. - `intToString` lost its last runtime callers and is removed. **Proof** - `internal/ai/memory/ids_test.go` mints 2,400 IDs from every generator on 16 goroutines and records alerts into two tenant stores concurrently. - Against the `origin/main` runtime files (via `-overlay`), `-race` reports DATA RACE at `incidents.go:1753/1760` and `remediation.go:355`, and every generator fails the uniqueness/shape check: four produce duplicate IDs, and `mem`/`pat-mem` produce a wrapped `-0` suffix. - With the fix, `go test -race ./internal/ai/memory/` passes, and `go test -race -count=5 -run TestLeavingMockModeClearsEveryRunningTenantMonitor ./internal/monitoring/` passes. **Residual:** across restarts, only the timestamp separates IDs. If the local clock repeats a second (DST fall-back or a backward step), a restarted sequence could still meet an earlier suffix. Persisted IDs and the stored format are unchanged. The `ai-runtime` contract records this. |
||
|
rcourtman
|
8237cb997a |
Resolve references to resources a manual link folded
When an operator link folds one resource into another, such as an agent into the vSphere or Proxmox VM it runs in, the folded resource's canonical ID stopped resolving. An availability check configured against the agent lost its target: the VM showed no check and the endpoint row kept no relationship. The resources API seeds from the monitor's already-folded listing and then replays the checks, so it lost the check too, and so does the monitor broadcast now that record ingest joins links. Reference reads by the folded ID (alert intent, operator state, API lookups) found nothing either. The merged resource now carries the IDs links folded into it, including IDs folded along a chain, in an in-memory field that rides clones of the listing. Resource seeding indexes them, so the monitor rebuild, the resources API and read-state overlays resolve a folded ID to the same merged row. The index keeps the holding rows and checks them on read: a holder that a later link merges away or that is re-keyed in place stays consistent, and an ID two rows hold resolves to nothing instead of to a weaker alias match. History does not follow folds: history bindings persist and join two journals in both directions, so they would outlive the link. Folded IDs never become superseded IDs either, because alert and availability migrations would rewrite those into the operator's stored config. |
||
|
rcourtman
|
cfe3a4c398 | Merge origin/main into refactor/drop-summary-chart-routes | ||
|
rcourtman
|
4a0a05858d |
Remove the summary chart routes no client calls
GET /api/charts/workloads-summary has had no first-party caller since the v6 import: ChartsAPI.getWorkloadsSummaryCharts arrived with it and nothing ever called it. GET /api/charts/storage-summary lost its last reader when storageSummaryTrendCache.ts was deleted on 2026-05-26. Both still cost something. The workloads route carried a payload cache, an SLO test in the release latency gate and a benchmark, and mock mode rebuilt the 24-hour storage aggregate on every sampler tick for a cache nothing read. No stable release ever requested either route from the browser: v6.0.0 and later carry no caller, and the storage trend reader shipped only in v6.0.0-rc.1 to rc.5. Neither route is in docs/API.md (a list of common endpoints, not a complete one), and pulse-pro, pulse-mobile and pulse-dev-infra never call them. A monitoring:read token could reach them, so a private script may notice, but nothing known depends on them. pulse-enterprise names the SLO test only as fixture data, and its latency gate passes a test that is absent from the candidate as removed. So the routes go with their clients: handlers, response types, the monitor trend helpers, the mock aggregate and its prewarm, the SLO budget, the benchmark, and the ChartsAPI methods and types. Requests to either path now get 404. buildMockWorkloadMetricHistorySeries goes too, because the workloads-summary handler was its only production caller. A TypeScript reference graph from src/index.tsx finds no production reference to the removed methods or types. The production build loses only the two static methods from the ChartsAPI chunk (326 B raw, 49 B gzip); every other file differs only by chunk hashes and SRI values. deadcode ./cmd/pulse and golangci unused report nothing newly unreachable. On private mock stacks the Proxmox overview in Trends, a guest History drawer and the storage page make the same chart requests before and after at 1440 and 390 px, and neither build requested the removed routes. Contract prose that still described either route as live now records the retirement, and the workload chart guard pins the per-type metric sets the surviving /api/charts/workloads route requests. |
||
|
rcourtman
|
89a05ed735 |
Pin each manual-link side from its own identity
A manual link merges two resources for display, and the durable identity pin was written from that merged identity. A link primary was pinned with the folded resource's machine key, and the store's one-owner-per-key rule then deleted the folded resource's own pin. A Proxmox node's pin records its qualified endpoint as the hostname, so on the next rebuild the node completed the linked agent's machine key from that pin, minted the agent's machine-derived ID and merged with the agent by ID before the link applied. With the node as primary, succession then rewrote the link into a self-link on the agent's ID; in either direction, removing the link left the pair merged. The registry now captures each link side's own pin when the link first joins it in a rebuild, and PersistIdentityPins writes those instead of the merged projection, keeping a pinnable folded side under its own canonical ID. Proxmox auto-links merge by identity before any manual link applies, so their boot-window pins are unchanged. A link side rewrites its whole row through a new ReplaceResourceIdentityPins. Like an upsert it keeps stored fields its own pin leaves empty, but it drops a strong key another resource holds, since an earlier release borrowed that key from the merged identity and the store's one-owner rule would otherwise delete one row or the other. A folded side drops a strong key a live resource or an earlier folded side already holds, so the two never trade it on every rebuild, and writes no pin when that would leave it without its machine evidence. A node row an earlier release filled with the agent's machine key is handed back only when the node cannot complete from it. With a qualified endpoint or a cluster name, completion still supplies the borrowed key before the link applies, so such a pair stays merged. |
||
|
rcourtman
|
c5416b6800 |
Alert on the Unraid temperature an agent disk shows (#2701)
Stacked on #2698 (which sits on #2694, #2685 and #2679); this PR's own change is the top commit, "Alert on the Unraid temperature an agent disk shows", plus a merge of main for an alerts.md anchor both touched. ## Why A host agent's physical disk can show a temperature that only the host's Unraid inventory reports: - a SMART row whose smartctl probe returned no temperature takes its Unraid row's reading (`HostSMARTDiskTemperature`), and - a disk only the inventory lists, such as a `--disk-exclude` match, is built from its Unraid row alone (`HostUnraidDiskTemperature`). Physical Disks (Temp cell and the `Running Hot` verdict, "at or above its alert threshold") and Patrol judge that reading against the alert disk temperature policy, but `CheckHost` only read the SMART row's own field. A disk could read as at its alert threshold while no alert fired. Unraid's own warning/critical disk temperature notifications do not make this redundant: Pulse already alerts on the smartctl readings and array state of the same hosts, which Unraid also notifies on. ## What changes - `unifiedresources.HostDiskTemperatureReadings(host)` lists the reading each of a host's disks shows, built with the registry's own adapters: one per SMART row the registry ingests, then one per Unraid row whose disk key no SMART row carries (the same rule as the history writer in #2698). - `CheckHost` judges those readings: - SMART rows keep their `agent:<host>/disk_temp:<device>` resource IDs. An inventory-only disk uses its block device token, which is the ID a plain Linux SMART row for that device uses, so a later SMART reading continues the same alert. - Per-type thresholds and host overrides use the disk type the disk shows. - Only a reading collected now fires or resolves, so a spun-down disk holds its alert. - When conflicting serials put two disks on one device, its alert judges the reading that stands highest against its own type's threshold. - An inventory-only disk matching the agent's `--disk-exclude` raises nothing, and an alert it raised before the exclusion resolves on the next report. This matches the documented exclusion from monitoring and how linked Proxmox hosts already treat excluded disks. - Absence cleanup still runs only on a non-empty SMART list. ## Verification - New alert tests in `internal/alerts/host_unraid_lifecycle_test.go` cover: the fallback, an inventory-only disk, an empty SMART list, two disks on one device (also of different types), a controller member under another device label, standby, exclusion, disk-type and host-override thresholds, holds, handover to a SMART reading, departure, and the exclusion clear. - Each fails on the previous `host.go`. - Removing any guard (exclusion skip, exclusion clear, disk type, collected-now, threshold ranking) fails its own test. - `TestHostDiskTemperatureReadingsMatchTheDisksTheRegistryShows` compares every reading with the disk the live registry ingests, including conflicting serials, a virtual device and a MegaRAID member Unraid lists under its block device. - `go test ./internal/alerts/... ./internal/unifiedresources/...` passes on the merged tree, as do the Unraid/SMART/temperature tests in `internal/monitoring`. |
||
|
rcourtman
|
b3b60f0632 |
Bind SAML logins to the browser that started them
With allowIdpInitiated off, the default, every SAML login failed: ProcessResponse passed crewjam/saml no request IDs, and crewjam refuses a Response whose InResponseTo is not among them. Only providers that allowed IdP-initiated logins, which skips the check, could sign in. MakeAuthRequest now records each AuthnRequest ID with the login's returnTo and the hash of a random token that /login sets as the HttpOnly SameSite=Lax cookie pulse_saml_login (__Host- over HTTPS), reused across tabs. The IdP posts its Response cross-site, which withholds Lax cookies, so the ACS answers a cookieless POST with a page that re-posts the same fields once from Pulse's origin. The Response must answer an unexpired request bound to the presenting browser; it spends that request and returns to the recorded path. Another browser's request is left for its owner. The request ID is read from the signature covering the assertion crewjam returns (the signed Response, else that assertion's own SubjectConfirmationData), not from crewjam's parsed fields and not from other signed assertions riding in the same unsigned Response. encoding/xml fills an un-namespaced attribute field from a namespace declaration with the same local name, and exclusive canonicalization drops an unused one from the signed form, so xmlns:InResponseTo could rebind a signed Response and xmlns:IssueInstant / xmlns:NotOnOrAfter let a stale one pass crewjam's age checks: an IdP-initiated login could be replayed indefinitely. The signature verifier now refuses signed content declaring a prefix named like an attribute crewjam reads, in both modes. RelayState is now escaped into the redirect query, so a returnTo with its own query string reaches the IdP whole. |
||
|
rcourtman
|
e7cabc72f1 |
Hide pprof routes on demo-mode instances (#2708)
The `/debug/pprof` handlers are gated on `RequireAdmin` plus a `settings:read` scope that session logins skip. A `DEMO_MODE` instance that signs visitors in as the configured admin therefore served every visitor goroutine and heap dumps, the process command line, and CPU profiles and execution traces that load the whole process. Demo mode already hides diagnostics, logs, admin users and manual discovery through the admin-operations policy; pprof was missing from it. ## Change - `internal/api/demo_mode_operations.go`: through the admin-operations policy, the demo guard answers `/debug/pprof` and every path below it with `404` for every method, ahead of route authentication, so a write probe such as the `POST` symbol lookup gets the same `404` instead of the generic demo `403`. Instances outside demo mode keep the routes, and `PULSE_PPROF_DISABLED` still removes them everywhere. - Contract notes in api-contracts (owner), agent-lifecycle and storage-recovery. ## Proof - `TestContract_DemoModeHidesPprofFromDemoAdminSession` signs in as the demo admin through the full router. The same session gets `200` from `/debug/pprof/cmdline` below the demo guard, and `404` for eight pprof paths × `GET`/`HEAD`/`OPTIONS`/`POST` through the served handler. Against the previous policy (`go test -overlay`) `GET` and `HEAD` returned pprof output (`200`, or the root's `301` into the index), `OPTIONS` got the router's preflight `200` and `POST` the generic demo `403`. - `TestPublicDemoAdminOperationsPolicyHidesEveryDebugRoute` parses the router files and fails if a literal route under `/debug/`, with or without a method prefix, escapes the policy for any of seven methods, so a new debug handler there cannot ship reachable on a demo by default. A temporary `GET /debug/vars` registration made it fail as expected. - Path-handling probe (not committed): 210 raw requests (35 request targets × six methods), covering trailing and double slashes, dot segments, `%2F`/`%2e` encodings, absolute-form targets and `CONNECT`, run against a copy of the router's mux dispatch. None reached a pprof handler; paths the mux would clean onto pprof only receive a redirect, and the policy catches the follow-up request. |
||
|
rcourtman
|
6263c580f7 |
Judge a linked agent's guest disks by that agent everywhere (#2712)
Follow-up to #2669, which made the guest drawer's filesystem rows follow a linked Pulse agent's freshness. The workload table's disk cell and the drawer History still judged the same guest by the Proxmox read reason, so one guest disagreed with itself across the three. ## What changes - **One rule for three readers.** `getWorkloadGuestDiskRead` (`workloadGuestPresentation.ts`) now drives the table disk cell, the drawer's filesystem rows and History. When the linked agent's filesystems fill in for a VM that has no Proxmox guest filesystems, the reading is current while the agent reports, last known once it is stale, and unavailable once the guest is stopped. Proxmox's own guest filesystems keep their read reason. - **Where the aggregate disk metric comes from.** On a manually linked VM, Proxmox records no guest disk usage without guest filesystems, so the registry's link merge keeps the agent's disk metric. A registry test pins that. - **Poller carry-forward.** `previousVMFromView` built the previous VM from the merged metric, so the next poll carried the agent's disk forward as a Proxmox `prev-agent-not-running` read. Proxmox priority then froze it over the agent's live value. Only Proxmox's own disk reading is carried now (`VMView.DiskFromLinkedAgent`). - **`agent.stale` is always on the wire.** The browser merges agent facets field by field, so an omitted false kept a resumed agent "stopped reporting" until a reload. This affected Machines rows and node drawers too. - **Withdrawn filesystems.** The websocket merge kept filesystems that a native Proxmox VM facet or a native agent facet (which always carries `stale`) no longer reported, including in the `platformData.agent` mirror that host threshold rows read. ## Verification - New tests, each failing on main: `GuestRow.test.tsx`, `GuestDrawer.test.tsx`, `workloadGuestPresentation.test.ts`, `resourceStateAdapters.test.ts`, `TestPreviousVMFromViewKeepsLinkedAgentDiskOutOfProxmoxCarry`, `TestAgentDataAlwaysSendsStale`, `TestVMViewDiskFromLinkedAgentFollowsSelectedMetricSource`, and `TestResourceRegistry_ManualGuestAgentLinkDiskMetricFollowsProxmoxGuestFilesystems`. The registry test is a characterization test; it fails when the agent outranks Proxmox on guest disk. - Private mock stack at 1440 and 390, with agents linked into VMs through `POST /api/resources/{vm}/link`: - Live agent, no QEMU guest agent: before, "- Unavailable · Guest agent not running", with History deferred; after, the agent's bars, nothing deferred. - Silent agent: before, "- Unavailable"; after, bars plus "Last known" (phone: "Prior"), with History deferred on the agent message. - Stopped guest: Unavailable. - A guest with Proxmox filesystems: unchanged. - Machines row for a standalone agent released from a silent hold, watched without a reload: on main it kept "(stale)"; now it clears. - Bundle: WorkloadsSurface 61.93 kB gzip against a 70.23 kB cap. ## Known residuals - Mock mode's websocket state does not apply manual links to resources ingested as records (`ingestRecords` never runs `applyManualLinks`), so the browser pass ran the Proxmox page on REST data. In production this likely affects links to vSphere or TrueNAS resources; it is tracked separately. - A stale Proxmox source can lose the aggregate to a live agent while its own filesystem rows and `prev-` reason remain. That guest stays on the Proxmox rule and reads its live aggregate as last known, which is the same as main and errs toward caution. A guest with no platform read reason can likewise keep a retained aggregate after the agent withdraws its filesystems, also as on main. Both need aggregate provenance on the wire. |
||
|
rcourtman
|
23bf99e1ba |
Keep alert settings a config save leaves out (#2689)
## Problem
Saving anything on Alerts > Thresholds turned flapping detection and
alert TTL cleanup off. The page's `PUT /api/alerts/config` body never
carries `flappingEnabled`, `maxAlertAgeDays`, `maxAcknowledgedAgeDays`,
`autoAcknowledgeAfterHours` or `customRules` (the page has no control
for any of them). `UpdateAlertConfig` decoded the body into a zero
`AlertConfig`, so every save wrote `false`/`0` for those fields, reset
the flapping tuning to defaults, dropped custom rules, and persisted
that. The `flapping`, `aggregation` and `ioNormalization` objects the
page sends are not `AlertConfig` fields and never enabled anything.
## Fix
- `PUT /api/alerts/config` now replaces only the top-level keys the body
carries. A key left out keeps its stored value. A sent key replaces its
setting whole, so `"overrides": {}` still clears overrides, and the
usual normalization still runs. A body that is not a JSON object
(including `null`) is a 400 and changes nothing.
- `alertconfig.ApplyAlertConfigUpdate` decodes the body on its own, so
sent keys get exact `encoding/json` semantics, including duplicates and
case-folded keys. It copies every unsent top-level field from a JSON
clone of the stored config.
- `Manager.ApplyConfigUpdate` reads, merges and applies under the
manager lock, so concurrent partial saves cannot revert each other. It
returns an owned snapshot, which the handler persists, so
`SaveAlertConfig`'s in-place normalization no longer writes into the
live config's maps. The handler runs one save at a time, so successful
saves reach `alerts.json` in the order they were applied.
- Stored configs are not rewritten; a value an earlier save zeroed stays
as it is.
## Verification
- Unit: `internal/alerts/config/update_test.go`; manager save path and
concurrent partial updates in
`internal/alerts/config_validation_test.go`; handler against a real
manager in `internal/api/alerting/alerts_test.go`. Each new regression
fails against main's code (overlay negative controls). The concurrency
test fails at round 0 against a non-atomic variant.
- `go test ./internal/alerts/... ./internal/api/alerting/` and the
`internal/api` alert endpoint, tenant and scope tests pass; the alert
tests pass under `-race`.
- Private stack (mock mode, Vite on main's frontend):
- On main, a node CPU change saved from the Thresholds page took
flapping and all three TTLs to false/0, in memory and in `alerts.json`.
- With this change, the same UI save kept them (true/7/1/24) and stored
the new threshold.
- The API repro (GET, drop the four keys, PUT) keeps them; explicit
`false`/`0` still applies; `null` and `[]` return 400.
- Five rounds of concurrent partial PUTs kept both changes in memory and
on disk.
## Follow-ups
- The thresholds page still sends the dead
`flapping`/`aggregation`/`ioNormalization` objects and hard-coded
`minimumDelta`/`suppressionWindow`/`hysteresisMargin`. A frontend
cleanup will follow once the open PRs touching
`alertsConfigurationModel.ts` land.
- `SaveAlertConfig` still normalizes caller-owned maps in place for
other callers (`ActivateAlerts`, startup paths), and the GET handler
encodes a shallow copy. That pre-existing race is tracked separately.
|
||
|
rcourtman
|
36d3f8c557 |
Judge pulse-relaxed guests by one threshold on poll and save (#2705)
Stacked on #2700 (which carries #2696 and #2692). Merge order is enforced by the stack: this change must not reach main before #2700's per-filesystem reevaluation (see "Why stacked" below). ## Problem `pulse-relaxed` lifts a guest's CPU, memory and disk triggers to a 95/92/95 floor. Only `CheckGuest` applied it. Config-save reevaluation rebuilt the guest from alert metadata without its tags (`guestSnapshotFromAlert`), so a save judged a relaxed guest's alert by its unrelaxed thresholds: - A relaxed guest with an unset memory threshold alerts at 92 while polling. On save that reads as no threshold, so the save resolved the alert and sent a recovery notification, and the next poll raised it again. - The floor also lifted an Off threshold (trigger 0, or -1 from the editors) like any low trigger, so tagging a guest relaxed turned an Off alert on, with no recovery band (`{95, 0}`). A save then re-judged it Off and resolved it, and the next poll raised it again. ## Change - The relaxed step moves into `effectiveAlertPolicyNoLock`'s guest branch, so `CheckGuest`, config-save reevaluation (including #2700's `resolveGuestAlertThresholdsNoLock`) and posture checks share one resolver. - `guestSnapshotFromAlert` reads the recorded `tags` back, as `[]string` live or `[]interface{}` after a restart, through a new `metadataStringsValue` reader. `storagePolicyAliases` now uses the same reader. - An Off threshold (trigger <= 0) stays Off under `pulse-relaxed`; an unset one still gets the floor, as the existing tests pin. The tag is documented (issue #863) as relaxing thresholds, so it must never make a guest more sensitive than its config. - `metricOptionsWithTags` drops the `tags` key from an open alert once its resource has no tags. Reevaluation now reads that key, so a removed `pulse-relaxed` left behind would judge the alert by the relaxed trigger while polling uses the guest's own, and the alert would flap. Notification tag routing reads the same key, so it also stops routing on removed tags. ## Why stacked A filesystem's own override replaces the relaxed guest disk threshold in `CheckGuest`. On main, reevaluation misses filesystem overrides (fixed by #2700). With this change alone, a save would judge a relaxed guest's filesystem alert by the 95 floor instead of, say, an 85 override, and flap it. `TestConfigSaveKeepsRelaxedGuestFilesystemOverride` fails on main plus this change and passes on this stack. ## Proof New tests in `internal/alerts/alerts_test.go` and `internal/alerts/unified_eval_test.go`. With `go test -overlay` mapping the six runtime files back to #2700's head, these fail: - `TestConfigSaveJudgesRelaxedGuestAlertByRelaxedThresholds` (the save resolved the alert) - `TestPulseRelaxedKeepsOffGuestThresholdOff` (a disk alert was raised with disk Off) - `TestApplyRelaxedGuestThresholds/off_thresholds_stay_off` (Off became `{95, 0}` / `{92, 0}`) - `TestCheckGuestDropsRemovedTagsFromOpenAlerts` (stale tags; with only the metadata assertion relaxed, the save resolves the alert) `go test ./internal/alerts/... ./internal/notifications/` and the monitoring alert tests pass. |
||
|
rcourtman
|
b7049ff778 |
Raise Proxmox disk temperature alerts under the per-type policy (#2680)
**Stacked on #2675** (TrueNAS disk temperature alerts by disk type). Merge that first; this branch contains its commit. ## Problem A Proxmox physical disk that only the node's own sensors report never raised a temperature alert: - `CheckDiskHealth` judges health and wearout only; - unified evaluation types TrueNAS physical disks, not PVE ones. So Physical Disks could read a disk **Running Hot** under the per-type thresholds while alerts stayed silent. Whether a disk alerted depended on whether a Pulse agent ran on the node: agent-covered disks alert through `CheckHost` at the same thresholds. #1347 shows users look for disk temperature thresholds in the Proxmox tabs. ## Change - **`CheckProxmoxDiskTemperature`** (`internal/alerts/proxmox_disk_temperature.go`) raises a `diskTemperature` metric alert under the disk's existing PVE alert reference. It is judged by the disk temperature policy: the disk type's `DiskTempByType` entry under the agent Disk Temp default, which switches it off for every type. The alert carries the disk's serial, WWN and `diskType`, like the health and wearout alerts. - **One alert per disk.** `mergeHostAgentSMARTIntoDisks` marks a disk that a still-reporting linked agent lists in its SMART report (`AgentSMARTReported`, internal, never serialized). That disk is the agent's: `CheckHost` alerts on it, and an open PVE alert closes with `moved_to_agent`. A lapsed agent owns nothing, so the node's own current readings keep alerting while its agent is silent. - **Holds and clears.** Only a reading collected this poll is judged (`diskinventory.TemperatureCollected`), so a retained value holds the alert. An excluded device, or a switched-off policy, closes it. - **Stability.** A new `proxmox-disk` alert delay defaults to 5s, so warnings wait the same 300s noisy-gauge window as agent and TrueNAS disk temperatures. Critical readings bypass it. - **Config save.** `reevaluateActiveAlertsLocked` re-judges the alert per disk type. Without this it would fall through to guest thresholds, which have no disk temperature, and resolve on every save. An alert without a recorded type uses the lowest per-type trigger. - **History.** `ProxmoxPhysicalDiskAlertIdentifiers` lists the new `<ref>::metric-threshold:diskTemperature` identifier, so resource history owns its rows by recorded hardware identity, as #2646 does for health and wearout. - **Copy.** The Agents "Disk temperature by type" card says it covers agent, TrueNAS and Proxmox disks. ## Evidence Private mock stack (8 PVE nodes) with a local-only overlay pinning PVE disk readings: | Disk | Physical Disks | Alert | |---|---|---| | pve1 NVMe 63°C | Healthy | none | | pve2 SATA 56°C | Running Hot | warning, trigger 55, after the 300s window | | pve3 NVMe 72°C | Running Hot | warning, trigger 70, after the 300s window | | pve3 SAS 63°C | Healthy | none | | pve3 SATA 66°C | Running Hot | critical at once | The alert card reads: "Samsung 870 EVO 1TB (/dev/nvme2) · Temperature · Temperature 66°C, above the 55°C alert level · Clears once it drops to 50°C or lower and stays there for 5 minutes." ## Tests - `internal/alerts/alerts_test.go`: - the policy (per type, raised trigger); - ownership with `moved_to_agent`, the hold without a reading, exclusion, the off switch; - the stability window; - re-judging on save, including the lowest-trigger hold; - history identifiers. - `internal/monitoring/physical_disk_roundtrip_test.go`: the marker for reporting agents only (not a lapsed one), the SMART fallback path, and that `checkPhysicalDiskAlerts` passes collected readings, ownership and exclusion. - `internal/unifiedresources/store_test.go`: a temperature alert row is owned by its disk and still found by a read by its reference. - `internal/models/deepcopy_test.go`: the marker survives cloning and stays out of JSON. - `ThresholdsTable.test.tsx`: the card copy. - Ten wiring points were mutation-checked (marker, lease condition, collected reading, ownership, exclusion, re-judge branch, lowest trigger, resolution, history identifier); each removal fails a test. ## Second opinion - Fixed: an offline agent's retained SMART rows kept ownership, so neither side alerted while the agent was silent. Ownership now needs a reporting lease. - Kept, documented in `alerts.md`: on the first poll after an agent is newly linked to a disk with an open PVE alert, both alerts can be open until the next disk poll. This is the same window as the node-to-agent usage alert handover. ## Not in this PR - Like the health and wearout alerts, a disk that leaves the inventory keeps its temperature alert until the 24-hour stale sweep. - Per-host Disk Temp overrides do not reach PVE-only disks, which have no agent host. |
||
|
rcourtman
|
3fb267333d |
Require POST for Assistant session mutation routes (#2670)
The session router dispatches `/api/ai/sessions/{id}/abort`,
`summarize`, `fork`, `undo` and `redo` by path alone, and none of those
handlers checked the HTTP method, so a GET or HEAD cancelled a running
turn, compacted or rewound a transcript, or persisted a fork.
- **Demo mode:** `DemoModeMiddleware` admits every GET/HEAD as a read,
so a public-demo session could mutate Assistant sessions despite the
read-only guard.
- **CSRF:** `CheckCSRF` skips GET/HEAD, and session cookies default to
SameSite=Lax, which browsers send on cross-site top-level GET
navigations. A link carrying a session ID could therefore trigger these
mutations for a signed-in user.
## Change
- `requireAssistantSessionMutationMethod` in
`internal/api/ai_handler.go`: abort, summarize, fork, undo, redo and
steer admit only POST and answer anything else with `405` + `Allow:
POST` before resolving the Assistant service. Steer already refused
non-POST but sent no `Allow` header; it now uses the same guard.
- The guard sits in the handlers, matching how chat, steer and the
approval approve/deny handlers already own their method contracts.
- `messages` (GET) and the unsupported `diff`/`revert`/`unrevert` routes
are unchanged. Session rename/delete and the collection route already
dispatch on the method.
- Callers: `frontend-modern/src/api/aiChat.ts` uses POST for all six;
pulse-mobile's abort call and its OTA compatibility probe use POST.
## Proof
- `TestAssistantSessionMutationsRequirePost`: GET/HEAD/PUT/DELETE get
405 + `Allow: POST` for every mutation, with no mutation call and no
`IsRunning` check reaching the service. POST still dispatches.
- `TestAssistantSessionReadSubresourcesKeepTheirMethods`: GET `messages`
returns 200 and GET `diff` returns 501, with no mutation either way.
- `TestDemoModeRouterRejectsAssistantSessionMutationsOverSafeMethods`:
drives `NewRouter(...).Handler()` with `DemoMode` on. GET/HEAD to every
mutation pass the demo guard and get 405, POST stays 403, nothing
mutates, and GET `messages` still reads.
- Negative controls via `go test -overlay`: on main's `ai_handler.go`,
every non-POST case fails and the demo test reports `GET abort status =
200`. With abort's guard moved after the `IsRunning` check, the ordering
assertion fails.
|
||
|
rcourtman
|
3a482c610c |
Judge source merges by the ingest's stale thresholds
mergeInto always passed nil thresholds to mergeMetrics, so every source merge into an existing row judged metric freshness by the registry defaults while the stale pass, the manual links and the monitor's configuration used the caller's thresholds. A Proxmox node polled every two minutes (four-minute threshold) merged with its auto-linked agent showed the silent agent's readings while the stale pass called the node's poll current, and a supplemental refresh of a linked vSphere VM kept the in-guest agent's memory over vSphere's current reading. Snapshot and record ingest now hold the caller's thresholds on the registry while they run, the way the guest loop holds its agent index, and mergeInto reads them. The ~50 snapshot helpers that reach mergeInto through ingest() keep their signatures. The stale pass and the metric merge now share one threshold rule, sourceStaleThreshold, so the merge also honours a sighting's expected update interval (slow Proxmox disk inventory) as the stale pass already did, and a source without a default falls back to 120s in both (the merge used 60s). Every defined source has a default, so that fallback reaches none. Outside mock mode the monitor configures only Proxmox, PBS and PMG, so those are the production sources this changes; mock mode also configures TrueNAS, vSphere and availability. |
||
|
rcourtman
|
1ac600ec2b |
Judge filesystem alerts by their own threshold on config save (#2700)
Stacked on #2696, which is stacked on #2692. The unique commit is the
last one,
|
||
|
rcourtman
|
9bd1b6e582 |
Let only POST narrate single-resource reports with Pulse Assistant
GET /api/admin/reports/generate resolved the tenant's AI narrator, so a PDF requested with GET made a paid provider call and appended a usage event to the AI cost ledger. GET passes the demo-mode guard and the CSRF check, and a SameSite=Lax session cookie rides a cross-site top-level GET navigation, so a link could spend a signed-in admin's AI budget. POST with a JSON body, decoded with the generate-multi rules, is now the canonical single-resource transport and the only one that may use the AI narrator. The settings UI sends it, following a new performanceReport.singleResourceMethod catalog field; a catalog without the field, and the legacy fallback catalog, keep the GET transport that older backends accept. GET stays available with the same query parameters but narrates deterministically, printing a note in place of the "Configure Pulse Assistant" tip, and other methods get 405 with Allow: GET, POST. GET also stops resolving the AI service at all: it reads Patrol findings through a new ExistingAIService lookup, because GetAIService constructs a tenant service on first use and that can list provider models or start background discovery. The router's reporting wiring moves into wireReportingAIResolvers so a test can drive it. Keeping GET rather than answering 405 avoids breaking documented API scripts (MSP.md pointed providers at it) for no security gain: CSV is never narrated and GET no longer reaches the provider. |
||
|
rcourtman
|
b0590c0ccf |
Join operator links on resources that arrive as records
The monitor rebuilds its registry from the state snapshot and then ingests supplemental records (vSphere, TrueNAS). Snapshot and resource ingest applied the operator's manual links, but record ingest did not, so a link whose side exists only as a record (an agent inside a vSphere or TrueNAS VM, an agent on a TrueNAS host) never joined in the registry behind the websocket broadcast and /api/state. The resources API seeds through resource ingest and showed the same pair merged, and the published inventory also flipped to merged whenever continuity hydration cloned the registry for an unrelated absent machine. Record ingest now applies the links, with the adapter's stale thresholds on the rebuild, live supplemental and overlay paths. Continuity records stay out of it: a saved enrollment only fills an absent machine and must not lend its linked guest an offline verdict. A link merge keeps the TrueNAS and vSphere payloads the primary lacks. Joining links in the rebuild exposed how identity treats a folded resource. Its ID was no longer live, so once an agent-type primary was pinned with the folded agent's machine key, pin succession re-keyed the link onto the primary itself and the next rebuild split the pair. That already broke snapshot links with a Proxmox node as primary. The registry now remembers each folded resource, and pin-driven and record-declared successions treat it as observed. Mock mode still builds its view from fixtures without the link store, so the mock broadcast is unchanged here. |
||
|
rcourtman
|
3cef2c1921 |
Verify manual SAML IdP signatures with the configured certificate
SAML providers configured with an SSO URL and a PEM signing certificate (idpCertificate or idpCertFile) instead of IdP metadata never verified an IdP signature. buildManualMetadata put the certificate into the IdP descriptor's X509Certificate field as a PEM block, but crewjam/saml base64-decodes that field as DER, so every ACS Response and LogoutResponse failed with "cannot parse certificate: illegal base64 data at input byte 0" before any key was compared. Providers configured from IdP metadata were unaffected because metadata carries base64 DER. Store base64 DER, as metadata does. The saved idpCertificate PEM and idpCertFile path are read as before; only the descriptor rebuilt in memory on each load changes, so no saved configuration needs migrating. TestContract_SAMLManualIDPCertificateVerifiesIdPSignatures signs an ACS Response and a LogoutResponse with one test IdP key and checks that inline-certificate, certificate-file and metadata providers all accept them, and that a Response signed by another key is refused. The test allows IdP-initiated responses because ProcessResponse records no AuthnRequest IDs, so at the default every ACS Response is refused on InResponseTo. That is a separate open bug, not fixed here. |
||
|
rcourtman
|
2eec0549a3 |
Accept SAML LogoutResponses only for the logout they answer
/api/saml/{id}/slo cleared the cookie-selected session for any
LogoutResponse the IdP had signed in the last 90 seconds. crewjam/saml
v0.5.1 checks a LogoutResponse's signature, Destination, IssueInstant,
Issuer and Status but never its InResponseTo, and the route must take
GET for the HTTP-Redirect binding, where the SameSite=Lax session cookie
rides a cross-site navigation and the CSRF check does not apply. A user
who logged out through Pulse could replay the IdP's answer to their own
logout through a link and log out anyone who followed it. Delivered as a
cross-site HTTP-POST form instead, the response arrived without the
victim's cookie, yet the handler's deletion cookies still logged their
browser out.
SP-initiated logout now records each LogoutRequest ID on the provider's
service for up to ten minutes, bound to the session-store hash of the
session it ends. The SLO handler spends the record the response's
InResponseTo names and refuses the response unless that record exists
and is unexpired and the browser carries either no session or the
session that logout ended. The genuine return carries no session,
because logout clears the cookie before redirecting to the IdP, so
requiring the cookie-selected session to match would refuse every real
SLO. Refusals answer 403 with a failed saml_slo_callback audit event,
and a misdelivered response still spends its record, so each
LogoutRequest is answered at most once.
The SLO handler no longer writes session state or cookies for an
accepted response: logout already invalidated the session and expired
its cookies, and the callback cannot see which session a cookieless POST
delivery's browser holds. InResponseTo is read only from the root
element's unprefixed attribute, because encoding/xml would also fill it
from a namespace declaration such as xmlns:InResponseTo, which exclusive
canonicalization can leave outside the signature when nothing uses it.
SP-initiated logout itself now answers 401 to a request without a
session cookie. The SAML routes are public and skip CSRF, so a
cookieless cross-site POST fell through to the plain logout and its
deletion cookies, a forced logout that a deployment with authentication
configured refuses on /api/logout as unauthenticated.
|
||
|
rcourtman
|
1231392e8d |
Judge a linked agent's guest disks by that agent everywhere
PR 2669 made the guest drawer's filesystem rows follow a linked Pulse agent's freshness when its filesystems fill in for a VM that has no Proxmox guest filesystems. The workload table's disk cell and the drawer History still judged the same reading only by the Proxmox read reason. So one guest read as current in the drawer and as "Unavailable: Guest agent not running" in the table and History, and a silent agent's reading could not show as last known there. getWorkloadGuestDiskRead now owns one rule for the table cell, the drawer rows and History: current while the agent reports, last known once it is stale, unavailable once the guest is stopped. Proxmox's own guest filesystems keep their read reason. The rule also covers the aggregate disk metric, because Proxmox records no guest disk usage without guest filesystems, so the manual link merge keeps the agent's metric. Three paths broke that: - The poller built its previous VM from the merged metric, so the next poll carried the agent's disk forward as a Proxmox prev- read, and Proxmox priority then froze it over the agent's live value. Only Proxmox's own disk reading is carried now. - agent.stale was omitempty, and the browser merges agent facets field by field, so a resumed agent stayed "stopped reporting" until a reload. It is always sent now. - The same merge kept filesystems a native Proxmox VM facet or a native agent facet had withdrawn, so the rows did not switch source. |
||
|
rcourtman
|
239cd06933 | Merge origin/main into fix/unraid-disk-temperature-alerts | ||
|
rcourtman
|
88d44099aa |
Stop tenant incident stores racing on shared ID counters
Each tenant monitor owns its own IncidentStore, but incident and incident-event IDs came from package-level int64 counters bumped with ++ under nothing but the calling store's mutex. Two tenants recording alerts at once raced on them: go test -race failed TestLeavingMockModeClearsEveryRunningTenantMonitor on PR 2698 at incidents.go:1753. The remediation record counter had the same shape, and remediation logs are created per org. All six internal/ai/memory ID generators now mint through one atomic helper. The numeric suffix also stops wrapping at 1000: even with atomic counters, one generator minting more than a thousand IDs in a wall-clock second repeated an ID, and incident notes find their incident by ID. The prefix and timestamp layout is unchanged, and intToString lost its last callers. ids_test.go mints 2,400 IDs from every generator on 16 goroutines and records alerts into two tenant stores concurrently. Against the old files it reports DATA RACE at incidents.go:1753/1760 and remediation.go:355, and every generator mints duplicate or wrapped IDs. |
||
|
rcourtman
|
0180b33431 |
Hide pprof routes on demo-mode instances
The /debug/pprof handlers are gated on RequireAdmin plus a settings:read scope that session logins skip. The public demo signs every visitor in as the configured admin, so any visitor could fetch goroutine and heap dumps and the process command line, or start CPU profiles and execution traces that load the whole process. DEMO_MODE already hides diagnostics, logs, admin users and manual discovery through the admin-operations policy; pprof was missing from it. The policy now has the demo guard answer /debug/pprof and every path below it with 404 for every method, ahead of route authentication. Instances outside demo mode keep the routes, and PULSE_PPROF_DISABLED still removes them everywhere. TestContract_DemoModeHidesPprofFromDemoAdminSession signs in as the demo admin through the full router, shows the same session still reads /debug/pprof/cmdline below the demo guard, and gets 404 for eight pprof paths through the served handler. On the previous policy GET and HEAD returned pprof output (200, or the root's 301 into the index) and POST got the generic demo 403. A policy test also fails if a literal route the router files register under /debug/, with or without a method prefix, escapes the policy for any common method. |
||
|
rcourtman
|
2ce3605454 |
Refuse GET and HEAD on discovery settings and SAML logout
Two routes registered without a mux method pattern changed state on any
method. The demo-mode read-only guard and the CSRF check both admit GET and
HEAD as reads, and the SameSite=Lax session cookie rides cross-site
top-level GET navigations, so each was writable by a safe method:
- /api/discovery/settings had no method check, so a GET or HEAD carrying a
JSON body changed the discovery staleness threshold, including on a
DEMO_MODE instance.
- SP-initiated SAML logout at /api/saml/{id}/logout cleared the session and
redirected to the IdP on GET. Nothing in the UI calls it, the /api/logout
fallback it delegates to is already POST-only, and the IdP's
LogoutResponse returns to /slo, not here.
Add requireRequestMethod (405 with Allow) as the shared handler-level
guard. Discovery settings keeps PUT, its documented method, and POST, which
it accepted before; SAML logout takes POST only. An audit of every route in
internal/api found no other caller-requested write reachable by GET or HEAD
beyond the Assistant session routes guarded in #2670. GET flows the
protocol requires (OIDC login and callback, SAML login initiation and SLO,
magic-link and cloud handoff, checkout bridges) and reads that run lazy
initialization or idempotent reconciliation are left as they are. One
known exception is deferred: single-resource report generation still runs
on GET and can call the configured AI narrator, which records provider
usage in the cost ledger; moving it behind POST changes the report API the
UI downloads from, so it is tracked separately.
|
||
|
rcourtman
|
71a723abb1 |
Read controller-member disk history at its written key (#2694)
## Problem A physical disk behind a RAID controller (target such as `megaraid,3`) with no usable serial or WWN had its SMART history written under one key and read under another, so its temperature and SMART charts stayed empty. - Host agent: writes under `HostSMARTDiskMetricID`, e.g. `host-pve:sdd@ctrl0/megaraid,3`. - Proxmox: writes under `PhysicalDiskMetricID`, which returns `ProxmoxPhysicalDiskSourceID`, e.g. `pve-node1--dev-sdx:sdx@/megaraid,0`. - Reader: `BuildMetricsTarget` passed those source IDs, which already name the member, to `PhysicalDiskMetaMetricID`. That function appended the topology again: `host-pve:sdd@ctrl0/megaraid,3:sdd@ctrl0/megaraid,3`. The same happened in a registry rehydrated from persisted resources. ## Fix `PhysicalDiskMetaMetricID` now follows the Proxmox writer. A fallback that already ends in the member's topology is returned unchanged. The topology is appended only to a fallback without it: - a Proxmox source ID from before members were scoped, which `PhysicalDiskMetricID` scopes the same way; - the canonical resource ID that `PhysicalDiskView.MetricResourceID` falls back to. TrueNAS disks carry no controller target and are unaffected. ## Existing history - **SMART:** no SMART writer used the doubled key, so samples already stored become readable without a migration. - **Disk I/O:** a linked node's agent with no SMART reading for a member, such as a standby row, files the device's I/O under the Proxmox disk's metrics target. That target was the doubled key. The I/O now lands beside the member's SMART history. Samples already stored under the doubled key are left to age out: nothing reads them, and they are the block path's counter filed under its one merged member. - Mock seeding keys these disks by `disk.ID`, the single-scoped key, so it already matched. ## Proof - `TestIdentitylessControllerMembersReadTheirWritersHistory` (`internal/monitoring/issue1595_collection_trust_test.go`) drives the real writers: `writeHostSMARTMetrics`, `writeHostPhysicalDiskIOMetrics` and the Proxmox `writeSMARTMetrics`. It then queries the store at `MetricsTarget` for five members in a live registry and in one rehydrated through a JSON round trip. The members are two agent members sharing a path, a lone agent member with I/O, a Proxmox member, and a standby member merged into its linked node's Proxmox row. Before the fix all ten targets read the doubled key. - `TestPhysicalDiskMetaMetricIDScopesAControllerMemberOnce` (`internal/unifiedresources/canonical_ids_types_test.go`) pins each fallback shape against its writer: agent, smartctl-labelled agent, Proxmox, legacy Proxmox, canonical resource ID, and a disk that is no controller member. Before the fix, three cases failed with the doubled key. ## Stacking This branch sits on #2685, which sits on #2679, because both edit `physical_disk_ids.go` and the proof file. It supersedes #2661, which carried the same reader fix but conflicted with this stack and with main. |
||
|
rcourtman
|
2eef42c512 |
Keep Off alert defaults off after saving (#2684)
## Summary
Switching a metric Off in a **Global Defaults** row (Alerts >
Thresholds) did not stick. After Save and a reload the metric was back
On at its factory threshold and kept alerting. This affected every
section whose factory threshold is positive: Machines CPU, memory, disk
and disk temperature, Proxmox node temperature, PBS, Docker, Kubernetes,
TrueNAS, vSphere and storage.
The editors stage `-1` for Off, and the save payload passed it through
as `{trigger: -1}`. Backend config normalization reads a negative
default trigger as unset and restores the factory value. Guest and node
CPU, memory and disk escaped only because nothing normalizes them.
- `buildAlertsConfigurationPayload` now writes every Off default as
`{trigger: 0, clear: 0}`, the value the engine and normalization both
read as off. Per-resource overrides are serialized separately and keep
`-1`, which the override path stores as written. A `-1` already stored
on a guest or node default becomes `0` on the next save.
- The Global Defaults **Custom** badge now treats `-1` and `0` as the
same Off value. Without that, a fresh install showed Custom on
Kubernetes, TrueNAS Systems and vSphere, and VMs & Containers turned
Custom after any save, though nothing had changed. An unset factory
value stays distinct from Off: an unset TrueNAS disk temperature follows
Disk temperature by type.
- A row's custom-threshold marker still compares raw values. It drives
the row's revert control and override removal, so a stored `-1` override
must stay visible against an Off default. A test pins that.
- The alerts contract no longer claims Off is written as `-1`
everywhere.
## Verification
- New payload test: every section's Off default is written as `{0, 0}`
and reads back Off. It fails on `main` with `guestDefaults.cpu: expected
{ trigger: -1 }`.
- New backend test: a `{0, 0}` default stays off through `UpdateConfig`
in every section, and a host at 99% raises no CPU, memory or disk alert
when the agent defaults are off. Both fail when the defaults are written
as `-1`.
- Live on a private mock stack. On `main`, Proxmox CPU and Temp switched
Off saved as `{-1}` and `{80, 75}`, and after reload Temp read On. With
the fix:
- Machines CPU at 1% raised 8 alerts. Switching it Off stored `{0, 0}`
and resolved all 8.
- Proxmox Temp Off resolved the node temperature alert.
- Nothing re-fired over two minutes.
- Values stayed Off after a backend restart and a reload. Machines,
Proxmox, Kubernetes, TrueNAS and vSphere were each checked at desktop or
phone width.
- Custom badge on a fresh install: no Proxmox, Kubernetes, TrueNAS or
vSphere group reads Custom, and after an unrelated save only the group
actually changed does.
|
||
|
rcourtman
|
e3a93efcee |
Judge pulse-relaxed guests by one threshold on poll and save
CheckGuest lifted a pulse-relaxed guest's CPU, memory and disk triggers
to the 95/92/95 floor itself, but config-save reevaluation rebuilt the
guest from alert metadata without its tags and judged the alert by the
unrelaxed thresholds. A relaxed guest whose memory threshold was unset
got the floor of 92 while polling, read as no threshold on save, and
the save resolved the alert with a recovery notification; the next poll
raised it again.
The floor also lifted an Off threshold (trigger 0, or -1 from the
editors) like any low trigger, so tagging a guest relaxed turned on an
alert its config switched off, with no recovery band at all ({95, 0}).
The tag is documented as relaxing thresholds; it must never make a
guest more sensitive than its config.
- Apply the relaxed step in effectiveAlertPolicyNoLock's guest branch,
so CheckGuest, reevaluation and any later resolver share it.
- Read the recorded tags back in guestSnapshotFromAlert ([]string live,
[]interface{} after a restart) through a shared metadata reader that
storagePolicyAliases now uses too.
- Leave an Off threshold off; an unset one still gets the floor.
- Drop the tags key from an open alert once its resource has no tags.
Reevaluation now reads it, and a removed pulse-relaxed left behind
would judge the alert by the relaxed trigger while polling uses the
guest's own.
This stacks on the per-filesystem reevaluation fix. Without it, a save
judges a filesystem alert by the guest's relaxed floor instead of the
filesystem's own lower override, and flaps it;
TestConfigSaveKeepsRelaxedGuestFilesystemOverride guards that.
The other new tests fail against the base branch's runtime files via
-overlay.
|
||
|
rcourtman
|
d02d6441dd |
Merge origin/main into fix/unraid-disk-temperature-alerts
Main's Proxmox node cleanup section landed at the same alerts.md anchor as the Unraid disk temperature section; both are kept, main's first. |
||
|
rcourtman
|
539eadfab2 |
Judge filesystem alerts by their own threshold on config save
A config save re-judges every active alert. Agent filesystem alerts were re-judged by the host disk threshold, ignoring the filesystem's own override and the per-type DiskFillByType threshold CheckHost uses. Guest filesystem alerts were resolved by their "<guestID>-disk-<key>" resource ID, which matches no guest-disk override and names no guest, so the guest's own override was missed too. A filesystem override of 70 raised an alert at 80 under a default of 90; saving any alert setting resolved it and sent a recovery, and the next poll raised it again as a new alert. CheckHost and CheckGuest now resolve a filesystem's threshold through hostDiskUsageThresholdNoLock and guestDiskUsageThresholdNoLock, and reevaluation calls the same helpers: resolveHostAlertThresholdsNoLock for agent filesystem alerts, and a new resolveGuestAlertThresholdsNoLock that resolves a guest filesystem alert's guest by the ID parseGuestAlertIdentity reads from its resource ID. The helpers return override and per-type thresholds as owned copies, so evaluation still never normalizes the live config. TestConfigSaveJudgesFilesystemAlertsByTheirOwnThreshold covers an agent filesystem override, an NVMe fill threshold, a guest filesystem override and a guest override. With host.go, guest.go and config_runtime.go overlaid by their origin/main or #2696 copies, all four fail: the unchanged save resolves the alert. Stacked on #2696. |
||
|
rcourtman
|
aa7053a07e | Merge remote-tracking branch 'origin/main' into fix/alert-config-keep-unsent-fields |