From ce6d9e21d36e7c3ed5241c70a1eda9c9518f11cc Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 11:36:32 -0400 Subject: [PATCH 01/14] =?UTF-8?q?test(host):=20red=20repro=20=E2=80=94=20L?= =?UTF-8?q?DAP=20authenticator=20has=20no=20wall-clock=20bound=20on=20sess?= =?UTF-8?q?ion=20activation=20(03/S2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...-ldap-historyread-async-plan.md.tasks.json | 2 +- .../LdapAuthResilienceTests.cs | 104 ++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json index b48a529d..d780299b 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json @@ -1,7 +1,7 @@ { "planPath": "archreview/plans/R2-08-ldap-historyread-async-plan.md", "tasks": [ - { "id": "R2-08-T1", "subject": "S2 stall-repro test (RED): slow LDAP backend must deny within AuthTimeout — fails on current code", "status": "pending", "blockedBy": [] }, + { "id": "R2-08-T1", "subject": "S2 stall-repro test (RED): slow LDAP backend must deny within AuthTimeout — fails on current code", "status": "completed", "blockedBy": [] }, { "id": "R2-08-T2", "subject": "LdapOptions: ConnectionTimeoutMs/AuthTimeoutMs/MaxConcurrentAuthentications/OutageFailureThreshold/OutageCooldownSeconds + ToLibraryOptions projection + validator rules", "status": "pending", "blockedBy": [] }, { "id": "R2-08-T3", "subject": "LdapAuthResult.IsSystemFailure seam + OtOpcUaLdapAuthService.Adapt mapping (ServiceAccountBindFailed => system-side)", "status": "pending", "blockedBy": [] }, { "id": "R2-08-T4", "subject": "LdapOpcUaUserAuthenticator: WaitAsync wall-clock bound + SemaphoreSlim concurrency cap with release-on-core-completion (turns T1 green)", "status": "pending", "blockedBy": ["R2-08-T1", "R2-08-T2"] }, diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs new file mode 100644 index 00000000..d43fafd4 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs @@ -0,0 +1,104 @@ +using System.Diagnostics; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; +using Xunit; +using ZB.MOM.WW.Auth.Abstractions.Roles; +using ZB.MOM.WW.OtOpcUa.Host.OpcUa; +using ZB.MOM.WW.OtOpcUa.Security.Ldap; + +namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests; + +/// +/// Resilience guard for (arch-review 03/S2). The OPC UA SDK +/// raises the impersonation callback synchronously under a SessionManager-wide event lock, so a +/// slow / hung LDAP bind inside it does not merely stall one SDK thread — it serializes EVERY session +/// activation server-wide. These tests prove the authenticator enforces a hard wall-clock bound, +/// bounded in-flight concurrency, and a directory-outage circuit at its own boundary (the SDK callback +/// being irreducibly synchronous), all fail-closed. No real LDAP server is used — a delaying fake +/// reproduces the outage shape offline. +/// +public sealed class LdapAuthResilienceTests +{ + /// + /// RED repro (03/S2): a backend that takes 5 s must NOT hold the caller for 5 s — the authenticator's + /// wall-clock bound denies (fail-closed) well before then. On current code (no bound) this fails: the + /// call returns Allow after the full 5 s. + /// + [Fact] + public async Task Authenticate_slow_backend_denies_within_AuthTimeout() + { + var ldap = new DelayingFakeLdap( + TimeSpan.FromSeconds(5), + new LdapAuthResult(true, "Alice", "alice", new[] { "configeditor" }, Array.Empty(), null)); + var mapper = new FakeMapper(g => g.ToArray()); + var sut = new LdapOpcUaUserAuthenticator( + ldap, + ScopeFactoryWith(mapper), + NullLogger.Instance); + + var sw = Stopwatch.StartNew(); + var result = await sut + .AuthenticateUserNameAsync("alice", "secret", CancellationToken.None) + .WaitAsync(TimeSpan.FromSeconds(10), CancellationToken.None); + sw.Stop(); + + result.Success.ShouldBeFalse(); + result.Error.ShouldNotBeNull(); + result.Error!.ShouldContain("timed out"); + sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(1.5)); + } + + /// Builds an IServiceScopeFactory whose scopes resolve the supplied mapper. + private static IServiceScopeFactory ScopeFactoryWith(IGroupRoleMapper mapper) + { + var services = new ServiceCollection(); + services.AddScoped(_ => mapper); + return services.BuildServiceProvider().GetRequiredService(); + } + + /// + /// A fake that awaits a configurable delay (observing the token) before + /// returning a fixed result — the offline stand-in for a slow / hung directory. Counts invocations so + /// tests can assert the outage circuit denies WITHOUT reaching the backend. + /// + private sealed class DelayingFakeLdap : ILdapAuthService + { + private readonly TimeSpan _delay; + private readonly LdapAuthResult _then; + private int _calls; + + /// Initializes the fake with a per-call delay and the result to return afterwards. + /// How long each authenticate call blocks before returning. + /// The result returned once the delay elapses. + public DelayingFakeLdap(TimeSpan delay, LdapAuthResult then) + { + _delay = delay; + _then = then; + } + + /// The number of times the backend was actually invoked. + public int Calls => Volatile.Read(ref _calls); + + /// Awaits the configured delay, then returns the canned result. + /// The username (ignored). + /// The password (ignored). + /// Cancellation token, observed by the delay. + public async Task AuthenticateAsync(string username, string password, CancellationToken ct = default) + { + Interlocked.Increment(ref _calls); + await Task.Delay(_delay, ct).ConfigureAwait(false); + return _then; + } + } + + /// Test group→role mapper driven by a delegate over the supplied groups. + private sealed class FakeMapper(Func, IReadOnlyList> map) : IGroupRoleMapper + { + /// Maps groups to roles via the configured delegate; Scope is always null. + /// The LDAP groups to map. + /// The cancellation token. + public Task> MapAsync(IReadOnlyList groups, CancellationToken ct) + => Task.FromResult(new GroupRoleMapping(map(groups), Scope: null)); + } +} From 09faa05f068dfd8d1b56d9ce296d35ee9c996802 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 11:38:37 -0400 Subject: [PATCH 02/14] feat(security): LDAP auth timeout/concurrency/outage-circuit options; project ConnectionTimeoutMs into the shared library (03/S2) --- ...-ldap-historyread-async-plan.md.tasks.json | 2 +- .../Configuration/LdapOptionsValidator.cs | 13 +++ .../Ldap/LdapOptions.cs | 44 +++++++++ .../LdapOptionsBindingTests.cs | 50 +++++++++++ .../LdapOptionsValidatorTests.cs | 89 +++++++++++++++++++ 5 files changed, 197 insertions(+), 1 deletion(-) diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json index d780299b..d5df7283 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json @@ -2,7 +2,7 @@ "planPath": "archreview/plans/R2-08-ldap-historyread-async-plan.md", "tasks": [ { "id": "R2-08-T1", "subject": "S2 stall-repro test (RED): slow LDAP backend must deny within AuthTimeout — fails on current code", "status": "completed", "blockedBy": [] }, - { "id": "R2-08-T2", "subject": "LdapOptions: ConnectionTimeoutMs/AuthTimeoutMs/MaxConcurrentAuthentications/OutageFailureThreshold/OutageCooldownSeconds + ToLibraryOptions projection + validator rules", "status": "pending", "blockedBy": [] }, + { "id": "R2-08-T2", "subject": "LdapOptions: ConnectionTimeoutMs/AuthTimeoutMs/MaxConcurrentAuthentications/OutageFailureThreshold/OutageCooldownSeconds + ToLibraryOptions projection + validator rules", "status": "completed", "blockedBy": [] }, { "id": "R2-08-T3", "subject": "LdapAuthResult.IsSystemFailure seam + OtOpcUaLdapAuthService.Adapt mapping (ServiceAccountBindFailed => system-side)", "status": "pending", "blockedBy": [] }, { "id": "R2-08-T4", "subject": "LdapOpcUaUserAuthenticator: WaitAsync wall-clock bound + SemaphoreSlim concurrency cap with release-on-core-completion (turns T1 green)", "status": "pending", "blockedBy": ["R2-08-T1", "R2-08-T2"] }, { "id": "R2-08-T5", "subject": "Directory-outage circuit: fail-fast deny after N consecutive system-side failures, half-open after cooldown", "status": "pending", "blockedBy": ["R2-08-T3", "R2-08-T4"] }, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LdapOptionsValidator.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LdapOptionsValidator.cs index 9b483a75..18309fe2 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LdapOptionsValidator.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LdapOptionsValidator.cs @@ -47,5 +47,18 @@ public sealed class LdapOptionsValidator : OptionsValidatorBase builder.RequireThat( !(options.Transport == LdapTransport.None && !options.AllowInsecure), "LDAP transport is None (plaintext) but AllowInsecure is false — set Transport to Ldaps/StartTls or set AllowInsecure for dev."); + + // S2 resilience knobs — a non-positive timeout / concurrency bound would silently disable the + // wall-clock/concurrency protection the OPC UA data-plane authenticator relies on, so fail fast. + builder.RequireThat(options.ConnectionTimeoutMs > 0, + "Ldap:ConnectionTimeoutMs must be > 0."); + builder.RequireThat(options.AuthTimeoutMs > 0, + "Ldap:AuthTimeoutMs must be > 0."); + builder.RequireThat(options.MaxConcurrentAuthentications > 0, + "Ldap:MaxConcurrentAuthentications must be > 0."); + // An open circuit with no cooldown would re-probe LDAP on every call — pointless. Only enforce + // a positive cooldown when the circuit is actually enabled (OutageFailureThreshold > 0; 0 disables). + builder.RequireThat(options.OutageFailureThreshold <= 0 || options.OutageCooldownSeconds > 0, + "Ldap:OutageCooldownSeconds must be > 0 when Ldap:OutageFailureThreshold > 0 (0 disables the circuit)."); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/LdapOptions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/LdapOptions.cs index 5b5cdde1..ffb5db57 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/LdapOptions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/LdapOptions.cs @@ -83,6 +83,49 @@ public sealed class LdapOptions /// public Dictionary GroupToRole { get; set; } = new(StringComparer.OrdinalIgnoreCase); + /// + /// Bounds (in milliseconds) the shared-library directory client's socket connect AND each + /// per-operation search inside a single bind flow. Projected into the library's + /// via . The + /// default (10000) matches the library default, so behaviour is unchanged unless configured. + /// This is the FIRST line of defence against a directory outage; the whole-flow + /// is the backstop (03/S2). + /// + public int ConnectionTimeoutMs { get; set; } = 10_000; + + /// + /// Hard whole-flow wall-clock bound (in milliseconds) the OPC UA data-plane authenticator + /// (LdapOpcUaUserAuthenticator) applies across the ENTIRE authenticate flow — bind plus + /// role-map — so the OPC UA SDK's synchronous impersonation callback (raised under a + /// SessionManager-wide event lock) can never be wedged by a slow / hung directory. + /// Deliberately > (default 15000) so a healthy-but-slow + /// full flow is not cut off while an outage is bounded by the library timeout first (03/S2). + /// + public int AuthTimeoutMs { get; set; } = 15_000; + + /// + /// Maximum number of in-flight OPC UA authentication flows the data-plane authenticator admits + /// at once. A caller that cannot acquire a slot within its budget is denied "backend busy" + /// (local backpressure, not counted against the outage circuit). Bounds accumulation of + /// orphaned / timed-out bind tasks during an outage (03/S2). Default 8. + /// + public int MaxConcurrentAuthentications { get; set; } = 8; + + /// + /// Number of CONSECUTIVE system-side failures (boundary timeout, directory-unreachable result, + /// or unexpected throw) after which the data-plane authenticator's outage circuit opens and + /// denies instantly for without touching LDAP. Any success + /// or user-side (credential) deny resets the counter. 0 disables the circuit (03/S2). + /// Default 3. + /// + public int OutageFailureThreshold { get; set; } = 3; + + /// + /// How long (in seconds) the outage circuit stays open before a half-open probe is allowed + /// through. Only meaningful when > 0. Default 15 (03/S2). + /// + public int OutageCooldownSeconds { get; set; } = 15; + /// /// Projects the wire fields onto the shared ZB.MOM.WW.Auth.Ldap /// the directory client consumes. App-only concerns @@ -104,5 +147,6 @@ public sealed class LdapOptions UserNameAttribute = UserNameAttribute, DisplayNameAttribute = DisplayNameAttribute, GroupAttribute = GroupAttribute, + ConnectionTimeoutMs = ConnectionTimeoutMs, }; } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsBindingTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsBindingTests.cs index c18b8d1e..d5f4d88c 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsBindingTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsBindingTests.cs @@ -61,6 +61,56 @@ public sealed class LdapOptionsBindingTests options.DevStubMode.ShouldBeFalse(); } + + /// + /// The five S2 resilience keys bind from the real Security:Ldap section (03/S2). + /// + [Fact] + public void Binding_reads_resilience_keys() + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["Security:Ldap:ConnectionTimeoutMs"] = "7000", + ["Security:Ldap:AuthTimeoutMs"] = "12000", + ["Security:Ldap:MaxConcurrentAuthentications"] = "5", + ["Security:Ldap:OutageFailureThreshold"] = "4", + ["Security:Ldap:OutageCooldownSeconds"] = "20", + }) + .Build(); + + var options = configuration.GetSection(LdapOptions.SectionName).Get(); + + options.ShouldNotBeNull(); + options.ConnectionTimeoutMs.ShouldBe(7000); + options.AuthTimeoutMs.ShouldBe(12000); + options.MaxConcurrentAuthentications.ShouldBe(5); + options.OutageFailureThreshold.ShouldBe(4); + options.OutageCooldownSeconds.ShouldBe(20); + } + + /// The resilience keys carry the documented defaults when absent from config. + [Fact] + public void Resilience_key_defaults_hold_when_absent() + { + var options = new LdapOptions(); + + options.ConnectionTimeoutMs.ShouldBe(10000); + options.AuthTimeoutMs.ShouldBe(15000); + options.MaxConcurrentAuthentications.ShouldBe(8); + options.OutageFailureThreshold.ShouldBe(3); + options.OutageCooldownSeconds.ShouldBe(15); + } + + /// ConnectionTimeoutMs is projected onto the shared-library options so the + /// socket-connect + per-search timeout is bounded inside the directory client. + [Fact] + public void ToLibraryOptions_projects_ConnectionTimeoutMs() + { + var options = new LdapOptions { ConnectionTimeoutMs = 4321 }; + + options.ToLibraryOptions().ConnectionTimeoutMs.ShouldBe(4321); + } } /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsValidatorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsValidatorTests.cs index 058f6ff1..9c5ff2b9 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsValidatorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOptionsValidatorTests.cs @@ -203,6 +203,95 @@ public sealed class LdapOptionsValidatorTests result.Failures.ShouldContain("Ldap:SearchBase is required when LDAP login is enabled."); } + /// Enabled with a non-positive ConnectionTimeoutMs fails validation (03/S2). + [Fact] + public void Enabled_with_nonpositive_ConnectionTimeoutMs_fails() + { + var options = new LdapOptions + { + Enabled = true, + Server = "ldap", + SearchBase = "dc=x", + Port = 389, + Transport = LdapTransport.Ldaps, + ConnectionTimeoutMs = 0, + }; + + Sut.Validate(null, options).Failed.ShouldBeTrue(); + } + + /// Enabled with a non-positive AuthTimeoutMs fails validation (03/S2). + [Fact] + public void Enabled_with_nonpositive_AuthTimeoutMs_fails() + { + var options = new LdapOptions + { + Enabled = true, + Server = "ldap", + SearchBase = "dc=x", + Port = 389, + Transport = LdapTransport.Ldaps, + AuthTimeoutMs = 0, + }; + + Sut.Validate(null, options).Failed.ShouldBeTrue(); + } + + /// Enabled with a non-positive MaxConcurrentAuthentications fails validation (03/S2). + [Fact] + public void Enabled_with_nonpositive_MaxConcurrentAuthentications_fails() + { + var options = new LdapOptions + { + Enabled = true, + Server = "ldap", + SearchBase = "dc=x", + Port = 389, + Transport = LdapTransport.Ldaps, + MaxConcurrentAuthentications = 0, + }; + + Sut.Validate(null, options).Failed.ShouldBeTrue(); + } + + /// A positive OutageFailureThreshold with a zero OutageCooldownSeconds fails — + /// an open circuit with no cooldown would probe on every call (03/S2). + [Fact] + public void OutageFailureThreshold_positive_with_zero_cooldown_fails() + { + var options = new LdapOptions + { + Enabled = true, + Server = "ldap", + SearchBase = "dc=x", + Port = 389, + Transport = LdapTransport.Ldaps, + OutageFailureThreshold = 3, + OutageCooldownSeconds = 0, + }; + + Sut.Validate(null, options).Failed.ShouldBeTrue(); + } + + /// A zero OutageFailureThreshold (circuit disabled) passes regardless of the + /// cooldown value (03/S2). + [Fact] + public void OutageFailureThreshold_zero_disables_circuit_and_passes() + { + var options = new LdapOptions + { + Enabled = true, + Server = "ldap", + SearchBase = "dc=x", + Port = 389, + Transport = LdapTransport.Ldaps, + OutageFailureThreshold = 0, + OutageCooldownSeconds = 0, + }; + + Sut.Validate(null, options).Succeeded.ShouldBeTrue(); + } + /// Enabled with port 0 reports the port-range failure using the shared primitive wording. [Fact] public void Enabled_with_zero_port_fails() From 0d5cb89d48cb01daa57b50f2f17fa4a9a00c1160 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 11:39:35 -0400 Subject: [PATCH 03/14] feat(security): LdapAuthResult.IsSystemFailure distinguishes directory outage from credential deny (03/S2) --- ...-ldap-historyread-async-plan.md.tasks.json | 2 +- .../Ldap/LdapAuthResult.cs | 13 ++++- .../Ldap/OtOpcUaLdapAuthService.cs | 8 ++- .../OtOpcUaLdapAuthServiceTests.cs | 50 +++++++++++++++++++ 4 files changed, 70 insertions(+), 3 deletions(-) diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json index d5df7283..08933cfd 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json @@ -3,7 +3,7 @@ "tasks": [ { "id": "R2-08-T1", "subject": "S2 stall-repro test (RED): slow LDAP backend must deny within AuthTimeout — fails on current code", "status": "completed", "blockedBy": [] }, { "id": "R2-08-T2", "subject": "LdapOptions: ConnectionTimeoutMs/AuthTimeoutMs/MaxConcurrentAuthentications/OutageFailureThreshold/OutageCooldownSeconds + ToLibraryOptions projection + validator rules", "status": "completed", "blockedBy": [] }, - { "id": "R2-08-T3", "subject": "LdapAuthResult.IsSystemFailure seam + OtOpcUaLdapAuthService.Adapt mapping (ServiceAccountBindFailed => system-side)", "status": "pending", "blockedBy": [] }, + { "id": "R2-08-T3", "subject": "LdapAuthResult.IsSystemFailure seam + OtOpcUaLdapAuthService.Adapt mapping (ServiceAccountBindFailed => system-side)", "status": "completed", "blockedBy": [] }, { "id": "R2-08-T4", "subject": "LdapOpcUaUserAuthenticator: WaitAsync wall-clock bound + SemaphoreSlim concurrency cap with release-on-core-completion (turns T1 green)", "status": "pending", "blockedBy": ["R2-08-T1", "R2-08-T2"] }, { "id": "R2-08-T5", "subject": "Directory-outage circuit: fail-fast deny after N consecutive system-side failures, half-open after cooldown", "status": "pending", "blockedBy": ["R2-08-T3", "R2-08-T4"] }, { "id": "R2-08-T6", "subject": "Outage-sim integration test (real library, unroutable host, via internal HandleImpersonation) + SDK m_eventLock contract doc + docs/security.md", "status": "pending", "blockedBy": ["R2-08-T4"] }, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/LdapAuthResult.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/LdapAuthResult.cs index 4ca801e6..a91cae04 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/LdapAuthResult.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/LdapAuthResult.cs @@ -7,4 +7,15 @@ public sealed record LdapAuthResult( string? Username, IReadOnlyList Groups, IReadOnlyList Roles, - string? Error); + string? Error) +{ + /// + /// true when this failure is a SYSTEM-side directory fault (the directory is unreachable or + /// the service account is misconfigured) rather than a user-credential deny. Feeds the OPC UA + /// data-plane authenticator's outage circuit (03/S2): a run of consecutive system-side failures + /// opens the circuit and fails fast, whereas user-side denies never trip it. Always false + /// on success and on user-side denies; set only on the delegated real path for the library's + /// directory-unreachable bucket. + /// + public bool IsSystemFailure { get; init; } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/OtOpcUaLdapAuthService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/OtOpcUaLdapAuthService.cs index 373ee4f4..6ddf8bb5 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/OtOpcUaLdapAuthService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/OtOpcUaLdapAuthService.cs @@ -113,7 +113,13 @@ public sealed class OtOpcUaLdapAuthService : ILdapAuthService if (result.Succeeded) return new(true, result.DisplayName, result.Username, result.Groups, [], null); - return new(false, null, username, [], [], FailureToError(result.Failure)); + // ServiceAccountBindFailed is the library's directory-unreachable / service-account-misconfigured + // bucket (its enum has no dedicated DirectoryUnavailable) — flag it as system-side so the OPC UA + // authenticator's outage circuit can distinguish an outage from a user-credential deny (03/S2). + return new(false, null, username, [], [], FailureToError(result.Failure)) + { + IsSystemFailure = result.Failure is LdapAuthFailure.ServiceAccountBindFailed, + }; } /// Folds a structured library failure code into the app's user-facing error text. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/OtOpcUaLdapAuthServiceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/OtOpcUaLdapAuthServiceTests.cs index d0b9354b..3146ba8b 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/OtOpcUaLdapAuthServiceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests/OtOpcUaLdapAuthServiceTests.cs @@ -121,6 +121,56 @@ public sealed class OtOpcUaLdapAuthServiceTests inner.Called.ShouldBeFalse(); } + /// A system-side library failure (ServiceAccountBindFailed — the directory-unreachable + /// bucket) adapts to IsSystemFailure == true so the OPC UA authenticator's outage circuit can + /// distinguish it from a user-credential deny (03/S2). + [Fact] + public async Task ServiceAccountBindFailed_adapts_to_IsSystemFailure_true() + { + var inner = new RecordingLibService(LibLdapAuthResult.Fail(LdapAuthFailure.ServiceAccountBindFailed)); + var sut = Build( + new LdapOptions { Enabled = true, DevStubMode = false, Transport = LdapTransport.Ldaps }, + inner); + + var result = await sut.AuthenticateAsync("alice", "secret", CancellationToken.None); + + result.Success.ShouldBeFalse(); + result.IsSystemFailure.ShouldBeTrue(); + } + + /// User-side library failures and success adapt to IsSystemFailure == false — they must + /// NOT trip the outage circuit (03/S2). + [Theory] + [InlineData(LdapAuthFailure.BadCredentials)] + [InlineData(LdapAuthFailure.UserNotFound)] + public async Task User_side_failures_adapt_to_IsSystemFailure_false(LdapAuthFailure failure) + { + var inner = new RecordingLibService(LibLdapAuthResult.Fail(failure)); + var sut = Build( + new LdapOptions { Enabled = true, DevStubMode = false, Transport = LdapTransport.Ldaps }, + inner); + + var result = await sut.AuthenticateAsync("alice", "wrong", CancellationToken.None); + + result.Success.ShouldBeFalse(); + result.IsSystemFailure.ShouldBeFalse(); + } + + /// A successful bind is never a system failure. + [Fact] + public async Task Success_is_not_a_system_failure() + { + var inner = new RecordingLibService(LibLdapAuthResult.Success("alice", "Alice", new[] { "ReadOnly" })); + var sut = Build( + new LdapOptions { Enabled = true, DevStubMode = false, Transport = LdapTransport.Ldaps }, + inner); + + var result = await sut.AuthenticateAsync("alice", "secret", CancellationToken.None); + + result.Success.ShouldBeTrue(); + result.IsSystemFailure.ShouldBeFalse(); + } + /// Records whether the library service was invoked and returns a canned result. private sealed class RecordingLibService(LibLdapAuthResult result) : LibILdapAuthService { From 21abe42a90585fc2ec0f070d9e7fc3fc8dcb1983 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 11:52:10 -0400 Subject: [PATCH 04/14] feat(host): hard wall-clock bound + bounded concurrency on OPC UA LDAP authentication (03/S2) --- ...-ldap-historyread-async-plan.md.tasks.json | 2 +- .../OpcUa/LdapOpcUaUserAuthenticator.cs | 105 ++++++++++++++++-- .../LdapAuthResilienceTests.cs | 103 ++++++++++++++++- .../LdapOpcUaUserAuthenticatorTests.cs | 13 ++- 4 files changed, 203 insertions(+), 20 deletions(-) diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json index 08933cfd..c76c8bfc 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json @@ -4,7 +4,7 @@ { "id": "R2-08-T1", "subject": "S2 stall-repro test (RED): slow LDAP backend must deny within AuthTimeout — fails on current code", "status": "completed", "blockedBy": [] }, { "id": "R2-08-T2", "subject": "LdapOptions: ConnectionTimeoutMs/AuthTimeoutMs/MaxConcurrentAuthentications/OutageFailureThreshold/OutageCooldownSeconds + ToLibraryOptions projection + validator rules", "status": "completed", "blockedBy": [] }, { "id": "R2-08-T3", "subject": "LdapAuthResult.IsSystemFailure seam + OtOpcUaLdapAuthService.Adapt mapping (ServiceAccountBindFailed => system-side)", "status": "completed", "blockedBy": [] }, - { "id": "R2-08-T4", "subject": "LdapOpcUaUserAuthenticator: WaitAsync wall-clock bound + SemaphoreSlim concurrency cap with release-on-core-completion (turns T1 green)", "status": "pending", "blockedBy": ["R2-08-T1", "R2-08-T2"] }, + { "id": "R2-08-T4", "subject": "LdapOpcUaUserAuthenticator: WaitAsync wall-clock bound + SemaphoreSlim concurrency cap with release-on-core-completion (turns T1 green)", "status": "completed", "blockedBy": ["R2-08-T1", "R2-08-T2"] }, { "id": "R2-08-T5", "subject": "Directory-outage circuit: fail-fast deny after N consecutive system-side failures, half-open after cooldown", "status": "pending", "blockedBy": ["R2-08-T3", "R2-08-T4"] }, { "id": "R2-08-T6", "subject": "Outage-sim integration test (real library, unroutable host, via internal HandleImpersonation) + SDK m_eventLock contract doc + docs/security.md", "status": "pending", "blockedBy": ["R2-08-T4"] }, { "id": "R2-08-T7", "subject": "S3 sequential-serving repro test (RED): MaxObservedConcurrency >= 2 on an 8-node Raw batch — fails on current code", "status": "pending", "blockedBy": [] }, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/LdapOpcUaUserAuthenticator.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/LdapOpcUaUserAuthenticator.cs index 4a03e34a..2d26b832 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/LdapOpcUaUserAuthenticator.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/LdapOpcUaUserAuthenticator.cs @@ -1,5 +1,6 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using ZB.MOM.WW.Auth.Abstractions.Roles; using ZB.MOM.WW.OtOpcUa.OpcUaServer.Security; using ZB.MOM.WW.OtOpcUa.Security.Ldap; @@ -16,22 +17,108 @@ namespace ZB.MOM.WW.OtOpcUa.Host.OpcUa; /// session identity for the downstream data-plane ACL evaluator. /// /// +/// /// This authenticator is registered as a singleton, but /// (and its DbContext-backed mapping service) is scoped. A per-call DI scope is opened to /// resolve the mapper so the singleton never captures a scoped dependency. +/// +/// +/// Resilience (arch-review 03/S2). The OPC UA SDK raises the impersonation callback that +/// reaches here synchronously, inside a SessionManager-wide event lock (verified +/// against OPCFoundation.NetStandard.Opc.Ua.Server 1.5.378 — +/// SessionManager.ActivateSessionAsync holds m_eventLock while invoking the +/// void impersonation delegate). A slow / hung LDAP bind inside that callback therefore does +/// not merely stall one SDK thread — it serializes every session activation server-wide +/// (Anonymous and X509 included). The callback contract is irreducibly synchronous (there is no +/// async impersonation callback), so the fix lives entirely at this boundary: a hard whole-flow +/// wall-clock bound () and a bounded in-flight concurrency cap +/// (), both fail-closed. All new paths deny. +/// /// -public sealed class LdapOpcUaUserAuthenticator( - ILdapAuthService ldap, - IServiceScopeFactory scopeFactory, - ILogger logger) - : IOpcUaUserAuthenticator +public sealed class LdapOpcUaUserAuthenticator : IOpcUaUserAuthenticator { + private readonly ILdapAuthService _ldap; + private readonly IServiceScopeFactory _scopeFactory; + private readonly ILogger _logger; + private readonly LdapOptions _options; + + /// Bounds in-flight authentication flows. A boundary timeout abandons its core task, which + /// keeps holding its slot until it actually completes — so an outage cannot accumulate unbounded + /// orphan binds. + private readonly SemaphoreSlim _gate; + + /// Constructs the authenticator from its dependencies and the bound LDAP options. + /// The app LDAP auth service (shared with the Admin UI login flow). + /// Opens a per-call DI scope to resolve the scoped role mapper. + /// The bound (timeout + concurrency knobs). + /// The logger. + public LdapOpcUaUserAuthenticator( + ILdapAuthService ldap, + IServiceScopeFactory scopeFactory, + IOptions options, + ILogger logger) + { + _ldap = ldap; + _scopeFactory = scopeFactory; + _options = options.Value; + _logger = logger; + _gate = new SemaphoreSlim(Math.Max(1, _options.MaxConcurrentAuthentications)); + } + /// public async Task AuthenticateUserNameAsync(string username, string password, CancellationToken ct) { + var authTimeout = TimeSpan.FromMilliseconds(Math.Max(1, _options.AuthTimeoutMs)); + + // Bounded in-flight concurrency: acquire a slot within our own wall-clock budget. All slots + // busy is LOCAL backpressure, not directory evidence — deny "busy" (fail-closed). + if (!await _gate.WaitAsync(authTimeout, ct).ConfigureAwait(false)) + { + _logger.LogWarning( + "LDAP authentication backend busy (concurrency cap {Cap}) — denying OPC UA user {User}", + _options.MaxConcurrentAuthentications, username); + return OpcUaUserAuthResult.Deny("Authentication backend busy"); + } + + // Release the slot when the CORE task completes — deliberately NOT in a finally here. A boundary + // timeout below abandons the core task (it runs to completion in the background; the underlying + // library is documented never-throw, so there are no unobserved exceptions), and it must keep + // counting against the cap until it finishes. The point of the boundary is to release the SDK + // thread (and m_eventLock), not to cancel the bind. + var core = AuthenticateCoreAsync(username, password, ct); + _ = core.ContinueWith( + static (_, state) => ((SemaphoreSlim)state!).Release(), + _gate, + CancellationToken.None, + TaskContinuationOptions.None, + TaskScheduler.Default); + try { - var result = await ldap.AuthenticateAsync(username, password, ct).ConfigureAwait(false); + return await core.WaitAsync(authTimeout, ct).ConfigureAwait(false); + } + catch (TimeoutException) + { + _logger.LogWarning( + "LDAP authentication timed out after {Timeout} for OPC UA user {User} — denying (fail-closed)", + authTimeout, username); + return OpcUaUserAuthResult.Deny("Authentication timed out"); + } + } + + /// + /// The unbounded core authenticate flow (bind + role-map) that the outer + /// wraps in a wall-clock bound + concurrency cap. Fail-closed: + /// any backend throw becomes a denial rather than escaping into the SDK. + /// + /// The username to authenticate. + /// The cleartext password. + /// Cancellation token. + private async Task AuthenticateCoreAsync(string username, string password, CancellationToken ct) + { + try + { + var result = await _ldap.AuthenticateAsync(username, password, ct).ConfigureAwait(false); if (!result.Success) { return OpcUaUserAuthResult.Deny(result.Error ?? "Invalid credentials"); @@ -42,7 +129,7 @@ public sealed class LdapOpcUaUserAuthenticator( } catch (Exception ex) when (ex is not OperationCanceledException) { - logger.LogWarning(ex, "LDAP authentication threw for OPC UA user {User}", username); + _logger.LogWarning(ex, "LDAP authentication threw for OPC UA user {User}", username); return OpcUaUserAuthResult.Deny("Authentication backend error"); } } @@ -62,7 +149,7 @@ public sealed class LdapOpcUaUserAuthenticator( { try { - await using var scope = scopeFactory.CreateAsyncScope(); + await using var scope = _scopeFactory.CreateAsyncScope(); var mapper = scope.ServiceProvider.GetRequiredService>(); var mapping = await mapper.MapAsync(groups, ct).ConfigureAwait(false); @@ -73,7 +160,7 @@ public sealed class LdapOpcUaUserAuthenticator( } catch (Exception ex) when (ex is not OperationCanceledException) { - logger.LogWarning(ex, + _logger.LogWarning(ex, "Role-map lookup failed for OPC UA user {User}; using pre-resolved baseline roles", username); return preResolved; } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs index d43fafd4..1447894b 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs @@ -1,6 +1,7 @@ using System.Diagnostics; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; using Shouldly; using Xunit; using ZB.MOM.WW.Auth.Abstractions.Roles; @@ -32,10 +33,7 @@ public sealed class LdapAuthResilienceTests TimeSpan.FromSeconds(5), new LdapAuthResult(true, "Alice", "alice", new[] { "configeditor" }, Array.Empty(), null)); var mapper = new FakeMapper(g => g.ToArray()); - var sut = new LdapOpcUaUserAuthenticator( - ldap, - ScopeFactoryWith(mapper), - NullLogger.Instance); + var sut = Build(ldap, mapper, new LdapOptions { AuthTimeoutMs = 200 }); var sw = Stopwatch.StartNew(); var result = await sut @@ -49,6 +47,69 @@ public sealed class LdapAuthResilienceTests sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(1.5)); } + /// A fast, healthy backend is unaffected by the resilience wrapping — success + roles pass + /// through untouched (03/S2). + [Fact] + public async Task Authenticate_fast_success_unaffected() + { + var ldap = new DelayingFakeLdap( + TimeSpan.Zero, + new LdapAuthResult(true, "Alice", "alice", new[] { "configeditor" }, Array.Empty(), null)); + var mapper = new FakeMapper(g => g.Select(x => x == "configeditor" ? "Designer" : x).ToArray()); + var sut = Build(ldap, mapper, new LdapOptions()); + + var result = await sut + .AuthenticateUserNameAsync("alice", "secret", CancellationToken.None) + .WaitAsync(TimeSpan.FromSeconds(10), CancellationToken.None); + + result.Success.ShouldBeTrue(); + result.DisplayName.ShouldBe("Alice"); + result.Roles.ShouldBe(new[] { "Designer" }); + } + + /// With the concurrency cap saturated (a parked in-flight bind holding the only slot), a + /// second call is denied "backend busy" within its own budget — it never waits behind the parked + /// bind indefinitely (03/S2). + [Fact] + public async Task Authenticate_saturated_concurrency_denies_busy() + { + var ldap = new GatedFakeLdap( + new LdapAuthResult(true, "Alice", "alice", new[] { "configeditor" }, Array.Empty(), null)); + var mapper = new FakeMapper(g => g.ToArray()); + // MaxConcurrentAuthentications = 1 → the second call cannot get a slot; small AuthTimeoutMs so the + // "busy" denial is quick and the first call's boundary also trips fast (its core stays parked, + // holding the slot). + var sut = Build(ldap, mapper, new LdapOptions { MaxConcurrentAuthentications = 1, AuthTimeoutMs = 300 }); + + // First call: acquires the only slot; its core parks in the gated fake. + var first = sut.AuthenticateUserNameAsync("alice", "secret", CancellationToken.None); + + // Wait until the fake has actually been entered (slot acquired, core parked). + var spin = Stopwatch.StartNew(); + while (ldap.Entered == 0 && spin.Elapsed < TimeSpan.FromSeconds(5)) + await Task.Delay(10, TestContext.Current.CancellationToken); + ldap.Entered.ShouldBe(1); + + // Second concurrent call: the slot is held by the parked core, so acquisition times out → busy. + var second = await sut + .AuthenticateUserNameAsync("bob", "secret", CancellationToken.None) + .WaitAsync(TimeSpan.FromSeconds(10), CancellationToken.None); + + second.Success.ShouldBeFalse(); + second.Error.ShouldNotBeNull(); + second.Error!.ShouldContain("busy"); + ldap.Entered.ShouldBe(1, "the second call must be denied without ever reaching the backend"); + + // Release the parked bind so the first call's core completes cleanly (no orphaned resources). + ldap.Release(); + await first.WaitAsync(TimeSpan.FromSeconds(10), CancellationToken.None); + } + + private static LdapOpcUaUserAuthenticator Build( + ILdapAuthService ldap, IGroupRoleMapper mapper, LdapOptions options) => + new(ldap, ScopeFactoryWith(mapper), Options.Create(options), + NullLogger.Instance); + /// Builds an IServiceScopeFactory whose scopes resolve the supplied mapper. private static IServiceScopeFactory ScopeFactoryWith(IGroupRoleMapper mapper) { @@ -92,6 +153,40 @@ public sealed class LdapAuthResilienceTests } } + /// + /// A fake that parks ASYNCHRONOUSLY (awaits a + /// ) until the test calls — used to hold an + /// in-flight bind so the concurrency cap is deterministically saturated WITHOUT blocking a thread. + /// Records how many calls have entered (past the slot acquire). + /// + private sealed class GatedFakeLdap : ILdapAuthService + { + private readonly TaskCompletionSource _park = new(TaskCreationOptions.RunContinuationsAsynchronously); + private readonly LdapAuthResult _then; + private int _entered; + + /// Initializes the fake with the result to return once released. + /// The result returned once is called. + public GatedFakeLdap(LdapAuthResult then) => _then = then; + + /// Number of authenticate calls that reached the backend body. + public int Entered => Volatile.Read(ref _entered); + + /// Releases the parked bind so its awaiting call can complete. + public void Release() => _park.TrySetResult(); + + /// Marks entry, awaits the parking gate, then returns the canned result. + /// The username (ignored). + /// The password (ignored). + /// Cancellation token, observed while parked. + public async Task AuthenticateAsync(string username, string password, CancellationToken ct = default) + { + Interlocked.Increment(ref _entered); + await _park.Task.WaitAsync(ct).ConfigureAwait(false); + return _then; + } + } + /// Test group→role mapper driven by a delegate over the supplied groups. private sealed class FakeMapper(Func, IReadOnlyList> map) : IGroupRoleMapper { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOpcUaUserAuthenticatorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOpcUaUserAuthenticatorTests.cs index 8f859751..b076de93 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOpcUaUserAuthenticatorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapOpcUaUserAuthenticatorTests.cs @@ -1,5 +1,6 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; using Shouldly; using Xunit; using ZB.MOM.WW.Auth.Abstractions.Roles; @@ -26,7 +27,7 @@ public sealed class LdapOpcUaUserAuthenticatorTests // group "configeditor" -> "Designer" (canonical, Task 1.7). var ldap = new FakeLdap(new LdapAuthResult(true, "Alice", "alice", new[] { "configeditor" }, Array.Empty(), null)); var mapper = new FakeMapper(g => g.Select(x => x == "configeditor" ? "Designer" : x).ToArray()); - var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), NullLogger.Instance); + var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), Options.Create(new LdapOptions()), NullLogger.Instance); var result = await sut.AuthenticateUserNameAsync("alice", "secret", CancellationToken.None); @@ -44,7 +45,7 @@ public sealed class LdapOpcUaUserAuthenticatorTests // nothing, so the union is exactly {Administrator}. var ldap = new FakeLdap(new LdapAuthResult(true, "dev", "dev", new[] { "dev" }, new[] { "Administrator" }, null)); var mapper = new FakeMapper(_ => Array.Empty()); - var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), NullLogger.Instance); + var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), Options.Create(new LdapOptions()), NullLogger.Instance); var result = await sut.AuthenticateUserNameAsync("dev", "anything", CancellationToken.None); @@ -59,7 +60,7 @@ public sealed class LdapOpcUaUserAuthenticatorTests { var ldap = new FakeLdap(new LdapAuthResult(true, "dev", "dev", new[] { "dev" }, new[] { "Administrator" }, null)); var mapper = new FakeMapper(_ => throw new InvalidOperationException("DB down")); - var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), NullLogger.Instance); + var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), Options.Create(new LdapOptions()), NullLogger.Instance); var result = await sut.AuthenticateUserNameAsync("dev", "anything", CancellationToken.None); @@ -73,7 +74,7 @@ public sealed class LdapOpcUaUserAuthenticatorTests { var ldap = new FakeLdap(new LdapAuthResult(false, null, "mallory", Array.Empty(), Array.Empty(), "Invalid username or password")); var mapper = new FakeMapper(g => g.ToArray()); - var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), NullLogger.Instance); + var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), Options.Create(new LdapOptions()), NullLogger.Instance); var result = await sut.AuthenticateUserNameAsync("mallory", "wrong", CancellationToken.None); @@ -87,7 +88,7 @@ public sealed class LdapOpcUaUserAuthenticatorTests { var ldap = new FakeLdap(_ => throw new InvalidOperationException("LDAP unreachable")); var mapper = new FakeMapper(g => g.ToArray()); - var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), NullLogger.Instance); + var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), Options.Create(new LdapOptions()), NullLogger.Instance); var result = await sut.AuthenticateUserNameAsync("anyone", "x", CancellationToken.None); @@ -102,7 +103,7 @@ public sealed class LdapOpcUaUserAuthenticatorTests { var ldap = new FakeLdap(new LdapAuthResult(true, null, "alice", new[] { "ReadOnly" }, Array.Empty(), null)); var mapper = new FakeMapper(g => g.ToArray()); - var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), NullLogger.Instance); + var sut = new LdapOpcUaUserAuthenticator(ldap, ScopeFactoryWith(mapper), Options.Create(new LdapOptions()), NullLogger.Instance); var result = await sut.AuthenticateUserNameAsync("alice", "x", CancellationToken.None); From 0c785415a274c0d2e626bbb7da79fbc0d4053a21 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 11:56:45 -0400 Subject: [PATCH 05/14] =?UTF-8?q?feat(host):=20LDAP=20directory-outage=20c?= =?UTF-8?q?ircuit=20=E2=80=94=20fail-fast=20denial=20during=20sustained=20?= =?UTF-8?q?outage=20(03/S2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...-ldap-historyread-async-plan.md.tasks.json | 2 +- .../OpcUa/LdapOpcUaUserAuthenticator.cs | 106 ++++++++++++- .../LdapAuthResilienceTests.cs | 140 ++++++++++++++++++ 3 files changed, 240 insertions(+), 8 deletions(-) diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json index c76c8bfc..824e864e 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json @@ -5,7 +5,7 @@ { "id": "R2-08-T2", "subject": "LdapOptions: ConnectionTimeoutMs/AuthTimeoutMs/MaxConcurrentAuthentications/OutageFailureThreshold/OutageCooldownSeconds + ToLibraryOptions projection + validator rules", "status": "completed", "blockedBy": [] }, { "id": "R2-08-T3", "subject": "LdapAuthResult.IsSystemFailure seam + OtOpcUaLdapAuthService.Adapt mapping (ServiceAccountBindFailed => system-side)", "status": "completed", "blockedBy": [] }, { "id": "R2-08-T4", "subject": "LdapOpcUaUserAuthenticator: WaitAsync wall-clock bound + SemaphoreSlim concurrency cap with release-on-core-completion (turns T1 green)", "status": "completed", "blockedBy": ["R2-08-T1", "R2-08-T2"] }, - { "id": "R2-08-T5", "subject": "Directory-outage circuit: fail-fast deny after N consecutive system-side failures, half-open after cooldown", "status": "pending", "blockedBy": ["R2-08-T3", "R2-08-T4"] }, + { "id": "R2-08-T5", "subject": "Directory-outage circuit: fail-fast deny after N consecutive system-side failures, half-open after cooldown", "status": "completed", "blockedBy": ["R2-08-T3", "R2-08-T4"] }, { "id": "R2-08-T6", "subject": "Outage-sim integration test (real library, unroutable host, via internal HandleImpersonation) + SDK m_eventLock contract doc + docs/security.md", "status": "pending", "blockedBy": ["R2-08-T4"] }, { "id": "R2-08-T7", "subject": "S3 sequential-serving repro test (RED): MaxObservedConcurrency >= 2 on an 8-node Raw batch — fails on current code", "status": "pending", "blockedBy": [] }, { "id": "R2-08-T8", "subject": "ServerHistorianOptions: HistoryReadBatchParallelism/MaxConcurrentHistoryReadBatches/HistoryReadDeadline + Validate warnings + node-manager properties + hosted-service wiring", "status": "pending", "blockedBy": [] }, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/LdapOpcUaUserAuthenticator.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/LdapOpcUaUserAuthenticator.cs index 2d26b832..bd97cd93 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/LdapOpcUaUserAuthenticator.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/LdapOpcUaUserAuthenticator.cs @@ -47,6 +47,16 @@ public sealed class LdapOpcUaUserAuthenticator : IOpcUaUserAuthenticator /// orphan binds. private readonly SemaphoreSlim _gate; + /// Guards the outage-circuit state ( + + /// ). + private readonly object _circuitLock = new(); + + /// Count of CONSECUTIVE system-side failures; reset by any success / user-side deny. + private int _consecutiveSystemFailures; + + /// the circuit stays open until; 0 = closed. + private long _circuitOpenUntilTicks; + /// Constructs the authenticator from its dependencies and the bound LDAP options. /// The app LDAP auth service (shared with the Admin UI login flow). /// Opens a per-call DI scope to resolve the scoped role mapper. @@ -70,8 +80,19 @@ public sealed class LdapOpcUaUserAuthenticator : IOpcUaUserAuthenticator { var authTimeout = TimeSpan.FromMilliseconds(Math.Max(1, _options.AuthTimeoutMs)); + // Directory-outage circuit: after a run of consecutive system-side failures, fail fast WITHOUT + // touching LDAP for the cooldown window — so a sustained outage cannot serialize every session + // activation behind a per-call stall (the SDK raises this callback under a server-wide lock). + if (IsCircuitOpen()) + { + _logger.LogDebug( + "LDAP outage circuit open — fast-denying OPC UA user {User} without a directory call", username); + return OpcUaUserAuthResult.Deny("Authentication backend unavailable"); + } + // Bounded in-flight concurrency: acquire a slot within our own wall-clock budget. All slots - // busy is LOCAL backpressure, not directory evidence — deny "busy" (fail-closed). + // busy is LOCAL backpressure, not directory evidence — deny "busy" (fail-closed) and do NOT + // feed the outage circuit. if (!await _gate.WaitAsync(authTimeout, ct).ConfigureAwait(false)) { _logger.LogWarning( @@ -95,10 +116,14 @@ public sealed class LdapOpcUaUserAuthenticator : IOpcUaUserAuthenticator try { - return await core.WaitAsync(authTimeout, ct).ConfigureAwait(false); + var outcome = await core.WaitAsync(authTimeout, ct).ConfigureAwait(false); + RecordOutcome(outcome.SystemFailure, username); + return outcome.Result; } catch (TimeoutException) { + // A boundary timeout is directory evidence — it feeds the circuit. + RecordOutcome(systemFailure: true, username); _logger.LogWarning( "LDAP authentication timed out after {Timeout} for OPC UA user {User} — denying (fail-closed)", authTimeout, username); @@ -106,34 +131,101 @@ public sealed class LdapOpcUaUserAuthenticator : IOpcUaUserAuthenticator } } + /// Whether the outage circuit is currently open (fast-deny window active). Always false when + /// the circuit is disabled ( ≤ 0). + private bool IsCircuitOpen() + { + if (_options.OutageFailureThreshold <= 0) return false; + lock (_circuitLock) + { + return _circuitOpenUntilTicks > Environment.TickCount64; + } + } + + /// + /// Updates the outage-circuit state from one authentication outcome. A system-side failure advances + /// the consecutive-failure streak and opens the circuit once it reaches + /// ; any non-system outcome (success or user-side + /// deny) resets the streak and closes the circuit. Half-open behaviour is implicit: once the cooldown + /// elapses lets the next call through with the streak still at threshold, + /// so a single further system failure re-opens immediately. + /// + /// Whether this outcome was a system-side (directory) failure. + /// The login name, for diagnostics. + private void RecordOutcome(bool systemFailure, string username) + { + if (_options.OutageFailureThreshold <= 0) return; // circuit disabled + + lock (_circuitLock) + { + if (systemFailure) + { + _consecutiveSystemFailures++; + if (_consecutiveSystemFailures >= _options.OutageFailureThreshold) + { + var wasOpen = _circuitOpenUntilTicks > Environment.TickCount64; + _circuitOpenUntilTicks = + Environment.TickCount64 + (long)Math.Max(1, _options.OutageCooldownSeconds) * 1000L; + if (!wasOpen) + { + _logger.LogWarning( + "LDAP outage circuit OPENED after {Failures} consecutive system-side failures — " + + "fast-denying for {Cooldown}s (last user {User})", + _consecutiveSystemFailures, _options.OutageCooldownSeconds, username); + } + } + } + else + { + if (_consecutiveSystemFailures >= _options.OutageFailureThreshold) + { + _logger.LogInformation( + "LDAP outage circuit CLOSED — directory recovered (probe succeeded for {User})", username); + } + _consecutiveSystemFailures = 0; + _circuitOpenUntilTicks = 0; + } + } + } + /// /// The unbounded core authenticate flow (bind + role-map) that the outer /// wraps in a wall-clock bound + concurrency cap. Fail-closed: - /// any backend throw becomes a denial rather than escaping into the SDK. + /// any backend throw becomes a denial rather than escaping into the SDK. The returned + /// flags directory-side faults (a system-side + /// deny, or an unexpected throw) so the caller can feed + /// the outage circuit; a user-credential deny is NOT a system failure. /// /// The username to authenticate. /// The cleartext password. /// Cancellation token. - private async Task AuthenticateCoreAsync(string username, string password, CancellationToken ct) + private async Task AuthenticateCoreAsync(string username, string password, CancellationToken ct) { try { var result = await _ldap.AuthenticateAsync(username, password, ct).ConfigureAwait(false); if (!result.Success) { - return OpcUaUserAuthResult.Deny(result.Error ?? "Invalid credentials"); + return new CoreOutcome( + OpcUaUserAuthResult.Deny(result.Error ?? "Invalid credentials"), result.IsSystemFailure); } var roles = await ResolveRolesAsync(result.Groups, result.Roles, username, ct).ConfigureAwait(false); - return OpcUaUserAuthResult.Allow(result.DisplayName ?? username, roles); + return new CoreOutcome(OpcUaUserAuthResult.Allow(result.DisplayName ?? username, roles), SystemFailure: false); } catch (Exception ex) when (ex is not OperationCanceledException) { _logger.LogWarning(ex, "LDAP authentication threw for OPC UA user {User}", username); - return OpcUaUserAuthResult.Deny("Authentication backend error"); + return new CoreOutcome(OpcUaUserAuthResult.Deny("Authentication backend error"), SystemFailure: true); } } + /// The result of the core authenticate flow plus whether it was a system-side (directory) + /// failure — the signal that drives the outage circuit. + /// The auth result to return to the SDK. + /// Whether the outcome was a directory-side fault. + private readonly record struct CoreOutcome(OpcUaUserAuthResult Result, bool SystemFailure); + /// /// Resolves the user's roles from their LDAP groups via the scoped /// , unioned with any pre-resolved roles (the DevStub diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs index 1447894b..58555439 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs @@ -105,6 +105,118 @@ public sealed class LdapAuthResilienceTests await first.WaitAsync(TimeSpan.FromSeconds(10), CancellationToken.None); } + /// After OutageFailureThreshold consecutive system-side failures the circuit opens and + /// the next call is fast-denied "backend unavailable" WITHOUT touching LDAP (03/S2). + [Fact] + public async Task Consecutive_system_failures_open_circuit_and_fast_deny() + { + var ldap = new DelayingFakeLdap(TimeSpan.Zero, SystemFailure()); + var mapper = new FakeMapper(g => g.ToArray()); + var sut = Build(ldap, mapper, + new LdapOptions { OutageFailureThreshold = 3, OutageCooldownSeconds = 60, AuthTimeoutMs = 5000 }); + + for (var i = 0; i < 3; i++) + { + var r = await sut.AuthenticateUserNameAsync("u", "p", CancellationToken.None) + .WaitAsync(TimeSpan.FromSeconds(10), CancellationToken.None); + r.Success.ShouldBeFalse(); + } + + ldap.Calls.ShouldBe(3); + + var sw = Stopwatch.StartNew(); + var blocked = await sut.AuthenticateUserNameAsync("u", "p", CancellationToken.None) + .WaitAsync(TimeSpan.FromSeconds(10), CancellationToken.None); + sw.Stop(); + + blocked.Success.ShouldBeFalse(); + blocked.Error.ShouldNotBeNull(); + blocked.Error!.ShouldContain("unavailable"); + ldap.Calls.ShouldBe(3, "an open circuit must fast-deny without reaching the backend"); + sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(1)); + } + + /// A user-side (credential) deny between system failures resets the streak, so the circuit + /// never opens — user denies are not directory evidence (03/S2). + [Fact] + public async Task User_side_deny_resets_the_system_failure_streak() + { + var ldap = new ScriptedFakeLdap( + SystemFailure(), SystemFailure(), UserDeny(), SystemFailure(), SystemFailure()); + var mapper = new FakeMapper(g => g.ToArray()); + var sut = Build(ldap, mapper, + new LdapOptions { OutageFailureThreshold = 3, OutageCooldownSeconds = 60, AuthTimeoutMs = 5000 }); + + // 5 scripted calls: sys, sys, user(reset), sys, sys → streak ends at 2 (< 3), circuit stays closed. + for (var i = 0; i < 5; i++) + { + await sut.AuthenticateUserNameAsync("u", "p", CancellationToken.None) + .WaitAsync(TimeSpan.FromSeconds(10), CancellationToken.None); + } + + // A 6th call must still REACH the backend (circuit never opened). + await sut.AuthenticateUserNameAsync("u", "p", CancellationToken.None) + .WaitAsync(TimeSpan.FromSeconds(10), CancellationToken.None); + ldap.Calls.ShouldBe(6); + } + + /// After the cooldown elapses the circuit half-opens: the next call probes the backend and a + /// success closes the circuit (03/S2). + [Fact] + public async Task Half_open_probe_after_cooldown_closes_on_success() + { + var ldap = new ScriptedFakeLdap( + SystemFailure(), SystemFailure(), SystemFailure(), + new LdapAuthResult(true, "Alice", "alice", new[] { "configeditor" }, Array.Empty(), null)); + var mapper = new FakeMapper(g => g.ToArray()); + var sut = Build(ldap, mapper, + new LdapOptions { OutageFailureThreshold = 3, OutageCooldownSeconds = 1, AuthTimeoutMs = 5000 }); + + for (var i = 0; i < 3; i++) + await sut.AuthenticateUserNameAsync("u", "p", CancellationToken.None) + .WaitAsync(TimeSpan.FromSeconds(10), CancellationToken.None); + ldap.Calls.ShouldBe(3); + + // Circuit open: this call is fast-denied without touching the backend. + var blocked = await sut.AuthenticateUserNameAsync("u", "p", CancellationToken.None) + .WaitAsync(TimeSpan.FromSeconds(10), CancellationToken.None); + blocked.Error!.ShouldContain("unavailable"); + ldap.Calls.ShouldBe(3); + + // Wait past the cooldown, then a probe reaches the backend and succeeds → circuit closes. + await Task.Delay(TimeSpan.FromMilliseconds(1200), TestContext.Current.CancellationToken); + + var probe = await sut.AuthenticateUserNameAsync("alice", "secret", CancellationToken.None) + .WaitAsync(TimeSpan.FromSeconds(10), CancellationToken.None); + probe.Success.ShouldBeTrue(); + ldap.Calls.ShouldBe(4, "the half-open probe reaches the backend"); + } + + /// OutageFailureThreshold = 0 disables the circuit — it never opens, every call reaches + /// the backend (03/S2). + [Fact] + public async Task Circuit_disabled_never_opens() + { + var ldap = new DelayingFakeLdap(TimeSpan.Zero, SystemFailure()); + var mapper = new FakeMapper(g => g.ToArray()); + var sut = Build(ldap, mapper, + new LdapOptions { OutageFailureThreshold = 0, OutageCooldownSeconds = 60, AuthTimeoutMs = 5000 }); + + for (var i = 0; i < 6; i++) + await sut.AuthenticateUserNameAsync("u", "p", CancellationToken.None) + .WaitAsync(TimeSpan.FromSeconds(10), CancellationToken.None); + + ldap.Calls.ShouldBe(6, "a disabled circuit never fast-denies"); + } + + private static LdapAuthResult SystemFailure() => + new(false, null, "u", Array.Empty(), Array.Empty(), "Unexpected authentication error") + { IsSystemFailure = true }; + + private static LdapAuthResult UserDeny() => + new(false, null, "u", Array.Empty(), Array.Empty(), "Invalid username or password") + { IsSystemFailure = false }; + private static LdapOpcUaUserAuthenticator Build( ILdapAuthService ldap, IGroupRoleMapper mapper, LdapOptions options) => new(ldap, ScopeFactoryWith(mapper), Options.Create(options), @@ -187,6 +299,34 @@ public sealed class LdapAuthResilienceTests } } + /// + /// A fake that returns a scripted sequence of results (one per call; + /// the last entry repeats once exhausted). Counts calls so the outage-circuit tests can assert the + /// backend was — or was NOT — reached on a given call. + /// + private sealed class ScriptedFakeLdap : ILdapAuthService + { + private readonly IReadOnlyList _script; + private int _calls; + + /// Initializes the fake with the scripted results, returned in order. + /// The results to return, one per call; the last repeats once exhausted. + public ScriptedFakeLdap(params LdapAuthResult[] script) => _script = script; + + /// The number of times the backend was invoked. + public int Calls => Volatile.Read(ref _calls); + + /// Returns the next scripted result (clamped to the last entry). + /// The username (ignored). + /// The password (ignored). + /// The cancellation token (ignored). + public Task AuthenticateAsync(string username, string password, CancellationToken ct = default) + { + var idx = Interlocked.Increment(ref _calls) - 1; + return Task.FromResult(_script[Math.Min(idx, _script.Count - 1)]); + } + } + /// Test group→role mapper driven by a delegate over the supplied groups. private sealed class FakeMapper(Func, IReadOnlyList> map) : IGroupRoleMapper { From 62a49779acf86846ac7bd872444db49eb675a8f5 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 12:00:35 -0400 Subject: [PATCH 06/14] test(host): unreachable-directory activation fails bounded; document the SDK impersonation threading contract (03/S2) --- ...-ldap-historyread-async-plan.md.tasks.json | 2 +- docs/security.md | 14 ++++++ .../OpcUaApplicationHost.cs | 13 +++++ .../ZB.MOM.WW.OtOpcUa.OpcUaServer.csproj | 3 ++ .../LdapAuthResilienceTests.cs | 50 +++++++++++++++++++ 5 files changed, 81 insertions(+), 1 deletion(-) diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json index 824e864e..9f18d8f3 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json @@ -6,7 +6,7 @@ { "id": "R2-08-T3", "subject": "LdapAuthResult.IsSystemFailure seam + OtOpcUaLdapAuthService.Adapt mapping (ServiceAccountBindFailed => system-side)", "status": "completed", "blockedBy": [] }, { "id": "R2-08-T4", "subject": "LdapOpcUaUserAuthenticator: WaitAsync wall-clock bound + SemaphoreSlim concurrency cap with release-on-core-completion (turns T1 green)", "status": "completed", "blockedBy": ["R2-08-T1", "R2-08-T2"] }, { "id": "R2-08-T5", "subject": "Directory-outage circuit: fail-fast deny after N consecutive system-side failures, half-open after cooldown", "status": "completed", "blockedBy": ["R2-08-T3", "R2-08-T4"] }, - { "id": "R2-08-T6", "subject": "Outage-sim integration test (real library, unroutable host, via internal HandleImpersonation) + SDK m_eventLock contract doc + docs/security.md", "status": "pending", "blockedBy": ["R2-08-T4"] }, + { "id": "R2-08-T6", "subject": "Outage-sim integration test (real library, unroutable host, via internal HandleImpersonation) + SDK m_eventLock contract doc + docs/security.md", "status": "completed", "blockedBy": ["R2-08-T4"] }, { "id": "R2-08-T7", "subject": "S3 sequential-serving repro test (RED): MaxObservedConcurrency >= 2 on an 8-node Raw batch — fails on current code", "status": "pending", "blockedBy": [] }, { "id": "R2-08-T8", "subject": "ServerHistorianOptions: HistoryReadBatchParallelism/MaxConcurrentHistoryReadBatches/HistoryReadDeadline + Validate warnings + node-manager properties + hosted-service wiring", "status": "pending", "blockedBy": [] }, { "id": "R2-08-T9", "subject": "Async-ify ServeNode + Processed/AtTime arms (ct threading, OCE=>BadTimeout, behavior-neutral)", "status": "pending", "blockedBy": ["R2-08-T7", "R2-08-T8"] }, diff --git a/docs/security.md b/docs/security.md index 0e7df469..3c925110 100644 --- a/docs/security.md +++ b/docs/security.md @@ -162,6 +162,20 @@ LDAP is configured under the `Security:Ldap` section (bound to `LdapOptions`, `s `GroupToRole` maps LDAP group names → roles (case-insensitive); a user gets every role whose source group is in their membership. The values are the canonical control-plane role strings (`Viewer` / `Designer` / `Administrator`, plus the appsettings-only `Operator` for the `DriverOperator` policy); the same map also supplies data-plane role strings (`ReadOnly`, `WriteOperate`, `WriteTune`, `WriteConfigure`, `AlarmAck`) — see [Role grant source (data-plane)](#role-grant-source-data-plane) below. `UserNameAttribute: "sAMAccountName"` is the critical AD override — the GLAuth dev default is `cn`, which is not how AD users are looked up; use `userPrincipalName` instead if operators log in with `user@corp.example.com` form. `LdapOptionsValidator` (`src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/LdapOptionsValidator.cs`) fails startup when `Transport = None` and `AllowInsecure = false` on a real-LDAP (non-DevStub) config. +### LDAP resilience — timeout, concurrency, outage circuit (arch-review 03/S2) + +The OPC UA SDK raises its impersonation (UserName) callback **synchronously**, inside a `SessionManager`-wide event lock (verified against `OPCFoundation.NetStandard.Opc.Ua.Server` 1.5.378). A slow or hung LDAP bind inside that callback therefore does not stall one SDK thread — it **serializes every session activation server-wide** (Anonymous and X509 included). Since the callback is irreducibly synchronous, `LdapOpcUaUserAuthenticator` bounds it at its own boundary with five `Security:Ldap` keys (all validated at startup by `LdapOptionsValidator` when `Enabled && !DevStubMode`): + +| Key | Default | Bounds | +|---|---|---| +| `ConnectionTimeoutMs` | `10000` | Projected into the shared library's `ConnectionTimeoutMs` — bounds the socket connect **and** each per-operation search. First line of defence against an outage. | +| `AuthTimeoutMs` | `15000` | Hard whole-flow wall-clock bound (bind **and** role-map) enforced at the authenticator boundary; a slow/hung flow denies **"Authentication timed out"** (fail-closed) and releases the SDK thread. Deliberately `> ConnectionTimeoutMs` so a healthy-but-slow flow is not cut off. | +| `MaxConcurrentAuthentications` | `8` | Max in-flight authentications; excess denies **"Authentication backend busy"** (local backpressure — does **not** feed the circuit). Bounds accumulation of orphaned/timed-out binds during an outage. | +| `OutageFailureThreshold` | `3` | Consecutive **system-side** failures (boundary timeout, directory-unreachable, or unexpected throw) that open the outage circuit. Any success or user-credential deny resets the streak. `0` disables the circuit. | +| `OutageCooldownSeconds` | `15` | How long the open circuit fast-denies **"Authentication backend unavailable"** without touching LDAP; after cooldown the next call half-open-probes, and one more system failure re-opens immediately. | + +Defaults are behaviour-neutral on a healthy directory. All new paths are **fail-closed** (they deny). A user-credential deny (wrong password) is fast when the directory is up and never trips the circuit; only a directory *outage* opens it. + --- ## Data-Plane Authorization diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs index 8bd0c3a5..7a62694e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs @@ -235,6 +235,19 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable /// the full SDK. Side-effects are confined to mutating /// and logging. /// + /// + /// SDK threading contract (verified against OPCFoundation.NetStandard.Opc.Ua.Server 1.5.378). + /// The SDK raises this handler SYNCHRONOUSLY — it is a plain void delegate, and there is no + /// async impersonation callback in this SDK version. Worse, SessionManager.ActivateSessionAsync + /// invokes it while holding a single SessionManager-wide event lock (m_eventLock), so a + /// handler that blocks does not merely stall one SDK request thread — it SERIALIZES every session + /// activation server-wide (Anonymous and X509 included, since even those paths must take the lock to + /// raise the handler). Consequently the block-bridge below + /// (GetAwaiter().GetResult() under ) is irreducible: the + /// callback must complete inline. Because the SDK gives no timeout, the hard wall-clock bound + bounded + /// concurrency + directory-outage circuit that keep a slow/hung directory from wedging the SDK live + /// inside the authenticator (LdapOpcUaUserAuthenticator, arch-review 03/S2) — NOT here. + /// /// The user authenticator to validate credentials. /// The impersonation event arguments to process. /// The logger for diagnostic output. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/ZB.MOM.WW.OtOpcUa.OpcUaServer.csproj b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/ZB.MOM.WW.OtOpcUa.OpcUaServer.csproj index 82dd01a3..e401905e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/ZB.MOM.WW.OtOpcUa.OpcUaServer.csproj +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/ZB.MOM.WW.OtOpcUa.OpcUaServer.csproj @@ -23,6 +23,9 @@ + + diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs index 58555439..f0d27517 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/LdapAuthResilienceTests.cs @@ -1,12 +1,17 @@ using System.Diagnostics; +using System.Text; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; +using Opc.Ua; +using Opc.Ua.Server; using Shouldly; using Xunit; using ZB.MOM.WW.Auth.Abstractions.Roles; using ZB.MOM.WW.OtOpcUa.Host.OpcUa; +using ZB.MOM.WW.OtOpcUa.OpcUaServer; using ZB.MOM.WW.OtOpcUa.Security.Ldap; +using LdapTransport = ZB.MOM.WW.Auth.Abstractions.Ldap.LdapTransport; namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests; @@ -209,6 +214,51 @@ public sealed class LdapAuthResilienceTests ldap.Calls.ShouldBe(6, "a disabled circuit never fast-denies"); } + /// + /// Outage-sim (offline-safe): the REAL + real shared library + /// pointed at an UNROUTABLE host (a TCP blackhole — the same outage shape as "stop the GLAuth + /// container"), driven through the REAL internal + /// block-bridge. The activation must fail bounded (well under the SDK's default ~10-20 s stall) with + /// and no granted identity (03/S2). + /// + [Fact] + public async Task Activation_with_unreachable_directory_fails_within_bound() + { + var options = new LdapOptions + { + Enabled = true, + DevStubMode = false, + Server = "10.255.255.1", // RFC-blackhole: SYN goes nowhere + Port = 3893, + Transport = LdapTransport.None, + AllowInsecure = true, + SearchBase = "dc=zb,dc=local", + ConnectionTimeoutMs = 500, + AuthTimeoutMs = 1000, + }; + + var ldap = new OtOpcUaLdapAuthService(Options.Create(options), NullLogger.Instance); + var mapper = new FakeMapper(g => g.ToArray()); + var authenticator = new LdapOpcUaUserAuthenticator( + ldap, ScopeFactoryWith(mapper), Options.Create(options), + NullLogger.Instance); + + var token = new UserNameIdentityToken { UserName = "alice", DecryptedPassword = Encoding.UTF8.GetBytes("secret") }; + var policy = new UserTokenPolicy(UserTokenType.UserName) { PolicyId = "username_basic256sha256" }; + var args = new ImpersonateEventArgs(token, policy, new EndpointDescription()); + + var sw = Stopwatch.StartNew(); + await Task.Run(() => OpcUaApplicationHost.HandleImpersonation( + authenticator, args, NullLogger.Instance), TestContext.Current.CancellationToken) + .WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + sw.Stop(); + + args.Identity.ShouldBeNull(); + args.IdentityValidationError.ShouldNotBeNull(); + args.IdentityValidationError.Code.ShouldBe(Opc.Ua.StatusCodes.BadIdentityTokenRejected); + sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(3)); + } + private static LdapAuthResult SystemFailure() => new(false, null, "u", Array.Empty(), Array.Empty(), "Unexpected authentication error") { IsSystemFailure = true }; From d5c6609cdf3eadff1fc23e08a18b71f4a512b28a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 12:01:47 -0400 Subject: [PATCH 07/14] =?UTF-8?q?test(opcuaserver):=20red=20repro=20?= =?UTF-8?q?=E2=80=94=20HistoryRead=20serves=20a=20batch=20strictly=20seque?= =?UTF-8?q?ntially=20(03/S3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...-ldap-historyread-async-plan.md.tasks.json | 2 +- .../NodeManagerHistoryReadConcurrencyTests.cs | 200 ++++++++++++++++++ 2 files changed, 201 insertions(+), 1 deletion(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadConcurrencyTests.cs diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json index 9f18d8f3..530ed9e9 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json @@ -7,7 +7,7 @@ { "id": "R2-08-T4", "subject": "LdapOpcUaUserAuthenticator: WaitAsync wall-clock bound + SemaphoreSlim concurrency cap with release-on-core-completion (turns T1 green)", "status": "completed", "blockedBy": ["R2-08-T1", "R2-08-T2"] }, { "id": "R2-08-T5", "subject": "Directory-outage circuit: fail-fast deny after N consecutive system-side failures, half-open after cooldown", "status": "completed", "blockedBy": ["R2-08-T3", "R2-08-T4"] }, { "id": "R2-08-T6", "subject": "Outage-sim integration test (real library, unroutable host, via internal HandleImpersonation) + SDK m_eventLock contract doc + docs/security.md", "status": "completed", "blockedBy": ["R2-08-T4"] }, - { "id": "R2-08-T7", "subject": "S3 sequential-serving repro test (RED): MaxObservedConcurrency >= 2 on an 8-node Raw batch — fails on current code", "status": "pending", "blockedBy": [] }, + { "id": "R2-08-T7", "subject": "S3 sequential-serving repro test (RED): MaxObservedConcurrency >= 2 on an 8-node Raw batch — fails on current code", "status": "completed", "blockedBy": [] }, { "id": "R2-08-T8", "subject": "ServerHistorianOptions: HistoryReadBatchParallelism/MaxConcurrentHistoryReadBatches/HistoryReadDeadline + Validate warnings + node-manager properties + hosted-service wiring", "status": "pending", "blockedBy": [] }, { "id": "R2-08-T9", "subject": "Async-ify ServeNode + Processed/AtTime arms (ct threading, OCE=>BadTimeout, behavior-neutral)", "status": "pending", "blockedBy": ["R2-08-T7", "R2-08-T8"] }, { "id": "R2-08-T10", "subject": "Async-ify ServeRawPaged (page read + tie-cluster over-fetch) + extract ServeEventsAsync", "status": "pending", "blockedBy": ["R2-08-T9"] }, diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadConcurrencyTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadConcurrencyTests.cs new file mode 100644 index 00000000..37599d8a --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadConcurrencyTests.cs @@ -0,0 +1,200 @@ +using System.Diagnostics; +using Microsoft.Extensions.Logging.Abstractions; +using Opc.Ua; +using Opc.Ua.Server; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using HistorianRead = ZB.MOM.WW.OtOpcUa.Core.Abstractions.HistoryReadResult; +using SdkHistoryReadResult = Opc.Ua.HistoryReadResult; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; + +/// +/// Arch-review 03/S3 — the node-manager's HistoryRead arms must serve a multi-node batch with BOUNDED +/// per-node parallelism, a per-request deadline, and a process-wide concurrent-batch limiter, rather +/// than block-bridging the gateway per node sequentially on an SDK request thread. Boots the same real +/// harness as and drives the +/// public HistoryRead with a delaying fake that records the +/// maximum observed concurrent reads (deterministic — no raw-timing dependence for the primary +/// assertions). +/// +public sealed class NodeManagerHistoryReadConcurrencyTests : IDisposable +{ + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + private readonly string _pkiRoot = Path.Combine( + Path.GetTempPath(), + $"otopcua-historyread-conc-{Guid.NewGuid():N}"); + + /// + /// RED repro (03/S3): a single Raw HistoryRead naming 8 historized nodes against a 150 ms-per-read + /// backend must serve at least two nodes concurrently. On the current sequential foreach the + /// max observed concurrency is pinned at 1, so this fails. + /// + [Fact] + public async Task Raw_batch_is_served_with_bounded_parallelism() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + var fake = new ConcurrencyTrackingHistorianDataSource(TimeSpan.FromMilliseconds(150)); + nm.HistorianDataSource = fake; + + var nodeIds = MaterializeHistorizedNodes(nm, count: 8); + + var details = new ReadRawModifiedDetails + { + StartTime = DateTime.UtcNow.AddHours(-1), + EndTime = DateTime.UtcNow, + NumValuesPerNode = 10, + IsReadModified = false, + }; + + var sw = Stopwatch.StartNew(); + await Task.Run(() => InvokeHistoryRead(server, nm, details, nodeIds), Ct) + .WaitAsync(TimeSpan.FromSeconds(15), Ct); + sw.Stop(); + + // Primary (deterministic): the batch was served with >1 node in flight at once. + fake.MaxObservedConcurrency.ShouldBeGreaterThanOrEqualTo(2); + + await host.DisposeAsync(); + } + + /// Materialises historized Float variables and returns their NodeIds. + private static NodeId[] MaterializeHistorizedNodes(OtOpcUaNodeManager nm, int count) + { + var ids = new NodeId[count]; + for (var i = 0; i < count; i++) + { + var key = $"eq/tag{i}"; + nm.EnsureVariable(key, parentFolderNodeId: null, displayName: $"Tag{i}", dataType: "Float", + writable: false, historianTagname: $"WW.Tag{i}"); + ids[i] = nm.TryGetVariable(key)!.NodeId; + } + + return ids; + } + + /// Issues one HistoryRead batch for all supplied node ids in order. + private static (IList Results, IList Errors) InvokeHistoryRead( + OtOpcUaSdkServer server, OtOpcUaNodeManager nm, HistoryReadDetails details, params NodeId[] nodeIds) + { + var context = new OperationContext( + new RequestHeader(), secureChannelContext: null, RequestType.HistoryRead, identity: null); + + var nodesToRead = nodeIds.Select(id => new HistoryReadValueId { NodeId = id }).ToList(); + var results = Enumerable.Repeat(null!, nodeIds.Length).ToList(); + var errors = Enumerable.Repeat(null!, nodeIds.Length).ToList(); + + nm.HistoryRead( + context, + details, + TimestampsToReturn.Both, + releaseContinuationPoints: false, + nodesToRead, + results, + errors); + + return (results, errors); + } + + /// + /// A delaying fake historian source that tracks the maximum number of reads in flight at once + /// (via interlocked live-counter) so a test can prove per-node parallelism deterministically. + /// + private sealed class ConcurrencyTrackingHistorianDataSource(TimeSpan delay) : IHistorianDataSource + { + private int _current; + private int _max; + + /// The highest number of concurrently in-flight reads observed. + public int MaxObservedConcurrency => Volatile.Read(ref _max); + + private async Task TrackAsync(CancellationToken ct) + { + var now = Interlocked.Increment(ref _current); + UpdateMax(now); + try + { + await Task.Delay(delay, ct).ConfigureAwait(false); + } + finally + { + Interlocked.Decrement(ref _current); + } + + return new HistorianRead( + new[] { new DataValueSnapshot(1.0f, StatusCodes.Good, DateTime.UtcNow, DateTime.UtcNow) }, null); + } + + private void UpdateMax(int observed) + { + int cur; + while ((cur = Volatile.Read(ref _max)) < observed && + Interlocked.CompareExchange(ref _max, observed, cur) != cur) + { + } + } + + public Task ReadRawAsync( + string fullReference, DateTime startUtc, DateTime endUtc, uint maxValuesPerNode, + CancellationToken cancellationToken) => TrackAsync(cancellationToken); + + public Task ReadProcessedAsync( + string fullReference, DateTime startUtc, DateTime endUtc, TimeSpan interval, + HistoryAggregateType aggregate, CancellationToken cancellationToken) => TrackAsync(cancellationToken); + + public Task ReadAtTimeAsync( + string fullReference, IReadOnlyList timestampsUtc, CancellationToken cancellationToken) => + TrackAsync(cancellationToken); + + public Task ReadEventsAsync( + string? sourceName, DateTime startUtc, DateTime endUtc, int maxEvents, + CancellationToken cancellationToken) => + Task.FromResult(new HistoricalEventsResult(Array.Empty(), null)); + + public HistorianHealthSnapshot GetHealthSnapshot() => NullHistorianDataSource.Instance.GetHealthSnapshot(); + + public void Dispose() + { + } + } + + private async Task<(OpcUaApplicationHost Host, OtOpcUaSdkServer Server)> BootAsync() + { + var host = new OpcUaApplicationHost( + new OpcUaApplicationHostOptions + { + ApplicationName = "OtOpcUa.HistoryReadConcTest", + ApplicationUri = $"urn:OtOpcUa.HistoryReadConcTest:{Guid.NewGuid():N}", + OpcUaPort = AllocateFreePort(), + PublicHostname = "localhost", + PkiStoreRoot = _pkiRoot, + }, + NullLogger.Instance); + + var server = new OtOpcUaSdkServer(); + await host.StartAsync(server, Ct); + return (host, server); + } + + private static int AllocateFreePort() + { + using var listener = new System.Net.Sockets.TcpListener(System.Net.IPAddress.Loopback, 0); + listener.Start(); + var port = ((System.Net.IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + return port; + } + + /// Cleans up the PKI root directory. + public void Dispose() + { + if (Directory.Exists(_pkiRoot)) + { + try { Directory.Delete(_pkiRoot, recursive: true); } + catch { /* best-effort cleanup */ } + } + } +} From 304442dba4995f1597c04270b784d76f03947bc4 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 12:03:27 -0400 Subject: [PATCH 08/14] feat(runtime): ServerHistorian HistoryRead parallelism/limiter/deadline knobs + node-manager wiring (03/S3) --- ...-ldap-historyread-async-plan.md.tasks.json | 2 +- .../OpcUa/OtOpcUaServerHostedService.cs | 7 +++ .../OtOpcUaNodeManager.cs | 24 +++++++++ .../Historian/ServerHistorianOptions.cs | 34 +++++++++++++ .../Historian/ServerHistorianOptionsTests.cs | 51 +++++++++++++++++++ 5 files changed, 117 insertions(+), 1 deletion(-) diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json index 530ed9e9..e0df2097 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json @@ -8,7 +8,7 @@ { "id": "R2-08-T5", "subject": "Directory-outage circuit: fail-fast deny after N consecutive system-side failures, half-open after cooldown", "status": "completed", "blockedBy": ["R2-08-T3", "R2-08-T4"] }, { "id": "R2-08-T6", "subject": "Outage-sim integration test (real library, unroutable host, via internal HandleImpersonation) + SDK m_eventLock contract doc + docs/security.md", "status": "completed", "blockedBy": ["R2-08-T4"] }, { "id": "R2-08-T7", "subject": "S3 sequential-serving repro test (RED): MaxObservedConcurrency >= 2 on an 8-node Raw batch — fails on current code", "status": "completed", "blockedBy": [] }, - { "id": "R2-08-T8", "subject": "ServerHistorianOptions: HistoryReadBatchParallelism/MaxConcurrentHistoryReadBatches/HistoryReadDeadline + Validate warnings + node-manager properties + hosted-service wiring", "status": "pending", "blockedBy": [] }, + { "id": "R2-08-T8", "subject": "ServerHistorianOptions: HistoryReadBatchParallelism/MaxConcurrentHistoryReadBatches/HistoryReadDeadline + Validate warnings + node-manager properties + hosted-service wiring", "status": "completed", "blockedBy": [] }, { "id": "R2-08-T9", "subject": "Async-ify ServeNode + Processed/AtTime arms (ct threading, OCE=>BadTimeout, behavior-neutral)", "status": "pending", "blockedBy": ["R2-08-T7", "R2-08-T8"] }, { "id": "R2-08-T10", "subject": "Async-ify ServeRawPaged (page read + tie-cluster over-fetch) + extract ServeEventsAsync", "status": "pending", "blockedBy": ["R2-08-T9"] }, { "id": "R2-08-T11", "subject": "RunBounded fan-out in all four HistoryRead arms, single bridge per arm (turns T7 green) + upper-bound + isolation tests", "status": "pending", "blockedBy": ["R2-08-T9", "R2-08-T10"] }, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs index c9b16310..fe582630 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs @@ -222,6 +222,13 @@ public sealed class OtOpcUaServerHostedService : IHostedService, IAsyncDisposabl // section is absent; NodeManager is non-null here (guarded above). _server.NodeManager.MaxTieClusterOverfetch = _serverHistorianOptions.MaxTieClusterOverfetch; + // Propagate the S3 HistoryRead concurrency bounds (arch-review 03/S3): bounded per-node fan-out + // within a batch, the process-wide concurrent-batch limiter, and the per-request deadline. Defaults + // (4 / 16 / 60 s) survive when the ServerHistorian section is absent. + _server.NodeManager.HistoryReadBatchParallelism = _serverHistorianOptions.HistoryReadBatchParallelism; + _server.NodeManager.MaxConcurrentHistoryReadBatches = _serverHistorianOptions.MaxConcurrentHistoryReadBatches; + _server.NodeManager.HistoryReadDeadline = _serverHistorianOptions.HistoryReadDeadline; + // ServiceLevel publisher needs IServerInternal — only available after Start. if (_server.CurrentInstance is { } serverInternal) { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 23c08e95..ef90e691 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -197,6 +197,30 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// public int MaxTieClusterOverfetch { get; set; } = 65536; + /// + /// Maximum historized nodes served concurrently within one HistoryRead batch (arch-review 03/S3). + /// Mirrors ServerHistorianOptions.HistoryReadBatchParallelism; the Host sets it at + /// StartAsync. ≤ 1 falls back to sequential serving. The default (4) survives when the + /// ServerHistorian section is absent. + /// + public int HistoryReadBatchParallelism { get; set; } = 4; + + /// + /// Process-wide cap on concurrently-served HistoryRead batches (arch-review 03/S3). A batch that + /// cannot acquire a slot within its (bounded) wait budget fast-fails every handle with + /// BadTooManyOperations. Mirrors ServerHistorianOptions.MaxConcurrentHistoryReadBatches; + /// the Host sets it at StartAsync. The default (16) survives when the section is absent. + /// + public int MaxConcurrentHistoryReadBatches { get; set; } = 16; + + /// + /// Per-request wall-clock deadline for a HistoryRead batch (arch-review 03/S3). A node still in + /// flight at the deadline surfaces BadTimeout. Non-positive = unbounded. Mirrors + /// ServerHistorianOptions.HistoryReadDeadline; the Host sets it at StartAsync. The + /// default (60 s) survives when the section is absent. + /// + public TimeSpan HistoryReadDeadline { get; set; } = TimeSpan.FromSeconds(60); + private volatile IHistoryContinuationStore _historyContinuationStore = new SessionHistoryContinuationStore(); /// diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ServerHistorianOptions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ServerHistorianOptions.cs index 98f4eb56..d714ccb9 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ServerHistorianOptions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ServerHistorianOptions.cs @@ -68,6 +68,32 @@ public sealed class ServerHistorianOptions /// public int MaxTieClusterOverfetch { get; init; } = 65536; + /// + /// Maximum number of historized nodes served CONCURRENTLY within one HistoryRead batch (arch-review + /// 03/S3). The SDK's HistoryRead* override surface is synchronous, so each arm block-bridges + /// once at its boundary; this bound turns the per-node fan-out inside that boundary from sequential + /// (worst case N × CallTimeout) into ⌈N / P⌉ × CallTimeout. It also caps the gateway-load + /// multiplication a single client can command. Default 4. + /// + public int HistoryReadBatchParallelism { get; init; } = 4; + + /// + /// Process-wide limit on the number of HistoryRead batches served CONCURRENTLY (arch-review 03/S3). + /// A flood of history clients degrades to BadTooManyOperations rather than exhausting the SDK + /// request-thread pool. Together with this caps total + /// in-flight gateway reads at MaxConcurrentHistoryReadBatches × HistoryReadBatchParallelism + /// (default 16 × 4 = 64). Default 16. + /// + public int MaxConcurrentHistoryReadBatches { get; init; } = 16; + + /// + /// Per-request wall-clock deadline for a HistoryRead batch (arch-review 03/S3). A single request can + /// no longer hold an SDK request thread longer than this regardless of node count; a node still + /// in flight at the deadline surfaces BadTimeout. Non-positive = unbounded (a + /// warning). Default 60 seconds. + /// + public TimeSpan HistoryReadDeadline { get; init; } = TimeSpan.FromSeconds(60); + /// Returns operator-facing misconfiguration warnings for an Enabled historian /// (empty when disabled or correctly configured). Pure — the registration logs each entry. /// @@ -100,6 +126,14 @@ public sealed class ServerHistorianOptions // so a zero/negative value is harmless (and noise-free) when the historian is disabled. if (MaxTieClusterOverfetch <= 0) warnings.Add($"ServerHistorian:MaxTieClusterOverfetch is {MaxTieClusterOverfetch} — must be > 0; HistoryRead-Raw cannot page within an oversized tie cluster and will surface BadHistoryOperationUnsupported for those reads."); + // S3 HistoryRead concurrency knobs — a non-positive value disables the corresponding bound, so warn + // (these gate the per-node fan-out / batch limiter that only run when a real data source is wired). + if (HistoryReadBatchParallelism <= 0) + warnings.Add($"ServerHistorian:HistoryReadBatchParallelism is {HistoryReadBatchParallelism} — must be > 0; HistoryRead falls back to serving each node sequentially."); + if (MaxConcurrentHistoryReadBatches <= 0) + warnings.Add($"ServerHistorian:MaxConcurrentHistoryReadBatches is {MaxConcurrentHistoryReadBatches} — must be > 0; the process-wide HistoryRead batch limiter is disabled."); + if (HistoryReadDeadline <= TimeSpan.Zero) + warnings.Add($"ServerHistorian:HistoryReadDeadline is {HistoryReadDeadline} — must be > 0; HistoryRead requests run unbounded (no per-request deadline)."); return warnings; } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ServerHistorianOptionsTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ServerHistorianOptionsTests.cs index 88acedae..458070cd 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ServerHistorianOptionsTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ServerHistorianOptionsTests.cs @@ -50,4 +50,55 @@ public sealed class ServerHistorianOptionsTests .Validate(); Assert.Contains(w, m => m.Contains("MaxTieClusterOverfetch")); } + + // ── S3 HistoryRead concurrency knobs (arch-review 03/S3) ──────────────────────────────────── + + [Fact] + public void HistoryRead_knob_defaults() + { + var o = new ServerHistorianOptions(); + Assert.Equal(4, o.HistoryReadBatchParallelism); + Assert.Equal(16, o.MaxConcurrentHistoryReadBatches); + Assert.Equal(TimeSpan.FromSeconds(60), o.HistoryReadDeadline); + } + + [Fact] + public void Enabled_with_nonpositive_HistoryReadBatchParallelism_warns() + { + var w = new ServerHistorianOptions + { Enabled = true, Endpoint = "https://h:5222", ApiKey = "histgw_x_y", HistoryReadBatchParallelism = 0 } + .Validate(); + Assert.Contains(w, m => m.Contains("HistoryReadBatchParallelism")); + } + + [Fact] + public void Enabled_with_nonpositive_MaxConcurrentHistoryReadBatches_warns() + { + var w = new ServerHistorianOptions + { Enabled = true, Endpoint = "https://h:5222", ApiKey = "histgw_x_y", MaxConcurrentHistoryReadBatches = 0 } + .Validate(); + Assert.Contains(w, m => m.Contains("MaxConcurrentHistoryReadBatches")); + } + + [Fact] + public void Enabled_with_nonpositive_HistoryReadDeadline_warns() + { + var w = new ServerHistorianOptions + { Enabled = true, Endpoint = "https://h:5222", ApiKey = "histgw_x_y", HistoryReadDeadline = TimeSpan.Zero } + .Validate(); + Assert.Contains(w, m => m.Contains("HistoryReadDeadline")); + } + + [Fact] + public void Disabled_with_nonpositive_HistoryRead_knobs_is_silent() + { + var w = new ServerHistorianOptions + { + Enabled = false, + HistoryReadBatchParallelism = 0, + MaxConcurrentHistoryReadBatches = 0, + HistoryReadDeadline = TimeSpan.Zero, + }.Validate(); + Assert.Empty(w); + } } From 1e83fe7085969053a134e978c5f2135e807870ab Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 12:04:54 -0400 Subject: [PATCH 09/14] refactor(opcuaserver): async-ify ServeNode + Processed/AtTime HistoryRead internals, thread cancellation (03/S3) --- ...-ldap-historyread-async-plan.md.tasks.json | 2 +- .../OtOpcUaNodeManager.cs | 38 ++++++++++++------- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json index e0df2097..4b4da57f 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json @@ -9,7 +9,7 @@ { "id": "R2-08-T6", "subject": "Outage-sim integration test (real library, unroutable host, via internal HandleImpersonation) + SDK m_eventLock contract doc + docs/security.md", "status": "completed", "blockedBy": ["R2-08-T4"] }, { "id": "R2-08-T7", "subject": "S3 sequential-serving repro test (RED): MaxObservedConcurrency >= 2 on an 8-node Raw batch — fails on current code", "status": "completed", "blockedBy": [] }, { "id": "R2-08-T8", "subject": "ServerHistorianOptions: HistoryReadBatchParallelism/MaxConcurrentHistoryReadBatches/HistoryReadDeadline + Validate warnings + node-manager properties + hosted-service wiring", "status": "completed", "blockedBy": [] }, - { "id": "R2-08-T9", "subject": "Async-ify ServeNode + Processed/AtTime arms (ct threading, OCE=>BadTimeout, behavior-neutral)", "status": "pending", "blockedBy": ["R2-08-T7", "R2-08-T8"] }, + { "id": "R2-08-T9", "subject": "Async-ify ServeNode + Processed/AtTime arms (ct threading, OCE=>BadTimeout, behavior-neutral)", "status": "completed", "blockedBy": ["R2-08-T7", "R2-08-T8"] }, { "id": "R2-08-T10", "subject": "Async-ify ServeRawPaged (page read + tie-cluster over-fetch) + extract ServeEventsAsync", "status": "pending", "blockedBy": ["R2-08-T9"] }, { "id": "R2-08-T11", "subject": "RunBounded fan-out in all four HistoryRead arms, single bridge per arm (turns T7 green) + upper-bound + isolation tests", "status": "pending", "blockedBy": ["R2-08-T9", "R2-08-T10"] }, { "id": "R2-08-T12", "subject": "Per-request HistoryRead deadline CTS — hung backend surfaces BadTimeout per node", "status": "pending", "blockedBy": ["R2-08-T11"] }, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index ef90e691..85499470 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -1861,13 +1861,14 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // and the single-shot backend returns every bucket in one read, so there is no "full page ⇒ // maybe more" signal to page on. Returning the complete aggregate result with a null CP is // spec-conformant (OPC UA Part 11 lets a server return all available data in one response). - ServeNode(handle, results, errors, (source, tagname) => source.ReadProcessedAsync( + // Interim: still bridged sequentially per node (bounded fan-out lands in a later slice). + ServeNodeAsync(handle, results, errors, (source, tagname, token) => source.ReadProcessedAsync( tagname, details.StartTime, details.EndTime, interval, aggregate.Value, - CancellationToken.None)); + token), CancellationToken.None).GetAwaiter().GetResult(); } } @@ -1886,10 +1887,11 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 var timestamps = details.ReqTimes?.ToList() ?? new List(); foreach (var handle in nodesToProcess) { - ServeNode(handle, results, errors, (source, tagname) => source.ReadAtTimeAsync( + // Interim: still bridged sequentially per node (bounded fan-out lands in a later slice). + ServeNodeAsync(handle, results, errors, (source, tagname, token) => source.ReadAtTimeAsync( tagname, timestamps, - CancellationToken.None)); + token), CancellationToken.None).GetAwaiter().GetResult(); } } @@ -2061,11 +2063,12 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// The service-level errors list to fill at handle.Index. /// Invokes the resolved data-source read with the resolved tagname; only called /// once the tagname is confirmed present. - private void ServeNode( + private async Task ServeNodeAsync( NodeHandle handle, IList results, IList errors, - Func> read) + Func> read, + CancellationToken ct) { var idString = handle.NodeId.Identifier?.ToString(); if (idString is null || !TryGetHistorizedTagname(idString, out var tagname)) @@ -2078,9 +2081,9 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 try { - // HistoryRead is NOT invoked under the node-manager Lock (unlike OnWriteValue), so blocking - // on the async source here is safe and won't freeze the address space. - var sourceResult = read(HistorianDataSource, tagname!).GetAwaiter().GetResult(); + // HistoryRead is NOT invoked under the node-manager Lock (unlike OnWriteValue), so awaiting + // the async source here is safe and won't freeze the address space. + var sourceResult = await read(HistorianDataSource, tagname!, ct).ConfigureAwait(false); var historyData = ToHistoryData(sourceResult); results[handle.Index] = new SdkHistoryReadResult @@ -2095,13 +2098,20 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 }; errors[handle.Index] = ServiceResult.Good; } + catch (OperationCanceledException) + { + // The per-request deadline elapsed while this node was in flight — surface BadTimeout for + // THIS node only (a spec-conformant history-read status), ahead of the generic catch. + errors[handle.Index] = StatusCodes.BadTimeout; + results[handle.Index] = new SdkHistoryReadResult { StatusCode = StatusCodes.BadTimeout }; + } catch (Exception ex) { - // One node's backend failure (throw / timeout / cancellation) must not throw out of the - // batch — surface a Bad status for THIS node only. This CustomNodeManager2 carries no - // ILogger (see ReportConditionEvent), so log through the SDK's static trace rather than - // swallowing silently. Utils.LogError is [Obsolete] in 1.5.378 (favours an ITelemetryContext - // this manager doesn't wire) — suppress the deprecation, matching the existing pattern. + // One node's backend failure (throw / timeout) must not throw out of the batch — surface a + // Bad status for THIS node only. This CustomNodeManager2 carries no ILogger (see + // ReportConditionEvent), so log through the SDK's static trace rather than swallowing silently. + // Utils.LogError is [Obsolete] in 1.5.378 (favours an ITelemetryContext this manager doesn't + // wire) — suppress the deprecation, matching the existing pattern. #pragma warning disable CS0618 // Type or member is obsolete Utils.LogError(ex, "OtOpcUaNodeManager: HistoryRead failed for node {0}", handle.NodeId); #pragma warning restore CS0618 From cef7d9b28198e2f00453864b482ce3788a8a8567 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 12:07:00 -0400 Subject: [PATCH 10/14] refactor(opcuaserver): async-ify ServeRawPaged + HistoryReadEvents internals (03/S3) --- ...-ldap-historyread-async-plan.md.tasks.json | 2 +- .../OtOpcUaNodeManager.cs | 127 ++++++++++++------ 2 files changed, 86 insertions(+), 43 deletions(-) diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json index 4b4da57f..a538f66f 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json @@ -10,7 +10,7 @@ { "id": "R2-08-T7", "subject": "S3 sequential-serving repro test (RED): MaxObservedConcurrency >= 2 on an 8-node Raw batch — fails on current code", "status": "completed", "blockedBy": [] }, { "id": "R2-08-T8", "subject": "ServerHistorianOptions: HistoryReadBatchParallelism/MaxConcurrentHistoryReadBatches/HistoryReadDeadline + Validate warnings + node-manager properties + hosted-service wiring", "status": "completed", "blockedBy": [] }, { "id": "R2-08-T9", "subject": "Async-ify ServeNode + Processed/AtTime arms (ct threading, OCE=>BadTimeout, behavior-neutral)", "status": "completed", "blockedBy": ["R2-08-T7", "R2-08-T8"] }, - { "id": "R2-08-T10", "subject": "Async-ify ServeRawPaged (page read + tie-cluster over-fetch) + extract ServeEventsAsync", "status": "pending", "blockedBy": ["R2-08-T9"] }, + { "id": "R2-08-T10", "subject": "Async-ify ServeRawPaged (page read + tie-cluster over-fetch) + extract ServeEventsAsync", "status": "completed", "blockedBy": ["R2-08-T9"] }, { "id": "R2-08-T11", "subject": "RunBounded fan-out in all four HistoryRead arms, single bridge per arm (turns T7 green) + upper-bound + isolation tests", "status": "pending", "blockedBy": ["R2-08-T9", "R2-08-T10"] }, { "id": "R2-08-T12", "subject": "Per-request HistoryRead deadline CTS — hung backend surfaces BadTimeout per node", "status": "pending", "blockedBy": ["R2-08-T11"] }, { "id": "R2-08-T13", "subject": "Process-wide concurrent-batch limiter — saturation degrades to BadTooManyOperations (lazy gate, bounded wait)", "status": "pending", "blockedBy": ["R2-08-T12"] }, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 85499470..e6e8b0d4 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -1825,9 +1825,11 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 continue; } - ServeRawPaged( + // Interim: still bridged sequentially per node (bounded fan-out lands in a later slice). + ServeRawPagedAsync( handle, session, nodesToRead, results, errors, - details.StartTime, details.EndTime, details.NumValuesPerNode); + details.StartTime, details.EndTime, details.NumValuesPerNode, + CancellationToken.None).GetAwaiter().GetResult(); } } @@ -1922,41 +1924,75 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 continue; } - try - { - // NOT under the node-manager Lock — block-bridging the async source is safe here. - var sourceResult = _historianDataSource.ReadEventsAsync( - sourceName, - details.StartTime, - details.EndTime, - // NumValuesPerNode==0 means "no limit — return ALL values" per OPC UA - // Part 4/11, so translate it to UNBOUNDED (int.MaxValue) here. Passing the int<=0 - // backend-default-cap sentinel instead would silently truncate a "give me everything" - // events read at the backend default. A positive cap passes through (clamped). - EventMaxEvents(details.NumValuesPerNode), - CancellationToken.None).GetAwaiter().GetResult(); + // Interim: still bridged sequentially per node (bounded fan-out lands in a later slice). + ServeEventsAsync(handle, sourceName, details, selectClauses, results, errors, CancellationToken.None) + .GetAwaiter().GetResult(); + } + } - var historyEvent = ProjectEvents(sourceResult.Events, selectClauses); - results[handle.Index] = new SdkHistoryReadResult - { - // No events ⇒ GoodNoData (the notifier is historized, the window just held no events). - StatusCode = sourceResult.Events.Count == 0 ? StatusCodes.GoodNoData : StatusCodes.Good, - HistoryData = new ExtensionObject(historyEvent), - // We never issue continuation points — every read returns the full window in one shot. - ContinuationPoint = null, - }; - errors[handle.Index] = ServiceResult.Good; - } - catch (Exception ex) + /// + /// Serve one registered event-history notifier handle: reads the source's events over the request + /// window, projects them onto the select clauses, and fills the service-level results/errors slots. + /// Per-node error isolation matches — a backend throw becomes a Bad + /// status for THIS node only and never throws out of the batch; a deadline cancellation surfaces + /// BadTimeout. + /// + /// The notifier node handle; handle.Index indexes results/errors. + /// The registered historian event-source name for this notifier. + /// The event-read details (window + count cap). + /// The event-filter select clauses (the fields to emit, in order). + /// The service-level results list to fill at handle.Index. + /// The service-level errors list to fill at handle.Index. + /// Per-request deadline token. + private async Task ServeEventsAsync( + NodeHandle handle, + string? sourceName, + ReadEventDetails details, + SimpleAttributeOperandCollection selectClauses, + IList results, + IList errors, + CancellationToken ct) + { + try + { + // NOT under the node-manager Lock — awaiting the async source is safe here. + var sourceResult = await _historianDataSource.ReadEventsAsync( + sourceName, + details.StartTime, + details.EndTime, + // NumValuesPerNode==0 means "no limit — return ALL values" per OPC UA + // Part 4/11, so translate it to UNBOUNDED (int.MaxValue) here. Passing the int<=0 + // backend-default-cap sentinel instead would silently truncate a "give me everything" + // events read at the backend default. A positive cap passes through (clamped). + EventMaxEvents(details.NumValuesPerNode), + ct).ConfigureAwait(false); + + var historyEvent = ProjectEvents(sourceResult.Events, selectClauses); + results[handle.Index] = new SdkHistoryReadResult { - // One node's backend failure must not throw out of the batch — surface Bad for THIS node - // only. This manager carries no ILogger, so log via the SDK's static trace (see ServeNode). + // No events ⇒ GoodNoData (the notifier is historized, the window just held no events). + StatusCode = sourceResult.Events.Count == 0 ? StatusCodes.GoodNoData : StatusCodes.Good, + HistoryData = new ExtensionObject(historyEvent), + // We never issue continuation points — every read returns the full window in one shot. + ContinuationPoint = null, + }; + errors[handle.Index] = ServiceResult.Good; + } + catch (OperationCanceledException) + { + // The per-request deadline elapsed while reading this notifier — BadTimeout for THIS node only. + errors[handle.Index] = StatusCodes.BadTimeout; + results[handle.Index] = new SdkHistoryReadResult { StatusCode = StatusCodes.BadTimeout }; + } + catch (Exception ex) + { + // One node's backend failure must not throw out of the batch — surface Bad for THIS node + // only. This manager carries no ILogger, so log via the SDK's static trace (see ServeNodeAsync). #pragma warning disable CS0618 // Type or member is obsolete - Utils.LogError(ex, "OtOpcUaNodeManager: HistoryReadEvents failed for node {0}", handle.NodeId); + Utils.LogError(ex, "OtOpcUaNodeManager: HistoryReadEvents failed for node {0}", handle.NodeId); #pragma warning restore CS0618 - errors[handle.Index] = StatusCodes.BadHistoryOperationUnsupported; - results[handle.Index] = new SdkHistoryReadResult { StatusCode = StatusCodes.BadHistoryOperationUnsupported }; - } + errors[handle.Index] = StatusCodes.BadHistoryOperationUnsupported; + results[handle.Index] = new SdkHistoryReadResult { StatusCode = StatusCodes.BadHistoryOperationUnsupported }; } } @@ -2149,7 +2185,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// The request window's (inclusive) lower bound, used for a fresh read. /// The (inclusive) upper bound of the read window; unchanged across pages. /// The client's per-page cap; 0 means "all values, no paging". - private void ServeRawPaged( + private async Task ServeRawPagedAsync( NodeHandle handle, ISession? session, IList nodesToRead, @@ -2157,7 +2193,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 IList errors, DateTime startTimeUtc, DateTime endUtc, - uint numValuesPerNode) + uint numValuesPerNode, + CancellationToken ct) { var inboundCp = nodesToRead[handle.Index].ContinuationPoint; @@ -2199,10 +2236,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 startUtc = startTimeUtc; } - // HistoryRead is NOT under the node-manager Lock — block-bridging the async source is safe. - var sourceResult = HistorianDataSource - .ReadRawAsync(tagname, startUtc, endUtc, numValuesPerNode, CancellationToken.None) - .GetAwaiter().GetResult(); + // HistoryRead is NOT under the node-manager Lock — awaiting the async source is safe. + var sourceResult = await HistorianDataSource + .ReadRawAsync(tagname, startUtc, endUtc, numValuesPerNode, ct) + .ConfigureAwait(false); var backendFull = HistoryPaging.IsFullPage(sourceResult.Samples.Count, numValuesPerNode); @@ -2232,9 +2269,9 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // Compute in uint so +1 can never wrap: Math.Max(1,…) also guards against a zero/negative // config value slipping through (Validate() warns but does not block startup). var overfetchCap = (uint)Math.Max(1, MaxTieClusterOverfetch) + 1u; - var cluster = HistorianDataSource - .ReadRawAsync(tagname, startUtc, startUtc, overfetchCap, CancellationToken.None) - .GetAwaiter().GetResult().Samples; + var cluster = (await HistorianDataSource + .ReadRawAsync(tagname, startUtc, startUtc, overfetchCap, ct) + .ConfigureAwait(false)).Samples; // Absurd burst: more ties than we're willing to buffer in memory. Preserve today's loud-fail // for that node rather than over-fetch an unbounded cluster; the operator's remedy is a @@ -2306,6 +2343,12 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 }; errors[handle.Index] = ServiceResult.Good; } + catch (OperationCanceledException) + { + // The per-request deadline elapsed while paging this node — BadTimeout for THIS node only. + errors[handle.Index] = StatusCodes.BadTimeout; + results[handle.Index] = new SdkHistoryReadResult { StatusCode = StatusCodes.BadTimeout }; + } catch (Exception ex) { // One node's backend failure must not throw out of the batch — Bad for THIS node only. From 8c365da754d23e0120a17e5a3373614ec64c871a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 12:09:53 -0400 Subject: [PATCH 11/14] feat(opcuaserver): bounded per-node parallel HistoryRead within a batch (03/S3) --- ...-ldap-historyread-async-plan.md.tasks.json | 2 +- .../OtOpcUaNodeManager.cs | 81 ++++++++++--- .../NodeManagerHistoryReadConcurrencyTests.cs | 106 ++++++++++++++++++ 3 files changed, 175 insertions(+), 14 deletions(-) diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json index a538f66f..4eb04faf 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json @@ -11,7 +11,7 @@ { "id": "R2-08-T8", "subject": "ServerHistorianOptions: HistoryReadBatchParallelism/MaxConcurrentHistoryReadBatches/HistoryReadDeadline + Validate warnings + node-manager properties + hosted-service wiring", "status": "completed", "blockedBy": [] }, { "id": "R2-08-T9", "subject": "Async-ify ServeNode + Processed/AtTime arms (ct threading, OCE=>BadTimeout, behavior-neutral)", "status": "completed", "blockedBy": ["R2-08-T7", "R2-08-T8"] }, { "id": "R2-08-T10", "subject": "Async-ify ServeRawPaged (page read + tie-cluster over-fetch) + extract ServeEventsAsync", "status": "completed", "blockedBy": ["R2-08-T9"] }, - { "id": "R2-08-T11", "subject": "RunBounded fan-out in all four HistoryRead arms, single bridge per arm (turns T7 green) + upper-bound + isolation tests", "status": "pending", "blockedBy": ["R2-08-T9", "R2-08-T10"] }, + { "id": "R2-08-T11", "subject": "RunBounded fan-out in all four HistoryRead arms, single bridge per arm (turns T7 green) + upper-bound + isolation tests", "status": "completed", "blockedBy": ["R2-08-T9", "R2-08-T10"] }, { "id": "R2-08-T12", "subject": "Per-request HistoryRead deadline CTS — hung backend surfaces BadTimeout per node", "status": "pending", "blockedBy": ["R2-08-T11"] }, { "id": "R2-08-T13", "subject": "Process-wide concurrent-batch limiter — saturation degrades to BadTooManyOperations (lazy gate, bounded wait)", "status": "pending", "blockedBy": ["R2-08-T12"] }, { "id": "R2-08-T14", "subject": "Docs (Historian.md + CLAUDE.md ServerHistorian table + security.md) + full touched-suite and whole-solution regression sweep + STATUS.md entry", "status": "pending", "blockedBy": ["R2-08-T5", "R2-08-T6", "R2-08-T13"] } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index e6e8b0d4..7539af9c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -1816,6 +1816,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 IDictionary cache) { var session = context.OperationContext?.Session; + var work = new List>(nodesToProcess.Count); foreach (var handle in nodesToProcess) { if (details.IsReadModified) @@ -1825,12 +1826,13 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 continue; } - // Interim: still bridged sequentially per node (bounded fan-out lands in a later slice). - ServeRawPagedAsync( + work.Add(() => ServeRawPagedAsync( handle, session, nodesToRead, results, errors, details.StartTime, details.EndTime, details.NumValuesPerNode, - CancellationToken.None).GetAwaiter().GetResult(); + CancellationToken.None)); } + + RunBounded(work, HistoryReadBatchParallelism); } /// @@ -1846,6 +1848,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 { // OPC UA ProcessingInterval is a Duration in milliseconds — convert once per batch. var interval = TimeSpan.FromMilliseconds(details.ProcessingInterval); + var work = new List>(nodesToProcess.Count); foreach (var handle in nodesToProcess) { // AggregateType is a per-node parallel collection (same length as nodesToRead, enforced by @@ -1863,15 +1866,17 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // and the single-shot backend returns every bucket in one read, so there is no "full page ⇒ // maybe more" signal to page on. Returning the complete aggregate result with a null CP is // spec-conformant (OPC UA Part 11 lets a server return all available data in one response). - // Interim: still bridged sequentially per node (bounded fan-out lands in a later slice). - ServeNodeAsync(handle, results, errors, (source, tagname, token) => source.ReadProcessedAsync( + var agg = aggregate.Value; + work.Add(() => ServeNodeAsync(handle, results, errors, (source, tagname, token) => source.ReadProcessedAsync( tagname, details.StartTime, details.EndTime, interval, - aggregate.Value, - token), CancellationToken.None).GetAwaiter().GetResult(); + agg, + token), CancellationToken.None)); } + + RunBounded(work, HistoryReadBatchParallelism); } /// @@ -1887,14 +1892,16 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 { // Snapshot the requested timestamps once — the same list is read for every node. var timestamps = details.ReqTimes?.ToList() ?? new List(); + var work = new List>(nodesToProcess.Count); foreach (var handle in nodesToProcess) { - // Interim: still bridged sequentially per node (bounded fan-out lands in a later slice). - ServeNodeAsync(handle, results, errors, (source, tagname, token) => source.ReadAtTimeAsync( + work.Add(() => ServeNodeAsync(handle, results, errors, (source, tagname, token) => source.ReadAtTimeAsync( tagname, timestamps, - token), CancellationToken.None).GetAwaiter().GetResult(); + token), CancellationToken.None)); } + + RunBounded(work, HistoryReadBatchParallelism); } /// @@ -1911,6 +1918,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // Snapshot the select clauses once — the same filter projects every node's events. var selectClauses = details.Filter?.SelectClauses ?? new SimpleAttributeOperandCollection(); + var work = new List>(nodesToProcess.Count); foreach (var handle in nodesToProcess) { var idString = handle.NodeId.Identifier?.ToString(); @@ -1924,10 +1932,11 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 continue; } - // Interim: still bridged sequentially per node (bounded fan-out lands in a later slice). - ServeEventsAsync(handle, sourceName, details, selectClauses, results, errors, CancellationToken.None) - .GetAwaiter().GetResult(); + var sn = sourceName; + work.Add(() => ServeEventsAsync(handle, sn, details, selectClauses, results, errors, CancellationToken.None)); } + + RunBounded(work, HistoryReadBatchParallelism); } /// @@ -2099,6 +2108,52 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// The service-level errors list to fill at handle.Index. /// Invokes the resolved data-source read with the resolved tagname; only called /// once the tagname is confirmed present. + /// + /// Run a batch of per-node HistoryRead work items with BOUNDED parallelism (arch-review 03/S3), + /// block-bridged ONCE at the arm boundary. Contract: every work item must be fully + /// self-contained — it fills its own results/errors slot and NEVER throws (all per-node handlers + /// catch every exception, including cancellation, and surface a Bad status for that node only). That + /// invariant is what makes the single GetAwaiter().GetResult() safe: + /// can never fault, so it cannot throw out of the synchronous SDK override. A degree ≤ 1 (or a + /// single item) runs sequentially — no semaphore, no extra tasks. + /// + /// The per-node work items (index-disjoint writes into the shared results/errors). + /// Max concurrent work items (). + private static void RunBounded(IReadOnlyList> work, int degree) + { + if (work.Count == 0) return; + if (degree <= 1 || work.Count == 1) + { + foreach (var item in work) + item().GetAwaiter().GetResult(); + return; + } + + using var gate = new SemaphoreSlim(degree); + var tasks = new Task[work.Count]; + for (var i = 0; i < work.Count; i++) + tasks[i] = RunGatedAsync(gate, work[i]); + + // Safe: every body is self-contained + never throws, so WhenAll never faults (see contract above). + Task.WhenAll(tasks).GetAwaiter().GetResult(); + } + + /// Awaits one work item under a semaphore permit, releasing it when the item completes. + /// The concurrency-limiting semaphore. + /// The per-node work item. + private static async Task RunGatedAsync(SemaphoreSlim gate, Func work) + { + await gate.WaitAsync().ConfigureAwait(false); + try + { + await work().ConfigureAwait(false); + } + finally + { + gate.Release(); + } + } + private async Task ServeNodeAsync( NodeHandle handle, IList results, diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadConcurrencyTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadConcurrencyTests.cs index 37599d8a..78bccd9a 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadConcurrencyTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadConcurrencyTests.cs @@ -61,6 +61,85 @@ public sealed class NodeManagerHistoryReadConcurrencyTests : IDisposable await host.DisposeAsync(); } + /// + /// Parallelism is bounded ABOVE by HistoryReadBatchParallelism: with the property set to 3 and + /// 12 nodes in flight, no more than 3 reads are ever concurrent (03/S3). + /// + [Fact] + public async Task Parallelism_is_bounded_above() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + nm.HistoryReadBatchParallelism = 3; + var fake = new ConcurrencyTrackingHistorianDataSource(TimeSpan.FromMilliseconds(100)); + nm.HistorianDataSource = fake; + + var nodeIds = MaterializeHistorizedNodes(nm, count: 12); + var details = new ReadRawModifiedDetails + { + StartTime = DateTime.UtcNow.AddHours(-1), + EndTime = DateTime.UtcNow, + NumValuesPerNode = 10, + IsReadModified = false, + }; + + await Task.Run(() => InvokeHistoryRead(server, nm, details, nodeIds), Ct) + .WaitAsync(TimeSpan.FromSeconds(15), Ct); + + fake.MaxObservedConcurrency.ShouldBeGreaterThanOrEqualTo(2); + fake.MaxObservedConcurrency.ShouldBeLessThanOrEqualTo(3); + + await host.DisposeAsync(); + } + + /// + /// One node's backend throw does not poison a parallel batch: that node surfaces + /// BadHistoryOperationUnsupported while the other seven return Good (per-node isolation holds + /// under parallelism — 03/S3). + /// + [Fact] + public async Task One_node_failure_does_not_poison_a_parallel_batch() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + + var src = DateTime.UtcNow.AddSeconds(-5); + var srv = DateTime.UtcNow; + // Node index 3's tagname throws; all others return a single good sample. + var fake = new PerTagnameFake(tagname => tagname == "WW.Tag3" + ? throw new InvalidOperationException("backend boom for Tag3") + : new HistorianRead(new[] { new DataValueSnapshot(1.0f, StatusCodes.Good, src, srv) }, null)); + nm.HistorianDataSource = fake; + + var nodeIds = MaterializeHistorizedNodes(nm, count: 8); + var details = new ReadRawModifiedDetails + { + StartTime = DateTime.UtcNow.AddHours(-1), + EndTime = DateTime.UtcNow, + NumValuesPerNode = 10, + IsReadModified = false, + }; + + var (results, errors) = await Task.Run(() => InvokeHistoryRead(server, nm, details, nodeIds), Ct) + .WaitAsync(TimeSpan.FromSeconds(15), Ct); + + for (var i = 0; i < 8; i++) + { + if (i == 3) + { + StatusCode.IsBad(errors[i].StatusCode).ShouldBeTrue(); + errors[i].StatusCode.Code.ShouldBe(StatusCodes.BadHistoryOperationUnsupported); + } + else + { + errors[i].StatusCode.Code.ShouldBe(StatusCodes.Good); + results[i].StatusCode.Code.ShouldBe(StatusCodes.Good); + } + } + + await host.DisposeAsync(); + } + /// Materialises historized Float variables and returns their NodeIds. private static NodeId[] MaterializeHistorizedNodes(OtOpcUaNodeManager nm, int count) { @@ -161,6 +240,33 @@ public sealed class NodeManagerHistoryReadConcurrencyTests : IDisposable } } + /// A per-tagname fake that delegates each raw read to a caller-supplied func (which may throw + /// to simulate a per-node backend failure). Processed/AtTime/Events delegate to the null source. + private sealed class PerTagnameFake(Func rawHandler) : IHistorianDataSource + { + public Task ReadRawAsync( + string fullReference, DateTime startUtc, DateTime endUtc, uint maxValuesPerNode, + CancellationToken cancellationToken) => Task.FromResult(rawHandler(fullReference)); + + public Task ReadProcessedAsync( + string fullReference, DateTime startUtc, DateTime endUtc, TimeSpan interval, + HistoryAggregateType aggregate, CancellationToken cancellationToken) => + NullHistorianDataSource.Instance.ReadProcessedAsync(fullReference, startUtc, endUtc, interval, aggregate, cancellationToken); + + public Task ReadAtTimeAsync( + string fullReference, IReadOnlyList timestampsUtc, CancellationToken cancellationToken) => + NullHistorianDataSource.Instance.ReadAtTimeAsync(fullReference, timestampsUtc, cancellationToken); + + public Task ReadEventsAsync( + string? sourceName, DateTime startUtc, DateTime endUtc, int maxEvents, + CancellationToken cancellationToken) => + NullHistorianDataSource.Instance.ReadEventsAsync(sourceName, startUtc, endUtc, maxEvents, cancellationToken); + + public HistorianHealthSnapshot GetHealthSnapshot() => NullHistorianDataSource.Instance.GetHealthSnapshot(); + + public void Dispose() { } + } + private async Task<(OpcUaApplicationHost Host, OtOpcUaSdkServer Server)> BootAsync() { var host = new OpcUaApplicationHost( From 888cf86655a60a19e12ce483c738a3b846237fb1 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 12:13:38 -0400 Subject: [PATCH 12/14] =?UTF-8?q?feat(opcuaserver):=20per-request=20Histor?= =?UTF-8?q?yRead=20deadline=20=E2=80=94=20hung=20backends=20surface=20BadT?= =?UTF-8?q?imeout=20(03/S3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...-ldap-historyread-async-plan.md.tasks.json | 2 +- .../OtOpcUaNodeManager.cs | 27 ++++++-- .../NodeManagerHistoryReadConcurrencyTests.cs | 66 +++++++++++++++++++ 3 files changed, 90 insertions(+), 5 deletions(-) diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json index 4eb04faf..2e262a98 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json @@ -12,7 +12,7 @@ { "id": "R2-08-T9", "subject": "Async-ify ServeNode + Processed/AtTime arms (ct threading, OCE=>BadTimeout, behavior-neutral)", "status": "completed", "blockedBy": ["R2-08-T7", "R2-08-T8"] }, { "id": "R2-08-T10", "subject": "Async-ify ServeRawPaged (page read + tie-cluster over-fetch) + extract ServeEventsAsync", "status": "completed", "blockedBy": ["R2-08-T9"] }, { "id": "R2-08-T11", "subject": "RunBounded fan-out in all four HistoryRead arms, single bridge per arm (turns T7 green) + upper-bound + isolation tests", "status": "completed", "blockedBy": ["R2-08-T9", "R2-08-T10"] }, - { "id": "R2-08-T12", "subject": "Per-request HistoryRead deadline CTS — hung backend surfaces BadTimeout per node", "status": "pending", "blockedBy": ["R2-08-T11"] }, + { "id": "R2-08-T12", "subject": "Per-request HistoryRead deadline CTS — hung backend surfaces BadTimeout per node", "status": "completed", "blockedBy": ["R2-08-T11"] }, { "id": "R2-08-T13", "subject": "Process-wide concurrent-batch limiter — saturation degrades to BadTooManyOperations (lazy gate, bounded wait)", "status": "pending", "blockedBy": ["R2-08-T12"] }, { "id": "R2-08-T14", "subject": "Docs (Historian.md + CLAUDE.md ServerHistorian table + security.md) + full touched-suite and whole-solution regression sweep + STATUS.md entry", "status": "pending", "blockedBy": ["R2-08-T5", "R2-08-T6", "R2-08-T13"] } ], diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 7539af9c..e059d535 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -1816,6 +1816,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 IDictionary cache) { var session = context.OperationContext?.Session; + using var deadline = CreateDeadline(); + var ct = deadline?.Token ?? CancellationToken.None; var work = new List>(nodesToProcess.Count); foreach (var handle in nodesToProcess) { @@ -1829,7 +1831,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 work.Add(() => ServeRawPagedAsync( handle, session, nodesToRead, results, errors, details.StartTime, details.EndTime, details.NumValuesPerNode, - CancellationToken.None)); + ct)); } RunBounded(work, HistoryReadBatchParallelism); @@ -1848,6 +1850,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 { // OPC UA ProcessingInterval is a Duration in milliseconds — convert once per batch. var interval = TimeSpan.FromMilliseconds(details.ProcessingInterval); + using var deadline = CreateDeadline(); + var ct = deadline?.Token ?? CancellationToken.None; var work = new List>(nodesToProcess.Count); foreach (var handle in nodesToProcess) { @@ -1873,7 +1877,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 details.EndTime, interval, agg, - token), CancellationToken.None)); + token), ct)); } RunBounded(work, HistoryReadBatchParallelism); @@ -1892,13 +1896,15 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 { // Snapshot the requested timestamps once — the same list is read for every node. var timestamps = details.ReqTimes?.ToList() ?? new List(); + using var deadline = CreateDeadline(); + var ct = deadline?.Token ?? CancellationToken.None; var work = new List>(nodesToProcess.Count); foreach (var handle in nodesToProcess) { work.Add(() => ServeNodeAsync(handle, results, errors, (source, tagname, token) => source.ReadAtTimeAsync( tagname, timestamps, - token), CancellationToken.None)); + token), ct)); } RunBounded(work, HistoryReadBatchParallelism); @@ -1918,6 +1924,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // Snapshot the select clauses once — the same filter projects every node's events. var selectClauses = details.Filter?.SelectClauses ?? new SimpleAttributeOperandCollection(); + using var deadline = CreateDeadline(); + var ct = deadline?.Token ?? CancellationToken.None; var work = new List>(nodesToProcess.Count); foreach (var handle in nodesToProcess) { @@ -1933,7 +1941,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 } var sn = sourceName; - work.Add(() => ServeEventsAsync(handle, sn, details, selectClauses, results, errors, CancellationToken.None)); + work.Add(() => ServeEventsAsync(handle, sn, details, selectClauses, results, errors, ct)); } RunBounded(work, HistoryReadBatchParallelism); @@ -2138,6 +2146,17 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 Task.WhenAll(tasks).GetAwaiter().GetResult(); } + /// + /// Create the per-request deadline source for a HistoryRead arm (arch-review 03/S3): a + /// that fires after , so a + /// single request can never hold an SDK request thread longer than the deadline regardless of node + /// count. Returns null when the deadline is non-positive (unbounded — the arm then passes + /// ), matching the ServerHistorianOptions warning. + /// + /// The deadline source, or null for an unbounded (non-positive) deadline. + private CancellationTokenSource? CreateDeadline() => + HistoryReadDeadline > TimeSpan.Zero ? new CancellationTokenSource(HistoryReadDeadline) : null; + /// Awaits one work item under a semaphore permit, releasing it when the item completes. /// The concurrency-limiting semaphore. /// The per-node work item. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadConcurrencyTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadConcurrencyTests.cs index 78bccd9a..f65c45f8 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadConcurrencyTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadConcurrencyTests.cs @@ -140,6 +140,37 @@ public sealed class NodeManagerHistoryReadConcurrencyTests : IDisposable await host.DisposeAsync(); } + /// + /// A per-request deadline bounds a hung backend: reads that only complete on token cancellation + /// surface BadTimeout for every node and the call returns within the deadline (plus slack) — + /// it does not hang an SDK request thread indefinitely (03/S3). + /// + [Fact] + public async Task Deadline_bounds_a_hung_backend() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + nm.HistoryReadDeadline = TimeSpan.FromMilliseconds(500); + nm.HistorianDataSource = new HangingHistorianDataSource(); + + var nodeIds = MaterializeHistorizedNodes(nm, count: 4); + var details = new ReadRawModifiedDetails + { + StartTime = DateTime.UtcNow.AddHours(-1), + EndTime = DateTime.UtcNow, + NumValuesPerNode = 10, + IsReadModified = false, + }; + + var (_, errors) = await Task.Run(() => InvokeHistoryRead(server, nm, details, nodeIds), Ct) + .WaitAsync(TimeSpan.FromSeconds(10), Ct); + + for (var i = 0; i < 4; i++) + errors[i].StatusCode.Code.ShouldBe(StatusCodes.BadTimeout); + + await host.DisposeAsync(); + } + /// Materialises historized Float variables and returns their NodeIds. private static NodeId[] MaterializeHistorizedNodes(OtOpcUaNodeManager nm, int count) { @@ -240,6 +271,41 @@ public sealed class NodeManagerHistoryReadConcurrencyTests : IDisposable } } + /// A fake whose reads complete ONLY on token cancellation — used to prove the per-request + /// deadline surfaces BadTimeout rather than hanging. + private sealed class HangingHistorianDataSource : IHistorianDataSource + { + private static async Task HangAsync(CancellationToken ct) + { + await Task.Delay(Timeout.Infinite, ct).ConfigureAwait(false); + return new HistorianRead(Array.Empty(), null); + } + + public Task ReadRawAsync( + string fullReference, DateTime startUtc, DateTime endUtc, uint maxValuesPerNode, + CancellationToken cancellationToken) => HangAsync(cancellationToken); + + public Task ReadProcessedAsync( + string fullReference, DateTime startUtc, DateTime endUtc, TimeSpan interval, + HistoryAggregateType aggregate, CancellationToken cancellationToken) => HangAsync(cancellationToken); + + public Task ReadAtTimeAsync( + string fullReference, IReadOnlyList timestampsUtc, CancellationToken cancellationToken) => + HangAsync(cancellationToken); + + public async Task ReadEventsAsync( + string? sourceName, DateTime startUtc, DateTime endUtc, int maxEvents, + CancellationToken cancellationToken) + { + await Task.Delay(Timeout.Infinite, cancellationToken).ConfigureAwait(false); + return new HistoricalEventsResult(Array.Empty(), null); + } + + public HistorianHealthSnapshot GetHealthSnapshot() => NullHistorianDataSource.Instance.GetHealthSnapshot(); + + public void Dispose() { } + } + /// A per-tagname fake that delegates each raw read to a caller-supplied func (which may throw /// to simulate a per-node backend failure). Processed/AtTime/Events delegate to the null source. private sealed class PerTagnameFake(Func rawHandler) : IHistorianDataSource From 3cebfae41761aaf753c6793edd599960bd3039b6 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 12:18:35 -0400 Subject: [PATCH 13/14] =?UTF-8?q?feat(opcuaserver):=20process-wide=20Histo?= =?UTF-8?q?ryRead=20batch=20limiter=20=E2=80=94=20flood=20degrades=20to=20?= =?UTF-8?q?BadTooManyOperations,=20not=20pool=20exhaustion=20(03/S3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...-ldap-historyread-async-plan.md.tasks.json | 2 +- .../OtOpcUaNodeManager.cs | 216 ++++++++++++------ .../NodeManagerHistoryReadConcurrencyTests.cs | 94 ++++++++ 3 files changed, 242 insertions(+), 70 deletions(-) diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json index 2e262a98..21955c78 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json @@ -13,7 +13,7 @@ { "id": "R2-08-T10", "subject": "Async-ify ServeRawPaged (page read + tie-cluster over-fetch) + extract ServeEventsAsync", "status": "completed", "blockedBy": ["R2-08-T9"] }, { "id": "R2-08-T11", "subject": "RunBounded fan-out in all four HistoryRead arms, single bridge per arm (turns T7 green) + upper-bound + isolation tests", "status": "completed", "blockedBy": ["R2-08-T9", "R2-08-T10"] }, { "id": "R2-08-T12", "subject": "Per-request HistoryRead deadline CTS — hung backend surfaces BadTimeout per node", "status": "completed", "blockedBy": ["R2-08-T11"] }, - { "id": "R2-08-T13", "subject": "Process-wide concurrent-batch limiter — saturation degrades to BadTooManyOperations (lazy gate, bounded wait)", "status": "pending", "blockedBy": ["R2-08-T12"] }, + { "id": "R2-08-T13", "subject": "Process-wide concurrent-batch limiter — saturation degrades to BadTooManyOperations (lazy gate, bounded wait)", "status": "completed", "blockedBy": ["R2-08-T12"] }, { "id": "R2-08-T14", "subject": "Docs (Historian.md + CLAUDE.md ServerHistorian table + security.md) + full touched-suite and whole-solution regression sweep + STATUS.md entry", "status": "pending", "blockedBy": ["R2-08-T5", "R2-08-T6", "R2-08-T13"] } ], "lastUpdated": "2026-07-12" diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index e059d535..964c2c76 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -221,6 +221,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// public TimeSpan HistoryReadDeadline { get; set; } = TimeSpan.FromSeconds(60); + /// Lazily-created process-wide HistoryRead-batch limiter (see ). + private volatile SemaphoreSlim? _historyReadBatchLimiter; + private readonly object _historyReadBatchLimiterLock = new(); + private volatile IHistoryContinuationStore _historyContinuationStore = new SessionHistoryContinuationStore(); /// @@ -1816,25 +1820,28 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 IDictionary cache) { var session = context.OperationContext?.Session; - using var deadline = CreateDeadline(); - var ct = deadline?.Token ?? CancellationToken.None; - var work = new List>(nodesToProcess.Count); - foreach (var handle in nodesToProcess) + WithHistoryReadBatchLimiter(nodesToProcess, results, errors, () => { - if (details.IsReadModified) + using var deadline = CreateDeadline(); + var ct = deadline?.Token ?? CancellationToken.None; + var work = new List>(nodesToProcess.Count); + foreach (var handle in nodesToProcess) { - // We never serve modified-value history; mark this node unsupported and move on. - errors[handle.Index] = StatusCodes.BadHistoryOperationUnsupported; - continue; + if (details.IsReadModified) + { + // We never serve modified-value history; mark this node unsupported and move on. + errors[handle.Index] = StatusCodes.BadHistoryOperationUnsupported; + continue; + } + + work.Add(() => ServeRawPagedAsync( + handle, session, nodesToRead, results, errors, + details.StartTime, details.EndTime, details.NumValuesPerNode, + ct)); } - work.Add(() => ServeRawPagedAsync( - handle, session, nodesToRead, results, errors, - details.StartTime, details.EndTime, details.NumValuesPerNode, - ct)); - } - - RunBounded(work, HistoryReadBatchParallelism); + RunBounded(work, HistoryReadBatchParallelism); + }); } /// @@ -1850,37 +1857,40 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 { // OPC UA ProcessingInterval is a Duration in milliseconds — convert once per batch. var interval = TimeSpan.FromMilliseconds(details.ProcessingInterval); - using var deadline = CreateDeadline(); - var ct = deadline?.Token ?? CancellationToken.None; - var work = new List>(nodesToProcess.Count); - foreach (var handle in nodesToProcess) + WithHistoryReadBatchLimiter(nodesToProcess, results, errors, () => { - // AggregateType is a per-node parallel collection (same length as nodesToRead, enforced by - // the base dispatcher). handle.Index is the node's position in that collection. - var aggregateNodeId = details.AggregateType[handle.Index]; - var aggregate = MapAggregate(aggregateNodeId); - if (aggregate is null) + using var deadline = CreateDeadline(); + var ct = deadline?.Token ?? CancellationToken.None; + var work = new List>(nodesToProcess.Count); + foreach (var handle in nodesToProcess) { - errors[handle.Index] = StatusCodes.BadAggregateNotSupported; - continue; + // AggregateType is a per-node parallel collection (same length as nodesToRead, enforced by + // the base dispatcher). handle.Index is the node's position in that collection. + var aggregateNodeId = details.AggregateType[handle.Index]; + var aggregate = MapAggregate(aggregateNodeId); + if (aggregate is null) + { + errors[handle.Index] = StatusCodes.BadAggregateNotSupported; + continue; + } + + // Processed is SINGLE-SHOT (no continuation point). Unlike Raw, ReadProcessedDetails carries + // NO client count cap (NumValuesPerNode) — the bucket count is deterministic (window / interval) + // and the single-shot backend returns every bucket in one read, so there is no "full page ⇒ + // maybe more" signal to page on. Returning the complete aggregate result with a null CP is + // spec-conformant (OPC UA Part 11 lets a server return all available data in one response). + var agg = aggregate.Value; + work.Add(() => ServeNodeAsync(handle, results, errors, (source, tagname, token) => source.ReadProcessedAsync( + tagname, + details.StartTime, + details.EndTime, + interval, + agg, + token), ct)); } - // Processed is SINGLE-SHOT (no continuation point). Unlike Raw, ReadProcessedDetails carries - // NO client count cap (NumValuesPerNode) — the bucket count is deterministic (window / interval) - // and the single-shot backend returns every bucket in one read, so there is no "full page ⇒ - // maybe more" signal to page on. Returning the complete aggregate result with a null CP is - // spec-conformant (OPC UA Part 11 lets a server return all available data in one response). - var agg = aggregate.Value; - work.Add(() => ServeNodeAsync(handle, results, errors, (source, tagname, token) => source.ReadProcessedAsync( - tagname, - details.StartTime, - details.EndTime, - interval, - agg, - token), ct)); - } - - RunBounded(work, HistoryReadBatchParallelism); + RunBounded(work, HistoryReadBatchParallelism); + }); } /// @@ -1896,18 +1906,21 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 { // Snapshot the requested timestamps once — the same list is read for every node. var timestamps = details.ReqTimes?.ToList() ?? new List(); - using var deadline = CreateDeadline(); - var ct = deadline?.Token ?? CancellationToken.None; - var work = new List>(nodesToProcess.Count); - foreach (var handle in nodesToProcess) + WithHistoryReadBatchLimiter(nodesToProcess, results, errors, () => { - work.Add(() => ServeNodeAsync(handle, results, errors, (source, tagname, token) => source.ReadAtTimeAsync( - tagname, - timestamps, - token), ct)); - } + using var deadline = CreateDeadline(); + var ct = deadline?.Token ?? CancellationToken.None; + var work = new List>(nodesToProcess.Count); + foreach (var handle in nodesToProcess) + { + work.Add(() => ServeNodeAsync(handle, results, errors, (source, tagname, token) => source.ReadAtTimeAsync( + tagname, + timestamps, + token), ct)); + } - RunBounded(work, HistoryReadBatchParallelism); + RunBounded(work, HistoryReadBatchParallelism); + }); } /// @@ -1924,27 +1937,30 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // Snapshot the select clauses once — the same filter projects every node's events. var selectClauses = details.Filter?.SelectClauses ?? new SimpleAttributeOperandCollection(); - using var deadline = CreateDeadline(); - var ct = deadline?.Token ?? CancellationToken.None; - var work = new List>(nodesToProcess.Count); - foreach (var handle in nodesToProcess) + WithHistoryReadBatchLimiter(nodesToProcess, results, errors, () => { - var idString = handle.NodeId.Identifier?.ToString(); - if (idString is null || !_eventNotifierSources.TryGetValue(idString, out var sourceName)) + using var deadline = CreateDeadline(); + var ct = deadline?.Token ?? CancellationToken.None; + var work = new List>(nodesToProcess.Count); + foreach (var handle in nodesToProcess) { - // Not a registered event-history source (plain folder / Null-source promotion) ⇒ unsupported. - // Set both errors and results explicitly on every bad path — don't rely on the SDK base - // pre-seeding results[i], so every path is self-contained and the contract is obvious. - errors[handle.Index] = StatusCodes.BadHistoryOperationUnsupported; - results[handle.Index] = new SdkHistoryReadResult { StatusCode = StatusCodes.BadHistoryOperationUnsupported }; - continue; + var idString = handle.NodeId.Identifier?.ToString(); + if (idString is null || !_eventNotifierSources.TryGetValue(idString, out var sourceName)) + { + // Not a registered event-history source (plain folder / Null-source promotion) ⇒ unsupported. + // Set both errors and results explicitly on every bad path — don't rely on the SDK base + // pre-seeding results[i], so every path is self-contained and the contract is obvious. + errors[handle.Index] = StatusCodes.BadHistoryOperationUnsupported; + results[handle.Index] = new SdkHistoryReadResult { StatusCode = StatusCodes.BadHistoryOperationUnsupported }; + continue; + } + + var sn = sourceName; + work.Add(() => ServeEventsAsync(handle, sn, details, selectClauses, results, errors, ct)); } - var sn = sourceName; - work.Add(() => ServeEventsAsync(handle, sn, details, selectClauses, results, errors, ct)); - } - - RunBounded(work, HistoryReadBatchParallelism); + RunBounded(work, HistoryReadBatchParallelism); + }); } /// @@ -2146,6 +2162,68 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 Task.WhenAll(tasks).GetAwaiter().GetResult(); } + /// + /// Run one HistoryRead arm under the process-wide concurrent-batch limiter (arch-review 03/S3). Each + /// arm waits up to min(HistoryReadDeadline, 5 s) for a slot; on expiry every handle in the + /// batch gets BadTooManyOperations (fast-fail under flood, brief waits under mild contention) + /// and is skipped. On acquisition the arm body runs and the slot is + /// released in a finally. Combined with this caps + /// total in-flight gateway reads at + /// MaxConcurrentHistoryReadBatches × HistoryReadBatchParallelism. + /// + /// The batch's handles — every slot is failed on limiter saturation. + /// The service-level results list. + /// The service-level errors list. + /// The arm body to run once a slot is acquired. + private void WithHistoryReadBatchLimiter( + List nodesToProcess, + IList results, + IList errors, + Action serve) + { + var gate = GetHistoryReadBatchLimiter(); + + // Bounded admission wait: cap the block at 5 s, and never exceed the per-request deadline. + var waitMs = HistoryReadDeadline > TimeSpan.Zero + ? Math.Min(HistoryReadDeadline.TotalMilliseconds, 5000.0) + : 5000.0; + + if (!gate.Wait(TimeSpan.FromMilliseconds(waitMs))) + { + foreach (var handle in nodesToProcess) + { + errors[handle.Index] = StatusCodes.BadTooManyOperations; + results[handle.Index] = new SdkHistoryReadResult { StatusCode = StatusCodes.BadTooManyOperations }; + } + return; + } + + try + { + serve(); + } + finally + { + gate.Release(); + } + } + + /// + /// Lazily create (double-checked) the process-wide HistoryRead-batch limiter, sized from + /// at first use so the Host's post-boot property set is + /// respected (the same benign wiring race as ). + /// + /// The shared batch limiter semaphore. + private SemaphoreSlim GetHistoryReadBatchLimiter() + { + var gate = _historyReadBatchLimiter; + if (gate is not null) return gate; + lock (_historyReadBatchLimiterLock) + { + return _historyReadBatchLimiter ??= new SemaphoreSlim(Math.Max(1, MaxConcurrentHistoryReadBatches)); + } + } + /// /// Create the per-request deadline source for a HistoryRead arm (arch-review 03/S3): a /// that fires after , so a diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadConcurrencyTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadConcurrencyTests.cs index f65c45f8..f8e97331 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadConcurrencyTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerHistoryReadConcurrencyTests.cs @@ -171,6 +171,55 @@ public sealed class NodeManagerHistoryReadConcurrencyTests : IDisposable await host.DisposeAsync(); } + /// + /// A saturated process-wide batch limiter degrades a second concurrent HistoryRead to + /// BadTooManyOperations (rather than exhausting the SDK request-thread pool); once the parked + /// batch completes the slot frees and a later read succeeds (03/S3). + /// + [Fact] + public async Task Saturated_limiter_rejects_with_BadTooManyOperations() + { + var (host, server) = await BootAsync(); + var nm = server.NodeManager!; + nm.MaxConcurrentHistoryReadBatches = 1; + nm.HistoryReadDeadline = TimeSpan.FromMilliseconds(200); + var fake = new GatedRawFake(); + nm.HistorianDataSource = fake; + + var ids = MaterializeHistorizedNodes(nm, count: 2); + var details = new ReadRawModifiedDetails + { + StartTime = DateTime.UtcNow.AddHours(-1), + EndTime = DateTime.UtcNow, + NumValuesPerNode = 10, + IsReadModified = false, + }; + + // First batch parks in the gated fake, holding the single limiter permit. + var first = Task.Run(() => InvokeHistoryRead(server, nm, details, ids[0]), Ct); + + var spin = Stopwatch.StartNew(); + while (fake.Entered == 0 && spin.Elapsed < TimeSpan.FromSeconds(5)) + await Task.Delay(10, Ct); + fake.Entered.ShouldBe(1); + + // Second concurrent batch: the limiter is saturated → all handles BadTooManyOperations. + var (_, errors2) = await Task.Run(() => InvokeHistoryRead(server, nm, details, ids[1]), Ct) + .WaitAsync(TimeSpan.FromSeconds(5), Ct); + errors2[0].StatusCode.Code.ShouldBe(StatusCodes.BadTooManyOperations); + fake.Entered.ShouldBe(1, "the rejected batch must never reach the backend"); + + // Release the parked batch; the slot frees and a later read succeeds. + fake.Release(); + await first.WaitAsync(TimeSpan.FromSeconds(10), Ct); + + var (_, errors3) = await Task.Run(() => InvokeHistoryRead(server, nm, details, ids[1]), Ct) + .WaitAsync(TimeSpan.FromSeconds(10), Ct); + errors3[0].StatusCode.Code.ShouldBe(StatusCodes.Good); + + await host.DisposeAsync(); + } + /// Materialises historized Float variables and returns their NodeIds. private static NodeId[] MaterializeHistorizedNodes(OtOpcUaNodeManager nm, int count) { @@ -306,6 +355,51 @@ public sealed class NodeManagerHistoryReadConcurrencyTests : IDisposable public void Dispose() { } } + /// A fake whose raw read parks (awaits a ) until the test + /// releases it — used to hold the process-wide batch limiter permit while a second batch is issued. + private sealed class GatedRawFake : IHistorianDataSource + { + private readonly TaskCompletionSource _gate = new(TaskCreationOptions.RunContinuationsAsynchronously); + private int _entered; + + /// Number of raw reads that reached the backend body. + public int Entered => Volatile.Read(ref _entered); + + /// Releases the parked read so its awaiting batch can complete. + public void Release() => _gate.TrySetResult(); + + public async Task ReadRawAsync( + string fullReference, DateTime startUtc, DateTime endUtc, uint maxValuesPerNode, + CancellationToken cancellationToken) + { + Interlocked.Increment(ref _entered); + // Deliberately ignore the per-request deadline token: the first batch must keep HOLDING the + // single limiter permit until the test releases it, so the second batch deterministically hits + // a saturated limiter (rather than the first batch's own deadline freeing the slot early). + await _gate.Task.ConfigureAwait(false); + return new HistorianRead( + new[] { new DataValueSnapshot(1.0f, StatusCodes.Good, DateTime.UtcNow, DateTime.UtcNow) }, null); + } + + public Task ReadProcessedAsync( + string fullReference, DateTime startUtc, DateTime endUtc, TimeSpan interval, + HistoryAggregateType aggregate, CancellationToken cancellationToken) => + NullHistorianDataSource.Instance.ReadProcessedAsync(fullReference, startUtc, endUtc, interval, aggregate, cancellationToken); + + public Task ReadAtTimeAsync( + string fullReference, IReadOnlyList timestampsUtc, CancellationToken cancellationToken) => + NullHistorianDataSource.Instance.ReadAtTimeAsync(fullReference, timestampsUtc, cancellationToken); + + public Task ReadEventsAsync( + string? sourceName, DateTime startUtc, DateTime endUtc, int maxEvents, + CancellationToken cancellationToken) => + NullHistorianDataSource.Instance.ReadEventsAsync(sourceName, startUtc, endUtc, maxEvents, cancellationToken); + + public HistorianHealthSnapshot GetHealthSnapshot() => NullHistorianDataSource.Instance.GetHealthSnapshot(); + + public void Dispose() { } + } + /// A per-tagname fake that delegates each raw read to a caller-supplied func (which may throw /// to simulate a per-node backend failure). Processed/AtTime/Events delegate to the null source. private sealed class PerTagnameFake(Func rawHandler) : IHistorianDataSource From 354d74d370c02a3b1221ee285b557470138761a8 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 12:21:33 -0400 Subject: [PATCH 14/14] docs: ServerHistorian HistoryRead knobs + Security:Ldap resilience keys; R2-08 status (03/S2, 03/S3) --- CLAUDE.md | 3 +++ .../R2-08-ldap-historyread-async-plan.md | 24 +++++++++++++++++++ ...-ldap-historyread-async-plan.md.tasks.json | 2 +- archreview/plans/STATUS.md | 1 + docs/Historian.md | 8 ++++++- 5 files changed, 36 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index eb60060c..1aec5946 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -265,6 +265,9 @@ authorization uses the standard `AccessLevels.HistoryRead` bit set at materializ | `CaCertificatePath` | `null` | PEM CA file pinning the gateway TLS chain; null/empty uses the OS trust store | | `CallTimeout` | `00:00:30` | Per-call deadline for each unary gateway read | | `MaxTieClusterOverfetch` | `65536` | Bounded over-fetch the HistoryRead-Raw paging uses to page within an oversized same-timestamp tie cluster (retained from the prior backend) | +| `HistoryReadBatchParallelism` | `4` | Max nodes served concurrently within one HistoryRead batch (03/S3); `⌈N/P⌉ × CallTimeout` worst case instead of sequential | +| `MaxConcurrentHistoryReadBatches` | `16` | Process-wide concurrent HistoryRead batch cap (03/S3); flood degrades to `BadTooManyOperations`. Total in-flight reads capped at 16 × 4 = 64 | +| `HistoryReadDeadline` | `00:01:00` | Per-request HistoryRead wall-clock deadline (03/S3); a node still in flight surfaces `BadTimeout`. Non-positive = unbounded | ### Alarm-history path (`AlarmHistorian` section) diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md b/archreview/plans/R2-08-ldap-historyread-async-plan.md index 9c1b477a..84dfd255 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md @@ -674,3 +674,27 @@ Commit: `docs: ServerHistorian HistoryRead knobs + Security:Ldap resilience keys the usual test-run + review cadence. Risk concentrated in T11 (parallel fan-out — mitigated by the thread-safety audit + the paging regression suite) and T4/T5 (auth-path semantics — mitigated by the fail-closed default posture and behavior-neutral defaults). + +## Execution deviations (R2-08) + +- **T1 RED shape.** Written against the *current* 3-arg ctor with a `Stopwatch` timing assertion (the + plan's explicitly-allowed alternative), not the intended-ctor compile-fail — so every commit compiles. + T4 then re-pointed it at the new `Options.Create(...)` ctor to turn it green. +- **Async parking fakes.** The concurrency-cap repro (`GatedFakeLdap`) and the limiter repro + (`GatedRawFake`) had to park **asynchronously** (await a `TaskCompletionSource`), not block a thread + synchronously — a synchronous block before the first `await` deadlocks the caller (the core task never + yields its `Task`). Documented inline in both fakes. +- **T6 InternalsVisibleTo.** Driving the real `internal static OpcUaApplicationHost.HandleImpersonation` + from `Host.IntegrationTests` required adding an `InternalsVisibleTo` for that test assembly to the + OpcUaServer csproj (the plan called for the internal drive but not the grant). Additive + reversible. +- **T13 fake ignores the deadline token.** `GatedRawFake.ReadRawAsync` deliberately does **not** observe + the per-request deadline `ct`: with a small `HistoryReadDeadline`, the parked first batch would + otherwise cancel its own read and free the limiter permit before the second batch checks (race). By + parking on the release gate only, the first batch holds the permit deterministically, so the second + batch reliably hits the saturated limiter → `BadTooManyOperations`. +- **T14 sweep scope.** Per the executor's memory constraint (heavy `*.IntegrationTests` / Playwright + suites leak ~16 GB and the LDAP integration harness spins an ephemeral openldap on :3894), the + whole-solution `dotnet test` was **not** run. Instead: a full-solution `dotnet build` (clean) + the + impacted **filtered** unit suites (Host.IntegrationTests LDAP-classes only, Security.Tests, + Runtime.Tests options, OpcUaServer.Tests HistoryRead). The whole-solution + live legs are deferred to a + serial heavy pass / VPN run. diff --git a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json index 21955c78..0c434070 100644 --- a/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json +++ b/archreview/plans/R2-08-ldap-historyread-async-plan.md.tasks.json @@ -14,7 +14,7 @@ { "id": "R2-08-T11", "subject": "RunBounded fan-out in all four HistoryRead arms, single bridge per arm (turns T7 green) + upper-bound + isolation tests", "status": "completed", "blockedBy": ["R2-08-T9", "R2-08-T10"] }, { "id": "R2-08-T12", "subject": "Per-request HistoryRead deadline CTS — hung backend surfaces BadTimeout per node", "status": "completed", "blockedBy": ["R2-08-T11"] }, { "id": "R2-08-T13", "subject": "Process-wide concurrent-batch limiter — saturation degrades to BadTooManyOperations (lazy gate, bounded wait)", "status": "completed", "blockedBy": ["R2-08-T12"] }, - { "id": "R2-08-T14", "subject": "Docs (Historian.md + CLAUDE.md ServerHistorian table + security.md) + full touched-suite and whole-solution regression sweep + STATUS.md entry", "status": "pending", "blockedBy": ["R2-08-T5", "R2-08-T6", "R2-08-T13"] } + { "id": "R2-08-T14", "subject": "Docs (Historian.md + CLAUDE.md ServerHistorian table + security.md) + full touched-suite and whole-solution regression sweep + STATUS.md entry", "status": "completed", "blockedBy": ["R2-08-T5", "R2-08-T6", "R2-08-T13"] } ], "lastUpdated": "2026-07-12" } diff --git a/archreview/plans/STATUS.md b/archreview/plans/STATUS.md index 5d12857e..45e9f4cf 100644 --- a/archreview/plans/STATUS.md +++ b/archreview/plans/STATUS.md @@ -176,6 +176,7 @@ what the unit leg cannot; (c) migrate the 3 xunit v2 holdouts once Akka ships an | **R2-02** | 01/S-6 (High, rank 2), 01/U-6, 01/S-8=03/S12 (rank 4), 04/C-7, 01/S-7=03/S13 | `r2/02-resilience-config-hardening` (off `1676c8f4`) | `4be13429`…`dd82e740` (17 task commits) | **Code-complete, offline-verified.** **S-6:** parser clamps timeout/retry/breaker overrides (timeout≤0→tier default, breaker==1→2, caps) with diagnostics + belt-and-braces `Build` clamp so pipeline construction can never throw Polly `ValidationException` (new `ResiliencePolicyRanges` in Core.Abstractions); brick-repro + `Build_NeverThrows_ForHostileDirectOptions` + factory-seam wiring proof; AdminUI `Validate()` range warnings. **S-8:** parser forces Write/AlarmAck retry→0 (spec invariant) **and** dispatch flips to `isIdempotent:false` (RED-first on the Runtime wiring test); non-idempotent arm caches the no-retry snapshot once/invoker (01/P-4) + tracker in-flight accounting; `WriteIdempotentAttribute` false claim corrected; retry cells disabled. **U-6:** bulkhead knob deleted end-to-end (migration-free; stored keys ignored) + `Options_properties_are_exactly_the_pipeline_wired_set` inertness guard. **C-7:** `ResilienceFormModel` JsonObject-bag round-trip preserves unknown keys; malformed JSON → `ParseFailed` lock + verbatim `ToJson` + discard button. **S-7:** per-`Create` `Interlocked` options-generation in the pipeline-cache key kills the respawn stale-pipeline race. Core resilience 121/121, AdminUI form 10/10, Analyzers 32/32, Runtime 364/364; 0 production OTOPCUA0001. **Deferred (T15/T18):** docker-dev `:9200` `/run` (warning banner, input lockout, unknown-key survival) + whole-solution sweep. | | **R2-10** | 03/U10=04/U-5 (Medium/Low, action #10) | `r2/10-resilience-observability` (off `46fedda3`) | `205d3980`…`` (11 task commits) | **Code-complete, offline-verified.** The resilience status tracker now has a production reader, mirroring the driver-health flow. Core truthfulness: `RecordBreakerClose` + derived `IsBreakerOpen` (breaker-open state derivable), `OnClosed` records the close, and `RecordSuccess` wired onto the invoker success paths (`ConsecutiveFailures` is a true consecutive counter, not stuck-forever). Transport: `DriverResilienceStatusChanged` Commons contract → periodic 5 s full-snapshot publisher `DriverResilienceStatusPublisherService` (driver nodes, registered in `AddOtOpcUaDriverFactories` with wiring guard `ResilienceStatusReaderRegistrationTests` — closes the built-but-never-wired class) → `driver-resilience-status` DPS topic → per-admin-node `DriverResilienceStatusBridge` → `IDriverResilienceStatusStore` → resilience chips (breaker-OPEN / consecutive-failures / in-flight / staleness) on `DriverStatusPanel.razor`, read in-process (self-HubConnection ban honored). Core.Tests resilience 31/31 (incl. deterministic breaker-close via a controllable TimeProvider), Host.IntegrationTests publisher+guard 6/6, AdminUI.Tests store 5/5; full-solution build clean. **Deferred (T10):** docker-dev live-`/run` — S7 dead-endpoint breaker-open observed on BOTH centrals + recovery-clear + staleness-dim (also the first live sighting of the #10 `circuit-breaker OPENED` log line, closing FOLLOWUP-10's behavioral gate). | +| **R2-08** | 03/S2 (High, action #8), 03/S3 (High) | `r2/08-ldap-historyread-async` (off `1a698cbb`) | `ce6d9e21`…`` (14 task commits) | **Code-complete, offline-verified.** **S2 (LDAP async/timeout):** the SDK raises the impersonation callback synchronously under a `SessionManager`-wide `m_eventLock` (verified vs. 1.5.378 — a hung bind serializes ALL activations), so the fix is entirely at the `LdapOpcUaUserAuthenticator` boundary: five new `Security:Ldap` keys (`ConnectionTimeoutMs`=10000 projected into the shared lib, `AuthTimeoutMs`=15000 hard whole-flow `WaitAsync` bound → fail-closed "timed out", `MaxConcurrentAuthentications`=8 `SemaphoreSlim` with release-on-core-completion so orphaned binds still count, `OutageFailureThreshold`=3/`OutageCooldownSeconds`=15 directory-outage circuit → fast-deny "unavailable", half-open probe) + `LdapAuthResult.IsSystemFailure` seam (mapped from `ServiceAccountBindFailed`) distinguishing outage from credential-deny + `LdapOptionsValidator` rules + `HandleImpersonation` contract doc + `docs/security.md`. Stall-repro RED→GREEN; offline outage-sim through the real `HandleImpersonation` against an unroutable host. **S3 (HistoryRead fan-out):** all four HistoryRead arms async-ified (`ServeNodeAsync`/`ServeRawPagedAsync`/`ServeEventsAsync`, OCE→`BadTimeout`) then bounded per-node fan-out (`RunBounded` semaphore + single arm-boundary bridge) + per-request deadline CTS (`HistoryReadDeadline`=60s) + process-wide lazy batch limiter (`MaxConcurrentHistoryReadBatches`=16 → flood degrades to `BadTooManyOperations`); 16×4=64 in-flight cap; three new `ServerHistorian` keys + node-manager props + hosted-service wiring + `docs/Historian.md`/CLAUDE.md tables. Sequential-serving RED→GREEN. Host.IntegrationTests (LDAP resilience) 13/13, Security.Tests 11/11, Runtime.Tests options 11/11, OpcUaServer.Tests HistoryRead 30/30. **Deferred (live):** S2 docker-dev unreachable-LDAP + GLAuth outage/recovery cycle; S3 `Category=LiveIntegration` gateway soak. | | **R2-11** | 01/C-1, 01/P-1 + 05/CONV-2, UNDER-1, UNDER-6 | `r2/11-tagconfig-consolidation` (off `46fedda3`) | first `2e14fe1f` … (24 task commits) | **Code-complete, offline-verified.** **Part 1 (C-1/P-1):** single `TagConfigIntent.Parse` + `DeviceConfigIntent` in Commons replace the four byte-parity TagConfig parse copies (composer/artifact/walker/DraftValidator) — composer now parses once per tag (P-1); a permanent `Runtime.Tests/TagConfigCorpusParityTests` (30-blob corpus) guards compose≡decode; `grep "MUST parse identically" src/` = 0. **Part 2 (CONV-2):** shared strict-capable `TagConfigJson` readers in Core.Abstractions replace the six copy-pasted `ReadEnum`s; all six driver parsers gain `Inspect()` warnings + honour the `writable` key (FOCAS **forced read-only**, 05/UNDER-1); FOCAS equipment tags run the capability-matrix pre-flight (05/UNDER-6); Modbus probe parses the factory DTO shape (`timeoutMs` no longer dropped). New `EquipmentTagConfigInspector` in ControlPlane + `AdminOperationsActor` deploy-gate wiring with `Deployment:TagConfigValidationMode` (**Warn default | Error opt-in**), proven through the actor (F10b consuming test). AdminUI: `writable` checkbox in the six typed editors. **Phase-C follow-up (deferred, recorded here):** flip the runtime parsers to strict (typo'd enum ⇒ `BadNodeIdUnknown` instead of wrong-width `Good`) once fleets have run `Error` mode clean — behavior change deliberately not shipped in this plan. **Deferred (T22/T24):** docker-dev `/run` of one typed editor's Writable checkbox + whole-solution serial sweep. | **Report anchor drifts to fold into the next report refresh** (from the R2-02 verification pass): (1) diff --git a/docs/Historian.md b/docs/Historian.md index 97a8abfd..21a88b84 100644 --- a/docs/Historian.md +++ b/docs/Historian.md @@ -69,7 +69,10 @@ and all HistoryRead calls on historized nodes return `GoodNoData` (empty, not an "AllowUntrustedServerCertificate": false, "CaCertificatePath": null, "CallTimeout": "00:00:30", - "MaxTieClusterOverfetch": 65536 + "MaxTieClusterOverfetch": 65536, + "HistoryReadBatchParallelism": 4, + "MaxConcurrentHistoryReadBatches": 16, + "HistoryReadDeadline": "00:01:00" } } ``` @@ -84,6 +87,9 @@ and all HistoryRead calls on historized nodes return `GoodNoData` (empty, not an | `CaCertificatePath` | string\|null | `null` | PEM CA file pinning the gateway's TLS chain. Null/empty uses the OS trust store. | | `CallTimeout` | TimeSpan | `00:00:30` | Per-call deadline applied to each unary gateway read. A read for a tag the historian does not yet know about — e.g. a freshly-historized tag that has **not** been provisioned (see [Tag auto-provisioning](#tag-auto-provisioning-ensuretags)) — can run to this **full** deadline before the gateway errors, so an unprovisioned-tag HistoryRead can block for ~30 s. Lower this to fail faster, at the cost of truncating legitimately long reads. | | `MaxTieClusterOverfetch` | int | `65536` | Maximum samples the server will fetch in one shot to page through a tie cluster (multiple samples sharing one `SourceTimestamp`). A cluster larger than this ceiling fails `BadHistoryOperationUnsupported`. Raise to handle abnormally large tie clusters; the default covers all normal-data cases. | +| `HistoryReadBatchParallelism` | int | `4` | Max historized nodes served **concurrently** within one HistoryRead batch (arch-review 03/S3). The SDK's `HistoryRead*` override surface is synchronous, so each arm bridges once at its boundary; this turns per-node serving from sequential (`N × CallTimeout` worst case) into `⌈N / P⌉ × CallTimeout`. Also caps the gateway-load multiplication a single client can command. `≤ 1` serves sequentially. | +| `MaxConcurrentHistoryReadBatches` | int | `16` | Process-wide limit on HistoryRead batches served **concurrently** (arch-review 03/S3). A flood of history clients degrades to `BadTooManyOperations` rather than exhausting the SDK request-thread pool. With `HistoryReadBatchParallelism` this caps total in-flight gateway reads at `MaxConcurrentHistoryReadBatches × HistoryReadBatchParallelism` (default 16 × 4 = 64). A batch that cannot get a slot within `min(HistoryReadDeadline, 5 s)` fails every handle `BadTooManyOperations`. | +| `HistoryReadDeadline` | TimeSpan | `00:01:00` | Per-request wall-clock deadline for a HistoryRead batch (arch-review 03/S3). A node still in flight at the deadline surfaces `BadTimeout`, so one request can never hold an SDK request thread longer than this regardless of node count. Non-positive = unbounded (a `Validate()` warning). | > **Do not commit `ApiKey` to `appsettings.json`.** Set it via the environment variable > `ServerHistorian__ApiKey`, a secrets store, or a deployment-time overlay. The checked-in default is