From 4380e2e78a5051c9f929035294251e5c49156eb9 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:11:25 -0400 Subject: [PATCH] fix(cluster): fail start when a late config layer changes Cluster:Roles (role overlays may not set it) --- ...3-04-one-role-authority-plan.md.tasks.json | 2 +- .../AkkaClusterOptionsValidator.cs | 42 ++++++++++++++++ .../AkkaClusterOptionsValidatorTests.cs | 49 +++++++++++++++++++ 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/archreview/plans/R3-04-one-role-authority-plan.md.tasks.json b/archreview/plans/R3-04-one-role-authority-plan.md.tasks.json index 319a4707..b1fb0428 100644 --- a/archreview/plans/R3-04-one-role-authority-plan.md.tasks.json +++ b/archreview/plans/R3-04-one-role-authority-plan.md.tasks.json @@ -51,7 +51,7 @@ { "id": "T7", "subject": "AkkaClusterOptionsValidator fails start when a late config layer changes Cluster:Roles (role overlays may not set it)", - "status": "pending", + "status": "completed", "blockedBy": [ "T2" ] diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/AkkaClusterOptionsValidator.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/AkkaClusterOptionsValidator.cs index 461befa6..ce683d76 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/AkkaClusterOptionsValidator.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/AkkaClusterOptionsValidator.cs @@ -36,12 +36,28 @@ namespace ZB.MOM.WW.OtOpcUa.Cluster; /// public sealed class AkkaClusterOptionsValidator : OptionsValidatorBase { + private readonly EffectiveRoles? _effectiveRoles; + + /// + /// DI-constructed by AddValidatedOptions (a plain AddSingleton), so a + /// constructor dependency is safe here. is optional (default + /// ) so a caller with no role-authority instance to hand — including + /// every test in this file that pre-dates R3-04 — keeps constructing this validator + /// parameterlessly; the post-overlay guard below (choice 4) is simply skipped when it is null. + /// + /// The registered role authority, or null to skip the post-overlay guard. + public AkkaClusterOptionsValidator(EffectiveRoles? effectiveRoles = null) + { + _effectiveRoles = effectiveRoles; + } + /// protected override void Validate(ValidationBuilder builder, AkkaClusterOptions options) { ArgumentNullException.ThrowIfNull(options); ValidateBootstrapGuard(builder, options.BootstrapGuard); + ValidateAgainstEffectiveRoles(builder, options); var seeds = options.SeedNodes ?? Array.Empty(); if (seeds.Length == 0) @@ -89,6 +105,32 @@ public sealed class AkkaClusterOptionsValidator : OptionsValidatorBase 0 when the guard is enabled (was {guard.ProbeConnectTimeoutMs})."); } + /// + /// R3-04 (choice 4, the residual divergence surface): Program.cs resolves + /// BEFORE the per-role appsettings overlay loads (the overlay's + /// filename depends on the answer), so if a role overlay — or a ${secret:} expansion — + /// ever set Cluster:Roles itself, the bound .Roles (which + /// sees the FULL, post-overlay configuration) would silently disagree with the + /// instance every other consumer holds. Compares as SETS + /// (case-insensitively) rather than sequence, because ordering carries no meaning here and a + /// re-ordering is not the hazard this rule exists to catch — a genuinely different role set + /// is. A no-op today: none of the four shipped overlays carries a Cluster section. + /// + private void ValidateAgainstEffectiveRoles(ValidationBuilder builder, AkkaClusterOptions options) + { + if (_effectiveRoles is null) return; + + var bound = new HashSet(options.Roles ?? Array.Empty(), StringComparer.OrdinalIgnoreCase); + var registered = new HashSet(_effectiveRoles.Gossiped, StringComparer.OrdinalIgnoreCase); + if (bound.SetEquals(registered)) return; + + builder.Add( + "Cluster:Roles was changed by a configuration layer applied after role resolution " + + $"(host view: [{string.Join(",", _effectiveRoles.Gossiped)}], bound: " + + $"[{string.Join(",", options.Roles ?? Array.Empty())}]). A per-role appsettings " + + "overlay may not set Cluster:Roles."); + } + /// /// True when addresses this node itself — host AND port. /// diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/AkkaClusterOptionsValidatorTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/AkkaClusterOptionsValidatorTests.cs index 8114898c..91140bf4 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/AkkaClusterOptionsValidatorTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/AkkaClusterOptionsValidatorTests.cs @@ -214,4 +214,53 @@ public sealed class AkkaClusterOptionsValidatorTests result.Failed.ShouldBeFalse(result.FailureMessage); } + + // ---- R3-04 choice 4: the post-overlay config-layer guard -------------------------------------- + // A role overlay (or a ${secret:} expansion) may not itself set Cluster:Roles — Program.cs + // resolves EffectiveRoles BEFORE the overlay loads (it needs the answer to pick the overlay's + // filename), so if a later config layer changed Cluster:Roles the bound options would disagree + // with every consumer holding the registered EffectiveRoles instance. + + [Fact] + public void A_config_layer_that_changes_Cluster_Roles_after_resolution_fails_naming_both_views() + { + var effectiveRoles = EffectiveRoles.Resolve(["driver"], null); + var options = CentralNode("site-a-1", Seed("site-a-1")); + options.Roles = new[] { "admin", "driver" }; // the post-overlay bound value disagrees + + var result = new AkkaClusterOptionsValidator(effectiveRoles).Validate(null, options); + + result.Failed.ShouldBeTrue(); + result.FailureMessage.ShouldContain("Cluster:Roles"); + result.FailureMessage.ShouldContain("driver"); + result.FailureMessage.ShouldContain("admin"); + } + + [Fact] + public void Bound_Roles_matching_the_registered_EffectiveRoles_passes() + { + var effectiveRoles = EffectiveRoles.Resolve(["driver", "cluster-SITE-A"], null); + var options = CentralNode("site-a-1", Seed("site-a-1")); + options.Roles = new[] { "driver", "cluster-SITE-A" }; + + var result = new AkkaClusterOptionsValidator(effectiveRoles).Validate(null, options); + + result.Failed.ShouldBeFalse(result.FailureMessage); + } + + /// + /// A test that never injects — the pre-T7 construction shape — + /// must keep passing. Without this, giving the ctor a required parameter would silently break + /// every other test in this file rather than a chosen few. + /// + [Fact] + public void No_EffectiveRoles_injected_skips_the_guard_entirely() + { + var options = CentralNode("site-a-1", Seed("site-a-1")); + options.Roles = new[] { "whatever-this-does-not-validate-roles-either" }; + + var result = new AkkaClusterOptionsValidator().Validate(null, options); + + result.Failed.ShouldBeFalse(result.FailureMessage); + } }