Compare commits
8 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 0e93587b1c | |||
| 4cc0215bb4 | |||
| 4b9bcddbcc | |||
| 2193d9f2e4 | |||
| 8b7ea3213d | |||
| 240d7aa8aa | |||
| 95295210b0 | |||
| e3155fbbf5 |
@@ -106,6 +106,28 @@ IF NOT EXISTS (SELECT 1 FROM dbo.ClusterNode WHERE NodeId = 'site-b-2:4053')
|
|||||||
(NodeId, ClusterId, Host, OpcUaPort, DashboardPort, AkkaPort, GrpcPort, ApplicationUri, ServiceLevelBase, Enabled, CreatedBy)
|
(NodeId, ClusterId, Host, OpcUaPort, DashboardPort, AkkaPort, GrpcPort, ApplicationUri, ServiceLevelBase, Enabled, CreatedBy)
|
||||||
VALUES ('site-b-2:4053', 'SITE-B', 'site-b-2', 4840, 8081, 4053, 4056, 'urn:OtOpcUa:site-b-2', 150, 1, 'docker-dev-seed');
|
VALUES ('site-b-2:4053', 'SITE-B', 'site-b-2', 4840, 8081, 4053, 4056, 'urn:OtOpcUa:site-b-2', 150, 1, 'docker-dev-seed');
|
||||||
|
|
||||||
|
------------------------------------------------------------------------------
|
||||||
|
-- Backfill: GrpcPort on rows that predate the column (#493)
|
||||||
|
--
|
||||||
|
-- Every insert above is INSERT-if-not-exists, which is the right shape for a re-runnable seed
|
||||||
|
-- but means a *new nullable* column never reaches rows created by an earlier version of this
|
||||||
|
-- file. On a persisted volume that is exactly what happened when Phase 5 added GrpcPort: the
|
||||||
|
-- six ClusterNode rows already existed, so they kept NULL, TelemetryNodeSource found no dial
|
||||||
|
-- targets, and TelemetryDial:Mode=Grpc silently connected to nothing (throttled Warning per
|
||||||
|
-- node — the code degrades gracefully, so nothing failed loudly).
|
||||||
|
--
|
||||||
|
-- Backfills NULL only. It must not overwrite a port an operator set deliberately on a
|
||||||
|
-- long-lived dev volume; a NULL is the unambiguous "this row predates the column" marker.
|
||||||
|
--
|
||||||
|
-- AkkaPort deliberately has no equivalent here: it is NOT NULL with a default of 4053, so
|
||||||
|
-- pre-existing rows already carry a usable value and there is nothing to backfill.
|
||||||
|
------------------------------------------------------------------------------
|
||||||
|
|
||||||
|
UPDATE dbo.ClusterNode
|
||||||
|
SET GrpcPort = 4056 -- matches Telemetry__GrpcListenPort on all six nodes in docker-compose.yml
|
||||||
|
WHERE GrpcPort IS NULL
|
||||||
|
AND CreatedBy = 'docker-dev-seed';
|
||||||
|
|
||||||
------------------------------------------------------------------------------
|
------------------------------------------------------------------------------
|
||||||
-- Galaxy MxAccess gateway — INTENTIONALLY NOT SEEDED (retired 2026-06-15)
|
-- Galaxy MxAccess gateway — INTENTIONALLY NOT SEEDED (retired 2026-06-15)
|
||||||
--
|
--
|
||||||
|
|||||||
@@ -128,6 +128,39 @@ transports. ScadaBridge itself has since closed that gap with this identical
|
|||||||
interceptor pattern, so Phase 5 ships authenticated from the start rather than deferring auth to a
|
interceptor pattern, so Phase 5 ships authenticated from the start rather than deferring auth to a
|
||||||
later hardening pass. See the design doc's superseded note for the full history.
|
later hardening pass. See the design doc's superseded note for the full history.
|
||||||
|
|
||||||
|
## Upgrading an existing deployment: populate `ClusterNode.GrpcPort` first (#493)
|
||||||
|
|
||||||
|
`GrpcPort` is a **nullable column added by Phase 5**, and nothing backfills it onto rows that
|
||||||
|
already existed. On an upgraded deployment every `ClusterNode` row therefore carries `NULL`,
|
||||||
|
central finds **no dial targets**, and flipping `TelemetryDial:Mode` to `Grpc` connects to
|
||||||
|
nothing. Fresh installs are unaffected. `AkkaPort` did not have this problem only because it is
|
||||||
|
`NOT NULL` with a default of 4053.
|
||||||
|
|
||||||
|
Nothing fails loudly, which is what makes this worth stating up front — the code degrades
|
||||||
|
gracefully, once per node, throttled:
|
||||||
|
|
||||||
|
```
|
||||||
|
ClusterNode <id> has no GrpcPort; it exposes no telemetry stream and is skipped in this dial refresh
|
||||||
|
(further skips of this node are silent)
|
||||||
|
```
|
||||||
|
|
||||||
|
Other symptoms: no `ESTABLISHED` sockets on the telemetry port (a `LISTEN` but nothing else in
|
||||||
|
`docker exec <node> cat /proc/net/tcp6`), and the `/alerts` / `/script-log` pill never turning
|
||||||
|
live in `Grpc` mode.
|
||||||
|
|
||||||
|
**Before flipping any node to `Grpc`**, set each driver node's `GrpcPort` to that node's own
|
||||||
|
`Telemetry:GrpcListenPort` — via the AdminUI node editor, or directly:
|
||||||
|
|
||||||
|
```sql
|
||||||
|
UPDATE dbo.ClusterNode SET GrpcPort = <that node's Telemetry:GrpcListenPort> WHERE NodeId = '<node>';
|
||||||
|
```
|
||||||
|
|
||||||
|
There is deliberately **no EF data migration** backfilling this. The correct value is that node's
|
||||||
|
own listener port, which is per-deployment configuration the database cannot derive; a migration
|
||||||
|
could only invent one, and a *wrong* dial target is worse than the `NULL` the dialer already skips
|
||||||
|
explicitly. The docker-dev seed does backfill (`GrpcPort IS NULL AND CreatedBy = 'docker-dev-seed'`
|
||||||
|
→ 4056) because there the port is fixed by `docker-compose.yml` and therefore actually knowable.
|
||||||
|
|
||||||
## Reconnect and reliability
|
## Reconnect and reliability
|
||||||
|
|
||||||
Central's `TelemetryDialSupervisor` (an admin-role actor) runs **one reconnecting dialer per
|
Central's `TelemetryDialSupervisor` (an admin-role actor) runs **one reconnecting dialer per
|
||||||
|
|||||||
@@ -60,6 +60,11 @@ public static class DraftValidator
|
|||||||
/// credential in the ConfigDb — persisted, replicated to every node's artifact cache, and readable by
|
/// credential in the ConfigDb — persisted, replicated to every node's artifact cache, and readable by
|
||||||
/// anyone with config access — while the runtime silently ignored it and the driver failed to connect.
|
/// anyone with config access — while the runtime silently ignored it and the driver failed to connect.
|
||||||
/// Discarding a secret on read is not the same as refusing to store it.</para>
|
/// Discarding a secret on read is not the same as refusing to store it.</para>
|
||||||
|
/// <para><b>Two of the three surfaces are checked here.</b> The third — a node's
|
||||||
|
/// <see cref="ClusterNode.DriverConfigOverridesJson"/> — is not reachable from
|
||||||
|
/// <see cref="DraftSnapshot"/> and is gated at its save instead (#499); see
|
||||||
|
/// <see cref="SqlCredentialGuard"/> for why a deploy gate is the wrong instrument for a value that
|
||||||
|
/// never enters the artifact.</para>
|
||||||
/// <para>Checked on <b>both</b> config surfaces a Sql driver reads: the instance's
|
/// <para>Checked on <b>both</b> config surfaces a Sql driver reads: the instance's
|
||||||
/// <see cref="DriverInstance.DriverConfig"/> and the <see cref="Device.DeviceConfig"/> of every device
|
/// <see cref="DriverInstance.DriverConfig"/> and the <see cref="Device.DeviceConfig"/> of every device
|
||||||
/// beneath it, because the two are merged before the DTO sees them — a credential pasted into the
|
/// beneath it, because the two are merged before the DTO sees them — a credential pasted into the
|
||||||
@@ -74,7 +79,7 @@ public static class DraftValidator
|
|||||||
/// </remarks>
|
/// </remarks>
|
||||||
private static void ValidateSqlConnectionStringNotPersisted(DraftSnapshot draft, List<ValidationError> errors)
|
private static void ValidateSqlConnectionStringNotPersisted(DraftSnapshot draft, List<ValidationError> errors)
|
||||||
{
|
{
|
||||||
const string ForbiddenKey = "connectionString";
|
const string ForbiddenKey = SqlCredentialGuard.ForbiddenKey;
|
||||||
|
|
||||||
var sqlInstanceIds = draft.DriverInstances
|
var sqlInstanceIds = draft.DriverInstances
|
||||||
.Where(d => string.Equals(d.DriverType, Core.Abstractions.DriverTypeNames.Sql, StringComparison.Ordinal))
|
.Where(d => string.Equals(d.DriverType, Core.Abstractions.DriverTypeNames.Sql, StringComparison.Ordinal))
|
||||||
@@ -85,7 +90,7 @@ public static class DraftValidator
|
|||||||
foreach (var d in draft.DriverInstances)
|
foreach (var d in draft.DriverInstances)
|
||||||
{
|
{
|
||||||
if (!sqlInstanceIds.Contains(d.DriverInstanceId)) continue;
|
if (!sqlInstanceIds.Contains(d.DriverInstanceId)) continue;
|
||||||
if (!HasTopLevelKey(d.DriverConfig, ForbiddenKey)) continue;
|
if (!SqlCredentialGuard.CarriesLiteralConnectionString(d.DriverConfig)) continue;
|
||||||
errors.Add(new("SqlConnectionStringPersisted",
|
errors.Add(new("SqlConnectionStringPersisted",
|
||||||
$"Sql driver instance '{d.DriverInstanceId}' has a '{ForbiddenKey}' key in its DriverConfig. " +
|
$"Sql driver instance '{d.DriverInstanceId}' has a '{ForbiddenKey}' key in its DriverConfig. " +
|
||||||
"A Sql driver must name its credentials indirectly via 'connectionStringRef', which resolves " +
|
"A Sql driver must name its credentials indirectly via 'connectionStringRef', which resolves " +
|
||||||
@@ -98,7 +103,7 @@ public static class DraftValidator
|
|||||||
foreach (var dev in draft.Devices)
|
foreach (var dev in draft.Devices)
|
||||||
{
|
{
|
||||||
if (!sqlInstanceIds.Contains(dev.DriverInstanceId)) continue;
|
if (!sqlInstanceIds.Contains(dev.DriverInstanceId)) continue;
|
||||||
if (!HasTopLevelKey(dev.DeviceConfig, ForbiddenKey)) continue;
|
if (!SqlCredentialGuard.CarriesLiteralConnectionString(dev.DeviceConfig)) continue;
|
||||||
errors.Add(new("SqlConnectionStringPersisted",
|
errors.Add(new("SqlConnectionStringPersisted",
|
||||||
$"Device '{dev.DeviceId}' on Sql driver instance '{dev.DriverInstanceId}' has a " +
|
$"Device '{dev.DeviceId}' on Sql driver instance '{dev.DriverInstanceId}' has a " +
|
||||||
$"'{ForbiddenKey}' key in its DeviceConfig. DeviceConfig is merged onto DriverConfig before " +
|
$"'{ForbiddenKey}' key in its DeviceConfig. DeviceConfig is merged onto DriverConfig before " +
|
||||||
@@ -107,34 +112,6 @@ public static class DraftValidator
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
|
||||||
/// True when <paramref name="json"/> is a JSON object carrying <paramref name="key"/> at its top level,
|
|
||||||
/// matched case-insensitively. Never throws — a blank, malformed or non-object blob simply has no keys,
|
|
||||||
/// and shaping the config JSON is another rule's job.
|
|
||||||
/// </summary>
|
|
||||||
/// <param name="json">The config blob to inspect.</param>
|
|
||||||
/// <param name="key">The property name to look for.</param>
|
|
||||||
/// <returns><see langword="true"/> when the key is present at the top level.</returns>
|
|
||||||
private static bool HasTopLevelKey(string? json, string key)
|
|
||||||
{
|
|
||||||
if (string.IsNullOrWhiteSpace(json)) return false;
|
|
||||||
try
|
|
||||||
{
|
|
||||||
using var doc = System.Text.Json.JsonDocument.Parse(json);
|
|
||||||
if (doc.RootElement.ValueKind != System.Text.Json.JsonValueKind.Object) return false;
|
|
||||||
foreach (var property in doc.RootElement.EnumerateObject())
|
|
||||||
{
|
|
||||||
if (string.Equals(property.Name, key, StringComparison.OrdinalIgnoreCase)) return true;
|
|
||||||
}
|
|
||||||
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
catch (System.Text.Json.JsonException)
|
|
||||||
{
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// <summary>WP7 Calculation-driver deploy gates. For every tag bound to a <c>Calculation</c> driver:
|
/// <summary>WP7 Calculation-driver deploy gates. For every tag bound to a <c>Calculation</c> driver:
|
||||||
/// <list type="number">
|
/// <list type="number">
|
||||||
/// <item><b>scriptId existence</b> — the tag's <c>TagConfig.scriptId</c> must be present and resolve to
|
/// <item><b>scriptId existence</b> — the tag's <c>TagConfig.scriptId</c> must be present and resolve to
|
||||||
|
|||||||
@@ -0,0 +1,139 @@
|
|||||||
|
using System.Text.Json;
|
||||||
|
|
||||||
|
namespace ZB.MOM.WW.OtOpcUa.Configuration.Validation;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// The one place that decides whether a persisted config blob carries a literal Sql
|
||||||
|
/// <c>connectionString</c> (Gitea #498 / #499).
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// <para>
|
||||||
|
/// A <c>Sql</c> driver names its credentials indirectly — <c>connectionStringRef</c> resolves from
|
||||||
|
/// the environment / secret store at Initialize — so nothing persisted should ever contain a
|
||||||
|
/// database password. The typed <c>SqlDriverConfigDto</c> already drops a <c>connectionString</c>
|
||||||
|
/// key on <b>read</b>, but discarding a secret on read is not the same as refusing to store it:
|
||||||
|
/// config blobs are schemaless JSON columns authored through raw-JSON textareas, so the key can
|
||||||
|
/// be written even though the runtime ignores it.
|
||||||
|
/// </para>
|
||||||
|
/// <para>
|
||||||
|
/// There are <b>three</b> surfaces a Sql driver's config is persisted on, and they need different
|
||||||
|
/// enforcement points:
|
||||||
|
/// <list type="bullet">
|
||||||
|
/// <item><c>DriverInstance.DriverConfig</c> and <c>Device.DeviceConfig</c> — both in the
|
||||||
|
/// deployed artifact, both visible to <c>DraftSnapshot</c>, both gated by
|
||||||
|
/// <c>DraftValidator</c>.</item>
|
||||||
|
/// <item><c>ClusterNode.DriverConfigOverridesJson</c> — the per-node override map. It is
|
||||||
|
/// <b>not</b> in <c>DraftSnapshot</c>, and deliberately does not go there: adding a
|
||||||
|
/// <c>ClusterNode</c> collection would widen the snapshot (and every builder of one) for a
|
||||||
|
/// single rule about a value that is not part of the artifact at all. More to the point, a
|
||||||
|
/// deploy gate is the wrong instrument here — the artifact never carries node overrides, so
|
||||||
|
/// blocking a deploy would not stop the credential being stored. It is already in the
|
||||||
|
/// database by then. This surface is therefore gated at the <b>save</b>, which is the only
|
||||||
|
/// point where "refuse to store it" is literally true.</item>
|
||||||
|
/// </list>
|
||||||
|
/// </para>
|
||||||
|
/// <para>
|
||||||
|
/// <b>Deliberately narrow.</b> This checks the <c>Sql</c> driver's <c>connectionString</c> key
|
||||||
|
/// only — not a general "credential-shaped key" sweep over every driver type. A broader rule
|
||||||
|
/// would start refusing configs that are legitimate today for drivers that never made this
|
||||||
|
/// guarantee, turning defence-in-depth into a regression. Widen it per driver, as each driver
|
||||||
|
/// gains an indirect-credential contract of its own.
|
||||||
|
/// </para>
|
||||||
|
/// </remarks>
|
||||||
|
public static class SqlCredentialGuard
|
||||||
|
{
|
||||||
|
/// <summary>The config key a Sql driver must never persist. Matched case-insensitively.</summary>
|
||||||
|
public const string ForbiddenKey = "connectionString";
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Whether <paramref name="configJson"/> is a JSON object carrying <see cref="ForbiddenKey"/> at
|
||||||
|
/// its top level.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// Matched <b>case-insensitively</b>: <c>System.Text.Json</c> binds <c>ConnectionString</c> to a
|
||||||
|
/// <c>connectionString</c> property by default, so a case variant is the same key, not a
|
||||||
|
/// different one. Only the top level is scanned — the DTO is flat, so a nested occurrence cannot
|
||||||
|
/// bind and is not the credential-shaped mistake this exists to catch.
|
||||||
|
/// </remarks>
|
||||||
|
/// <param name="configJson">The config blob to inspect.</param>
|
||||||
|
/// <returns><see langword="true"/> when the key is present at the top level.</returns>
|
||||||
|
public static bool CarriesLiteralConnectionString(string? configJson)
|
||||||
|
{
|
||||||
|
if (string.IsNullOrWhiteSpace(configJson)) return false;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
using var doc = JsonDocument.Parse(configJson);
|
||||||
|
return doc.RootElement.ValueKind == JsonValueKind.Object
|
||||||
|
&& HasTopLevelKey(doc.RootElement, ForbiddenKey);
|
||||||
|
}
|
||||||
|
catch (JsonException)
|
||||||
|
{
|
||||||
|
// A malformed blob simply has no keys. Shaping the config JSON is another rule's job, and
|
||||||
|
// throwing here would turn a formatting mistake into a credential-guard failure.
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// The <c>DriverInstanceId</c>s in a <c>ClusterNode.DriverConfigOverridesJson</c> map whose
|
||||||
|
/// override object carries a literal connection string, restricted to instances that are actually
|
||||||
|
/// Sql drivers.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// The override map is shaped <c>{ "<DriverInstanceId>": { …driver config keys… } }</c> and is
|
||||||
|
/// merged onto the cluster-level <c>DriverConfig</c>, so a credential pasted into one lands in
|
||||||
|
/// exactly the place <see cref="CarriesLiteralConnectionString"/> already refuses on the driver
|
||||||
|
/// itself.
|
||||||
|
/// </remarks>
|
||||||
|
/// <param name="overridesJson">The node's override map. Null / blank / malformed yields no
|
||||||
|
/// violations.</param>
|
||||||
|
/// <param name="sqlDriverInstanceIds">The ids of driver instances whose <c>DriverType</c> is
|
||||||
|
/// <c>Sql</c>. Keys outside this set are ignored — a non-Sql driver never made the indirect-credential
|
||||||
|
/// guarantee, so flagging it would be a regression, not defence in depth.</param>
|
||||||
|
/// <returns>The offending driver-instance ids, in document order; empty when there are none.</returns>
|
||||||
|
public static IReadOnlyList<string> FindNodeOverrideViolations(
|
||||||
|
string? overridesJson, IReadOnlyCollection<string> sqlDriverInstanceIds)
|
||||||
|
{
|
||||||
|
if (string.IsNullOrWhiteSpace(overridesJson) || sqlDriverInstanceIds.Count == 0)
|
||||||
|
{
|
||||||
|
return [];
|
||||||
|
}
|
||||||
|
|
||||||
|
var sqlIds = sqlDriverInstanceIds as IReadOnlySet<string>
|
||||||
|
?? sqlDriverInstanceIds.ToHashSet(StringComparer.Ordinal);
|
||||||
|
|
||||||
|
try
|
||||||
|
{
|
||||||
|
using var doc = JsonDocument.Parse(overridesJson);
|
||||||
|
if (doc.RootElement.ValueKind != JsonValueKind.Object) return [];
|
||||||
|
|
||||||
|
List<string>? offenders = null;
|
||||||
|
foreach (var entry in doc.RootElement.EnumerateObject())
|
||||||
|
{
|
||||||
|
if (!sqlIds.Contains(entry.Name)) continue;
|
||||||
|
if (entry.Value.ValueKind != JsonValueKind.Object) continue;
|
||||||
|
if (!HasTopLevelKey(entry.Value, ForbiddenKey)) continue;
|
||||||
|
|
||||||
|
(offenders ??= []).Add(entry.Name);
|
||||||
|
}
|
||||||
|
|
||||||
|
return (IReadOnlyList<string>?)offenders ?? [];
|
||||||
|
}
|
||||||
|
catch (JsonException)
|
||||||
|
{
|
||||||
|
return [];
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>Whether <paramref name="obj"/> carries <paramref name="key"/> at its top level,
|
||||||
|
/// case-insensitively.</summary>
|
||||||
|
private static bool HasTopLevelKey(JsonElement obj, string key)
|
||||||
|
{
|
||||||
|
foreach (var property in obj.EnumerateObject())
|
||||||
|
{
|
||||||
|
if (string.Equals(property.Name, key, StringComparison.OrdinalIgnoreCase)) return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -9,6 +9,8 @@
|
|||||||
@using System.ComponentModel.DataAnnotations
|
@using System.ComponentModel.DataAnnotations
|
||||||
@using ZB.MOM.WW.OtOpcUa.Configuration
|
@using ZB.MOM.WW.OtOpcUa.Configuration
|
||||||
@using ZB.MOM.WW.OtOpcUa.Configuration.Entities
|
@using ZB.MOM.WW.OtOpcUa.Configuration.Entities
|
||||||
|
@using ZB.MOM.WW.OtOpcUa.Configuration.Validation
|
||||||
|
@using ZB.MOM.WW.OtOpcUa.Core.Abstractions
|
||||||
@inject IDbContextFactory<OtOpcUaConfigDbContext> DbFactory
|
@inject IDbContextFactory<OtOpcUaConfigDbContext> DbFactory
|
||||||
@inject NavigationManager Nav
|
@inject NavigationManager Nav
|
||||||
@inject AuthenticationStateProvider AuthState
|
@inject AuthenticationStateProvider AuthState
|
||||||
@@ -34,6 +36,10 @@ else
|
|||||||
{
|
{
|
||||||
<EditForm Model="_form" OnValidSubmit="SubmitAsync" FormName="nodeEdit">
|
<EditForm Model="_form" OnValidSubmit="SubmitAsync" FormName="nodeEdit">
|
||||||
<DataAnnotationsValidator />
|
<DataAnnotationsValidator />
|
||||||
|
@* Without this, a validation failure suppresses OnValidSubmit with NO feedback whatsoever —
|
||||||
|
the Save button simply does nothing and the operator has no way to tell why. That is how the
|
||||||
|
NodeId-regex defect above stayed invisible. *@
|
||||||
|
<ValidationSummary class="alert alert-danger py-2 px-3" />
|
||||||
<section class="panel rise" style="animation-delay:.02s">
|
<section class="panel rise" style="animation-delay:.02s">
|
||||||
<div class="panel-head">Identity</div>
|
<div class="panel-head">Identity</div>
|
||||||
<div style="padding:1rem">
|
<div style="padding:1rem">
|
||||||
@@ -178,6 +184,33 @@ else
|
|||||||
}
|
}
|
||||||
|
|
||||||
await using var db = await DbFactory.CreateDbContextAsync();
|
await using var db = await DbFactory.CreateDbContextAsync();
|
||||||
|
|
||||||
|
// #499 — the third Sql-credential surface. DriverConfigOverridesJson is merged onto the
|
||||||
|
// cluster-level DriverConfig, so a literal connectionString pasted here leaks exactly as one
|
||||||
|
// pasted into the driver itself. The deploy gate cannot see it (DraftSnapshot carries no
|
||||||
|
// ClusterNode rows) and would be the wrong place anyway: node overrides never enter the
|
||||||
|
// artifact, so by the time a deploy ran the credential would already be stored. Refuse the
|
||||||
|
// save instead. The message names the driver instance but NEVER the value.
|
||||||
|
if (!string.IsNullOrWhiteSpace(_form.DriverConfigOverridesJson))
|
||||||
|
{
|
||||||
|
var sqlDriverIds = await db.DriverInstances
|
||||||
|
.Where(d => d.ClusterId == ClusterId && d.DriverType == DriverTypeNames.Sql)
|
||||||
|
.Select(d => d.DriverInstanceId)
|
||||||
|
.ToListAsync();
|
||||||
|
|
||||||
|
var offenders = SqlCredentialGuard.FindNodeOverrideViolations(
|
||||||
|
_form.DriverConfigOverridesJson, sqlDriverIds);
|
||||||
|
if (offenders.Count > 0)
|
||||||
|
{
|
||||||
|
_error =
|
||||||
|
$"Driver config overrides carry a '{SqlCredentialGuard.ForbiddenKey}' for Sql driver " +
|
||||||
|
$"instance(s) {string.Join(", ", offenders)}. A Sql driver must name its credentials " +
|
||||||
|
"indirectly via 'connectionStringRef', which resolves from the environment / secret " +
|
||||||
|
"store at Initialize; a literal connection string here would be stored in the config " +
|
||||||
|
"database, and the runtime ignores it anyway. Remove the key.";
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
if (IsNew)
|
if (IsNew)
|
||||||
{
|
{
|
||||||
if (await db.ClusterNodes.AnyAsync(n => n.NodeId == _form.NodeId))
|
if (await db.ClusterNodes.AnyAsync(n => n.NodeId == _form.NodeId))
|
||||||
@@ -194,7 +227,7 @@ else
|
|||||||
OpcUaPort = _form.OpcUaPort,
|
OpcUaPort = _form.OpcUaPort,
|
||||||
DashboardPort = _form.DashboardPort,
|
DashboardPort = _form.DashboardPort,
|
||||||
ApplicationUri = _form.ApplicationUri,
|
ApplicationUri = _form.ApplicationUri,
|
||||||
ServiceLevelBase = _form.ServiceLevelBase,
|
ServiceLevelBase = (byte)_form.ServiceLevelBase,
|
||||||
Enabled = _form.Enabled,
|
Enabled = _form.Enabled,
|
||||||
DriverConfigOverridesJson = string.IsNullOrWhiteSpace(_form.DriverConfigOverridesJson) ? null : _form.DriverConfigOverridesJson,
|
DriverConfigOverridesJson = string.IsNullOrWhiteSpace(_form.DriverConfigOverridesJson) ? null : _form.DriverConfigOverridesJson,
|
||||||
CreatedAt = DateTime.UtcNow,
|
CreatedAt = DateTime.UtcNow,
|
||||||
@@ -210,7 +243,7 @@ else
|
|||||||
entity.OpcUaPort = _form.OpcUaPort;
|
entity.OpcUaPort = _form.OpcUaPort;
|
||||||
entity.DashboardPort = _form.DashboardPort;
|
entity.DashboardPort = _form.DashboardPort;
|
||||||
entity.ApplicationUri = _form.ApplicationUri;
|
entity.ApplicationUri = _form.ApplicationUri;
|
||||||
entity.ServiceLevelBase = _form.ServiceLevelBase;
|
entity.ServiceLevelBase = (byte)_form.ServiceLevelBase;
|
||||||
entity.Enabled = _form.Enabled;
|
entity.Enabled = _form.Enabled;
|
||||||
entity.DriverConfigOverridesJson = string.IsNullOrWhiteSpace(_form.DriverConfigOverridesJson) ? null : _form.DriverConfigOverridesJson;
|
entity.DriverConfigOverridesJson = string.IsNullOrWhiteSpace(_form.DriverConfigOverridesJson) ? null : _form.DriverConfigOverridesJson;
|
||||||
}
|
}
|
||||||
@@ -254,14 +287,26 @@ else
|
|||||||
|
|
||||||
private sealed class FormModel
|
private sealed class FormModel
|
||||||
{
|
{
|
||||||
[Required, RegularExpression("^[A-Za-z0-9_-]+$")]
|
// The ':' and '.' are REQUIRED, not permissive. Every NodeId in this system is "host:port" —
|
||||||
|
// ClusterRoleInfo and ConfigPublishCoordinator derive it as member.Address.Host:Port, the seed
|
||||||
|
// writes "central-1:4053", and NodeDeploymentState.NodeId is FK-bound to it. The previous
|
||||||
|
// pattern forbade the colon, so EVERY existing node failed validation and OnValidSubmit never
|
||||||
|
// ran: the Save button did nothing at all, silently. Hostnames may also be dotted
|
||||||
|
// (line3-opc-a.plant.local:4053).
|
||||||
|
[Required, RegularExpression("^[A-Za-z0-9_.:-]+$")]
|
||||||
public string NodeId { get; set; } = "";
|
public string NodeId { get; set; } = "";
|
||||||
[Required] public string Host { get; set; } = "";
|
[Required] public string Host { get; set; } = "";
|
||||||
[Range(1, 65535)] public int OpcUaPort { get; set; } = 4840;
|
[Range(1, 65535)] public int OpcUaPort { get; set; } = 4840;
|
||||||
[Range(1, 65535)] public int DashboardPort { get; set; } = 8081;
|
[Range(1, 65535)] public int DashboardPort { get; set; } = 8081;
|
||||||
[Required, RegularExpression("^urn:[A-Za-z0-9_:./-]+$")]
|
[Required, RegularExpression("^urn:[A-Za-z0-9_:./-]+$")]
|
||||||
public string ApplicationUri { get; set; } = "";
|
public string ApplicationUri { get; set; } = "";
|
||||||
[Range(0, 255)] public byte ServiceLevelBase { get; set; } = 200;
|
// int, NOT byte. Blazor's InputNumber<T> rejects System.Byte outright — its static
|
||||||
|
// constructor throws "The type 'System.Byte' is not a supported numeric type", which escapes
|
||||||
|
// BuildRenderTree unhandled and takes the WHOLE page to an HTTP 500. Binding this field to
|
||||||
|
// the entity's own byte type meant /clusters/{id}/nodes/{nodeId} and .../nodes/new never
|
||||||
|
// rendered at all. The Range attribute still holds it to a byte's domain, and SubmitAsync
|
||||||
|
// casts on the way to the entity, so the stored type is unchanged.
|
||||||
|
[Range(0, 255)] public int ServiceLevelBase { get; set; } = 200;
|
||||||
public bool Enabled { get; set; } = true;
|
public bool Enabled { get; set; } = true;
|
||||||
public string? DriverConfigOverridesJson { get; set; }
|
public string? DriverConfigOverridesJson { get; set; }
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -170,6 +170,7 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers
|
|||||||
/// <summary>Interval between bounded post-connect re-discovery passes. Production default 2s; tests
|
/// <summary>Interval between bounded post-connect re-discovery passes. Production default 2s; tests
|
||||||
/// inject a tiny value so the loop runs without real-time waits.</summary>
|
/// inject a tiny value so the loop runs without real-time waits.</summary>
|
||||||
private readonly TimeSpan _rediscoverInterval;
|
private readonly TimeSpan _rediscoverInterval;
|
||||||
|
private readonly TimeSpan _healthPollInterval;
|
||||||
|
|
||||||
/// <summary>Cap on the number of post-connect re-discovery passes — a backstop so a never-stabilising
|
/// <summary>Cap on the number of post-connect re-discovery passes — a backstop so a never-stabilising
|
||||||
/// (or perpetually-empty) discovered set cannot spin the loop forever. Production default 15.</summary>
|
/// (or perpetually-empty) discovered set cannot spin the loop forever. Production default 15.</summary>
|
||||||
@@ -239,6 +240,9 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers
|
|||||||
/// <param name="rediscoverDiscoverTimeout">Optional per-pass timeout for <see cref="ITagDiscovery.DiscoverAsync"/>; defaults to 30 seconds.</param>
|
/// <param name="rediscoverDiscoverTimeout">Optional per-pass timeout for <see cref="ITagDiscovery.DiscoverAsync"/>; defaults to 30 seconds.</param>
|
||||||
/// <param name="invoker">Optional Phase 6.1 resilience invoker wrapping this driver's capability
|
/// <param name="invoker">Optional Phase 6.1 resilience invoker wrapping this driver's capability
|
||||||
/// calls; defaults to <see cref="NullDriverCapabilityInvoker"/> (pass-through) when not supplied.</param>
|
/// calls; defaults to <see cref="NullDriverCapabilityInvoker"/> (pass-through) when not supplied.</param>
|
||||||
|
/// <param name="healthPollInterval">Optional period of the health-poll heartbeat, which also drives the
|
||||||
|
/// subscription reconcile (<see cref="ReconcileSubscription"/>); defaults to
|
||||||
|
/// <see cref="HealthPollInterval"/>. Exists so a test can drive the reconcile without waiting 30 s.</param>
|
||||||
/// <returns>A <see cref="Akka.Actor.Props"/> instance configured to create the wrapped <see cref="DriverInstanceActor"/>.</returns>
|
/// <returns>A <see cref="Akka.Actor.Props"/> instance configured to create the wrapped <see cref="DriverInstanceActor"/>.</returns>
|
||||||
public static Props Props(
|
public static Props Props(
|
||||||
IDriver driver,
|
IDriver driver,
|
||||||
@@ -249,7 +253,8 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers
|
|||||||
TimeSpan? rediscoverInterval = null,
|
TimeSpan? rediscoverInterval = null,
|
||||||
int rediscoverMaxAttempts = DefaultRediscoverMaxAttempts,
|
int rediscoverMaxAttempts = DefaultRediscoverMaxAttempts,
|
||||||
TimeSpan? rediscoverDiscoverTimeout = null,
|
TimeSpan? rediscoverDiscoverTimeout = null,
|
||||||
IDriverCapabilityInvoker? invoker = null) =>
|
IDriverCapabilityInvoker? invoker = null,
|
||||||
|
TimeSpan? healthPollInterval = null) =>
|
||||||
Akka.Actor.Props.Create(() => new DriverInstanceActor(
|
Akka.Actor.Props.Create(() => new DriverInstanceActor(
|
||||||
driver,
|
driver,
|
||||||
reconnectInterval ?? DefaultReconnectInterval,
|
reconnectInterval ?? DefaultReconnectInterval,
|
||||||
@@ -259,7 +264,8 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers
|
|||||||
rediscoverInterval,
|
rediscoverInterval,
|
||||||
rediscoverMaxAttempts,
|
rediscoverMaxAttempts,
|
||||||
rediscoverDiscoverTimeout,
|
rediscoverDiscoverTimeout,
|
||||||
invoker));
|
invoker,
|
||||||
|
healthPollInterval));
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Returns true when the driver should boot in DEV-STUB mode based on host platform and
|
/// Returns true when the driver should boot in DEV-STUB mode based on host platform and
|
||||||
@@ -293,6 +299,8 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers
|
|||||||
/// <param name="rediscoverDiscoverTimeout">Per-pass timeout for <see cref="ITagDiscovery.DiscoverAsync"/>; defaults to 30 seconds.</param>
|
/// <param name="rediscoverDiscoverTimeout">Per-pass timeout for <see cref="ITagDiscovery.DiscoverAsync"/>; defaults to 30 seconds.</param>
|
||||||
/// <param name="invoker">Phase 6.1 resilience invoker wrapping this driver's capability calls;
|
/// <param name="invoker">Phase 6.1 resilience invoker wrapping this driver's capability calls;
|
||||||
/// defaults to <see cref="NullDriverCapabilityInvoker"/> (pass-through) when null.</param>
|
/// defaults to <see cref="NullDriverCapabilityInvoker"/> (pass-through) when null.</param>
|
||||||
|
/// <param name="healthPollInterval">Period of the health-poll heartbeat, which also drives the
|
||||||
|
/// subscription reconcile; defaults to <see cref="HealthPollInterval"/> when null.</param>
|
||||||
public DriverInstanceActor(
|
public DriverInstanceActor(
|
||||||
IDriver driver,
|
IDriver driver,
|
||||||
TimeSpan reconnectInterval,
|
TimeSpan reconnectInterval,
|
||||||
@@ -302,7 +310,8 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers
|
|||||||
TimeSpan? rediscoverInterval = null,
|
TimeSpan? rediscoverInterval = null,
|
||||||
int rediscoverMaxAttempts = DefaultRediscoverMaxAttempts,
|
int rediscoverMaxAttempts = DefaultRediscoverMaxAttempts,
|
||||||
TimeSpan? rediscoverDiscoverTimeout = null,
|
TimeSpan? rediscoverDiscoverTimeout = null,
|
||||||
IDriverCapabilityInvoker? invoker = null)
|
IDriverCapabilityInvoker? invoker = null,
|
||||||
|
TimeSpan? healthPollInterval = null)
|
||||||
{
|
{
|
||||||
_driver = driver;
|
_driver = driver;
|
||||||
_invoker = invoker ?? NullDriverCapabilityInvoker.Instance;
|
_invoker = invoker ?? NullDriverCapabilityInvoker.Instance;
|
||||||
@@ -313,6 +322,7 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers
|
|||||||
_rediscoverInterval = rediscoverInterval ?? DefaultRediscoverInterval;
|
_rediscoverInterval = rediscoverInterval ?? DefaultRediscoverInterval;
|
||||||
_rediscoverMaxAttempts = rediscoverMaxAttempts;
|
_rediscoverMaxAttempts = rediscoverMaxAttempts;
|
||||||
_rediscoverDiscoverTimeout = rediscoverDiscoverTimeout ?? DefaultRediscoverDiscoverTimeout;
|
_rediscoverDiscoverTimeout = rediscoverDiscoverTimeout ?? DefaultRediscoverDiscoverTimeout;
|
||||||
|
_healthPollInterval = healthPollInterval ?? HealthPollInterval;
|
||||||
OtOpcUaTelemetry.DriverInstanceLifecycle.Add(1,
|
OtOpcUaTelemetry.DriverInstanceLifecycle.Add(1,
|
||||||
new KeyValuePair<string, object?>("event", startStubbed ? "spawn_stub" : "spawn"),
|
new KeyValuePair<string, object?>("event", startStubbed ? "spawn_stub" : "spawn"),
|
||||||
new KeyValuePair<string, object?>("driver_type", driver.DriverType));
|
new KeyValuePair<string, object?>("driver_type", driver.DriverType));
|
||||||
@@ -335,7 +345,7 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers
|
|||||||
// actor starts, before any state transition fires. Also start the periodic heartbeat so
|
// actor starts, before any state transition fires. Also start the periodic heartbeat so
|
||||||
// long-lived Healthy drivers keep their snapshot fresh for newly-joined SignalR clients.
|
// long-lived Healthy drivers keep their snapshot fresh for newly-joined SignalR clients.
|
||||||
PublishHealthSnapshot();
|
PublishHealthSnapshot();
|
||||||
Timers.StartPeriodicTimer("health-poll", HealthPollTick.Instance, HealthPollInterval);
|
Timers.StartPeriodicTimer("health-poll", HealthPollTick.Instance, _healthPollInterval);
|
||||||
}
|
}
|
||||||
|
|
||||||
private void Stubbed()
|
private void Stubbed()
|
||||||
@@ -489,7 +499,58 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers
|
|||||||
// to Self (HandleSubscribeAsync already logged the underlying cause). Swallow it so it doesn't dead-letter.
|
// to Self (HandleSubscribeAsync already logged the underlying cause). Swallow it so it doesn't dead-letter.
|
||||||
Receive<SubscriptionFailed>(msg =>
|
Receive<SubscriptionFailed>(msg =>
|
||||||
_log.Debug("DriverInstance {Id}: resubscribe reported failure: {Reason}", _driverInstanceId, msg.Reason));
|
_log.Debug("DriverInstance {Id}: resubscribe reported failure: {Reason}", _driverInstanceId, msg.Reason));
|
||||||
Receive<HealthPollTick>(_ => PublishHealthSnapshot());
|
Receive<HealthPollTick>(_ =>
|
||||||
|
{
|
||||||
|
PublishHealthSnapshot();
|
||||||
|
ReconcileSubscription();
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Periodic desired-vs-actual subscription reconcile (#488), riding the existing <c>health-poll</c>
|
||||||
|
/// tick. While Connected, a driver that has a desired subscription set but no live handle is
|
||||||
|
/// re-subscribed.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// <para>
|
||||||
|
/// The desired set is otherwise only (re)applied on a <b>Connected transition</b>
|
||||||
|
/// (<see cref="ResubscribeDesired"/>) or on a deploy's <see cref="SetDesiredSubscriptions"/>.
|
||||||
|
/// Nothing re-asserted it for a subscription that was lost while the driver <i>stayed</i>
|
||||||
|
/// Connected — a failed subscribe whose retry never came, or a handle dropped down an error
|
||||||
|
/// path. That is the hole this closes.
|
||||||
|
/// </para>
|
||||||
|
/// <para>
|
||||||
|
/// <b>This is a state-machine consistency check, not a probe.</b> It can only see that this
|
||||||
|
/// actor holds no handle; it cannot detect a handle that is present but dead server-side,
|
||||||
|
/// because <c>ISubscriptionHandle</c> is opaque (a <c>DiagnosticId</c> string and nothing
|
||||||
|
/// else). Detecting that needs a liveness member on <c>ISubscribable</c> implemented across
|
||||||
|
/// all eight drivers, each with different subscription mechanics — deliberately deferred
|
||||||
|
/// until a real occurrence shows the handle outliving the subscription.
|
||||||
|
/// </para>
|
||||||
|
/// <para>
|
||||||
|
/// Gated on <see cref="ISubscribable"/>: a driver that cannot subscribe at all may still have
|
||||||
|
/// been handed a desired set, and re-telling <c>Subscribe</c> to it every tick would fail
|
||||||
|
/// every tick, forever.
|
||||||
|
/// </para>
|
||||||
|
/// <para>
|
||||||
|
/// A redundant <c>Subscribe</c> is possible but harmless: a tick sitting in the mailbox ahead
|
||||||
|
/// of an already-queued <c>Subscribe</c> observes the same "no handle" state and tells a
|
||||||
|
/// second one. <see cref="HandleSubscribeAsync"/> is a <c>ReceiveAsync</c> (so no tick can be
|
||||||
|
/// processed mid-subscribe) and drops any prior subscription before establishing the new one,
|
||||||
|
/// so the duplicate costs one extra round-trip and converges. Suppressing it would need a
|
||||||
|
/// pending-subscribe flag threaded through every self-tell site — more state than the race
|
||||||
|
/// is worth.
|
||||||
|
/// </para>
|
||||||
|
/// </remarks>
|
||||||
|
private void ReconcileSubscription()
|
||||||
|
{
|
||||||
|
if (_driver is not ISubscribable) return;
|
||||||
|
if (_desiredRefs.Count == 0 || _subscriptionHandle is not null) return;
|
||||||
|
|
||||||
|
_log.Info(
|
||||||
|
"DriverInstance {Id}: connected with {Count} desired subscription ref(s) but no live subscription handle — re-subscribing (periodic reconcile)",
|
||||||
|
_driverInstanceId, _desiredRefs.Count);
|
||||||
|
Self.Tell(new Subscribe(_desiredRefs, _desiredInterval));
|
||||||
}
|
}
|
||||||
|
|
||||||
private void Reconnecting()
|
private void Reconnecting()
|
||||||
|
|||||||
@@ -0,0 +1,132 @@
|
|||||||
|
using Shouldly;
|
||||||
|
using Xunit;
|
||||||
|
using ZB.MOM.WW.OtOpcUa.Configuration.Validation;
|
||||||
|
|
||||||
|
namespace ZB.MOM.WW.OtOpcUa.Configuration.Tests;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// <see cref="SqlCredentialGuard"/> — the shared "a Sql driver never persists a literal connection
|
||||||
|
/// string" check behind both the #498 deploy gate and the #499 node-override save gate.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// The node-override half is the one #499 is about: <c>ClusterNode.DriverConfigOverridesJson</c> is a
|
||||||
|
/// map keyed by <c>DriverInstanceId</c> that is merged onto the cluster-level <c>DriverConfig</c>, and
|
||||||
|
/// it is invisible to <c>DraftSnapshot</c> — so nothing gated it at all before.
|
||||||
|
/// </remarks>
|
||||||
|
[Trait("Category", "Unit")]
|
||||||
|
public sealed class SqlCredentialGuardTests
|
||||||
|
{
|
||||||
|
/// <summary>The literal an operator would paste into a raw-JSON textarea.</summary>
|
||||||
|
private const string Leaked =
|
||||||
|
"""{"provider":"SqlServer","connectionString":"Server=sql,1433;Database=Mes;User ID=sa;Password=hunter2"}""";
|
||||||
|
|
||||||
|
private const string Clean = """{"provider":"SqlServer","connectionStringRef":"Sql:Line3"}""";
|
||||||
|
|
||||||
|
// ---- CarriesLiteralConnectionString --------------------------------------------------------
|
||||||
|
|
||||||
|
/// <summary>The key is caught at the top level of a config blob.</summary>
|
||||||
|
[Fact]
|
||||||
|
public void A_literal_connectionString_is_detected()
|
||||||
|
=> SqlCredentialGuard.CarriesLiteralConnectionString(Leaked).ShouldBeTrue();
|
||||||
|
|
||||||
|
/// <summary>The supported indirect form is not a violation — otherwise the rule would block the fix
|
||||||
|
/// it tells operators to apply.</summary>
|
||||||
|
[Fact]
|
||||||
|
public void The_indirect_connectionStringRef_form_is_clean()
|
||||||
|
=> SqlCredentialGuard.CarriesLiteralConnectionString(Clean).ShouldBeFalse();
|
||||||
|
|
||||||
|
/// <summary>System.Text.Json binds <c>ConnectionString</c> to a <c>connectionString</c> property by
|
||||||
|
/// default, so a case variant is the same key — not a bypass.</summary>
|
||||||
|
[Theory]
|
||||||
|
[InlineData("""{"ConnectionString":"Server=x;Password=p"}""")]
|
||||||
|
[InlineData("""{"CONNECTIONSTRING":"Server=x;Password=p"}""")]
|
||||||
|
[InlineData("""{"connectionstring":"Server=x;Password=p"}""")]
|
||||||
|
public void Case_variants_are_the_same_key(string json)
|
||||||
|
=> SqlCredentialGuard.CarriesLiteralConnectionString(json).ShouldBeTrue();
|
||||||
|
|
||||||
|
/// <summary>Blank, malformed and non-object blobs simply have no keys. Shaping the config JSON is
|
||||||
|
/// another rule's job, and throwing here would turn a formatting mistake into a credential failure.
|
||||||
|
/// </summary>
|
||||||
|
[Theory]
|
||||||
|
[InlineData(null)]
|
||||||
|
[InlineData("")]
|
||||||
|
[InlineData(" ")]
|
||||||
|
[InlineData("{ not json")]
|
||||||
|
[InlineData("[1,2,3]")]
|
||||||
|
[InlineData("\"a string\"")]
|
||||||
|
public void Blank_malformed_and_non_object_blobs_are_not_violations(string? json)
|
||||||
|
=> SqlCredentialGuard.CarriesLiteralConnectionString(json).ShouldBeFalse();
|
||||||
|
|
||||||
|
/// <summary>Only the top level is scanned. The DTO is flat, so a nested occurrence cannot bind and is
|
||||||
|
/// not the credential-shaped mistake this rule exists to catch.</summary>
|
||||||
|
[Fact]
|
||||||
|
public void A_nested_occurrence_is_not_flagged()
|
||||||
|
=> SqlCredentialGuard
|
||||||
|
.CarriesLiteralConnectionString("""{"nested":{"connectionString":"Server=x"}}""")
|
||||||
|
.ShouldBeFalse();
|
||||||
|
|
||||||
|
// ---- FindNodeOverrideViolations (#499) -----------------------------------------------------
|
||||||
|
|
||||||
|
/// <summary>The #499 leak: a credential pasted into a node's per-driver override map.</summary>
|
||||||
|
[Fact]
|
||||||
|
public void A_credential_in_a_node_override_for_a_Sql_driver_is_a_violation()
|
||||||
|
{
|
||||||
|
var overrides = $$"""{"di-sql": {{Leaked}} }""";
|
||||||
|
|
||||||
|
SqlCredentialGuard.FindNodeOverrideViolations(overrides, ["di-sql"])
|
||||||
|
.ShouldBe(["di-sql"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>Every offending instance is reported, not just the first — an operator fixing one at a
|
||||||
|
/// time would otherwise need as many save attempts as there are leaks.</summary>
|
||||||
|
[Fact]
|
||||||
|
public void Every_offending_instance_is_reported()
|
||||||
|
{
|
||||||
|
var overrides = $$"""{"di-a": {{Leaked}}, "di-clean": {{Clean}}, "di-b": {{Leaked}} }""";
|
||||||
|
|
||||||
|
SqlCredentialGuard.FindNodeOverrideViolations(overrides, ["di-a", "di-b", "di-clean"])
|
||||||
|
.ShouldBe(["di-a", "di-b"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>Keys outside the Sql set are ignored. A non-Sql driver never made the
|
||||||
|
/// indirect-credential guarantee, so flagging it would break a config that is legitimate today —
|
||||||
|
/// a regression, not defence in depth.</summary>
|
||||||
|
[Fact]
|
||||||
|
public void An_override_for_a_non_Sql_driver_is_ignored()
|
||||||
|
{
|
||||||
|
var overrides = $$"""{"di-modbus": {{Leaked}} }""";
|
||||||
|
|
||||||
|
SqlCredentialGuard.FindNodeOverrideViolations(overrides, ["di-sql"]).ShouldBeEmpty();
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>The realistic override — a node-specific endpoint — must keep saving.</summary>
|
||||||
|
[Fact]
|
||||||
|
public void A_normal_override_is_clean()
|
||||||
|
{
|
||||||
|
var overrides = """{"di-sql": {"commandTimeoutSeconds": 45}}""";
|
||||||
|
|
||||||
|
SqlCredentialGuard.FindNodeOverrideViolations(overrides, ["di-sql"]).ShouldBeEmpty();
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>Nothing to check when the cluster has no Sql drivers, or the node has no overrides.</summary>
|
||||||
|
[Theory]
|
||||||
|
[InlineData(null)]
|
||||||
|
[InlineData("")]
|
||||||
|
[InlineData(" ")]
|
||||||
|
[InlineData("{ not json")]
|
||||||
|
[InlineData("[1,2,3]")]
|
||||||
|
public void Blank_and_malformed_override_maps_yield_no_violations(string? overrides)
|
||||||
|
=> SqlCredentialGuard.FindNodeOverrideViolations(overrides, ["di-sql"]).ShouldBeEmpty();
|
||||||
|
|
||||||
|
/// <summary>An empty Sql-driver set short-circuits — there is no instance the key could belong to.</summary>
|
||||||
|
[Fact]
|
||||||
|
public void No_Sql_drivers_means_no_violations()
|
||||||
|
=> SqlCredentialGuard.FindNodeOverrideViolations($$"""{"di-sql": {{Leaked}} }""", [])
|
||||||
|
.ShouldBeEmpty();
|
||||||
|
|
||||||
|
/// <summary>A non-object override entry cannot carry config keys, and must not throw.</summary>
|
||||||
|
[Fact]
|
||||||
|
public void A_non_object_override_entry_is_skipped()
|
||||||
|
=> SqlCredentialGuard.FindNodeOverrideViolations("""{"di-sql": "connectionString"}""", ["di-sql"])
|
||||||
|
.ShouldBeEmpty();
|
||||||
|
}
|
||||||
+151
-49
@@ -10,59 +10,109 @@ using ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Runtime;
|
|||||||
namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests.Runtime;
|
namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests.Runtime;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// PR 6.2 — pins the EventPump's bounded-channel + drop-newest behavior. We
|
/// PR 6.2 — pins the EventPump's bounded-channel + drop-newest behavior. We hold the dispatch loop
|
||||||
/// hold the dispatch loop with a slow handler so the channel fills, then verify
|
/// with a blocking handler so the channel fills, then verify the producer keeps reading from the gw
|
||||||
/// the producer keeps reading from the gw stream and increments the
|
/// stream and increments the <c>galaxy.events.dropped</c> counter rather than blocking.
|
||||||
/// <c>galaxy.events.dropped</c> counter rather than blocking.
|
|
||||||
/// </summary>
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// These tests observe two background loops (<c>RunAsync</c> / <c>DispatchLoopAsync</c>) through a
|
||||||
|
/// process-wide <see cref="Meter"/>, so every wait here is a <b>presence</b> assertion polled up to
|
||||||
|
/// <see cref="Budget"/> — never a fixed sleep. A fixed sleep is a bet that both loops get scheduled
|
||||||
|
/// within it, and #503 is what losing that bet looks like when the box is busy running other test
|
||||||
|
/// assemblies.
|
||||||
|
/// </remarks>
|
||||||
public sealed class EventPumpBoundedChannelTests
|
public sealed class EventPumpBoundedChannelTests
|
||||||
{
|
{
|
||||||
/// <summary>Verifies that the event pump drops newest events when the bounded channel fills and records metrics for dropped events.</summary>
|
/// <summary>Upper bound on how long a poll waits for a background loop to reach its terminal state.
|
||||||
|
/// Generous on purpose: it is spent only when something is genuinely broken, and it cannot turn a
|
||||||
|
/// failing assertion into a passing one.</summary>
|
||||||
|
private static readonly TimeSpan Budget = TimeSpan.FromSeconds(15);
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// With the dispatch loop genuinely held and the channel therefore full, the producer keeps
|
||||||
|
/// draining the gw stream and counts every rejected write on <c>galaxy.events.dropped</c> —
|
||||||
|
/// newest-dropped, not back-pressure.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// <para>
|
||||||
|
/// The arrangement is staged so the arithmetic is exact rather than scheduler-dependent
|
||||||
|
/// (#503): emit ONE event, wait until the dispatcher has actually entered the handler (so it
|
||||||
|
/// holds that event and the channel is empty), and only then emit the remaining nine. The
|
||||||
|
/// channel accepts <c>capacity</c> of them and the rest must be dropped —
|
||||||
|
/// <c>10 − 1 held − 2 buffered = 7</c>, every run.
|
||||||
|
/// </para>
|
||||||
|
/// </remarks>
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task Drops_newest_when_channel_fills_and_records_metric()
|
public async Task Drops_newest_when_channel_fills_and_records_metric()
|
||||||
{
|
{
|
||||||
var counters = StartMeterCapture();
|
const int totalEvents = 10;
|
||||||
|
const int channelCapacity = 2;
|
||||||
|
// One event is held inside the handler; `channelCapacity` more fit in the channel; the rest are dropped.
|
||||||
|
const int expectedDropped = totalEvents - 1 - channelCapacity;
|
||||||
|
|
||||||
|
// Unique per run: the counters are filtered to this exact galaxy.client tag, which is what keeps a
|
||||||
|
// parallel sibling test's pump out of them (#503). A literal name would work today and break the
|
||||||
|
// day someone reuses it.
|
||||||
|
var clientName = $"PumpTest-{Guid.NewGuid():N}";
|
||||||
|
var counters = StartMeterCapture(clientName);
|
||||||
|
|
||||||
|
// A SYNCHRONOUS gate, deliberately. OnDataChange is EventHandler<T> — void-returning — so an
|
||||||
|
// `async (_, _) => await gate` lambda is async void: it returns to the dispatch loop at the first
|
||||||
|
// await and holds nothing. That is exactly what #503 was: the loop drained freely, drops happened
|
||||||
|
// only when the producer happened to outrun the consumer, and the assertions rode on scheduling
|
||||||
|
// luck. Blocking the loop's own thread is what makes "the dispatcher is held" true.
|
||||||
|
var dispatchGate = new ManualResetEventSlim(false);
|
||||||
|
var handlerEntered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||||
|
EventPump? pump = null;
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
var subscriber = new ManualSubscriber();
|
var subscriber = new ManualSubscriber();
|
||||||
var registry = new SubscriptionRegistry();
|
var registry = new SubscriptionRegistry();
|
||||||
registry.Register(1, [new TagBinding("Tag.A", ItemHandle: 7)]);
|
registry.Register(1, [new TagBinding("Tag.A", ItemHandle: 7)]);
|
||||||
|
|
||||||
// Tiny channel + slow handler ⇒ producer hits FullMode.Wait → TryWrite false
|
pump = new EventPump(
|
||||||
// for every overflow event.
|
subscriber, registry, channelCapacity: channelCapacity, clientName: clientName);
|
||||||
var dispatchGate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
|
pump.OnDataChange += (_, _) =>
|
||||||
await using var pump = new EventPump(
|
|
||||||
subscriber, registry, channelCapacity: 2, clientName: "PumpTest");
|
|
||||||
pump.OnDataChange += async (_, _) =>
|
|
||||||
{
|
{
|
||||||
// Block the dispatch loop until we've shoved enough events through to
|
handlerEntered.TrySetResult();
|
||||||
// overflow the bounded channel. Consume the gate exactly once.
|
dispatchGate.Wait();
|
||||||
await dispatchGate.Task.ConfigureAwait(false);
|
|
||||||
};
|
};
|
||||||
pump.Start();
|
pump.Start();
|
||||||
|
|
||||||
const int totalEvents = 10;
|
// Stage 1 — one event, then wait for the dispatcher to take it and block. After this the
|
||||||
for (var i = 0; i < totalEvents; i++)
|
// channel is provably empty and exactly one event is in flight inside the handler.
|
||||||
|
await subscriber.EmitAsync(itemHandle: 7, value: 0);
|
||||||
|
await handlerEntered.Task.WaitAsync(Budget);
|
||||||
|
|
||||||
|
// Stage 2 — the rest. `channelCapacity` are accepted; the remainder must be dropped.
|
||||||
|
for (var i = 1; i < totalEvents; i++)
|
||||||
{
|
{
|
||||||
await subscriber.EmitAsync(itemHandle: 7, value: i);
|
await subscriber.EmitAsync(itemHandle: 7, value: i);
|
||||||
}
|
}
|
||||||
// Give the producer a beat to run TryWrite for every event.
|
|
||||||
await Task.Delay(150);
|
|
||||||
|
|
||||||
// Capacity 2 + 1 in-flight in the dispatcher = 3 may have been accepted; the
|
// PRESENCE assertion on the discriminator (#500's rule): poll until the drop count reaches its
|
||||||
// remainder should have hit the dropped counter. Don't pin exact counts —
|
// terminal value rather than sleeping a fixed 150 ms and hoping the producer got scheduled.
|
||||||
// the scheduler can interleave; pin the invariants instead.
|
// The budget is an upper bound before giving up — spending it costs nothing on the happy path,
|
||||||
counters.Received.ShouldBeGreaterThanOrEqualTo(totalEvents);
|
// and no amount of it can make a genuinely broken pump pass. Dropped starts at 0, so this
|
||||||
counters.Dropped.ShouldBeGreaterThan(0,
|
// cannot be satisfied by the initial state.
|
||||||
"with capacity=2 and a held dispatcher we must drop at least one of 10 events");
|
await WaitUntilAsync(() => counters.Dropped == expectedDropped,
|
||||||
(counters.Received).ShouldBe(counters.Dispatched + counters.Dropped + counters.InFlight,
|
$"expected {expectedDropped} dropped events");
|
||||||
"received = dispatched + dropped + (events still queued)");
|
|
||||||
|
|
||||||
// Release the dispatcher so DisposeAsync can drain cleanly.
|
counters.Received.ShouldBe(totalEvents,
|
||||||
dispatchGate.TrySetResult();
|
"the producer must keep reading the gw stream through the overflow, not back-pressure");
|
||||||
|
counters.Dropped.ShouldBe(expectedDropped);
|
||||||
|
counters.Dispatched.ShouldBe(0,
|
||||||
|
"the dispatch loop is still blocked in the handler, so nothing has completed dispatch — " +
|
||||||
|
"if this is non-zero the handler is not actually holding the loop and the test proves nothing");
|
||||||
}
|
}
|
||||||
finally
|
finally
|
||||||
{
|
{
|
||||||
|
// Release BEFORE disposing: DisposeAsync awaits the dispatch loop, and that loop is parked on
|
||||||
|
// this gate. Releasing it here (not at the end of the try) is what keeps a failed assertion a
|
||||||
|
// failure instead of a hang.
|
||||||
|
dispatchGate.Set();
|
||||||
|
if (pump is not null) await pump.DisposeAsync();
|
||||||
|
dispatchGate.Dispose();
|
||||||
counters.Dispose();
|
counters.Dispose();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -110,23 +160,21 @@ public sealed class EventPumpBoundedChannelTests
|
|||||||
// Poll until at least one galaxy.events.received measurement tagged
|
// Poll until at least one galaxy.events.received measurement tagged
|
||||||
// galaxy.client=Driver-X lands in the listener, rather than using a
|
// galaxy.client=Driver-X lands in the listener, rather than using a
|
||||||
// fixed delay that races under parallel test load on a busy box.
|
// fixed delay that races under parallel test load on a busy box.
|
||||||
var deadline = DateTime.UtcNow.AddSeconds(5);
|
// Budget, not 5 s: the same contention that produced #503 stretches how long the
|
||||||
bool found = false;
|
// producer loop takes to be scheduled, and a presence poll costs nothing to over-budget.
|
||||||
while (DateTime.UtcNow < deadline)
|
await WaitUntilAsync(
|
||||||
{
|
() =>
|
||||||
listener.RecordObservableInstruments();
|
|
||||||
bool hasMatch;
|
|
||||||
lock (captured)
|
|
||||||
{
|
{
|
||||||
hasMatch = captured.Any(c =>
|
listener.RecordObservableInstruments();
|
||||||
c.Instrument == "galaxy.events.received" &&
|
lock (captured)
|
||||||
c.Tags.Any(t => t.Key == "galaxy.client" &&
|
{
|
||||||
string.Equals((string?)t.Value, "Driver-X", StringComparison.Ordinal)));
|
return captured.Any(c =>
|
||||||
}
|
c.Instrument == "galaxy.events.received" &&
|
||||||
if (hasMatch) { found = true; break; }
|
c.Tags.Any(t => t.Key == "galaxy.client" &&
|
||||||
await Task.Delay(25);
|
string.Equals((string?)t.Value, "Driver-X", StringComparison.Ordinal)));
|
||||||
}
|
}
|
||||||
_ = found; // assertion happens below after dispose
|
},
|
||||||
|
"a galaxy.events.received measurement tagged galaxy.client=Driver-X");
|
||||||
}
|
}
|
||||||
|
|
||||||
// The static Meter is shared across all EventPump instances in the test
|
// The static Meter is shared across all EventPump instances in the test
|
||||||
@@ -146,7 +194,46 @@ public sealed class EventPumpBoundedChannelTests
|
|||||||
ours.ShouldContain(c => c.Instrument == "galaxy.events.received");
|
ours.ShouldContain(c => c.Instrument == "galaxy.events.received");
|
||||||
}
|
}
|
||||||
|
|
||||||
private static CounterCapture StartMeterCapture()
|
/// <summary>Polls <paramref name="condition"/> until it holds or <see cref="Budget"/> elapses, failing
|
||||||
|
/// with <paramref name="what"/> on timeout. The presence-assertion counterpart to a fixed delay.</summary>
|
||||||
|
private static async Task WaitUntilAsync(Func<bool> condition, string what)
|
||||||
|
{
|
||||||
|
var deadline = DateTime.UtcNow + Budget;
|
||||||
|
while (DateTime.UtcNow < deadline)
|
||||||
|
{
|
||||||
|
if (condition()) return;
|
||||||
|
await Task.Delay(10);
|
||||||
|
}
|
||||||
|
condition().ShouldBeTrue($"timed out after {Budget.TotalSeconds:0}s waiting: {what}");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Captures the three EventPump counters for <b>one specific pump</b>, identified by its
|
||||||
|
/// <c>galaxy.client</c> tag.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// <para>
|
||||||
|
/// The <c>galaxy.client</c> filter is the fix for #503, and it is not optional. The pump's
|
||||||
|
/// <see cref="Meter"/> is <b>static</b> — one instance for the whole process — and
|
||||||
|
/// <c>InstrumentPublished</c> can only select by <i>meter</i>, so an unfiltered listener
|
||||||
|
/// receives measurements from every <see cref="EventPump"/> alive anywhere in the assembly.
|
||||||
|
/// xUnit runs test classes in parallel and several of them build pumps
|
||||||
|
/// (<c>EventPumpStreamFaultTests</c>, the <c>Driver-X</c> test below), so a sibling's events
|
||||||
|
/// landed in these counters — that is why the flake needed a busy box: it needed the two
|
||||||
|
/// tests to overlap in time. The leak was measured directly, as <c>Received = 11</c> in a run
|
||||||
|
/// that emitted 10.
|
||||||
|
/// </para>
|
||||||
|
/// <para>
|
||||||
|
/// The old <c>Received == Dispatched + Dropped + InFlight</c> assertion is what turned the
|
||||||
|
/// leak into a failure: a foreign increment landing between its four separate
|
||||||
|
/// <see cref="Interlocked.Read(ref long)"/> calls made the two sides disagree. Hence the
|
||||||
|
/// reported subject, <c>counters.Received</c>.
|
||||||
|
/// </para>
|
||||||
|
/// </remarks>
|
||||||
|
/// <param name="clientName">The <c>galaxy.client</c> tag value to accept. Must be unique to the
|
||||||
|
/// calling test — pass a fresh GUID-suffixed name rather than a literal, so a future sibling test
|
||||||
|
/// cannot silently reintroduce the leak by reusing the same name.</param>
|
||||||
|
private static CounterCapture StartMeterCapture(string clientName)
|
||||||
{
|
{
|
||||||
var capture = new CounterCapture();
|
var capture = new CounterCapture();
|
||||||
var listener = new MeterListener();
|
var listener = new MeterListener();
|
||||||
@@ -154,8 +241,20 @@ public sealed class EventPumpBoundedChannelTests
|
|||||||
{
|
{
|
||||||
if (instr.Meter.Name == EventPump.MeterName) l.EnableMeasurementEvents(instr);
|
if (instr.Meter.Name == EventPump.MeterName) l.EnableMeasurementEvents(instr);
|
||||||
};
|
};
|
||||||
listener.SetMeasurementEventCallback<long>((instr, value, _, _) =>
|
listener.SetMeasurementEventCallback<long>((instr, value, tags, _) =>
|
||||||
{
|
{
|
||||||
|
var mine = false;
|
||||||
|
foreach (var tag in tags)
|
||||||
|
{
|
||||||
|
if (tag.Key == "galaxy.client" &&
|
||||||
|
string.Equals((string?)tag.Value, clientName, StringComparison.Ordinal))
|
||||||
|
{
|
||||||
|
mine = true;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (!mine) return;
|
||||||
|
|
||||||
switch (instr.Name)
|
switch (instr.Name)
|
||||||
{
|
{
|
||||||
case "galaxy.events.received": Interlocked.Add(ref capture._received, value); break;
|
case "galaxy.events.received": Interlocked.Add(ref capture._received, value); break;
|
||||||
@@ -178,8 +277,11 @@ public sealed class EventPumpBoundedChannelTests
|
|||||||
public long Dispatched => Interlocked.Read(ref _dispatched);
|
public long Dispatched => Interlocked.Read(ref _dispatched);
|
||||||
/// <summary>Gets the count of dropped events.</summary>
|
/// <summary>Gets the count of dropped events.</summary>
|
||||||
public long Dropped => Interlocked.Read(ref _dropped);
|
public long Dropped => Interlocked.Read(ref _dropped);
|
||||||
/// <summary>Gets the count of in-flight events.</summary>
|
// There is deliberately no InFlight member. It could only be DERIVED as
|
||||||
public long InFlight => Math.Max(0, Received - Dispatched - Dropped);
|
// Received - Dispatched - Dropped, which made the old
|
||||||
|
// `Received == Dispatched + Dropped + InFlight` assertion a tautology: it could not fail on a
|
||||||
|
// real defect, only on a torn read across the four separate Interlocked.Read calls. Assert the
|
||||||
|
// three measured counters directly instead (#503).
|
||||||
/// <summary>Disposes the meter listener.</summary>
|
/// <summary>Disposes the meter listener.</summary>
|
||||||
public void Dispose() => Listener?.Dispose();
|
public void Dispose() => Listener?.Dispose();
|
||||||
}
|
}
|
||||||
|
|||||||
+31
-5
@@ -86,8 +86,19 @@ public sealed class DriverHostActorNativeAlarmAckRoutingTests : RuntimeActorTest
|
|||||||
|
|
||||||
actor.Tell(new DriverHostActor.RouteNativeAlarmAck("Plant/gw/dev1/does-not-exist", "cmt", "alice"));
|
actor.Tell(new DriverHostActor.RouteNativeAlarmAck("Plant/gw/dev1/does-not-exist", "cmt", "alice"));
|
||||||
|
|
||||||
// Give the (fire-and-forget) handler time to run; the unmapped node must produce no ack.
|
// POSITIVE CONTROL, not a sleep (#501). `Acks` is empty at t=0, so any assertion made before the
|
||||||
AwaitAssert(() => recorder.Acks.ShouldBeEmpty(), duration: TimeSpan.FromMilliseconds(800));
|
// host has processed the message above passes without testing anything — and AwaitAssert returns on
|
||||||
|
// its FIRST success, so it spends none of its duration and is the wrong tool for an absence.
|
||||||
|
//
|
||||||
|
// Instead send a MAPPED ack afterwards. The host processes its mailbox in order and forwards to the
|
||||||
|
// child via Tell, and the child handles RouteAlarmAck with ReceiveAsync (which suspends its own
|
||||||
|
// mailbox), so by the time this one reaches the driver the unmapped one has provably already been
|
||||||
|
// handled. Whatever `Acks` holds then is the complete answer.
|
||||||
|
actor.Tell(new DriverHostActor.RouteNativeAlarmAck(AlarmRawPath, "control", "alice"));
|
||||||
|
|
||||||
|
AwaitAssert(() => recorder.Acks.ShouldContain(a => a.Comment == "control"), duration: Timeout);
|
||||||
|
recorder.Acks.Count.ShouldBe(1,
|
||||||
|
"only the control ack may reach the driver — the unmapped condition NodeId must have been dropped");
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>On a SECONDARY node the ack is gated off (same primary gate as the inbound write path): the
|
/// <summary>On a SECONDARY node the ack is gated off (same primary gate as the inbound write path): the
|
||||||
@@ -110,10 +121,25 @@ public sealed class DriverHostActorNativeAlarmAckRoutingTests : RuntimeActorTest
|
|||||||
},
|
},
|
||||||
CorrelationId.NewId()));
|
CorrelationId.NewId()));
|
||||||
|
|
||||||
actor.Tell(new DriverHostActor.RouteNativeAlarmAck(AlarmRawPath, "cmt", "alice"));
|
actor.Tell(new DriverHostActor.RouteNativeAlarmAck(AlarmRawPath, "gated", "alice"));
|
||||||
|
|
||||||
// No ack reached the driver — the gate short-circuited before the inverse-map lookup.
|
// POSITIVE CONTROL (#501) — same reasoning as the unmapped-node test, with the role itself as the
|
||||||
AwaitAssert(() => recorder.Acks.ShouldBeEmpty(), duration: TimeSpan.FromMilliseconds(800));
|
// control: return the node to PRIMARY and re-send the SAME mapped ack. Mailbox ordering means that
|
||||||
|
// once this one reaches the driver, the Secondary-gated one has already been processed. The
|
||||||
|
// discriminator is the comment, so a leaked ack is visible rather than merged into the count.
|
||||||
|
actor.Tell(new RedundancyStateChanged(
|
||||||
|
new[]
|
||||||
|
{
|
||||||
|
new NodeRedundancyState(TestNode, RedundancyRole.Primary,
|
||||||
|
IsClusterLeader: true, IsDriverPrimary: true, AsOfUtc: DateTime.UtcNow),
|
||||||
|
},
|
||||||
|
CorrelationId.NewId()));
|
||||||
|
actor.Tell(new DriverHostActor.RouteNativeAlarmAck(AlarmRawPath, "control", "alice"));
|
||||||
|
|
||||||
|
AwaitAssert(() => recorder.Acks.ShouldContain(a => a.Comment == "control"), duration: Timeout);
|
||||||
|
recorder.Acks.ShouldNotContain(a => a.Comment == "gated",
|
||||||
|
"the Primary gate must have dropped the ack sent while this node was Secondary");
|
||||||
|
recorder.Acks.Count.ShouldBe(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>Spawns the host with the recording alarm-driver factory, dispatches the deployment, and waits
|
/// <summary>Spawns the host with the recording alarm-driver factory, dispatches the deployment, and waits
|
||||||
|
|||||||
+122
@@ -0,0 +1,122 @@
|
|||||||
|
using Akka.Actor;
|
||||||
|
using Shouldly;
|
||||||
|
using Xunit;
|
||||||
|
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
||||||
|
using ZB.MOM.WW.OtOpcUa.Runtime.Drivers;
|
||||||
|
using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness;
|
||||||
|
|
||||||
|
namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Gitea #488 — the periodic desired-vs-actual subscription reconcile that rides
|
||||||
|
/// <see cref="DriverInstanceActor"/>'s existing <c>health-poll</c> tick.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// <para>
|
||||||
|
/// The hole it closes: the desired ref set is re-applied on a <b>Connected transition</b> and on a
|
||||||
|
/// deploy's <c>SetDesiredSubscriptions</c>, and nowhere else. A subscription lost while the driver
|
||||||
|
/// <i>stayed</i> Connected — the shape reproduced here, a subscribe that failed and was never
|
||||||
|
/// retried — left the actor permanently subscribed to nothing while reporting Healthy.
|
||||||
|
/// </para>
|
||||||
|
/// <para>
|
||||||
|
/// All three tests drive a short <c>healthPollInterval</c> rather than the production 30 s. The
|
||||||
|
/// two absence assertions are bounded by a positive control (a counted number of ticks provably
|
||||||
|
/// processed), never by a bare sleep.
|
||||||
|
/// </para>
|
||||||
|
/// </remarks>
|
||||||
|
public sealed class DriverInstanceActorSubscriptionReconcileTests : RuntimeActorTestBase
|
||||||
|
{
|
||||||
|
private static readonly TimeSpan FastPoll = TimeSpan.FromMilliseconds(100);
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// The reconcile itself: the first subscribe fails, leaving the actor Connected with a desired set
|
||||||
|
/// and no handle. Nothing else would ever retry it — no reconnect, no deploy — so a second
|
||||||
|
/// subscribe can only come from the periodic reconcile.
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public void A_lost_subscription_is_re_established_while_still_connected()
|
||||||
|
{
|
||||||
|
var driver = new SubscribableStubDriver { SubscribeFailuresBeforeSuccess = 1 };
|
||||||
|
var actor = Sys.ActorOf(DriverInstanceActor.Props(driver, healthPollInterval: FastPoll));
|
||||||
|
|
||||||
|
actor.Tell(new DriverInstanceActor.SetDesiredSubscriptions(
|
||||||
|
new[] { "tag-a" }, TimeSpan.FromMilliseconds(100)));
|
||||||
|
actor.Tell(new DriverInstanceActor.InitializeRequested("{}"));
|
||||||
|
|
||||||
|
// The connect-path subscribe runs and throws, so the handle stays null.
|
||||||
|
AwaitCondition(() => driver.SubscribeCount >= 1, PresenceBudget);
|
||||||
|
|
||||||
|
// Only the reconcile can produce this second attempt — and it must succeed this time.
|
||||||
|
AwaitCondition(() => driver.SubscribeCount >= 2, PresenceBudget);
|
||||||
|
driver.LastSubscribedRefs.ShouldBe(new[] { "tag-a" });
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// A healthy subscription is left alone — the reconcile must not re-subscribe on every tick, which
|
||||||
|
/// would churn the backend every 30 s in production for no reason.
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public void A_healthy_subscription_is_not_re_subscribed_on_every_tick()
|
||||||
|
{
|
||||||
|
var publisher = new CountingHealthPublisher();
|
||||||
|
var driver = new SubscribableStubDriver();
|
||||||
|
var actor = Sys.ActorOf(DriverInstanceActor.Props(
|
||||||
|
driver, healthPublisher: publisher, healthPollInterval: FastPoll));
|
||||||
|
|
||||||
|
actor.Tell(new DriverInstanceActor.SetDesiredSubscriptions(
|
||||||
|
new[] { "tag-a" }, TimeSpan.FromMilliseconds(100)));
|
||||||
|
actor.Tell(new DriverInstanceActor.InitializeRequested("{}"));
|
||||||
|
AwaitCondition(() => driver.SubscribeCount >= 1, PresenceBudget);
|
||||||
|
|
||||||
|
// POSITIVE CONTROL for the absence below. Every health-poll tick publishes a snapshot, so the
|
||||||
|
// publisher count is a direct observation of ticks PROCESSED. Waiting for several of them is what
|
||||||
|
// makes "SubscribeCount is still 1" mean "the reconcile ran and declined" rather than "the timer
|
||||||
|
// had not fired yet" — the latter would pass instantly and prove nothing.
|
||||||
|
var before = publisher.Count;
|
||||||
|
AwaitCondition(() => publisher.Count >= before + 4, PresenceBudget);
|
||||||
|
|
||||||
|
driver.SubscribeCount.ShouldBe(1,
|
||||||
|
"the handle is live, so the reconcile must leave it alone");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// A driver that is not <see cref="ISubscribable"/> can still be handed a desired set. Reconciling
|
||||||
|
/// it would tell <c>Subscribe</c> every tick and fail every tick, forever — so the reconcile is
|
||||||
|
/// gated on the capability.
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public void A_non_subscribable_driver_is_never_reconciled()
|
||||||
|
{
|
||||||
|
var publisher = new CountingHealthPublisher();
|
||||||
|
var driver = new StubDriver(); // IDriver only — no ISubscribable
|
||||||
|
var actor = Sys.ActorOf(DriverInstanceActor.Props(
|
||||||
|
driver, healthPublisher: publisher, healthPollInterval: FastPoll));
|
||||||
|
|
||||||
|
actor.Tell(new DriverInstanceActor.SetDesiredSubscriptions(
|
||||||
|
new[] { "tag-a" }, TimeSpan.FromMilliseconds(100)));
|
||||||
|
actor.Tell(new DriverInstanceActor.InitializeRequested("{}"));
|
||||||
|
|
||||||
|
// Same positive control: assert the reconcile stays silent across several PROCESSED ticks.
|
||||||
|
// EventFilter bounds the absence to this block, and the reconcile announces itself at Info —
|
||||||
|
// the first test above is the proof that this message does appear when the reconcile fires.
|
||||||
|
EventFilter.Info(contains: "periodic reconcile").Expect(0, () =>
|
||||||
|
{
|
||||||
|
var before = publisher.Count;
|
||||||
|
AwaitCondition(() => publisher.Count >= before + 4, PresenceBudget);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>Counts health-snapshot publishes, which is one per <c>health-poll</c> tick once the actor
|
||||||
|
/// is running — the observable that turns "time passed" into "ticks were processed".</summary>
|
||||||
|
private sealed class CountingHealthPublisher : IDriverHealthPublisher
|
||||||
|
{
|
||||||
|
private int _count;
|
||||||
|
|
||||||
|
/// <summary>Number of snapshots published so far.</summary>
|
||||||
|
public int Count => Volatile.Read(ref _count);
|
||||||
|
|
||||||
|
/// <inheritdoc />
|
||||||
|
public void Publish(string clusterId, string driverInstanceId, DriverHealth health, int errorCount5Min)
|
||||||
|
=> Interlocked.Increment(ref _count);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -114,6 +114,18 @@ internal sealed class SubscribableStubDriver : StubDriver, ISubscribable
|
|||||||
/// synchronously-completed stub task hides (the continuation otherwise runs inline).</summary>
|
/// synchronously-completed stub task hides (the continuation otherwise runs inline).</summary>
|
||||||
public bool UnsubscribeYields { get; set; }
|
public bool UnsubscribeYields { get; set; }
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// How many of the first <see cref="SubscribeAsync"/> calls throw before one succeeds. Default 0
|
||||||
|
/// (always succeed) leaves existing behaviour untouched.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// This is how a test reaches the state the #488 reconcile exists for: the actor is Connected and
|
||||||
|
/// holds a desired ref set, but the subscribe that should have established the handle failed, so
|
||||||
|
/// <c>_subscriptionHandle</c> is null and nothing else will ever re-assert it — there is no
|
||||||
|
/// connect transition coming, and no deploy.
|
||||||
|
/// </remarks>
|
||||||
|
public int SubscribeFailuresBeforeSuccess { get; set; }
|
||||||
|
|
||||||
/// <summary>Subscribes to the specified full references.</summary>
|
/// <summary>Subscribes to the specified full references.</summary>
|
||||||
/// <param name="fullReferences">The full references to subscribe to.</param>
|
/// <param name="fullReferences">The full references to subscribe to.</param>
|
||||||
/// <param name="publishingInterval">The publishing interval.</param>
|
/// <param name="publishingInterval">The publishing interval.</param>
|
||||||
@@ -121,8 +133,13 @@ internal sealed class SubscribableStubDriver : StubDriver, ISubscribable
|
|||||||
public Task<ISubscriptionHandle> SubscribeAsync(
|
public Task<ISubscriptionHandle> SubscribeAsync(
|
||||||
IReadOnlyList<string> fullReferences, TimeSpan publishingInterval, CancellationToken cancellationToken)
|
IReadOnlyList<string> fullReferences, TimeSpan publishingInterval, CancellationToken cancellationToken)
|
||||||
{
|
{
|
||||||
Interlocked.Increment(ref SubscribeCount);
|
var attempt = Interlocked.Increment(ref SubscribeCount);
|
||||||
LastSubscribedRefs = fullReferences;
|
LastSubscribedRefs = fullReferences;
|
||||||
|
if (attempt <= SubscribeFailuresBeforeSuccess)
|
||||||
|
{
|
||||||
|
throw new InvalidOperationException($"stub subscribe failure #{attempt}");
|
||||||
|
}
|
||||||
|
|
||||||
return Task.FromResult<ISubscriptionHandle>(_handle);
|
return Task.FromResult<ISubscriptionHandle>(_handle);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -78,7 +78,9 @@ public sealed class PeerProbeSupervisorTests : RuntimeActorTestBase
|
|||||||
duration: PresenceBudget);
|
duration: PresenceBudget);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>Verifies a single-node snapshot (just the local node) spawns no children.</summary>
|
/// <summary>Verifies a single-node snapshot (just the local node) spawns no children — the local node
|
||||||
|
/// is never its own probe peer. Established against a following two-node snapshot as a positive
|
||||||
|
/// control, so the count of 1 (rather than 2) is what carries the "no local child" claim.</summary>
|
||||||
[Fact]
|
[Fact]
|
||||||
public void Single_node_snapshot_spawns_no_children()
|
public void Single_node_snapshot_spawns_no_children()
|
||||||
{
|
{
|
||||||
@@ -87,7 +89,20 @@ public sealed class PeerProbeSupervisorTests : RuntimeActorTestBase
|
|||||||
|
|
||||||
sup.Tell(Snapshot(State(Local, RedundancyRole.Primary)));
|
sup.Tell(Snapshot(State(Local, RedundancyRole.Primary)));
|
||||||
|
|
||||||
AwaitAssert(() => sup.UnderlyingActor.ChildCount.ShouldBe(0),
|
// POSITIVE CONTROL (#501). This is an ABSENCE assertion, and ChildCount is 0 before the snapshot
|
||||||
|
// above has been processed at all — so asserting it directly passes at t=0 and proves nothing
|
||||||
|
// (AwaitAssert returns on its first success, spending none of its duration). Unlike the other
|
||||||
|
// ChildCount.ShouldBe(0) waits in this class, there is no preceding ShouldBe(1) to make it a
|
||||||
|
// transition.
|
||||||
|
//
|
||||||
|
// Follow with a snapshot that DOES spawn exactly one child. The supervisor processes its mailbox
|
||||||
|
// in order, so once the count reaches 1 the single-node snapshot has provably been handled — and
|
||||||
|
// had it spawned a child for the local node, the count would be 2.
|
||||||
|
sup.Tell(Snapshot(
|
||||||
|
State(Local, RedundancyRole.Primary),
|
||||||
|
State(Peer, RedundancyRole.Secondary)));
|
||||||
|
|
||||||
|
AwaitAssert(() => sup.UnderlyingActor.ChildCount.ShouldBe(1),
|
||||||
duration: PresenceBudget);
|
duration: PresenceBudget);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user