Merge branch 'worktree-agent-a95eaaa8a3181ad4c' into arch-review-remediation
This commit is contained in:
Joseph Doherty
@@ -0,0 +1,227 @@
|
||||
using Microsoft.Extensions.Configuration;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using ZB.MOM.WW.LocalDb;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// WP1.3 — CDC capture is installed only on a node that actually replicates.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// These assert on the TRIGGERS in <c>sqlite_master</c> rather than on
|
||||
/// <c>ILocalDb.ReplicatedTables</c>, because the trigger set is what costs anything: the
|
||||
/// registry entry is a dictionary lookup, while each trigger runs two extra INSERTs plus a
|
||||
/// <c>json_object</c> serialization of the full row inside every write transaction on the
|
||||
/// table. An unreplicated node paid that on every store-and-forward enqueue, every static
|
||||
/// override, every event log row, forever, for an oplog with no reader.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// The naming <c>__localdb_{table}_{ai|au|ad}</c> is the library's
|
||||
/// (<c>TriggerSqlGenerator.TriggerName</c>), matched by prefix here so a fourth trigger kind
|
||||
/// would be caught rather than quietly ignored.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
public class SiteLocalDbCdcRegistrationTests : IDisposable
|
||||
{
|
||||
private readonly string _root;
|
||||
private readonly List<ServiceProvider> _providers = [];
|
||||
|
||||
public SiteLocalDbCdcRegistrationTests()
|
||||
{
|
||||
_root = Path.Combine(Path.GetTempPath(), $"localdb-cdc-{Guid.NewGuid():N}");
|
||||
Directory.CreateDirectory(_root);
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
foreach (var provider in _providers)
|
||||
{
|
||||
try { provider.Dispose(); } catch { /* best effort */ }
|
||||
}
|
||||
|
||||
Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools();
|
||||
try { Directory.Delete(_root, recursive: true); } catch { /* best effort */ }
|
||||
GC.SuppressFinalize(this);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void UnreplicatedNode_InstallsNoCaptureTriggers()
|
||||
{
|
||||
// No LocalDb:Replication section at all — site-b and site-c on the rig.
|
||||
var db = BuildDatabase(Config());
|
||||
|
||||
Assert.Empty(CaptureTriggers(db));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void UnreplicatedNode_StillCreatesEveryTable()
|
||||
{
|
||||
// Skipping registration must not skip the DDL: the node is fully functional
|
||||
// standalone, it just has nobody to ship its changes to. A guard placed around the
|
||||
// schema block instead of the registration block would fail here and nowhere else.
|
||||
var db = BuildDatabase(Config());
|
||||
|
||||
var tables = TableNames(db);
|
||||
|
||||
foreach (var table in AllSiteTables)
|
||||
Assert.Contains(table, tables);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void InitiatorNode_InstallsCaptureTriggers()
|
||||
{
|
||||
// PeerAddress + ApiKey — site-a node-a, the half that dials.
|
||||
var db = BuildDatabase(Config(
|
||||
peerAddress: "http://peer:8083", apiKey: "cdc-test-key"));
|
||||
|
||||
AssertCaptureTriggersCoverTheReplicatedTables(db);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void PassiveNode_WithApiKeyButNoPeerAddress_StillInstallsCaptureTriggers()
|
||||
{
|
||||
// The reason the predicate is an OR. Site-a node-b sets ApiKey and nothing else:
|
||||
// one bidirectional stream, dialled by one side. Keying on PeerAddress alone would
|
||||
// leave this node capturing nothing, so its own writes would never reach the
|
||||
// initiator and the pair would converge in one direction only — silently.
|
||||
var db = BuildDatabase(Config(apiKey: "cdc-test-key"));
|
||||
|
||||
AssertCaptureTriggersCoverTheReplicatedTables(db);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ReplicatedNode_InstallsNoCaptureTriggersOnTheCentralOnlyNotificationTables()
|
||||
{
|
||||
// The security property, restated at the trigger level: notification_lists and
|
||||
// smtp_configurations exist but must never be captured, because the only payload
|
||||
// they ever historically held was plaintext SMTP passwords.
|
||||
var db = BuildDatabase(Config(apiKey: "cdc-test-key"));
|
||||
|
||||
var triggers = CaptureTriggers(db);
|
||||
|
||||
Assert.DoesNotContain(triggers, t => t.StartsWith("__localdb_notification_lists_", StringComparison.Ordinal));
|
||||
Assert.DoesNotContain(triggers, t => t.StartsWith("__localdb_smtp_configurations_", StringComparison.Ordinal));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void UnreplicatedNode_StillRunsTheLegacyMigrator()
|
||||
{
|
||||
// The migrator is deliberately outside the guard. It renames the legacy file to
|
||||
// ".migrated" on success, which is the observable proof it ran without needing a
|
||||
// capture trigger to look at.
|
||||
var legacyPath = Path.Combine(_root, "legacy-store-and-forward.db");
|
||||
SeedLegacyStoreAndForward(legacyPath);
|
||||
|
||||
_ = BuildDatabase(Config(storeAndForwardPath: legacyPath));
|
||||
|
||||
Assert.False(File.Exists(legacyPath));
|
||||
Assert.True(File.Exists(legacyPath + ".migrated"));
|
||||
}
|
||||
|
||||
// ---- helpers ----------------------------------------------------------------------
|
||||
|
||||
/// <summary>The ten replicated tables plus the two deliberately-unregistered ones.</summary>
|
||||
private static readonly string[] AllSiteTables =
|
||||
[
|
||||
"OperationTracking", "site_events", "sf_messages", "deployed_configurations",
|
||||
"static_attribute_overrides", "shared_scripts", "external_systems",
|
||||
"database_connections", "data_connection_definitions", "native_alarm_state",
|
||||
"notification_lists", "smtp_configurations",
|
||||
];
|
||||
|
||||
private static readonly string[] ReplicatedTables =
|
||||
[
|
||||
"OperationTracking", "site_events", "sf_messages", "deployed_configurations",
|
||||
"static_attribute_overrides", "shared_scripts", "external_systems",
|
||||
"database_connections", "data_connection_definitions", "native_alarm_state",
|
||||
];
|
||||
|
||||
private static void AssertCaptureTriggersCoverTheReplicatedTables(ILocalDb db)
|
||||
{
|
||||
var triggers = CaptureTriggers(db);
|
||||
|
||||
foreach (var table in ReplicatedTables)
|
||||
{
|
||||
// All three kinds, so a partial install is a failure rather than a pass.
|
||||
foreach (var suffix in new[] { "ai", "au", "ad" })
|
||||
Assert.Contains($"__localdb_{table}_{suffix}", triggers);
|
||||
}
|
||||
}
|
||||
|
||||
private ILocalDb BuildDatabase(IConfiguration config)
|
||||
{
|
||||
var provider = new ServiceCollection()
|
||||
.AddZbLocalDb(config, db => SiteLocalDbSetup.OnReady(db, config))
|
||||
.BuildServiceProvider();
|
||||
_providers.Add(provider);
|
||||
|
||||
return provider.GetRequiredService<ILocalDb>();
|
||||
}
|
||||
|
||||
private IConfiguration Config(
|
||||
string? peerAddress = null, string? apiKey = null, string? storeAndForwardPath = null)
|
||||
{
|
||||
var values = new Dictionary<string, string?>
|
||||
{
|
||||
["LocalDb:Path"] = Path.Combine(_root, "consolidated.db"),
|
||||
["ScadaBridge:Node:NodeName"] = "node-a",
|
||||
|
||||
// Every legacy path is pinned inside this test's own directory. Left unset they
|
||||
// resolve CWD-relative (./data/…), and the migrator RENAMES whatever it finds —
|
||||
// so an unlucky run could eat a real file from the test binary's output folder.
|
||||
["ScadaBridge:StoreAndForward:SqliteDbPath"] =
|
||||
storeAndForwardPath ?? Path.Combine(_root, "absent-store-and-forward.db"),
|
||||
["ScadaBridge:Database:SiteDbPath"] = Path.Combine(_root, "absent-scadabridge.db"),
|
||||
["ScadaBridge:SiteEventLog:DatabasePath"] = Path.Combine(_root, "absent-events.db"),
|
||||
["ScadaBridge:OperationTracking:ConnectionString"] =
|
||||
$"Data Source={Path.Combine(_root, "absent-tracking.db")}",
|
||||
};
|
||||
|
||||
if (peerAddress is not null) values["LocalDb:Replication:PeerAddress"] = peerAddress;
|
||||
if (apiKey is not null) values["LocalDb:Replication:ApiKey"] = apiKey;
|
||||
|
||||
return new ConfigurationBuilder().AddInMemoryCollection(values).Build();
|
||||
}
|
||||
|
||||
private static void SeedLegacyStoreAndForward(string path)
|
||||
{
|
||||
using var connection = new Microsoft.Data.Sqlite.SqliteConnection($"Data Source={path}");
|
||||
connection.Open();
|
||||
ZB.MOM.WW.ScadaBridge.StoreAndForward.StoreAndForwardSchema.Apply(connection);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Every LocalDb capture trigger in the file. Matched on the library's
|
||||
/// <c>__localdb_</c> prefix in C# rather than with SQL <c>LIKE</c>, where the underscores
|
||||
/// are single-character wildcards and the pattern would need escaping to mean itself.
|
||||
/// </summary>
|
||||
private static HashSet<string> CaptureTriggers(ILocalDb db)
|
||||
{
|
||||
using var connection = db.CreateConnection();
|
||||
using var cmd = connection.CreateCommand();
|
||||
cmd.CommandText = "SELECT name FROM sqlite_master WHERE type = 'trigger'";
|
||||
using var reader = cmd.ExecuteReader();
|
||||
|
||||
var names = new HashSet<string>(StringComparer.Ordinal);
|
||||
while (reader.Read())
|
||||
{
|
||||
var name = reader.GetString(0);
|
||||
if (name.StartsWith("__localdb_", StringComparison.Ordinal)) names.Add(name);
|
||||
}
|
||||
|
||||
return names;
|
||||
}
|
||||
|
||||
private static HashSet<string> TableNames(ILocalDb db)
|
||||
{
|
||||
using var connection = db.CreateConnection();
|
||||
using var cmd = connection.CreateCommand();
|
||||
cmd.CommandText = "SELECT name FROM sqlite_master WHERE type = 'table'";
|
||||
using var reader = cmd.ExecuteReader();
|
||||
|
||||
var names = new HashSet<string>(StringComparer.Ordinal);
|
||||
while (reader.Read()) names.Add(reader.GetString(0));
|
||||
return names;
|
||||
}
|
||||
}
|
||||
@@ -56,9 +56,16 @@ public class SiteLocalDbWiringTests : IDisposable
|
||||
["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551",
|
||||
["ScadaBridge:Cluster:SeedNodes:1"] = "akka.tcp://scadabridge@localhost:2552",
|
||||
|
||||
// The consolidated site database. No Replication section at all — this
|
||||
// fixture is also the default-OFF pin: storage must work standalone.
|
||||
// The consolidated site database, configured exactly like the PASSIVE half of
|
||||
// the rig's replicated pair (site-a node-b): an ApiKey and no PeerAddress.
|
||||
//
|
||||
// The key is what makes this a replicating node, and therefore what makes the
|
||||
// CDC registration assertions below apply at all — capture is installed only
|
||||
// when replication is configured. The absent PeerAddress keeps this fixture the
|
||||
// default-OFF pin at the same time: nothing dials, so the engine must still
|
||||
// resolve and idle rather than throw or report a connection.
|
||||
["LocalDb:Path"] = _tempDbPath,
|
||||
["LocalDb:Replication:ApiKey"] = "wiring-test-localdb-sync-key",
|
||||
});
|
||||
|
||||
builder.Services.AddGrpc();
|
||||
|
||||
@@ -113,6 +113,12 @@ public abstract class LocalDbSitePairHarness : IAsyncLifetime
|
||||
{
|
||||
["LocalDb:Path"] = path,
|
||||
["ScadaBridge:Node:NodeName"] = nodeName,
|
||||
// OnReady installs the CDC capture triggers only on a node that has
|
||||
// replication configured, so the key has to be here and not only in
|
||||
// ReplicationConfig below — without it these nodes would run the sync
|
||||
// engine over an oplog nothing ever writes to, and every convergence
|
||||
// scenario would time out with two intact but unrelated databases.
|
||||
["LocalDb:Replication:ApiKey"] = SharedApiKey,
|
||||
// Point the legacy migrators at paths that do not exist, so they no-op rather
|
||||
// than picking up stray files from the test working directory. The two Phase 2
|
||||
// defaults matter most: unlike the Phase 1 pair they resolve inside ./data/,
|
||||
|
||||
Reference in New Issue
Block a user