diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Grpc/DeploymentArtifactService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Grpc/DeploymentArtifactService.cs index 2aa6eab2..6b634233 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Grpc/DeploymentArtifactService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Grpc/DeploymentArtifactService.cs @@ -4,7 +4,6 @@ using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; using ZB.MOM.WW.OtOpcUa.Commons.Protos.DeploymentArtifact.V1; using ZB.MOM.WW.OtOpcUa.Configuration; -using ZB.MOM.WW.OtOpcUa.Configuration.Enums; // The generated service container is itself named DeploymentArtifactService; alias its nested server // base so this impl can keep the natural name without colliding with the generated type. using GeneratedServiceBase = @@ -13,17 +12,27 @@ using GeneratedServiceBase = namespace ZB.MOM.WW.OtOpcUa.AdminUI.Grpc; /// -/// Central-side artifact serve (per-cluster mesh Phase 3). Streams a sealed deployment's +/// Central-side artifact serve (per-cluster mesh Phase 3). Streams a deployment's /// ArtifactBlob to a driver node fetching in FetchAndCache mode. Registered only on /// admin-role nodes (which hold the central SQL connection) and only when /// ConfigServe:GrpcListenPort > 0. /// /// /// -/// NotFound is deliberately indistinguishable across three cases — unknown id, a -/// deployment that is not sealed, and an empty blob. The first is existence-hiding; the last -/// is the #485 rule (a zero-length artifact is "no answer," never "a configuration with no -/// drivers"). Streaming empty bytes as if valid is exactly what that defect class forbids. +/// Serves any deployment that has a non-empty blob — status is NOT gated. A node +/// fetches the artifact during apply, before the deployment is sealed: central seals +/// only once every node has acked Applied, and a FetchAndCache node cannot ack until it +/// has fetched. Gating on Sealed would deadlock every such deploy (seal-needs-ack → +/// ack-needs-fetch → fetch-needs-sealed), and it matches Direct mode, which reads the same +/// ArtifactBlob from SQL while the row is still AwaitingApplyAcks. (Caught on the +/// Phase 3 live gate.) Access is already gated by the shared-key interceptor, so there is no +/// reason to hide a non-sealed deployment. +/// +/// +/// NotFound is indistinguishable across two cases — unknown id and an empty blob. The +/// first is existence-hiding; the second is the #485 rule (a zero-length artifact is "no +/// answer," never "a configuration with no drivers"). Streaming empty bytes as if valid is +/// exactly what that defect class forbids. /// /// public sealed class DeploymentArtifactGrpcService : GeneratedServiceBase @@ -62,17 +71,16 @@ public sealed class DeploymentArtifactGrpcService : GeneratedServiceBase var row = await db.Deployments .AsNoTracking() .Where(d => d.DeploymentId == id) - .Select(d => new { d.Status, d.ArtifactBlob }) + .Select(d => new { d.ArtifactBlob }) .FirstOrDefaultAsync(context.CancellationToken); - // Existence-hiding + #485: unknown / not-sealed / empty collapse to one NotFound. - if (row is null || row.Status != DeploymentStatus.Sealed || row.ArtifactBlob.Length == 0) + // Existence-hiding + #485: unknown id / empty blob collapse to one NotFound. Status is NOT + // gated — a node fetches during apply, before the deployment is sealed (see the class remarks). + if (row is null || row.ArtifactBlob.Length == 0) { _log.LogDebug( - "Artifact fetch for {DeploymentId} -> NotFound (found={Found}, sealed={Sealed}, bytes={Bytes})", - request.DeploymentId, row is not null, - row is not null && row.Status == DeploymentStatus.Sealed, - row?.ArtifactBlob.Length ?? 0); + "Artifact fetch for {DeploymentId} -> NotFound (found={Found}, bytes={Bytes})", + request.DeploymentId, row is not null, row?.ArtifactBlob.Length ?? 0); throw new RpcException(new Status(StatusCode.NotFound, "unknown deployment")); } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Grpc/DeploymentArtifactServiceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Grpc/DeploymentArtifactServiceTests.cs index f31d9843..5b561462 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Grpc/DeploymentArtifactServiceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Grpc/DeploymentArtifactServiceTests.cs @@ -73,16 +73,21 @@ public class DeploymentArtifactServiceTests } [Fact] - public async Task A_non_sealed_deployment_throws_NotFound() + public async Task A_non_sealed_deployment_with_a_blob_is_served() { + // A node fetches DURING apply, before the deployment is sealed (seal-needs-ack → + // ack-needs-fetch). Gating on Sealed would deadlock every FetchAndCache deploy. So a + // non-sealed deployment that has a blob must stream — matching Direct mode reading the same + // AwaitingApplyAcks row from SQL. (Regression for the Phase 3 live-gate deadlock.) var dbName = $"artsvc-{Guid.NewGuid():N}"; - var id = Seed(dbName, DeploymentStatus.AwaitingApplyAcks, [1, 2, 3]); + var blob = new byte[] { 1, 2, 3 }; + var id = Seed(dbName, DeploymentStatus.AwaitingApplyAcks, blob); - var ex = await Should.ThrowAsync(() => Service(dbName).Fetch( - new FetchRequest { DeploymentId = id.ToString() }, - new CollectingStreamWriter(), new FakeServerCallContext())); + var writer = new CollectingStreamWriter(); + await Service(dbName).Fetch( + new FetchRequest { DeploymentId = id.ToString() }, writer, new FakeServerCallContext()); - ex.StatusCode.ShouldBe(StatusCode.NotFound); + writer.Reassembled().ShouldBe(blob); } [Fact]