From 1023209d52da16df1760eb11df9bbc29214be7a7 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 7 Jun 2026 10:42:13 -0400 Subject: [PATCH] feat(deploy): reject Tag/VirtualTag NodeId collisions at deploy (surgical DraftValidator gate) --- .../Validation/DraftSnapshotFactory.cs | 48 +++++++++ .../AdminOperations/AdminOperationsActor.cs | 22 +++++ .../DraftSnapshotFactoryTests.cs | 99 +++++++++++++++++++ .../AdminOperationsActorTests.cs | 54 ++++++++++ 4 files changed, 223 insertions(+) create mode 100644 src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftSnapshotFactory.cs create mode 100644 tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftSnapshotFactoryTests.cs diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftSnapshotFactory.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftSnapshotFactory.cs new file mode 100644 index 00000000..50cf4b68 --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftSnapshotFactory.cs @@ -0,0 +1,48 @@ +using Microsoft.EntityFrameworkCore; + +namespace ZB.MOM.WW.OtOpcUa.Configuration.Validation; + +/// +/// Materialises a from the live config DB so +/// can run against the current edit state at deploy time. +/// +/// +/// +/// This is a whole-DB ("global") snapshot — every cluster's rows in one pass — which is +/// what the deploy path needs: the admin-operations actor snapshots and flattens the +/// entire config, not one cluster. The validator's rules compare entity-level +/// ClusterId fields against each other (e.g. namespace binding), so the snapshot's +/// own is not read by any rule and is left empty. +/// +/// +/// is a placeholder (the generation model was +/// dropped); no rule reads it. is empty because +/// there is no prior-generation table to diff against. +/// / are left null so the path-length rule uses its +/// conservative upper bound. +/// +/// +public static class DraftSnapshotFactory +{ + /// Builds a from the current config DB rows. + /// The config DB context to read from. + /// Cancellation token. + /// A snapshot populated from the live DB, ready for . + public static async Task FromConfigDbAsync(OtOpcUaConfigDbContext db, CancellationToken ct = default) + => new DraftSnapshot + { + GenerationId = 0, // generation model dropped; placeholder (no rule reads it) + ClusterId = string.Empty, // global snapshot; rules compare entity ClusterId fields, not this + Namespaces = await db.Namespaces.AsNoTracking().ToListAsync(ct), + DriverInstances = await db.DriverInstances.AsNoTracking().ToListAsync(ct), + Devices = await db.Devices.AsNoTracking().ToListAsync(ct), + UnsAreas = await db.UnsAreas.AsNoTracking().ToListAsync(ct), + UnsLines = await db.UnsLines.AsNoTracking().ToListAsync(ct), + Equipment = await db.Equipment.AsNoTracking().ToListAsync(ct), + Tags = await db.Tags.AsNoTracking().ToListAsync(ct), + VirtualTags = await db.VirtualTags.AsNoTracking().ToListAsync(ct), + PollGroups = await db.PollGroups.AsNoTracking().ToListAsync(ct), + PriorEquipment = [], + ActiveReservations = await db.ExternalIdReservations.AsNoTracking().ToListAsync(ct), + }; +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs index d081a3a5..3334648f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs @@ -8,6 +8,7 @@ using ZB.MOM.WW.OtOpcUa.Commons.Types; using ZB.MOM.WW.OtOpcUa.Configuration; using ZB.MOM.WW.OtOpcUa.Configuration.Entities; using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Configuration.Validation; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.ControlPlane.AdminOperations; @@ -78,6 +79,27 @@ public sealed class AdminOperationsActor : ReceiveActor return; } + // Surgical pre-seal gate: reject only on a Tag↔VirtualTag NodeId collision. The other + // DraftValidator rules still run (one pass) but must NOT block here — they are dormant + // and the current non-canonical company overlay would otherwise fail them. Filter to the + // single collision code so a real OPC UA address-space clash can never be deployed. + var draft = await DraftSnapshotFactory.FromConfigDbAsync(db); + var collisions = DraftValidator.Validate(draft) + .Where(e => e.Code == "EquipmentSignalNameCollision") + .ToList(); + if (collisions.Count > 0) + { + var summary = string.Join("; ", collisions.Select(e => e.Message)); + _log.Warning("StartDeployment rejected (signal collision): {Summary}", summary); + replyTo.Tell(new StartDeploymentResult( + StartDeploymentOutcome.Rejected, + DeploymentId: null, + RevisionHash: null, + Message: summary, + msg.CorrelationId)); + return; + } + var artifact = await ConfigComposer.SnapshotAndFlattenAsync(db); var deploymentId = DeploymentId.NewId(); var revHash = RevisionHash.Parse(artifact.RevisionHash); diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftSnapshotFactoryTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftSnapshotFactoryTests.cs new file mode 100644 index 00000000..c3e44ba6 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftSnapshotFactoryTests.cs @@ -0,0 +1,99 @@ +using Microsoft.EntityFrameworkCore; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Configuration.Validation; + +namespace ZB.MOM.WW.OtOpcUa.Configuration.Tests; + +/// +/// Verifies materialises a +/// from the live config DB whose Tag/VirtualTag rows feed the +/// equipment-signal collision rule — the one rule wired into the deploy gate (Task 3). +/// +[Trait("Category", "Unit")] +public sealed class DraftSnapshotFactoryTests : IDisposable +{ + private readonly OtOpcUaConfigDbContext _db; + + /// Initializes a new instance with an isolated in-memory config DB. + public DraftSnapshotFactoryTests() + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase($"draft-snapshot-{Guid.NewGuid():N}") + .Options; + _db = new OtOpcUaConfigDbContext(options); + } + + /// Disposes the database context. + public void Dispose() => _db.Dispose(); + + /// Seeds one Equipment plus a Tag and a VirtualTag sharing (EquipmentId, Name); the + /// snapshot must carry both signal collections AND the validator must flag the collision. + [Fact] + public async Task FromConfigDb_populates_Tags_and_VirtualTags_and_surfaces_collision() + { + SeedEquipment("eq-1"); + _db.Tags.Add(BuildTag(equipmentId: "eq-1", name: "speed")); + _db.VirtualTags.Add(BuildVirtualTag(equipmentId: "eq-1", name: "speed")); + await _db.SaveChangesAsync(); + + var snapshot = await DraftSnapshotFactory.FromConfigDbAsync(_db); + + snapshot.Tags.Count.ShouldBe(1); + snapshot.VirtualTags.Count.ShouldBe(1); + DraftValidator.Validate(snapshot).ShouldContain(e => e.Code == "EquipmentSignalNameCollision"); + } + + /// A Tag and a VirtualTag with distinct names under the same equipment do not collide, + /// so the snapshot validates clean of the collision code. + [Fact] + public async Task FromConfigDb_no_collision_when_names_differ() + { + SeedEquipment("eq-1"); + _db.Tags.Add(BuildTag(equipmentId: "eq-1", name: "speed")); + _db.VirtualTags.Add(BuildVirtualTag(equipmentId: "eq-1", name: "temperature")); + await _db.SaveChangesAsync(); + + var snapshot = await DraftSnapshotFactory.FromConfigDbAsync(_db); + + snapshot.Tags.Count.ShouldBe(1); + snapshot.VirtualTags.Count.ShouldBe(1); + DraftValidator.Validate(snapshot).ShouldNotContain(e => e.Code == "EquipmentSignalNameCollision"); + } + + private void SeedEquipment(string equipmentId) + { + var uuid = Guid.NewGuid(); + _db.Equipment.Add(new Equipment + { + EquipmentUuid = uuid, + EquipmentId = equipmentId, + Name = "eq", + DriverInstanceId = "d", + UnsLineId = "line-a", + MachineCode = "m", + }); + } + + private static Tag BuildTag(string equipmentId, string name) => new() + { + TagId = $"tag-{name}", + DriverInstanceId = "d", + EquipmentId = equipmentId, + Name = name, + DataType = "Float", + AccessLevel = TagAccessLevel.Read, + TagConfig = "{}", + }; + + private static VirtualTag BuildVirtualTag(string equipmentId, string name) => new() + { + VirtualTagId = $"vtag-{name}", + EquipmentId = equipmentId, + Name = name, + DataType = "Float", + ScriptId = "s-1", + }; +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorTests.cs index 559110cb..2156ae30 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorTests.cs @@ -43,6 +43,60 @@ public sealed class AdminOperationsActorTests : ControlPlaneActorTestBase db.ConfigEdits.Single().EntityType.ShouldBe("Deployment"); } + /// Verifies the surgical DraftValidator gate: a Tag↔VirtualTag NodeId collision in + /// the live config rejects the deploy (422-mapped ) + /// before any coordinator dispatch — and inserts no Deployment row. + [Fact] + public void StartDeployment_rejects_on_Tag_VirtualTag_NodeId_collision() + { + var dbFactory = NewInMemoryDbFactory(); + using (var db = dbFactory.CreateDbContext()) + { + db.Equipment.Add(new Configuration.Entities.Equipment + { + EquipmentUuid = Guid.NewGuid(), + EquipmentId = "eq-1", + Name = "eq", + DriverInstanceId = "d", + UnsLineId = "line-a", + MachineCode = "m", + }); + db.Tags.Add(new Configuration.Entities.Tag + { + TagId = "tag-speed", + DriverInstanceId = "d", + EquipmentId = "eq-1", + Name = "speed", + DataType = "Float", + AccessLevel = TagAccessLevel.Read, + TagConfig = "{}", + }); + db.VirtualTags.Add(new Configuration.Entities.VirtualTag + { + VirtualTagId = "vtag-speed", + EquipmentId = "eq-1", + Name = "speed", + DataType = "Float", + ScriptId = "s-1", + }); + db.SaveChanges(); + } + + var coordinator = CreateTestProbe("coord"); + var actor = Sys.ActorOf(AdminOperationsActor.Props(dbFactory, coordinator.Ref, Enumerable.Empty())); + + actor.Tell(new StartDeployment("joe", CorrelationId.NewId())); + + coordinator.ExpectNoMsg(TimeSpan.FromMilliseconds(500)); + var reply = ExpectMsg(TimeSpan.FromSeconds(3)); + reply.Outcome.ShouldBe(StartDeploymentOutcome.Rejected); + reply.Message.ShouldNotBeNull(); + reply.Message.ShouldContain("collide"); // the rule's message text + + using var verify = dbFactory.CreateDbContext(); + verify.Deployments.Count().ShouldBe(0); + } + /// Verifies that starting a deployment is refused when another is in flight. [Fact] public void StartDeployment_refuses_when_another_is_in_flight()