From dbc4ca54fdaf98e55201c6522afd1e39dc514d4a Mon Sep 17 00:00:00 2001 From: rcourtman Date: Mon, 24 Aug 2026 22:43:47 +0100 Subject: [PATCH] Prevent connection alerts from bypassing offline policy --- .../v6/internal/subsystems/agent-lifecycle.md | 5 + .../v6/internal/subsystems/alerts.md | 9 + .../v6/internal/subsystems/api-contracts.md | 5 + .../v6/internal/subsystems/monitoring.md | 5 + .../internal/subsystems/storage-recovery.md | 5 + internal/alerts/canonical_lifecycle.go | 50 +++-- internal/alerts/config_runtime.go | 10 + internal/alerts/connection.go | 102 +++++++++- internal/alerts/connection_test.go | 178 ++++++++++++++++++ internal/alerts/intent_policy_test.go | 30 +++ internal/alerts/notification_policy.go | 2 +- internal/api/connections_aggregator.go | 8 +- internal/api/connections_aggregator_test.go | 19 ++ internal/api/connections_alerts.go | 15 +- ...connections_alerts_branchcov0722am_test.go | 18 +- internal/api/connections_types.go | 1 + internal/api/contract_test.go | 14 ++ .../monitoring/monitor_alert_intent_test.go | 9 + internal/monitoring/monitor_client_init.go | 2 +- .../monitoring/monitor_client_reconnect.go | 2 +- .../monitoring/monitor_pbs_coverage_test.go | 3 + internal/monitoring/monitor_pbs_pmg.go | 6 +- .../monitoring/platform_instance_identity.go | 13 ++ 23 files changed, 458 insertions(+), 53 deletions(-) create mode 100644 internal/monitoring/platform_instance_identity.go diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index a8108b5d7..365fab15a 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -74,6 +74,11 @@ The shared `internal/api/ai_handlers.go` route may also reopen a dismissed Patrol finding and mirror that state into the unified findings store. This is AI finding-state management only; it grants no agent enrollment, report, profile, update, removal, command, or fleet-control authority. +The shared Connections aggregation may carry a JSON-excluded PBS/PMG +monitor-resource identity into alert evaluation so the owning resource's +availability policy is applied. That alert-only routing identity must not +become an agent enrollment, host continuity, command-channel, or fleet +identity, and it must not alter the public Connections payload. The JSON-excluded Proxmox VM/LXC I/O-rate validity fields carried by `internal/models/models.go` are likewise monitoring-owned sidecar evidence. They distinguish a valid idle interval from an unknown rate for history, diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index ed6463d29..a24687526 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -263,6 +263,15 @@ notification-owned queue truth. When retained terminal failures exist, the Destinations surface exposes explicit retry and dismiss actions with consequence confirmations, refreshes health and delivery history after either action, and never instructs the operator to delete queue storage. +Platform `connection-degraded` alerts are availability observations of their +owning PVE, PBS, PMG, VMware, or TrueNAS resource, not a separate policy +surface. They must honor that resource's disabled and connectivity-disabled +override, the platform-wide alert and offline-alert switches, offline intent +and quiet-hours policy, and must clear immediately when that policy becomes +disabled. The connection snapshot must carry the owning monitor resource ID +used by registry alias resolution; the ledger's display ID is not a substitute +for that policy identity. A second connection detector must never notify around +a resource's offline-alert toggle. Alert runtime state has one explicit ownership boundary: `AlertConfig.enabled` controls detector evaluation and in-product alert visibility, while `AlertConfig.activationState` controls external notification delivery only. diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 20a80708a..cb3c134c0 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -347,6 +347,11 @@ raw profile or metadata desire. A profile that wants commands enabled but is served to a runtime token without an allowed `agent:exec` binding is desired-disabled for the connections payload, matching `/api/agents/agent/{id}/config` and agent report responses. +The internal connection-to-alert snapshot also carries the platform monitor's +resource ID separately from the public connection-row ID. Alert policy lookup +must resolve that source identity through the unified registry so a PBS or PMG +offline override saved under the registry resource cannot be bypassed by the +ledger's `pbs:` or `pmg:` display identity. PVE setup API consumers, generated scripts, runtime setup, installer setup, and browser manual guidance must share one `PulseMonitor` privilege contract: diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 73775f35a..abfbb387c 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -40,6 +40,11 @@ Monitoring owns source freshness cadence for Proxmox, PBS, and PMG resources: the stale threshold is derived from the configured polling interval with a minimum floor, so API-facing resource status must not degrade merely because a healthy source is between normal poll cycles. +PBS and PMG configured instances also have one monitoring-owned runtime +resource identity constructor. Poll publication, connection status, setup and +auto-registration checks, canonical alias resolution, and alert-policy bridges +must all use that constructor rather than rebuilding `pbs-` or +`pmg-` independently. Proxmox guest enumeration is a generation boundary. VM and LXC collection and enrichment must finish before one `State.UpdateGuestsForInstance` publication, so readers never observe a VM-only or LXC-only intermediate snapshot. A failed diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index a25c693f5..54c2f0b04 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -50,6 +50,11 @@ The shared alerts API may persist and apply `schedule.initialNotify` for email, webhook, or Apprise delivery. That notification routing is not storage-health, backup, recovery-point, restore, or protection evidence; storage/recovery surfaces must not infer product state from the selected destination. +The shared Connections aggregation path may also carry an unexported PBS/PMG +monitor-resource identity into alert evaluation so availability policy resolves +against the owning resource rather than the public ledger row ID. That routing +identity must not enter the public Connections payload, become recovery-point +identity, or be interpreted as backup, restore, or protection evidence. The shared AI handlers may reopen dismissed Patrol findings and synchronize the unified finding projection. That finding-state transition does not create, delete, validate, or restore recovery points and must not be treated as diff --git a/internal/alerts/canonical_lifecycle.go b/internal/alerts/canonical_lifecycle.go index 96aa6dc30..d1ab109d6 100644 --- a/internal/alerts/canonical_lifecycle.go +++ b/internal/alerts/canonical_lifecycle.go @@ -10,23 +10,25 @@ import ( ) type canonicalLifecycleAlertParams struct { - Spec alertspecs.ResourceAlertSpec - Evidence alertspecs.AlertEvidence - Tracking map[string]int - TrackingKey string - AlertID string - AlertType string - ResourceID string - ResourceName string - Node string - Instance string - Message string - Metadata map[string]interface{} - AddToRecent bool - AddToHistory bool - RateLimit bool - DispatchAsync bool - IntentBackup BackupIntentContext + Spec alertspecs.ResourceAlertSpec + Evidence alertspecs.AlertEvidence + IntentSignal string + PolicyDisabledNoLock func() bool + Tracking map[string]int + TrackingKey string + AlertID string + AlertType string + ResourceID string + ResourceName string + Node string + Instance string + Message string + Metadata map[string]interface{} + AddToRecent bool + AddToHistory bool + RateLimit bool + DispatchAsync bool + IntentBackup BackupIntentContext } type canonicalStatefulAlertParams struct { @@ -336,6 +338,14 @@ func (m *Manager) evaluateCanonicalLifecycleAlert(params canonicalLifecycleAlert }() defer m.mu.Unlock() + // Recheck mutable policy while holding the same lock used for lifecycle + // state and dispatch. This closes the save-vs-dispatch race where policy + // could be disabled after a detector's initial snapshot but before it + // activated the alert. + if params.PolicyDisabledNoLock != nil && params.PolicyDisabledNoLock() { + params.Spec.Disabled = true + } + storageKey := canonicalTrackingKeyForSpec(params.Spec, params.AlertID) trackingKey := storageKey @@ -374,8 +384,8 @@ func (m *Manager) evaluateCanonicalLifecycleAlert(params canonicalLifecycleAlert } } - intentSignal := "" - if params.Spec.Kind == alertspecs.AlertSpecKindConnectivity || params.Spec.Kind == alertspecs.AlertSpecKindPoweredState { + intentSignal := params.IntentSignal + if intentSignal == "" && (params.Spec.Kind == alertspecs.AlertSpecKindConnectivity || params.Spec.Kind == alertspecs.AlertSpecKindPoweredState) { intentSignal = string(AlertIntentSignalOffline) } if intentSignal != "" && existing == nil { @@ -384,7 +394,7 @@ func (m *Manager) evaluateCanonicalLifecycleAlert(params canonicalLifecycleAlert if decision.StateChanged { m.saveActiveAlertsAsync("lifecycle intent state") } - if decision.Effective.Explicit && conditionActive && !decision.ShouldActivate { + if conditionActive && !decision.ShouldActivate && (decision.Effective.Explicit || decision.Suppressed) { result.State.State = alertspecs.AlertStatePending result.State.Reason = decision.Reason if pending, ok := m.intentPending[storageKey]; ok && !pending.FirstMatchedAt.IsZero() { diff --git a/internal/alerts/config_runtime.go b/internal/alerts/config_runtime.go index bb02bc53f..3f792c8db 100644 --- a/internal/alerts/config_runtime.go +++ b/internal/alerts/config_runtime.go @@ -377,6 +377,16 @@ func (m *Manager) reevaluateActiveAlertsLocked() { primaryResourceType = resourceTypeKeys[0] } + if connectionType, ok := connectionTypeFromAlert(alert); ok { + policyResourceID := metadataStringValue(alert.Metadata, "policyResourceID") + if m.connectionDegradedPolicyDisabledNoLock(resourceID, policyResourceID, connectionType) { + alertsToResolve = append(alertsToResolve, alertID) + delete(m.connectionDegradedCount, resourceID) + delete(m.offlineRecoveryConfirmations, alertID) + } + continue + } + if alert.Type == "queue-depth" || alert.Type == "queue-deferred" || alert.Type == "queue-hold" || alert.Type == "message-age" { if m.config.DisableAllPMG { alertsToResolve = append(alertsToResolve, alertID) diff --git a/internal/alerts/connection.go b/internal/alerts/connection.go index efd768f13..a0ff31c25 100644 --- a/internal/alerts/connection.go +++ b/internal/alerts/connection.go @@ -54,14 +54,15 @@ type ConnectionErrorSnapshot struct { // this shape before invoking CheckConnection so alerts does not depend on // api. type ConnectionSnapshot struct { - ID string - Name string - Type ConnectionType - State ConnectionState - StateReason string - Enabled bool - LastSeen *time.Time - LastError *ConnectionErrorSnapshot + ID string + PolicyResourceID string + Name string + Type ConnectionType + State ConnectionState + StateReason string + Enabled bool + LastSeen *time.Time + LastError *ConnectionErrorSnapshot } // connectionDegradedAlertType is the alert.Type emitted for connection-degraded @@ -83,6 +84,77 @@ func isPlatformConnectionType(t ConnectionType) bool { } } +// connectionDegradedPolicyDisabledNoLock applies the alert policy owned by +// the platform resource to its canonical connection-degraded lifecycle. A +// platform connection is another observation of that resource's +// availability, not an independent alert-policy surface. Callers must hold at +// least m.mu.RLock. +func (m *Manager) connectionDegradedPolicyDisabledNoLock(resourceID, policyResourceID string, connectionType ConnectionType) bool { + if !m.config.Enabled { + return true + } + + thresholdType := "" + switch connectionType { + case ConnectionTypePVE: + if m.config.DisableAllNodes || m.config.DisableAllNodesOffline { + return true + } + thresholdType = "node" + case ConnectionTypePBS: + if m.config.DisableAllPBS || m.config.DisableAllPBSOffline { + return true + } + thresholdType = "pbs" + case ConnectionTypePMG: + if m.config.DisableAllPMG || m.config.DisableAllPMGOffline { + return true + } + thresholdType = "pmg" + case ConnectionTypeVMware: + if m.config.DisableAllVMware { + return true + } + thresholdType = "vmware-host" + case ConnectionTypeTrueNAS: + if m.config.DisableAllTrueNAS { + return true + } + thresholdType = "truenas-system" + default: + return true + } + + policyResourceID = strings.TrimSpace(policyResourceID) + if policyResourceID == "" { + policyResourceID = resourceID + } + thresholds := m.resolveResourceThresholds(thresholdType, policyResourceID) + return thresholds.Disabled || thresholds.DisableConnectivity +} + +func connectionTypeFromAlert(alert *Alert) (ConnectionType, bool) { + if alert == nil || alert.Type != connectionDegradedAlertType { + return "", false + } + connectionType := ConnectionType(strings.TrimSpace(metadataStringValue(alert.Metadata, "connectionType"))) + return connectionType, isPlatformConnectionType(connectionType) +} + +// suppressConnectionDegradedAlert immediately removes detector tracking and +// any active alert when the connection or its availability policy is disabled. +// Policy changes are authoritative and do not need healthy-poll confirmation. +func (m *Manager) suppressConnectionDegradedAlert(snap ConnectionSnapshot) { + alertID := canonicalDiscreteStateStateID(snap.ID, connectionDegradedStateKey) + + m.mu.Lock() + delete(m.connectionDegradedCount, snap.ID) + delete(m.offlineRecoveryConfirmations, alertID) + m.mu.Unlock() + + m.clearAlert(alertID) +} + // CheckConnection raises or clears the connection-degraded alert for one // platform connection. Severity scales with observed state: stale → warning, // unreachable / unauthorized → critical. State=active runs through the @@ -95,8 +167,11 @@ func (m *Manager) CheckConnection(snap ConnectionSnapshot) { if strings.TrimSpace(snap.ID) == "" { return } - if !snap.Enabled || snap.State == ConnectionStatePaused { - m.clearConnectionDegradedAlert(snap) + m.mu.RLock() + policyDisabled := m.connectionDegradedPolicyDisabledNoLock(snap.ID, snap.PolicyResourceID, snap.Type) + m.mu.RUnlock() + if !snap.Enabled || snap.State == ConnectionStatePaused || policyDisabled { + m.suppressConnectionDegradedAlert(snap) return } @@ -162,6 +237,9 @@ func (m *Manager) CheckConnection(snap ConnectionSnapshot) { "connectionType": string(snap.Type), "state": string(snap.State), } + if policyResourceID := strings.TrimSpace(snap.PolicyResourceID); policyResourceID != "" { + metadata["policyResourceID"] = policyResourceID + } if reason != "" { metadata["stateReason"] = reason } @@ -187,6 +265,10 @@ func (m *Manager) CheckConnection(snap ConnectionSnapshot) { Observed: string(snap.State), }, }, + IntentSignal: string(AlertIntentSignalOffline), + PolicyDisabledNoLock: func() bool { + return m.connectionDegradedPolicyDisabledNoLock(snap.ID, snap.PolicyResourceID, snap.Type) + }, Tracking: m.connectionDegradedCount, TrackingKey: snap.ID, AlertID: alertID, diff --git a/internal/alerts/connection_test.go b/internal/alerts/connection_test.go index 565dfc6fe..056dde299 100644 --- a/internal/alerts/connection_test.go +++ b/internal/alerts/connection_test.go @@ -179,6 +179,184 @@ func TestCheckConnection(t *testing.T) { }) } +func TestCheckConnectionHonorsOwningResourceAvailabilityPolicy(t *testing.T) { + pbs, adapter, canonicalID := newPBSOfflinePolicyFixture(t) + connectionID := "pbs:" + pbs.Name + resolvedPolicyID, ok := adapter.ResolveCanonicalResourceID(pbs.ID) + if !ok || resolvedPolicyID != canonicalID { + t.Fatalf("PBS policy ID %q resolved to %q, %t; want %q", pbs.ID, resolvedPolicyID, ok, canonicalID) + } + + newPBSConnectionManager := func(t *testing.T) (*Manager, chan *Alert) { + t.Helper() + m := newTestManager(t) + m.SetResourceIntentIdentityResolver(adapter.ResolveCanonicalResourceID) + cfg := m.GetConfig() + cfg.ActivationState = ActivationActive + m.UpdateConfig(cfg) + fired := make(chan *Alert, 4) + m.SetAlertCallback(func(alert *Alert) { fired <- alert }) + return m, fired + } + snap := ConnectionSnapshot{ + ID: connectionID, + PolicyResourceID: pbs.ID, + Name: pbs.Name, + Type: ConnectionTypePBS, + State: ConnectionStateUnreachable, + Enabled: true, + } + alertID := canonicalDiscreteStateStateID(snap.ID, connectionDegradedStateKey) + + t.Run("canonical per-resource offline toggle blocks alert and notification", func(t *testing.T) { + m, fired := newPBSConnectionManager(t) + cfg := m.GetConfig() + cfg.Overrides = map[string]ThresholdConfig{ + canonicalID: {DisableConnectivity: true}, + } + m.UpdateConfig(cfg) + + for range 5 { + m.CheckConnection(snap) + } + + if testHasActiveAlert(t, m, alertID) { + t.Fatal("offline-disabled PBS created a connection-degraded alert") + } + m.mu.RLock() + _, tracked := m.connectionDegradedCount[snap.ID] + m.mu.RUnlock() + if tracked { + t.Fatal("offline-disabled PBS retained connection-degraded confirmation state") + } + select { + case alert := <-fired: + t.Fatalf("offline-disabled PBS dispatched %q", alert.ID) + default: + } + }) + + t.Run("global PBS offline toggle blocks the parallel detector", func(t *testing.T) { + m, fired := newPBSConnectionManager(t) + cfg := m.GetConfig() + cfg.DisableAllPBSOffline = true + m.UpdateConfig(cfg) + + for range 5 { + m.CheckConnection(snap) + } + + if testHasActiveAlert(t, m, alertID) { + t.Fatal("global PBS offline toggle left connection-degraded active") + } + select { + case alert := <-fired: + t.Fatalf("global PBS offline toggle dispatched %q", alert.ID) + default: + } + }) + + t.Run("enabling the toggle immediately resolves a standing alert", func(t *testing.T) { + m, fired := newPBSConnectionManager(t) + for range 3 { + m.CheckConnection(snap) + } + testRequireActiveAlert(t, m, alertID) + select { + case <-fired: + default: + t.Fatal("enabled PBS connection did not dispatch its firing alert") + } + + cfg := m.GetConfig() + cfg.Overrides = map[string]ThresholdConfig{ + canonicalID: {DisableConnectivity: true}, + } + m.UpdateConfig(cfg) + + if testHasActiveAlert(t, m, alertID) { + t.Fatal("policy update did not immediately resolve connection-degraded") + } + m.mu.RLock() + _, tracked := m.connectionDegradedCount[snap.ID] + m.mu.RUnlock() + if tracked { + t.Fatal("policy update left connection-degraded confirmation state") + } + }) + + t.Run("expected-offline intent blocks both state and notification", func(t *testing.T) { + m, fired := newPBSConnectionManager(t) + m.SetOperatorIntentContextResolver(func(resourceID string, observedAt time.Time) (OperatorIntentContext, bool) { + if resourceID != connectionID { + return OperatorIntentContext{}, false + } + return OperatorIntentContext{MonitoringMode: "expected_offline", LifecycleState: "active"}, true + }) + + for range 5 { + m.CheckConnection(snap) + } + + if testHasActiveAlert(t, m, alertID) { + t.Fatal("expected-offline PBS created a connection-degraded alert") + } + select { + case alert := <-fired: + t.Fatalf("expected-offline PBS dispatched %q", alert.ID) + default: + } + }) +} + +func TestConnectionDegradedUsesOfflineNotificationPolicy(t *testing.T) { + alert := &Alert{Type: connectionDegradedAlertType} + if got := quietHoursCategoryForAlert(alert); got != "offline" { + t.Fatalf("connection-degraded quiet-hours category = %q, want offline", got) + } +} + +func TestConnectionDegradedPolicyUsesEveryPlatformResourceOverride(t *testing.T) { + tests := []struct { + name string + connectionType ConnectionType + resourceID string + }{ + {name: "PVE", connectionType: ConnectionTypePVE, resourceID: "pve:lab"}, + {name: "PBS", connectionType: ConnectionTypePBS, resourceID: "pbs:backup"}, + {name: "PMG", connectionType: ConnectionTypePMG, resourceID: "pmg:mail"}, + {name: "VMware", connectionType: ConnectionTypeVMware, resourceID: "vmware:vcenter"}, + {name: "TrueNAS", connectionType: ConnectionTypeTrueNAS, resourceID: "truenas:nas"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + m := newTestManager(t) + cfg := m.GetConfig() + cfg.Overrides = map[string]ThresholdConfig{ + test.resourceID: {DisableConnectivity: true}, + } + m.UpdateConfig(cfg) + + snap := ConnectionSnapshot{ + ID: test.resourceID, + Name: test.name, + Type: test.connectionType, + State: ConnectionStateUnreachable, + Enabled: true, + } + for range 5 { + m.CheckConnection(snap) + } + + alertID := canonicalDiscreteStateStateID(snap.ID, connectionDegradedStateKey) + if testHasActiveAlert(t, m, alertID) { + t.Fatalf("%s connectivity-disabled override created connection-degraded", test.name) + } + }) + } +} + func TestClearConnectionDegradedAlert(t *testing.T) { t.Run("clears an active alert after the recovery confirmation gate", func(t *testing.T) { m := newTestManager(t) diff --git a/internal/alerts/intent_policy_test.go b/internal/alerts/intent_policy_test.go index b0567c16a..1aa9b9dc6 100644 --- a/internal/alerts/intent_policy_test.go +++ b/internal/alerts/intent_policy_test.go @@ -11,6 +11,36 @@ import ( func intPointer(value int) *int { return &value } func boolPointer(value bool) *bool { return &value } +func TestConnectionDegradedCanonicalLifecycleHonorsOwningOfflinePolicy(t *testing.T) { + pbs, adapter, canonicalID := newPBSOfflinePolicyFixture(t) + m := newTestManager(t) + m.SetResourceIntentIdentityResolver(adapter.ResolveCanonicalResourceID) + + cfg := m.GetConfig() + cfg.ActivationState = ActivationActive + cfg.Overrides = map[string]ThresholdConfig{ + canonicalID: {DisableConnectivity: true}, + } + m.UpdateConfig(cfg) + + snapshot := ConnectionSnapshot{ + ID: "pbs:" + pbs.Name, + PolicyResourceID: pbs.ID, + Name: pbs.Name, + Type: ConnectionTypePBS, + State: ConnectionStateUnreachable, + Enabled: true, + } + for range 5 { + m.CheckConnection(snapshot) + } + + alertID := canonicalDiscreteStateStateID(snapshot.ID, connectionDegradedStateKey) + if testHasActiveAlert(t, m, alertID) { + t.Fatal("connection-degraded bypassed the canonical PBS offline policy") + } +} + func TestAlertIntentPolicyResolutionPrecedenceIsFieldByField(t *testing.T) { m := NewManagerWithDataDir(t.TempDir()) t.Cleanup(m.Stop) diff --git a/internal/alerts/notification_policy.go b/internal/alerts/notification_policy.go index 5bb41fcfd..831d0dd72 100644 --- a/internal/alerts/notification_policy.go +++ b/internal/alerts/notification_policy.go @@ -410,7 +410,7 @@ func quietHoursCategoryForAlert(alert *Alert) string { return "offline" } return "performance" - case "connectivity", "offline", "powered-off", "docker-host-offline": + case "connectivity", "offline", "powered-off", "docker-host-offline", connectionDegradedAlertType: return "offline" } diff --git a/internal/api/connections_aggregator.go b/internal/api/connections_aggregator.go index ab1c2fa06..b39930834 100644 --- a/internal/api/connections_aggregator.go +++ b/internal/api/connections_aggregator.go @@ -186,11 +186,15 @@ func buildConnections(in aggregatorInputs) []Connection { } for _, pbs := range in.pbsInstances { interval := effectivePollInterval(in.pbsPollingInterval, in.plannedPollIntervals["pbs::"+pbs.Name]) - out = append(out, buildPBSConnection(pbs, in.instanceHealth, now, interval, in.pbsReportedNodeNames[pbs.Name])) + connection := buildPBSConnection(pbs, in.instanceHealth, now, interval, in.pbsReportedNodeNames[pbs.Name]) + connection.alertPolicyResourceID = monitoring.PBSMonitorResourceID(pbs.Name) + out = append(out, connection) } for _, pmg := range in.pmgInstances { interval := effectivePollInterval(in.pmgPollingInterval, in.plannedPollIntervals["pmg::"+pmg.Name]) - out = append(out, buildPMGConnection(pmg, in.instanceHealth, now, interval)) + connection := buildPMGConnection(pmg, in.instanceHealth, now, interval) + connection.alertPolicyResourceID = monitoring.PMGMonitorResourceID(pmg.Name) + out = append(out, connection) } for _, vmw := range in.vmwareInstances { out = append(out, buildVMwareConnection(vmw, in.instanceHealth, in.vmwareSummaries, now)) diff --git a/internal/api/connections_aggregator_test.go b/internal/api/connections_aggregator_test.go index 20630e1ec..c5cb88f8d 100644 --- a/internal/api/connections_aggregator_test.go +++ b/internal/api/connections_aggregator_test.go @@ -138,6 +138,25 @@ func TestBuildConnections_SortsByTypeThenName(t *testing.T) { } } +func TestBuildConnectionsCarriesMonitorPolicyIdentityForPBSAndPMG(t *testing.T) { + got := buildConnections(aggregatorInputs{ + pbsInstances: []config.PBSInstance{{Name: "backup-main"}}, + pmgInstances: []config.PMGInstance{{Name: "mail-main"}}, + now: time.Now(), + }) + + policyIDs := make(map[ConnectionType]string, len(got)) + for _, connection := range got { + policyIDs[connection.Type] = connection.alertPolicyResourceID + } + if policyIDs[ConnectionTypePBS] != monitoring.PBSMonitorResourceID("backup-main") { + t.Fatalf("PBS alert policy ID = %q, want monitor resource identity", policyIDs[ConnectionTypePBS]) + } + if policyIDs[ConnectionTypePMG] != monitoring.PMGMonitorResourceID("mail-main") { + t.Fatalf("PMG alert policy ID = %q, want monitor resource identity", policyIDs[ConnectionTypePMG]) + } +} + func TestBuildConnections_PVEPausedRespectsDisabled(t *testing.T) { in := aggregatorInputs{ pveInstances: []config.PVEInstance{{Name: "pve1", Host: "https://pve1.lan:8006", Disabled: true}}, diff --git a/internal/api/connections_alerts.go b/internal/api/connections_alerts.go index dc8e519a5..7c2229a48 100644 --- a/internal/api/connections_alerts.go +++ b/internal/api/connections_alerts.go @@ -114,13 +114,14 @@ func snapshotConnectionsForAlerts(connections []Connection) []alerts.ConnectionS } snap := alerts.ConnectionSnapshot{ - ID: conn.ID, - Name: conn.Name, - Type: alertType, - State: alerts.ConnectionState(conn.State), - StateReason: conn.StateReason, - Enabled: conn.Enabled, - LastSeen: conn.LastSeen, + ID: conn.ID, + PolicyResourceID: conn.alertPolicyResourceID, + Name: conn.Name, + Type: alertType, + State: alerts.ConnectionState(conn.State), + StateReason: conn.StateReason, + Enabled: conn.Enabled, + LastSeen: conn.LastSeen, } if conn.LastError != nil { snap.LastError = &alerts.ConnectionErrorSnapshot{ diff --git a/internal/api/connections_alerts_branchcov0722am_test.go b/internal/api/connections_alerts_branchcov0722am_test.go index 6e40e156d..51b4d69c8 100644 --- a/internal/api/connections_alerts_branchcov0722am_test.go +++ b/internal/api/connections_alerts_branchcov0722am_test.go @@ -100,10 +100,11 @@ func TestBranchcov0722SnapshotConnectionsForAlerts(t *testing.T) { }, }, { - ID: "pbs:store-1", - Type: ConnectionTypePBS, - Name: "store-1", - State: ConnectionStateActive, + ID: "pbs:store-1", + Type: ConnectionTypePBS, + Name: "store-1", + State: ConnectionStateActive, + alertPolicyResourceID: "pbs-store-1", }, } @@ -138,10 +139,11 @@ func TestBranchcov0722SnapshotConnectionsForAlerts(t *testing.T) { // Sparse row with no LastSeen / no LastError: those fields stay nil. wantPBS := alerts.ConnectionSnapshot{ - ID: "pbs:store-1", - Name: "store-1", - Type: alerts.ConnectionTypePBS, - State: alerts.ConnectionStateActive, + ID: "pbs:store-1", + PolicyResourceID: "pbs-store-1", + Name: "store-1", + Type: alerts.ConnectionTypePBS, + State: alerts.ConnectionStateActive, } if !reflect.DeepEqual(got[1], wantPBS) { t.Fatalf("pbs snapshot mismatch:\n got %+v\n want %+v", got[1], wantPBS) diff --git a/internal/api/connections_types.go b/internal/api/connections_types.go index 72ff1e8ad..a9d560514 100644 --- a/internal/api/connections_types.go +++ b/internal/api/connections_types.go @@ -186,6 +186,7 @@ type Connection struct { agentTokenID string commandChannelConnected *bool inventoryCompleteness *RuntimeInventoryCompleteness + alertPolicyResourceID string } type ConnectionSystemComponentRole string diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 187ca21dc..741afc4b2 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -96,6 +96,20 @@ func TestContractPatrolInternalBridgePreservesBoundedToolAuthority(t *testing.T) } } +func TestContractConnectionAlertPolicyIdentityStaysInternal(t *testing.T) { + payload, err := json.Marshal(Connection{ + ID: "pbs:backup-main", + Type: ConnectionTypePBS, + alertPolicyResourceID: monitoring.PBSMonitorResourceID("backup-main"), + }) + if err != nil { + t.Fatalf("marshal connection: %v", err) + } + if bytes.Contains(payload, []byte("alertPolicyResourceID")) || bytes.Contains(payload, []byte("pbs-backup-main")) { + t.Fatalf("internal alert policy identity leaked into public connection payload: %s", payload) + } +} + func TestContractAIChatRestartReappliesLiveRuntimeWiring(t *testing.T) { mockSvc := &MockAIService{} mockSvc.On("Restart", tmock.Anything, tmock.Anything).Return(nil) diff --git a/internal/monitoring/monitor_alert_intent_test.go b/internal/monitoring/monitor_alert_intent_test.go index f14395bb4..95fe9f05e 100644 --- a/internal/monitoring/monitor_alert_intent_test.go +++ b/internal/monitoring/monitor_alert_intent_test.go @@ -39,6 +39,15 @@ func TestInstallOperatorIntentResolverProjectsCanonicalResourcePolicy(t *testing } } +func TestPlatformMonitorResourceIdentityConstructors(t *testing.T) { + if got := PBSMonitorResourceID("backup-main"); got != "pbs-backup-main" { + t.Fatalf("PBS monitor resource ID = %q", got) + } + if got := PMGMonitorResourceID("mail-main"); got != "pmg-mail-main" { + t.Fatalf("PMG monitor resource ID = %q", got) + } +} + func TestResolveBackupIntentContextRequiresFreshActiveMatchingEvidence(t *testing.T) { now := time.Date(2026, 7, 20, 12, 0, 0, 0, time.UTC) state := models.NewState() diff --git a/internal/monitoring/monitor_client_init.go b/internal/monitoring/monitor_client_init.go index 733180578..6e35ffb89 100644 --- a/internal/monitoring/monitor_client_init.go +++ b/internal/monitoring/monitor_client_init.go @@ -112,7 +112,7 @@ func (m *Monitor) initPBSClients(cfg *config.Config) { m.stalenessTracker.UpdateError(InstanceTypePBS, pbsInst.Name) } m.publishPBSConnectionOutcome(models.PBSInstance{ - ID: "pbs-" + pbsInst.Name, + ID: PBSMonitorResourceID(pbsInst.Name), Name: pbsInst.Name, Host: pbsInst.Host, GuestURL: pbsInst.GuestURL, diff --git a/internal/monitoring/monitor_client_reconnect.go b/internal/monitoring/monitor_client_reconnect.go index d2f9f509b..e948f0671 100644 --- a/internal/monitoring/monitor_client_reconnect.go +++ b/internal/monitoring/monitor_client_reconnect.go @@ -166,7 +166,7 @@ func (m *Monitor) retryFailedConnections(ctx context.Context) { m.stalenessTracker.UpdateError(InstanceTypePBS, pbsInst.Name) } m.publishPBSConnectionOutcome(models.PBSInstance{ - ID: "pbs-" + pbsInst.Name, + ID: PBSMonitorResourceID(pbsInst.Name), Name: pbsInst.Name, Host: pbsInst.Host, GuestURL: pbsInst.GuestURL, diff --git a/internal/monitoring/monitor_pbs_coverage_test.go b/internal/monitoring/monitor_pbs_coverage_test.go index 3ed910a6c..d008a8209 100644 --- a/internal/monitoring/monitor_pbs_coverage_test.go +++ b/internal/monitoring/monitor_pbs_coverage_test.go @@ -90,6 +90,9 @@ func TestPollPBSInstanceDoesNotQueryExcludedDatastoreDetails(t *testing.T) { if len(snapshot.PBSInstances) != 1 { t.Fatalf("PBS instances = %+v, want one", snapshot.PBSInstances) } + if snapshot.PBSInstances[0].ID != PBSMonitorResourceID("pbs-excludes") { + t.Fatalf("PBS runtime identity = %q, want canonical monitor identity", snapshot.PBSInstances[0].ID) + } datastores := snapshot.PBSInstances[0].Datastores if len(datastores) != 1 || datastores[0].Name != "internal" { t.Fatalf("datastores = %+v, want only internal", datastores) diff --git a/internal/monitoring/monitor_pbs_pmg.go b/internal/monitoring/monitor_pbs_pmg.go index 4d614b8d9..6021ed993 100644 --- a/internal/monitoring/monitor_pbs_pmg.go +++ b/internal/monitoring/monitor_pbs_pmg.go @@ -241,7 +241,7 @@ func (m *Monitor) pollPBSInstance(ctx context.Context, instanceName string, clie // Initialize PBS instance with default values pbsInst = models.PBSInstance{ - ID: "pbs-" + instanceName, + ID: PBSMonitorResourceID(instanceName), Name: instanceName, Host: instanceCfg.Host, GuestURL: instanceCfg.GuestURL, @@ -484,7 +484,7 @@ func (m *Monitor) pollPBSInstance(ctx context.Context, instanceName string, clie AliasIDs: []string{pbsInst.ID + "/" + ds.Name}, Name: ds.Name, Node: instanceName, // Use PBS instance name as "node" - Instance: "pbs-" + instanceName, + Instance: PBSMonitorResourceID(instanceName), Type: "pbs", Status: ds.Status, Total: ds.Total, @@ -694,7 +694,7 @@ func (m *Monitor) pollPMGInstance(ctx context.Context, instanceName string, clie now := time.Now() pmgInst := models.PMGInstance{ - ID: "pmg-" + instanceName, + ID: PMGMonitorResourceID(instanceName), Name: instanceName, Host: instanceCfg.Host, GuestURL: instanceCfg.GuestURL, diff --git a/internal/monitoring/platform_instance_identity.go b/internal/monitoring/platform_instance_identity.go new file mode 100644 index 000000000..5c747f448 --- /dev/null +++ b/internal/monitoring/platform_instance_identity.go @@ -0,0 +1,13 @@ +package monitoring + +// PBSMonitorResourceID is the stable runtime identity used by PBS state, +// alerts, and canonical-resource alias resolution for one configured instance. +func PBSMonitorResourceID(instanceName string) string { + return "pbs-" + instanceName +} + +// PMGMonitorResourceID is the stable runtime identity used by PMG state, +// alerts, and canonical-resource alias resolution for one configured instance. +func PMGMonitorResourceID(instanceName string) string { + return "pmg-" + instanceName +}