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

fix(cluster): all four role-scoped validators read the authority — 'Driver' no longer skips every driver rule (archreview 01/C-9)

Browse Source
This commit is contained in:
Joseph Doherty
2026-08-20 09:51:23 -04:00
parent 792e8f422a
commit bcec88ef70
10 changed files with 172 additions and 88 deletions
Download Patch File Download Diff File Expand all files Collapse all files
archreview/plans/R3-04-one-role-authority-plan.md.tasks.json
+1 -1
View File
@@ -27,7 +27,7 @@
{
"id": "T4",
"subject": "All four role-scoped validators consume EffectiveRoles: SplitTopology/MeshTransport/Telemetry/ConfigSource (01/C-9)",
"status": "pending",
"status": "completed",
"blockedBy": [
"T2"
]
src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ConfigSourceOptionsValidator.cs
+12 -16
View File
@@ -15,18 +15,17 @@ namespace ZB.MOM.WW.OtOpcUa.Cluster;
/// </remarks>
public sealed class ConfigSourceOptionsValidator : IValidateOptions<ConfigSourceOptions>
{
private readonly IConfiguration _configuration;
private readonly EffectiveRoles _effectiveRoles;
/// <summary>
/// DI-constructed by <c>AddValidatedOptions</c> (a plain <c>AddSingleton</c>), so a
/// constructor dependency is safe here. <see cref="IConfiguration"/> — not
/// <c>IClusterRoleInfo</c> — is the source of this node's roles: <c>IClusterRoleInfo</c>'s
/// implementation needs the <c>ActorSystem</c>, which does not exist yet at
/// <c>ValidateOnStart</c> time.
/// constructor dependency is safe here. This validator's only use of configuration is this
/// node's roles, so it takes <see cref="EffectiveRoles"/> — the single role authority
/// (archreview R3-04) — directly rather than <see cref="IConfiguration"/>.
/// </summary>
public ConfigSourceOptionsValidator(IConfiguration configuration)
public ConfigSourceOptionsValidator(EffectiveRoles effectiveRoles)
{
_configuration = configuration;
_effectiveRoles = effectiveRoles;
}
/// <inheritdoc />
@@ -93,15 +92,12 @@ public sealed class ConfigSourceOptionsValidator : IValidateOptions<ConfigSource
// Per-cluster mesh Phase 4: a driver-only node has no central ConfigDb connection to fall back
// on, so Direct is only ever valid on a fused admin+driver node (admin holds the SQL
// connection). Roles come from configuration, not IClusterRoleInfo — see the ctor remark — and
// through RoleParser.ResolveEffectiveRoles so the OTOPCUA_ROLES fallback applies (Gitea #532).
// Reading Cluster:Roles alone exempted the exact node this rule exists for: an env-only
// driver-only node has no ConfigDb (Program.cs gates it on the same env var) yet saw an empty
// role list here and sailed through on Direct, to read its configuration from nothing.
var roles = RoleParser.ResolveEffectiveRoles(
_configuration.GetSection("Cluster:Roles").Get<string[]>());
var isDriver = Array.IndexOf(roles, RoleParser.Driver) >= 0;
var isAdmin = Array.IndexOf(roles, RoleParser.Admin) >= 0;
// connection). EffectiveRoles is the single role authority (archreview R3-04): its precedence
// already applies the OTOPCUA_ROLES fallback (Gitea #532) and its normalization already folds
// case, so neither an env-only driver-only node nor one spelled 'Driver' can sail through here
// on Direct to read its configuration from nothing.
var isDriver = _effectiveRoles.HasDriver;
var isAdmin = _effectiveRoles.HasAdmin;
if (isDriver && !isAdmin && isDirect)
{
src/Core/ZB.MOM.WW.OtOpcUa.Cluster/MeshTransportOptionsValidator.cs
+16 -17
View File
@@ -17,19 +17,21 @@ namespace ZB.MOM.WW.OtOpcUa.Cluster;
public sealed class MeshTransportOptionsValidator : IValidateOptions<MeshTransportOptions>
{
private readonly IConfiguration _configuration;
private readonly EffectiveRoles _effectiveRoles;
/// <summary>
/// DI-constructed by <c>AddValidatedOptions</c> (a plain <c>AddSingleton</c>), so a
/// constructor dependency is safe here. <see cref="IConfiguration"/> — not
/// <c>IClusterRoleInfo</c> — is the source of this node's roles: <c>IClusterRoleInfo</c>'s
/// implementation needs the <c>ActorSystem</c>, which does not exist yet at
/// <c>ValidateOnStart</c> time. Same shape as <see cref="ConfigSourceOptionsValidator"/> and
/// <see cref="TelemetryOptionsValidator"/>.
/// DI-constructed by <c>AddValidatedOptions</c> (a plain <c>AddSingleton</c>), so constructor
/// dependencies are safe here. <see cref="IConfiguration"/> is still needed for the
/// <c>ConfigServe:GrpcListenPort</c> cross-read (the admin-side obligation below) —
/// <see cref="EffectiveRoles"/> is the single role authority (archreview R3-04) this rule reads
/// roles through instead of re-deriving them.
/// </summary>
/// <param name="configuration">The application configuration, read for <c>Cluster:Roles</c>.</param>
public MeshTransportOptionsValidator(IConfiguration configuration)
/// <param name="configuration">The application configuration, read for <c>ConfigServe:GrpcListenPort</c>.</param>
/// <param name="effectiveRoles">This node's resolved effective roles.</param>
public MeshTransportOptionsValidator(IConfiguration configuration, EffectiveRoles effectiveRoles)
{
_configuration = configuration;
_effectiveRoles = effectiveRoles;
}
/// <inheritdoc />
@@ -68,15 +70,12 @@ public sealed class MeshTransportOptionsValidator : IValidateOptions<MeshTranspo
// follows it is a separate scope). Scoped to the driver role because an admin-only central node
// dials nodes rather than being dialled, and resolves per-cluster keys at runtime through
// MeshTransportOptions.ClusterPsks / the Secrets store, so requiring a single GrpcPsk and an
// endpoint list of it would demand configuration it never reads. Roles come from
// configuration, not IClusterRoleInfo (see the ctor remark), and through
// RoleParser.ResolveEffectiveRoles so the OTOPCUA_ROLES fallback applies (Gitea #532) — a node
// configured only through the env var is still the driver that has to dial central, and
// exempting it here would leave its acks rejected unauthenticated at runtime.
var roles = RoleParser.ResolveEffectiveRoles(
_configuration.GetSection("Cluster:Roles").Get<string[]>());
var isDriver = Array.IndexOf(roles, RoleParser.Driver) >= 0;
var isAdmin = Array.IndexOf(roles, RoleParser.Admin) >= 0;
// endpoint list of it would demand configuration it never reads. EffectiveRoles is the single
// role authority (archreview R3-04) — its precedence already applies the OTOPCUA_ROLES
// fallback and its normalization already folds case, so a node configured only through the env
// var, or spelled 'Driver', is still the driver that has to dial central.
var isDriver = _effectiveRoles.HasDriver;
var isAdmin = _effectiveRoles.HasAdmin;
if (isGrpc && isDriver)
{
src/Core/ZB.MOM.WW.OtOpcUa.Cluster/SplitTopologyTransportValidator.cs
+17 -35
View File
@@ -36,58 +36,40 @@ namespace ZB.MOM.WW.OtOpcUa.Cluster;
public sealed class SplitTopologyTransportValidator : IValidateOptions<MeshTransportOptions>
{
private readonly IConfiguration _configuration;
private readonly EffectiveRoles _effectiveRoles;
/// <summary>
/// DI-constructed by <c>AddValidatedOptions</c> (a plain <c>AddSingleton</c>), so a
/// constructor dependency is safe here. <see cref="IConfiguration"/> — not
/// <c>IClusterRoleInfo</c> — is the source of this node's roles and the three transport modes:
/// <c>IClusterRoleInfo</c>'s implementation needs the <c>ActorSystem</c>, which does not exist
/// yet at <c>ValidateOnStart</c> time. Reading the modes from configuration too (rather than
/// from three injected <c>IOptions</c>) mirrors how the sibling validators cross-read
/// <c>Cluster:Roles</c>, and avoids a service-resolution ordering dependency at validation
/// time.
/// DI-constructed by <c>AddValidatedOptions</c> (a plain <c>AddSingleton</c>), so constructor
/// dependencies are safe here. <see cref="IConfiguration"/> is still needed for the three
/// transport-mode cross-reads (<c>MeshTransport:Mode</c>, <c>Telemetry:Mode</c>,
/// <c>TelemetryDial:Mode</c>) — not <c>IClusterRoleInfo</c>, whose implementation needs the
/// <c>ActorSystem</c>, which does not exist yet at <c>ValidateOnStart</c> time.
/// <see cref="EffectiveRoles"/> is the single role authority (archreview R3-04) — this rule no
/// longer re-derives the effective, normalized role list itself.
/// </summary>
public SplitTopologyTransportValidator(IConfiguration configuration)
public SplitTopologyTransportValidator(IConfiguration configuration, EffectiveRoles effectiveRoles)
{
_configuration = configuration;
_effectiveRoles = effectiveRoles;
}
/// <inheritdoc />
public ValidateOptionsResult Validate(string? name, MeshTransportOptions options)
{
// The options instance is intentionally unused: every value this rule depends on is cross-read
// from IConfiguration, the same way ConfigSourceOptionsValidator reads Cluster:Roles.
//
// 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 when that 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. Until Gitea #532 this validator was the ONLY code that applied
// that fallback — it is now RoleParser.ResolveEffectiveRoles, shared with the
// AkkaClusterOptions PostConfigure that finally makes the option itself agree.
// (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
// 'Driver' / 'Admin' cannot slip past the ordinal driver / admin checks. (IsClusterRole
// itself is case-insensitive as of #532; the trim is still this rule's own business.)
var configured = _configuration.GetSection("Cluster:Roles").Get<string[]>() ?? Array.Empty<string>();
var effective = RoleParser.ResolveEffectiveRoles(configured);
var normalized = Array.ConvertAll(effective, r => r.Trim().ToLowerInvariant());
var clusterRoleIndex = Array.FindIndex(normalized, RoleParser.IsClusterRole);
if (clusterRoleIndex < 0)
// from IConfiguration (the three transport modes) or EffectiveRoles (this node's roles).
if (_effectiveRoles.ClusterRole is null)
{
// 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;
}
// 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;
// ClusterId keeps the operator's original spelling (EffectiveRoles preserves it verbatim by
// design — see EffectiveRoles' remarks) so the message names what they actually wrote.
var clusterId = _effectiveRoles.ClusterId;
var isDriver = _effectiveRoles.HasDriver;
var isAdmin = _effectiveRoles.HasAdmin;
var errors = new List<string>();
src/Core/ZB.MOM.WW.OtOpcUa.Cluster/TelemetryOptions.cs
+12 -14
View File
@@ -87,18 +87,17 @@ public sealed class TelemetryDialOptions
/// </remarks>
public sealed class TelemetryOptionsValidator : IValidateOptions<TelemetryOptions>
{
private readonly IConfiguration _configuration;
private readonly EffectiveRoles _effectiveRoles;
/// <summary>
/// DI-constructed by <c>AddValidatedOptions</c> (a plain <c>AddSingleton</c>), so a
/// constructor dependency is safe here. <see cref="IConfiguration"/> — not
/// <c>IClusterRoleInfo</c> — is the source of this node's roles: <c>IClusterRoleInfo</c>'s
/// implementation needs the <c>ActorSystem</c>, which does not exist yet at
/// <c>ValidateOnStart</c> time.
/// constructor dependency is safe here. This validator's only use of configuration is this
/// node's roles, so it takes <see cref="EffectiveRoles"/> — the single role authority
/// (archreview R3-04) — directly rather than <see cref="IConfiguration"/>.
/// </summary>
public TelemetryOptionsValidator(IConfiguration configuration)
public TelemetryOptionsValidator(EffectiveRoles effectiveRoles)
{
_configuration = configuration;
_effectiveRoles = effectiveRoles;
}
/// <inheritdoc />
@@ -118,12 +117,11 @@ public sealed class TelemetryOptionsValidator : IValidateOptions<TelemetryOption
+ $"Expected '{TelemetryOptions.ModeDps}' or '{TelemetryOptions.ModeGrpc}'.");
}
// Through ResolveEffectiveRoles so the OTOPCUA_ROLES fallback applies (Gitea #532) — a node
// configured only through the env var is still a driver node and must satisfy the Grpc-mode
// requirements below.
var roles = RoleParser.ResolveEffectiveRoles(
_configuration.GetSection("Cluster:Roles").Get<string[]>());
var isDriver = Array.IndexOf(roles, RoleParser.Driver) >= 0;
// EffectiveRoles is the single role authority (archreview R3-04) — its precedence already
// applies the OTOPCUA_ROLES fallback (Gitea #532) and its normalization already folds case, so
// a node configured only through the env var, or spelled 'Driver', still satisfies the
// Grpc-mode requirements below.
var isDriver = _effectiveRoles.HasDriver;
if (isGrpc)
{
@@ -137,7 +135,7 @@ public sealed class TelemetryOptionsValidator : IValidateOptions<TelemetryOption
+ $"{TelemetryOptions.SectionName}:{nameof(TelemetryOptions.GrpcListenPort)}.");
}
if (roles.Length > 0 && string.IsNullOrEmpty(options.ApiKey))
if (_effectiveRoles.Gossiped.Count > 0 && string.IsNullOrEmpty(options.ApiKey))
{
errors.Add(
$"{TelemetryOptions.SectionName}:{nameof(TelemetryOptions.ApiKey)} is empty. Under "
tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/ConfigSourceOptionsValidatorTests.cs
+2 -1
View File
@@ -23,7 +23,8 @@ public class ConfigSourceOptionsValidatorTests
var configuration = new ConfigurationBuilder().AddInMemoryCollection(pairs).Build();
return new ConfigSourceOptionsValidator(configuration).Validate(ConfigSourceOptions.SectionName, o);
return new ConfigSourceOptionsValidator(EffectiveRoles.Resolve(configuration))
.Validate(ConfigSourceOptions.SectionName, o);
}
private static ConfigSourceOptions ValidFetch() => new()
tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/MeshTransportOptionsValidatorTests.cs
+1 -1
View File
@@ -39,7 +39,7 @@ public class MeshTransportOptionsValidatorTests
var configuration = new ConfigurationBuilder().AddInMemoryCollection(pairs).Build();
return new MeshTransportOptionsValidator(configuration)
return new MeshTransportOptionsValidator(configuration, EffectiveRoles.Resolve(configuration))
.Validate(MeshTransportOptions.SectionName, o);
}
tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/RoleCaseVariantValidatorTests.cs
+107
View File
@@ -0,0 +1,107 @@
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Options;
using Shouldly;
using Xunit;
namespace ZB.MOM.WW.OtOpcUa.Cluster.Tests;
/// <summary>
/// R3-04 (01/C-9): a mixed-case role must fire the SAME fail-fast rule the canonical spelling
/// fires, on ALL FOUR role-scoped validators. Before the fix, three of the four
/// (<see cref="MeshTransportOptionsValidator"/>, <see cref="ConfigSourceOptionsValidator"/>,
/// <see cref="TelemetryOptionsValidator"/>) compared roles ordinally and silently PASSED a
/// misconfigured node spelled <c>Driver</c>/<c>Admin</c> — the load-bearing assertion in every
/// case below is the FAILURE, not the pass, because a validator whose driver/admin branch never
/// runs also "passes" vacuously.
/// </summary>
/// <remarks>
/// <see cref="SplitTopologyTransportValidator"/> already normalized before this fix (its own
/// comment names the hazard) — its case here is the BUILT-IN falsification control: it must have
/// been passing (i.e. correctly failing the misconfigured node) even before T4 landed, proving
/// the Theory shape itself is capable of catching a real regression.
/// </remarks>
[Collection(RoleEnvironmentCollection.Name)]
public sealed class RoleCaseVariantValidatorTests
{
public static readonly TheoryData<string> DriverSpellings = new()
{
"driver", "Driver", " driver ", "DRIVER",
};
private static IConfiguration Config(params (string Key, string Value)[] pairs) =>
new ConfigurationBuilder()
.AddInMemoryCollection(pairs.ToDictionary(p => p.Key, p => (string?)p.Value))
.Build();
// ---- SplitTopologyTransportValidator: the built-in control (already normalized pre-fix) -------
[Theory]
[MemberData(nameof(DriverSpellings))]
public void SplitTopology_fails_on_Dps_mesh_transport_for_every_driver_spelling(string driverSpelling)
{
var configuration = Config(
("Cluster:Roles:0", driverSpelling),
("Cluster:Roles:1", "cluster-SITE-A"),
("MeshTransport:Mode", MeshTransportOptions.ModeDps),
("Telemetry:Mode", TelemetryOptions.ModeGrpc));
var result = new SplitTopologyTransportValidator(configuration, EffectiveRoles.Resolve(configuration))
.Validate(MeshTransportOptions.SectionName, new MeshTransportOptions());
result.Failed.ShouldBeTrue();
result.FailureMessage.ShouldContain("MeshTransport:Mode");
}
// ---- MeshTransportOptionsValidator: GrpcPsk required for a driver node under Grpc mode --------
[Theory]
[MemberData(nameof(DriverSpellings))]
public void MeshTransport_Grpc_driver_node_without_a_psk_fails_for_every_driver_spelling(string driverSpelling)
{
var configuration = Config(("Cluster:Roles:0", driverSpelling), ("Cluster:Roles:1", "cluster-SITE-A"));
var options = new MeshTransportOptions
{
Mode = MeshTransportOptions.ModeGrpc,
GrpcPsk = null,
CentralGrpcEndpoints = ["http://central-1:4055"],
};
var result = new MeshTransportOptionsValidator(configuration, EffectiveRoles.Resolve(configuration))
.Validate(MeshTransportOptions.SectionName, options);
result.Failed.ShouldBeTrue();
result.FailureMessage.ShouldContain("MeshTransport:GrpcPsk");
}
// ---- ConfigSourceOptionsValidator: a driver-only node on Direct fails ---------------------------
[Theory]
[MemberData(nameof(DriverSpellings))]
public void ConfigSource_driver_only_node_on_Direct_fails_for_every_driver_spelling(string driverSpelling)
{
var configuration = Config(("Cluster:Roles:0", driverSpelling));
var options = new ConfigSourceOptions { Mode = ConfigSourceOptions.ModeDirect };
var result = new ConfigSourceOptionsValidator(EffectiveRoles.Resolve(configuration))
.Validate(ConfigSourceOptions.SectionName, options);
result.Failed.ShouldBeTrue();
result.FailureMessage.ShouldContain(ConfigSourceOptions.ModeFetchAndCache);
}
// ---- TelemetryOptionsValidator: a driver node under Grpc mode needs a listen port ---------------
[Theory]
[MemberData(nameof(DriverSpellings))]
public void Telemetry_Grpc_driver_node_with_no_listen_port_fails_for_every_driver_spelling(string driverSpelling)
{
var configuration = Config(("Cluster:Roles:0", driverSpelling));
var options = new TelemetryOptions { Mode = TelemetryOptions.ModeGrpc, ApiKey = "k" };
var result = new TelemetryOptionsValidator(EffectiveRoles.Resolve(configuration))
.Validate(TelemetryOptions.SectionName, options);
result.Failed.ShouldBeTrue();
result.FailureMessage.ShouldContain(nameof(TelemetryOptions.GrpcListenPort));
}
}
tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/SplitTopologyTransportValidatorTests.cs
+2 -2
View File
@@ -35,7 +35,7 @@ public class SplitTopologyTransportValidatorTests
// The validated options instance is never read (all four values come from IConfiguration), so a
// default MeshTransportOptions is a fine stand-in.
return new SplitTopologyTransportValidator(configuration)
return new SplitTopologyTransportValidator(configuration, EffectiveRoles.Resolve(configuration))
.Validate(MeshTransportOptions.SectionName, new MeshTransportOptions());
}
@@ -246,7 +246,7 @@ public class SplitTopologyTransportValidatorTests
var pairs = new Dictionary<string, string?> { ["MeshTransport:Mode"] = MeshTransportOptions.ModeDps };
var configuration = new ConfigurationBuilder().AddInMemoryCollection(pairs).Build();
var result = new SplitTopologyTransportValidator(configuration)
var result = new SplitTopologyTransportValidator(configuration, EffectiveRoles.Resolve(configuration))
.Validate(MeshTransportOptions.SectionName, new MeshTransportOptions());
result.Failed.ShouldBeTrue();
tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/TelemetryOptionsValidatorTests.cs
+2 -1
View File
@@ -23,7 +23,8 @@ public class TelemetryOptionsValidatorTests
var configuration = new ConfigurationBuilder().AddInMemoryCollection(pairs).Build();
return new TelemetryOptionsValidator(configuration).Validate(TelemetryOptions.SectionName, o);
return new TelemetryOptionsValidator(EffectiveRoles.Resolve(configuration))
.Validate(TelemetryOptions.SectionName, o);
}
[Fact]