diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/DatabaseOptionsValidator.cs b/src/ZB.MOM.WW.ScadaBridge.Host/DatabaseOptionsValidator.cs new file mode 100644 index 00000000..823ec2f1 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.Host/DatabaseOptionsValidator.cs @@ -0,0 +1,28 @@ +using ZB.MOM.WW.Configuration; + +namespace ZB.MOM.WW.ScadaBridge.Host; + +/// +/// Validates at host startup (arch-review 08 +/// round 2 NF4). All three connection settings are nullable and role-dependent +/// (central needs the SQL Server strings, a site needs only the SQLite path), +/// so a null value is valid. A present-but-whitespace value is not — it +/// signals a mis-set config key that would fail opaquely at first DB use. +/// +public sealed class DatabaseOptionsValidator : OptionsValidatorBase +{ + /// + protected override void Validate(ValidationBuilder builder, DatabaseOptions options) + { + RequireNotWhitespace(builder, options.ConfigurationDb, nameof(DatabaseOptions.ConfigurationDb)); + RequireNotWhitespace(builder, options.MachineDataDb, nameof(DatabaseOptions.MachineDataDb)); + RequireNotWhitespace(builder, options.SiteDbPath, nameof(DatabaseOptions.SiteDbPath)); + } + + // null is valid (role-dependent); reject only a present-but-blank value. + private static void RequireNotWhitespace(ValidationBuilder builder, string? value, string field) + { + builder.RequireThat(value is null || !string.IsNullOrWhiteSpace(value), + $"ScadaBridge:Database:{field} is set but blank — either remove it or give it a real value."); + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/LoggingOptionsValidator.cs b/src/ZB.MOM.WW.ScadaBridge.Host/LoggingOptionsValidator.cs new file mode 100644 index 00000000..1adbffdd --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.Host/LoggingOptionsValidator.cs @@ -0,0 +1,27 @@ +using Serilog.Events; +using ZB.MOM.WW.Configuration; + +namespace ZB.MOM.WW.ScadaBridge.Host; + +/// +/// Validates at host startup (arch-review 08 round +/// 2 NF4). is parsed into a Serilog +/// by LoggerConfigurationFactory.ParseLevel, +/// which silently falls back to Information for an unrecognised value. +/// This validator surfaces that misconfiguration at boot instead: a present, +/// non-blank value must name a real level (Verbose/Debug/Information/Warning/ +/// Error/Fatal, case-insensitive). A null/blank value is treated as "unset" +/// (defaults to Information), mirroring the parser. +/// +public sealed class LoggingOptionsValidator : OptionsValidatorBase +{ + /// + protected override void Validate(ValidationBuilder builder, LoggingOptions options) + { + builder.RequireThat( + string.IsNullOrWhiteSpace(options.MinimumLevel) + || Enum.TryParse(options.MinimumLevel, ignoreCase: true, out _), + $"ScadaBridge:Logging:{nameof(LoggingOptions.MinimumLevel)} ('{options.MinimumLevel}') " + + $"is not a recognised Serilog level. Valid: {string.Join(", ", Enum.GetNames())}."); + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/NodeOptionsValidator.cs b/src/ZB.MOM.WW.ScadaBridge.Host/NodeOptionsValidator.cs new file mode 100644 index 00000000..1b5a7623 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.Host/NodeOptionsValidator.cs @@ -0,0 +1,34 @@ +using ZB.MOM.WW.Configuration; + +namespace ZB.MOM.WW.ScadaBridge.Host; + +/// +/// Validates at host startup (arch-review 08 round 2 +/// NF4). The headline invariant: must be +/// non-empty — an empty value normalises to a NULL SourceNode audit +/// column, silently degrading the audit trail (the wonder-app-vd03 failure +/// mode), so the host must fail fast at boot instead. Ports are bounded to the +/// TCP range; 0 stays valid because it requests a dynamically-assigned +/// port (CompositionRootTests rely on this). +/// +public sealed class NodeOptionsValidator : OptionsValidatorBase +{ + /// + protected override void Validate(ValidationBuilder builder, NodeOptions options) + { + builder.RequireThat(!string.IsNullOrWhiteSpace(options.NodeName), + $"ScadaBridge:Node:{nameof(NodeOptions.NodeName)} must be a non-empty label " + + "— an empty node name stamps the SourceNode audit column NULL."); + + RequirePort(builder, options.RemotingPort, nameof(NodeOptions.RemotingPort)); + RequirePort(builder, options.GrpcPort, nameof(NodeOptions.GrpcPort)); + RequirePort(builder, options.MetricsPort, nameof(NodeOptions.MetricsPort)); + } + + // 0 stays valid (dynamic-port request); reject only out-of-TCP-range values. + private static void RequirePort(ValidationBuilder builder, int port, string field) + { + builder.RequireThat(port is >= 0 and <= 65_535, + $"ScadaBridge:Node:{field} ({port}) must be in [0, 65535]."); + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/SiteServiceRegistration.cs b/src/ZB.MOM.WW.ScadaBridge.Host/SiteServiceRegistration.cs index 104b38fe..b24d0277 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/SiteServiceRegistration.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/SiteServiceRegistration.cs @@ -157,19 +157,33 @@ public static class SiteServiceRegistration /// Application configuration supplying the option values. public static void BindSharedOptions(IServiceCollection services, IConfiguration config) { - services.Configure(config.GetSection("ScadaBridge:Node")); + // Bind + eagerly validate: an empty NodeName would stamp the SourceNode audit + // column NULL, so fail the host at boot with a key-naming message instead of + // silently degrading the audit trail (arch-review 08r2 NF4). + services.AddOptions().Bind(config.GetSection("ScadaBridge:Node")).ValidateOnStart(); + services.TryAddEnumerable( + ServiceDescriptor.Singleton, NodeOptionsValidator>()); // Bind + eagerly validate: ClusterOptionsValidator is registered (TryAddEnumerable) // by the ClusterInfrastructure module, so chaining ValidateOnStart() here makes a bad // ScadaBridge:Cluster section fail fast at host build instead of lazily on first resolve. services.AddOptions().Bind(config.GetSection("ScadaBridge:Cluster")).ValidateOnStart(); - services.Configure(config.GetSection("ScadaBridge:Database")); + // Bind + eagerly validate: a present-but-blank connection setting fails fast + // here rather than opaquely at first DB use (arch-review 08r2 NF4). + services.AddOptions().Bind(config.GetSection("ScadaBridge:Database")).ValidateOnStart(); + services.TryAddEnumerable( + ServiceDescriptor.Singleton, DatabaseOptionsValidator>()); services.Configure(config.GetSection("ScadaBridge:Communication")); // Bind + eagerly validate: HealthMonitoringOptionsValidator is registered (TryAddEnumerable) // by the HealthMonitoring module, so chaining ValidateOnStart() here makes a bad // ScadaBridge:HealthMonitoring section fail fast at host build instead of lazily on first resolve. services.AddOptions().Bind(config.GetSection("ScadaBridge:HealthMonitoring")).ValidateOnStart(); services.Configure(config.GetSection("ScadaBridge:Notification")); - services.Configure(config.GetSection("ScadaBridge:Logging")); + // Bind + eagerly validate: an unrecognised MinimumLevel would silently fall back + // to Information; fail fast at boot so the operator's intended floor is honoured + // (arch-review 08r2 NF4). + services.AddOptions().Bind(config.GetSection("ScadaBridge:Logging")).ValidateOnStart(); + services.TryAddEnumerable( + ServiceDescriptor.Singleton, LoggingOptionsValidator>()); // Audit Log — exposes ScadaBridge:Node:NodeName to downstream audit // writers so they can stamp the SourceNode column. Registered here in diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ActorPathTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ActorPathTests.cs index 4bd84ab9..3cd1a9de 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ActorPathTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ActorPathTests.cs @@ -48,6 +48,7 @@ public class CentralActorPathTests : IAsyncLifetime { config.AddInMemoryCollection(new Dictionary { + ["ScadaBridge:Node:NodeName"] = "central-a", ["ScadaBridge:Node:NodeHostname"] = "localhost", ["ScadaBridge:Node:RemotingPort"] = "0", ["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:25510", @@ -171,6 +172,7 @@ public class SiteActorPathTests : IAsyncLifetime config.AddInMemoryCollection(new Dictionary { ["ScadaBridge:Node:Role"] = "Site", + ["ScadaBridge:Node:NodeName"] = "node-a", ["ScadaBridge:Node:NodeHostname"] = "localhost", ["ScadaBridge:Node:SiteId"] = "TestSite", ["ScadaBridge:Node:RemotingPort"] = "0", diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/AkkaHostedServiceAuditWiringTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/AkkaHostedServiceAuditWiringTests.cs index 9e9cbf61..5fad810a 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/AkkaHostedServiceAuditWiringTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/AkkaHostedServiceAuditWiringTests.cs @@ -114,6 +114,7 @@ public class CentralAuditWiringTests : IDisposable { config.AddInMemoryCollection(new Dictionary { + ["ScadaBridge:Node:NodeName"] = "central-a", ["ScadaBridge:Node:NodeHostname"] = "localhost", ["ScadaBridge:Node:RemotingPort"] = "0", ["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551", @@ -294,6 +295,7 @@ public class SiteAuditWiringTests : IDisposable builder.Configuration.AddInMemoryCollection(new Dictionary { ["ScadaBridge:Node:Role"] = "Site", + ["ScadaBridge:Node:NodeName"] = "node-a", ["ScadaBridge:Node:NodeHostname"] = "test-site", ["ScadaBridge:Node:SiteId"] = "TestSite", ["ScadaBridge:Node:RemotingPort"] = "0", diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CompositionRootTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CompositionRootTests.cs index f054b651..a1ae5025 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CompositionRootTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CompositionRootTests.cs @@ -108,6 +108,7 @@ public class CentralCompositionRootTests : IDisposable { config.AddInMemoryCollection(new Dictionary { + ["ScadaBridge:Node:NodeName"] = "central-a", ["ScadaBridge:Node:NodeHostname"] = "localhost", ["ScadaBridge:Node:RemotingPort"] = "0", ["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551", @@ -348,6 +349,7 @@ public class SiteCompositionRootTests : IDisposable builder.Configuration.AddInMemoryCollection(new Dictionary { ["ScadaBridge:Node:Role"] = "Site", + ["ScadaBridge:Node:NodeName"] = "node-a", ["ScadaBridge:Node:NodeHostname"] = "test-site", ["ScadaBridge:Node:SiteId"] = "TestSite", ["ScadaBridge:Node:RemotingPort"] = "0", diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/HealthCheckTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/HealthCheckTests.cs index 1a17b58c..298d959b 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/HealthCheckTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/HealthCheckTests.cs @@ -44,6 +44,7 @@ public class HealthCheckTests : IDisposable { config.AddInMemoryCollection(new Dictionary { + ["ScadaBridge:Node:NodeName"] = "central-a", ["ScadaBridge:Node:NodeHostname"] = "localhost", ["ScadaBridge:Node:RemotingPort"] = "0", ["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551", diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/HostStartupTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/HostStartupTests.cs index d1286046..2ff6c6d7 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/HostStartupTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/HostStartupTests.cs @@ -41,6 +41,7 @@ public class HostStartupTests : IDisposable { config.AddInMemoryCollection(new Dictionary { + ["ScadaBridge:Node:NodeName"] = "central-a", ["ScadaBridge:Node:NodeHostname"] = "localhost", ["ScadaBridge:Node:RemotingPort"] = "0", ["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551", @@ -74,6 +75,7 @@ public class HostStartupTests : IDisposable builder.Configuration.AddInMemoryCollection(new Dictionary { ["ScadaBridge:Node:Role"] = "Site", + ["ScadaBridge:Node:NodeName"] = "node-a", ["ScadaBridge:Node:NodeHostname"] = "test-site", ["ScadaBridge:Node:SiteId"] = "TestSite", ["ScadaBridge:Node:RemotingPort"] = "0", @@ -103,6 +105,7 @@ public class HostStartupTests : IDisposable builder.Configuration.AddInMemoryCollection(new Dictionary { ["ScadaBridge:Node:Role"] = "Site", + ["ScadaBridge:Node:NodeName"] = "node-a", ["ScadaBridge:Node:NodeHostname"] = "test-site", ["ScadaBridge:Node:SiteId"] = "TestSite", ["ScadaBridge:Node:RemotingPort"] = "0", diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/MetricsEndpointTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/MetricsEndpointTests.cs index 90118bf4..f5aeb8ee 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/MetricsEndpointTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/MetricsEndpointTests.cs @@ -40,6 +40,7 @@ public class MetricsEndpointTests : IDisposable { config.AddInMemoryCollection(new Dictionary { + ["ScadaBridge:Node:NodeName"] = "central-a", ["ScadaBridge:Node:NodeHostname"] = "localhost", ["ScadaBridge:Node:RemotingPort"] = "0", ["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551", diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/NodeOptionsValidatorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/NodeOptionsValidatorTests.cs new file mode 100644 index 00000000..3a6267f8 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/NodeOptionsValidatorTests.cs @@ -0,0 +1,107 @@ +using ZB.MOM.WW.ScadaBridge.Host; + +namespace ZB.MOM.WW.ScadaBridge.Host.Tests; + +/// +/// Eager startup validation for the shared Host options (arch-review 08 round 2 +/// NF4). Covers , +/// , and +/// in one file. The headline case: an +/// empty normalises to a NULL SourceNode +/// audit column, so it must fail the host at boot rather than silently degrade +/// the audit trail. +/// +public class NodeOptionsValidatorTests +{ + // ---- NodeOptions ---- + + [Fact] + public void Node_DefaultsWithNodeName_AreValid() + { + var validator = new NodeOptionsValidator(); + Assert.True(validator.Validate(null, new NodeOptions { NodeName = "node-a" }).Succeeded); + } + + [Theory] + [InlineData("")] + [InlineData(" ")] + public void Node_EmptyOrWhitespaceNodeName_Fails(string nodeName) + { + var validator = new NodeOptionsValidator(); + var result = validator.Validate(null, new NodeOptions { NodeName = nodeName }); + Assert.False(result.Succeeded); + Assert.Contains(result.Failures!, + f => f.Contains(nameof(NodeOptions.NodeName), StringComparison.Ordinal)); + } + + [Fact] + public void Node_ZeroRemotingPort_IsValid() + { + // CompositionRootTests uses RemotingPort = 0 to request a dynamic port. + var validator = new NodeOptionsValidator(); + Assert.True(validator.Validate(null, + new NodeOptions { NodeName = "node-a", RemotingPort = 0 }).Succeeded); + } + + [Theory] + [InlineData(-1)] + [InlineData(65_536)] + public void Node_PortOutOfRange_Fails(int port) + { + var validator = new NodeOptionsValidator(); + var result = validator.Validate(null, + new NodeOptions { NodeName = "node-a", GrpcPort = port }); + Assert.False(result.Succeeded); + Assert.Contains(result.Failures!, + f => f.Contains(nameof(NodeOptions.GrpcPort), StringComparison.Ordinal)); + } + + // ---- DatabaseOptions ---- + + [Fact] + public void Database_AllNull_IsValid() + { + // All three fields are nullable and role-dependent; null stays valid. + var validator = new DatabaseOptionsValidator(); + Assert.True(validator.Validate(null, new DatabaseOptions()).Succeeded); + } + + [Fact] + public void Database_WhitespaceValue_Fails() + { + var validator = new DatabaseOptionsValidator(); + var result = validator.Validate(null, new DatabaseOptions { ConfigurationDb = " " }); + Assert.False(result.Succeeded); + Assert.Contains(result.Failures!, + f => f.Contains(nameof(DatabaseOptions.ConfigurationDb), StringComparison.Ordinal)); + } + + // ---- LoggingOptions ---- + + [Fact] + public void Logging_DefaultInformation_IsValid() + { + var validator = new LoggingOptionsValidator(); + Assert.True(validator.Validate(null, new LoggingOptions()).Succeeded); + } + + [Theory] + [InlineData("Verbose")] + [InlineData("debug")] + [InlineData("FATAL")] + public void Logging_RecognisedLevel_IsValid(string level) + { + var validator = new LoggingOptionsValidator(); + Assert.True(validator.Validate(null, new LoggingOptions { MinimumLevel = level }).Succeeded); + } + + [Fact] + public void Logging_UnknownLevel_Fails() + { + var validator = new LoggingOptionsValidator(); + var result = validator.Validate(null, new LoggingOptions { MinimumLevel = "Chatty" }); + Assert.False(result.Succeeded); + Assert.Contains(result.Failures!, + f => f.Contains(nameof(LoggingOptions.MinimumLevel), StringComparison.Ordinal)); + } +}