dohertj2/lmxopcua
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(r2-02): range-clamp ResilienceConfig overrides in the parser (01/S-6) — timeout/retry/breaker clamped with diagnostics, never brick

Browse Source
This commit is contained in:
Joseph Doherty
2026-07-13 09:53:28 -04:00
parent 4be13429b6
commit 294d74c1de
3 changed files with 184 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json
+1 -1
View File
@@ -3,7 +3,7 @@
"tasks": [
{ "id": 0, "subject": "S-6 brick-repro test (RED): parsed timeoutSeconds:0 / breakerFailureThreshold:1 must not brick capability calls — ResilienceConfigBrickTests, expect 2 failures on f6eaa267", "status": "pending", "blockedBy": [] },
{ "id": 1, "subject": "Add ResiliencePolicyRanges constants to Core.Abstractions (single source of truth for parser/builder/AdminUI, derived from Polly.Core 8.6.6 validation)", "status": "completed", "blockedBy": [] },
{ "id": 2, "subject": "S-6 parser: clamp timeout/retry/breaker overrides with appending diagnostics (timeout<=0 -> tier default; breaker==1 -> 2; caps) + 8 parser tests", "status": "pending", "blockedBy": [0, 1] },
{ "id": 2, "subject": "S-6 parser: clamp timeout/retry/breaker overrides with appending diagnostics (timeout<=0 -> tier default; breaker==1 -> 2; caps) + 8 parser tests", "status": "completed", "blockedBy": [0, 1] },
{ "id": 3, "subject": "S-6 builder: belt-and-braces clamp in Build so pipeline construction can never throw ValidationException + Build_NeverThrows_ForHostileDirectOptions (turns task 0 green)", "status": "pending", "blockedBy": [0, 1] },
{ "id": 4, "subject": "S-6 production-wiring guard: DriverCapabilityInvokerFactory.Create with hostile JSON yields a working invoker + logged clamp diagnostic; re-run ResilienceInvokerFactoryRegistrationTests", "status": "pending", "blockedBy": [2, 3] },
{ "id": 5, "subject": "S-6 AdminUI: ResilienceFormModel.Validate() range messages (via ResiliencePolicyRanges through the transitive Core.Abstractions ref) + warning block in DriverResilienceSection", "status": "pending", "blockedBy": [1] },
src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceOptionsParser.cs
+71 -4
View File
@@ -83,15 +83,16 @@ public static class DriverResilienceOptionsParser
{
if (!Enum.TryParse<DriverCapability>(capName, ignoreCase: true, out var capability))
{
parseDiagnostic ??= $"Unknown capability '{capName}' in ResilienceConfig; skipped.";
AppendDiagnostic(ref parseDiagnostic, $"Unknown capability '{capName}' in ResilienceConfig; skipped.");
continue;
}
var basePolicy = merged[capability];
merged[capability] = new CapabilityPolicy(
var mergedPolicy = new CapabilityPolicy(
TimeoutSeconds: overridePolicy.TimeoutSeconds ?? basePolicy.TimeoutSeconds,
RetryCount: overridePolicy.RetryCount ?? basePolicy.RetryCount,
BreakerFailureThreshold: overridePolicy.BreakerFailureThreshold ?? basePolicy.BreakerFailureThreshold);
merged[capability] = ClampPolicy(capability, mergedPolicy, basePolicy, ref parseDiagnostic);
}
}
@@ -102,9 +103,9 @@ public static class DriverResilienceOptionsParser
if (shape.RecycleIntervalSeconds is int secs)
{
if (secs <= 0)
parseDiagnostic ??= $"RecycleIntervalSeconds must be positive; got {secs} — ignoring.";
AppendDiagnostic(ref parseDiagnostic, $"RecycleIntervalSeconds must be positive; got {secs} — ignoring.");
else if (tier != DriverTier.C)
parseDiagnostic ??= $"RecycleIntervalSeconds is Tier C only; Tier {tier} driver will not scheduled-recycle.";
AppendDiagnostic(ref parseDiagnostic, $"RecycleIntervalSeconds is Tier C only; Tier {tier} driver will not scheduled-recycle.");
else
recycleIntervalSeconds = secs;
}
@@ -119,6 +120,72 @@ public static class DriverResilienceOptionsParser
};
}
/// <summary>
/// Clamp a merged per-capability override to the legal <see cref="ResiliencePolicyRanges"/> so a
/// downstream Polly pipeline build can never throw <c>ValidationException</c> (01/S-6). Each clamp
/// appends a diagnostic naming the field + what happened. A non-positive timeout snaps to the
/// capability's <paramref name="tierDefault"/> (a nonsense value, not merely an extreme — snapping
/// to 1 s could be a 30× surprise on a Discover policy); everything else clamps to the nearest legal
/// value. Preserves maximum operator intent while never bricking.
/// </summary>
private static CapabilityPolicy ClampPolicy(
DriverCapability capability,
CapabilityPolicy merged,
CapabilityPolicy tierDefault,
ref string? diagnostic)
{
var timeout = merged.TimeoutSeconds;
if (timeout <= 0)
{
AppendDiagnostic(ref diagnostic,
$"{capability}.timeoutSeconds {timeout} is non-positive; using the tier default ({tierDefault.TimeoutSeconds}s).");
timeout = tierDefault.TimeoutSeconds;
}
else if (timeout > ResiliencePolicyRanges.MaxTimeoutSeconds)
{
AppendDiagnostic(ref diagnostic,
$"{capability}.timeoutSeconds {timeout} exceeds the max ({ResiliencePolicyRanges.MaxTimeoutSeconds}s); clamped.");
timeout = ResiliencePolicyRanges.MaxTimeoutSeconds;
}
var retry = merged.RetryCount;
if (retry < ResiliencePolicyRanges.MinRetryCount)
{
AppendDiagnostic(ref diagnostic, $"{capability}.retryCount {retry} is negative; clamped to {ResiliencePolicyRanges.MinRetryCount}.");
retry = ResiliencePolicyRanges.MinRetryCount;
}
else if (retry > ResiliencePolicyRanges.MaxRetryCount)
{
AppendDiagnostic(ref diagnostic, $"{capability}.retryCount {retry} exceeds the cap ({ResiliencePolicyRanges.MaxRetryCount}); clamped.");
retry = ResiliencePolicyRanges.MaxRetryCount;
}
var breaker = merged.BreakerFailureThreshold;
if (breaker < ResiliencePolicyRanges.MinBreakerFailureThreshold)
{
AppendDiagnostic(ref diagnostic, $"{capability}.breakerFailureThreshold {breaker} is negative; breaker disabled.");
breaker = ResiliencePolicyRanges.MinBreakerFailureThreshold;
}
else if (breaker == 1)
{
AppendDiagnostic(ref diagnostic,
$"{capability}.breakerFailureThreshold 1 is below Polly's minimum throughput; raised to {ResiliencePolicyRanges.MinEnabledBreakerFailureThreshold} (closest to open-on-first-failure intent).");
breaker = ResiliencePolicyRanges.MinEnabledBreakerFailureThreshold;
}
else if (breaker > ResiliencePolicyRanges.MaxBreakerFailureThreshold)
{
AppendDiagnostic(ref diagnostic,
$"{capability}.breakerFailureThreshold {breaker} exceeds the cap ({ResiliencePolicyRanges.MaxBreakerFailureThreshold}); clamped.");
breaker = ResiliencePolicyRanges.MaxBreakerFailureThreshold;
}
return merged with { TimeoutSeconds = timeout, RetryCount = retry, BreakerFailureThreshold = breaker };
}
/// <summary>Append a diagnostic to the running channel (joined with "; ") so multiple clamps all surface.</summary>
private static void AppendDiagnostic(ref string? diagnostic, string message)
=> diagnostic = diagnostic is null ? message : $"{diagnostic}; {message}";
private sealed class ResilienceConfigShape
{
/// <summary>Gets or sets the maximum concurrent bulkhead requests.</summary>
tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResilienceOptionsParserTests.cs
+112
View File
@@ -222,4 +222,116 @@ public sealed class DriverResilienceOptionsParserTests
options.RecycleIntervalSeconds.ShouldBeNull();
diag.ShouldContain("must be positive");
}
// ---- 01/S-6: range clamps (clamp + diagnostic, never brick) ----
/// <summary>A non-positive timeout is nonsense; it snaps to the capability's tier default with a diagnostic.</summary>
[Fact]
public void TimeoutSeconds_Zero_FallsBackToTierDefault_WithDiagnostic()
{
var options = DriverResilienceOptionsParser.ParseOrDefaults(
DriverTier.A, "{\"capabilityPolicies\":{\"Read\":{\"timeoutSeconds\":0}}}", out var diag);
var tierDefault = DriverResilienceOptions.GetTierDefaults(DriverTier.A)[DriverCapability.Read];
options.Resolve(DriverCapability.Read).TimeoutSeconds.ShouldBe(tierDefault.TimeoutSeconds);
diag.ShouldNotBeNull();
diag.ShouldContain("timeoutSeconds");
}
/// <summary>A timeout above Polly's ceiling clamps to the max with a diagnostic.</summary>
[Fact]
public void TimeoutSeconds_AbovePollyMax_Clamped()
{
var options = DriverResilienceOptionsParser.ParseOrDefaults(
DriverTier.A, "{\"capabilityPolicies\":{\"Read\":{\"timeoutSeconds\":999999}}}", out var diag);
options.Resolve(DriverCapability.Read).TimeoutSeconds.ShouldBe(ResiliencePolicyRanges.MaxTimeoutSeconds);
diag.ShouldNotBeNull();
diag.ShouldContain("timeoutSeconds");
}
/// <summary>A negative retry count clamps to zero with a diagnostic.</summary>
[Fact]
public void RetryCount_Negative_ClampedToZero()
{
var options = DriverResilienceOptionsParser.ParseOrDefaults(
DriverTier.A, "{\"capabilityPolicies\":{\"Read\":{\"retryCount\":-1}}}", out var diag);
options.Resolve(DriverCapability.Read).RetryCount.ShouldBe(0);
diag.ShouldNotBeNull();
diag.ShouldContain("retryCount");
}
/// <summary>An absurd retry count clamps to the operational cap with a diagnostic.</summary>
[Fact]
public void RetryCount_AboveCap_ClampedTo100()
{
var options = DriverResilienceOptionsParser.ParseOrDefaults(
DriverTier.A, "{\"capabilityPolicies\":{\"Read\":{\"retryCount\":2000000000}}}", out var diag);
options.Resolve(DriverCapability.Read).RetryCount.ShouldBe(ResiliencePolicyRanges.MaxRetryCount);
diag.ShouldNotBeNull();
diag.ShouldContain("retryCount");
}
/// <summary>A breaker threshold of 1 (below Polly's MinimumThroughput floor) raises to 2 with a diagnostic.</summary>
[Fact]
public void BreakerThreshold_One_ClampedToTwo()
{
var options = DriverResilienceOptionsParser.ParseOrDefaults(
DriverTier.A, "{\"capabilityPolicies\":{\"Read\":{\"breakerFailureThreshold\":1}}}", out var diag);
options.Resolve(DriverCapability.Read).BreakerFailureThreshold.ShouldBe(
ResiliencePolicyRanges.MinEnabledBreakerFailureThreshold);
diag.ShouldNotBeNull();
diag.ShouldContain("breakerFailureThreshold");
}
/// <summary>A negative breaker threshold disables the breaker (0) with a diagnostic.</summary>
[Fact]
public void BreakerThreshold_Negative_DisablesBreaker()
{
var options = DriverResilienceOptionsParser.ParseOrDefaults(
DriverTier.A, "{\"capabilityPolicies\":{\"Read\":{\"breakerFailureThreshold\":-5}}}", out var diag);
options.Resolve(DriverCapability.Read).BreakerFailureThreshold.ShouldBe(0);
diag.ShouldNotBeNull();
diag.ShouldContain("breakerFailureThreshold");
}
/// <summary>Multiple clamps in one policy all surface in the (appended) diagnostic string.</summary>
[Fact]
public void MultipleClamps_AllSurfaceInDiagnostic()
{
var options = DriverResilienceOptionsParser.ParseOrDefaults(
DriverTier.A,
"{\"capabilityPolicies\":{\"Read\":{\"timeoutSeconds\":0,\"retryCount\":-1,\"breakerFailureThreshold\":1}}}",
out var diag);
diag.ShouldNotBeNull();
diag.ShouldContain("timeoutSeconds");
diag.ShouldContain("retryCount");
diag.ShouldContain("breakerFailureThreshold");
var read = options.Resolve(DriverCapability.Read);
read.TimeoutSeconds.ShouldBe(DriverResilienceOptions.GetTierDefaults(DriverTier.A)[DriverCapability.Read].TimeoutSeconds);
read.RetryCount.ShouldBe(0);
read.BreakerFailureThreshold.ShouldBe(ResiliencePolicyRanges.MinEnabledBreakerFailureThreshold);
}
/// <summary>In-range overrides pass through untouched with no diagnostic.</summary>
[Fact]
public void InRangeOverrides_PassThrough_NoDiagnostic()
{
var options = DriverResilienceOptionsParser.ParseOrDefaults(
DriverTier.A,
"{\"capabilityPolicies\":{\"Read\":{\"timeoutSeconds\":5,\"retryCount\":3,\"breakerFailureThreshold\":4}}}",
out var diag);
diag.ShouldBeNull();
var read = options.Resolve(DriverCapability.Read);
read.TimeoutSeconds.ShouldBe(5);
read.RetryCount.ShouldBe(3);
read.BreakerFailureThreshold.ShouldBe(4);
}
}