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 52d0a593..b203876b 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 @@ -5,7 +5,7 @@ { "id": "T1", "subject": "EffectiveRoles value type: one factory, one precedence (Cluster:Roles > OTOPCUA_ROLES), two-layer normalization, both-source validation (01/S-12, 01/C-9)", - "status": "pending", + "status": "completed", "blockedBy": [] }, { @@ -94,7 +94,7 @@ }, { "id": "T13", - "subject": "LIVE GATE: docker-dev six-node rig — pre-fix baseline, per-node banner vs /hosts gossip parity, site-a Primary election, deploy seals Applied incl. site-b-2, unknown-role fail-fast", + "subject": "LIVE GATE: docker-dev six-node rig \u2014 pre-fix baseline, per-node banner vs /hosts gossip parity, site-a Primary election, deploy seals Applied incl. site-b-2, unknown-role fail-fast", "status": "pending", "blockedBy": [ "T3", diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/EffectiveRoles.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/EffectiveRoles.cs new file mode 100644 index 00000000..c06dc4c0 --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/EffectiveRoles.cs @@ -0,0 +1,164 @@ +using Microsoft.Extensions.Configuration; + +namespace ZB.MOM.WW.OtOpcUa.Cluster; + +/// Which of the two role sources resolved from. +public enum RoleSource +{ + /// Neither source named any role. + None, + + /// Cluster:Roles was non-empty and won outright. + Configured, + + /// Cluster:Roles was empty; OTOPCUA_ROLES supplied the roles. + Environment, +} + +/// The node's effective cluster roles — the SINGLE authority for "what is this node". +/// Resolved once, from one documented precedence, and consumed by BOTH the Host's conditional +/// wiring and the Akka role list, so the two cannot disagree (archreview 01/S-12 = 03/C5). +/// +/// Precedence: Cluster:Roles when non-empty, else OTOPCUA_ROLES, +/// else empty. Unchanged from +/// — what changed is that Program.cs now obeys it too, instead of reading the environment +/// directly. +/// Normalization is two-layer, on purpose. lower-cases plain +/// roles (Akka matches gossiped role names ORDINALLY, and every consumer compares against a +/// lowercase constant) but keeps a cluster role's ClusterId suffix VERBATIM — lower-casing that +/// would rename cluster-SITE-A on a running fleet, and a rolling pair upgrade would gossip +/// two different role names mid-window. Lower-casing a plain role changes the wire only for a +/// node that is already broken (archreview 01/S-12 = 03/C5, 01/C-9). +/// Validation runs on BOTH sources. The environment path was always validated by +/// ; the configured path previously accepted anything verbatim +/// (Cluster:Roles: ["Drivr"] silently gossiped a role that matched nothing). Both now fail +/// fast, naming the offending key, on an unknown role — a miscased but KNOWN role still +/// normalizes silently rather than failing, or an operator's Driver would brick the node +/// at the exact moment they upgrade. +/// +public sealed class EffectiveRoles +{ + private EffectiveRoles(IReadOnlyList gossiped, RoleSource source) + { + Gossiped = gossiped; + Source = source; + ClusterRole = RoleParser.ClusterRoleFromRoles(gossiped); + ClusterId = ClusterRole is null ? null : RoleParser.ClusterIdFromRole(ClusterRole); + } + + /// The configuration key reads for the configured + /// side of the precedence. + public const string ConfiguredRolesKey = "Cluster:Roles"; + + /// The roles as they are handed to Akka — becomes + /// this list, unmodified. + public IReadOnlyList Gossiped { get; } + + /// Which source resolved these roles. + public RoleSource Source { get; } + + /// True when is among (case-insensitive). + public bool HasAdmin => Has(RoleParser.Admin); + + /// True when is among (case-insensitive). + public bool HasDriver => Has(RoleParser.Driver); + + /// True when is among (case-insensitive). + public bool HasDev => Has(RoleParser.Dev); + + /// True when is among , compared case-insensitively. + /// The role to test for. + public bool Has(string role) => + Gossiped.Any(r => string.Equals(r, role, StringComparison.OrdinalIgnoreCase)); + + /// The first cluster-scoped role in , original spelling preserved, + /// or null when this node carries none. + public string? ClusterRole { get; } + + /// The ClusterId this node belongs to (the suffix of ), or null + /// for a legacy / single-mesh node. + public string? ClusterId { get; } + + /// The one factory: resolves the effective roles off 's + /// Cluster:Roles and the process's OTOPCUA_ROLES environment variable. + /// The application configuration. + /// The resolved . + /// Either source names an unknown role. + public static EffectiveRoles Resolve(IConfiguration configuration) + { + ArgumentNullException.ThrowIfNull(configuration); + + var configured = configuration.GetSection(ConfiguredRolesKey).Get(); + var envRoles = Environment.GetEnvironmentVariable(RoleParser.RolesEnvVar); + return Resolve(configured, envRoles); + } + + /// Resolves the effective roles from explicit values — the overload + /// delegates to. + /// The bound Cluster:Roles value. + /// The raw OTOPCUA_ROLES value. + /// The resolved . + /// Either source names an unknown role. + public static EffectiveRoles Resolve(IReadOnlyList? configured, string? envRoles) + { + if (configured is { Count: > 0 }) + { + return new EffectiveRoles(NormalizeAndValidateConfigured(configured), RoleSource.Configured); + } + + if (!string.IsNullOrWhiteSpace(envRoles)) + { + // RoleParser.Parse already trims, lower-cases (fully — including a cluster suffix, which + // is intentionally different from the configured path; see ClusterIdFromRole's remarks), + // dedupes and validates. + return new EffectiveRoles(RoleParser.Parse(envRoles), RoleSource.Environment); + } + + return new EffectiveRoles(Array.Empty(), RoleSource.None); + } + + /// Normalizes and validates the Cluster:Roles entries: a plain role is trimmed and + /// lower-cased; a cluster role keeps its ClusterId suffix verbatim but gets a normalized + /// (lower-case) cluster- prefix. Throws naming the offending index and value on anything + /// that is neither. + private static string[] NormalizeAndValidateConfigured(IReadOnlyList configured) + { + var result = new string[configured.Count]; + for (var i = 0; i < configured.Count; i++) + { + var raw = configured[i]; + var trimmed = raw?.Trim() ?? string.Empty; + + if (trimmed.Length == 0) + { + throw Unknown(i, raw); + } + + if (RoleParser.IsClusterRole(trimmed)) + { + var clusterId = RoleParser.ClusterIdFromRole(trimmed); + if (clusterId.Length == 0) + { + throw Unknown(i, raw); + } + + result[i] = RoleParser.ClusterRolePrefix + clusterId; + continue; + } + + var lowered = trimmed.ToLowerInvariant(); + if (lowered is not (RoleParser.Admin or RoleParser.Driver or RoleParser.Dev)) + { + throw Unknown(i, raw); + } + + result[i] = lowered; + } + + return result; + } + + private static ArgumentException Unknown(int index, string? raw) => + new($"{ConfiguredRolesKey}[{index}] = '{raw}' is not a known role. Allowed: {RoleParser.Admin}, " + + $"{RoleParser.Driver}, {RoleParser.Dev}, or '{RoleParser.ClusterRolePrefix}'."); +} diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/RoleParser.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/RoleParser.cs index 789a0084..f9d58a6b 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/RoleParser.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/RoleParser.cs @@ -111,12 +111,15 @@ public static class RoleParser /// The bound Cluster:Roles value. /// The raw OTOPCUA_ROLES value. /// The effective roles; empty when neither source names any. - /// The environment value names an unknown role. - public static string[] ResolveEffectiveRoles(IReadOnlyList? configuredRoles, string? envRoles) - { - if (configuredRoles is { Count: > 0 }) return [.. configuredRoles]; - return Parse(envRoles); - } + /// Either source names an unknown role. + /// + /// Thin delegation to — + /// the single role authority (archreview R3-04). is the type to + /// reach for directly; this overload is retained so existing callers of the array-returning + /// shape keep compiling. + /// + public static string[] ResolveEffectiveRoles(IReadOnlyList? configuredRoles, string? envRoles) => + [.. EffectiveRoles.Resolve(configuredRoles, envRoles).Gossiped]; /// Reads the effective roles for the current process, resolving the environment itself. /// The bound Cluster:Roles value. diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/EffectiveRolesTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/EffectiveRolesTests.cs new file mode 100644 index 00000000..d7dce4ca --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/EffectiveRolesTests.cs @@ -0,0 +1,213 @@ +using Microsoft.Extensions.Configuration; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Cluster.Tests; + +/// +/// is the single role authority (archreview R3-04, 01/S-12 = 03/C5, +/// 01/C-9): one factory, one precedence (Cluster:Roles non-empty wins, else +/// OTOPCUA_ROLES, else empty), two-layer normalization (plain roles lower-cased, a cluster +/// role's ClusterId suffix preserved verbatim), and validation on BOTH sources. +/// +[Collection(RoleEnvironmentCollection.Name)] +public sealed class EffectiveRolesTests +{ + // ---- Precedence matrix ------------------------------------------------------------------- + + [Fact] + public void Configured_only_resolves_from_Configured_source() + { + var roles = EffectiveRoles.Resolve(["admin", "driver"], null); + + roles.Source.ShouldBe(RoleSource.Configured); + roles.Gossiped.ShouldBe(new[] { "admin", "driver" }); + } + + [Fact] + public void Environment_only_resolves_from_Environment_source() + { + var roles = EffectiveRoles.Resolve(null, "admin,driver"); + + roles.Source.ShouldBe(RoleSource.Environment); + roles.Gossiped.ShouldBe(new[] { "admin", "driver" }); + } + + [Fact] + public void Both_agree_resolves_from_Configured_source() + { + var roles = EffectiveRoles.Resolve(["driver"], "driver"); + + roles.Source.ShouldBe(RoleSource.Configured); + roles.Gossiped.ShouldBe(new[] { "driver" }); + } + + [Fact] + public void Both_differ_Configured_wins_outright() + { + var roles = EffectiveRoles.Resolve(["admin"], "driver,cluster-SITE-A"); + + roles.Source.ShouldBe(RoleSource.Configured); + roles.Gossiped.ShouldBe(new[] { "admin" }); + } + + [Fact] + public void Neither_source_resolves_empty_with_None_source() + { + var roles = EffectiveRoles.Resolve(null, null); + + roles.Source.ShouldBe(RoleSource.None); + roles.Gossiped.ShouldBeEmpty(); + } + + [Fact] + public void Empty_configured_array_falls_back_to_the_environment() + { + var roles = EffectiveRoles.Resolve([], "driver"); + + roles.Source.ShouldBe(RoleSource.Environment); + roles.Gossiped.ShouldBe(new[] { "driver" }); + } + + // ---- Gossiped normalization: two-layer (choice 2) ---------------------------------------- + + [Fact] + public void Configured_plain_role_is_trimmed_and_lower_cased_in_Gossiped() + { + var roles = EffectiveRoles.Resolve([" Driver "], null); + + roles.Gossiped.ShouldBe(new[] { "driver" }); + } + + /// + /// THE falsification control for choice 2: a cluster role's ClusterId suffix must survive + /// UNCHANGED — lower-casing it would rename the Akka role of a running deployment + /// (cluster-SITE-A vs cluster-site-a are different gossiped roles). + /// + [Fact] + public void Configured_cluster_role_suffix_case_is_preserved_in_Gossiped() + { + var roles = EffectiveRoles.Resolve(["Cluster-SITE-A"], null); + + // The "cluster-" prefix normalizes (it is a plain marker), the "SITE-A" suffix does not. + roles.Gossiped.ShouldBe(new[] { "cluster-SITE-A" }); + } + + [Fact] + public void Configured_cluster_role_with_already_lowercase_prefix_keeps_suffix_verbatim() + { + var roles = EffectiveRoles.Resolve(["cluster-Site-A"], null); + + roles.Gossiped.ShouldBe(new[] { "cluster-Site-A" }); + } + + [Fact] + public void Environment_cluster_role_suffix_IS_lowercased_unlike_Configured() + { + // Unchanged from RoleParser.Parse's existing (documented) behaviour: the two sources + // normalize differently on purpose (RoleParser.ClusterIdFromRole's remarks). + var roles = EffectiveRoles.Resolve(null, "driver,cluster-SITE-A"); + + roles.Gossiped.ShouldBe(new[] { "driver", "cluster-site-a" }); + } + + // ---- Has*/HasAdmin/HasDriver case-insensitivity ------------------------------------------ + + [Theory] + [InlineData("driver")] + [InlineData("Driver")] + [InlineData("DRIVER")] + [InlineData(" driver ")] + public void HasDriver_true_for_every_casing(string configuredRole) + { + EffectiveRoles.Resolve([configuredRole], null).HasDriver.ShouldBeTrue(); + } + + [Fact] + public void HasAdmin_false_when_admin_is_not_present() + { + EffectiveRoles.Resolve(["driver"], null).HasAdmin.ShouldBeFalse(); + } + + [Fact] + public void Has_is_case_insensitive_for_an_arbitrary_role() + { + var roles = EffectiveRoles.Resolve(["Admin", "Driver"], null); + + roles.Has("admin").ShouldBeTrue(); + roles.Has("ADMIN").ShouldBeTrue(); + roles.Has("driver").ShouldBeTrue(); + roles.Has("dev").ShouldBeFalse(); + } + + // ---- Unknown / invalid role from EITHER source throws, naming the key -------------------- + + [Fact] + public void Unknown_role_from_the_configured_source_throws_naming_the_key() + { + var ex = Should.Throw(() => EffectiveRoles.Resolve(["Drivr"], null)); + + ex.Message.ShouldContain("Cluster:Roles"); + ex.Message.ShouldContain("Drivr"); + } + + [Fact] + public void Unknown_role_from_the_environment_source_throws() + { + // The env path already validated via RoleParser.Parse — this pins that it still does. + Should.Throw(() => EffectiveRoles.Resolve(null, "admin,master")); + } + + // ---- ClusterRole / ClusterId --------------------------------------------------------------- + + [Fact] + public void ClusterRole_and_ClusterId_are_null_for_a_legacy_node() + { + var roles = EffectiveRoles.Resolve(["admin", "driver"], null); + + roles.ClusterRole.ShouldBeNull(); + roles.ClusterId.ShouldBeNull(); + } + + [Fact] + public void ClusterRole_and_ClusterId_resolve_for_a_split_topology_node() + { + var roles = EffectiveRoles.Resolve(["driver", "cluster-SITE-A"], null); + + roles.ClusterRole.ShouldBe("cluster-SITE-A"); + roles.ClusterId.ShouldBe("SITE-A"); + } + + // ---- Resolve(IConfiguration) overload ------------------------------------------------------ + + [Fact] + public void Resolve_from_configuration_reads_Cluster_Roles() + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["Cluster:Roles:0"] = "driver", + ["Cluster:Roles:1"] = "cluster-SITE-A", + }) + .Build(); + + var roles = EffectiveRoles.Resolve(configuration); + + roles.Source.ShouldBe(RoleSource.Configured); + roles.Gossiped.ShouldBe(new[] { "driver", "cluster-SITE-A" }); + } + + [Fact] + public void Resolve_from_configuration_falls_back_to_the_environment() + { + using (new EnvVarScope(RoleParser.RolesEnvVar, "admin,driver")) + { + var configuration = new ConfigurationBuilder().Build(); + + var roles = EffectiveRoles.Resolve(configuration); + + roles.Source.ShouldBe(RoleSource.Environment); + roles.Gossiped.ShouldBe(new[] { "admin", "driver" }); + } + } +}