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.
2026-09-28 21:08:34 -04:00
parent 6f1787abe1
commit b1b49bb36c
2 changed files with 77 additions and 4 deletions
@@ -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);
    }

@@ -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<DeploymentManagerActor>.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<ArtifactDeploymentResponse>(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<ArtifactDeploymentResponse>(TimeSpan.FromSeconds(10));
+        Assert.False(response.Success);
+        dcl.ExpectMsg<ZB.MOM.WW.ScadaBridge.Commons.Messages.DataConnection.CreateConnectionCommand>(
+            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