From 5aba418074492daad4699caa941ade80214f33f9 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 7 Jun 2026 11:19:23 -0400 Subject: [PATCH] feat(deploy): activate full DraftValidator gate (reject on any validation error) --- .../AdminOperations/AdminOperationsActor.cs | 21 ++-- .../DraftValidatorTests.cs | 111 ++++++++++++++++++ .../AdminOperationsActorTests.cs | 60 ++++++++-- 3 files changed, 174 insertions(+), 18 deletions(-) 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 3334648f..cea2f062 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs @@ -79,18 +79,19 @@ 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. + // Full pre-seal gate: run the complete DraftValidator (one pass, all rules) and reject + // on ANY validation error. The config is now canonical — the company overlay loader emits + // canonical EquipmentIds and the seed is clean — so every rule (UNS segments, EquipmentId + // derivation, cross-cluster/namespace binding, driver-namespace compat, signal collisions, + // …) gates the deploy. A green build in this repo does not prove the config is valid; this + // is the last guard before a bad address space (or a non-derived EquipmentId) ships. var draft = await DraftSnapshotFactory.FromConfigDbAsync(db); - var collisions = DraftValidator.Validate(draft) - .Where(e => e.Code == "EquipmentSignalNameCollision") - .ToList(); - if (collisions.Count > 0) + var errors = DraftValidator.Validate(draft); + if (errors.Count > 0) { - var summary = string.Join("; ", collisions.Select(e => e.Message)); - _log.Warning("StartDeployment rejected (signal collision): {Summary}", summary); + var summary = string.Join("; ", errors.Select(e => $"[{e.Code}] {e.Message}")); + _log.Warning("StartDeployment rejected ({Count} validation error(s)): {Summary}", + errors.Count, summary); replyTo.Tell(new StartDeploymentResult( StartDeploymentOutcome.Rejected, DeploymentId: null, diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorTests.cs index 045e32f6..ad769f92 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorTests.cs @@ -186,6 +186,117 @@ public sealed class DraftValidatorTests errors.ShouldContain(e => e.Code == "UnsSegmentInvalid"); } + // ------------------------------------------------------------------------------------ + // Full-config probe — proves a realistic canonical deployed config passes ALL rules. + // This guards the deploy-path gate flip (reject-on-ANY-error): if a clean canonical + // config fired any rule, flipping the gate would block every deploy. + // ------------------------------------------------------------------------------------ + + /// Probe for the deploy-path gate activation: a full, realistic config modelling + /// the REAL deployed shape (a SystemPlatform namespace + GalaxyMxGateway driver + SystemPlatform + /// Tags with EquipmentId=null from the OtOpcUa seed, PLUS an Equipment namespace + Modbus driver + /// + UNS area/line + canonical Equipment rows + VirtualTags from the company overlay) must + /// produce ZERO validation errors so the reject-on-any-error gate is safe to activate. + [Fact] + public void Full_realistic_config_passes_all_rules() + { + // SystemPlatform side (OtOpcUa seed shape): Galaxy hierarchy, no Equipment, Tags carry EquipmentId=null. + var spNamespace = new Namespace + { + NamespaceId = "MAIN-OPCUA-systemplatform", + ClusterId = "MAIN", + Kind = NamespaceKind.SystemPlatform, + NamespaceUri = "urn:zb:main:systemplatform", + }; + var spDriver = new DriverInstance + { + DriverInstanceId = "main-galaxy", + ClusterId = "MAIN", + NamespaceId = spNamespace.NamespaceId, + Name = "Galaxy", + DriverType = "GalaxyMxGateway", + DriverConfig = "{}", + }; + + // Equipment side (company overlay): Modbus driver in an Equipment namespace + UNS + canonical equipment. + var eqNamespace = new Namespace + { + NamespaceId = "MAIN-OPCUA-equipment", + ClusterId = "MAIN", + Kind = NamespaceKind.Equipment, + NamespaceUri = "urn:zb:main:equipment", + }; + var eqDriver = new DriverInstance + { + DriverInstanceId = "main-modbus", + ClusterId = "MAIN", + NamespaceId = eqNamespace.NamespaceId, + Name = "Modbus", + DriverType = "Modbus", + DriverConfig = "{}", + }; + + var area = new UnsArea { UnsAreaId = "area-filling", ClusterId = "MAIN", Name = "filling" }; + var line = new UnsLine { UnsLineId = "line-1", UnsAreaId = area.UnsAreaId, Name = "line-1" }; + + // Canonical EquipmentIds — derived from the EquipmentUuid via the same rule the overlay loader uses. + var rinserUuid = Guid.NewGuid(); + var fillerUuid = Guid.NewGuid(); + var rinser = new Equipment + { + EquipmentUuid = rinserUuid, + EquipmentId = DraftValidator.DeriveEquipmentId(rinserUuid), + Name = "rinser-01", + DriverInstanceId = eqDriver.DriverInstanceId, + UnsLineId = line.UnsLineId, + MachineCode = "machine_001", + ZTag = null, + SAPID = null, + }; + var filler = new Equipment + { + EquipmentUuid = fillerUuid, + EquipmentId = DraftValidator.DeriveEquipmentId(fillerUuid), + Name = "filler-02", + DriverInstanceId = eqDriver.DriverInstanceId, + UnsLineId = line.UnsLineId, + MachineCode = "machine_002", + ZTag = null, + SAPID = null, + }; + + var draft = new DraftSnapshot + { + GenerationId = 0, + ClusterId = string.Empty, // global snapshot — matches DraftSnapshotFactory.FromConfigDbAsync + // Enterprise/Site left null — matches the deploy path's conservative fallback + Namespaces = [spNamespace, eqNamespace], + DriverInstances = [spDriver, eqDriver], + UnsAreas = [area], + UnsLines = [line], + Equipment = [rinser, filler], + Tags = + [ + // SystemPlatform tags from the seed: EquipmentId null, Galaxy folder hierarchy. + BuildTag(equipmentId: null, name: "PV", folderPath: "Area.Tank01"), + BuildTag(equipmentId: null, name: "SP", folderPath: "Area.Tank01"), + ], + VirtualTags = + [ + // One per equipment, names distinct within each owning equipment. + BuildVirtualTag(equipmentId: rinser.EquipmentId, name: "oee"), + BuildVirtualTag(equipmentId: filler.EquipmentId, name: "throughput"), + ], + }; + + var errors = DraftValidator.Validate(draft); + + errors.ShouldBeEmpty( + "a realistic canonical deployed config must pass every DraftValidator rule so the " + + "reject-on-any-error deploy gate is safe; firing rules: " + + string.Join("; ", errors.Select(e => $"[{e.Code}] {e.Message}"))); + } + // ------------------------------------------------------------------------------------ // ValidateNoEquipmentSignalNameCollision — Tag/VirtualTag NodeId collision // ------------------------------------------------------------------------------------ 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 2156ae30..8898c43c 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorTests.cs @@ -43,19 +43,24 @@ 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. + /// Verifies the full DraftValidator gate (reject on ANY error): a Tag↔VirtualTag + /// NodeId collision in the live config rejects the deploy (422-mapped + /// ) before any coordinator dispatch — and inserts + /// no Deployment row. The colliding equipment uses a canonical EquipmentId so the rejection is + /// attributable to the collision rule, not to EquipmentIdNotDerived. [Fact] public void StartDeployment_rejects_on_Tag_VirtualTag_NodeId_collision() { + var uuid = Guid.NewGuid(); + var equipmentId = Configuration.Validation.DraftValidator.DeriveEquipmentId(uuid); + var dbFactory = NewInMemoryDbFactory(); using (var db = dbFactory.CreateDbContext()) { db.Equipment.Add(new Configuration.Entities.Equipment { - EquipmentUuid = Guid.NewGuid(), - EquipmentId = "eq-1", + EquipmentUuid = uuid, + EquipmentId = equipmentId, Name = "eq", DriverInstanceId = "d", UnsLineId = "line-a", @@ -65,7 +70,7 @@ public sealed class AdminOperationsActorTests : ControlPlaneActorTestBase { TagId = "tag-speed", DriverInstanceId = "d", - EquipmentId = "eq-1", + EquipmentId = equipmentId, Name = "speed", DataType = "Float", AccessLevel = TagAccessLevel.Read, @@ -74,7 +79,7 @@ public sealed class AdminOperationsActorTests : ControlPlaneActorTestBase db.VirtualTags.Add(new Configuration.Entities.VirtualTag { VirtualTagId = "vtag-speed", - EquipmentId = "eq-1", + EquipmentId = equipmentId, Name = "speed", DataType = "Float", ScriptId = "s-1", @@ -91,7 +96,46 @@ public sealed class AdminOperationsActorTests : ControlPlaneActorTestBase var reply = ExpectMsg(TimeSpan.FromSeconds(3)); reply.Outcome.ShouldBe(StartDeploymentOutcome.Rejected); reply.Message.ShouldNotBeNull(); - reply.Message.ShouldContain("collide"); // the rule's message text + reply.Message.ShouldContain("EquipmentSignalNameCollision"); // the rule's error code + reply.Message.ShouldContain("collide"); // the rule's message text + + using var verify = dbFactory.CreateDbContext(); + verify.Deployments.Count().ShouldBe(0); + } + + /// Verifies the full gate rejects a config whose Equipment carries a NON-canonical + /// EquipmentId (not == DraftValidator.DeriveEquipmentId(uuid)): the deploy is + /// with EquipmentIdNotDerived in the message, + /// no coordinator dispatch, and no Deployment row. This is the rule the surgical gate used to + /// let through and the reason the full activation was probed first. + [Fact] + public void StartDeployment_rejects_on_non_canonical_EquipmentId() + { + var dbFactory = NewInMemoryDbFactory(); + using (var db = dbFactory.CreateDbContext()) + { + db.Equipment.Add(new Configuration.Entities.Equipment + { + EquipmentUuid = Guid.NewGuid(), + EquipmentId = "EQ-operator-typed", // NOT derived from the UUID + Name = "rinser-01", + DriverInstanceId = "d", + UnsLineId = "line-a", + MachineCode = "m", + }); + 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("EquipmentIdNotDerived"); using var verify = dbFactory.CreateDbContext(); verify.Deployments.Count().ShouldBe(0);