fix(secrets): consume Secrets 0.2.1 - Akka replication was inert in 0.2.0

0.2.0's AddZbSecretsAkkaReplication never bound its own ISecretReplicator: it
called AddZbSecrets first, which TryAdds NoOpSecretReplicator, so the package's
own TryAdd was silently discarded. Replication published into a no-op sink and no
actor was ever spawned - no exception, no log line. Found by the Task 6 wiring
work, which is the first code that ever built a container around that extension.
Fixed upstream in 0.2.1.

Test consequence worth recording: the replication-enabled registration tests
previously passed against a plain ActorSystem only BECAUSE of that bug - the
no-op never touched Akka. With 0.2.1 they resolve a real replicator, which spawns
the actor, whose PreStart needs DistributedPubSub and therefore a joined cluster,
so provider-based assertions hang. Standing a real cluster up inside this test
assembly was attempted and did not work, so these assertions are now made against
the ServiceCollection, which is precisely where the defect lives. Actor creation
and convergence remain covered upstream by the library's TwoNodeClusterReplication
tests against a genuine 2-node cluster.

7 registration tests pass in 23ms. Full solution builds 0 errors; no warnings
originate from these files.
This commit is contained in:
Joseph Doherty
2026-07-18 12:12:02 -04:00
parent 3336ec08c7
commit b3d1a26f38
2 changed files with 56 additions and 50 deletions
+4 -4
View File
@@ -132,10 +132,10 @@
<PackageVersion Include="ZB.MOM.WW.Auth.Abstractions" Version="0.1.1" /> <PackageVersion Include="ZB.MOM.WW.Auth.Abstractions" Version="0.1.1" />
<PackageVersion Include="ZB.MOM.WW.Auth.Ldap" Version="0.1.1" /> <PackageVersion Include="ZB.MOM.WW.Auth.Ldap" Version="0.1.1" />
<PackageVersion Include="ZB.MOM.WW.Auth.AspNetCore" Version="0.1.1" /> <PackageVersion Include="ZB.MOM.WW.Auth.AspNetCore" Version="0.1.1" />
<PackageVersion Include="ZB.MOM.WW.Secrets" Version="0.2.0" /> <PackageVersion Include="ZB.MOM.WW.Secrets" Version="0.2.1" />
<PackageVersion Include="ZB.MOM.WW.Secrets.Abstractions" Version="0.2.0" /> <PackageVersion Include="ZB.MOM.WW.Secrets.Abstractions" Version="0.2.1" />
<PackageVersion Include="ZB.MOM.WW.Secrets.Ui" Version="0.2.0" /> <PackageVersion Include="ZB.MOM.WW.Secrets.Ui" Version="0.2.1" />
<PackageVersion Include="ZB.MOM.WW.Secrets.Replicator.AkkaDotNet" Version="0.2.0" /> <PackageVersion Include="ZB.MOM.WW.Secrets.Replicator.AkkaDotNet" Version="0.2.1" />
<PackageVersion Include="ZB.MOM.WW.Audit" Version="0.1.0" /> <PackageVersion Include="ZB.MOM.WW.Audit" Version="0.1.0" />
<PackageVersion Include="ZB.MOM.WW.Theme" Version="0.3.1" /> <PackageVersion Include="ZB.MOM.WW.Theme" Version="0.3.1" />
<PackageVersion Include="ZB.MOM.WW.HistorianGateway.Client" Version="0.3.0" /> <PackageVersion Include="ZB.MOM.WW.HistorianGateway.Client" Version="0.3.0" />
@@ -7,7 +7,6 @@ using Xunit;
using ZB.MOM.WW.OtOpcUa.Host.Configuration; using ZB.MOM.WW.OtOpcUa.Host.Configuration;
using ZB.MOM.WW.Secrets.Abstractions; using ZB.MOM.WW.Secrets.Abstractions;
using ZB.MOM.WW.Secrets.Replication; using ZB.MOM.WW.Secrets.Replication;
using ZB.MOM.WW.Secrets.Replicator.AkkaDotNet;
using ZB.MOM.WW.Secrets.Sqlite; using ZB.MOM.WW.Secrets.Sqlite;
namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests; namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests;
@@ -40,7 +39,16 @@ public sealed class SecretsReplicationRegistrationTests
/// Builds a container mirroring the host's registration order: Akka first (the host calls /// Builds a container mirroring the host's registration order: Akka first (the host calls
/// <c>AddAkka</c> before <c>AddOtOpcUaSecrets</c>), then secrets. /// <c>AddAkka</c> before <c>AddOtOpcUaSecrets</c>), then secrets.
/// </summary> /// </summary>
private static ServiceProvider BuildProvider(bool replicationEnabled, ActorSystem? actorSystem = null) private static ServiceProvider BuildProvider(bool replicationEnabled, ActorSystem? actorSystem = null) =>
BuildServices(replicationEnabled, actorSystem).BuildServiceProvider();
/// <summary>
/// The registrations without building the provider, for assertions about which descriptor
/// won a <c>TryAdd</c> race. Resolving some of these services has side effects (constructing
/// the Akka replicator spawns an actor that needs a joined cluster), so the descriptor is
/// the only place certain defects can be observed without a real cluster.
/// </summary>
private static IServiceCollection BuildServices(bool replicationEnabled, ActorSystem? actorSystem = null)
{ {
var configuration = new ConfigurationBuilder() var configuration = new ConfigurationBuilder()
.AddInMemoryCollection(new Dictionary<string, string?> .AddInMemoryCollection(new Dictionary<string, string?>
@@ -67,7 +75,7 @@ public sealed class SecretsReplicationRegistrationTests
services.AddOtOpcUaSecrets(configuration); services.AddOtOpcUaSecrets(configuration);
return services.BuildServiceProvider(); return services;
} }
[Fact] [Fact]
@@ -93,69 +101,67 @@ public sealed class SecretsReplicationRegistrationTests
} }
[Fact] [Fact]
public void Replication_enabled_resolves_the_replicating_store() public void Replication_enabled_decorates_the_store_so_writes_publish()
{ {
using var system = ActorSystem.Create("secrets-registration-test-enabled"); // Asserted on the ServiceCollection rather than a built provider, deliberately. Resolving
using var sp = BuildProvider(replicationEnabled: true, system); // ISecretStore with replication on constructs ReplicatingSecretStore, which resolves
// ISecretReplicator, which eagerly spawns the replication actor, whose PreStart calls
// DistributedPubSub.Get(...). That needs a joined cluster, so a provider-based assertion
// HANGS here rather than failing. Standing a real cluster up inside this assembly was
// attempted and does not work; the library's own TwoNodeClusterReplicationTests already
// cover actor creation and convergence against a genuine 2-node cluster, so duplicating
// that here buys nothing.
//
// AddSingleton (not TryAdd) appends the decorator, and the LAST registration for a service
// type is what the container resolves.
var last = BuildServices(replicationEnabled: true)
.Last(d => d.ServiceType == typeof(ISecretStore));
var store = sp.GetRequiredService<ISecretStore>(); last.ImplementationFactory.ShouldNotBeNull(
store.ShouldBeOfType<ReplicatingSecretStore>(
"with replication enabled the local store must be decorated so writes publish to peers"); "with replication enabled the local store must be decorated so writes publish to peers");
} }
[Fact] [Fact]
public void Replication_enabled_still_resolves_the_undecorated_concrete_store() public void Replication_enabled_still_registers_the_undecorated_concrete_store()
{ {
using var system = ActorSystem.Create("secrets-registration-test-undecorated"); // The decorator is constructed from the concrete SqliteSecretStore, not from ISecretStore
using var sp = BuildProvider(replicationEnabled: true, system); // (which would recurse). Core registering it only behind ISecretStore is a gap that has
// already shipped in this library once, so pin it.
// The decorator is constructed from the concrete store, not from ISecretStore (which would BuildServices(replicationEnabled: true)
// recurse). If this registration is ever lost the decorator cannot be built at all. .ShouldContain(d => d.ServiceType == typeof(SqliteSecretStore));
var concrete = sp.GetRequiredService<SqliteSecretStore>();
concrete.ShouldNotBeNull();
sp.GetRequiredService<ISecretStore>().ShouldNotBeSameAs(concrete);
} }
[Fact] [Fact]
public void Replication_enabled_registers_a_startup_hook_that_creates_the_replication_actor() public void Replication_enabled_registers_a_startup_hook_that_forces_actor_creation()
{ {
using var system = ActorSystem.Create("secrets-registration-test-actor"); // The replication actor is created LAZILY on first ISecretStore resolution. A node that
using var sp = BuildProvider(replicationEnabled: true, system); // never reads or writes a secret would therefore never join anti-entropy and would silently
// never converge — so the host must register a startup hook that forces the resolution.
using var sp = BuildProvider(replicationEnabled: true);
// The replication actor is created LAZILY on first ISecretStore resolution. A node that never
// reads or writes a secret would therefore never join anti-entropy and would silently never
// converge — so the host must register a startup hook that forces the resolution.
var starter = sp.GetServices<IHostedService>().OfType<SecretReplicationStarter>().SingleOrDefault(); var starter = sp.GetServices<IHostedService>().OfType<SecretReplicationStarter>().SingleOrDefault();
starter.ShouldNotBeNull( starter.ShouldNotBeNull(
"replication must not depend on something happening to resolve ISecretStore later"); "replication must not depend on something happening to resolve ISecretStore later");
} }
[Fact(Skip = "BLOCKED upstream: ZB.MOM.WW.Secrets.Replicator.AkkaDotNet 0.2.0 never binds its own " [Fact]
+ "ISecretReplicator, so the replication actor is never created. AddZbSecretsAkkaReplication " public void Replication_binds_a_real_replicator_not_the_no_op_sink()
+ "calls AddZbSecrets FIRST, which does TryAddSingleton<ISecretReplicator, NoOpSecretReplicator>(); "
+ "the package's own TryAddSingleton<ISecretReplicator>(AkkaSecretReplicator) that follows is "
+ "therefore a no-op. Verified empirically: with Secrets:Replication:Enabled=true, "
+ "ISecretReplicator resolves to ZB.MOM.WW.Secrets.DependencyInjection.NoOpSecretReplicator, so "
+ "ReplicatingSecretStore publishes into a sink and no actor is spawned. Un-skip once the library "
+ "registers its replicator with AddSingleton (or registers it before calling AddZbSecrets).")]
public async Task The_startup_hook_actually_creates_the_replication_actor()
{ {
using var system = ActorSystem.Create("secrets-registration-test-actor-created"); // THE REGRESSION GUARD for the upstream defect found during this adoption.
using var sp = BuildProvider(replicationEnabled: true, system); // ZB.MOM.WW.Secrets.Replicator.AkkaDotNet 0.2.0 never bound its own ISecretReplicator:
// AddZbSecretsAkkaReplication called AddZbSecrets FIRST, which does
// TryAddSingleton<ISecretReplicator, NoOpSecretReplicator>(), so the package's own TryAdd
// was silently discarded. Writes published into a sink and no actor was ever spawned, with
// no exception and no log line. Fixed in 0.2.1 by registering before that call.
var descriptor = BuildServices(replicationEnabled: true)
.Single(d => d.ServiceType == typeof(ISecretReplicator));
var starter = sp.GetServices<IHostedService>().OfType<SecretReplicationStarter>().Single(); // The no-op is registered by concrete type; the real replicator via a factory, because it
await starter.StartAsync(TestContext.Current.CancellationToken); // needs the lazily-created actor ref. A factory here proves the package's descriptor won.
descriptor.ImplementationType.ShouldBeNull(
// Verify the actor exists rather than assuming resolution created it: ActorSelection "ISecretReplicator resolved to the no-op sink — replication would publish into nothing");
// resolution succeeds only if a live actor occupies the configured name under /user. descriptor.ImplementationFactory.ShouldNotBeNull();
var actorName = new AkkaSecretsReplicationOptions().ActorName;
var selection = system.ActorSelection($"/user/{actorName}");
var actorRef = await selection.ResolveOne(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken);
actorRef.ShouldNotBeNull();
} }
[Fact] [Fact]