fix(site-secrets): report every site node's secret mirror failures on the health report (M1)
A DeployedSecretsMirror failure on the STANDBY node (e.g. a master-key mismatch
that stops a deployed secret decrypting there) was only logged on that node:
site health reports are sent by the active node only, so central never saw it.
- New 12th LocalDb-replicated table secret_mirror_status (node_name PK,
failures_json of {SecretName, Message}, checked_at; no FKs). Each node's
mirror writes only its own row, and only when its failure set changes plus
once per start-up (every write replicates via CDC). Best-effort: a failed
write is a Warning and is retried next pass. Node key = INodeIdentityProvider
NodeName, falling back to the machine name.
- SecretMirrorStatusReporter (SiteRuntime, every node, 30 s) reads every row
into ISiteHealthCollector.SetSecretMirrorStatuses -> additive nullable
SiteHealthReport.SecretMirrorStatuses (null = no data).
- Wire: SiteHealthReportDto.secret_mirror_statuses = 33 (presence wrapper so
null != empty); vendored CentralControl.cs regenerated; mapper both ways.
- Central UI Health card: 'Secret mirror failing' badge + compact per-node
node / secret / message list.
- Tests: mirror status write-on-change/first-pass/failure isolation, storage
round-trip, reporter/collector, DTO mapper round-trip incl. null/empty,
bUnit, replicated-table set (twelve) + FK guard wording.
This commit is contained in:
@@ -1,7 +1,9 @@
|
||||
using Microsoft.Extensions.Hosting;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Microsoft.Extensions.Options;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Messages.Artifacts;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Messages.Health;
|
||||
using ZB.MOM.WW.ScadaBridge.SiteRuntime.Persistence;
|
||||
using ZB.MOM.WW.Secrets.Abstractions;
|
||||
|
||||
@@ -153,6 +155,18 @@ public sealed record MirrorResult(
|
||||
/// so a LATER failure — even one with an identical fingerprint — logs at Error again. This state
|
||||
/// is in-memory only (per node process), which matches the ledger's own node-local scope.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Status row for central (review finding M1).</b> A failure on the STANDBY node used to be
|
||||
/// visible only in that node's own log — site health reports are sent by the active node only.
|
||||
/// After each pass this mirror records its node's current failure set (secret name + message,
|
||||
/// never a value) in the LocalDb-replicated <c>secret_mirror_status</c> table, keyed by node
|
||||
/// name; <c>SecretMirrorStatusReporter</c> on the active node reads every node's row onto the
|
||||
/// health report. Because every write replicates via CDC, the row is written only when the
|
||||
/// failure set (compared as sorted name+message pairs) differs from what this process last
|
||||
/// wrote — plus once on the first pass after start-up, so a restart refreshes a stale row. The
|
||||
/// write is best-effort: a failure logs a Warning (repeats at Debug) and is retried on the next
|
||||
/// pass, and never affects the pass's own <see cref="MirrorResult"/>.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
public sealed class DeployedSecretsMirror : BackgroundService
|
||||
{
|
||||
@@ -165,6 +179,17 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
private readonly IOptions<SiteRuntimeOptions> _options;
|
||||
private readonly ILogger<DeployedSecretsMirror> _logger;
|
||||
private readonly TimeProvider _time;
|
||||
private readonly string _nodeName;
|
||||
|
||||
/// <summary>
|
||||
/// The failure set (sorted name+message pairs) this process last wrote successfully to
|
||||
/// <c>secret_mirror_status</c>, or <see langword="null"/> before the first successful write
|
||||
/// — which is what forces the once-per-start-up write (review finding M1).
|
||||
/// </summary>
|
||||
private IReadOnlyList<SecretMirrorFailure>? _lastWrittenStatus;
|
||||
|
||||
/// <summary>Whether a failed status write has already been logged at Warning (de-spam).</summary>
|
||||
private bool _statusWriteFailureLogged;
|
||||
|
||||
private readonly SemaphoreSlim _gate = new(1, 1);
|
||||
private bool _disposed;
|
||||
@@ -199,6 +224,11 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
/// <param name="options">Supplies <see cref="SiteRuntimeOptions.DeployedSecretsMirrorInterval"/>.</param>
|
||||
/// <param name="logger">Logger instance. Never logs secret values — names, revisions, and KEK ids only.</param>
|
||||
/// <param name="time">Clock driving the reconcile loop's <see cref="PeriodicTimer"/> and the tombstone-recovery stamp.</param>
|
||||
/// <param name="nodeIdentity">
|
||||
/// Supplies this node's name (<c>node-a</c>/<c>node-b</c>), the key of its
|
||||
/// <c>secret_mirror_status</c> row. Optional: when absent or unconfigured, the machine name
|
||||
/// is used instead.
|
||||
/// </param>
|
||||
public DeployedSecretsMirror(
|
||||
SiteStorageService storage,
|
||||
ISecretStore store,
|
||||
@@ -206,7 +236,8 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
ISecretCacheInvalidator? invalidator,
|
||||
IOptions<SiteRuntimeOptions> options,
|
||||
ILogger<DeployedSecretsMirror> logger,
|
||||
TimeProvider time)
|
||||
TimeProvider time,
|
||||
INodeIdentityProvider? nodeIdentity = null)
|
||||
{
|
||||
_storage = storage ?? throw new ArgumentNullException(nameof(storage));
|
||||
_store = store ?? throw new ArgumentNullException(nameof(store));
|
||||
@@ -215,6 +246,7 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
_options = options ?? throw new ArgumentNullException(nameof(options));
|
||||
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
|
||||
_time = time ?? throw new ArgumentNullException(nameof(time));
|
||||
_nodeName = nodeIdentity?.NodeName ?? Environment.MachineName;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -314,6 +346,8 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
rows.Count(r => !r.IsRemoved) + unreadable.Count(u => !u.IsRemoved), failed.Count);
|
||||
}
|
||||
|
||||
await RecordStatusIfChangedAsync(failedNames, failed).ConfigureAwait(false);
|
||||
|
||||
return result;
|
||||
}
|
||||
finally
|
||||
@@ -322,6 +356,42 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Writes this node's <c>secret_mirror_status</c> row when the pass's failure set differs
|
||||
/// from the last one this process wrote (or nothing has been written yet since start-up).
|
||||
/// Best-effort: never throws, so the pass result is unaffected by a status-write fault.
|
||||
/// See the class remarks (review finding M1).
|
||||
/// </summary>
|
||||
private async Task RecordStatusIfChangedAsync(IReadOnlyList<string> failedNames, IReadOnlyList<string> failed)
|
||||
{
|
||||
var current = failedNames
|
||||
.Select((name, i) => new SecretMirrorFailure(name, failed[i]))
|
||||
.OrderBy(f => f.SecretName, StringComparer.Ordinal)
|
||||
.ThenBy(f => f.Message, StringComparer.Ordinal)
|
||||
.ToList();
|
||||
|
||||
if (_lastWrittenStatus is not null && _lastWrittenStatus.SequenceEqual(current))
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
await _storage.SetSecretMirrorStatusAsync(_nodeName, current, _time.GetUtcNow()).ConfigureAwait(false);
|
||||
_lastWrittenStatus = current;
|
||||
_statusWriteFailureLogged = false;
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger.Log(
|
||||
_statusWriteFailureLogged ? LogLevel.Debug : LogLevel.Warning,
|
||||
ex,
|
||||
"Failed to record the deployed secrets mirror status for node {NodeName}; retrying next pass",
|
||||
_nodeName);
|
||||
_statusWriteFailureLogged = true;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Handles a tombstoned <c>deployed_secrets</c> row: deletes it from this node's store, but
|
||||
/// only if this node had previously ledgered it as mirrored — an out-of-band, same-named
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
using Microsoft.Extensions.Hosting;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using ZB.MOM.WW.ScadaBridge.HealthMonitoring;
|
||||
using ZB.MOM.WW.ScadaBridge.SiteRuntime.Persistence;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Secrets;
|
||||
|
||||
/// <summary>
|
||||
/// Review finding M1: site-side hosted service that periodically reads EVERY node's row from
|
||||
/// the LocalDb-replicated <c>secret_mirror_status</c> table and pushes the list into
|
||||
/// <see cref="ISiteHealthCollector.SetSecretMirrorStatuses"/>, so the next health report
|
||||
/// carries each site node's deployed-secrets mirror failures — the standby's included.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// Runs on every site node (the read is a cheap local SELECT), but only the active node's
|
||||
/// report is ever sent (<c>HealthReportSender</c>), which is exactly why the status lives in a
|
||||
/// REPLICATED table: the standby writes its row, CDC carries it to the active node, and this
|
||||
/// reporter on the active node forwards it. A site pair WITHOUT LocalDb replication reports
|
||||
/// only the active node's row — the same prerequisite the deployed secrets themselves have.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Lives in SiteRuntime (which already references HealthMonitoring and owns
|
||||
/// <see cref="SiteStorageService"/>), following <c>SiteStreamAlarmDropReporter</c>: immediate
|
||||
/// first probe, fixed cadence, exceptions logged and swallowed. A failed read leaves the
|
||||
/// previous snapshot on the collector rather than clearing it.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
public sealed class SecretMirrorStatusReporter : BackgroundService
|
||||
{
|
||||
/// <summary>Default poll cadence (30 s), matching the site health report interval.</summary>
|
||||
internal static readonly TimeSpan DefaultPollInterval = TimeSpan.FromSeconds(30);
|
||||
|
||||
private readonly ISiteHealthCollector _collector;
|
||||
private readonly SiteStorageService _storage;
|
||||
private readonly ILogger<SecretMirrorStatusReporter> _logger;
|
||||
private readonly TimeSpan _pollInterval;
|
||||
|
||||
/// <summary>Initializes a new instance of <see cref="SecretMirrorStatusReporter"/>.</summary>
|
||||
/// <param name="collector">The site health collector receiving the statuses.</param>
|
||||
/// <param name="storage">Site storage exposing <c>secret_mirror_status</c>.</param>
|
||||
/// <param name="logger">Logger instance.</param>
|
||||
/// <param name="pollInterval">Poll interval override; defaults to <see cref="DefaultPollInterval"/> (30 s).</param>
|
||||
public SecretMirrorStatusReporter(
|
||||
ISiteHealthCollector collector,
|
||||
SiteStorageService storage,
|
||||
ILogger<SecretMirrorStatusReporter> logger,
|
||||
TimeSpan? pollInterval = null)
|
||||
{
|
||||
_collector = collector ?? throw new ArgumentNullException(nameof(collector));
|
||||
_storage = storage ?? throw new ArgumentNullException(nameof(storage));
|
||||
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
|
||||
_pollInterval = pollInterval ?? DefaultPollInterval;
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
protected override async Task ExecuteAsync(CancellationToken stoppingToken)
|
||||
{
|
||||
await ProbeAsync().ConfigureAwait(false);
|
||||
|
||||
while (!stoppingToken.IsCancellationRequested)
|
||||
{
|
||||
try
|
||||
{
|
||||
await Task.Delay(_pollInterval, stoppingToken).ConfigureAwait(false);
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
{
|
||||
break;
|
||||
}
|
||||
|
||||
await ProbeAsync().ConfigureAwait(false);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Reads every node's mirror status and pushes it onto the collector; never throws.
|
||||
/// Public so a test can drive one deterministic probe.
|
||||
/// </summary>
|
||||
/// <returns>A task that completes when the probe has finished.</returns>
|
||||
public async Task ProbeAsync()
|
||||
{
|
||||
try
|
||||
{
|
||||
var statuses = await _storage.GetSecretMirrorStatusesAsync().ConfigureAwait(false);
|
||||
_collector.SetSecretMirrorStatuses(statuses);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
// Catch-all is deliberate: health reporting must never take the node down, and
|
||||
// the loop must survive so the next tick retries.
|
||||
_logger.LogWarning(ex, "SecretMirrorStatusReporter probe failed; next tick will retry.");
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user