diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs
index 1104eb0f..921b5069 100644
--- a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs
@@ -111,7 +111,8 @@ var expanderUsesSharedSqlStore =
var expanderServices = new ServiceCollection();
if (expanderUsesSharedSqlStore)
{
- SecretsRegistration.EnsureCentralSharedStoreConnectionString(configuration);
+ SecretsRegistration.EnsureCentralSharedStoreConnectionString(
+ configuration, $"{SecretsRegistration.ReplicationModeKey}=Grpc");
expanderServices.AddZbSecretsSqlServerStore(configuration, "Secrets");
}
else
diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs b/src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs
index 821138c0..5a144b39 100644
--- a/src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs
@@ -257,34 +257,45 @@ public static class SecretsRegistration
}
///
- /// The one definition of "a Central node in Grpc mode has a usable shared-store connection
- /// string", shared by and the Layer-A expander in
- /// Program.cs so the two can never diverge. The expander runs FIRST on a real boot —
- /// without this sharing, a bad value would die inside SqlConnection with a generic
- /// "initialization string" format error before the designed message ever fired.
+ /// The one definition of "a Central node has a usable shared-store connection string", shared
+ /// by every caller that puts a Central node on the shared SQL-Server store — the Grpc-hub
+ /// branch of , the =SqlServer
+ /// branch, and the Layer-A expander in Program.cs — so none of them can diverge on what
+ /// "usable" means. The expander runs FIRST on a real boot — without this sharing, a bad value
+ /// would die inside SqlConnection with a generic "initialization string" format error
+ /// before the designed message ever fired.
///
/// Application configuration.
+ ///
+ /// Names the setting that put this Central node on the shared store, e.g.
+ /// "Secrets:Replication:Mode=Grpc" or "Secrets:Store:Provider=SqlServer" — folded
+ /// into the blank-connection-string message so a boot failure on either path names its own
+ /// real trigger instead of misdescribing the other one (a Store:Provider=SqlServer failure
+ /// must not claim Grpc mode is in effect, and vice versa).
+ ///
/// The validated connection string.
///
/// is blank or contains a ${secret:} reference.
///
- internal static string EnsureCentralSharedStoreConnectionString(IConfiguration config)
+ internal static string EnsureCentralSharedStoreConnectionString(
+ IConfiguration config, string triggerContext)
{
ArgumentNullException.ThrowIfNull(config);
+ ArgumentException.ThrowIfNullOrWhiteSpace(triggerContext);
var hubConnectionString = config[HubConnectionStringKey];
if (string.IsNullOrWhiteSpace(hubConnectionString))
{
- // Fail closed, matching the mode's philosophy: central in Grpc mode REQUIRES the
+ // Fail closed, matching the mode's philosophy: this configuration REQUIRES the
// shared SQL store. An independent local store per central node is exactly the
// divergence scadaproj#4 recorded — one central hub answered authenticated followers
// with an EMPTY manifest, so a hub failover would "succeed" against nothing and stop
// convergence silently.
throw new InvalidOperationException(
$"{HubConnectionStringKey} is empty, but this node is Central with "
- + $"{ReplicationModeKey}=Grpc. Central in Grpc mode requires the "
- + "SHARED SQL-Server secret store — an independent local store per "
+ + $"{triggerContext}. That configuration requires the SHARED "
+ + "SQL-Server secret store — an independent local store per "
+ "central node is the divergence scadaproj#4 recorded (a hub "
+ "failover would 'succeed' against an empty manifest). Supply the "
+ "connection string via appsettings or the environment "
@@ -341,8 +352,9 @@ public static class SecretsRegistration
{
// Same fail-closed rules as the Grpc-mode central branch below: blank, or a
// ${secret:} reference (bootstrap circularity — the pre-host expander needs this
- // same store to resolve it).
- EnsureCentralSharedStoreConnectionString(config);
+ // same store to resolve it). Names ITS OWN trigger (Store:Provider=SqlServer, not
+ // Replication:Mode=Grpc) — this branch runs only when replication is off.
+ EnsureCentralSharedStoreConnectionString(config, $"{StoreProviderKey}=SqlServer");
// Registers the shared SQL-Server store as ISecretStore directly — no hub, no
// follower sweep. Both central nodes just point at the same database, so they serve
@@ -364,7 +376,7 @@ public static class SecretsRegistration
// instead of surfacing later as a package validation message or a
// malformed-connection-string fault on first use. On a real boot the Layer-A
// expander in Program.cs has already run the same check even earlier.
- EnsureCentralSharedStoreConnectionString(config);
+ EnsureCentralSharedStoreConnectionString(config, $"{ReplicationModeKey}=Grpc");
// Central's store is the SHARED SQL-Server store: registers
// SqlServerSecretStore as ISecretStore BEFORE calling AddZbSecrets internally,
diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/StartupValidator.cs b/src/ZB.MOM.WW.ScadaBridge.Host/StartupValidator.cs
index a353e2c6..ef65431e 100644
--- a/src/ZB.MOM.WW.ScadaBridge.Host/StartupValidator.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.Host/StartupValidator.cs
@@ -238,6 +238,7 @@ public static class StartupValidator
var seedNodes = configuration.GetSection("ScadaBridge:Cluster:SeedNodes").Get>();
var provider = InboundApiKeyStoreOptions.ReadProvider(configuration);
var secretsStoreProvider = configuration["Secrets:Store:Provider"];
+ var secretsReplicationEnabled = configuration.GetValue("Secrets:Replication:Enabled");
var isClusteredCentral = role == "Central" && seedNodes is { Count: >= 2 };
var warnings = new List();
@@ -255,7 +256,16 @@ public static class StartupValidator
// explicitly "Sqlite". A fleet that ships secrets to sites on every artifact deploy needs
// both central nodes to deploy the SAME secret set; a per-node store means each node has
// silently drifted the moment anyone edits a secret on only one of them.
+ //
+ // Gated off when Secrets:Replication:Enabled is true (review round 1, finding #1): that
+ // flag already puts this central node on a topology that shares state across the pair —
+ // the pre-existing gRPC hub (whose OWN central-side store is the shared SQL-Server store,
+ // not per-node SQLite — see SecretsRegistration's Grpc branch) or SQL-Server replication
+ // (ReplicatingSecretStore). Firing this warning there would be a false positive against
+ // the design's own named production topology (scadaproj#3), telling an operator to set a
+ // key that is already functionally moot.
if (isClusteredCentral
+ && !secretsReplicationEnabled
&& !string.Equals(secretsStoreProvider, "SqlServer", StringComparison.OrdinalIgnoreCase))
{
warnings.Add(
diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SecretsReplicationWiringTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SecretsReplicationWiringTests.cs
index 3a1b2213..03d4e601 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SecretsReplicationWiringTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SecretsReplicationWiringTests.cs
@@ -230,9 +230,15 @@ public class SecretsReplicationWiringTests
var ex = Assert.Throws(
() => services.AddScadaBridgeSecrets(config, SecretsNodeRole.Central));
- // Same helper, same message style as the Grpc-mode central check.
+ // Same helper, same message style as the Grpc-mode central check — but this path's
+ // message must name ITS OWN trigger (review round 1, finding #2), never claim
+ // Secrets:Replication:Mode=Grpc, which is not in effect here.
Assert.Contains(
SecretsRegistration.HubConnectionStringKey, ex.Message, StringComparison.Ordinal);
+ Assert.Contains(
+ $"{SecretsRegistration.StoreProviderKey}=SqlServer", ex.Message, StringComparison.Ordinal);
+ Assert.DoesNotContain(
+ $"{SecretsRegistration.ReplicationModeKey}=Grpc", ex.Message, StringComparison.Ordinal);
}
[Fact]
@@ -267,6 +273,32 @@ public class SecretsReplicationWiringTests
Assert.IsType(provider.GetRequiredService());
}
+ ///
+ /// Precedence pin (review round 1, finding #3): with BOTH triggers configured, the
+ /// pre-existing Secrets:Replication:Enabled=true Grpc-hub topology wins, and the new
+ /// Store:Provider=SqlServer branch defers to it rather than skipping hub registration. The
+ /// Store:Provider branch's own guard (!ReplicationEnabledKey) is what makes this so —
+ /// this test locks the ordering in against a future refactor of
+ /// that reorders the two branches.
+ ///
+ [Fact]
+ public void Central_ReplicationEnabledGrpcMode_TakesPrecedenceOverStoreProvider()
+ {
+ IConfiguration config = BuildConfig(
+ [
+ .. GrpcCentralConfig(),
+ ("Secrets:Store:Provider", "SqlServer"),
+ ]);
+
+ using ServiceProvider provider = BuildProvider(config, SecretsNodeRole.Central);
+
+ // Same shared SQL-Server store either way, but the Grpc hub is ALSO wired here — the
+ // Store:Provider-only path never registers the interceptor or the follower sweep.
+ Assert.IsType(provider.GetRequiredService());
+ Assert.NotNull(provider.GetService());
+ Assert.True(SecretsRegistration.UsesGrpcHub(config));
+ }
+
// ---------------------------------------------------------------------------------------
// SQL-Server mode — regression pins. Behaviour must be byte-for-byte what it was before the
// mode key existed, both when the mode is left unset and when it is named explicitly.
diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/StartupValidatorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/StartupValidatorTests.cs
index 3d5c3527..628fe73f 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/StartupValidatorTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/StartupValidatorTests.cs
@@ -771,6 +771,25 @@ public class StartupValidatorTests
Assert.Empty(warnings);
}
+ [Fact]
+ public void ClusteredCentral_ReplicationEnabled_NoSecretsStoreWarning()
+ {
+ // Review round 1, finding #1: Secrets:Replication:Enabled=true already puts this central
+ // node on a topology that shares state across the pair — the pre-existing gRPC hub
+ // (whose central-side store is the shared SQL-Server store, not per-node SQLite) or
+ // SQL-Server replication. No Secrets:Store:Provider is set at all here — the exact case
+ // that used to warn before this fix — and it must stay silent because the node is not
+ // actually on a per-node SQLite store in this configuration.
+ var values = ValidCentralConfig();
+ values["ScadaBridge:InboundApi:ApiKeyStore:Provider"] = "SqlServer";
+ values["Secrets:Replication:Enabled"] = "true";
+ var config = BuildConfig(values);
+
+ var warnings = StartupValidator.CollectWarnings(config);
+
+ Assert.Empty(warnings);
+ }
+
[Fact]
public void CollectWarnings_SilentForSingleSeedCentral()
{