test(site): pin the active-node notification-config purge; scope the guarded write
Task 12 + Task 13. No production behaviour change in either. Task 12: DeploymentManagerActor.HandleDeployArtifacts already purges notification_lists and smtp_configurations on every artifact apply, but nothing pinned the actor's CALL to it — ArtifactStorageTests covers the storage method only. Task 15 deletes SiteReplicationActor's copy, making this the sole remaining call site, and Task 16 edits this actor's wiring; dropping the call would leave plaintext SMTP passwords on disk with every suite still green. Verified red-first by commenting the call out: the pin fails with that message. Task 13: StoreDeployedConfigIfNewerAsync STAYS. Re-verified both callers — SiteReplicationActor:375 (dies at Task 15) and SiteReconciliationActor:166 (survives). Reconciliation is a per-node startup self-heal against central whose fetch races real deploys, so the deployed_at guard still does real work there. Doc comment rewritten to say so, and to warn against porting the guard onto the replication path where it would fight the HLC rather than help it. Also corrected a stale 'guarded standby write' section header in the tests. Re-ran the Task 13 step 2 scope check: ConfigFetchRetryCount's only production reader remains SiteReplicationActor:157, so its option + validator rule stay until Task 17, after Task 15 deletes the actor. Verified: build 0 warnings; SiteRuntime 533, Host 329, StoreAndForward 153, LocalDb integration 16 — all pass. Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
This commit is contained in:
@@ -156,9 +156,22 @@ public class SiteStorageService
|
||||
/// clause so the guard is atomic with no application-level read-modify-write.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// This is the standby-node write path for replicated configs. The active-node
|
||||
/// apply path (<see cref="StoreDeployedConfigAsync"/>) remains unguarded and always
|
||||
/// overwrites, because the active node's write is always authoritative.
|
||||
/// <b>This is the reconciliation write path.</b> <c>SiteReconciliationActor</c> runs a
|
||||
/// per-node startup self-heal against central: it asks central what this node should be
|
||||
/// running and fetches anything missing. That fetch races real deploys, so the
|
||||
/// <c>deployed_at</c> guard is what stops a slow reconcile response from overwriting a
|
||||
/// newer config that landed while it was in flight. The active-node apply path
|
||||
/// (<see cref="StoreDeployedConfigAsync"/>) remains unguarded and always overwrites,
|
||||
/// because a deploy is always authoritative.
|
||||
/// <para>
|
||||
/// It was originally the <i>standby</i> write path as well, under notify-and-fetch: the
|
||||
/// standby was told a deploy had happened and fetched the config itself. LocalDb Phase 2
|
||||
/// replaced that with change-data-capture — the config row simply replicates — so the
|
||||
/// standby no longer writes here at all, and last-writer-wins on the primary key (not
|
||||
/// this guard) is what orders concurrent writes between the two nodes. Reconciliation is
|
||||
/// the reason the method survives; do not port the <c>deployed_at</c> guard onto the
|
||||
/// replication path, where it would fight the HLC rather than help it.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <paramref name="deployedAtOverride"/> is exposed for testing so that the exact
|
||||
/// <c>deployed_at</c> value can be controlled without sleeping between calls.
|
||||
|
||||
@@ -2,6 +2,7 @@ using Akka.Actor;
|
||||
using Akka.TestKit.Xunit2;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Messages.Artifacts;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Messages.Deployment;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Messages.DebugView;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Messages.InboundApi;
|
||||
@@ -883,6 +884,87 @@ public class DeploymentManagerActorTests : TestKit, IDisposable
|
||||
Assert.Equal("SenderPump", response.InstanceUniqueName);
|
||||
}
|
||||
|
||||
// ── LocalDb Phase 2 / Task 12: the active-node central-only purge ──
|
||||
|
||||
[Fact]
|
||||
public async Task ApplyingArtifacts_PurgesCentralOnlyNotificationConfig()
|
||||
{
|
||||
// Security cleanup. notification_lists and smtp_configurations can hold plaintext
|
||||
// SMTP passwords written by a pre-2026-07-10 build, and the ACTIVE node's artifact
|
||||
// apply is what clears them (DeploymentManagerActor.HandleDeployArtifacts). The
|
||||
// standby's copy of this call lives in SiteReplicationActor and dies with it at
|
||||
// Task 15, which makes this call site the ONLY remaining one.
|
||||
//
|
||||
// Nothing pins it today: ArtifactStorageTests covers the storage method, not the
|
||||
// actor's call to it, so Task 16's edits to this actor could drop the call and every
|
||||
// suite would stay green. That is precisely the kind of silent security regression
|
||||
// this test exists to prevent — verified red-first by commenting out the call.
|
||||
await SeedCentralOnlyRowsAsync();
|
||||
Assert.Equal(1, await RowCountAsync("notification_lists"));
|
||||
Assert.Equal(1, await RowCountAsync("smtp_configurations"));
|
||||
|
||||
var manager = CreateDeploymentManager();
|
||||
manager.Tell(new DeployArtifactsCommand(
|
||||
DeploymentId: "dep-purge-1",
|
||||
SharedScripts: null,
|
||||
ExternalSystems: null,
|
||||
DatabaseConnections: null,
|
||||
NotificationLists: null,
|
||||
DataConnections: null,
|
||||
SmtpConfigurations: null,
|
||||
Timestamp: DateTimeOffset.UtcNow));
|
||||
|
||||
// The apply runs on a Task.Run inside the actor, so poll rather than assert once.
|
||||
await AwaitPurgedAsync();
|
||||
}
|
||||
|
||||
private async Task SeedCentralOnlyRowsAsync()
|
||||
{
|
||||
// Seeded through the service's own (already-open) LocalDb connection — a raw
|
||||
// SqliteConnection would lack the pragmas and the zb_hlc_next() UDF the site
|
||||
// tables' capture triggers call. Raw SQL is the only way these rows can exist at
|
||||
// all now that the site-side write paths are gone.
|
||||
await using var connection = _storage.CreateConnection();
|
||||
await using var command = connection.CreateCommand();
|
||||
command.CommandText = """
|
||||
INSERT INTO notification_lists (name, recipient_emails, updated_at)
|
||||
VALUES ('Ops Team', '["ops@example.com"]', @u);
|
||||
INSERT INTO smtp_configurations
|
||||
(name, server, port, auth_mode, from_address, username, password, oauth_config, updated_at)
|
||||
VALUES ('smtp.example.com:587', 'smtp.example.com', 587, 'BasicAuth',
|
||||
'noreply@example.com', 'smtpuser', 'PLAINTEXT-SECRET', NULL, @u);
|
||||
""";
|
||||
command.Parameters.AddWithValue("@u", DateTimeOffset.UtcNow.ToString("O"));
|
||||
await command.ExecuteNonQueryAsync();
|
||||
}
|
||||
|
||||
private async Task<long> RowCountAsync(string table)
|
||||
{
|
||||
await using var connection = _storage.CreateConnection();
|
||||
await using var command = connection.CreateCommand();
|
||||
command.CommandText = $"SELECT COUNT(*) FROM {table}";
|
||||
return (long)(await command.ExecuteScalarAsync())!;
|
||||
}
|
||||
|
||||
private async Task AwaitPurgedAsync()
|
||||
{
|
||||
var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(10);
|
||||
while (DateTime.UtcNow < deadline)
|
||||
{
|
||||
if (await RowCountAsync("notification_lists") == 0 &&
|
||||
await RowCountAsync("smtp_configurations") == 0)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
await Task.Delay(50);
|
||||
}
|
||||
|
||||
Assert.Fail(
|
||||
"Artifact apply did not purge the central-only notification/SMTP rows. " +
|
||||
"The plaintext SMTP password is still on disk.");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// In-test fake <see cref="IDeploymentConfigFetcher"/>: returns a canned config JSON
|
||||
/// (notify-and-fetch success) or throws a canned exception (fetch failure), and records
|
||||
|
||||
+6
-1
@@ -226,7 +226,12 @@ public class SiteStorageServiceTests : IAsyncLifetime, IDisposable
|
||||
Assert.Empty(overrides);
|
||||
}
|
||||
|
||||
// ── Task 13: StoreDeployedConfigIfNewerAsync (guarded standby write) ──
|
||||
// ── StoreDeployedConfigIfNewerAsync (the deployed_at-guarded write) ──
|
||||
//
|
||||
// Originally the standby's notify-and-fetch write path. LocalDb Phase 2 replaced that
|
||||
// with change-data-capture, so the surviving caller is SiteReconciliationActor's
|
||||
// startup self-heal against central, where the guard still stops a slow reconcile
|
||||
// response from overwriting a newer config that landed while it was in flight.
|
||||
|
||||
/// <summary>
|
||||
/// Seeds a deployed_configurations row with an explicit deployed_at timestamp using the same
|
||||
|
||||
Reference in New Issue
Block a user