mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-19 15:22:15 +08:00
8a10139b8c
# Description
A live OPC UA compliance audit of `samples/PumpDeviceIntegrationServer`
against the **OPC 40223 Pumps** and **OPC 10000-100 (DI)** companion
specifications found **13 address-space violations** plus a
serialization bug in the MCP tool. Every fix lands at its **root cause
in the shared stack** rather than as a workaround in the sample, so all
servers benefit.
The sample now exposes **N fully simulated pumps** (`--pumps N`) and
advertises the DI conformance facets it actually satisfies.
> This branch has been merged with `master` including **#4116** ("Add
simulation to PumpDeviceIntegrationServer and Event modelling"), which
independently reworked the same pump simulation and the same fluent
alarm builders. See [Relationship to #4116](#relationship-to-4116) below
for how the two were reconciled.
## Findings and fixes
| # | Finding | Root cause | Fix |
|---|---------|-----------|-----|
| 1 | `OverTempAlarm` created by the fluent API did not exist in the
address space | `AttachAlarm` attached the alarm with `AddChild` but
never indexed it into the node manager | Register the node; the same gap
existed in `AddObject`, `CreateInstance` and the state-machine creators
|
| 2 | Alarms were unsubscribable — `EventNotifier = 0` everywhere, no
`HasNotifier` from `Server` | No event-source wiring at all | Set
`HasCondition`, initialise
`SourceNode`/`SourceName`/`ConditionName`/`InputNode`, promote
`EventNotifier` up the ancestor chain, publish `HasNotifier` from the
`Server` object |
| 3 | Every instance NodeId was minted in the **DI standard namespace**
(`ns=4;s=5001_Pump #1`) | `DiNodeManager.New` inherited the parent's
namespace index | New `DiNodeManager.InstanceNamespaceIndex`; instances
move to the server's application namespace |
| 4 | `Server/Namespaces` described only 2 of 6 namespaces | The
NodeSet2 importer drops nodes parented to namespace 0, so companion-spec
metadata objects never reached the address space — and no route covered
the server's own namespace | New `NamespaceMetadataPublisher` walks the
`NamespaceArray` and fills version/publication date from
`ModelDependencyAttribute` |
| 5 | Vendor BrowseNames (`Diagnostics`, `LastError`, …) sat in the DI
namespace | Builders defaulted to the parent's namespace | Default to
the server namespace; **spec-defined BrowseNames are unchanged** |
| 6 | `AccessLevel="5"` from the Pumps NodeSet was reported as `1` | The
ModelDesign `AccessLevel` enum is not `[Flags]` and cannot represent
`CurrentRead \| HistoryRead` | Carry the verbatim NodeSet2 bitmask and
emit named `AccessLevels` constants |
| 7 | `Historizing = true` while `HistoryRead` returned
`BadHistoryOperationUnsupported` | Nothing reconciled the declared
history surface with the wired historians | Startup clears the
advertisement (including masking the attribute read callbacks) when no
provider resolves; the sample wires a real historian |
| 8 | Mandatory `ProductInstanceUri` unset | Sample omission | Populated
on every pump |
| 9 | Mandatory `TrueState`/`FalseState` empty on every supervision
boolean | Absent in the official NodeSet; a server must still populate
them | Populated on every pump |
| 10 | Machinery `Machines` folder empty (OPC 40001-1 §9.2 says
**shall**) | `TryAddToMachinesFolder` used `AddChild`, which
*re-parents* the device away from `DeviceSet` | Add the `Organizes`
reference instead |
| 11 | BrowseName contained a space and `#` | Sample naming | `Pump_1`
with the readable label in `DisplayName` |
| 12 | `Maintenance` group materialised but empty | Sample omission |
Populated |
| 13 | Uninitialised values reported `Good` | No initial-value status |
`BadWaitingForInitialData` until the first simulation tick |
| — | `ServerProfileArray` advertised only `StandardUA2017` |
`DiNodeManager` did not implement the existing `IConformanceContributor`
| Contribute DI 1.05 server facets computed at runtime from what is
actually wired, merged with `StandardUA2017` |
| — | MCP tool serialised boolean `false` and numeric `0` as JSON `null`
| `Variant.Null` is `default`, so `Variant.Equals` adopts the non-null
type and compares against the default value | Dispatch on `BuiltInType`
and use typed `TryGetValue` accessors instead of the prohibited
`AsBoxedObject` |
## Relationship to #4116
#4116 landed on `master` while this branch was in flight and targeted
overlapping ground. The merge is a **union of both intents**, not a
fast-forward:
- **`AlarmBuilderExtensions`** — kept both sides. #4116 contributes the
parent-must-be-an-Object guard, `SetEnableState`, and the
`HasEventSource` references; this branch contributes the `HasCondition`
reference, condition source initialisation, **registration of the alarm
with the node manager so it is browsable**, notifier promotion up the
whole ancestor chain, and root-notifier registration.
- **`SupervisionBuilderExtensions`** — took #4116's `SetAlarmActive`. It
is more spec-correct than this branch's version: it honours
`EnabledState`, resets Acked/Confirmed on activation, and computes
`Retain` per OPC 10000-9 so an unacknowledged Condition stays retained
after going inactive. This branch's null guard was kept so a malformed
alarm cannot dereference a missing `EnabledState`.
- **`PumpNodeManager.Configure.cs`** — took #4116's **push-based**
simulation (`RegisterPumpSimulation`, `CreatePumpSimulation`,
`IValueUpdater<T>`, `Initialize`/`Advance`/`Publish`, deterministic
per-pump phase offsets) as the structure, then folded in this branch's
compliance work: configurable pump count, Maintenance group,
`TrueState`/`FalseState`, historian wiring, per-pump mandatory
Identification values, and `BadWaitingForInitialData` until the first
tick.
- **README** — union, keeping #4116's address-space validation workflow
section.
Two test adjustments were needed, both because the merge legitimately
changed behaviour the tests pinned:
- `PumpHostedReferenceTests` now resolves pump NodeIds from the browse
result instead of hard-coding the DI namespace, and waits for the first
published value. `PumpCreatedAfterStartupJoinsTheLiveSimulationAsync`
was **added** to restore #4116's coverage that a pump created after
startup through `ConfigureDevicesFor` joins the live simulation.
- `FluentAlarmRegistrationIntegrationTests` no longer asserts that
`Retain` clears when an alarm goes inactive — that pinned this branch's
weaker semantics, and #4116's Part 9 behaviour is correct.
## Breaking change
Runtime-created DI device instance NodeIds move from the companion-spec
namespace to the server's application namespace, e.g.
`ns=<DI>;s=5001_Pump #1` becomes `ns=<application>;s=5001_Pump #1`. This
invalidates NodeIds cached by clients from earlier 2.0 previews. It is
**not** a model change — standard DI/Machinery/Pumps type NodeIds and
spec-defined BrowseNames are untouched.
Device integration hosting is new in 2.0, so there is nothing to migrate
from 1.5.378 and this is deliberately *not* in `docs/MigrationGuide.md`.
Anyone tracking 2.0 previews should rediscover devices by browsing
`DeviceSet` rather than persisting NodeIds, and resolve namespace
indexes from `NamespaceArray` per connection.
## Verification
Compliance is pinned by regression tests that assert **through the
server's service surface** (`BrowseAsync` / `ReadAsync` /
`HistoryReadAsync`), not by inspecting `NodeState` objects in memory.
That distinction mattered: three defects survived in-memory assertions
and were only caught through the read path — most subtly, `Historizing`
was cleared while `AccessLevel` still advertised `HistoryRead`, because
attribute read callbacks re-applied the NodeSet bits.
Post-merge on net10.0:
| Suite | Result |
|---|---|
| `Opc.Ua.Di.Tests` | 326 passed |
| `Opc.Ua.Server.Tests` | 3846 passed, 5 skipped |
| `Opc.Ua.Tools.Tests` | 288 passed |
| `Opc.Ua.History.Tests` | 506 passed, 27 skipped |
| `Opc.Ua.SourceGeneration.Core.Tests` | 3722 passed, 8 skipped |
The full solution was also built and tested on **net48** before the
merge.
Review feedback from `copilot-pull-request-reviewer` has been addressed
in commit `badad31dc`: the shared `ServerObjectState` lock was removed
(the forward `HasNotifier` edge is published through the owning node
manager instead), MCP array serialization now preserves element types,
and the pump option guards report the specific rejected option. The only
failure is
`ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync`, a
pre-existing certificate-store issue that **passes when run in
isolation** and is unrelated to this change.
## Documentation
Updated `docs/DeviceIntegration.md`, `docs/MigrationGuide.md`,
`docs/HistoricalAccess.md`, `docs/DeveloperGuide.md`,
`docs/McpServer.md`, `docs/README.md`,
`samples/PumpDeviceIntegrationServer/README.md` and
`tools/Opc.Ua.Mcp/README.md`, and consolidated the four node-manager
documents into a single [`docs/NodeManagers.md`](docs/NodeManagers.md) —
overview, built-in managers (including `MasterNodeManager`), core vs
custom, registration, server address-space metadata, and source
generation — with a TOC and all inbound links repointed.
## Related Issues
No tracking issue exists for this work yet — it originated from an
ad-hoc compliance audit of the pump sample. Given the size of the change
and the runtime-breaking NodeId move, please open (or link) a tracking
issue so the design is recorded as an ADR before merging.
- Fixes #
## Checklist
_Put an `x` in the boxes that apply. You can complete these step by step
after opening the PR._
- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [x] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0d50c3fc-58b1-4c99-85a4-1e32c88666d9
203 lines
10 KiB
Markdown
203 lines
10 KiB
Markdown
# NodeManagement service set
|
|
|
|
The OPC UA NodeManagement service set (OPC 10000-4 §5.8) gives clients
|
|
the ability to manipulate the server's address space at runtime:
|
|
|
|
| Service | Description |
|
|
| --- | --- |
|
|
| `AddNodes` | Create a new node beneath an existing parent (Object, Variable, ...). |
|
|
| `DeleteNodes` | Remove a node and, optionally, all references that target it. |
|
|
| `AddReferences` | Add a reference between two existing nodes. |
|
|
| `DeleteReferences` | Remove a reference between two existing nodes. |
|
|
|
|
The SDK ships full request validation, per-item dispatch, audit-event
|
|
emission, and per-NodeManager opt-in. NodeManagers do not need to
|
|
override the four `StandardServer` service methods themselves.
|
|
|
|
## Architecture
|
|
|
|
1. The client request lands on `StandardServer.AddNodesAsync` /
|
|
`DeleteNodesAsync` / `AddReferencesAsync` / `DeleteReferencesAsync`.
|
|
2. Each service override validates the request and forwards the
|
|
per-item collection to the matching
|
|
`IMasterNodeManager.AddNodesAsync` etc. dispatcher.
|
|
3. `MasterNodeManager` looks up the owning NodeManager for every item
|
|
(the requested NodeId namespace, or the BrowseName namespace when the
|
|
Server assigns the NodeId, for `AddNodes`; node for `DeleteNodes`; and
|
|
source for reference changes)
|
|
and validates the matching `PermissionType` before asking it to
|
|
perform the work via the optional `INodeManagementAsyncNodeManager`
|
|
interface. `AddNodes` requires `AddNode` in the target namespace's
|
|
default policy and `AddReference` on the parent. Every Node permission
|
|
is evaluated together with the Node's applicable
|
|
`AccessRestrictions`.
|
|
4. The aggregated per-item results are wrapped in the matching response
|
|
envelope and the audit event (`AuditAddNodesEvent`,
|
|
`AuditDeleteNodesEvent`, `AuditAddReferencesEvent`,
|
|
`AuditDeleteReferencesEvent`) is emitted with an aggregated status.
|
|
|
|
NodeManagers that do not implement `INodeManagementAsyncNodeManager` —
|
|
or have not opted in — return `BadUserAccessDenied`, which is the
|
|
status defined for "server does not allow this operation". This keeps
|
|
the default behavior of every existing NodeManager unchanged.
|
|
|
|
## Opting in: `INodeManagementAsyncNodeManager`
|
|
|
|
```csharp
|
|
public interface INodeManagementAsyncNodeManager
|
|
{
|
|
// Sub-class returns true to opt in. Default is false.
|
|
bool AllowNodeManagement { get; }
|
|
|
|
ValueTask<(ServiceResult result, NodeId addedNodeId)> AddNodeAsync(
|
|
OperationContext context,
|
|
AddNodesItem item,
|
|
CancellationToken cancellationToken = default);
|
|
|
|
ValueTask<ServiceResult> DeleteNodeAsync(
|
|
OperationContext context,
|
|
DeleteNodesItem item,
|
|
CancellationToken cancellationToken = default);
|
|
|
|
ValueTask<ServiceResult> AddReferenceAsync(
|
|
OperationContext context,
|
|
AddReferencesItem item,
|
|
CancellationToken cancellationToken = default);
|
|
|
|
ValueTask<ServiceResult> DeleteReferenceAsync(
|
|
OperationContext context,
|
|
DeleteReferencesItem item,
|
|
CancellationToken cancellationToken = default);
|
|
}
|
|
```
|
|
|
|
`AsyncCustomNodeManager` already implements
|
|
`INodeManagementAsyncNodeManager` with default behavior that handles
|
|
every common case. Sub-classes opt in by overriding a single property:
|
|
|
|
```csharp
|
|
public sealed class MyNodeManager : AsyncCustomNodeManager
|
|
{
|
|
public MyNodeManager(IServerInternal server, ApplicationConfiguration config, ILogger logger)
|
|
: base(server, config, logger, "http://example.org/MyNodes/")
|
|
{
|
|
}
|
|
|
|
public override bool AllowNodeManagement => true;
|
|
}
|
|
```
|
|
|
|
That is the only change needed for a NodeManager to start accepting
|
|
AddNodes / DeleteNodes / AddReferences / DeleteReferences requests.
|
|
|
|
The reference server's `ReferenceNodeManager` opts in this way; you can
|
|
use it as the working example.
|
|
|
|
## Dispatch and routing rules
|
|
|
|
- **AddNodes**: routed by the namespace of `RequestedNewNodeId` when
|
|
the caller supplies one. When `RequestedNewNodeId` is null, routed
|
|
to the parent's owning NodeManager. As a pragmatic fallback for the
|
|
common case where the parent is `ObjectsFolder` (owned by the
|
|
read-only `CoreNodeManager`), the dispatcher routes to the first
|
|
NodeManager that has opted in to NodeManagement.
|
|
- **DeleteNodes**: routed to the owning NodeManager of the target node.
|
|
- **AddReferences / DeleteReferences**: routed to the source node's
|
|
owning NodeManager. Target-side validation and inverse mutation occur
|
|
only when the target is explicitly local: `targetServerUri` is empty,
|
|
and the `ExpandedNodeId` has neither a server index nor namespace URI.
|
|
When that target lives in a different opted-in NodeManager, the
|
|
dispatcher resolves the actual source and target `NodeClass`, checks
|
|
both local endpoints before mutation, and mirrors the complementary
|
|
edge. If the inverse mutation fails or is cancelled, the source
|
|
mutation is compensated with an independent bounded cleanup token
|
|
before the original failure is returned or rethrown.
|
|
|
|
## Request handling on `AsyncCustomNodeManager`
|
|
|
|
The default implementation honors the following request fields:
|
|
|
|
| Field | Behavior |
|
|
| --- | --- |
|
|
| `RequestedNewNodeId` | Used as the new node's `NodeId` when valid; otherwise `BadNodeIdRejected`. When null, a fresh `NodeId` is allocated via `INodeIdFactory.New`; if the override returns a null `NodeId` (for example, because it derives identifiers from a parent that is not yet attached) the SDK falls back to the base allocator so AddNodes always yields a usable identifier. `BadNodeIdExists` if it collides with an existing node. |
|
|
| `BrowseName` | Must be non-null. `BadBrowseNameDuplicated` when a sibling under the same parent already uses the same browse name — both local children and previously-added cross-NodeManager children attached to the same parent by this NodeManager are checked. |
|
|
| `ParentNodeId` | Used to attach the new child to the parent. Cross-NodeManager parents are supported — the forward `parent → child` edge is added via the master so the parent's NodeManager records it, and the inverse `child → parent` edge is added on the new node. |
|
|
| `ReferenceTypeId` | Must be a `HierarchicalReferences` subtype. |
|
|
| `NodeAttributes` | Optional `VariableAttributes` / `ObjectAttributes` are applied to the new node (`DisplayName`, `Description`, `DataType`, `ValueRank`, `AccessLevel`, `UserAccessLevel`, `Historizing`, `MinimumSamplingInterval`, `Value`, `EventNotifier`). |
|
|
| `DeleteTargetReferences` | When true, `DeleteNodes` also removes references on other NodeManagers that target the deleted node. |
|
|
| `DeleteBidirectional` | When true on `DeleteReferences`, the inverse edge is also deleted only for an explicitly local target. Remote targets remain source-side operations. |
|
|
|
|
## Error codes returned
|
|
|
|
| Service | Status | Reason |
|
|
| --- | --- | --- |
|
|
| AddNodes | `BadBrowseNameInvalid` | `BrowseName` is null. |
|
|
| AddNodes | `BadParentNodeIdInvalid` | `ParentNodeId` is null or unknown. |
|
|
| AddNodes | `BadReferenceTypeIdInvalid` | `ReferenceTypeId` is null or unknown. |
|
|
| AddNodes | `BadReferenceNotAllowed` | `ReferenceTypeId` is not a hierarchical reference. |
|
|
| AddNodes | `BadNodeIdRejected` | The requested or BrowseName namespace has no owning NodeManager, or `RequestedNewNodeId` is outside the selected NodeManager's namespace. |
|
|
| AddNodes | `BadNodeIdExists` | `RequestedNewNodeId` already exists. |
|
|
| AddNodes | `BadBrowseNameDuplicated` | A sibling beneath the same local parent already uses the browse name. |
|
|
| AddNodes | `BadNodeClassInvalid` | Only `Object` and `Variable` are supported by the default implementation. |
|
|
| AddNodes | `BadNodeAttributesInvalid` | The supplied attributes extension object does not match the node class. |
|
|
| AddNodes / DeleteNodes / AddReferences / DeleteReferences | `BadUserAccessDenied` | The owning NodeManager has not opted in to NodeManagement or the Session lacks `AddNode`, `DeleteNode`, `AddReference`, or `RemoveReference` permission. |
|
|
| AddNodes / DeleteNodes / AddReferences / DeleteReferences | `BadSecurityModeInsufficient` | An applicable namespace, parent, source, or local target `AccessRestrictions` policy requires a stronger SecureChannel. |
|
|
| DeleteNodes | `BadNodeIdInvalid` / `BadNodeIdUnknown` | The node to delete is null or unknown. |
|
|
| AddReferences | `BadSourceNodeIdInvalid` | Source is null or unknown to this NodeManager. |
|
|
| AddReferences | `BadTargetNodeIdInvalid` | Target is null. |
|
|
| AddReferences | `BadDuplicateReferenceNotAllowed` | The exact reference (type + direction + target) already exists on the source. |
|
|
| DeleteReferences | `BadNoMatch` | No reference with the exact triple (type + direction + target) was found on the source. |
|
|
|
|
## Audit events
|
|
|
|
Every NodeManagement service emits its corresponding audit event:
|
|
|
|
- `AuditAddNodesEventType`
|
|
- `AuditDeleteNodesEventType`
|
|
- `AuditAddReferencesEventType`
|
|
- `AuditDeleteReferencesEventType`
|
|
|
|
The audit `Status` is the first bad per-item status, or `Good` when
|
|
every item succeeded. Audit events are emitted from both the success
|
|
and failure paths so an early `ServiceResultException` (request-level
|
|
validation) still appears in the audit log.
|
|
|
|
## Client-side example
|
|
|
|
The standard `Session` / `ManagedSession` AddNodes / DeleteNodes /
|
|
AddReferences / DeleteReferences APIs work unchanged against any
|
|
server whose NodeManager has opted in. See OPC 10000-4 §5.8 for the
|
|
service contract.
|
|
|
|
## Custom behavior
|
|
|
|
To customize behavior for a particular NodeManager (e.g. enforce a
|
|
custom NodeId scheme, persist the new node, or veto specific node
|
|
classes), override the matching method on
|
|
`INodeManagementAsyncNodeManager` after opting in. For example, to
|
|
restrict additions to `BaseObjectState`:
|
|
|
|
```csharp
|
|
public override async ValueTask<(ServiceResult result, NodeId addedNodeId)> AddNodeAsync(
|
|
OperationContext context,
|
|
AddNodesItem item,
|
|
CancellationToken cancellationToken = default)
|
|
{
|
|
if (item.NodeClass != NodeClass.Object)
|
|
{
|
|
return (new ServiceResult(StatusCodes.BadNodeClassInvalid), NodeId.Null);
|
|
}
|
|
return await base.AddNodeAsync(context, item, cancellationToken).ConfigureAwait(false);
|
|
}
|
|
```
|
|
|
|
## See also
|
|
|
|
- [Reference Server](../samples/README.md) — opt-in example in
|
|
`ReferenceNodeManager`.
|
|
- [Source generated NodeManagers](NodeManagers.md#source-generated-node-managers) —
|
|
combine NodeManagement with generated models.
|
|
- [Model Change Tracking](ModelChangeTracking.md) — `AddNodes` and
|
|
`DeleteNodes` emit `GeneralModelChangeEvent` automatically when
|
|
model change tracking is enabled.
|