Merge branch 'fix/r3-04-one-role-authority' — R3-04: EffectiveRoles is the one role authority (Cluster:Roles wins over OTOPCUA_ROLES, normalised, validated, fail-start on unknown/overlay-changed roles); Program.cs + all four validators consume it; case-insensitive gossip predicates; source-scan guard (archreview 01/S-12 = 03/C5, 01/C-9)

2026-08-20 11:05:09 -04:00
parent d79d51dd4f e6a36f4ba8
commit f1b976f5e2
41 changed files with 2176 additions and 159 deletions
@@ -0,0 +1,220 @@
+using System.Collections.Immutable;
+using System.Reflection;
+using Akka.Actor;
+using Akka.Cluster;
+using Akka.Configuration;
+using Akka.Util;
+using Microsoft.Extensions.Logging.Abstractions;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.Audit;
+using ZB.MOM.WW.OtOpcUa.Cluster;
+using ZB.MOM.WW.OtOpcUa.Commons.Messages.Redundancy;
+using ZB.MOM.WW.OtOpcUa.ControlPlane.Fleet;
+using ZB.MOM.WW.OtOpcUa.ControlPlane.Redundancy;
+
+namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Tests;
+
+/// <summary>
+///     R3-04 (01/C-9, census rows 17/20-24/30): a PEER gossiping a miscased plain role (a
+///     rolling-upgrade-window peer running an older build — <see cref="EffectiveRoles"/> (T1) already
+///     means a node on the FIXED build cannot gossip a miscased role itself) must not read as a
+///     stranger to the six production predicates this plan names.
+/// </summary>
+/// <remarks>
+///     <para>
+///         <b>Deviation from the plan's stated T5 test (recorded per the execution brief):</b> the
+///         plan asked for "a <c>RedundancyStateActor</c> selection case where the member gossips
+///         'Driver' and a Primary IS elected". That is not achievable from this repo.
+///         <see cref="RedundancyStateActor.SelectOldestUpMemberOfRole"/> /
+///         <see cref="RedundancyStateActor.SelectDriverPrimary"/> delegate to
+///         <c>ZB.MOM.WW.Health.Akka</c>'s <c>ClusterActiveNode.OldestUpMember</c>, which matches via
+///         Akka's OWN <c>Member.HasRole</c> — an ordinal comparison this repo does not own, the exact
+///         same class of "not ours to change" the plan itself already carves out for
+///         <c>ClusterSingletonOptions.Role</c> (rows 18/19). <see cref="Primary_election_itself_remains_ordinal_and_still_elects_no_one_for_an_all_miscased_pair"/>
+///         below pins that TRUE (negative) outcome instead of asserting a false positive.
+///     </para>
+///     <para>
+///         What R3-04 DOES fix and this file proves: every consumer that reads gossiped roles
+///         DIRECTLY (not through that external oldest-member helper) is now case-insensitive, so an
+///         individual miscased member is no longer invisible to failover candidacy, address
+///         reconciliation, or the redundancy snapshot's own role classification.
+///     </para>
+/// </remarks>
+public sealed class GossipedRoleCaseInsensitivityTests
+{
+    // ---- Pure-unit: ClusterNodeAddressReconcilerActor.FilterOwnClusterDriverMembers ---------------
+    // (public static over IEnumerable<Member> — no live cluster needed)
+
+    private static int _memberUid;
+
+    // Akka's Member.Create factories are internal — same reflection idiom as
+    // ClusterNodeAddressReconcilerTests.UpMember.
+    private static readonly MethodInfo MemberCreate = typeof(Member).GetMethod(
+        "Create",
+        BindingFlags.Static | BindingFlags.NonPublic | BindingFlags.Public,
+        binder: null,
+        types:
+        [
+            typeof(UniqueAddress), typeof(int), typeof(MemberStatus),
+            typeof(ImmutableHashSet<string>), typeof(AppVersion),
+        ],
+        modifiers: null)
+        ?? throw new InvalidOperationException("Akka.Cluster.Member.Create(5-arg) not found — Akka API changed");
+
+    private static Member UpMember(string host, int port, params string[] roles) =>
+        (Member)MemberCreate.Invoke(null,
+        [
+            new UniqueAddress(new Address("akka.tcp", "otopcua", host, port), Interlocked.Increment(ref _memberUid)),
+            1,
+            MemberStatus.Up,
+            roles.ToImmutableHashSet(),
+            AppVersion.Zero,
+        ])!;
+
+    [Fact]
+    public void FilterOwnClusterDriverMembers_recognises_a_miscased_driver_role()
+    {
+        var members = new[] { UpMember("site-a-1", 4053, "Driver", "cluster-SITE-A") };
+
+        var result = ClusterNodeAddressReconcilerActor.FilterOwnClusterDriverMembers(members, "cluster-SITE-A");
+
+        result.ShouldHaveSingleItem();
+    }
+
+    [Fact]
+    public void FilterOwnClusterDriverMembers_recognises_a_miscased_cluster_role()
+    {
+        var members = new[] { UpMember("site-a-1", 4053, "driver", "Cluster-SITE-A") };
+
+        // The caller's OWN cluster role names the canonical spelling; the member gossips a different
+        // casing ("Cluster-SITE-A") — GossipedRolesHave must still match the two as the same role.
+        var result = ClusterNodeAddressReconcilerActor.FilterOwnClusterDriverMembers(members, "cluster-site-a");
+
+        result.ShouldHaveSingleItem();
+    }
+
+    // ---- Live single-node cluster: BuildSnapshot classification + ManualFailoverService -----------
+
+    private const int Port = 19_560;
+
+    private static Config NodeConfig(int port) => ConfigurationFactory.ParseString($$"""
+        akka {
+            loglevel = "WARNING"
+            actor.provider = "Akka.Cluster.ClusterActorRefProvider, Akka.Cluster"
+            remote.dot-netty.tcp {
+                hostname = "127.0.0.1"
+                port = {{port}}
+            }
+            cluster {
+                seed-nodes = ["akka.tcp://gossip-case-test@127.0.0.1:{{port}}"]
+                roles = ["Driver"]
+                min-nr-of-members = 1
+                run-coordinated-shutdown-when-down = off
+                downing-provider-class = ""
+            }
+        }
+        """);
+
+    private static async Task<ActorSystem> StartSingleNodeAsync()
+    {
+        var system = ActorSystem.Create("gossip-case-test", NodeConfig(Port));
+        var cluster = Akka.Cluster.Cluster.Get(system);
+        var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(30);
+        while (DateTime.UtcNow < deadline)
+        {
+            if (cluster.State.Members.Any(m => m.Status == MemberStatus.Up)) return system;
+            await Task.Delay(100);
+        }
+
+        await system.Terminate();
+        throw new TimeoutException("node never came Up");
+    }
+
+    [Fact]
+    public async Task A_member_gossiping_a_miscased_driver_role_classifies_Secondary_not_Detached()
+    {
+        // Pins the fix that IS achievable: BuildSnapshot's own role classification (member.Roles
+        // Contains -> GossipedRolesHave) no longer reads a genuinely-driver-but-miscased member as
+        // Detached, even though (see class remarks) it can never be named Primary by itself.
+        var system = await StartSingleNodeAsync();
+        try
+        {
+            var tcs = new TaskCompletionSource<RedundancyStateChanged>(
+                TaskCreationOptions.RunContinuationsAsynchronously);
+            system.ActorOf(
+                RedundancyStateActor.Props(broadcast: msg =>
+                {
+                    if (msg is RedundancyStateChanged changed) tcs.TrySetResult(changed);
+                }),
+                $"redundancy-{Guid.NewGuid():N}");
+
+            var snapshot = await tcs.Task.WaitAsync(TimeSpan.FromSeconds(15));
+
+            var node = snapshot.Nodes.ShouldHaveSingleItem();
+            node.Role.ShouldBe(
+                RedundancyRole.Secondary,
+                "gossiping 'Driver' must not read as Detached — it is a real driver member, just "
+                + "miscased; BEFORE R3-04 this was Detached (ordinal Contains(\"driver\") on \"Driver\")");
+        }
+        finally
+        {
+            await system.Terminate();
+        }
+    }
+
+    /// <summary>
+    ///     THE recorded deviation: proves the residual gap is real, not assumed. A pair (here,
+    ///     trivially, a single node) that gossips ONLY 'Driver' still elects no Primary — because that
+    ///     decision runs through Akka's own ordinal <c>Member.HasRole</c> inside the vendored
+    ///     <c>ClusterActiveNode.OldestUpMember</c>, which this repo cannot patch. Tracked as a
+    ///     follow-up against <c>ZB.MOM.WW.Health.Akka</c>, not closed by R3-04.
+    /// </summary>
+    [Fact]
+    public async Task Primary_election_itself_remains_ordinal_and_still_elects_no_one_for_an_all_miscased_pair()
+    {
+        var system = await StartSingleNodeAsync();
+        try
+        {
+            var members = Akka.Cluster.Cluster.Get(system).State.Members;
+
+            RedundancyStateActor.SelectOldestUpMemberOfRole(members, "driver").ShouldBeNull(
+                "known residual (R3-04 execution notes): SelectOldestUpMemberOfRole delegates to "
+                + "ClusterActiveNode.OldestUpMember -> Akka's own ordinal Member.HasRole, which this "
+                + "repo does not own — GossipedRolesHave cannot reach inside it");
+        }
+        finally
+        {
+            await system.Terminate();
+        }
+    }
+
+    [Fact]
+    public async Task ManualFailoverService_recognises_a_miscased_driver_node_as_a_candidate()
+    {
+        // FailOverCore refuses below 2 driver members (nothing to fail over TO), so this single-node
+        // fixture exercises GetSnapshot()'s PrimaryAddress instead — which shares the same
+        // OrderedDrivers candidate selection this test is actually about.
+        var system = await StartSingleNodeAsync();
+        try
+        {
+            var service = new ManualFailoverService(
+                () => system, new RecordingAuditWriter(), NullLogger<ManualFailoverService>.Instance);
+
+            var snapshot = service.GetSnapshot();
+
+            snapshot.PrimaryAddress.ShouldNotBeNull();
+            snapshot.PrimaryAddress!.ShouldContain(Port.ToString());
+        }
+        finally
+        {
+            await system.Terminate();
+        }
+    }
+
+    private sealed class RecordingAuditWriter : IAuditWriter
+    {
+        public Task WriteAsync(AuditEvent auditEvent, CancellationToken cancellationToken = default) =>
+            Task.CompletedTask;
+    }
+}
@@ -0,0 +1,188 @@
+using Microsoft.AspNetCore.Builder;
+using Microsoft.Extensions.Configuration;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.LocalDb;
+using ZB.MOM.WW.OtOpcUa.Cluster;
+using ZB.MOM.WW.OtOpcUa.Core.Authorization;
+using ZB.MOM.WW.OtOpcUa.Host.Configuration;
+using ZB.MOM.WW.OtOpcUa.Host.OpcUa;
+using ZB.MOM.WW.OtOpcUa.OpcUaServer.Security;
+using ZB.MOM.WW.OtOpcUa.Security;
+
+namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests;
+
+/// <summary>
+///     R3-04 (01/S-12 = 03/C5): the exact shape the finding describes — a node configured ONLY
+///     through <c>Cluster:Roles</c> (<c>OTOPCUA_ROLES</c> unset) — must wire the driver stack, not
+///     the "no roles at all" host view a pre-fix <c>Program.cs</c> computed.
+/// </summary>
+/// <remarks>
+///     <para>
+///         Follows <see cref="DriverOnlyNoConfigDbBootTests"/>'s direct-host-build pattern
+///         deliberately: <c>WebApplicationFactory&lt;Program&gt;</c> is not driven here because
+///         starting the real host triggers the Akka cluster join and needs SQL Server (see
+///         <c>TwoNodeClusterHarness</c>). What is under test is DI resolution off the SAME
+///         composition Program.cs runs, so a container is built and resolved without starting it.
+///     </para>
+///     <para>
+///         Unlike <see cref="DriverOnlyNoConfigDbBootTests"/>, <c>hasAdmin</c>/<c>hasDriver</c> are
+///         NOT passed in as bool parameters here — they are derived from
+///         <see cref="EffectiveRoles.Resolve(IConfiguration)"/> exactly as <c>Program.cs</c> (T3)
+///         does, off a configuration carrying ONLY <c>Cluster:Roles</c>. The role DERIVATION is
+///         therefore genuinely under test, not just the conditional wiring downstream of it.
+///     </para>
+/// </remarks>
+public sealed class ClusterRolesOnlyBootTests : IDisposable
+{
+    private readonly List<WebApplication> _apps = [];
+    private readonly List<string> _dbPaths = [];
+
+    /// <summary>
+    ///     Builds the slice of <c>Program.cs</c>'s conditional wiring this finding is about: the
+    ///     LocalDb registration (<c>:230</c>, inside <c>if (hasDriver)</c>) and the
+    ///     <c>IOpcUaUserAuthenticator</c> selection (<c>:414-454</c>), driven by
+    ///     <paramref name="deriveRoles"/> — the T3 (fixed) or pre-fix derivation, selected by the
+    ///     caller so the two behaviours are provably different rather than assumed to be.
+    /// </summary>
+    private (IServiceProvider Services, bool HasAdmin, bool HasDriver) BuildGraph(
+        Dictionary<string, string?> configuration,
+        Func<IConfiguration, (bool HasAdmin, bool HasDriver)> deriveRoles)
+    {
+        var dbPath = Path.Combine(Path.GetTempPath(), $"otopcua-r304-boot-{Guid.NewGuid():N}.db");
+        _dbPaths.Add(dbPath);
+        configuration["LocalDb:Path"] = dbPath;
+
+        var builder = WebApplication.CreateBuilder(new WebApplicationOptions { Args = [] });
+        builder.Configuration.AddInMemoryCollection(configuration);
+        builder.Services.AddLogging();
+
+        var (hasAdmin, hasDriver) = deriveRoles(builder.Configuration);
+
+        builder.Services.AddOtOpcUaCluster(builder.Configuration);
+
+        // Program.cs:230 — driver-role only.
+        if (hasDriver)
+            builder.Services.AddOtOpcUaLocalDb(builder.Configuration);
+
+        // Program.cs:414-454 — the IOpcUaUserAuthenticator selection block, reproduced verbatim
+        // (the hasDriver-only leg only — hasAdmin needs a ConfigDb this test does not configure).
+        if (hasDriver)
+        {
+            builder.Services.AddSingleton<DataPlaneAuthorityState>();
+            builder.Services.AddSingleton<IDataPlaneAuthoritySource>(
+                sp => sp.GetRequiredService<DataPlaneAuthorityState>());
+            builder.Services.AddSingleton<IApiKeyStore, ArtifactApiKeyStore>();
+            builder.Services.AddSingleton<IOpcUaUserAuthenticator, ApiKeyOpcUaUserAuthenticator>();
+        }
+        else
+        {
+            builder.Services.AddSingleton<IOpcUaUserAuthenticator>(NullOpcUaUserAuthenticator.Instance);
+        }
+        builder.Services.Configure<ApiKeyOptions>(o => o.Pepper = "test-pepper");
+
+        var app = builder.Build();
+        _apps.Add(app);
+        return (app.Services, hasAdmin, hasDriver);
+    }
+
+    /// <summary>T3's fix: resolve through the single role authority.</summary>
+    private static (bool HasAdmin, bool HasDriver) FixedDerivation(IConfiguration configuration)
+    {
+        var roles = EffectiveRoles.Resolve(configuration);
+        return (roles.HasAdmin, roles.HasDriver);
+    }
+
+    /// <summary>
+    ///     The pre-T3 derivation — <c>Program.cs:61-63</c> before this plan, verbatim: reads
+    ///     <c>OTOPCUA_ROLES</c> only, never <c>Cluster:Roles</c>. Takes <see cref="IConfiguration"/>
+    ///     purely to match <see cref="FixedDerivation"/>'s signature; it does not use it.
+    /// </summary>
+    private static (bool HasAdmin, bool HasDriver) PreFixDerivation(IConfiguration _)
+    {
+        var roles = RoleParser.Parse(Environment.GetEnvironmentVariable("OTOPCUA_ROLES"));
+        return (roles.Contains("admin"), roles.Contains("driver"));
+    }
+
+    [Fact]
+    public void A_ClusterRolesOnly_node_wires_the_driver_stack()
+    {
+        var previous = Environment.GetEnvironmentVariable("OTOPCUA_ROLES");
+        Environment.SetEnvironmentVariable("OTOPCUA_ROLES", null);
+        try
+        {
+            var (services, hasAdmin, hasDriver) = BuildGraph(
+                new Dictionary<string, string?> { ["Cluster:Roles:0"] = "driver" },
+                FixedDerivation);
+
+            hasAdmin.ShouldBeFalse();
+            hasDriver.ShouldBeTrue();
+
+            services.GetService<ILocalDb>().ShouldNotBeNull(
+                "a Cluster:Roles-only driver node must have LocalDb wired");
+            services.GetRequiredService<IOpcUaUserAuthenticator>().ShouldBeOfType<ApiKeyOpcUaUserAuthenticator>(
+                "a Cluster:Roles-only driver node must authenticate api keys, not deny everything");
+
+            var akkaOptions = services.GetRequiredService<IOptions<AkkaClusterOptions>>().Value;
+            akkaOptions.Roles.ShouldBe(new[] { "driver" });
+        }
+        finally
+        {
+            Environment.SetEnvironmentVariable("OTOPCUA_ROLES", previous);
+        }
+    }
+
+    /// <summary>
+    ///     THE negative control the plan requires: the SAME <c>Cluster:Roles</c>-only configuration,
+    ///     run through the derivation Program.cs used BEFORE T3, must reproduce exactly the outage
+    ///     01/S-12 describes — no LocalDb, and every UserName token denied. Proves the fixed test
+    ///     above is asserting a real behaviour change, not a tautology.
+    /// </summary>
+    [Fact]
+    public void The_same_configuration_through_the_preFix_derivation_gets_no_driver_stack_at_all()
+    {
+        var previous = Environment.GetEnvironmentVariable("OTOPCUA_ROLES");
+        Environment.SetEnvironmentVariable("OTOPCUA_ROLES", null);
+        try
+        {
+            var (services, hasAdmin, hasDriver) = BuildGraph(
+                new Dictionary<string, string?> { ["Cluster:Roles:0"] = "driver" },
+                PreFixDerivation);
+
+            hasAdmin.ShouldBeFalse();
+            hasDriver.ShouldBeFalse(
+                "the pre-fix derivation reads OTOPCUA_ROLES ONLY — Cluster:Roles=[\"driver\"] must be invisible to it");
+
+            services.GetService<ILocalDb>().ShouldBeNull(
+                "the pre-fix host view has no roles at all, so LocalDb is never registered");
+            services.GetRequiredService<IOpcUaUserAuthenticator>().ShouldBeOfType<NullOpcUaUserAuthenticator>(
+                "the pre-fix host view denies every UserName token — the exact outage 01/S-12 describes");
+
+            // The Akka side is unaffected by which derivation Program.cs used — AddOtOpcUaCluster (T2)
+            // always resolves Cluster:Roles itself, so even the pre-fix HOST view's blindness left the
+            // AKKA gossip correct. This is the two-authority split the finding is named for: Akka said
+            // "driver", Program.cs's conditional wiring said nothing.
+            var akkaOptions = services.GetRequiredService<IOptions<AkkaClusterOptions>>().Value;
+            akkaOptions.Roles.ShouldBe(new[] { "driver" });
+        }
+        finally
+        {
+            Environment.SetEnvironmentVariable("OTOPCUA_ROLES", previous);
+        }
+    }
+
+    public void Dispose()
+    {
+        foreach (var app in _apps)
+            ((IDisposable)app).Dispose();
+
+        foreach (var path in _dbPaths)
+        {
+            try { if (File.Exists(path)) File.Delete(path); }
+            catch { /* best-effort cleanup */ }
+        }
+    }
+}