fix(cluster,acl): OTOPCUA_ROLES fallback + one string-identity rule (#532, #533)

Two independent defects surfaced in review of #520 stage 3.

#532 — Cluster:Roles had no OTOPCUA_ROLES fallback, despite two doc sites
promising one. A node configured only through the env var — which drives ALL of
Program.cs's conditional wiring, so it reads as the authoritative knob — bound an
empty Roles array and joined the cluster carrying no Akka roles at all. It ran,
served, and logged nothing wrong, while central reported "no driver-role nodes
are Up" and every deployment to it timed out. Not hypothetical: the field fix on
wonder-app-vd03 was to hand-add Cluster__Roles__0/1 to a Windows service's
registry environment. Anyone reading the code to diagnose that box would have
concluded from the XML doc that the fallback was already applied.

AddOtOpcUaCluster now post-configures AkkaClusterOptions through the shared
RoleParser.ResolveEffectiveRoles, so BuildClusterOptions (Akka's own member
roles), ClusterRoleInfo, the ControlPlane singleton scope and
ConfigDbApiKeyStore's ClusterId all see one answer. Two sibling validators
(ConfigSource, Telemetry) read Cluster:Roles directly and were exempting the
exact node they exist for — an env-only driver-only node has no ConfigDb and
could sit on ConfigSource:Mode=Direct unchallenged. They resolve the same way now.

Also collapsed the four open-coded `Roles.FirstOrDefault(IsClusterRole)` +
ClusterIdFromRole copies into RoleParser.ClusterIdFromRoles. They had drifted:
three classified the cluster role ordinally, so a verbatim-bound `Cluster-SITE-A`
yielded a null ClusterId — and a null ClusterId applies NO cluster filter, which
made #528's cross-cluster over-grant guard fail OPEN. IsClusterRole is now
case-insensitive; the ClusterId suffix keeps its authored spelling, because
normalizing it would silently rename the Akka role of a running deployment.

#533 — three string-identity rules in one authorization path. AclScopeRef used
default (ordinal) equality while the trie builder filtered ClusterId and hashed
principals OrdinalIgnoreCase; the deploy gate's cross-cluster check was ordinal
too, which was the worse half — it could reject a whole deploy over a ClusterId
case difference the runtime would have accepted. It only worked because resolver
and builder were handed the same NodeAcl instances.

The rule is now stated once, on AclScopeRef, and applied throughout:
identifiers (ScopeId, ClusterId, Principal) compare OrdinalIgnoreCase; path
segments compare Ordinal. AclScopePathResolver re-keys its own maps rather than
documenting a comparer contract for callers, so the two independent producers —
EF at the gate, artifact JSON on the node — cannot diverge from the outside.
PermissionTrieNode.Children stays Ordinal: those are browse names, and Speed /
speed are two nodes that can coexist under one device.

Split PermissionTrieBuildDiagnosticReason.RetiredScopeKind out of
UnresolvedScope. A legacy FolderSegment row and a typo'd id had the same message
and different remedies.

Falsified: five mutations, each reddening only its own guard — remove the
PostConfigure (3 tests), revert IsClusterRole to ordinal (3), revert AclScopeRef
equality (1), revert the resolver's map comparer (6), revert the gate's
comparisons (2), remove the FolderSegment short-circuit (1).

Test isolation: OTOPCUA_ROLES is process-global and xUnit runs classes in
parallel, so the three validator suites that now resolve effective roles share a
non-parallel collection. Without it, cases passing no roles would intermittently
observe another class's env var.

Full solution build clean; full suite green apart from two pre-existing failures
confirmed identical on 3a6b2a1d (PrimaryGateFailoverTests, and
DriverProbeRegistrationTests expecting 10 probe types where 12 now register)
plus the AbLegacy integration suite, which needs a docker fixture.

Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
This commit is contained in:
Joseph Doherty
2026-08-01 01:51:22 -04:00
parent 3a6b2a1daf
commit 06c56d7719
24 changed files with 708 additions and 85 deletions
@@ -48,10 +48,26 @@ public sealed class AkkaClusterOptions
public string[] SeedNodes { get; set; } = Array.Empty<string>();
/// <summary>
/// Cluster roles for this node. When empty the role list comes from
/// <c>OTOPCUA_ROLES</c> via <see cref="RoleParser"/>. Allowed values:
/// <c>admin</c>, <c>driver</c>, <c>dev</c>.
/// Cluster roles for this node. Allowed values: <c>admin</c>, <c>driver</c>, <c>dev</c>, plus one
/// per-cluster <c>cluster-{ClusterId}</c> role (per-cluster mesh, Phase 6).
/// </summary>
/// <remarks>
/// <para>
/// When this is empty the list falls back to the <c>OTOPCUA_ROLES</c> environment variable,
/// applied by <c>AddOtOpcUaCluster</c> through
/// <see cref="RoleParser.ResolveEffectiveRoles(IReadOnlyList{string}?)"/>. That fallback was
/// described here — and relied upon in the field — for a year before it was implemented; a
/// node configured only through the env var joined the cluster carrying no roles at all.
/// See Gitea #532 and <c>docs/deployments/wonder-app-vd03-makino-z-34184.md</c>.
/// </para>
/// <para>
/// The two sources normalize differently: values bound from <c>Cluster:Roles</c> are taken
/// verbatim, while the environment path is lower-cased by <see cref="RoleParser.Parse"/>.
/// Akka compares gossiped role names ordinally, so <b>configure both nodes of a redundant
/// pair through the same source</b> — a pair split across the two with a mixed-case
/// <c>cluster-{ClusterId}</c> would gossip two different role names and elect two singletons.
/// </para>
/// </remarks>
public string[] Roles { get; set; } = Array.Empty<string>();
/// <summary>
@@ -39,22 +39,26 @@ public sealed class ClusterRoleInfo : IClusterRoleInfo, IDisposable
// sharing a bind IP) stay distinct. Production hosts have unique DNS names so the port
// suffix is harmless redundancy.
_localNode = CommonsNodeId.Parse($"{options.Value.PublicHostname}:{options.Value.Port}");
_localRoles = new HashSet<string>(options.Value.Roles, StringComparer.Ordinal);
// OrdinalIgnoreCase (Gitea #532): Cluster:Roles binds verbatim, so a node authored with 'Driver'
// used to answer HasRole("driver") false while gossiping a role Akka would still match against
// its own ordinal comparison — the node was a driver to the cluster and not to itself.
_localRoles = new HashSet<string>(options.Value.Roles, StringComparer.OrdinalIgnoreCase);
// Derived from the node's OWN configured roles (not live Cluster.State) so this identity is
// available at host-build time, before the cluster forms. Iterate the ORIGINAL array — not
// _localRoles — because first-wins requires configuration order, which HashSet does not
// guarantee.
var clusterRoles = options.Value.Roles.Where(RoleParser.IsClusterRole).ToArray();
if (clusterRoles.Length > 0)
// guarantee. The OTOPCUA_ROLES fallback is already applied to the bound options by
// AddOtOpcUaCluster's PostConfigure, so nothing is re-resolved here.
var clusterRoles = RoleParser.ClusterRolesIn(options.Value.Roles);
if (clusterRoles.Count > 0)
{
_clusterRole = clusterRoles[0];
_clusterId = RoleParser.ClusterIdFromRole(_clusterRole);
if (clusterRoles.Length > 1)
if (clusterRoles.Count > 1)
{
_logger.LogWarning(
"Node configured with {Count} cluster-scoped roles ({Roles}); using the first ({ClusterRole})",
clusterRoles.Length, string.Join(", ", clusterRoles), _clusterRole);
clusterRoles.Count, string.Join(", ", clusterRoles), _clusterRole);
}
}
@@ -93,11 +93,15 @@ 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 Cluster:Roles directly, not IClusterRoleInfo — see the ctor
// remark.
var roles = _configuration.GetSection("Cluster:Roles").Get<string[]>() ?? Array.Empty<string>();
var isDriver = Array.IndexOf(roles, "driver") >= 0;
var isAdmin = Array.IndexOf(roles, "admin") >= 0;
// 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;
if (isDriver && !isAdmin && isDirect)
{
@@ -23,15 +23,110 @@ public static class RoleParser
/// <summary>Returns true when <paramref name="role"/> is a cluster-scoped role — starts with
/// <see cref="ClusterRolePrefix"/> and carries a non-empty ClusterId suffix.</summary>
/// <remarks>
/// The prefix test is <b>case-insensitive</b> (Gitea #532). It was ordinal, which meant a
/// <c>Cluster:Roles</c> entry spelled <c>Cluster-SITE-A</c> — that path binds verbatim, unlike
/// the <see cref="Parse"/> path, which lower-cases — classified as an ordinary role. The node
/// then resolved a <see langword="null"/> ClusterId while still gossiping a cluster-scoped role,
/// so it looked split-topology to its peers and un-scoped to itself.
/// <see cref="SplitTopologyTransportValidator"/> pre-normalized for exactly this reason;
/// <c>ClusterRoleInfo</c> did not, and got the null.
/// </remarks>
/// <param name="role">The role name to test.</param>
public static bool IsClusterRole(string role) =>
role.StartsWith(ClusterRolePrefix, StringComparison.Ordinal) && role.Length > ClusterRolePrefix.Length;
role is not null
&& role.StartsWith(ClusterRolePrefix, StringComparison.OrdinalIgnoreCase)
&& role.Length > ClusterRolePrefix.Length;
/// <summary>Extracts the ClusterId from a cluster-scoped role, e.g. <c>cluster-SITE-A</c> → <c>SITE-A</c>.
/// Assumes <see cref="IsClusterRole"/> is true for <paramref name="role"/>.</summary>
/// <remarks>
/// The suffix keeps the operator's own spelling. It is NOT case-normalized here because the two
/// role sources disagree by construction — <c>Cluster:Roles</c> binds verbatim
/// (<c>cluster-SITE-A</c> ⇒ <c>SITE-A</c>) while <see cref="Parse"/> lower-cases the
/// <c>OTOPCUA_ROLES</c> path (⇒ <c>site-a</c>) — and normalizing one side would silently rename
/// the Akka role of a running deployment (docker-dev's rig configures <c>cluster-MAIN</c> /
/// <c>cluster-SITE-A</c> verbatim today). Every consumer of a ClusterId therefore compares it
/// case-insensitively; see the identity rule on <c>AclScopeRef</c> (Gitea #533).
/// </remarks>
/// <param name="role">The cluster-scoped role name.</param>
public static string ClusterIdFromRole(string role) => role[ClusterRolePrefix.Length..];
/// <summary>
/// The cluster-scoped roles in <paramref name="roles"/>, in the order given. More than one is a
/// misconfiguration the caller is expected to report — a node belongs to exactly one mesh.
/// </summary>
/// <param name="roles">The node's roles.</param>
/// <returns>The cluster-scoped roles, original spelling preserved.</returns>
public static IReadOnlyList<string> ClusterRolesIn(IEnumerable<string>? roles) =>
roles is null ? Array.Empty<string>() : roles.Where(IsClusterRole).ToArray();
/// <summary>The first cluster-scoped role in <paramref name="roles"/>, or null when there is none.</summary>
/// <param name="roles">The node's roles.</param>
/// <returns>The cluster role, e.g. <c>cluster-SITE-A</c>.</returns>
public static string? ClusterRoleFromRoles(IEnumerable<string>? roles) =>
roles?.FirstOrDefault(IsClusterRole);
/// <summary>
/// The ClusterId this node belongs to, derived from its own roles; null when it carries no
/// cluster-scoped role (a legacy / single-mesh node).
/// </summary>
/// <remarks>
/// The single home for a derivation that was open-coded at four call sites before Gitea #532
/// (<c>ClusterRoleInfo</c>, the ControlPlane singleton scope, <c>SplitTopologyTransportValidator</c>
/// and <c>ConfigDbApiKeyStore</c>) — each with its own view of whether the classification was
/// case-sensitive, which is how the mixed-case gap above survived in three of the four.
/// </remarks>
/// <param name="roles">The node's roles.</param>
/// <returns>The ClusterId, or <see langword="null"/>.</returns>
public static string? ClusterIdFromRoles(IEnumerable<string>? roles)
{
var role = ClusterRoleFromRoles(roles);
return role is null ? null : ClusterIdFromRole(role);
}
/// <summary>
/// Resolves a node's <b>effective</b> Akka roles: <c>Cluster:Roles</c> when it is non-empty,
/// otherwise the <c>OTOPCUA_ROLES</c> environment variable through <see cref="Parse"/>.
/// </summary>
/// <remarks>
/// <para>
/// ⚠️ <b>This fallback was documented for a year before it existed</b> (Gitea #532).
/// <c>AkkaClusterOptions.Roles</c>' own XML doc and <see cref="SplitTopologyTransportValidator"/>
/// both described it; only the validator implemented it, privately. A node configured solely
/// through <c>OTOPCUA_ROLES</c> — which drives every bit of <c>Program.cs</c>'s conditional
/// wiring, so it reads as the authoritative knob — joined the cluster carrying <b>no Akka
/// roles at all</b>. It ran, served, and logged nothing wrong, while central reported "no
/// driver-role nodes are Up" and every deployment to it timed out. That is not hypothetical:
/// see <c>docs/deployments/wonder-app-vd03-makino-z-34184.md</c>, where the field fix was to
/// hand-add <c>Cluster__Roles__0/1</c> to a Windows service's registry environment.
/// </para>
/// <para>
/// <b>Configured wins outright</b> — a stray <c>OTOPCUA_ROLES</c> can never widen an explicit
/// <c>Cluster:Roles</c>. The two sources normalize differently on purpose (see
/// <see cref="ClusterIdFromRole"/>), so a redundant pair should be configured through the
/// same one.
/// </para>
/// </remarks>
/// <param name="configuredRoles">The bound <c>Cluster:Roles</c> value.</param>
/// <param name="envRoles">The raw <c>OTOPCUA_ROLES</c> value.</param>
/// <returns>The effective roles; empty when neither source names any.</returns>
/// <exception cref="ArgumentException">The environment value names an unknown role.</exception>
public static string[] ResolveEffectiveRoles(IReadOnlyList<string>? configuredRoles, string? envRoles)
{
if (configuredRoles is { Count: > 0 }) return [.. configuredRoles];
return Parse(envRoles);
}
/// <summary>Reads the effective roles for the current process, resolving the environment itself.</summary>
/// <param name="configuredRoles">The bound <c>Cluster:Roles</c> value.</param>
/// <returns>The effective roles.</returns>
public static string[] ResolveEffectiveRoles(IReadOnlyList<string>? configuredRoles) =>
ResolveEffectiveRoles(configuredRoles, Environment.GetEnvironmentVariable(RolesEnvVar));
/// <summary>The environment variable the role list falls back to when <c>Cluster:Roles</c> is empty.</summary>
public const string RolesEnvVar = "OTOPCUA_ROLES";
/// <summary>Parses a comma-separated string of role names into a validated array.</summary>
/// <param name="raw">The raw role string to parse.</param>
/// <returns>The distinct, lower-cased, validated role names.</returns>
@@ -34,6 +34,22 @@ public static class ServiceCollectionExtensions
services.AddValidatedOptions<AkkaClusterOptions, AkkaClusterOptionsValidator>(
configuration, AkkaClusterOptions.SectionName);
// Gitea #532: apply the OTOPCUA_ROLES fallback the Roles property has always DOCUMENTED, in the
// one place every consumer reads through. Before this, a node configured only through the env
// var — the knob that drives all of Program.cs's role wiring — bound an EMPTY Roles array and so
// joined the cluster role-less: invisible to RoleLeader/MembersWithRole, ineligible for every
// role-scoped singleton, and un-deployable ("no driver-role nodes are Up") with nothing in its
// own logs to say so. Doing it here rather than at each call site means BuildClusterOptions
// (Akka's own member roles), ClusterRoleInfo, the ControlPlane singleton scope and
// ConfigDbApiKeyStore's ClusterId all see the same answer.
//
// PostConfigure, not a bind trick: it runs after the section binds and before ValidateOnStart,
// so the validators below see the effective list. An unknown role in the env var throws out of
// RoleParser.Parse — the same throw Program.cs already takes on the same value, so an invalid
// env var cannot reach here on the production path in the first place.
services.PostConfigure<AkkaClusterOptions>(
options => options.Roles = RoleParser.ResolveEffectiveRoles(options.Roles));
// Per-cluster mesh Phase 2: which transport carries central→node commands. Validated at
// startup for the same reason the cluster options are — a contact point that cannot resolve
// produces silence, not an error, and silence is indistinguishable from "nothing to do".
@@ -61,17 +61,17 @@ public sealed class SplitTopologyTransportValidator : IValidateOptions<MeshTrans
// 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.
// 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
// 'Cluster-SITE-A' / 'Driver' / 'Admin' cannot slip past the ordinal IsClusterRole /
// driver / admin checks.
// '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 = configured.Length > 0
? configured
: RoleParser.Parse(Environment.GetEnvironmentVariable("OTOPCUA_ROLES"));
var effective = RoleParser.ResolveEffectiveRoles(configured);
var normalized = Array.ConvertAll(effective, r => r.Trim().ToLowerInvariant());
@@ -118,8 +118,12 @@ public sealed class TelemetryOptionsValidator : IValidateOptions<TelemetryOption
+ $"Expected '{TelemetryOptions.ModeDps}' or '{TelemetryOptions.ModeGrpc}'.");
}
var roles = _configuration.GetSection("Cluster:Roles").Get<string[]>() ?? Array.Empty<string>();
var isDriver = Array.IndexOf(roles, "driver") >= 0;
// 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;
if (isGrpc)
{
@@ -71,10 +71,16 @@ public static class DraftValidator
/// both places — the accepted cost of not inverting the project graph for one gate.
/// </para>
/// <para>
/// Existence is checked with the same ordinal, untrimmed comparison
/// Existence is checked with the same <b>case-insensitive, untrimmed</b> comparison
/// <c>AclScopePathResolver</c>'s dictionary lookups use, so a row this rule passes is a row that
/// resolver resolves. The realm check mirrors <c>NodeAclRealms.TryCanonicalize</c> instead
/// (trimmed, case-insensitive) for the same reason.
/// (trimmed, case-insensitive) for the same reason. Both this rule and the resolver were
/// ordinal until Gitea #533; the cross-cluster comparison below was ordinal too, which was the
/// worse half — it could reject a deploy as cross-cluster over a <c>ClusterId</c> the runtime
/// would have matched, and the two spellings genuinely occur (see
/// <c>RoleParser.ClusterIdFromRole</c>). The rule now is: identifiers compare
/// <c>OrdinalIgnoreCase</c>, path segments compare <c>Ordinal</c>; it is stated in full on
/// <c>AclScopeRef</c>.
/// </para>
/// <para>
/// <b>Containment is checked as well as existence.</b> A grant in cluster A naming an entity that
@@ -115,8 +121,8 @@ public static class DraftValidator
static Dictionary<string, string> Link<T>(
IReadOnlyList<T> rows, Func<T, string> key, Func<T, string> value) =>
rows.GroupBy(key, StringComparer.Ordinal)
.ToDictionary(g => g.Key, g => value(g.First()), StringComparer.Ordinal);
rows.GroupBy(key, StringComparer.OrdinalIgnoreCase)
.ToDictionary(g => g.Key, g => value(g.First()), StringComparer.OrdinalIgnoreCase);
// Owning cluster, or null when a link in the chain is dangling. Direct-ClusterId kinds bottom out
// immediately; the rest hop through their parent.
@@ -240,7 +246,7 @@ public static class DraftValidator
continue;
}
if (!string.Equals(owner, acl.ClusterId, StringComparison.Ordinal))
if (!string.Equals(owner, acl.ClusterId, StringComparison.OrdinalIgnoreCase))
errors.Add(new("NodeAclScopeCrossCluster",
$"NodeAcl '{id}' is a grant in cluster '{acl.ClusterId}' but is scoped at {acl.ScopeKind} " +
$"'{scopeId}', which belongs to cluster '{owner}'. The id is real, so the grant would resolve " +
@@ -8,15 +8,57 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Authorization;
/// Identifies a scope for path lookup. Keyed on the kind <b>as well as</b> the id.
/// </summary>
/// <remarks>
/// The kind is part of the key because <c>NodeAcl.ScopeId</c> is a bare string with no foreign key
/// and no type discriminator of its own — ids are prefixed by convention (<c>RF…</c>, <c>EQ…</c>)
/// but nothing enforces it. Keying on the id alone would let a <c>DeviceId</c> that happened to
/// equal a <c>UnsAreaId</c> hand a grant the other entity's paths, which is a silent
/// wrong-node authorization rather than a visible error.
/// <para>
/// The kind is part of the key because <c>NodeAcl.ScopeId</c> is a bare string with no foreign key
/// and no type discriminator of its own — ids are prefixed by convention (<c>RF…</c>, <c>EQ…</c>)
/// but nothing enforces it. Keying on the id alone would let a <c>DeviceId</c> that happened to
/// equal a <c>UnsAreaId</c> hand a grant the other entity's paths, which is a silent
/// wrong-node authorization rather than a visible error.
/// </para>
/// <para>
/// <b>The string-identity rule for the whole authorization path (Gitea #533), stated here because
/// this type is where it was first violated:</b>
/// </para>
/// <list type="bullet">
/// <item>
/// <b>Identifiers compare <c>OrdinalIgnoreCase</c></b> — <c>ScopeId</c>, <c>ClusterId</c>,
/// <c>Principal</c>. They are opaque handles, not addresses; two spellings of one id always
/// mean the same entity, and the sources genuinely differ in case (a <c>ClusterId</c> arrives
/// lower-cased through <c>OTOPCUA_ROLES</c> and verbatim through <c>Cluster:Roles</c> — see
/// <c>RoleParser.ClusterIdFromRole</c> — and a <c>ScopeId</c> is operator free-text in the ACL
/// editor until the stage-7 picker lands).
/// </item>
/// <item>
/// <b>Path segments compare <c>Ordinal</c></b> — every level of <see cref="NodeAclPath"/>,
/// and therefore <c>PermissionTrieNode.Children</c>. They are OPC UA browse names, where
/// <c>Speed</c> and <c>speed</c> are two different nodes that can coexist under one device
/// (<c>RawPaths.Comparer</c> is ordinal, so raw sibling-name uniqueness is too).
/// </item>
/// </list>
/// <para>
/// Before #533 this record struct used the compiler's default equality, i.e. ordinal, while the
/// trie builder filtered <c>ClusterId</c> and hashed principals <c>OrdinalIgnoreCase</c> — three
/// rules in one path. It worked only because the resolver and the builder were handed the same
/// <c>NodeAcl</c> instances, so the <c>ScopeId</c> strings were literally the same object. Feed
/// the two sides from different snapshots — which is the entire reason this resolver is
/// input-shape agnostic, EF on the gate and artifact JSON on the node — and any case drift drops
/// <b>every</b> grant on that scope. Fail-closed and diagnosed, but a silent loss of authority.
/// </para>
/// </remarks>
/// <param name="Kind">The authored scope kind.</param>
/// <param name="ScopeId">The authored scope id.</param>
public readonly record struct AclScopeRef(NodeAclScopeKind Kind, string ScopeId);
public readonly record struct AclScopeRef(NodeAclScopeKind Kind, string ScopeId)
{
/// <summary>Equality over the kind plus a case-insensitive id — see the rule on the type.</summary>
/// <param name="other">The other reference.</param>
/// <returns><see langword="true"/> when both name the same scope.</returns>
public bool Equals(AclScopeRef other) =>
Kind == other.Kind && string.Equals(ScopeId, other.ScopeId, StringComparison.OrdinalIgnoreCase);
/// <inheritdoc />
public override int GetHashCode() =>
HashCode.Combine(Kind, StringComparer.OrdinalIgnoreCase.GetHashCode(ScopeId ?? string.Empty));
}
/// <summary>
/// Produces the <c>scopePaths</c> lookup <see cref="PermissionTrieBuilder.Build"/> consumes: for each
@@ -88,21 +130,29 @@ public sealed class AclScopePathResolver
ArgumentNullException.ThrowIfNull(equipment);
ArgumentNullException.ThrowIfNull(references);
_folders = folders;
_drivers = drivers;
_devices = devices;
_groups = groups;
_tags = tags;
_areas = areas;
_lines = lines;
_equipment = equipment;
// Every map is re-keyed OrdinalIgnoreCase rather than used as handed in. Ids are identifiers, so
// they follow the identifier half of the rule on AclScopeRef (Gitea #533) — and doing it HERE,
// rather than documenting a comparer contract for callers, is what makes the rule impossible to
// get wrong from the outside: the two producers (EF entities at the deploy gate, artifact JSON on
// the node) build these dictionaries independently and neither can now diverge from the other.
_folders = ById(folders);
_drivers = ById(drivers);
_devices = ById(devices);
_groups = ById(groups);
_tags = ById(tags);
_areas = ById(areas);
_lines = ById(lines);
_equipment = ById(equipment);
_references = references;
// The projected group map goes in with the same comparer, so a raw ACL path and the RawPath that
// forms the node's NodeId still agree segment-for-segment — RawPathResolver emits each entity's
// OWN name, so a case-insensitive id lookup widens which chains resolve and never what they spell.
_rawPaths = new RawPathResolver(
folders,
drivers,
devices,
groups.ToDictionary(kv => kv.Key, kv => (kv.Value.ParentId, kv.Value.Name), StringComparer.Ordinal));
_folders,
_drivers,
_devices,
ById(groups.ToDictionary(kv => kv.Key, kv => (kv.Value.ParentId, kv.Value.Name))));
}
/// <summary>
@@ -172,8 +222,10 @@ public sealed class AclScopePathResolver
return TryGroupSegments(scopeId) is { } groupSegs ? [RawPath(groupSegs)] : [];
// Cluster never reaches here (the caller short-circuits it), and FolderSegment is the retired
// v2 kind with no v3 entity behind it — deliberately unresolvable, so an operator who still
// has such a row gets a diagnostic naming it rather than a grant that quietly does nothing.
// v2 kind with no v3 entity behind it — deliberately unresolvable. The trie builder now
// short-circuits FolderSegment too, so it reports RetiredScopeKind rather than the
// indistinguishable-from-a-typo UnresolvedScope (Gitea #533); this arm stays as the answer
// for a direct ResolveOne caller.
case NodeAclScopeKind.Cluster:
case NodeAclScopeKind.FolderSegment:
default:
@@ -197,7 +249,7 @@ public sealed class AclScopePathResolver
// grant has to cover all four or it is bypassable through whichever one it missed.
foreach (var reference in _references)
{
if (!string.Equals(reference.TagId, tagId, StringComparison.Ordinal)) continue;
if (!string.Equals(reference.TagId, tagId, StringComparison.OrdinalIgnoreCase)) continue;
if (TryEquipmentSegments(reference.EquipmentId) is not { } equipSegs) continue;
paths.Add(UnsPath([.. equipSegs, reference.EffectiveName]));
}
@@ -276,6 +328,17 @@ public sealed class AclScopePathResolver
return [.. deviceSegs, .. chain];
}
/// <summary>Re-keys an id map under the identifier comparer — see the rule on <see cref="AclScopeRef"/>.</summary>
private static Dictionary<string, TValue> ById<TValue>(IReadOnlyDictionary<string, TValue> source)
{
var copy = new Dictionary<string, TValue>(source.Count, StringComparer.OrdinalIgnoreCase);
// Last-wins on a case-collision, matching the GroupBy/First tolerance the deploy gate applies to
// duplicate ids: a duplicate is another rule's business, and throwing here would fault the whole
// decode over one malformed row.
foreach (var (key, value) in source) copy[key] = value;
return copy;
}
// No params-array overloads here: string[] binds to both params and IEnumerable, and the resulting
// ambiguity would be resolved silently by overload rules rather than by intent.
private static NodeAclPath RawPath(IEnumerable<string> segments) =>
@@ -69,6 +69,18 @@ public static class PermissionTrieBuilder
continue;
}
// Ahead of the ScopeId checks, mirroring the deploy gate's ordering (DraftValidator's
// NodeAclScopeKindRetired): the retirement is the actionable fact, and a row carrying this
// kind can never resolve whatever its ScopeId, so reporting it as "unresolved" would send an
// operator hunting for a typo in an id that was never the problem.
if (row.ScopeKind == NodeAclScopeKind.FolderSegment)
{
diagnostic?.Invoke(new PermissionTrieBuildDiagnostic(
row.NodeAclId, row.ScopeKind, row.ScopeId,
PermissionTrieBuildDiagnosticReason.RetiredScopeKind));
continue;
}
// A sub-cluster row with no ScopeId names nothing. It used to vanish here with no signal at
// all — unlike an unresolvable ScopeId, which at least emitted a diagnostic. Both are
// authoring mistakes and both now say so.
@@ -151,13 +163,23 @@ public enum PermissionTrieBuildDiagnosticReason
{
/// <summary>
/// The row's <c>ScopeId</c> did not resolve to any node in the deployed address space — a dangling
/// id, a deleted entity, or a scope kind with no v3 meaning (<see cref="NodeAclScopeKind.FolderSegment"/>).
/// id, or an entity someone deleted after the grant was authored. The remedy is to correct the id
/// or delete the grant.
/// </summary>
UnresolvedScope,
/// <summary>The row is narrower than <see cref="NodeAclScopeKind.Cluster"/> but carries no <c>ScopeId</c>.</summary>
MissingScopeId,
/// <summary>
/// The row's <see cref="NodeAclScopeKind"/> is retired and has no v3 entity behind it
/// (<see cref="NodeAclScopeKind.FolderSegment"/>), so it can never resolve whatever its
/// <c>ScopeId</c>. Split out of <see cref="UnresolvedScope"/> by Gitea #533: the two have
/// different remedies — re-scope the grant to <see cref="NodeAclScopeKind.RawFolder"/> versus
/// fix an id — and conflating them told an operator with a legacy row to go looking for a typo.
/// </summary>
RetiredScopeKind,
/// <summary>
/// A resolved path was empty or did not begin with a realm segment, so it addresses a trie node
/// the walk cannot reach. This is a bug in the scope-path producer, not an authoring mistake.
@@ -231,7 +231,7 @@ public static class ServiceCollectionExtensions
// on the fused central node). AkkaClusterOptions is pure configuration, available before the
// cluster forms — the same source ClusterRoleInfo.ClusterRole itself derives from — so the
// scope is identical. First cluster role wins, in configuration order (matching ClusterRoleInfo).
var clusterRole = clusterOptions.Roles.FirstOrDefault(RoleParser.IsClusterRole);
var clusterRole = RoleParser.ClusterRoleFromRoles(clusterOptions.Roles);
return new ClusterSingletonOptions { Role = clusterRole ?? RoleParser.Driver };
}
}
@@ -61,8 +61,12 @@ public sealed class ConfigDbApiKeyStore : IApiKeyStore
/// <remarks>
/// <para>
/// This is the same derivation <see cref="Cluster.ClusterRoleInfo"/> performs for
/// <c>IClusterRoleInfo.ClusterId</c>, reusing the same <see cref="RoleParser"/> primitives —
/// first cluster-scoped role wins, as it does there.
/// <c>IClusterRoleInfo.ClusterId</c> — literally so as of Gitea #532, which collapsed four
/// open-coded copies into <see cref="RoleParser.ClusterIdFromRoles"/>. The copies had drifted:
/// three classified the cluster role case-sensitively and one did not, and only one applied
/// the <c>OTOPCUA_ROLES</c> fallback. On a node that hit either gap this method returned
/// <see langword="null"/>, and a null ClusterId applies no filter at all — so the
/// cross-cluster over-grant this store exists to prevent failed <b>open</b>.
/// </para>
/// <para>
/// ⚠️ <b>Takes <see cref="AkkaClusterOptions"/> (pure configuration), NOT
@@ -79,8 +83,7 @@ public sealed class ConfigDbApiKeyStore : IApiKeyStore
{
ArgumentNullException.ThrowIfNull(options);
var clusterRole = options.Roles.FirstOrDefault(RoleParser.IsClusterRole);
return clusterRole is null ? null : RoleParser.ClusterIdFromRole(clusterRole);
return RoleParser.ClusterIdFromRoles(options.Roles);
}
/// <inheritdoc />