From b1b49bb36ca707ee8682ae29a38d8c7490f0bc30 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 28 Sep 2026 21:08:34 -0400 Subject: [PATCH] fix(site-secrets): mirror secrets before pushing artifact data connections to the DCL Final review M2: HandleDeployArtifacts now sends ApplyArtifactDataConnectionsToDcl after the shipped secrets are written and mirrored (from a finally, so the push still goes out if the secrets step fails), so a connection created by the same deploy resolves a newly shipped secret on first connect. Definitions are still stored at the same point; response semantics unchanged. --- .../Actors/DeploymentManagerActor.cs | 21 ++++++- ...loymentManagerActorArtifactSecretsTests.cs | 60 ++++++++++++++++++- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs index d174c266..7a164cdd 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs @@ -3231,6 +3231,13 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers Task.Run(async () => { + // M2 (final review): the DCL push for this deploy's data connections is captured + // here and sent only AFTER the shipped secrets are written and mirrored (in the + // finally below), so a connection whose config references a secret this same deploy + // ships resolves it on its FIRST connect instead of failing "not found" and waiting + // out the reconnect interval. The definitions are still stored in SQLite at the same + // point as before, and the push still goes out if the secrets step throws. + ApplyArtifactDataConnectionsToDcl? pendingDclApply = null; try { // Each non-null artifact list is the COMPLETE system-wide set @@ -3328,8 +3335,9 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers // artifact bundle. Reconciled removals ride the same // message so the live DCL connection actor is stopped // on the actor thread alongside the hash-cache eviction. - self.Tell(new ApplyArtifactDataConnectionsToDcl( - command.DataConnections, removedConnections)); + // Sent after the secrets step — see pendingDclApply above (M2). + pendingDclApply = new ApplyArtifactDataConnectionsToDcl( + command.DataConnections, removedConnections); } // SMTP configuration is @@ -3417,6 +3425,15 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers return new ArtifactDeploymentResponse( command.DeploymentId, "", false, ex.Message, DateTimeOffset.UtcNow); } + finally + { + // Runs before the Task completes, so the DCL push is still enqueued ahead of the + // PipeTo reply, as it was before the reorder. + if (pendingDclApply != null) + { + self.Tell(pendingDclApply); + } + } }).PipeTo(sender); } diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerActorArtifactSecretsTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerActorArtifactSecretsTests.cs index cf705226..92915da8 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerActorArtifactSecretsTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerActorArtifactSecretsTests.cs @@ -1,4 +1,5 @@ using Akka.Actor; +using Akka.TestKit; using Akka.TestKit.Xunit2; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; @@ -47,7 +48,7 @@ public class DeploymentManagerActorArtifactSecretsTests : TestKit, IDisposable TestLocalDb.DeleteFiles(path); } - private IActorRef CreateDeploymentManager(DeployedSecretsMirror? secretsMirror = null) + private IActorRef CreateDeploymentManager(DeployedSecretsMirror? secretsMirror = null, IActorRef? dclManager = null) { var options = new SiteRuntimeOptions(); // Positional, not named, past the constructor's first six parameters: a named @@ -62,7 +63,7 @@ public class DeploymentManagerActorArtifactSecretsTests : TestKit, IDisposable null, // streamManager: no stream manager in tests options, NullLogger.Instance, - null, // dclManager + dclManager, null, // healthCollector null, // serviceProvider null, // loggerFactory @@ -245,6 +246,61 @@ public class DeploymentManagerActorArtifactSecretsTests : TestKit, IDisposable Assert.Null(r2.ErrorMessage); } + [Fact] + public void DataConnections_PushedToDcl_OnlyAfterShippedSecretsAreMirrored() + { + // M2 (final review): a connection whose config references a secret shipped by THIS + // deploy must find it on its first connect, so the DCL create must not reach the + // DCL manager before the secret is in this node's store. + var store = new FakeSecretStore(); + var resolver = new FakeSecretResolver(); + var mirror = CreateMirror(store, resolver); + var dcl = CreateTestProbe(); + bool? secretPresentWhenDclSawCreate = null; + dcl.SetAutoPilot(new DelegateAutoPilot((_, msg) => + { + if (msg is ZB.MOM.WW.ScadaBridge.Commons.Messages.DataConnection.CreateConnectionCommand) + { + secretPresentWhenDclSawCreate ??= store.Contains("gw-key"); + } + + return AutoPilot.KeepRunning; + })); + var manager = CreateDeploymentManager(mirror, dcl.Ref); + + var command = ArtifactsCommand("dep-order", secrets: [Art("gw-key", 1)]) with + { + DataConnections = [new DataConnectionArtifact("Gw", "OpcUa", "{}", null, 3)], + }; + manager.Tell(command); + + var response = ExpectMsg(TimeSpan.FromSeconds(10)); + Assert.True(response.Success); + Assert.Equal(1, response.SecretsApplied); + AwaitCondition(() => secretPresentWhenDclSawCreate.HasValue, TimeSpan.FromSeconds(10)); + Assert.True(secretPresentWhenDclSawCreate); + } + + [Fact] + public async Task DataConnections_StillStoredAndPushed_WhenSecretsStepFails() + { + // The reorder must not lose the connection push when the secrets step fails. + var dcl = CreateTestProbe(); + var manager = CreateDeploymentManager(secretsMirror: null, dclManager: dcl.Ref); + + var command = ArtifactsCommand("dep-order-fail", secrets: [Art("gw-key", 1)]) with + { + DataConnections = [new DataConnectionArtifact("Gw", "OpcUa", "{}", null, 3)], + }; + manager.Tell(command); + + var response = ExpectMsg(TimeSpan.FromSeconds(10)); + Assert.False(response.Success); + dcl.ExpectMsg( + TimeSpan.FromSeconds(10)); + Assert.Contains(await _storage.GetAllDataConnectionDefinitionsAsync(), d => d.Name == "Gw"); + } + // ── Fakes (mirrors DeployedSecretsMirrorTests' fakes; kept local/minimal per-task) ── private sealed class FakeSecretStore : ISecretStore