refactor(host): remove legacy site-psk-invalidation topic (#105)
Removes the #96 compatibility path that the unified central-cache-invalidation topic (#100 item 3, 153609ef; compat3a31dd2d) carried for one release: - CentralCacheInvalidationBus no longer publishes a bare SitePskInvalidated on site-psk-invalidation; LegacySitePskTopic constant deleted. - CentralCacheInvalidationSubscriber no longer subscribes to that topic or handles a bare SitePskInvalidated; it applies envelopes only. central-cache-invalidation behaviour is unchanged (one envelope per invalidation, origin filtering, per-handler fault isolation). Gate met:8e32f353(carries153609efand3a31dd2d) runs on every central node - DEV vd03 (record581e25ca) and QA vq03/vq04 (record8ba5d5d7). Mixed-version / rolling-upgrade constraint: a central node built before153609eflistens only on site-psk-invalidation and no longer receives PSK invalidations from a newer peer (only its TTL and the site-rejection refresh bound the staleness). Mixed pairs across that boundary are no longer supported; central pairs are stopped and started together anyway. Documented in Component-Host.md and Component-Communication.md. Tests: the legacy publish/subscribe tests are replaced by PskRotation_OnOneNode_ReachesThePeersCaches_ThroughTheUnifiedTopic (node A publishes, node B's PSK cache and secret-resolver cache are invalidated, and nothing else is published) and a bare-message-ignored test with an envelope positive control. The two-node cluster test now asserts the PSK invalidation arrives exactly once and the publisher applies nothing. Claude-Session: https://claude.ai/code/session_01GT67ACmpMR7b9LGRPLijMi
This commit is contained in:
Joseph Doherty
@@ -208,8 +208,8 @@ Central caches each resolved key, and drops the entry (Gitea #96) when: the site
|
||||
secret `SB-GRPC-PSK-<siteId>` is written through central's secrets write path (`/admin/secrets`) —
|
||||
`SitePskRotationInvalidator` wraps the secrets library's `ISecretCacheInvalidator` and also
|
||||
broadcasts a `SitePskInvalidated` on the central cache-invalidation topic `central-cache-invalidation`
|
||||
(ScadaBridge#100 item 3; the #96 topic `site-psk-invalidation` is kept for one release for
|
||||
mixed-version pairs — see Component-Host.md) so the peer central node drops its copy too; the site rejects a call with `PermissionDenied` /
|
||||
(ScadaBridge#100 item 3; the #96 topic `site-psk-invalidation` was removed in ScadaBridge#105, so
|
||||
both central nodes must run a build carrying the unified topic — see Component-Host.md) so the peer central node drops its copy too; the site rejects a call with `PermissionDenied` /
|
||||
`Unauthenticated` (`PskRejectionRefreshHandler`, wrapped around every per-site channel by
|
||||
`WithSiteCredentials`, so the next call re-reads the key — this covers keys rotated outside the
|
||||
process, e.g. by the `secret` CLI, after at most the resolver's `Secrets:ResolveCacheTtl`); a
|
||||
|
||||
@@ -355,5 +355,5 @@ built)* section for where the shipped code deviates from the original design).
|
||||
- **Inbound method warm-up is a hosted service** (`InboundMethodWarmupService`, registered after `AkkaHostedService`), not a pre-`RunAsync` loop — see Component-InboundAPI.md.
|
||||
- **`SitePskProvider` distinguishes a secret-store fault from a missing secret.** A store fault logs a Warning with the exception and throws `SecretStoreUnavailableException` (not cached); only a genuinely absent/empty/tombstoned secret produces the "seed `SB-GRPC-PSK-{site}`" message.
|
||||
- **`SitePskProvider`'s cache follows an in-place rotation (Gitea #96).** Central replaces the secrets library's `ISecretCacheInvalidator` with `SitePskRotationInvalidator` (`AddSitePskRotationInvalidation`), which drops the site's cached key when `SB-GRPC-PSK-{site}` is written and publishes `SitePskInvalidated` to the peer over the central cache-invalidation topic (below). A site's auth rejection and a `SitePsks` reload also drop the cache — see Component-Communication.md → *Authentication*.
|
||||
- **Central cache invalidation (ScadaBridge#100 item 3).** One `DistributedPubSub` topic, `central-cache-invalidation`, carries typed `ICentralCacheInvalidation` messages (Commons `Messages/Communication/CentralCacheInvalidations.cs`) between the two central nodes, each wrapped in a `CentralCacheInvalidationEnvelope` stamped with the publishing process's origin id. `AddCentralCacheInvalidation` registers `CentralCacheInvalidationBus` as `ICentralCacheInvalidationPublisher`; `AkkaHostedService.RegisterCentralActors` gives it the mediator and starts `CentralCacheInvalidationSubscriber`, which skips its own node's envelopes and hands the peer's to every registered `ICentralCacheInvalidationHandler`: `SitePskRotationInvalidator` (`SitePskInvalidated`, #96), `RoleMappingCache` (`RoleMappingsInvalidated`, audit D1), `ExternalDefinitionLookupCache` (`ExternalDefinitionsInvalidated`, audit D3 / #83) and the Inbound API's `InboundApiCacheInvalidationHandler` (`InboundApiMethodInvalidated` — compiled handler, known-bad record and cached row of one method, #103). Publishers invalidate locally first, post-commit, then publish; delivery is best-effort, so each cache's TTL or revision check stays the fallback. Migration: the #96 topic `site-psk-invalidation` is still published to (bare `SitePskInvalidated`) and subscribed, so a briefly mixed-version central pair keeps exchanging PSK invalidations — remove it once every central node runs this build.
|
||||
- **Central cache invalidation (ScadaBridge#100 item 3).** One `DistributedPubSub` topic, `central-cache-invalidation`, carries typed `ICentralCacheInvalidation` messages (Commons `Messages/Communication/CentralCacheInvalidations.cs`) between the two central nodes, each wrapped in a `CentralCacheInvalidationEnvelope` stamped with the publishing process's origin id. `AddCentralCacheInvalidation` registers `CentralCacheInvalidationBus` as `ICentralCacheInvalidationPublisher`; `AkkaHostedService.RegisterCentralActors` gives it the mediator and starts `CentralCacheInvalidationSubscriber`, which skips its own node's envelopes and hands the peer's to every registered `ICentralCacheInvalidationHandler`: `SitePskRotationInvalidator` (`SitePskInvalidated`, #96), `RoleMappingCache` (`RoleMappingsInvalidated`, audit D1), `ExternalDefinitionLookupCache` (`ExternalDefinitionsInvalidated`, audit D3 / #83) and the Inbound API's `InboundApiCacheInvalidationHandler` (`InboundApiMethodInvalidated` — compiled handler, known-bad record and cached row of one method, #103). Publishers invalidate locally first, post-commit, then publish; delivery is best-effort, so each cache's TTL or revision check stays the fallback. The #96 topic `site-psk-invalidation` (bare `SitePskInvalidated`), kept for one release as a mixed-version bridge, was removed in ScadaBridge#105 once every central node (DEV vd03, QA vq03/vq04) ran `8e32f353`, which carries the unified topic. **Rolling-upgrade constraint:** a central node built before ScadaBridge#100 item 3 (`153609ef`) listens only on the removed topic, so it no longer receives PSK invalidations from a newer peer (only its TTL and the site-rejection refresh bound its staleness); both central nodes of a pair must run a build that carries `central-cache-invalidation`. The stop-both/start-both upgrade of a central pair already guarantees this — never run a central pair half-upgraded across that boundary.
|
||||
- Central registers `ExternalSystemGatewayOptions.DefinitionCacheTtl = 10 s` ahead of `AddExternalSystemGateway()` so an explicit config value wins; sites leave it off.
|
||||
|
||||
@@ -26,11 +26,13 @@ namespace ZB.MOM.WW.ScadaBridge.Host;
|
||||
/// the subscriber skips its own.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Migration from <c>site-psk-invalidation</c>.</b> A <see cref="SitePskInvalidated"/> is also
|
||||
/// published, bare, on <see cref="LegacySitePskTopic"/>, and the subscriber still listens there,
|
||||
/// so a central pair that is briefly mixed-version during an upgrade keeps exchanging PSK
|
||||
/// invalidations. Applying a PSK invalidation twice is harmless. Remove both once every central
|
||||
/// node runs this build.
|
||||
/// <b>No legacy topic.</b> The #96 per-cache topic <c>site-psk-invalidation</c> was published to
|
||||
/// and subscribed for one release as a mixed-version bridge; it was removed in ScadaBridge#105
|
||||
/// once every central node ran a build with this topic. A central node built before
|
||||
/// ScadaBridge#100 item 3 listens only on the old topic and so no longer receives PSK
|
||||
/// invalidations from a newer peer (its TTL still bounds the staleness): the two central nodes
|
||||
/// of a pair must run builds that both carry this topic, which the stop-both/start-both
|
||||
/// upgrade of a central pair already guarantees.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
public sealed class CentralCacheInvalidationBus : ICentralCacheInvalidationPublisher
|
||||
@@ -38,9 +40,6 @@ public sealed class CentralCacheInvalidationBus : ICentralCacheInvalidationPubli
|
||||
/// <summary>The single central cache-invalidation topic.</summary>
|
||||
public const string Topic = "central-cache-invalidation";
|
||||
|
||||
/// <summary>The #96 per-cache topic, kept for mixed-version central pairs (see remarks).</summary>
|
||||
public const string LegacySitePskTopic = "site-psk-invalidation";
|
||||
|
||||
private readonly ILogger<CentralCacheInvalidationBus> _logger;
|
||||
private volatile IActorRef? _mediator;
|
||||
|
||||
@@ -77,10 +76,6 @@ public sealed class CentralCacheInvalidationBus : ICentralCacheInvalidationPubli
|
||||
try
|
||||
{
|
||||
mediator.Tell(new Publish(Topic, new CentralCacheInvalidationEnvelope(OriginId, invalidation)));
|
||||
if (invalidation is SitePskInvalidated psk)
|
||||
{
|
||||
mediator.Tell(new Publish(LegacySitePskTopic, psk));
|
||||
}
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
@@ -101,8 +96,7 @@ public sealed class CentralCacheInvalidationBus : ICentralCacheInvalidationPubli
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Central actor that subscribes to <see cref="CentralCacheInvalidationBus.Topic"/> (and the
|
||||
/// legacy <see cref="CentralCacheInvalidationBus.LegacySitePskTopic"/>) and hands every
|
||||
/// Central actor that subscribes to <see cref="CentralCacheInvalidationBus.Topic"/> and hands every
|
||||
/// invalidation published by the peer central node to this node's
|
||||
/// <see cref="ICentralCacheInvalidationHandler"/>s.
|
||||
/// </summary>
|
||||
@@ -127,8 +121,6 @@ public sealed class CentralCacheInvalidationSubscriber : ReceiveActor
|
||||
Apply(envelope.Invalidation);
|
||||
}
|
||||
});
|
||||
// Bare message on the #96 topic, from a peer that predates the shared topic.
|
||||
Receive<SitePskInvalidated>(Apply);
|
||||
Receive<SubscribeAck>(_ => { });
|
||||
}
|
||||
|
||||
@@ -156,7 +148,6 @@ public sealed class CentralCacheInvalidationSubscriber : ReceiveActor
|
||||
{
|
||||
var mediator = DistributedPubSub.Get(Context.System).Mediator;
|
||||
mediator.Tell(new Subscribe(CentralCacheInvalidationBus.Topic, Self));
|
||||
mediator.Tell(new Subscribe(CentralCacheInvalidationBus.LegacySitePskTopic, Self));
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
|
||||
@@ -108,9 +108,9 @@ public sealed class SitePskRotationInvalidator : ISecretCacheInvalidator, ICentr
|
||||
|
||||
/// <summary>
|
||||
/// Peer message: the gRPC preshared key of <paramref name="SiteId"/> was rotated on a central node.
|
||||
/// Rides the central cache-invalidation topic; kept in this namespace because the #96
|
||||
/// <c>site-psk-invalidation</c> topic carries it bare by this type name (see
|
||||
/// <see cref="CentralCacheInvalidationBus"/>).
|
||||
/// Rides the central cache-invalidation topic inside a <see cref="CentralCacheInvalidationEnvelope"/>
|
||||
/// (see <see cref="CentralCacheInvalidationBus"/>); the #96 bare <c>site-psk-invalidation</c>
|
||||
/// topic was removed in ScadaBridge#105.
|
||||
/// </summary>
|
||||
/// <param name="SiteId">The site whose key changed.</param>
|
||||
public sealed record SitePskInvalidated(string SiteId) : ICentralCacheInvalidation;
|
||||
|
||||
@@ -44,7 +44,7 @@ public class CentralCacheInvalidationBusTests : TestKit
|
||||
var envelope = Assert.IsType<CentralCacheInvalidationEnvelope>(publish.Message);
|
||||
Assert.Equal(_bus.OriginId, envelope.OriginId);
|
||||
Assert.IsType<RoleMappingsInvalidated>(envelope.Invalidation);
|
||||
mediator.ExpectNoMsg(TimeSpan.FromMilliseconds(100)); // only PSK also uses the legacy topic
|
||||
mediator.ExpectNoMsg(TimeSpan.FromMilliseconds(100)); // one topic, one publish (#105)
|
||||
}
|
||||
|
||||
/// <summary>Before the mediator exists publishing is a silent no-op.</summary>
|
||||
|
||||
@@ -26,8 +26,8 @@ public class SitePskRotationInvalidatorTests : TestKit
|
||||
|
||||
/// <summary>
|
||||
/// Verifies that writing a site's PSK secret evicts the resolver cache, drops the site's
|
||||
/// cached key and publishes the invalidation on the shared topic and, for a mixed-version
|
||||
/// peer, bare on the legacy #96 topic.
|
||||
/// cached key and publishes the invalidation once, on the shared topic only (the #96
|
||||
/// <c>site-psk-invalidation</c> topic was removed in ScadaBridge#105).
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void WritingAPskSecret_DropsTheCachedKey_AndTellsThePeer()
|
||||
@@ -46,9 +46,7 @@ public class SitePskRotationInvalidatorTests : TestKit
|
||||
var envelope = Assert.IsType<CentralCacheInvalidationEnvelope>(shared.Message);
|
||||
Assert.Equal(new SitePskInvalidated("site-a"), envelope.Invalidation);
|
||||
Assert.Equal(_bus.OriginId, envelope.OriginId);
|
||||
var legacy = mediator.ExpectMsg<Publish>();
|
||||
Assert.Equal("site-psk-invalidation", legacy.Topic);
|
||||
Assert.Equal(new SitePskInvalidated("site-a"), legacy.Message);
|
||||
mediator.ExpectNoMsg(TimeSpan.FromMilliseconds(200));
|
||||
}
|
||||
|
||||
/// <summary>Negative control: any other secret only evicts the resolver cache.</summary>
|
||||
@@ -101,18 +99,58 @@ public class SitePskRotationInvalidatorTests : TestKit
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Migration: a bare <see cref="SitePskInvalidated"/> from a peer still on the #96 topic is
|
||||
/// applied too.
|
||||
/// ScadaBridge#105: a PSK rotation written on one central node reaches the peer node's PSK
|
||||
/// cache and its secret-resolver cache through the unified topic alone. Node A's invalidator
|
||||
/// publishes; whatever node A puts on the mediator is delivered to node B's subscriber exactly
|
||||
/// as <c>DistributedPubSub</c> would deliver it to a subscriber of that topic.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Subscriber_AppliesALegacyBarePskInvalidation()
|
||||
public void PskRotation_OnOneNode_ReachesThePeersCaches_ThroughTheUnifiedTopic()
|
||||
{
|
||||
// Node A: the node the secret is written on.
|
||||
var mediatorA = CreateTestProbe();
|
||||
_bus.SetMediator(mediatorA.Ref);
|
||||
var invalidatorA = Create(_bus);
|
||||
|
||||
// Node B: the peer, with its own bus (own origin id) and its own caches.
|
||||
var innerB = Substitute.For<ISecretCacheInvalidator>();
|
||||
var pskB = Substitute.For<ISitePskProvider>();
|
||||
var busB = new CentralCacheInvalidationBus(NullLogger<CentralCacheInvalidationBus>.Instance);
|
||||
var invalidatorB = new SitePskRotationInvalidator(innerB, pskB, busB, NullLogger<SitePskRotationInvalidator>.Instance);
|
||||
var subscriberB = Sys.ActorOf(Props.Create(() => new CentralCacheInvalidationSubscriber(
|
||||
busB, new ICentralCacheInvalidationHandler[] { invalidatorB })));
|
||||
|
||||
invalidatorA.Invalidate(new SecretName("SB-GRPC-PSK-site-d"));
|
||||
|
||||
// Deliver every publish on the unified topic to B; nothing else is published.
|
||||
var publish = mediatorA.ExpectMsg<Publish>();
|
||||
Assert.Equal("central-cache-invalidation", publish.Topic);
|
||||
subscriberB.Tell(publish.Message);
|
||||
mediatorA.ExpectNoMsg(TimeSpan.FromMilliseconds(200));
|
||||
|
||||
AwaitAssert(() =>
|
||||
{
|
||||
pskB.Received(1).Invalidate("site-d");
|
||||
innerB.Received(1).Invalidate(new SecretName("SB-GRPC-PSK-site-d"));
|
||||
});
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// ScadaBridge#105: the bare <see cref="SitePskInvalidated"/> that the removed #96 topic
|
||||
/// carried is no longer handled — the subscriber only applies envelopes. The envelope row is
|
||||
/// the positive control.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Subscriber_IgnoresABarePskInvalidation_ButAppliesTheEnvelope()
|
||||
{
|
||||
var invalidator = Create(_bus);
|
||||
var subscriber = Sys.ActorOf(Props.Create(() => new CentralCacheInvalidationSubscriber(
|
||||
_bus, new ICentralCacheInvalidationHandler[] { invalidator })));
|
||||
|
||||
subscriber.Tell(new SitePskInvalidated("site-c"));
|
||||
subscriber.Tell(new CentralCacheInvalidationEnvelope("peer-node", new SitePskInvalidated("site-e")));
|
||||
|
||||
AwaitAssert(() => _psk.Received(1).Invalidate("site-c"));
|
||||
AwaitAssert(() => _psk.Received(1).Invalidate("site-e"));
|
||||
_psk.DidNotReceive().Invalidate("site-c");
|
||||
}
|
||||
}
|
||||
|
||||
+7
-6
@@ -76,12 +76,13 @@ public class CentralCacheInvalidationTwoNodeTests
|
||||
Assert.Contains(applied, i => i is RoleMappingsInvalidated);
|
||||
Assert.Contains(new ExternalDefinitionsInvalidated(), applied);
|
||||
Assert.Contains(new InboundApiMethodInvalidated("MesMoveIn"), applied);
|
||||
// The PSK invalidation arrives twice during the migration window (shared + legacy topic).
|
||||
Assert.Contains(new SitePskInvalidated("site-a"), applied);
|
||||
|
||||
// The publisher skips its own envelopes; only the bare legacy PSK message (no origin)
|
||||
// comes back to it, which is the pre-existing #96 behaviour and idempotent.
|
||||
// One topic only since ScadaBridge#105: the PSK invalidation arrives exactly once.
|
||||
await Task.Delay(500);
|
||||
Assert.All(a.Handler.Applied, i => Assert.IsType<SitePskInvalidated>(i));
|
||||
Assert.Single(b.Handler.Applied, i => i is SitePskInvalidated);
|
||||
Assert.Contains(new SitePskInvalidated("site-a"), b.Handler.Applied);
|
||||
|
||||
// The publisher skips its own envelopes, and with the legacy bare PSK topic gone nothing
|
||||
// comes back to it at all.
|
||||
Assert.Empty(a.Handler.Applied);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user