fix(site-secrets): prune secret_mirror_status rows of decommissioned or renamed nodes
Each node now re-writes its mirror-status row every SecretMirrorStatusRefreshInterval (10 min) as a heartbeat; SecretMirrorStatusReporter leaves off the health report and deletes any row not refreshed for SecretMirrorStatusStaleAfter (1 h, validated >= 2x the refresh). The delete is conditional on the checked_at it read, so a node that came back keeps its fresh row; a returning node re-writes its row on its first pass.
This commit is contained in:
@@ -13,7 +13,7 @@ namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.Health;
|
||||
/// and human-readable messages only — never a secret value or ciphertext.
|
||||
/// </remarks>
|
||||
/// <param name="NodeName">The reporting node's semantic name (<c>node-a</c>/<c>node-b</c>), or its host name when unconfigured.</param>
|
||||
/// <param name="CheckedAtUtc">When that node last recorded a CHANGE in its failure set (or its first pass after start-up) — not the time of its latest pass.</param>
|
||||
/// <param name="CheckedAtUtc">When that node last wrote its row: on a CHANGE in its failure set, on its first pass after start-up, or on the periodic refresh (<c>SiteRuntime:SecretMirrorStatusRefreshInterval</c>, 10 min) — not the time of its latest pass. Rows older than <c>SecretMirrorStatusStaleAfter</c> are pruned.</param>
|
||||
/// <param name="Failures">That node's current mirror failures; empty when the node's mirror is healthy.</param>
|
||||
public sealed record SecretMirrorNodeStatus(
|
||||
string NodeName,
|
||||
|
||||
@@ -1006,6 +1006,30 @@ public class SiteStorageService
|
||||
return results;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Deletes <paramref name="nodeName"/>'s <c>secret_mirror_status</c> row, but only while it
|
||||
/// still carries <paramref name="checkedAtUtc"/> — so a row the node re-wrote after the caller
|
||||
/// read it (it came back, or refreshed just in time) is never deleted. The delete replicates
|
||||
/// like any other write.
|
||||
/// </summary>
|
||||
/// <param name="nodeName">The row key.</param>
|
||||
/// <param name="checkedAtUtc">The <c>checked_at</c> the caller read and judged stale.</param>
|
||||
/// <returns><see langword="true"/> when the row was deleted.</returns>
|
||||
public async Task<bool> DeleteSecretMirrorStatusIfUnchangedAsync(string nodeName, DateTimeOffset checkedAtUtc)
|
||||
{
|
||||
ArgumentException.ThrowIfNullOrWhiteSpace(nodeName);
|
||||
|
||||
await using var connection = OpenConnection();
|
||||
|
||||
await using var command = connection.CreateCommand();
|
||||
command.CommandText =
|
||||
"DELETE FROM secret_mirror_status WHERE node_name = @nodeName AND checked_at = @checkedAt";
|
||||
command.Parameters.AddWithValue("@nodeName", nodeName);
|
||||
command.Parameters.AddWithValue("@checkedAt", checkedAtUtc.ToUniversalTime().ToString("O"));
|
||||
|
||||
return await command.ExecuteNonQueryAsync() > 0;
|
||||
}
|
||||
|
||||
// ── Shared Script CRUD ──
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -163,7 +163,10 @@ public sealed record MirrorResult(
|
||||
/// 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
|
||||
/// wrote — plus once on the first pass after start-up, so a restart refreshes a stale row, and
|
||||
/// at least every <see cref="SiteRuntimeOptions.SecretMirrorStatusRefreshInterval"/> so the
|
||||
/// row's <c>checked_at</c> works as a heartbeat: <c>SecretMirrorStatusReporter</c> prunes the
|
||||
/// row of a node that has stopped refreshing it (decommissioned or renamed). 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>
|
||||
@@ -188,6 +191,9 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
/// </summary>
|
||||
private IReadOnlyList<SecretMirrorFailure>? _lastWrittenStatus;
|
||||
|
||||
/// <summary>When this process last wrote its <c>secret_mirror_status</c> row (heartbeat refresh).</summary>
|
||||
private DateTimeOffset _lastWrittenStatusAt;
|
||||
|
||||
/// <summary>Whether a failed status write has already been logged at Warning (de-spam).</summary>
|
||||
private bool _statusWriteFailureLogged;
|
||||
|
||||
@@ -358,7 +364,8 @@ 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).
|
||||
/// from the last one this process wrote, nothing has been written yet since start-up, or the
|
||||
/// last write is older than <see cref="SiteRuntimeOptions.SecretMirrorStatusRefreshInterval"/>.
|
||||
/// Best-effort: never throws, so the pass result is unaffected by a status-write fault.
|
||||
/// See the class remarks (review finding M1).
|
||||
/// </summary>
|
||||
@@ -370,15 +377,19 @@ public sealed class DeployedSecretsMirror : BackgroundService
|
||||
.ThenBy(f => f.Message, StringComparer.Ordinal)
|
||||
.ToList();
|
||||
|
||||
if (_lastWrittenStatus is not null && _lastWrittenStatus.SequenceEqual(current))
|
||||
var now = _time.GetUtcNow();
|
||||
if (_lastWrittenStatus is not null
|
||||
&& _lastWrittenStatus.SequenceEqual(current)
|
||||
&& now - _lastWrittenStatusAt < _options.Value.SecretMirrorStatusRefreshInterval)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
await _storage.SetSecretMirrorStatusAsync(_nodeName, current, _time.GetUtcNow()).ConfigureAwait(false);
|
||||
await _storage.SetSecretMirrorStatusAsync(_nodeName, current, now).ConfigureAwait(false);
|
||||
_lastWrittenStatus = current;
|
||||
_lastWrittenStatusAt = now;
|
||||
_statusWriteFailureLogged = false;
|
||||
}
|
||||
catch (Exception ex)
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
using Microsoft.Extensions.Hosting;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Messages.Health;
|
||||
using ZB.MOM.WW.ScadaBridge.HealthMonitoring;
|
||||
using ZB.MOM.WW.ScadaBridge.SiteRuntime.Persistence;
|
||||
|
||||
@@ -25,6 +26,15 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Secrets;
|
||||
/// first probe, fixed cadence, exceptions logged and swallowed. A failed read leaves the
|
||||
/// previous snapshot on the collector rather than clearing it.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Stale rows.</b> Every live node re-writes its row at least every
|
||||
/// <see cref="SiteRuntimeOptions.SecretMirrorStatusRefreshInterval"/>, so a row whose
|
||||
/// <c>checked_at</c> is older than <see cref="SiteRuntimeOptions.SecretMirrorStatusStaleAfter"/>
|
||||
/// belongs to a node that is no longer running its mirror — decommissioned, renamed (the row is
|
||||
/// keyed by node name), or down for that long. Such a row is left off the report and deleted
|
||||
/// (only while it still carries the stale <c>checked_at</c>, so a node that came back in the
|
||||
/// meantime keeps its fresh row); a returning node writes its row again on its first pass.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
public sealed class SecretMirrorStatusReporter : BackgroundService
|
||||
{
|
||||
@@ -35,22 +45,33 @@ public sealed class SecretMirrorStatusReporter : BackgroundService
|
||||
private readonly SiteStorageService _storage;
|
||||
private readonly ILogger<SecretMirrorStatusReporter> _logger;
|
||||
private readonly TimeSpan _pollInterval;
|
||||
private readonly TimeSpan _staleAfter;
|
||||
private readonly TimeProvider _time;
|
||||
|
||||
/// <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>
|
||||
/// <param name="staleAfter">
|
||||
/// Age after which a row is pruned; defaults to the
|
||||
/// <see cref="SiteRuntimeOptions.SecretMirrorStatusStaleAfter"/> default (1 hour).
|
||||
/// </param>
|
||||
/// <param name="time">Clock used to age rows; defaults to <see cref="TimeProvider.System"/>.</param>
|
||||
public SecretMirrorStatusReporter(
|
||||
ISiteHealthCollector collector,
|
||||
SiteStorageService storage,
|
||||
ILogger<SecretMirrorStatusReporter> logger,
|
||||
TimeSpan? pollInterval = null)
|
||||
TimeSpan? pollInterval = null,
|
||||
TimeSpan? staleAfter = null,
|
||||
TimeProvider? time = 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;
|
||||
_staleAfter = staleAfter ?? new SiteRuntimeOptions().SecretMirrorStatusStaleAfter;
|
||||
_time = time ?? TimeProvider.System;
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
@@ -74,7 +95,8 @@ public sealed class SecretMirrorStatusReporter : BackgroundService
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Reads every node's mirror status and pushes it onto the collector; never throws.
|
||||
/// Reads every node's mirror status, prunes rows older than the stale threshold, and pushes
|
||||
/// the rest 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>
|
||||
@@ -83,7 +105,20 @@ public sealed class SecretMirrorStatusReporter : BackgroundService
|
||||
try
|
||||
{
|
||||
var statuses = await _storage.GetSecretMirrorStatusesAsync().ConfigureAwait(false);
|
||||
_collector.SetSecretMirrorStatuses(statuses);
|
||||
var cutoff = _time.GetUtcNow() - _staleAfter;
|
||||
var live = new List<SecretMirrorNodeStatus>(statuses.Count);
|
||||
foreach (var status in statuses)
|
||||
{
|
||||
if (status.CheckedAtUtc >= cutoff)
|
||||
{
|
||||
live.Add(status);
|
||||
continue;
|
||||
}
|
||||
|
||||
await PruneAsync(status).ConfigureAwait(false);
|
||||
}
|
||||
|
||||
_collector.SetSecretMirrorStatuses(live);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
@@ -92,4 +127,30 @@ public sealed class SecretMirrorStatusReporter : BackgroundService
|
||||
_logger.LogWarning(ex, "SecretMirrorStatusReporter probe failed; next tick will retry.");
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Deletes one stale row; a failed delete is logged and retried on the next probe (the row
|
||||
/// stays off the report either way).
|
||||
/// </summary>
|
||||
private async Task PruneAsync(SecretMirrorNodeStatus status)
|
||||
{
|
||||
try
|
||||
{
|
||||
if (await _storage.DeleteSecretMirrorStatusIfUnchangedAsync(status.NodeName, status.CheckedAtUtc)
|
||||
.ConfigureAwait(false))
|
||||
{
|
||||
_logger.LogInformation(
|
||||
"Removed the secret mirror status row for node {NodeName}: not refreshed since {CheckedAtUtc:O} " +
|
||||
"(older than {StaleAfter}); the node is decommissioned, renamed or down. It is re-created " +
|
||||
"on the node's next mirror pass if it returns.",
|
||||
status.NodeName, status.CheckedAtUtc, _staleAfter);
|
||||
}
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger.LogWarning(ex,
|
||||
"Failed to remove the stale secret mirror status row for node {NodeName}; next tick will retry.",
|
||||
status.NodeName);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -145,7 +145,9 @@ public static class ServiceCollectionExtensions
|
||||
services.AddHostedService(sp => new SecretMirrorStatusReporter(
|
||||
sp.GetRequiredService<HealthMonitoring.ISiteHealthCollector>(),
|
||||
sp.GetRequiredService<SiteStorageService>(),
|
||||
sp.GetRequiredService<ILogger<SecretMirrorStatusReporter>>()));
|
||||
sp.GetRequiredService<ILogger<SecretMirrorStatusReporter>>(),
|
||||
staleAfter: sp.GetRequiredService<IOptions<SiteRuntimeOptions>>().Value.SecretMirrorStatusStaleAfter,
|
||||
time: sp.GetRequiredService<TimeProvider>()));
|
||||
|
||||
return services;
|
||||
}
|
||||
|
||||
@@ -274,4 +274,26 @@ public class SiteRuntimeOptions
|
||||
/// cadence. Default: 15 seconds.
|
||||
/// </summary>
|
||||
public TimeSpan DeployedSecretsMirrorInterval { get; set; } = TimeSpan.FromSeconds(15);
|
||||
|
||||
/// <summary>
|
||||
/// How often <see cref="Secrets.DeployedSecretsMirror"/> re-writes this node's
|
||||
/// <c>secret_mirror_status</c> row even when its failure set has not changed — the row's
|
||||
/// <c>checked_at</c> doubles as a liveness heartbeat, so a row that stops being refreshed
|
||||
/// can be recognised as belonging to a node that is gone (decommissioned, renamed, or down)
|
||||
/// and pruned by <see cref="Secrets.SecretMirrorStatusReporter"/> after
|
||||
/// <see cref="SecretMirrorStatusStaleAfter"/>. Each refresh is one replicated row write.
|
||||
/// Default: 10 minutes.
|
||||
/// </summary>
|
||||
public TimeSpan SecretMirrorStatusRefreshInterval { get; set; } = TimeSpan.FromMinutes(10);
|
||||
|
||||
/// <summary>
|
||||
/// Age (by <c>checked_at</c>) after which a <c>secret_mirror_status</c> row is treated as
|
||||
/// belonging to a node that no longer exists: <see cref="Secrets.SecretMirrorStatusReporter"/>
|
||||
/// leaves it off the health report and deletes it (the delete replicates like any other
|
||||
/// write). A live node re-writes its row every <see cref="SecretMirrorStatusRefreshInterval"/>
|
||||
/// and once at start-up, so only a node that has stopped running its mirror for this long
|
||||
/// loses its row — and gets it back on its first pass after it returns. Must be at least
|
||||
/// twice <see cref="SecretMirrorStatusRefreshInterval"/>. Default: 1 hour.
|
||||
/// </summary>
|
||||
public TimeSpan SecretMirrorStatusStaleAfter { get; set; } = TimeSpan.FromHours(1);
|
||||
}
|
||||
|
||||
@@ -136,5 +136,13 @@ public sealed class SiteRuntimeOptionsValidator : OptionsValidatorBase<SiteRunti
|
||||
$"ScadaBridge:SiteRuntime:DeployedSecretsMirrorInterval must be a positive duration " +
|
||||
$"(was {options.DeployedSecretsMirrorInterval}); it is the DeployedSecretsMirror " +
|
||||
"reconcile PeriodicTimer period.");
|
||||
builder.RequireThat(options.SecretMirrorStatusRefreshInterval > TimeSpan.Zero,
|
||||
$"ScadaBridge:SiteRuntime:SecretMirrorStatusRefreshInterval must be a positive duration " +
|
||||
$"(was {options.SecretMirrorStatusRefreshInterval}); it is how often each node re-writes its " +
|
||||
"secret_mirror_status row.");
|
||||
builder.RequireThat(options.SecretMirrorStatusStaleAfter >= options.SecretMirrorStatusRefreshInterval * 2,
|
||||
$"ScadaBridge:SiteRuntime:SecretMirrorStatusStaleAfter (was {options.SecretMirrorStatusStaleAfter}) must be " +
|
||||
$"at least twice SecretMirrorStatusRefreshInterval (was {options.SecretMirrorStatusRefreshInterval}); " +
|
||||
"otherwise a live node's row could be pruned between two refreshes.");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user