feat(secrets): opt-in Akka cluster secret replication (default OFF; upstream blocker documented)
Routes the host's secrets registration through a new AddOtOpcUaSecrets extension that gates the ISecretStore implementation on Secrets:Replication:Enabled. Opt-in gate (default FALSE) This call decides which ISecretStore every node resolves — including driver-role nodes with no auth/AdminUI, where a wrong store surfaces as drivers failing to open sessions rather than as a failing test. With the flag false the wiring is the pre-existing AddZbSecrets(config, "Secrets") call, unchanged, so current behavior is byte-identical. With it true, AddZbSecretsAkkaReplication replaces that call (it invokes AddZbSecrets internally; calling both would double-register). Extracted to a named extension specifically so the registration is testable: Program.cs is top-level statements and cannot be exercised by a container test, which is how a "registered but never resolvable" defect ships unnoticed. Serializer HOCON AkkaSecretsReplication.SerializationConfig is merged into the ActorSystem config inside the AddAkka configurator, conditionally on the same gate — a non-replicating node carries no bindings for messages it will never see. Merged via AddHocon(..., HoconAddMode.Append), Akka.Hosting's fallback merge and the same mode the existing base-config merge uses; a raw Config.WithFallback would fight the builder's own assembly. Lazy-actor mitigation The replication actor is created lazily on first ISecretStore resolution, so a node that never touches a secret would never announce a manifest and would silently never converge. SecretReplicationStarter (IHostedService) resolves the store once at startup to make participation unconditional. KNOWN BLOCKER — replication is currently NON-FUNCTIONAL; do not enable ZB.MOM.WW.Secrets.Replicator.AkkaDotNet 0.2.0 never binds its own ISecretReplicator. AddZbSecretsAkkaReplication calls AddZbSecrets FIRST, which does TryAddSingleton<ISecretReplicator, NoOpSecretReplicator>(); the package's own TryAddSingleton<ISecretReplicator>(AkkaSecretReplicator) that follows is therefore a no-op. Verified empirically in a built container: with Enabled=true, ISecretReplicator resolves to NoOpSecretReplicator, so ReplicatingSecretStore publishes into a sink and no actor is ever spawned. Consequence: the startup hook cannot create the actor, and the test asserting it does is committed Skipped with the evidence. Not worked around here — the fix belongs upstream (AddSingleton, or register before calling AddZbSecrets). Because the flag defaults false, this commit is inert in production. Tests: SecretsReplicationRegistrationTests (new) — disabled path resolves plain SqliteSecretStore and needs no ActorSystem; enabled path resolves ReplicatingSecretStore AND the undecorated concrete SqliteSecretStore the decorator is built from (the exact registration gap that shipped once); startup hook registered only when enabled. Red before wiring (4 assertion failures), green after: 6 pass, 1 skipped (blocker above). Build: 861 warnings / 0 errors, unchanged from baseline (full --no-incremental A/B). Host.IntegrationTests: 123 pass, 6 skip, 1 fail — AbCip_Green_AgainstSim, verified pre-existing on the stashed tree (fixture-gated). Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
This commit is contained in:
Joseph Doherty
@@ -0,0 +1,55 @@
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Hosting;
|
||||
using ZB.MOM.WW.Secrets.Abstractions;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Host.Configuration;
|
||||
|
||||
/// <summary>
|
||||
/// Forces this node's secret-replication actor into existence at startup.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// The replication actor is created <b>lazily</b>, on the first resolution of
|
||||
/// <see cref="ISecretStore"/> — resolving the store builds <c>ReplicatingSecretStore</c>,
|
||||
/// which takes <c>ISecretReplicator</c>, whose factory touches
|
||||
/// <c>SecretReplicationActorProvider.ActorRef</c> and thereby spawns the actor.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Laziness is correct for the library (the <c>ActorSystem</c> is usually registered after
|
||||
/// the replication call), but it makes participation in anti-entropy accidental: a node that
|
||||
/// happens never to read or write a secret — a plausible steady state for a driver-role node
|
||||
/// whose driver configs carry no <c>${secret:}</c> references — would never create the actor,
|
||||
/// never announce its manifest, and never converge. Nothing would fail; it would just
|
||||
/// silently not replicate. Resolving the store once at startup makes participation
|
||||
/// unconditional.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Known ineffective against ZB.MOM.WW.Secrets.Replicator.AkkaDotNet 0.2.0.</b> That
|
||||
/// version never binds its own <c>ISecretReplicator</c>:
|
||||
/// <c>AddZbSecretsAkkaReplication</c> calls <c>AddZbSecrets</c> first, which
|
||||
/// <c>TryAdd</c>s <c>NoOpSecretReplicator</c>, making the package's own subsequent
|
||||
/// <c>TryAddSingleton<ISecretReplicator></c> a no-op. Resolving the store therefore
|
||||
/// builds a <c>ReplicatingSecretStore</c> around a no-op replicator and spawns no actor.
|
||||
/// This hook is correct and stays in place for when the library is fixed, but replication
|
||||
/// must be treated as <b>non-functional</b> until then — see the skipped test
|
||||
/// <c>SecretsReplicationRegistrationTests.The_startup_hook_actually_creates_the_replication_actor</c>.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
/// <param name="services">Root provider used to resolve the (decorated) secret store exactly once.</param>
|
||||
public sealed class SecretReplicationStarter(IServiceProvider services) : IHostedService
|
||||
{
|
||||
/// <summary>Resolves <see cref="ISecretStore"/>, which spawns the replication actor.</summary>
|
||||
/// <param name="cancellationToken">Unused — resolution is synchronous and non-blocking.</param>
|
||||
/// <returns>A completed task.</returns>
|
||||
public Task StartAsync(CancellationToken cancellationToken)
|
||||
{
|
||||
// The resolution itself is the side effect; the instance is deliberately unused.
|
||||
_ = services.GetRequiredService<ISecretStore>();
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
|
||||
/// <summary>No-op: the actor's lifetime is the actor system's.</summary>
|
||||
/// <param name="cancellationToken">Unused.</param>
|
||||
/// <returns>A completed task.</returns>
|
||||
public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask;
|
||||
}
|
||||
@@ -0,0 +1,82 @@
|
||||
using Microsoft.Extensions.Configuration;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Hosting;
|
||||
using ZB.MOM.WW.Secrets.DependencyInjection;
|
||||
using ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.DependencyInjection;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Host.Configuration;
|
||||
|
||||
/// <summary>
|
||||
/// Single registration point for the secrets subsystem on every OtOpcUa node, admin- and
|
||||
/// driver-role alike. Exists as a named extension rather than an inline <c>AddZbSecrets</c> call
|
||||
/// in <c>Program.cs</c> so the choice of <c>ISecretStore</c> implementation is covered by a DI
|
||||
/// resolution test — <c>Program.cs</c> is top-level statements and cannot be exercised directly,
|
||||
/// which is exactly how a "registered but never resolvable" wiring defect ships unnoticed.
|
||||
/// </summary>
|
||||
public static class SecretsRegistration
|
||||
{
|
||||
/// <summary>Configuration key gating cluster secret replication. Absent or false = off.</summary>
|
||||
public const string ReplicationEnabledKey = "Secrets:Replication:Enabled";
|
||||
|
||||
/// <summary>Configuration section holding <c>AkkaSecretsReplicationOptions</c>.</summary>
|
||||
public const string ReplicationSectionPath = "Secrets:Replication";
|
||||
|
||||
/// <summary>Configuration section holding the core secrets options.</summary>
|
||||
public const string SecretsSectionPath = "Secrets";
|
||||
|
||||
/// <summary>
|
||||
/// True when cluster secret replication is switched on in configuration. Defaults to false —
|
||||
/// absent configuration must mean "off".
|
||||
/// </summary>
|
||||
/// <param name="configuration">The application configuration.</param>
|
||||
/// <returns><c>true</c> when replication is enabled.</returns>
|
||||
public static bool IsReplicationEnabled(IConfiguration configuration)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(configuration);
|
||||
return configuration.GetValue(ReplicationEnabledKey, defaultValue: false);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Registers the secrets subsystem. Replication is <b>opt-in</b>: unless
|
||||
/// <see cref="ReplicationEnabledKey"/> is true this is exactly the plain local SQLite wiring
|
||||
/// the host has always used.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// The gate is deliberately default-deny. This call decides which <c>ISecretStore</c> the
|
||||
/// container hands to <b>every</b> node, including driver-role nodes with no auth or
|
||||
/// AdminUI surface, where a bad store surfaces as drivers failing to open sessions rather
|
||||
/// than as a failing test. Enabling replication by default would change secret resolution
|
||||
/// across a production fleet with nobody having asked for it.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <c>AddZbSecretsAkkaReplication</c> calls <c>AddZbSecrets</c> internally, so the two
|
||||
/// branches are alternatives, never additive — calling both would double-register.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
/// <param name="services">The service collection to add to.</param>
|
||||
/// <param name="configuration">The application configuration.</param>
|
||||
/// <returns>The same <paramref name="services"/> instance, for chaining.</returns>
|
||||
public static IServiceCollection AddOtOpcUaSecrets(
|
||||
this IServiceCollection services,
|
||||
IConfiguration configuration)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(services);
|
||||
ArgumentNullException.ThrowIfNull(configuration);
|
||||
|
||||
if (!IsReplicationEnabled(configuration))
|
||||
{
|
||||
services.AddZbSecrets(configuration, SecretsSectionPath);
|
||||
return services;
|
||||
}
|
||||
|
||||
services.AddZbSecretsAkkaReplication(configuration, SecretsSectionPath, ReplicationSectionPath);
|
||||
|
||||
// Anti-entropy participation must not hinge on this node happening to touch a secret — see
|
||||
// SecretReplicationStarter for why the library's lazy actor creation is insufficient here.
|
||||
services.AddSingleton<SecretReplicationStarter>();
|
||||
services.AddHostedService(sp => sp.GetRequiredService<SecretReplicationStarter>());
|
||||
|
||||
return services;
|
||||
}
|
||||
}
|
||||
@@ -41,6 +41,7 @@ using Microsoft.Extensions.DependencyInjection;
|
||||
using ZB.MOM.WW.Secrets.Abstractions;
|
||||
using ZB.MOM.WW.Secrets.Configuration;
|
||||
using ZB.MOM.WW.Secrets.DependencyInjection;
|
||||
using ZB.MOM.WW.Secrets.Replicator.AkkaDotNet;
|
||||
using ZB.MOM.WW.Secrets.Sqlite;
|
||||
|
||||
// Roles drive the entire conditional wiring below — see ZB.MOM.WW.OtOpcUa.Cluster.RoleParser.
|
||||
@@ -313,6 +314,14 @@ if (hasDriver)
|
||||
builder.Services.AddAkka("otopcua", (ab, sp) =>
|
||||
{
|
||||
ab.WithOtOpcUaClusterBootstrap(sp);
|
||||
// Secret-replication wire protocol → its own serializer, merged only when replication is on so a
|
||||
// non-replicating node carries no serializer bindings for messages it will never see. Appended
|
||||
// (Akka.Hosting's fallback merge — the same mode WithOtOpcUaClusterBootstrap uses for the base
|
||||
// config) rather than a raw Config.WithFallback, which would fight the builder's own assembly.
|
||||
// Without this the protocol DTOs would round-trip on Akka's default JSON serializer, leaving the
|
||||
// serializer that carries secret ciphertext an inherited default rather than an explicit choice.
|
||||
if (SecretsRegistration.IsReplicationEnabled(builder.Configuration))
|
||||
ab.AddHocon(AkkaSecretsReplication.SerializationConfig, HoconAddMode.Append);
|
||||
if (hasAdmin)
|
||||
{
|
||||
ab.WithOtOpcUaControlPlaneSingletons();
|
||||
@@ -350,7 +359,8 @@ if (hasAdmin)
|
||||
}
|
||||
|
||||
// Registered unconditionally: driver-role nodes resolve Layer-B DriverConfig secrets and have no auth/DP/AdminUI.
|
||||
builder.Services.AddZbSecrets(builder.Configuration, "Secrets");
|
||||
// Cluster replication is opt-in behind Secrets:Replication:Enabled (default false) — see SecretsRegistration.
|
||||
builder.Services.AddOtOpcUaSecrets(builder.Configuration);
|
||||
|
||||
builder.Services.AddOtOpcUaHealth();
|
||||
builder.Services.AddOtOpcUaObservability(builder.Configuration);
|
||||
|
||||
@@ -18,7 +18,13 @@
|
||||
"EnvVarName": "ZB_SECRETS_MASTER_KEY"
|
||||
},
|
||||
"RunMigrationsOnStartup": true,
|
||||
"ResolveCacheTtl": "00:00:30"
|
||||
"ResolveCacheTtl": "00:00:30",
|
||||
"Replication": {
|
||||
"_comment": "Peer-to-peer secret replication over the Akka cluster. OPT-IN: Enabled=false keeps the plain local SQLite store on every node. KNOWN NON-FUNCTIONAL against ZB.MOM.WW.Secrets.Replicator.AkkaDotNet 0.2.0 — that version's ISecretReplicator registration is shadowed by the NoOp one AddZbSecrets registers first, so enabling this decorates the store but replicates nothing and starts no actor. Do not enable until the library is fixed. All nodes must also share the same KEK.",
|
||||
"Enabled": false,
|
||||
"AnnounceInterval": "00:00:30",
|
||||
"ActorName": "zb-secret-replication"
|
||||
}
|
||||
},
|
||||
"ServerHistorian": {
|
||||
"_comment": "Server-side HistoryRead backend (the ZB.MOM.WW.HistorianGateway gRPC client). Disabled => NullHistorianDataSource (historized nodes return GoodNoData). The gateway must run RuntimeDb:EventReadsEnabled=true for alarm-history ReadEvents, and the API key must carry historian:read + historian:write + historian:tags:write scopes.",
|
||||
|
||||
+168
@@ -0,0 +1,168 @@
|
||||
using Akka.Actor;
|
||||
using Microsoft.Extensions.Configuration;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Hosting;
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.OtOpcUa.Host.Configuration;
|
||||
using ZB.MOM.WW.Secrets.Abstractions;
|
||||
using ZB.MOM.WW.Secrets.Replication;
|
||||
using ZB.MOM.WW.Secrets.Replicator.AkkaDotNet;
|
||||
using ZB.MOM.WW.Secrets.Sqlite;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests;
|
||||
|
||||
/// <summary>
|
||||
/// Guards the production DI wiring for cluster secret replication (<c>AddOtOpcUaSecrets</c>).
|
||||
/// This is the registration half of a defect class that has already shipped once in this library
|
||||
/// family: every unit test passed because nothing ever built a container, so a decorator that
|
||||
/// could not actually be constructed went unnoticed.
|
||||
///
|
||||
/// Two properties matter here, and both are load-bearing on <b>driver-role</b> nodes that have no
|
||||
/// auth/AdminUI surface and whose only symptom of a broken registration is drivers failing to
|
||||
/// open sessions:
|
||||
/// <list type="number">
|
||||
/// <item>
|
||||
/// With replication off (the default), <see cref="ISecretStore"/> must resolve to the
|
||||
/// plain <see cref="SqliteSecretStore"/> — byte-identical to the pre-replication host.
|
||||
/// </item>
|
||||
/// <item>
|
||||
/// With replication on, <see cref="ISecretStore"/> must resolve to
|
||||
/// <see cref="ReplicatingSecretStore"/> <b>and</b> the undecorated concrete
|
||||
/// <see cref="SqliteSecretStore"/> must still resolve, because the decorator is
|
||||
/// constructed from it. A missing concrete registration is the exact gap that shipped.
|
||||
/// </item>
|
||||
/// </list>
|
||||
/// </summary>
|
||||
public sealed class SecretsReplicationRegistrationTests
|
||||
{
|
||||
/// <summary>
|
||||
/// Builds a container mirroring the host's registration order: Akka first (the host calls
|
||||
/// <c>AddAkka</c> before <c>AddOtOpcUaSecrets</c>), then secrets.
|
||||
/// </summary>
|
||||
private static ServiceProvider BuildProvider(bool replicationEnabled, ActorSystem? actorSystem = null)
|
||||
{
|
||||
var configuration = new ConfigurationBuilder()
|
||||
.AddInMemoryCollection(new Dictionary<string, string?>
|
||||
{
|
||||
// A per-test SQLite path keeps these hermetic; nothing here opens the file, because
|
||||
// resolving the store does not touch the database.
|
||||
["Secrets:SqlitePath"] = Path.Combine(Path.GetTempPath(), $"otopcua-secrets-{Guid.NewGuid():N}.db"),
|
||||
["Secrets:MasterKey:Source"] = "Environment",
|
||||
["Secrets:MasterKey:EnvVarName"] = "ZB_SECRETS_MASTER_KEY",
|
||||
["Secrets:RunMigrationsOnStartup"] = "false",
|
||||
["Secrets:Replication:Enabled"] = replicationEnabled ? "true" : "false",
|
||||
["Secrets:Replication:AnnounceInterval"] = "00:00:30",
|
||||
["Secrets:Replication:ActorName"] = "zb-secret-replication",
|
||||
})
|
||||
.Build();
|
||||
|
||||
var services = new ServiceCollection();
|
||||
services.AddLogging();
|
||||
|
||||
// The replicating store reaches the ActorSystem through SecretReplicationActorProvider.
|
||||
// The host supplies one via Akka.Hosting; a plain system is equivalent for registration.
|
||||
if (actorSystem is not null)
|
||||
services.AddSingleton(actorSystem);
|
||||
|
||||
services.AddOtOpcUaSecrets(configuration);
|
||||
|
||||
return services.BuildServiceProvider();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Replication_disabled_resolves_the_plain_sqlite_store()
|
||||
{
|
||||
using var sp = BuildProvider(replicationEnabled: false);
|
||||
|
||||
var store = sp.GetRequiredService<ISecretStore>();
|
||||
|
||||
store.ShouldBeOfType<SqliteSecretStore>(
|
||||
"replication is opt-in — with Secrets:Replication:Enabled false the host must resolve the "
|
||||
+ "plain local store, so existing nodes are behaviourally unchanged");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Replication_disabled_does_not_require_an_actor_system()
|
||||
{
|
||||
// A driver-role node builds its container before the ActorSystem is reachable for secret
|
||||
// resolution; the default path must not have taken a dependency on it.
|
||||
using var sp = BuildProvider(replicationEnabled: false);
|
||||
|
||||
Should.NotThrow(() => sp.GetRequiredService<ISecretStore>());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Replication_enabled_resolves_the_replicating_store()
|
||||
{
|
||||
using var system = ActorSystem.Create("secrets-registration-test-enabled");
|
||||
using var sp = BuildProvider(replicationEnabled: true, system);
|
||||
|
||||
var store = sp.GetRequiredService<ISecretStore>();
|
||||
|
||||
store.ShouldBeOfType<ReplicatingSecretStore>(
|
||||
"with replication enabled the local store must be decorated so writes publish to peers");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Replication_enabled_still_resolves_the_undecorated_concrete_store()
|
||||
{
|
||||
using var system = ActorSystem.Create("secrets-registration-test-undecorated");
|
||||
using var sp = BuildProvider(replicationEnabled: true, system);
|
||||
|
||||
// The decorator is constructed from the concrete store, not from ISecretStore (which would
|
||||
// recurse). If this registration is ever lost the decorator cannot be built at all.
|
||||
var concrete = sp.GetRequiredService<SqliteSecretStore>();
|
||||
|
||||
concrete.ShouldNotBeNull();
|
||||
sp.GetRequiredService<ISecretStore>().ShouldNotBeSameAs(concrete);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Replication_enabled_registers_a_startup_hook_that_creates_the_replication_actor()
|
||||
{
|
||||
using var system = ActorSystem.Create("secrets-registration-test-actor");
|
||||
using var sp = BuildProvider(replicationEnabled: true, system);
|
||||
|
||||
// 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();
|
||||
|
||||
starter.ShouldNotBeNull(
|
||||
"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 "
|
||||
+ "ISecretReplicator, so the replication actor is never created. AddZbSecretsAkkaReplication "
|
||||
+ "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");
|
||||
using var sp = BuildProvider(replicationEnabled: true, system);
|
||||
|
||||
var starter = sp.GetServices<IHostedService>().OfType<SecretReplicationStarter>().Single();
|
||||
await starter.StartAsync(TestContext.Current.CancellationToken);
|
||||
|
||||
// Verify the actor exists rather than assuming resolution created it: ActorSelection
|
||||
// resolution succeeds only if a live actor occupies the configured name under /user.
|
||||
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]
|
||||
public void Replication_disabled_registers_no_startup_hook()
|
||||
{
|
||||
using var sp = BuildProvider(replicationEnabled: false);
|
||||
|
||||
sp.GetServices<IHostedService>().OfType<SecretReplicationStarter>().ShouldBeEmpty();
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user