feat(options): eager startup validation for central-component options (Transport, SiteCallAudit, DeploymentManager) (arch-review 08 §1.5)

Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
This commit is contained in:
Joseph Doherty
2026-07-10 06:52:07 -04:00
parent fc918d4679
commit 6bf9dfb3c5
12 changed files with 436 additions and 4 deletions
@@ -0,0 +1,31 @@
using ZB.MOM.WW.Configuration;
namespace ZB.MOM.WW.ScadaBridge.DeploymentManager;
/// <summary>
/// Validates <see cref="DeploymentManagerOptions"/> at startup. Each timeout is
/// applied as a linked-CTS deadline around a lifecycle command, per-site artifact
/// deployment, or operation-lock acquisition; a zero/negative value makes the
/// deadline fire immediately (or the <see cref="System.Threading.CancellationTokenSource"/>
/// constructor throw), breaking deployments with an opaque failure. Registered
/// with <c>ValidateOnStart()</c> so a bad <c>ScadaBridge:DeploymentManager</c>
/// section fails fast at boot with a clear, key-naming message.
/// </summary>
public sealed class DeploymentManagerOptionsValidator : OptionsValidatorBase<DeploymentManagerOptions>
{
/// <inheritdoc />
protected override void Validate(ValidationBuilder builder, DeploymentManagerOptions options)
{
builder.RequireThat(options.LifecycleCommandTimeout > TimeSpan.Zero,
$"ScadaBridge:DeploymentManager:LifecycleCommandTimeout must be a positive duration " +
$"(was {options.LifecycleCommandTimeout}); it bounds a lifecycle command round-trip.");
builder.RequireThat(options.ArtifactDeploymentTimeoutPerSite > TimeSpan.Zero,
$"ScadaBridge:DeploymentManager:ArtifactDeploymentTimeoutPerSite must be a positive duration " +
$"(was {options.ArtifactDeploymentTimeoutPerSite}); it bounds per-site artifact deployment.");
builder.RequireThat(options.OperationLockTimeout > TimeSpan.Zero,
$"ScadaBridge:DeploymentManager:OperationLockTimeout must be a positive duration " +
$"(was {options.OperationLockTimeout}); it bounds acquiring an instance operation lock.");
}
}
@@ -1,4 +1,6 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Microsoft.Extensions.Options;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Transport;
namespace ZB.MOM.WW.ScadaBridge.DeploymentManager;
@@ -27,8 +29,14 @@ public static class ServiceCollectionExtensions
{
// Ensure the options class is always resolvable.
// The Host binds OptionsSection to appsettings.json; absent that binding
// the declared option-class defaults apply.
services.AddOptions<DeploymentManagerOptions>();
// the declared option-class defaults apply. Eager startup validation
// (arch-review 08 §1.5): ValidateOnStart makes a zero/negative timeout —
// which would fire a linked-CTS deadline immediately (or throw in the
// CancellationTokenSource ctor) — fail fast at boot with a clear,
// key-naming message. Applies even though defaults are code-configured.
services.AddOptions<DeploymentManagerOptions>().ValidateOnStart();
services.TryAddEnumerable(
ServiceDescriptor.Singleton<IValidateOptions<DeploymentManagerOptions>, DeploymentManagerOptionsValidator>());
services.AddSingleton<OperationLockManager>();
// Push-based deployment-status notification. Registered
@@ -11,6 +11,7 @@
<PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" />
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions" />
<PackageReference Include="Microsoft.Extensions.Options" />
<PackageReference Include="ZB.MOM.WW.Configuration" />
</ItemGroup>
<ItemGroup>
@@ -1,5 +1,6 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Microsoft.Extensions.Options;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Kpi;
using ZB.MOM.WW.ScadaBridge.SiteCallAudit.Kpi;
@@ -37,8 +38,15 @@ public static class ServiceCollectionExtensions
{
ArgumentNullException.ThrowIfNull(services);
// Eager startup validation (arch-review 08 §1.5): ValidateOnStart makes a
// bad ScadaBridge:SiteCallAudit section — a zero/negative cadence that would
// spin an Akka scheduler, or a non-positive KPI/retention knob — fail fast
// at boot with a clear, key-naming message.
services.AddOptions<SiteCallAuditOptions>()
.BindConfiguration(OptionsSection);
.BindConfiguration(OptionsSection)
.ValidateOnStart();
services.TryAddEnumerable(
ServiceDescriptor.Singleton<IValidateOptions<SiteCallAuditOptions>, SiteCallAuditOptionsValidator>());
// KPI History: the central recorder enumerates every registered
// IKpiSampleSource each sampling pass; this one snapshots the SiteCalls
@@ -0,0 +1,48 @@
using ZB.MOM.WW.Configuration;
namespace ZB.MOM.WW.ScadaBridge.SiteCallAudit;
/// <summary>
/// Validates <see cref="SiteCallAuditOptions"/> at startup. The durations feed
/// stuck-call detection, KPI windowing, the central→site relay Ask timeout, and
/// the two central-singleton scheduler cadences (reconciliation + purge); a
/// zero/negative value makes an Akka scheduler spin or a KPI window collapse.
/// Registered with <c>ValidateOnStart()</c> so a bad <c>ScadaBridge:SiteCallAudit</c>
/// section fails fast at boot with a clear, key-naming message. Validates the
/// BASE properties — the <c>Resolved*</c> computed properties clamp bad cadence
/// values away and would hide the misconfiguration.
/// </summary>
public sealed class SiteCallAuditOptionsValidator : OptionsValidatorBase<SiteCallAuditOptions>
{
/// <inheritdoc />
protected override void Validate(ValidationBuilder builder, SiteCallAuditOptions options)
{
builder.RequireThat(options.StuckAgeThreshold > TimeSpan.Zero,
$"ScadaBridge:SiteCallAudit:StuckAgeThreshold must be a positive duration " +
$"(was {options.StuckAgeThreshold}); it is the age past which a cached call is flagged stuck.");
builder.RequireThat(options.KpiInterval > TimeSpan.Zero,
$"ScadaBridge:SiteCallAudit:KpiInterval must be a positive duration " +
$"(was {options.KpiInterval}); it is the trailing window for the throughput KPIs.");
builder.RequireThat(options.RelayTimeout > TimeSpan.Zero,
$"ScadaBridge:SiteCallAudit:RelayTimeout must be a positive duration " +
$"(was {options.RelayTimeout}); it is the Ask timeout for the central→site Retry/Discard relay.");
builder.RequireThat(options.ReconciliationInterval > TimeSpan.Zero,
$"ScadaBridge:SiteCallAudit:ReconciliationInterval must be a positive duration " +
$"(was {options.ReconciliationInterval}); it is the period of the per-site reconciliation pull.");
builder.RequireThat(options.ReconciliationBatchSize > 0,
$"ScadaBridge:SiteCallAudit:ReconciliationBatchSize must be positive " +
$"(was {options.ReconciliationBatchSize}); it is the max rows requested per PullSiteCalls RPC.");
builder.RequireThat(options.PurgeInterval > TimeSpan.Zero,
$"ScadaBridge:SiteCallAudit:PurgeInterval must be a positive duration " +
$"(was {options.PurgeInterval}); it is the period of the daily terminal-row purge tick.");
builder.RequireThat(options.RetentionDays > 0,
$"ScadaBridge:SiteCallAudit:RetentionDays must be positive " +
$"(was {options.RetentionDays}); it is the retention window for terminal SiteCalls rows.");
}
}
@@ -15,6 +15,7 @@
<PackageReference Include="Microsoft.Extensions.Options" />
<!-- BindConfiguration extension for the SiteCallAuditOptions binding. -->
<PackageReference Include="Microsoft.Extensions.Options.ConfigurationExtensions" />
<PackageReference Include="ZB.MOM.WW.Configuration" />
</ItemGroup>
<ItemGroup>
@@ -22,7 +22,14 @@ public static class ServiceCollectionExtensions
public static IServiceCollection AddTransport(this IServiceCollection services)
{
ArgumentNullException.ThrowIfNull(services);
services.AddOptions<TransportOptions>().BindConfiguration(OptionsSection);
// Eager startup validation (arch-review 08 §1.5): ValidateOnStart makes a
// bad ScadaBridge:Transport section — a zero/negative cap, or (critically)
// a Pbkdf2Iterations below the security floor that would silently weaken
// bundle key derivation — fail fast at boot with a clear, key-naming
// message instead of surfacing deep inside the import pipeline.
services.AddOptions<TransportOptions>().BindConfiguration(OptionsSection).ValidateOnStart();
services.TryAddEnumerable(
ServiceDescriptor.Singleton<IValidateOptions<TransportOptions>, TransportOptionsValidator>());
services.TryAddSingleton(TimeProvider.System);
// Pipeline building blocks: stateless services live as singletons; the
@@ -0,0 +1,67 @@
using ZB.MOM.WW.Configuration;
namespace ZB.MOM.WW.ScadaBridge.Transport;
/// <summary>
/// Validates <see cref="TransportOptions"/> at startup. These knobs gate bundle
/// size, the zip-bomb defences, unlock rate limiting, the schema-version stamp,
/// and — security-critically — the PBKDF2 iteration count used to derive the
/// bundle encryption key. A zero/negative value would either disable a cap,
/// crash the import pipeline, or (for <see cref="TransportOptions.Pbkdf2Iterations"/>)
/// silently weaken key derivation. Registered with <c>ValidateOnStart()</c> so a
/// bad <c>ScadaBridge:Transport</c> section fails fast at boot with a clear,
/// key-naming message.
/// </summary>
public sealed class TransportOptionsValidator : OptionsValidatorBase<TransportOptions>
{
/// <summary>
/// Minimum acceptable PBKDF2 iteration count. A config typo (e.g. 6_000 in
/// place of the 600_000 default) must not silently weaken bundle key
/// derivation, so anything below this floor is rejected at startup.
/// </summary>
private const int MinPbkdf2Iterations = 100_000;
/// <inheritdoc />
protected override void Validate(ValidationBuilder builder, TransportOptions options)
{
builder.RequireThat(options.BundleSessionTtlMinutes > 0,
$"ScadaBridge:Transport:BundleSessionTtlMinutes must be a positive number of minutes " +
$"(was {options.BundleSessionTtlMinutes}); it is the TTL for an in-progress import session.");
builder.RequireThat(options.MaxBundleSizeMb > 0,
$"ScadaBridge:Transport:MaxBundleSizeMb must be positive " +
$"(was {options.MaxBundleSizeMb}); it caps the accepted bundle size.");
builder.RequireThat(options.MaxBundleEntryDecompressedMb > 0,
$"ScadaBridge:Transport:MaxBundleEntryDecompressedMb must be positive " +
$"(was {options.MaxBundleEntryDecompressedMb}); it caps the decompressed size of any single zip entry.");
builder.RequireThat(options.MaxBundleEntryCount > 0,
$"ScadaBridge:Transport:MaxBundleEntryCount must be positive " +
$"(was {options.MaxBundleEntryCount}); it caps the number of entries inside a bundle zip.");
builder.RequireThat(options.MaxBundleEntryCompressionRatio > 0,
$"ScadaBridge:Transport:MaxBundleEntryCompressionRatio must be positive " +
$"(was {options.MaxBundleEntryCompressionRatio}); it caps the per-entry compression ratio.");
builder.RequireThat(options.MaxUnlockAttemptsPerSession > 0,
$"ScadaBridge:Transport:MaxUnlockAttemptsPerSession must be positive " +
$"(was {options.MaxUnlockAttemptsPerSession}); it caps failed passphrase attempts before a session locks.");
builder.RequireThat(options.MaxUnlockAttemptsPerIpPerHour > 0,
$"ScadaBridge:Transport:MaxUnlockAttemptsPerIpPerHour must be positive " +
$"(was {options.MaxUnlockAttemptsPerIpPerHour}); it caps unlock attempts per IP address per hour.");
builder.RequireThat(options.Pbkdf2Iterations >= MinPbkdf2Iterations,
$"ScadaBridge:Transport:Pbkdf2Iterations must be at least {MinPbkdf2Iterations:N0} " +
$"(was {options.Pbkdf2Iterations}); a lower value silently weakens bundle key derivation.");
builder.RequireThat(options.SchemaVersionMajor > 0,
$"ScadaBridge:Transport:SchemaVersionMajor must be positive " +
$"(was {options.SchemaVersionMajor}); it is the major bundle-schema version this instance emits and accepts.");
builder.RequireThat(!string.IsNullOrWhiteSpace(options.SourceEnvironment),
"ScadaBridge:Transport:SourceEnvironment must be a non-empty name; it is stamped into " +
"BundleManifest.SourceEnvironment and the export filename.");
}
}
@@ -11,6 +11,7 @@
<PackageReference Include="Microsoft.Extensions.Hosting.Abstractions" />
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions" />
<PackageReference Include="Microsoft.Extensions.Options.ConfigurationExtensions" />
<PackageReference Include="ZB.MOM.WW.Configuration" />
</ItemGroup>
<ItemGroup>
@@ -0,0 +1,56 @@
using Microsoft.Extensions.Options;
namespace ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests;
/// <summary>
/// Arch-review 08 §1.5: <see cref="DeploymentManagerOptions"/> timeouts are
/// applied as linked-CTS deadlines around lifecycle commands, per-site artifact
/// deployment, and operation-lock acquisition. A zero/negative duration makes a
/// deadline fire immediately (or a <c>CancellationTokenSource</c> constructor
/// throw), so a misconfigured section must be rejected at startup with a clear,
/// key-naming message rather than silently breaking deployments.
/// </summary>
public class DeploymentManagerOptionsValidatorTests
{
private static ValidateOptionsResult Validate(DeploymentManagerOptions options) =>
new DeploymentManagerOptionsValidator().Validate(Options.DefaultName, options);
[Fact]
public void DefaultOptions_AreValid()
{
var result = Validate(new DeploymentManagerOptions());
Assert.True(result.Succeeded, result.FailureMessage);
}
[Fact]
public void ZeroLifecycleCommandTimeout_IsRejected()
{
var result = Validate(new DeploymentManagerOptions { LifecycleCommandTimeout = TimeSpan.Zero });
Assert.True(result.Failed);
Assert.Contains("LifecycleCommandTimeout", result.FailureMessage);
}
[Fact]
public void NegativeLifecycleCommandTimeout_IsRejected()
{
var result = Validate(new DeploymentManagerOptions { LifecycleCommandTimeout = TimeSpan.FromSeconds(-1) });
Assert.True(result.Failed);
Assert.Contains("LifecycleCommandTimeout", result.FailureMessage);
}
[Fact]
public void ZeroArtifactDeploymentTimeoutPerSite_IsRejected()
{
var result = Validate(new DeploymentManagerOptions { ArtifactDeploymentTimeoutPerSite = TimeSpan.Zero });
Assert.True(result.Failed);
Assert.Contains("ArtifactDeploymentTimeoutPerSite", result.FailureMessage);
}
[Fact]
public void ZeroOperationLockTimeout_IsRejected()
{
var result = Validate(new DeploymentManagerOptions { OperationLockTimeout = TimeSpan.Zero });
Assert.True(result.Failed);
Assert.Contains("OperationLockTimeout", result.FailureMessage);
}
}
@@ -0,0 +1,81 @@
using Microsoft.Extensions.Options;
namespace ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests;
/// <summary>
/// Arch-review 08 §1.5: <see cref="SiteCallAuditOptions"/> drives stuck-call
/// detection, KPI windowing, the central→site relay Ask timeout, and the two
/// central-singleton scheduler cadences plus their retention window. A
/// zero/negative duration or non-positive count must be rejected at startup with
/// a clear, key-naming message rather than making an Akka scheduler spin or a KPI
/// query silently degrade. The <c>Resolved*</c> computed properties clamp away
/// bad cadence values, so the BASE properties are validated here.
/// </summary>
public class SiteCallAuditOptionsValidatorTests
{
private static ValidateOptionsResult Validate(SiteCallAuditOptions options) =>
new SiteCallAuditOptionsValidator().Validate(Options.DefaultName, options);
[Fact]
public void DefaultOptions_AreValid()
{
var result = Validate(new SiteCallAuditOptions());
Assert.True(result.Succeeded, result.FailureMessage);
}
[Fact]
public void ZeroStuckAgeThreshold_IsRejected()
{
var result = Validate(new SiteCallAuditOptions { StuckAgeThreshold = TimeSpan.Zero });
Assert.True(result.Failed);
Assert.Contains("StuckAgeThreshold", result.FailureMessage);
}
[Fact]
public void ZeroKpiInterval_IsRejected()
{
var result = Validate(new SiteCallAuditOptions { KpiInterval = TimeSpan.Zero });
Assert.True(result.Failed);
Assert.Contains("KpiInterval", result.FailureMessage);
}
[Fact]
public void ZeroRelayTimeout_IsRejected()
{
var result = Validate(new SiteCallAuditOptions { RelayTimeout = TimeSpan.Zero });
Assert.True(result.Failed);
Assert.Contains("RelayTimeout", result.FailureMessage);
}
[Fact]
public void ZeroReconciliationInterval_IsRejected()
{
var result = Validate(new SiteCallAuditOptions { ReconciliationInterval = TimeSpan.Zero });
Assert.True(result.Failed);
Assert.Contains("ReconciliationInterval", result.FailureMessage);
}
[Fact]
public void ZeroReconciliationBatchSize_IsRejected()
{
var result = Validate(new SiteCallAuditOptions { ReconciliationBatchSize = 0 });
Assert.True(result.Failed);
Assert.Contains("ReconciliationBatchSize", result.FailureMessage);
}
[Fact]
public void ZeroPurgeInterval_IsRejected()
{
var result = Validate(new SiteCallAuditOptions { PurgeInterval = TimeSpan.Zero });
Assert.True(result.Failed);
Assert.Contains("PurgeInterval", result.FailureMessage);
}
[Fact]
public void ZeroRetentionDays_IsRejected()
{
var result = Validate(new SiteCallAuditOptions { RetentionDays = 0 });
Assert.True(result.Failed);
Assert.Contains("RetentionDays", result.FailureMessage);
}
}
@@ -0,0 +1,123 @@
using Microsoft.Extensions.Options;
namespace ZB.MOM.WW.ScadaBridge.Transport.Tests;
/// <summary>
/// Arch-review 08 §1.5: <see cref="TransportOptions"/> feeds bundle size caps,
/// zip-bomb defences, unlock rate limits and — security-critically — the PBKDF2
/// iteration count used to derive the bundle encryption key. A zero/negative or
/// otherwise degenerate value must be rejected at startup with a clear,
/// key-naming message rather than silently weakening key derivation or crashing
/// deep inside the import pipeline.
/// </summary>
public class TransportOptionsValidatorTests
{
private static ValidateOptionsResult Validate(TransportOptions options) =>
new TransportOptionsValidator().Validate(Options.DefaultName, options);
[Fact]
public void DefaultOptions_AreValid()
{
var result = Validate(new TransportOptions());
Assert.True(result.Succeeded, result.FailureMessage);
}
[Fact]
public void ZeroBundleSessionTtlMinutes_IsRejected()
{
var result = Validate(new TransportOptions { BundleSessionTtlMinutes = 0 });
Assert.True(result.Failed);
Assert.Contains("BundleSessionTtlMinutes", result.FailureMessage);
}
[Fact]
public void ZeroMaxBundleSizeMb_IsRejected()
{
var result = Validate(new TransportOptions { MaxBundleSizeMb = 0 });
Assert.True(result.Failed);
Assert.Contains("MaxBundleSizeMb", result.FailureMessage);
}
[Fact]
public void ZeroPbkdf2Iterations_IsRejected()
{
var result = Validate(new TransportOptions { Pbkdf2Iterations = 0 });
Assert.True(result.Failed);
Assert.Contains("Pbkdf2Iterations", result.FailureMessage);
}
[Fact]
public void Pbkdf2Iterations_BelowSecurityFloor_IsRejected()
{
// A config typo (e.g. 6_000 instead of 600_000) must not silently weaken
// bundle key derivation: anything under 100_000 is rejected.
var result = Validate(new TransportOptions { Pbkdf2Iterations = 99_999 });
Assert.True(result.Failed);
Assert.Contains("Pbkdf2Iterations", result.FailureMessage);
}
[Fact]
public void Pbkdf2Iterations_AtSecurityFloor_IsValid()
{
var result = Validate(new TransportOptions { Pbkdf2Iterations = 100_000 });
Assert.True(result.Succeeded, result.FailureMessage);
}
[Fact]
public void ZeroMaxUnlockAttemptsPerSession_IsRejected()
{
var result = Validate(new TransportOptions { MaxUnlockAttemptsPerSession = 0 });
Assert.True(result.Failed);
Assert.Contains("MaxUnlockAttemptsPerSession", result.FailureMessage);
}
[Fact]
public void ZeroMaxBundleEntryDecompressedMb_IsRejected()
{
var result = Validate(new TransportOptions { MaxBundleEntryDecompressedMb = 0 });
Assert.True(result.Failed);
Assert.Contains("MaxBundleEntryDecompressedMb", result.FailureMessage);
}
[Fact]
public void ZeroMaxBundleEntryCount_IsRejected()
{
var result = Validate(new TransportOptions { MaxBundleEntryCount = 0 });
Assert.True(result.Failed);
Assert.Contains("MaxBundleEntryCount", result.FailureMessage);
}
[Fact]
public void ZeroMaxBundleEntryCompressionRatio_IsRejected()
{
var result = Validate(new TransportOptions { MaxBundleEntryCompressionRatio = 0 });
Assert.True(result.Failed);
Assert.Contains("MaxBundleEntryCompressionRatio", result.FailureMessage);
}
[Fact]
public void ZeroMaxUnlockAttemptsPerIpPerHour_IsRejected()
{
var result = Validate(new TransportOptions { MaxUnlockAttemptsPerIpPerHour = 0 });
Assert.True(result.Failed);
Assert.Contains("MaxUnlockAttemptsPerIpPerHour", result.FailureMessage);
}
[Fact]
public void ZeroSchemaVersionMajor_IsRejected()
{
var result = Validate(new TransportOptions { SchemaVersionMajor = 0 });
Assert.True(result.Failed);
Assert.Contains("SchemaVersionMajor", result.FailureMessage);
}
[Theory]
[InlineData("")]
[InlineData(" ")]
public void BlankSourceEnvironment_IsRejected(string value)
{
var result = Validate(new TransportOptions { SourceEnvironment = value });
Assert.True(result.Failed);
Assert.Contains("SourceEnvironment", result.FailureMessage);
}
}