fix(scadabridge): default MetricsPort to 8084 (avoid site RemotingPort collision) + validate port distinctness

2026-06-01 16:46:59 -04:00
parent fe25ac3e51
commit c41cb41c7b
11 changed files with 139 additions and 11 deletions
@@ -352,6 +352,105 @@ public class StartupValidatorTests
        Assert.Null(ex);
    }

+    [Theory]
+    [InlineData("0")]
+    [InlineData("-1")]
+    [InlineData("65536")]
+    [InlineData("abc")]
+    public void Site_InvalidMetricsPort_FailsValidation(string metricsPort)
+    {
+        var values = ValidSiteConfig();
+        values["ScadaBridge:Node:MetricsPort"] = metricsPort;
+        var config = BuildConfig(values);
+
+        var ex = Assert.Throws<InvalidOperationException>(() => StartupValidator.Validate(config));
+        Assert.Contains("MetricsPort must be 1-65535", ex.Message);
+    }
+
+    [Fact]
+    public void Site_ValidMetricsPort_PassesValidation()
+    {
+        var values = ValidSiteConfig();
+        values["ScadaBridge:Node:MetricsPort"] = "8084";
+        var config = BuildConfig(values);
+
+        var ex = Record.Exception(() => StartupValidator.Validate(config));
+        Assert.Null(ex);
+    }
+
+    [Fact]
+    public void Site_MetricsPortEqualsRemotingPort_FailsValidation()
+    {
+        // Host-007 regression: the Kestrel metrics (HTTP/1.1) listener port must
+        // differ from RemotingPort. Identical values cause the metrics listener
+        // and Akka.Remote to contend for the same port at runtime.
+        var values = ValidSiteConfig();
+        values["ScadaBridge:Node:RemotingPort"] = "8082";
+        values["ScadaBridge:Node:MetricsPort"] = "8082";
+        var config = BuildConfig(values);
+
+        var ex = Assert.Throws<InvalidOperationException>(() => StartupValidator.Validate(config));
+        Assert.Contains("MetricsPort must differ from RemotingPort", ex.Message);
+    }
+
+    [Fact]
+    public void Site_MetricsPortEqualsGrpcPort_FailsValidation()
+    {
+        // Host-007 regression: the metrics listener port must differ from GrpcPort.
+        var values = ValidSiteConfig();
+        values["ScadaBridge:Node:GrpcPort"] = "8083";
+        values["ScadaBridge:Node:MetricsPort"] = "8083";
+        var config = BuildConfig(values);
+
+        var ex = Assert.Throws<InvalidOperationException>(() => StartupValidator.Validate(config));
+        Assert.Contains("MetricsPort must differ from GrpcPort", ex.Message);
+    }
+
+    [Fact]
+    public void Site_DefaultMetricsPortEqualsRemotingPort_FailsValidation()
+    {
+        // MetricsPort absent => NodeOptions default 8084. A site whose RemotingPort
+        // is also 8084 must still be rejected.
+        var values = ValidSiteConfig();
+        values["ScadaBridge:Node:RemotingPort"] = "8084";
+        // Keep GrpcPort distinct so only the metrics-vs-remoting rule fires.
+        values["ScadaBridge:Node:GrpcPort"] = "8083";
+        // Seed nodes default to the remoting port (8082) in ValidSiteConfig; realign
+        // them to 8084 so the seed-vs-grpc rule is not what trips here.
+        values["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@site-a-node1:8084";
+        values["ScadaBridge:Cluster:SeedNodes:1"] = "akka.tcp://scadabridge@site-a-node2:8084";
+        var config = BuildConfig(values);
+
+        var ex = Assert.Throws<InvalidOperationException>(() => StartupValidator.Validate(config));
+        Assert.Contains("MetricsPort must differ from RemotingPort", ex.Message);
+    }
+
+    [Fact]
+    public void Site_MetricsPortDiffersFromRemotingAndGrpc_PassesValidation()
+    {
+        var values = ValidSiteConfig();
+        values["ScadaBridge:Node:RemotingPort"] = "8082";
+        values["ScadaBridge:Node:GrpcPort"] = "8083";
+        values["ScadaBridge:Node:MetricsPort"] = "8084";
+        var config = BuildConfig(values);
+
+        var ex = Record.Exception(() => StartupValidator.Validate(config));
+        Assert.Null(ex);
+    }
+
+    [Fact]
+    public void Central_InvalidMetricsPort_NotValidated()
+    {
+        // The metrics-port rules apply to Site nodes only; a Central node runs no
+        // metrics listener, so an out-of-range MetricsPort must not fail startup.
+        var values = ValidCentralConfig();
+        values["ScadaBridge:Node:MetricsPort"] = "0";
+        var config = BuildConfig(values);
+
+        var ex = Record.Exception(() => StartupValidator.Validate(config));
+        Assert.Null(ex);
+    }
+
    [Fact]
    public void MultipleErrors_AllReported()
    {