From 6dbbc7ad04c3c5c11c58943f7f70b73a046e4f9f Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 1 Jun 2026 19:04:13 -0400 Subject: [PATCH] refactor: ScadaBridge StartupValidator -> ConfigPreflight (byte-compatible) --- .../StartupValidator.cs | 158 +++++++++--------- .../ZB.MOM.WW.ScadaBridge.Host.csproj | 1 + 2 files changed, 84 insertions(+), 75 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/StartupValidator.cs b/src/ZB.MOM.WW.ScadaBridge.Host/StartupValidator.cs index 7c0a9121..f7ab3ed6 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/StartupValidator.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/StartupValidator.cs @@ -1,3 +1,5 @@ +using ZB.MOM.WW.Configuration; + namespace ZB.MOM.WW.ScadaBridge.Host; /// @@ -10,92 +12,98 @@ public static class StartupValidator /// The application configuration to validate. public static void Validate(IConfiguration configuration) { - var errors = new List(); - + // Resolve the same locals the original imperative validator used, so the + // cross-field predicates below can close over them. ConfigPreflight.Require + // passes config[key] to each predicate, but the cross-field rules ignore that + // argument and read these resolved values instead — preserving the exact + // conditions (and therefore the byte-identical failure messages and ordering) + // of the original StartupValidator. var nodeSection = configuration.GetSection("ScadaBridge:Node"); var role = nodeSection["Role"]; - if (string.IsNullOrEmpty(role) || (role != "Central" && role != "Site")) - errors.Add("ScadaBridge:Node:Role must be 'Central' or 'Site'"); - - if (string.IsNullOrEmpty(nodeSection["NodeHostname"])) - errors.Add("ScadaBridge:Node:NodeHostname is required"); var portStr = nodeSection["RemotingPort"]; - if (!int.TryParse(portStr, out var port) || port < 1 || port > 65535) - errors.Add("ScadaBridge:Node:RemotingPort must be 1-65535"); - - if (role == "Site" && string.IsNullOrEmpty(nodeSection["SiteId"])) - errors.Add("ScadaBridge:Node:SiteId is required for Site nodes"); - - if (role == "Central") - { - var dbSection = configuration.GetSection("ScadaBridge:Database"); - if (string.IsNullOrEmpty(dbSection["ConfigurationDb"])) - errors.Add("ScadaBridge:Database:ConfigurationDb connection string required for Central"); - - var secSection = configuration.GetSection("ScadaBridge:Security"); - if (string.IsNullOrEmpty(secSection["LdapServer"])) - errors.Add("ScadaBridge:Security:LdapServer required for Central"); - if (string.IsNullOrEmpty(secSection["JwtSigningKey"])) - errors.Add("ScadaBridge:Security:JwtSigningKey required for Central"); - } + bool portValid = int.TryParse(portStr, out var port) && port >= 1 && port <= 65535; var seedNodes = configuration.GetSection("ScadaBridge:Cluster:SeedNodes").Get>(); - if (seedNodes == null || seedNodes.Count < 2) - errors.Add("ScadaBridge:Cluster:SeedNodes must have at least 2 entries"); - if (role == "Site") - { - var grpcPortStr = nodeSection["GrpcPort"]; - int grpcPort = 8083; // NodeOptions default when the key is absent - if (grpcPortStr != null && (!int.TryParse(grpcPortStr, out grpcPort) || grpcPort < 1 || grpcPort > 65535)) - errors.Add("ScadaBridge:Node:GrpcPort must be 1-65535"); + // GrpcPort: default 8083 when absent; only fails the range rule when the key is + // present AND invalid. The out-param assignment mirrors the original so the + // resolved grpcPort feeds the cross-field rules even on a parse failure. + var grpcPortStr = nodeSection["GrpcPort"]; + int grpcPort = 8083; // NodeOptions default when the key is absent + bool grpcValid = !(grpcPortStr != null && (!int.TryParse(grpcPortStr, out grpcPort) || grpcPort < 1 || grpcPort > 65535)); - // Host-007 / REQ-HOST-4: the gRPC (Kestrel HTTP/2) port and the Akka - // remoting port must differ. Identical values make Kestrel and - // Akka.Remote contend for the same TCP port and fail opaquely at - // runtime. Uses the resolved GrpcPort, including the 8083 default. - if (port == grpcPort) - errors.Add("ScadaBridge:Node:GrpcPort must differ from RemotingPort"); + // MetricsPort: default 8084 when absent; same parse-or-default contract as GrpcPort. + var metricsPortStr = nodeSection["MetricsPort"]; + int metricsPort = 8084; // NodeOptions default when the key is absent + bool metricsValid = !(metricsPortStr != null && (!int.TryParse(metricsPortStr, out metricsPort) || metricsPort < 1 || metricsPort > 65535)); - var metricsPortStr = nodeSection["MetricsPort"]; - int metricsPort = 8084; // NodeOptions default when the key is absent - if (metricsPortStr != null && (!int.TryParse(metricsPortStr, out metricsPort) || metricsPort < 1 || metricsPort > 65535)) - errors.Add("ScadaBridge:Node:MetricsPort must be 1-65535"); - - // Host-007 / REQ-HOST-4: the Kestrel metrics (HTTP/1.1) listener port - // must differ from BOTH the Akka remoting port and the gRPC port. - // A collision makes the metrics listener contend with Akka.Remote or - // the gRPC listener for the same TCP port and fail opaquely at runtime. - // Uses the resolved MetricsPort, including the 8084 default. - if (metricsPort == port) - errors.Add("ScadaBridge:Node:MetricsPort must differ from RemotingPort"); - if (metricsPort == grpcPort) - errors.Add("ScadaBridge:Node:MetricsPort must differ from GrpcPort"); - - var dbSection = configuration.GetSection("ScadaBridge:Database"); - if (string.IsNullOrEmpty(dbSection["SiteDbPath"])) - errors.Add("ScadaBridge:Database:SiteDbPath required for Site nodes"); - - // Host-004: a seed node must reference an Akka.Remote endpoint, never the - // Kestrel HTTP/2 gRPC port. A seed entry whose port equals this node's - // GrpcPort would make a joining node attempt an Akka.Remote TCP - // association against the gRPC listener and fail. - if (seedNodes != null) + ConfigPreflight.For(configuration) + // Role / NodeHostname / RemotingPort (unconditional) + .Require("ScadaBridge:Node:Role", + _ => !(string.IsNullOrEmpty(role) || (role != "Central" && role != "Site")), + "must be 'Central' or 'Site'") + .Require("ScadaBridge:Node:NodeHostname", + _ => !string.IsNullOrEmpty(nodeSection["NodeHostname"]), + "is required") + .Require("ScadaBridge:Node:RemotingPort", + _ => portValid, + "must be 1-65535") + // SiteId (Site only) — note: OUTSIDE the big Site block in the original, + // so it must run before the unconditional SeedNodes-count rule. + .When(role == "Site", p => p + .Require("ScadaBridge:Node:SiteId", + _ => !string.IsNullOrEmpty(nodeSection["SiteId"]), + "is required for Site nodes")) + // Central-only database/security rules. + .When(role == "Central", p => p + .Require("ScadaBridge:Database:ConfigurationDb", + _ => !string.IsNullOrEmpty(configuration.GetSection("ScadaBridge:Database")["ConfigurationDb"]), + "connection string required for Central") + .Require("ScadaBridge:Security:LdapServer", + _ => !string.IsNullOrEmpty(configuration.GetSection("ScadaBridge:Security")["LdapServer"]), + "required for Central") + .Require("ScadaBridge:Security:JwtSigningKey", + _ => !string.IsNullOrEmpty(configuration.GetSection("ScadaBridge:Security")["JwtSigningKey"]), + "required for Central")) + // SeedNodes count (unconditional, after SiteId). + .Require("ScadaBridge:Cluster:SeedNodes", + _ => seedNodes != null && seedNodes.Count >= 2, + "must have at least 2 entries") + // The big Site-only block: GrpcPort/MetricsPort validity + cross-field + // collisions + SiteDbPath + seed-node-port loop, in the original order. + .When(role == "Site", p => { - foreach (var seed in seedNodes) - { - if (SeedNodePort(seed) == grpcPort) - errors.Add( - $"ScadaBridge:Cluster:SeedNodes entry '{seed}' must not target the gRPC port " + - $"({grpcPort}); seed nodes must reference Akka remoting ports"); - } - } - } + // Host-007 / REQ-HOST-4: GrpcPort range, then GrpcPort vs RemotingPort. + p.Require("ScadaBridge:Node:GrpcPort", _ => grpcValid, "must be 1-65535"); + // Identical GrpcPort/RemotingPort make Kestrel and Akka.Remote contend + // for the same TCP port. Uses the resolved GrpcPort, including 8083. + p.Require("ScadaBridge:Node:GrpcPort", _ => port != grpcPort, "must differ from RemotingPort"); - if (errors.Count > 0) - throw new InvalidOperationException( - $"Configuration validation failed:\n{string.Join("\n", errors.Select(e => $" - {e}"))}"); + // Host-007 / REQ-HOST-4: MetricsPort range, then MetricsPort vs both ports. + p.Require("ScadaBridge:Node:MetricsPort", _ => metricsValid, "must be 1-65535"); + // The Kestrel metrics (HTTP/1.1) listener port must differ from BOTH the + // Akka remoting port and the gRPC port. Uses the resolved MetricsPort (8084 default). + p.Require("ScadaBridge:Node:MetricsPort", _ => metricsPort != port, "must differ from RemotingPort"); + p.Require("ScadaBridge:Node:MetricsPort", _ => metricsPort != grpcPort, "must differ from GrpcPort"); + + p.Require("ScadaBridge:Database:SiteDbPath", + _ => !string.IsNullOrEmpty(configuration.GetSection("ScadaBridge:Database")["SiteDbPath"]), + "required for Site nodes"); + + // Host-004: a seed node must reference an Akka.Remote endpoint, never the + // Kestrel HTTP/2 gRPC port. A seed entry whose port equals this node's + // GrpcPort would make a joining node attempt an Akka.Remote TCP + // association against the gRPC listener and fail. + foreach (var seed in seedNodes ?? Enumerable.Empty()) + { + p.Require("ScadaBridge:Cluster:SeedNodes", + _ => SeedNodePort(seed) != grpcPort, + $"entry '{seed}' must not target the gRPC port " + + $"({grpcPort}); seed nodes must reference Akka remoting ports"); + } + }) + .ThrowIfInvalid(); } /// diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/ZB.MOM.WW.ScadaBridge.Host.csproj b/src/ZB.MOM.WW.ScadaBridge.Host/ZB.MOM.WW.ScadaBridge.Host.csproj index b266031b..64c59471 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/ZB.MOM.WW.ScadaBridge.Host.csproj +++ b/src/ZB.MOM.WW.ScadaBridge.Host/ZB.MOM.WW.ScadaBridge.Host.csproj @@ -28,6 +28,7 @@ +