Files
lmxopcua/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorNodeAclScopeTests.cs
T
Joseph Doherty 06c56d7719 fix(cluster,acl): OTOPCUA_ROLES fallback + one string-identity rule (#532, #533)
Two independent defects surfaced in review of #520 stage 3.

#532 — Cluster:Roles had no OTOPCUA_ROLES fallback, despite two doc sites
promising one. A node configured only through the env var — which drives ALL of
Program.cs's conditional wiring, so it reads as the authoritative knob — bound an
empty Roles array and joined the cluster carrying no Akka roles at all. It ran,
served, and logged nothing wrong, while central reported "no driver-role nodes
are Up" and every deployment to it timed out. Not hypothetical: the field fix on
wonder-app-vd03 was to hand-add Cluster__Roles__0/1 to a Windows service's
registry environment. Anyone reading the code to diagnose that box would have
concluded from the XML doc that the fallback was already applied.

AddOtOpcUaCluster now post-configures AkkaClusterOptions through the shared
RoleParser.ResolveEffectiveRoles, so BuildClusterOptions (Akka's own member
roles), ClusterRoleInfo, the ControlPlane singleton scope and
ConfigDbApiKeyStore's ClusterId all see one answer. Two sibling validators
(ConfigSource, Telemetry) read Cluster:Roles directly and were exempting the
exact node they exist for — an env-only driver-only node has no ConfigDb and
could sit on ConfigSource:Mode=Direct unchallenged. They resolve the same way now.

Also collapsed the four open-coded `Roles.FirstOrDefault(IsClusterRole)` +
ClusterIdFromRole copies into RoleParser.ClusterIdFromRoles. They had drifted:
three classified the cluster role ordinally, so a verbatim-bound `Cluster-SITE-A`
yielded a null ClusterId — and a null ClusterId applies NO cluster filter, which
made #528's cross-cluster over-grant guard fail OPEN. IsClusterRole is now
case-insensitive; the ClusterId suffix keeps its authored spelling, because
normalizing it would silently rename the Akka role of a running deployment.

#533 — three string-identity rules in one authorization path. AclScopeRef used
default (ordinal) equality while the trie builder filtered ClusterId and hashed
principals OrdinalIgnoreCase; the deploy gate's cross-cluster check was ordinal
too, which was the worse half — it could reject a whole deploy over a ClusterId
case difference the runtime would have accepted. It only worked because resolver
and builder were handed the same NodeAcl instances.

The rule is now stated once, on AclScopeRef, and applied throughout:
identifiers (ScopeId, ClusterId, Principal) compare OrdinalIgnoreCase; path
segments compare Ordinal. AclScopePathResolver re-keys its own maps rather than
documenting a comparer contract for callers, so the two independent producers —
EF at the gate, artifact JSON on the node — cannot diverge from the outside.
PermissionTrieNode.Children stays Ordinal: those are browse names, and Speed /
speed are two nodes that can coexist under one device.

Split PermissionTrieBuildDiagnosticReason.RetiredScopeKind out of
UnresolvedScope. A legacy FolderSegment row and a typo'd id had the same message
and different remedies.

Falsified: five mutations, each reddening only its own guard — remove the
PostConfigure (3 tests), revert IsClusterRole to ordinal (3), revert AclScopeRef
equality (1), revert the resolver's map comparer (6), revert the gate's
comparisons (2), remove the FolderSegment short-circuit (1).

Test isolation: OTOPCUA_ROLES is process-global and xUnit runs classes in
parallel, so the three validator suites that now resolve effective roles share a
non-parallel collection. Without it, cases passing no roles would intermittently
observe another class's env var.

Full solution build clean; full suite green apart from two pre-existing failures
confirmed identical on 3a6b2a1d (PrimaryGateFailoverTests, and
DriverProbeRegistrationTests expecting 10 probe types where 12 now register)
plus the AbLegacy integration suite, which needs a docker fixture.

Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
2026-08-01 01:51:22 -04:00

460 lines
20 KiB
C#

using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
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;
/// <summary>
/// The #520 stage-3 deploy gate for <see cref="NodeAcl"/> scopes. An unresolvable scope is dropped by
/// the permission-trie builder and the deploy still seals green, so the row reads as a live grant in the
/// AdminUI while authorizing nothing — these pin that it fails the deploy instead.
/// </summary>
[Trait("Category", "Unit")]
public sealed class DraftValidatorNodeAclScopeTests
{
/// <summary>The cluster every ACL row under test is authored in.</summary>
private const string Own = "c";
/// <summary>A second, fully-populated cluster in the same snapshot — the cross-cluster counterparty.</summary>
private const string Other = "other";
private const string OwnEquipId = "EQ-000000000000";
private const string OtherEquipId = "EQ-0000000b0000";
private static NodeAcl Acl(NodeAclScopeKind kind, string? scopeId, string id = "acl-1") => new()
{
NodeAclId = id,
ClusterId = Own,
Principal = "svc-line3",
ScopeKind = kind,
ScopeId = scopeId,
PermissionFlags = NodePermissions.Read,
};
/// <summary>
/// A draft carrying one of each scoped entity in <see cref="Own"/> and a mirrored set in
/// <see cref="Other"/>. Two clusters, not one, so that a cross-cluster id is genuinely present in the
/// snapshot: if the rule's lookups were cluster-filtered such an id would present as merely
/// unresolved and the containment check could never fire.
/// </summary>
private static DraftSnapshot Draft(params NodeAcl[] acls) => new()
{
GenerationId = 1,
ClusterId = Own,
NodeAcls = acls,
RawFolders =
[
new RawFolder { RawFolderId = "rf-1", ClusterId = Own, Name = "plant" },
new RawFolder { RawFolderId = "rf-2", ClusterId = Other, Name = "plant2" },
],
DriverInstances =
[
new DriverInstance
{
DriverInstanceId = "di-1", ClusterId = Own, Name = "modbus",
DriverType = "Modbus", DriverConfig = "{}", RawFolderId = "rf-1",
},
new DriverInstance
{
DriverInstanceId = "di-2", ClusterId = Other, Name = "modbus2",
DriverType = "Modbus", DriverConfig = "{}", RawFolderId = "rf-2",
},
],
Devices =
[
new Device { DeviceId = "dev-1", DriverInstanceId = "di-1", Name = "dev1", DeviceConfig = "{}" },
new Device { DeviceId = "dev-2", DriverInstanceId = "di-2", Name = "dev2", DeviceConfig = "{}" },
],
TagGroups =
[
new TagGroup { TagGroupId = "tg-1", DeviceId = "dev-1", Name = "group1" },
new TagGroup { TagGroupId = "tg-2", DeviceId = "dev-2", Name = "group2" },
],
UnsAreas =
[
new UnsArea { UnsAreaId = "area-1", ClusterId = Own, Name = "north" },
new UnsArea { UnsAreaId = "area-2", ClusterId = Other, Name = "south" },
],
UnsLines =
[
new UnsLine { UnsLineId = "line-1", UnsAreaId = "area-1", Name = "line3" },
new UnsLine { UnsLineId = "line-2", UnsAreaId = "area-2", Name = "line4" },
],
Equipment =
[
new Equipment
{
EquipmentUuid = Guid.Parse("00000000-0000-0000-0000-0000000000aa"),
EquipmentId = OwnEquipId,
Name = "filler", UnsLineId = "line-1", MachineCode = "m",
},
new Equipment
{
EquipmentUuid = Guid.Parse("0000000b-0000-0000-0000-000000000000"),
EquipmentId = OtherEquipId,
Name = "packer", UnsLineId = "line-2", MachineCode = "m",
},
],
Tags =
[
new Tag
{
TagId = "tag-1", DeviceId = "dev-1", Name = "speed",
DataType = "Float", AccessLevel = TagAccessLevel.Read, TagConfig = "{}",
},
new Tag
{
TagId = "tag-2", DeviceId = "dev-2", Name = "speed2",
DataType = "Float", AccessLevel = TagAccessLevel.Read, TagConfig = "{}",
},
],
};
/// <summary>
/// The fixture itself must trip no rule, or a test asserting on the whole error list is really
/// asserting on unrelated noise. This also pins both hardcoded EquipmentIds against the canonical
/// derivation, which a later edit to the fixture's UUIDs would otherwise silently break.
/// </summary>
[Fact]
public void The_fixture_with_no_acl_rows_is_clean()
{
DraftValidator.Validate(Draft()).ShouldBeEmpty();
DraftValidator.DeriveEquipmentId(Guid.Parse("00000000-0000-0000-0000-0000000000aa")).ShouldBe(OwnEquipId);
DraftValidator.DeriveEquipmentId(Guid.Parse("0000000b-0000-0000-0000-000000000000")).ShouldBe(OtherEquipId);
}
/// <summary>
/// The row shape the trie builder drops without a word: a narrower kind with nothing to narrow to.
/// </summary>
[Theory]
[InlineData(null)]
[InlineData("")]
[InlineData(" ")]
public void A_non_Cluster_kind_without_a_ScopeId_is_a_deploy_error(string? scopeId)
{
var errors = DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.Equipment, scopeId)));
errors.ShouldContain(e => e.Code == "NodeAclScopeIdMissing" && e.Context == "acl-1");
}
/// <summary>
/// A Cluster grant ignores ScopeId, so a populated one means the author picked the wrong kind and the
/// grant is <em>broader</em> than intended — the direction that must never pass silently.
/// </summary>
[Fact]
public void A_Cluster_kind_carrying_a_ScopeId_is_a_deploy_error()
{
var errors = DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.Cluster, "EQ-000000000000")));
errors.ShouldContain(e => e.Code == "NodeAclScopeIdUnexpected" && e.Context == "acl-1");
}
[Fact]
public void A_Cluster_kind_with_no_ScopeId_passes()
{
DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.Cluster, null)))
.ShouldNotContain(e => e.Code.StartsWith("NodeAcl", StringComparison.Ordinal));
}
/// <summary>
/// One case per scoped kind: a well-formed id that names no such entity must fail. Kept exhaustive
/// because the kind→collection mapping is hand-written, and a kind wired to the wrong collection
/// would still "work" for every id that happens to exist in both.
/// </summary>
[Theory]
[InlineData(NodeAclScopeKind.UnsArea)]
[InlineData(NodeAclScopeKind.UnsLine)]
[InlineData(NodeAclScopeKind.Equipment)]
[InlineData(NodeAclScopeKind.Tag)]
[InlineData(NodeAclScopeKind.RawFolder)]
[InlineData(NodeAclScopeKind.RawDriver)]
[InlineData(NodeAclScopeKind.RawDevice)]
[InlineData(NodeAclScopeKind.RawTagGroup)]
public void A_ScopeId_naming_no_such_entity_is_a_deploy_error(NodeAclScopeKind kind)
{
var errors = DraftValidator.Validate(Draft(Acl(kind, "does-not-exist")));
errors.ShouldContain(e => e.Code == "NodeAclScopeUnresolved" && e.Context == "acl-1");
}
/// <summary>
/// The counterpart to the theory above: each kind pointed at the id from its OWN collection must
/// pass. Without this the rule could reject everything and still look correct.
/// </summary>
[Theory]
[InlineData(NodeAclScopeKind.UnsArea, "area-1")]
[InlineData(NodeAclScopeKind.UnsLine, "line-1")]
[InlineData(NodeAclScopeKind.Equipment, OwnEquipId)]
[InlineData(NodeAclScopeKind.Tag, "tag-1")]
[InlineData(NodeAclScopeKind.RawFolder, "rf-1")]
[InlineData(NodeAclScopeKind.RawDriver, "di-1")]
[InlineData(NodeAclScopeKind.RawDevice, "dev-1")]
[InlineData(NodeAclScopeKind.RawTagGroup, "tg-1")]
public void A_ScopeId_naming_a_real_entity_of_its_own_kind_passes(NodeAclScopeKind kind, string scopeId)
{
DraftValidator.Validate(Draft(Acl(kind, scopeId)))
.ShouldNotContain(e => e.Code.StartsWith("NodeAcl", StringComparison.Ordinal));
}
/// <summary>
/// The containment check, one case per kind. Each id is real and resolves to a real trie path — it
/// just belongs to another cluster, so the grant would confer authority across a cluster boundary.
/// Three of these kinds carry ClusterId directly; the other five are only reachable by walking a
/// parent chain, which is why every kind is covered rather than a representative one.
/// </summary>
[Theory]
[InlineData(NodeAclScopeKind.UnsArea, "area-2")]
[InlineData(NodeAclScopeKind.UnsLine, "line-2")]
[InlineData(NodeAclScopeKind.Equipment, OtherEquipId)]
[InlineData(NodeAclScopeKind.Tag, "tag-2")]
[InlineData(NodeAclScopeKind.RawFolder, "rf-2")]
[InlineData(NodeAclScopeKind.RawDriver, "di-2")]
[InlineData(NodeAclScopeKind.RawDevice, "dev-2")]
[InlineData(NodeAclScopeKind.RawTagGroup, "tg-2")]
public void A_ScopeId_naming_an_entity_in_another_cluster_is_a_deploy_error(
NodeAclScopeKind kind, string scopeId)
{
var errors = DraftValidator.Validate(Draft(Acl(kind, scopeId)));
errors.ShouldContain(e => e.Code == "NodeAclScopeCrossCluster" && e.Context == "acl-1");
}
/// <summary>
/// The trap this check exists to avoid: if the lookups were cluster-filtered, a cross-cluster id
/// would present as unresolved, the operator would be told to fix a typo in an id that is not a typo,
/// and the containment rule would never fire at all. The two codes must be mutually exclusive.
/// </summary>
[Theory]
[InlineData(NodeAclScopeKind.UnsArea, "area-2")]
[InlineData(NodeAclScopeKind.UnsLine, "line-2")]
[InlineData(NodeAclScopeKind.Equipment, OtherEquipId)]
[InlineData(NodeAclScopeKind.Tag, "tag-2")]
[InlineData(NodeAclScopeKind.RawFolder, "rf-2")]
[InlineData(NodeAclScopeKind.RawDriver, "di-2")]
[InlineData(NodeAclScopeKind.RawDevice, "dev-2")]
[InlineData(NodeAclScopeKind.RawTagGroup, "tg-2")]
public void A_cross_cluster_scope_is_never_also_reported_as_unresolved(
NodeAclScopeKind kind, string scopeId)
{
var errors = DraftValidator.Validate(Draft(Acl(kind, scopeId)));
errors.ShouldNotContain(e => e.Code == "NodeAclScopeUnresolved");
errors.Where(e => e.Code.StartsWith("NodeAcl", StringComparison.Ordinal)).ShouldHaveSingleItem();
}
/// <summary>
/// Gitea #533 — the gate's own identifier comparisons follow the same rule as the resolver's:
/// identifiers compare <c>OrdinalIgnoreCase</c>. Both were ordinal, which put the gate and the
/// runtime into disagreement in the direction that costs the most: the deploy gate would reject a
/// whole configuration as cross-cluster over a <c>ClusterId</c> case difference that
/// <c>PermissionTrieBuilder</c> — which has always matched case-insensitively — would have accepted.
/// </summary>
[Fact]
public void A_case_drifted_ClusterId_is_not_reported_as_cross_cluster()
{
var acl = Acl(NodeAclScopeKind.RawDevice, "dev-1");
acl.ClusterId = Own.ToUpperInvariant();
var errors = DraftValidator.Validate(Draft(acl));
errors.ShouldNotContain(e => e.Code == "NodeAclScopeCrossCluster");
errors.ShouldNotContain(e => e.Code == "NodeAclScopeUnresolved");
}
/// <summary>
/// The same rule applied to the <c>ScopeId</c> lookups, so a row this gate passes is a row
/// <c>AclScopePathResolver</c> resolves — the property the gate's whole value rests on. A gate
/// stricter than the resolver blocks deploys that would have worked; a gate looser than it lets
/// through the silently-dropped grants it exists to catch.
/// </summary>
[Fact]
public void A_case_drifted_ScopeId_resolves_at_the_gate_too()
{
DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.RawDevice, "DEV-1")))
.ShouldNotContain(e => e.Code.StartsWith("NodeAcl", StringComparison.Ordinal));
}
/// <summary>Negative control: relaxing case must not stop a genuinely foreign cluster from being caught.</summary>
[Fact]
public void A_genuinely_different_ClusterId_is_still_cross_cluster()
{
DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.RawDevice, "dev-2")))
.ShouldContain(e => e.Code == "NodeAclScopeCrossCluster");
}
/// <summary>The error must name both clusters — which one the grant is in and which one owns the
/// entity — because "re-author under the other cluster" and "repoint at the local entity" are different
/// fixes and the operator picks between them from this message alone.</summary>
[Fact]
public void The_cross_cluster_message_names_both_clusters()
{
var error = DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.RawDevice, "dev-2")))
.First(e => e.Code == "NodeAclScopeCrossCluster");
error.Message.ShouldContain($"'{Own}'");
error.Message.ShouldContain($"'{Other}'");
}
/// <summary>
/// An entity whose parent link dangles belongs to no cluster, so containment is unknowable — and it
/// resolves to no scope path either, which is the unresolved case. It must report that, once, rather
/// than a cross-cluster error built on a null owner.
/// </summary>
[Fact]
public void A_device_whose_driver_is_missing_reports_unresolved_only()
{
var draft = new DraftSnapshot
{
GenerationId = 1,
ClusterId = Own,
NodeAcls = [Acl(NodeAclScopeKind.RawDevice, "dev-orphan")],
Devices =
[
new Device
{
DeviceId = "dev-orphan", DriverInstanceId = "di-gone",
Name = "orphan", DeviceConfig = "{}",
},
],
};
var errors = DraftValidator.Validate(draft);
errors.ShouldContain(e => e.Code == "NodeAclScopeUnresolved" && e.Context == "acl-1");
errors.ShouldNotContain(e => e.Code == "NodeAclScopeCrossCluster");
}
/// <summary>The UNS counterpart, and a two-hop break rather than one: Equipment reaches its cluster
/// through UnsLine and only then UnsArea, so a missing line strands it just as a missing driver strands
/// a device.</summary>
[Fact]
public void An_equipment_whose_line_is_missing_reports_unresolved_only()
{
var uuid = Guid.Parse("00000000-0000-0000-0000-0000000000aa");
var draft = new DraftSnapshot
{
GenerationId = 1,
ClusterId = Own,
NodeAcls = [Acl(NodeAclScopeKind.Equipment, OwnEquipId)],
Equipment =
[
new Equipment
{
EquipmentUuid = uuid, EquipmentId = DraftValidator.DeriveEquipmentId(uuid),
Name = "filler", UnsLineId = "line-gone", MachineCode = "m",
},
],
};
var errors = DraftValidator.Validate(draft);
errors.ShouldContain(e => e.Code == "NodeAclScopeUnresolved" && e.Context == "acl-1");
errors.ShouldNotContain(e => e.Code == "NodeAclScopeCrossCluster");
}
/// <summary>
/// ScopeId is a bare string with no foreign key, so a real id of the <em>wrong</em> kind is exactly
/// the mistake that would otherwise hand a grant another entity's subtree.
/// </summary>
[Fact]
public void A_real_id_of_the_wrong_kind_is_still_unresolved()
{
var errors = DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.UnsArea, "dev-1")));
errors.ShouldContain(e => e.Code == "NodeAclScopeUnresolved" && e.Context == "acl-1");
}
/// <summary>Both realm tokens and both full namespace URIs are accepted, in any case — matching
/// <c>NodeAclRealms.TryCanonicalize</c>, which is what actually resolves the row at build time.</summary>
[Theory]
[InlineData("raw")]
[InlineData("uns")]
[InlineData("RAW")]
[InlineData("Uns")]
[InlineData(V3NodeIds.RawNamespaceUri)]
[InlineData(V3NodeIds.UnsNamespaceUri)]
[InlineData(" raw ")]
public void A_Namespace_scope_naming_a_realm_passes(string scopeId)
{
DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.Namespace, scopeId)))
.ShouldNotContain(e => e.Code.StartsWith("NodeAcl", StringComparison.Ordinal));
}
/// <summary>The server has exactly two namespaces; anything else names none of them.</summary>
[Theory]
[InlineData("ns=2")]
[InlineData("https://zb.com/otopcua/ns")] // the retired v2 single namespace
[InlineData("equipment")]
public void A_Namespace_scope_naming_no_realm_is_a_deploy_error(string scopeId)
{
var errors = DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.Namespace, scopeId)));
errors.ShouldContain(e => e.Code == "NodeAclNamespaceInvalid" && e.Context == "acl-1");
}
/// <summary>
/// FolderSegment survives in the enum only so later ordinals do not shift; it has no v3 entity and
/// can never resolve, so the error has to name the replacement rather than just say "unresolved".
/// </summary>
[Fact]
public void The_retired_FolderSegment_kind_is_a_deploy_error_naming_RawFolder()
{
var error = DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.FolderSegment, "some-folder")))
.Where(e => e.Code.StartsWith("NodeAcl", StringComparison.Ordinal))
.ShouldHaveSingleItem();
error.Code.ShouldBe("NodeAclScopeKindRetired");
error.Context.ShouldBe("acl-1");
error.Message.ShouldContain("RawFolder");
}
/// <summary>
/// A FolderSegment row with no ScopeId is both retired and missing an id. The retirement is the
/// actionable fact — the kind can never resolve whatever its ScopeId — so telling the operator to
/// supply one would send them to fix the wrong thing.
/// </summary>
[Theory]
[InlineData(null)]
[InlineData("")]
[InlineData(" ")]
public void FolderSegment_retirement_wins_over_the_missing_ScopeId_error(string? scopeId)
{
var error = DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.FolderSegment, scopeId)))
.Where(e => e.Code.StartsWith("NodeAcl", StringComparison.Ordinal))
.ShouldHaveSingleItem();
error.Code.ShouldBe("NodeAclScopeKindRetired");
}
/// <summary>
/// An ordinal outside the enum comes from a hand-edited row or a stale schema. Every other check
/// switches on the kind, so it must be rejected up front rather than falling into a default branch.
/// </summary>
[Fact]
public void An_undefined_ScopeKind_ordinal_is_a_deploy_error()
{
var errors = DraftValidator.Validate(Draft(Acl((NodeAclScopeKind)99, "anything")));
errors.ShouldContain(e => e.Code == "NodeAclScopeKindUndefined" && e.Context == "acl-1");
}
/// <summary>Every bad row is reported in one pass — the validator's stated contract, and what lets an
/// operator fix a batch of ACLs in one edit instead of one deploy attempt each.</summary>
[Fact]
public void All_offending_rows_are_reported_in_one_pass()
{
var errors = DraftValidator.Validate(Draft(
Acl(NodeAclScopeKind.Equipment, null, "acl-a"),
Acl(NodeAclScopeKind.Cluster, OwnEquipId, "acl-b"),
Acl(NodeAclScopeKind.RawDevice, "nope", "acl-c"),
Acl(NodeAclScopeKind.RawDevice, "dev-2", "acl-d")));
errors.Where(e => e.Code.StartsWith("NodeAcl", StringComparison.Ordinal))
.Select(e => e.Context)
.ShouldBe(["acl-a", "acl-b", "acl-c", "acl-d"], ignoreOrder: true);
}
}