diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index f7aba26ab8..4ed5d3ff47 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -15,6 +15,17 @@ ## Purpose +### TrueNAS probes are not fleet or inventory liveness + +A separate settings connection probe does not establish runtime provider +freshness, verified fleet credentials, observed resources or readiness for +monitored-system accounting. These remain tied to actual scheduled refreshes. +Stored-secret selection, tenant isolation and authentication responses remain +unchanged. The connections continuity test seeds its success from a real local +runtime poll rather than a probe-shaped synthetic success. The API contract +control retains a runtime inventory error despite a subsequent successful +system-information probe; no synthetic success establishes installed acceptance. + ### Podman interval CPU evidence — issue #2447 Every one-shot container stats response retains a cumulative CPU baseline, diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index deffbfb875..32da5c32ad 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -20,6 +20,21 @@ ## Purpose +### TrueNAS saved probes do not rewrite runtime evidence — issue #2382 + +The saved connection test uses stored server-owned secrets, or merges an edited +payload under the existing validation and tenant boundary. It reads system +information on a separate client and reports the probe's result in its existing +response. Neither success nor failure rewrites the runtime provider's `poll` +timestamps, last error, failure count, retry schedule, `observed` contribution +or supplemental-inventory readiness. A successful probe can coexist with failed +required inventory collection; only a completed runtime refresh can clear that +runtime failure. No new route, response field or permission is added. +`TestTrueNASSavedProbePreservesRuntimeEvidence` in `internal/api/contract_test.go` +uses an HTTP appliance analogue through the actual client, poller and handlers, +including prior inventory followed by required-method failure and no initial poll. +This is diagnostic truthfulness, not native appliance/session recovery. + ### Resource maintenance API reference The canonical and shipped API references document the existing authenticated diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 38bca29d81..c2e2bb42c7 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -273,8 +273,11 @@ or appliance health, and sending keepalives is not proactive pong-timeout detect Successful TrueNAS refreshes target start-to-start cadence, bounded below by completion plus min(five seconds, configured interval), preventing back-to-back load on slow appliances. Failed refreshes retain a full completion-based retry -interval. Manual connection tests retain completion-based scheduling. Observed -last-attempt and last-success timestamps remain completion timestamps; resource +interval. Manual connection probes use a separate client and do not alter +scheduling or poll evidence: success must not clear a runtime error or advance +last-attempt, last-success, observed collection or inventory readiness. Failure +remains in the probe response, not a fabricated runtime attempt. Observed +last-attempt and last-success timestamps remain runtime completion timestamps; resource freshness thresholds must not be extended to hide genuinely stale devices. `TestAuthenticatedRPCSurvivesIdleTransportTimeout` and @@ -285,6 +288,11 @@ server, opening-context cancellation, concurrent controls/RPCs and sender dispos `internal/monitoring/truenas_poller_test.go` verifies due boundaries, short/slow cycles, completion timestamps and unchanged failure backoff. These are synthetic runtime proofs, not native firmware timeout or reporter-resolution evidence. +`TestTrueNASSavedProbePreservesRuntimeEvidence` in `internal/api/contract_test.go` +checks the actual saved probe and list handlers against never-polled and +previously observed-but-failing runtime states. A synthetic successful login +with a required pool-method failure is not attribution of the native #2382 +regression; the failing appliance/session method remains field evidence. ### TrueNAS empty app inventories and session-disposal evidence diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 0d7f50f36c..d451abd1df 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -21,6 +21,17 @@ ## Purpose +### TrueNAS connection probes do not settle recovery inventory + +A successful settings probe reads system information only, on a separate client. +It cannot advance the runtime poll watermark, manufacture an initial settled +inventory or update the last observed recovery/resource contribution. A probe +failure likewise does not impersonate a completed inventory refresh. Actual +runtime auth/permission failures and existing cache withdrawal remain effective; +a later probe success must not erase them. Source controls cover never-polled +and previously observed-but-failing states through the real API boundary. +No restore, guest mutation or native appliance acceptance is implied. + ### Partial backup inventory is not empty recovery evidence The existing Proxmox Backups view treats PVE and PBS inventories as independent diff --git a/internal/api/connections_handlers_continuity_test.go b/internal/api/connections_handlers_continuity_test.go index d74d9cd322..e1b78cb1e9 100644 --- a/internal/api/connections_handlers_continuity_test.go +++ b/internal/api/connections_handlers_continuity_test.go @@ -164,26 +164,12 @@ func TestConnectionsLedgerSeparatesTelemetryHealthFromCommandAdmission(t *testin } func TestConnectionsHandleListUsesTrueNASPollerRuntimeSummary(t *testing.T) { - persistence := config.NewConfigPersistence(t.TempDir()) - connection := config.TrueNASInstance{ - ID: "tn1", - Name: "TrueNAS", - Host: "truenas.lan", - APIKey: "secret", - UseHTTPS: true, - Enabled: true, - PollIntervalSecs: 60, - MonitorDatasets: true, - MonitorPools: true, - MonitorReplication: true, - } - if err := persistence.SaveTrueNASConfig([]config.TrueNASInstance{connection}); err != nil { - t.Fatalf("SaveTrueNASConfig: %v", err) - } - - poller := monitoring.NewTrueNASPoller(nil, time.Minute, nil) - successAt := time.Now().UTC().Add(-30 * time.Second) - poller.RecordConnectionTestSuccess("default", connection.ID, connection, successAt) + f := newTrueNASPollEvidenceFixture(t) + persistence, connection, poller := f.persistence, f.connection, f.poller + poller.Start(context.Background()) + waitForTrueNASPollEvidence(t, f, func(s monitoring.TrueNASConnectionSummary) bool { return s.Poll.LastSuccessAt != nil }) + poller.Stop() + successAt := *f.summary().Poll.LastSuccessAt handler := NewConnectionsHandlers( func(context.Context) *config.Config { return nil }, @@ -209,7 +195,7 @@ func TestConnectionsHandleListUsesTrueNASPollerRuntimeSummary(t *testing.T) { var tn *Connection for i := range resp.Connections { - if resp.Connections[i].ID == "truenas:tn1" { + if resp.Connections[i].ID == "truenas:"+connection.ID { tn = &resp.Connections[i] break } diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index e02f948562..666e92f1d1 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -25131,3 +25131,67 @@ func TestContract_HostedRuntimeAgentInstallCommandCarriesToken(t *testing.T) { } } } + +func TestTrueNASSavedProbePreservesRuntimeEvidence(t *testing.T) { + for _, state := range []string{"never polled", "failed after observed inventory"} { + t.Run(state, func(t *testing.T) { + f := newTrueNASPollEvidenceFixture(t) + if state != "never polled" { + f.poller.Start(context.Background()) + waitForTrueNASPollEvidence(t, f, func(s monitoring.TrueNASConnectionSummary) bool { + return s.Poll.LastSuccessAt != nil && s.Observed != nil + }) + f.failPools.Store(true) + waitForTrueNASPollEvidence(t, f, func(s monitoring.TrueNASConnectionSummary) bool { return s.Poll.LastError != nil }) + f.poller.Stop() + if !strings.Contains(f.summary().Poll.LastError.Message, "pools") { + t.Fatalf("expected required inventory failure: %+v", f.summary().Poll) + } + } + before, err := json.Marshal(f.summary()) + if err != nil { + t.Fatal(err) + } + readyBefore, settledBefore := f.poller.SupplementalInventoryReadyAt(nil, "default") + for _, failProbe := range []bool{false, true, false} { + f.failLogin.Store(failProbe) + rec := httptest.NewRecorder() + f.handler.HandleTestSavedConnection(rec, httptest.NewRequest(http.MethodPost, "/api/truenas/connections/"+f.connection.ID+"/test", nil)) + wantStatus := http.StatusOK + if failProbe { + wantStatus = http.StatusBadRequest + } + if rec.Code != wantStatus { + t.Fatalf("probe status = %d, want %d: %s", rec.Code, wantStatus, rec.Body.String()) + } + after, err := json.Marshal(f.summary()) + if err != nil { + t.Fatal(err) + } + if string(after) != string(before) { + t.Errorf("probe changed runtime polling evidence: before=%s after=%s", before, after) + } + readyAfter, settledAfter := f.poller.SupplementalInventoryReadyAt(nil, "default") + if settledAfter != settledBefore || !readyAfter.Equal(readyBefore) { + t.Errorf("probe changed inventory readiness: (%v,%t) -> (%v,%t)", readyBefore, settledBefore, readyAfter, settledAfter) + } + } + // Verify the same runtime truth is exposed through the actual list + // handler, not just the poller's internal summary accessor. + rec := httptest.NewRecorder() + f.handler.HandleList(rec, httptest.NewRequest(http.MethodGet, "/api/truenas/connections", nil)) + var rows []trueNASConnectionResponse + if rec.Code != http.StatusOK || json.Unmarshal(rec.Body.Bytes(), &rows) != nil || len(rows) != 1 { + t.Fatalf("list failed: %d %s", rec.Code, rec.Body.String()) + } + exposed, _ := json.Marshal(rows[0].Poll) + poll, _ := json.Marshal(f.summary().Poll) + if string(exposed) != string(poll) { + t.Fatalf("list poll evidence = %s, want %s", exposed, poll) + } + if strings.Contains(rec.Body.String(), f.connection.APIKey) { + t.Fatal("list exposed the stored key") + } + }) + } +} diff --git a/internal/api/truenas_handlers.go b/internal/api/truenas_handlers.go index 62ad0dc002..c6f1f0266d 100644 --- a/internal/api/truenas_handlers.go +++ b/internal/api/truenas_handlers.go @@ -412,11 +412,6 @@ func (h *TrueNASHandlers) HandleTestSavedConnection(w http.ResponseWriter, r *ht } invalidConfig, err := h.testConnectionInstance(r, instance) if err != nil { - if !hasPayload && h != nil && h.getPoller != nil { - if poller := h.getPoller(r.Context()); poller != nil { - poller.RecordConnectionTestFailure(resolveTenantOrgID(r), connectionID, instance, err, time.Now().UTC()) - } - } if invalidConfig { writeErrorResponse(w, http.StatusBadRequest, "truenas_invalid_config", "Invalid TrueNAS connection configuration", map[string]string{"error": err.Error()}) return @@ -424,11 +419,10 @@ func (h *TrueNASHandlers) HandleTestSavedConnection(w http.ResponseWriter, r *ht writeErrorResponse(w, http.StatusBadRequest, "truenas_connection_failed", "Failed to connect to TrueNAS", map[string]string{"error": err.Error()}) return } - if !hasPayload && h != nil && h.getPoller != nil { - if poller := h.getPoller(r.Context()); poller != nil { - poller.RecordConnectionTestSuccess(resolveTenantOrgID(r), connectionID, instance, time.Now().UTC()) - } - } + // This probe reads system information through a separate client. It + // neither refreshes the runtime provider nor contributes inventory, so + // it must not clear poll errors, advance freshness/readiness or defer + // the next scheduled poll. Its result belongs to this response only. writeJSON(w, http.StatusOK, map[string]any{"success": true}) return } diff --git a/internal/api/truenas_handlers_test.go b/internal/api/truenas_handlers_test.go index 7f700ec045..79086ec57d 100644 --- a/internal/api/truenas_handlers_test.go +++ b/internal/api/truenas_handlers_test.go @@ -1381,7 +1381,7 @@ func TestTrueNASHandlers_HandlePreviewSavedConnection_ReturnsUnavailableWhenSupp assertMonitoredSystemUsageUnavailableReason(t, rec, monitoring.MonitoredSystemUsageUnavailableSupplementalInventoryRebuildPending) } -func TestTrueNASHandlers_HandleTestSavedConnection_UpdatesPollSummaryFailure(t *testing.T) { +func TestTrueNASHandlers_HandleTestSavedConnection_FailureDoesNotInventPollAttempt(t *testing.T) { setTrueNASFeatureForTest(t, true) connection := config.TrueNASInstance{ @@ -1415,11 +1415,11 @@ func TestTrueNASHandlers_HandleTestSavedConnection_UpdatesPollSummaryFailure(t * t.Fatalf("expected 400, got %d: %s", rec.Code, rec.Body.String()) } summary := poller.ConnectionSummaries("default", []config.TrueNASInstance{connection})[connection.ID] - if summary.Poll == nil || summary.Poll.LastError == nil { - t.Fatalf("expected saved retest failure to update poll summary, got %+v", summary.Poll) + if summary.Poll == nil || summary.Poll.LastError != nil || summary.Poll.LastAttemptAt != nil || summary.Poll.LastSuccessAt != nil { + t.Fatalf("a separate failed probe must not invent a runtime poll attempt, got %+v", summary.Poll) } - if summary.Poll.LastError.Message != "authentication failed" { - t.Fatalf("expected failure message preserved, got %+v", summary.Poll.LastError) + if !strings.Contains(rec.Body.String(), "authentication failed") { + t.Fatalf("expected probe failure in its own response, got %s", rec.Body.String()) } } diff --git a/internal/api/truenas_poll_evidence_test.go b/internal/api/truenas_poll_evidence_test.go new file mode 100644 index 0000000000..c276929256 --- /dev/null +++ b/internal/api/truenas_poll_evidence_test.go @@ -0,0 +1,81 @@ +package api + +import ( + "context" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" +) + +type trueNASPollEvidenceFixture struct { + handler *TrueNASHandlers + persistence *config.ConfigPersistence + poller *monitoring.TrueNASPoller + connection config.TrueNASInstance + failPools atomic.Bool + failLogin atomic.Bool +} + +// A deterministic REST appliance analogue: system.info can succeed while a +// required inventory method fails. This proves the source distinction, not +// the cause or transport of either #2382 operator's native failure. +func newTrueNASPollEvidenceFixture(t *testing.T) *trueNASPollEvidenceFixture { + t.Helper() + setTrueNASFeatureForTest(t, true) + f := &trueNASPollEvidenceFixture{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/api/v2.0/system/info": + if f.failLogin.Load() { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"error":"probe login denied"}`)) + return + } + _, _ = w.Write([]byte(`{"hostname":"poll-fixture","version":"TrueNAS-SCALE-24.10.2","uptime_seconds":100}`)) + case "/api/v2.0/pool": + if f.failPools.Load() { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":"inventory unavailable"}`)) + return + } + _, _ = w.Write([]byte(`[{"id":1,"name":"tank","status":"ONLINE","size":1000,"allocated":400,"free":600}]`)) + case "/api/v2.0/pool/dataset", "/api/v2.0/disk", "/api/v2.0/alert/list": + _, _ = w.Write([]byte(`[]`)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(server.Close) + cfg := &config.Config{DataPath: t.TempDir()} + f.handler, f.persistence, _ = newTrueNASHandlersForTest(t, cfg) + f.connection = trueNASInstanceFromRawURL(t, "poll-fixture", server.URL, true) + if err := f.persistence.SaveTrueNASConfig([]config.TrueNASInstance{f.connection}); err != nil { + t.Fatal(err) + } + f.poller = monitoring.NewTrueNASPoller(config.NewMultiTenantPersistence(cfg.DataPath), 50*time.Millisecond, nil) + f.handler.getPoller = func(context.Context) *monitoring.TrueNASPoller { return f.poller } + t.Cleanup(f.poller.Stop) + return f +} + +func (f *trueNASPollEvidenceFixture) summary() monitoring.TrueNASConnectionSummary { + return f.poller.ConnectionSummaries("default", []config.TrueNASInstance{f.connection})[f.connection.ID] +} + +func waitForTrueNASPollEvidence(t *testing.T, f *trueNASPollEvidenceFixture, ready func(monitoring.TrueNASConnectionSummary) bool) { + t.Helper() + deadline := time.Now().Add(3 * time.Second) + for time.Now().Before(deadline) { + if ready(f.summary()) { + return + } + time.Sleep(10 * time.Millisecond) + } + t.Fatalf("missing runtime poll evidence: %+v", f.summary()) +} diff --git a/internal/monitoring/truenas_poller.go b/internal/monitoring/truenas_poller.go index 51bf22ffc9..debcdc8b7a 100644 --- a/internal/monitoring/truenas_poller.go +++ b/internal/monitoring/truenas_poller.go @@ -861,55 +861,6 @@ func (p *TrueNASPoller) recordConnectionFailureLocked( } } -// RecordConnectionTestSuccess updates one saved TrueNAS connection summary after -// a manual row-level test without clearing the last observed contribution -// summary. -func (p *TrueNASPoller) RecordConnectionTestSuccess( - orgID string, - connID string, - instance config.TrueNASInstance, - at time.Time, -) { - if p == nil { - return - } - connID = strings.TrimSpace(connID) - if connID == "" { - return - } - orgID = normalizeTrueNASOrgID(orgID) - instance.ApplyDefaults() - - p.mu.Lock() - defer p.mu.Unlock() - p.recordConnectionSuccessLocked(orgID, connID, instance, at, at, nil) -} - -// RecordConnectionTestFailure updates one saved TrueNAS connection summary after -// a manual row-level test failure while preserving any previously observed -// resource contribution summary. -func (p *TrueNASPoller) RecordConnectionTestFailure( - orgID string, - connID string, - instance config.TrueNASInstance, - err error, - at time.Time, -) { - if p == nil || err == nil { - return - } - connID = strings.TrimSpace(connID) - if connID == "" { - return - } - orgID = normalizeTrueNASOrgID(orgID) - instance.ApplyDefaults() - - p.mu.Lock() - defer p.mu.Unlock() - p.recordConnectionFailureLocked(orgID, connID, instance, err, at) -} - func buildTrueNASObservedSummary(snapshot *truenas.FixtureSnapshot) *TrueNASConnectionObservedSummary { if snapshot == nil { return nil diff --git a/internal/monitoring/truenas_poller_test.go b/internal/monitoring/truenas_poller_test.go index fcf3a34c68..290814828e 100644 --- a/internal/monitoring/truenas_poller_test.go +++ b/internal/monitoring/truenas_poller_test.go @@ -396,7 +396,7 @@ func TestTrueNASPollerConnectionSummariesCaptureFailures(t *testing.T) { } } -func TestTrueNASPollerManualConnectionTestsUpdateSummariesWithoutClearingObservedCounts(t *testing.T) { +func TestTrueNASPollerRuntimeRecoveryUpdatesSummariesAndObservedCounts(t *testing.T) { poller := NewTrueNASPoller(nil, time.Minute, nil) connection := config.TrueNASInstance{ ID: "manual-test-conn", @@ -416,24 +416,28 @@ func TestTrueNASPollerManualConnectionTestsUpdateSummariesWithoutClearingObserve } firstSuccess := time.Date(2026, time.March, 30, 10, 0, 0, 0, time.UTC) failureAt := firstSuccess.Add(2 * time.Minute) - manualSuccessAt := failureAt.Add(2 * time.Minute) + recoveryAt := failureAt.Add(2 * time.Minute) poller.mu.Lock() poller.recordConnectionSuccessLocked("default", connection.ID, connection, firstSuccess, firstSuccess, snapshot) poller.recordConnectionFailureLocked("default", connection.ID, connection, errors.New("manual auth failed"), failureAt) poller.mu.Unlock() - poller.RecordConnectionTestSuccess("default", connection.ID, connection, manualSuccessAt) + snapshot.CollectedAt = recoveryAt + poller.mu.Lock() + poller.recordConnectionSuccessLocked("default", connection.ID, connection, recoveryAt, recoveryAt, snapshot) + poller.mu.Unlock() summary := poller.ConnectionSummaries("default", []config.TrueNASInstance{connection})[connection.ID] if summary.Poll == nil || summary.Poll.LastSuccessAt == nil { - t.Fatalf("expected manual success to update poll summary, got %+v", summary.Poll) + t.Fatalf("expected runtime recovery to update poll summary, got %+v", summary.Poll) } - if summary.Poll.LastError != nil { - t.Fatalf("expected manual success to clear previous error, got %+v", summary.Poll.LastError) + if summary.Poll.LastError != nil || summary.Poll.ConsecutiveFailures != 0 || !summary.Poll.LastSuccessAt.Equal(recoveryAt) { + t.Fatalf("expected runtime recovery to clear previous error, got %+v", summary.Poll) } - if summary.Observed == nil || summary.Observed.Host != "manual-test" || summary.Observed.StoragePools != 1 { - t.Fatalf("expected observed summary to be preserved after manual success, got %+v", summary.Observed) + if summary.Observed == nil || summary.Observed.Host != "manual-test" || summary.Observed.StoragePools != 1 || + summary.Observed.CollectedAt == nil || !summary.Observed.CollectedAt.Equal(recoveryAt) { + t.Fatalf("expected observed summary to advance after runtime recovery, got %+v", summary.Observed) } } @@ -1653,7 +1657,9 @@ func TestTrueNASPollerSupplementalInventoryReadyAtUsesPersistedActiveConnections } attemptedAt := time.Now().UTC() - poller.RecordConnectionTestSuccess("default", connection.ID, connection, attemptedAt) + poller.mu.Lock() + poller.recordConnectionSuccessLocked("default", connection.ID, connection, attemptedAt, attemptedAt, &truenas.FixtureSnapshot{CollectedAt: attemptedAt}) + poller.mu.Unlock() readyAt, settled := poller.SupplementalInventoryReadyAt(nil, "default") if !settled {