dohertj2/lmxopcua
1
0
Fork 0
Code Issues 12 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(cluster): validate split topology against EFFECTIVE roles, not raw Cluster:Roles

Browse Source 
Code review found SplitTopologyTransportValidator read the wrong role view,
defeating its fail-loud purpose two ways:
 (a) Roles-source divergence. AkkaClusterOptions.Roles binds Cluster:Roles
     but falls back to OTOPCUA_ROLES (RoleParser.Parse) when it is empty. A
     node configured with only OTOPCUA_ROLES=driver,cluster-SITE-A genuinely
     carries the cluster role in Akka, yet the validator saw no cluster role
     and passed silently on MeshTransport:Mode=Dps (the documented incident).
 (b) Case. The Cluster:Roles bind path is verbatim while RoleParser.Parse
     lowercases the OTOPCUA_ROLES path; 'Cluster-SITE-A' / 'Driver' / 'Admin'
     slipped past the ordinal checks.

Resolve effective roles the same way the node does — Cluster:Roles, else
RoleParser.Parse(env OTOPCUA_ROLES) exactly as Program.cs — then normalize
both (trim + ToLowerInvariant) before IsClusterRole/driver/admin. The message
still names the ClusterId in the operator's original spelling. Exemption and
all other behaviour unchanged. Contained to the validator.

Tests: OTOPCUA_ROLES-only cluster role on Dps fails; mixed-case roles on Dps
fail; cluster role with neither admin nor driver needs only ClusterClient;
Cluster:Roles wins over a stray OTOPCUA_ROLES. 16/16 green (124/124 project).

Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
Joseph Doherty
2026-07-24 02:29:41 -04:00
parent a886d5e6e0
commit 344b0d3334
2 changed files with 108 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Core/ZB.MOM.WW.OtOpcUa.Cluster/SplitTopologyTransportValidator.cs
+28 -8
View File
@@ -21,8 +21,10 @@ namespace ZB.MOM.WW.OtOpcUa.Cluster;
/// </para>
/// <para>
/// <b>The rule is conditional and must stay that way.</b> It fires only when this node's
/// <c>Cluster:Roles</c> contains at least one cluster-scoped role. A legacy / single-mesh /
/// test node has none, so DPS is still legitimate and the validator passes unconditionally.
/// <i>effective</i> Akka roles contain at least one cluster-scoped role — resolved the same
/// way the node resolves them (<c>Cluster:Roles</c>, falling back to <c>OTOPCUA_ROLES</c> when
/// that is empty, normalized case-insensitively). A legacy / single-mesh / test node has none,
/// so DPS is still legitimate and the validator passes unconditionally.
/// </para>
/// <para>
/// <b>Not validated here:</b> <c>ConfigSource:Mode</c>. Phase 4's
@@ -55,19 +57,37 @@ public sealed class SplitTopologyTransportValidator : IValidateOptions<MeshTrans
{
// The options instance is intentionally unused: every value this rule depends on is cross-read
// from IConfiguration, the same way ConfigSourceOptionsValidator reads Cluster:Roles.
var roles = _configuration.GetSection("Cluster:Roles").Get<string[]>() ?? Array.Empty<string>();
//
// Resolve the EFFECTIVE Akka roles exactly as the node does, or this "fail loud" rule fires on
// the wrong role view and passes silently on the two shapes it exists to catch:
// (a) Roles-source divergence. AkkaClusterOptions.Roles binds Cluster:Roles but falls back to
// OTOPCUA_ROLES (via RoleParser.Parse — the same call Program.cs makes) when Cluster:Roles
// is empty. A node configured with only OTOPCUA_ROLES=driver,cluster-SITE-A genuinely
// carries the cluster role in Akka, so it must be validated too.
// (b) Case. The Cluster:Roles bind path is verbatim (no lowercasing), whereas RoleParser.Parse
// lowercases the OTOPCUA_ROLES path. Normalize BOTH here (trim + ToLowerInvariant) so
// 'Cluster-SITE-A' / 'Driver' / 'Admin' cannot slip past the ordinal IsClusterRole /
// driver / admin checks.
var configured = _configuration.GetSection("Cluster:Roles").Get<string[]>() ?? Array.Empty<string>();
var effective = configured.Length > 0
? configured
: RoleParser.Parse(Environment.GetEnvironmentVariable("OTOPCUA_ROLES"));
var clusterRole = Array.Find(roles, RoleParser.IsClusterRole);
if (clusterRole is null)
var normalized = Array.ConvertAll(effective, r => r.Trim().ToLowerInvariant());
var clusterRoleIndex = Array.FindIndex(normalized, RoleParser.IsClusterRole);
if (clusterRoleIndex < 0)
{
// No cluster-{ClusterId} role → not the split topology → DPS is legitimate. This is the
// exemption that keeps the validator a no-op for every legacy / single-mesh / test node.
return ValidateOptionsResult.Success;
}
var clusterId = RoleParser.ClusterIdFromRole(clusterRole);
var isDriver = Array.IndexOf(roles, RoleParser.Driver) >= 0;
var isAdmin = Array.IndexOf(roles, RoleParser.Admin) >= 0;
// Classification is normalized (above); the ClusterId in the message is taken from the
// operator's original spelling so it names what they actually wrote.
var clusterId = RoleParser.ClusterIdFromRole(effective[clusterRoleIndex].Trim());
var isDriver = Array.IndexOf(normalized, RoleParser.Driver) >= 0;
var isAdmin = Array.IndexOf(normalized, RoleParser.Admin) >= 0;
var errors = new List<string>();
tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/SplitTopologyTransportValidatorTests.cs
+80
View File
@@ -184,4 +184,84 @@ public class SplitTopologyTransportValidatorTests
telemetryDialMode: "grpc",
"admin", "driver", "cluster-MAIN").Succeeded.ShouldBeTrue();
}
[Fact]
public void Cluster_role_from_OTOPCUA_ROLES_only_on_Dps_fails()
{
// Roles-source divergence: Cluster:Roles is empty, so the node's effective Akka roles come from
// OTOPCUA_ROLES (via RoleParser.Parse) exactly as Program.cs resolves them. A validator that
// read only Cluster:Roles would see no cluster role and pass silently on Dps — the documented
// production-incident shape.
using (new EnvVarScope("OTOPCUA_ROLES", "driver,cluster-SITE-A"))
{
var pairs = new Dictionary<string, string?> { ["MeshTransport:Mode"] = MeshTransportOptions.ModeDps };
var configuration = new ConfigurationBuilder().AddInMemoryCollection(pairs).Build();
var result = new SplitTopologyTransportValidator(configuration)
.Validate(MeshTransportOptions.SectionName, new MeshTransportOptions());
result.Failed.ShouldBeTrue();
result.FailureMessage.ShouldContain("MeshTransport:Mode");
}
}
[Fact]
public void Cluster_role_from_OTOPCUA_ROLES_is_ignored_when_Cluster_Roles_is_set()
{
// When Cluster:Roles is populated it wins outright — the OTOPCUA_ROLES fallback is not consulted,
// mirroring AkkaClusterOptions.Roles. A non-cluster Cluster:Roles therefore stays exempt even if
// a stray OTOPCUA_ROLES carries a cluster role.
using (new EnvVarScope("OTOPCUA_ROLES", "driver,cluster-SITE-A"))
{
Validate(
meshMode: MeshTransportOptions.ModeDps,
telemetryMode: MeshTransportOptions.ModeDps,
telemetryDialMode: MeshTransportOptions.ModeDps,
"admin", "driver").Succeeded.ShouldBeTrue();
}
}
[Fact]
public void Mixed_case_cluster_and_role_names_on_Dps_fail()
{
// Case-sensitivity: the Cluster:Roles bind path is verbatim, so mixed-case entries must be
// normalized here or they slip past the ordinal IsClusterRole / driver checks and pass silently.
var result = Validate(
meshMode: MeshTransportOptions.ModeDps,
telemetryMode: TelemetryOptions.ModeGrpc,
telemetryDialMode: null,
"Driver", "Cluster-SITE-A");
result.Failed.ShouldBeTrue();
result.FailureMessage.ShouldContain("MeshTransport:Mode");
}
[Fact]
public void Cluster_role_node_with_neither_admin_nor_driver_only_needs_ClusterClient()
{
// A cluster-role node that is neither admin nor driver has no telemetry surface to constrain —
// only the always-on MeshTransport=ClusterClient rule applies. Telemetry/TelemetryDial left on
// their Dps defaults must not fail it.
Validate(
meshMode: MeshTransportOptions.ModeClusterClient,
telemetryMode: null,
telemetryDialMode: null,
"cluster-SITE-A").Succeeded.ShouldBeTrue();
}
/// <summary>Sets an environment variable for the scope's lifetime and restores its prior value on dispose.</summary>
private sealed class EnvVarScope : IDisposable
{
private readonly string _name;
private readonly string? _previous;
public EnvVarScope(string name, string? value)
{
_name = name;
_previous = Environment.GetEnvironmentVariable(name);
Environment.SetEnvironmentVariable(name, value);
}
public void Dispose() => Environment.SetEnvironmentVariable(_name, _previous);
}
}