mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-18 06:31:35 +08:00
fix/encoder-stack-depth-json-flake
2739 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Marc Schier
|
d3d25178af | Merge branch 'master' into fix/encoder-stack-depth-json-flake | ||
|
Marc
|
fbd204bcb9 |
Make the encoder stack-depth tests hold on a reused thread stack
DecodeWithUnboundedNestingLimitFailsBeforeTheStackOverflows(Json, VariantArray) and (Json, DataValue) failed intermittently on Linux CI: the 450-level JSON input decoded without tripping the stack check. A thread that asks for a 256 KB stack can run on more. glibc reuses the cached stack of an exited thread when it is at most four times the requested size, and this fixture's own 1 MB threads leave such stacks behind. The stack check measures the stack the thread got, and 450 JSON levels fit a 1 MB stack. Running a 1 MB thread right before the decode reproduces the failure on Linux 20 times out of 20; Windows does not reuse stacks this way. The unbounded-limit tests now run on the 1 MB and the 256 KB stack and nest 5000 levels, twice the deepest nesting of any codec and shape that fits a 1 MB stack (2477 levels: JSON EncodeableTree, Linux, Release). The 2000 levels of the other codecs were too few as well: on Linux, encoding a Binary EncodeableTree of 2000 levels fits a 1 MB stack. The JSON input is written by hand because the JsonEncoder cannot nest deeper than 1000 levels, and NestedJsonMatchesTheJsonEncoder keeps it equal to the encoder's output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Marc Schier
|
60dc5ab404 |
Reset the push coordinator after each PushComplianceRegressionTests test (#4719)
# Description `dotnet test tests/Opc.Ua.Server.Tests` exits 1 even when every test passes. The assembly-level `LeakDetectionSetup` teardown reports `Certificate leak detected: 3 instance(s) created but not disposed`. Cause: `CloseAndUpdateRejectsRetainedCrlOfRemovedCaAsync` and `CloseAndUpdateAcceptsVersion3CertificateWithoutExtensionsAsync` stage TrustList updates through `CloseAndUpdate` and finish without `ApplyChanges` or `CancelChanges`. `TearDown` disposed the TrustLists but not the staged operations, so the `PushConfigurationTransactionCoordinator` kept the staged and original `CertificateCollection`s undisposed. CI did not catch it: `.github/scripts/get-test-verdict.ps1` accepts a nonzero exit when the TRX counters show no failures, and reports it only as an "at-exit stall" warning. Fix: `TearDown` calls `m_coordinator.Reset()`, which discards the staged operations and disposes their collections. This is a test-only change; in production, a session's staged operations are applied, cancelled, or discarded when the session closes. ### Validation | Command | Result | | --- | --- | | `dotnet test tests/Opc.Ua.Server.Tests -f net10.0 -p:CustomTestTarget=net10.0 --filter "FullyQualifiedName~PushComplianceRegressionTests"`, before the fix | 15 passed, but exit 1: `TearDown failed for test fixture Opc.Ua.Server.Tests.LeakDetectionSetup`, `Certificate leak detected: 3 instance(s) created but not disposed (created=49, disposed=46)` | | Same, after the fix, 3 runs | 15 passed, exit 0, no leak report | | Same with `-f net48 -p:CustomTestTarget=net48` | 15 passed, exit 0 | | `dotnet test tests/Opc.Ua.Server.Tests -f net10.0 -p:CustomTestTarget=net10.0` | 8386 passed, 0 failed, 5 skipped; exit 0 | ## Related Issues - No tracking issue. Found while explaining the exit code 1 of a full `Opc.Ua.Server.Tests` run for #4716. ## Checklist - [ ] 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. - [ ] 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. - [ ] 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> |
||
|
Marc Schier
|
aeeef5b420 |
Send UDP multicast on the network interface the transport joins (#4711)
# Description `UdpDatagramTransport` joined multicast groups on the resolved network interface: the connection address's `NetworkInterface`, the `PreferredNetworkInterface` option, or the first operational interface. It never set `IP_MULTICAST_IF` / `IPV6_MULTICAST_IF`, though, so multicast data and discovery announcements left through the operating system's default multicast route. On a computer with several network interfaces, a publisher configured for one network sent on another, and subscribers on the configured network missed the messages (OPC 10000-14 §7.3.2.2, `NetworkAddressDataType.NetworkInterface`). `ConfigureSocket` now sets the multicast send interface to the resolved interface for IPv4 and IPv6 sockets. That includes the separate socket that sends discovery announcements for loopback-bound connections. A failure is logged at Debug level, like the other socket options. **Behavior change:** without a configured interface, multicast now leaves through the first operational non-loopback interface, which is the interface subscribers join on, instead of the default multicast route. `docs/PubSub.md` and the XML docs of `PreferredNetworkInterface` and `UdpNetworkInterfaceResolver` describe this and recommend naming the interface on multi-homed computers. Tests (`UdpDatagramTransportMulticastInterfaceTests`): - The IPv4 and IPv6 egress options equal the resolved interface. Before the fix they were `0.0.0.0` and `0`. - A publisher and subscriber pinned to the loopback interface exchange a payload. A raw control sender tells a host without loopback multicast apart from a publisher that sends elsewhere. The test fails with the bug on Linux; Windows also delivers local copies sent on other interfaces, so there it only confirms the exchange. ### Validation | Command | Result | | --- | --- | | `dotnet test tests/Opc.Ua.PubSub.Udp.Tests -f net10.0 -p:CustomTestTarget=net10.0 --filter "FullyQualifiedName~UdpDatagramTransportMulticastInterfaceTests"`, with the fix temporarily disabled | IPv4 and IPv6 option tests failed (`Expected: <192.168.178.101> But was: <0.0.0.0>`, `Expected: 4 But was: 0`) | | The same tests from the Windows-built binaries in a Linux container (`mcr.microsoft.com/dotnet/sdk:10.0`, `dotnet test Opc.Ua.PubSub.Udp.Tests.dll`), fix disabled, 5 runs | 3 of 3 failed in every run, including the loopback exchange (the control sender arrived, the publisher's multicast did not) | | Same, with the fix, 5 runs | 3 of 3 passed in every run | | `dotnet test tests/Opc.Ua.PubSub.Udp.Tests -f net10.0 -p:CustomTestTarget=net10.0` | 237 passed (Windows, and the same binaries in the Linux container) | | `dotnet test tests/Opc.Ua.PubSub.Udp.Tests -f net48 -p:CustomTestTarget=net48` | 232 passed | | `dotnet test tests/Opc.Ua.PubSub.Tests -f net10.0 -p:CustomTestTarget=net10.0` | 1490 passed, 1 skipped | | `dotnet build src/Opc.Ua.PubSub.Udp -c Release` (net10.0, net9.0, net8.0, net48) | 0 warnings | | The docs/PubSub.md first exchange (unicast loopback), run verbatim, with #4710 applied | 11 DataSets; the unsecured-frame warning appears once, as documented | ## Related Issues - No tracking issue. Found while verifying the PubSub first-exchange documentation (follow-up to #4555). ## Checklist - [ ] 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. - [ ] 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> |
||
|
Marc Schier
|
9600f0de9c |
Let servers start in globalization-invariant mode (#4716)
# Description
A server did not start in globalization-invariant mode
(`InvariantGlobalization`, or
`DOTNET_SYSTEM_GLOBALIZATION_INVARIANT=1`), which Native AOT and
container images often enable. When the server loaded its default status
texts, `ResourceManager` created `CultureInfo("en-US")`. In this mode
the runtime throws `CultureNotFoundException` for every culture except
the invariant one, so the server crashed in
`ResourceManager.LoadDefaultText`.
`ResourceManager` now:
- keeps the locale id of each translation table as a string, and falls
back to its language subtag to find another region of the same language;
- when the runtime cannot create the culture, parses a locale id that
matches the grammar of RFC 5646 §2.1 into the recommended casing of its
subtags. It still rejects a language without a region, and rejects a tag
that does not match the grammar, as before;
- caches the resolved locale ids, so loading the default texts creates
each culture once.
`docs/NativeAoT.md` no longer warns against invariant mode for servers.
The new tests (`ResourceManagerInvariantGlobalizationTests`) reproduce
both invariant-mode behaviors of the runtime: no cultures at all, and
cultures that carry invariant data. They also cover malformed tags, and
well-formed tags with extended language, variant, extension, and private
use subtags.
The branch includes a merge of master: #4707 moved translation lookups
to a lock-free snapshot of the tables, and the snapshot now copies the
locale-id-based tables.
### Validation
| Command | Result |
| --- | --- |
| `dotnet test tests/Opc.Ua.Server.Tests -f net10.0
-p:CustomTestTarget=net10.0 --filter
"FullyQualifiedName~ResourceManager"` | 100 passed, including the 39
tests of the new fixture. Before the fix, 10 of its first 22 tests
failed; 7 of the 17 grammar cases failed against the first version of
the parser |
| `dotnet test tests/Opc.Ua.Server.Tests -f net48
-p:CustomTestTarget=net48 --filter "FullyQualifiedName~ResourceManager"`
| 100 passed |
| `dotnet test tests/Opc.Ua.Server.Tests -f net10.0
-p:CustomTestTarget=net10.0`, on
|
||
|
Marc Schier
|
e80d0ca9ac |
Use a 4-byte KeyNonce in the reference PubSub sample (#4710)
# Description
The first PubSub exchange in
[docs/PubSub.md](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/docs/PubSub.md#run-a-first-publisher-and-subscriber)
delivers no DataSets on master or in the 2.0.0 source. The subscriber
only logs `DataSetReader Reader 1 faulted on MessageReceiveTimeout`,
while the publisher logs `UADP security wrap failed; dropping message`
with `ArgumentException: AES-CTR nonce must be exactly 12 bytes` for
every NetworkMessage.
Cause: the reference PubSub sample's demo keys use a 12-byte KeyNonce.
Since #4573, the AES-CTR counter block follows OPC 10000-14 Table 157
(`KeyNonce[4] || MessageNonce[8] || BlockCounter[4]`). With the sample's
keys, the publisher builds a 20-byte nonce, and `AesCtrTransform`
rejects it. The 2.0.0 release (release/2.0 at
|
||
|
Marc Schier
|
0af7ae3a23 |
Update the guides and the MCP server docs for the 2.0.0 release (#4713)
# Description The packages on nuget.org are stable 2.0.0 now. The guides still told readers to use `--prerelease` and preview-only versions, and kept caveats that applied only to 2.0.0-preview.5 and earlier previews. Getting started, Your first information model, and Source-generated data types: - Add the packages without `--prerelease` and name 2.0.0 as the verified version. - With 2.0.0, a client with no server reports `BadNotConnected`, so the troubleshooting row drops the preview.5 `SocketException`. - 2.0.0 generates code for methods that declare their arguments inline, so the tutorial drops the preview.5 workaround note and its troubleshooting row, and says both forms are possible. MCP server (`docs/McpServer.md` and the tool's NuGet README `tools/Opc.Ua.Mcp/McpREADME.md`): - Install and run commands use the stable tool: `dotnet tool exec OPCFoundation.NetStandard.Opc.Ua.Mcp`, `dnx …`, and `dotnet tool install --global …`, without `--prerelease` or `2.0.0-preview.*`. - The stable tool bundles `Opc.Ua.Mcp.Robotics` and `Opc.Ua.Mcp.Vision`, so the note that `vision` and `vision,robotics` need the current source is removed. The docs now state that only the standalone `Mcp.Robotics` and `Mcp.Vision` libraries are preview packages (`2.0.0-preview.200`). - Not changed: `tools/Opc.Ua.Mcp/.mcp/server.json` still declares `"version": "1.0.0"` and lists only the original profiles in its description. ### Validation | Command | Result | | --- | --- | | Getting started Route B, the first-model tutorial, and every troubleshooting row, run exactly as documented against the 2.0.0 packages | Output matches the guides; 0 warnings | | Native AOT publish of the Route B server and client | 0 warnings; the native executables completed the trust flow and connected | | `dotnet tool exec OPCFoundation.NetStandard.Opc.Ua.Mcp -- --version`, `dnx OPCFoundation.NetStandard.Opc.Ua.Mcp -- --version`, `dotnet tool exec "OPCFoundation.NetStandard.Opc.Ua.Mcp@2.0.*" -- --version` | `2.0.0+bc5da1006…` for each | | `dotnet tool install` of `OPCFoundation.NetStandard.Opc.Ua.Mcp` without `--prerelease` (into a temporary tool path) | Installs 2.0.0 | | `dotnet tool exec` of an uncached version with stdin redirected, as an MCP client launches it | Runs without a confirmation prompt | | MCP stdio `initialize` + `tools/list` against the 2.0.0 tool with `--profile vision,robotics`, `vision`, `robotics`, `full` | 64, 26, 42, and 138 tools | | Markdown link, anchor, contents, and fence checks over `docs/` | 0 issues | ## Related Issues - No tracking issue. Follow-up to #4555 after the 2.0.0 release. ## Checklist - [ ] 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. - [ ] 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. - [ ] 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> |
||
|
romanett
|
36194f6a02 |
Avoid boxing and per-value copies on encoder, enum and server hot paths (#4715)
## Summary Allocation audit of the per-message / per-value paths (binary codec, built-in types, server request path, monitored items, client notification dispatch) looking for boxing and avoidable per-value copies. Measured with `GC.GetAllocatedBytesForCurrentThread` probe loops on net10.0 **and net48**, and with `dotnet-trace` allocation traces of the server load harness (#4707) for read/write/browse/subscription, server and client side — a value type in the "allocated types" list is a box. Wire output and error behaviour are unchanged. ### Fixed | Where | Cause | Frequency | Before → after | |---|---|---|---| | `Polyfills/System.IO.cs` (net48/netstandard2.0) | `BinaryWriter.Write(ReadOnlySpan<byte>)` polyfill copied with `ToArray()` — every string, array, ByteString, Guid write | per value | 32–64 B → 0; BrowseResponse×50 encode 15.3 → 10.9 KB | | `EnumHelper` (net48/netstandard2.0) | `EnumToInt32`/`Int32ToEnum`/`EnumToInt64` boxed outside `NET8_0_OR_GREATER` (every enum field, `Variant.From<TEnum>`, `EnumValue.From`) | per value | 24 B → 0 | | `BinaryDecoder.ReadString` + stream raw reads (net48) | temp `byte[]` per string, `BinaryReader.ReadBytes` | per value | BrowseResponse×50 decode 15.7 → 11.3 KB | | `WriteGuid` / `ReadGuid` (all) | `ToByteArray()` / `byte[16]` | per value (also Guid NodeIds) | 40 B → 0 each | | `VariantHelper.TryCastTo<TEnum>` (`DataValue.GetValue<TEnum>`), `EventRecordFieldReaders.GetEnum` | `Enum.ToObject` box | per call / per event field | 24 B → 0 | | `NodeManagerServiceDispatcher.GetManagerHandleAsync` | interface enumerator per validated node | per node | Write-100: 151.8 → 148.5 KB/request | ### Cold paths (third commit) - `Server.Auditing` write handler: `Convert.ToBoolean(object)` got a boxed `Variant` (not `IConvertible`) and threw `InvalidCastException` on any in-process write; now takes the boolean, `BadTypeMismatch` otherwise (new test). - `UpdateServerServiceLevel`: `Convert.ToByte(byte, IFormatProvider)` bound to the object overload. - Client publish-state / recovery-policy `HasFlag` (boxes on net48) → `(x & F) == F`. - `SequenceReader` big-endian float on net48/netstandard2.0 used `Convert.ToSingle(int)` (numeric conversion) instead of reinterpreting the bits. - JSON enum strings: one `Enum.GetName` + `string.Concat` instead of `IsDefined` + boxing `CoreUtils.Format`; same output (new test for the `EnumValue` overload). Left as is: `SamplingGroup` start-up `Convert.ToInt32(double, provider)` (rewritten by #4707), `SecureChannelRenewed` formatting (transport files in flight on the client perf branch), `Utils.IsEqual<T>` object path for structs (cold callers; changing it would mix `CompareTo` and `Equals` semantics), Machinery `HasFlag` (startup config). ### Not changed here - `BinaryEncoder.GetNodeIdEncoding` `Convert.ToUInt32(object)` / `Convert.ToByte(enum)` (72 B per NodeId encode, largest single item) and `ChannelAsyncOperation.Complete` boxing the `int` result are handled on the client performance branch. - By design: `Variant` / `NodeId` store `ArrayOf<T>`, Guid, ByteString identifiers boxed (`Variant.Copy` re-boxes RolePermissions during permission checks). ## Tests - New `BinaryCodecAllocationTests`: round trips in buffer and stream decoder mode, truncation errors, allocation per call (< 1 B/call over 20k calls; the counter can be off by an allocation quantum when other tests GC). - `Enum.ToObject` equivalence for `TryCastTo` and `EventRecordFieldReaders.GetEnum` across all underlying types. - net10.0 (third commit): Types 11465, affected Server 87, Client subscription 642 passed; net48 Types 11458 passed, `UA.slnx` 0 warnings. - net10.0: Types 11464, Core.Encoders 2947, Server 8386 passed; Core 6197 passed, 1 unrelated (`ActivityTraceFlags_Metrics_ShouldRecordMetricsOnly`, passes alone). - net48: Types 11457, Core.Encoders 2947, EventRecordFieldReaders 25 passed; `UA.slnx` builds with 0 warnings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
28f1401324 |
Server: cut request-path lock contention and allocation; add load harness (#4707)
Profiling the reference server under load showed most request-path CPU spent spinning on contended locks, plus several avoidable allocations per request or per node. This PR removes them, switches the reference server to Server GC, and adds the load harness used to measure it. ## Results Paired A/B against `master` (3 alternating rounds, medians, 20 sessions, workstation GC on both sides, 24-core machine): | Scenario | Throughput | p50 | p90 | Server CPU / op | Server alloc / op | Lock contention | |---|---|---|---|---|---|---| | Read, 1 value | **+45%** (94k → 137k req/s) | −20% | −46% | −37% | −39% | −94% | | Read, 100 values | **+40%** | −26% | −17% | −24% | −25% | +130% ¹ | | Read, 100 values, SignAndEncrypt | **+48%** | −36% | −20% | −30% | −26% | +106% ¹ | | Browse (Server object) | **+33%** | −31% | −12% | −13% | −14% | −40% | | Write, 100 values | +13% | −12% | −4% | −10% | −19% | −23% | | Data-change subscriptions | ±0 ² | ≈ | ≈ | −14% | −17% | −29% | ¹ The faster server spends more time in the remaining per-node locks. ² Throughput is bounded by the writer rate in this scenario. Separately, Server GC instead of workstation GC doubles Read-100 throughput (21k → 44k req/s) and cuts p90 from 2.1 ms to 0.6 ms, because workstation GC paused the server for about half the wall time under load. The working set grows from ~210 MB to ~430 MB. ## Changes - **`ObjectPool<T>`**: lock-free slot array. `ConcurrentBag.Count` took every per-thread lock on each `Return` of the pooled response value-task source, once per request. - **`FairRequestQueue`**: - drops redundant gate entries: the fail-fast pre-checks and the stopped check read volatile fields and are re-checked under the gate; the owner count is folded into selection; - coalesces wake pulses with a pending flag, so the bounded signal channel's lock is only taken when no token is outstanding. Wake-count semantics in the tests are unchanged. - **`DefaultServerResourceIsolationProvider`**: - compares peer addresses without `GetAddressBytes()` allocations; - builds the unsecured verification context only when a classifier is configured. - **TCP listener / channel**: caches the certificate DER copies per channel, keyed on the certificate instance. `X509Certificate2.RawData` returns a fresh copy on every access, and the listener read it for every request. - **`EndpointBase`**: resolves the request `ActivitySource` once instead of `Assembly.GetCallingAssembly()` per request. - **Node managers**: - one `ServerSystemContext` copy per request (cached on the `OperationContext`) for per-node callbacks: permission metadata, node metadata, view membership, method lookup; - removes a discarded copy in `GetReferenceDescriptionAsync`; - per-request operation caches are plain `Dictionary` instead of a `ConcurrentDictionary` with 24 lock objects; - the validation cache is built in one pass; - fixes zero-capacity error lists. - **`ResourceManager`**: translation lookups read an immutable snapshot that is rebuilt after writes. Browse serialised on the global lock for every display name. - **`MonitoredItem`**: shares the default `DataChangeFilter` instead of allocating one per sampled value. - **Reference server**: `ServerGarbageCollection=true`; new "Garbage collector mode" section in `docs/ServerScalability.md`. - **`perf/`**: `ServerLoadHarness` (out-of-process server plus load driver, server-side CPU/GC/alloc/contention deltas, optional `dotnet-trace` capture), `TraceAnalyzer` (TraceEvent-based CPU/allocation/contention aggregation), and paired A/B scripts. Not part of `UA.slnx`. ### Behaviour notes for custom node managers - The `IDictionary<NodeId, NodeState>` operation cache handed to overrides is now a `Dictionary`. It was a `ConcurrentDictionary`, but only one request flow uses it. - Within one request, per-node callbacks of a node manager share one `ServerSystemContext` copy. ### Later commits on this PR - **Permission checks without `NodeMetadata`** (`9068684c3`): the role-permission and access-restriction validators run on an internal `PermissionMetadata` struct; `AsyncCustomNodeManager` fills it on the stack from the same attributes and namespace defaults. Browse's reference filter reads 3 attributes per target instead of 13. Read 100 values: 120.6 → 95.7 KB/request, ~21k → 26k req/s; Browse 67.6 → 51.8 KB, 26.7k → 29.8k req/s. - **Read-only channel evidence** (`936b28d76`, `dc638ab9f`): `SecureChannelContext.ClientChannelCertificate`, `ServerChannelCertificate` and `ChannelThumbprint` are `ByteString`, so the cached per-channel certificate buffer is shared without a copy and cannot be altered by handlers or classifiers. **Public API change**, see the new section in `docs/MigrationGuide.md`. The isolation provider's revalidation snapshot still copies. - **Session lookup** (`936b28d76`): `SessionManager.TryGetSessionContext` takes the candidate snapshot without the bindings lock; the validating check under the lock is unchanged (2 instead of 4 global lock acquisitions per request). - Unit tests for `ObjectPool`, the per-request system context cache, the translation snapshot, `SecureChannelContext` null semantics and a stopped `FairRequestQueue`; `perf/` follows the repository conventions. ## Testing - `Opc.Ua.Server.Tests` (net10.0): 8386 passed before the `ByteString` change. After it, 13 secure-session and isolation tests failed; both causes are fixed in `dc638ab9f` and those tests (25 with their variants) pass. A full rerun of the Server, Sessions, Core, Redundancy, LDS and GDS suites on the final commit is in progress. - Full solution builds for net10.0 and net48; `dotnet format whitespace` clean. ### o6-automation opcua-benchmarks (sampling, deep pipelines) Replicated both tests of the [o6-automation benchmark](https://github.com/o6-automation/opcua-benchmarks) in the harness (`--scenario counters`, and `read` with 10 clients × N outstanding), paired against `origin/master`: | Test | master | this PR | |---|---|---| | Subscription sampling, 100 Hz, every item ≥ 99 % of samples, 1 item | FAIL (75–94 %) | PASS (100 %) | | same, 1024 items | FAIL (64–73 %) | PASS (99.9 %) | | same, capacity | — | between 4096 and 6144 items (~410–520k samples/s) | | Read, 10 clients × 100 outstanding, None | stalls (0–1.7k req/s) | 121–131k req/s | | Read, 10 clients × 100 outstanding, SignAndEncrypt | stalls (0 req/s) | 89–91k req/s | | Read, 10 clients × 1000 outstanding | channels faulted (`BadTcpNotEnoughResources`) | 0 errors, ~45–54k req/s | - **Fixed-rate sampling** (`30b769d13`): the sampling loop waited one interval after each sample, adding timer overshoot and sampling time to every cycle; samples now have absolute deadlines. - **Async response send** (`cdac5c5ca`): `SendResponse` blocked thread-pool workers on the channel gate; with many requests in flight the receive loop that was handed the gate waited for a pool thread and the channel stalled. The TCP, Kestrel TCP and WebSocket listeners now use `SendResponseAsync`. - **Backpressure instead of channel fault** (`191cd2ba7`): server channels stop reading while more than 100 responses are queued and resume at 50, instead of faulting the channel. Client channels keep reading. At 10,000 outstanding requests throughput is ~50k req/s with or without the limit engaged, so the remaining gap to the 100-per-client case is not the backpressure. ## Not changed - `SentMessageQueue` deep-clones every `NotificationMessage`. Retained messages return their pooled payloads on acknowledgement while the response copy may still be queued for send, and a client can acknowledge a sequence number before receiving it; the clone keeps a recycled payload from reaching another session's response. Removing it needs a send-completion hook from the transport. - Generated responses allocate a default `ResponseHeader` that is replaced (~2-3 % of per-request allocation). Fixing it means making structure fields of every generated type lazily initialized, a generator-wide change to default values, equality and encoding. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
1fb1134974 |
Tests: scope MeterListeners to the meter under test (fix flaky ClientBaseTests metrics test) (#4708)
## Problem `ClientBaseTests.ActivityTraceFlags_Metrics_ShouldRecordMetricsOnly` failed intermittently in full-suite runs: ``` Expected: "opc.ua.client.request.duration" But was: "opc.ua.channel.reconnect.duration" ``` `TelemetryContextBase.CreateMeter()` returns a new `Meter` per call, named after the calling assembly, so `ClientBase`'s meter and `ClientChannelManagerMetrics`' meter are both named `Opc.Ua.Core`. `TestMeterListener` enabled every instrument whose `Meter.Name` matched, so reconnect-duration measurements from `[Parallelizable]` channel manager fixtures were recorded and could precede the request duration. ## Fix (test-only) - `ClientBaseTests.TestMeterListener`: enable instruments only when `ReferenceEquals(instrument.Meter, meter)`, i.e. the meter instance the client under test created. Assertions unchanged. - `CryptoProviderModelTests.AuditorPublishesAProviderMetric`: same flaw — it enabled every `opc.ua.crypto*` instrument process-wide (fixture is `ParallelScope.All`), so another auditor's uncertified gauge could overwrite the observed count. Now scoped to the auditor's own meter (read via reflection, as `ClientBaseTests` already does for `ClientBase`). Other `MeterListener` users reviewed and left as-is: `NodeCacheTests` / `AdapterMetricsTests` are non-parallel fixtures; channel manager listeners only assert existence and filter by endpoint tag. ## Verification - `ClientBaseTests` + `CryptoProviderModelTests`: 4× consecutive, 25/25. - `FullyQualifiedName~Opc.Ua.Core.Tests.Stack` (net10.0): 2 clean passes, 3548 passed / 80 skipped. One attempt hung in unrelated `ClientChannelManagerManagedTests` async tests (`ReconnectAsyncNotifiesAttachedParticipantsAsync` / `ReleasingOneLeaseKeepsChannelAliveForOtherAsync`) and was rerun — a separate intermittent hang. - net48 build of `Opc.Ua.Core.Tests`: 0 warnings, 0 errors; `dotnet format whitespace` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
7fe7794a19 |
Enable AES-GCM and ChaCha20-Poly1305 policies on net48 via BouncyCastle (stacked on #4703) (#4705)
> Follow-up to #4703 (now merged to master; this PR has been retargeted
to `master`).
Part of #4510. #4703 enabled the CBC ECC policies on net48 through a
BouncyCastle raw-ECDH polyfill. The AEAD policies were still unavailable
there because .NET Framework has no `AesGcm` or `ChaCha20Poly1305`. This
PR adds those ciphers the same way, so on net48 all of these are now
offered:
`ECC_{nistP256,nistP384,brainpoolP256r1,brainpoolP384r1}_AesGcm` /
`_ChaChaPoly`, `RSA_DH_AesGcm`, `RSA_DH_ChaChaPoly`.
## Changes
- **Polyfill**
`src/Opc.Ua.Security.Certificates/Org.BouncyCastle/AeadCiphers.cs`
(internal, `#if NETFRAMEWORK`, next to `EcdhRawAgreement`): `AesGcm` and
`ChaCha20Poly1305` types with the .NET 8 BCL surface. They wrap
BouncyCastle's `GcmBlockCipher(AesEngine)` and
`Org.BouncyCastle.Crypto.Modes.ChaCha20Poly1305`.
- A tag mismatch throws `CryptographicException`, never BC's
`InvalidCipherTextException`, so
`PlatformSymmetricCryptoProvider.DecryptAuthenticated` still returns
`false` and the channel still raises a protocol error.
- GCM releases plaintext blocks before it checks the tag, so plaintext
stays in a scratch buffer and is copied out only after the tag verifies.
On failure the destination is cleared, as the BCL does. Scratch buffers
and the key copy are zeroed.
- **Opc.Ua.Core**: aliases the polyfill types on `NETFRAMEWORK` and
removes the `NET8_0_OR_GREATER` gates in `SecurityPolicies`
(`SupportsAesGcmPolicy` / `SupportsChaCha20Poly1305Policy`),
`CryptoUtils` (`SymmetricEncryptAndSign` / `SymmetricDecryptAndVerify`,
AEAD helpers) and `PlatformSymmetricCryptoProvider`. The channel, the
provider facet and the encrypted ECC user tokens (`EncryptedSecret`,
through `CryptoUtils`) now run the same code on every target. .NET 8+
keeps the BCL ciphers.
- **FIPS** (requested by the #4703 session, after
|
||
|
romanett
|
2ba58a8cbf |
WoT: move GeneratesEvent off Object instances (OPC 10000-3 §7.15) (#4706)
Fixes #4669. OPC 10000-3 §7.15 says the SourceNode of `GeneratesEvent` "shall be an ObjectType, a VariableType or a Method". CTT `Base Info Core Structure 2 001.js` flags every Object that is the source of one. #4667 fixed this in the source generator. This PR fixes the three remaining WoT places. ### 1. WotCon asset registry - `AssetRegistry.BuildEventNode` / `RemoveEventTypes` no longer add or remove `GeneratesEvent` from the asset Object. Event delivery already relies on `EventNotifier = SubscribeToEvents` and root-notifier registration. - Tests now assert the reference is **absent** and the notifier bit is set. The re-apply test asserts that the old EventType node is gone. ### 2. WoT NodeSet converter (`src/Opc.Ua.Types/Wot`) - **TD → NodeSet:** a last pass (`RelocateInstanceGeneratesEvent`) moves forward `GeneratesEvent` references off an Object/Variable root: - An event the bound type already declares needs nothing more. Matching is by qualified BrowseName against the local context's declarations, the same rule as the declaration merge. - Any other event is declared by a synthesized non-abstract `<rootLocal>Type`. It derives from the bound type (or from `BaseObjectType` / `BaseDataVariableType` when the TD binds none) and becomes the instance's `HasTypeDefinition`. A Variable root gets a `UAVariableType` with the instance's DataType and ValueRank. - Thing Models are unchanged: the projected type keeps its `GeneratesEvent`. - **NodeSet → TD:** - An instance's event affordances are now collected from its type definition and that type's supertypes. Instance-level `GeneratesEvent` is still read for third-party NodeSets. - In a single readable document, a type that only carries one instance's events (only `HasSubtype` + `GeneratesEvent`, typing exactly one instance) is not chosen as root. It is folded back: its supertype becomes the type binding, and no link is written when that supertype is the default base type. The TD → NodeSet → TD round trip stays readable, with no `uav:nodes` fallback. - A document set gives the carrier its own Thing Model, and the instance TD binds to it. - **Archive check:** an event affordance is accepted when the root's type definition, or one of its supertypes, generates it. - `docs/WoTNodeSetConversion.md` documents the behaviour in a new "Event source" row. ### 3. Aggregation sample - `SamplePump.NodeSet2.xml`: each pump is typed by its own `PumpType` subtype, `ns=1;s=Pump1.Type` (`Pump_1Type`) and `Pump2.Type`, which declares its alarm EventTypes. - It is per pump because each pump owns its alarm EventTypes. `Pump2.Type` is cloned with the rest of Pump2. - The pumps keep `EventNotifier="1"` and no longer carry `GeneratesEvent`. - The checked-in documents are regenerated, adding `sample-pump-pump-1type.json` / `-2type.json`. The pump TDs still list the alarm event affordances. - The sample binding lookup ignores Thing Models, because the type's TM declares the same EventType as the pump TD. A declaration missing from the instance still fails as before. - Docs: `WoTConnectivity.md` (asset event affordances, §12.4.2) and `WotBindings.md`. The `GeneratesEvent` clause in the sample-limitation note was already stale, so I removed it. ### Runtime check `WotNativeOwnershipIndex` only uses `GeneratesEvent` in its alias table. Projected event routing takes the owner from `uav:componentOf` / `uav:id` and the notifier from Parent/`HasNotifier`. Nothing in `Opc.Ua.WotCon.Server` reads an instance-level `GeneratesEvent`. ### Tests (net10.0) - `Opc.Ua.Types.Tests`: 11446 passed, 1 skipped. New `WotInstanceEventSourceTests` covers object and variable roots, TMs, the TD with no events, the folded round trip, the legacy instance reference and the document set. Two bound-type cases were added to `WotDeclarationMergeBoundaryTests`. - `Opc.Ua.WotCon.Bindings.Tests`: 714 passed. - `Opc.Ua.WotCon.Tests`: 1496 passed, including the byte-exact sample regeneration tests. - net48 build of both test projects is clean, and `dotnet format whitespace` is clean. Not run: CTT `Base Info Core Structure 2 001.js` (p02-base-info) against a WotCon server. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> |
||
|
romanett
|
1da9fc0543 |
Redundancy.Server session mirroring: fix fast reconnect lifecycle (#4627) (#4704)
Fixes #4627 — the eight Redundancy.Server session mirroring defects (HotAndMirrored fast reconnect). ## Changes | ID | Fix | |----|-----| | RS-1 | The owning replica mirrors a liveness heartbeat (`LastContactAt`), written at most once per quarter of the session timeout while the client uses the session (`DistributedSessionOptions.LivenessInterval`, default 1 s). A restore expires the timeout after the later of the last activation and the last mirrored contact, plus the heartbeat's maximum lag. | | RS-2 | Entries record their owning replica (`OwnerId`). Only the owner mirrors and deletes the entry. A restore takes ownership with a conditional write. A stale copy on the previous replica is dropped without deleting the entry, both when its heartbeat sees the change and when the client fails back to it. `ISharedSessionStore` gains `TryReplaceAsync`, `TryRemoveAsync` and `EnumerateAsync`. Over a store without CAS (the CRDT gossip store) they fall back to compare-then-write. | | RS-3 | A restored session keeps the SessionId from `CreateSession`: `Session.RequestedId`, and `DiagnosticsNodeManager` honours a requested id when it is free. `DistributedSessionManager` mints GUID SessionIds that are unique across the replica set. Continuation points therefore stay under one owner id across repeated failovers. | | RS-4 | Sessions whose user token is encrypted with an ECC/RSA-DH EphemeralKey are flagged. Their restore is rejected up front (`EphemeralKeyUnavailable`), before anything is consumed or counted as a failed authentication. The documented option from the issue was chosen over mirroring ephemeral private keys. | | RS-5 | New `SessionManager.AdmitRestoredSessionAsync` hook: the mirrored nonce is consumed only after the restored activation has passed the signature, identity and continuity checks. If a restored session's first activation fails, the session is discarded without the virtual close, so the shared entry is untouched. Restored copies that are not admitted now also release their diagnostics node, and `CurrentSessionCount` is balanced for restored sessions. | | RS-6 | Expired entries are deleted on restore and by a periodic sweep (`SweepInterval`, default 5 min). The sweep also purges consumed-nonce markers that no live entry references and that are older than `NonceMarkerRetention` (default 1 h), via `ISingleUseNonceRegistry.PurgeAsync`. | | RS-7 | `SharedKeyValueSessionStore` and `DistributedSessionManagerFactory` now throw for an external store without an `IRecordProtector`, as the DI registration already does. Pass `NullRecordProtector.Instance` to opt out explicitly. | | RS-8 | Entries carry the original server certificate. A restored session accepts a client signature over either that certificate or its own one (`Session.OriginalServerCertificate`). | Entries are now written at `SecurityStateVersion` 5. Entries at versions 3 and 4 can still be restored, without an owner. ### API changes (Redundancy.Server) - New members on `ISharedSessionStore` and `ISingleUseNonceRegistry`. - An optional `TimeProvider` parameter on `SharedSingleUseNonceRegistry`. - New `DistributedSessionOptions` properties. - `SharedSessionEntry` v5 fields. - `SharedKeyValueSessionStore` and `DistributedSessionManagerFactory` throw for external stores without a protector. ## Tests - New unit tests for every finding: the restore/heartbeat/ownership/sweep/SessionId paths of `DistributedSessionManager`; store conditional writes, the CAS fallback and v5 round-trips; the registry purge; the factory/store guard; base `SessionRestoreTests` (admitted once, discarded on validation failure or rejected admission); and `SessionOriginalServerCertificateTests`. - New end-to-end test `SessionUsedPastItsTimeoutFailsOverWithItsSessionIdAsync`. The client uses the session for 2× its timeout, then fails over. The test checks that the SessionId is preserved and that the standby serves that SessionId and owns the entry. It also closes the stale copy and checks the entry survives. With the heartbeat disabled, the test fails as expected. - Local net10.0 runs: Redundancy.Server.Tests 883/883, Server.Tests 8347 passed / 5 skipped, Sessions.Tests 849 passed / 379 skipped (the usual baseline). The net48 build is clean. The Redundancy session tests pass on net48; the ECC-only cases are skipped there. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
d552634258 |
Enable ECC security policies on net48 via a BouncyCastle raw-ECDH polyfill (#4703)
Closes #4510. ## Summary OPC UA Part 6 feeds the **raw** ECDH shared secret into HKDF. .NET Framework only exposes `ECDiffieHellman.DeriveKeyMaterial`, which always hashes it, so the ECC SecureChannel and user-token policies were unavailable on `net48` (`SecurityPolicies.GetInfo` returned `null`). This adds a BouncyCastle polyfill for just that primitive: - **`Opc.Ua.Security.Certificates/Org.BouncyCastle/EcdhRawAgreement.cs`** (`net48` only): an internal `DeriveRawSecretAgreement(this ECDiffieHellman, ECDiffieHellmanPublicKey)` extension using `ECDHBasicAgreement`. It returns the X coordinate padded to the field size, exactly like the .NET 8 BCL method. Keys stay CNG keys; ECDSA signatures and HKDF stay on the platform — only the agreement step is managed code. The point is validated (on curve, not infinity) and mismatched curves are rejected. - Because the extension mirrors the .NET 8 signature, `Nonce.GenerateSecret` now has one code path on every target. - `SupportsRawEccSecretAgreement()` is removed; the CBC-based ECC policies (`ECC_nistP256`, `ECC_nistP384`, `ECC_brainpoolP256r1`, `ECC_brainpoolP384r1`) are offered on every target (still subject to OS curve support). - BouncyCastle was already a `net48` dependency of `Opc.Ua.Security.Certificates`; no new package. (Only `net48` is downlevel now — `netstandard2.1`/`net472` from the issue text are no longer targets.) **Not in scope:** the `_AesGcm` / `_ChaChaPoly` ECC variants (and PubSub DTLS) still need .NET 8 because .NET Framework has no AEAD ciphers — follow-up stacked on this branch. ## Docs `EccProfiles.md` (Known limitations), `migrate/2.0.x/certificates.md`, `ProductionChecklist.md`, `Profiles.md`, `WhatsNewIn2.0.md`. Calls out that on `net48` the agreement step does not run in a FIPS/validated module. ## Tests (local) | Suite | Result | |---|---| | `Opc.Ua.Core.Tests` full, net48 | 5956 passed, 0 failed | | `EccInteropTests` net48 vs 1.5.378 peer (net8.0, native raw ECDH) | 26 passed; 28 skipped (AEAD + curve25519/448) — all four CBC curves, both directions, UserName tokens, token renewal | | `Opc.Ua.Sessions.Tests` ECC filter, net48 | 52 passed, 0 failed | | Server `SessionCoverageTests`/`EphemeralNonce*`, Configuration builder tests, net48 | all pass | | Nonce/ECC/EncryptedSecret + session tests, net10.0 | all pass | New tests: symmetric agreement on all four curves; on net48 an oracle against CNG (`SHA256(raw) == DeriveKeyFromHash`); curve mismatch and off-curve rejection. Tests that asserted the old net48 limitation were updated. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
78e66f3473 |
Aggregates: TreatUncertainAsBad never applies to bounding values; DurationGood/Bad count Good/Bad regions only (#4702)
# Description Server-side aggregate fixes that follow three answers from the OPC Foundation on Mantis: - [11462](https://mantis.opcfoundation.org/view.php?id=11462) (~0025840): TreatUncertainAsBad does not apply to bounding values. - [11425](https://mantis.opcfoundation.org/view.php?id=11425) (~0025847): DurationGood counts only Good regions and DurationBad only Bad regions, and TreatUncertainAsBad does not apply. - [11426](https://mantis.opcfoundation.org/view.php?id=11426) (~0025852): Minimum, Maximum, the ActualTime variants and Range look at Uncertain values as if they are Good. The principle in 11425 and 11426 is that the aggregate definition wins over TreatUncertainAsBad. The CTT-side defects are filed as [11487](https://mantis.opcfoundation.org/view.php?id=11487). ### Bounding values (`AggregateCalculator`, `StartEndAggregateCalculator`) - Simple (§3.1.9) and Interpolated (§3.1.8) Bounding Values are formed from **non-Bad** raw values (`IsBoundCandidate`), whatever TreatUncertainAsBad is. - An Uncertain raw value before or after the timestamp makes the bound `UncertainDataSubNormal`. - A raw value exactly at the timestamp is the bound and keeps its own status. - Only a **Bad** value after a Simple bound reverts to stepped interpolation. - StartBound/EndBound keep an Uncertain bound instead of `Bad_NoData` (Tables 76/77). - WorstQuality2 now includes an Uncertain start bound, as §5.4.3.36 requires ("the server shall ignore the TreatUncertainAsBad for this aggregate"). ### How aggregates use the values (unchanged rule, two gaps closed) TreatUncertainAsBad still governs how an aggregate uses the values inside the interval, including an Uncertain bound. - `GetRegionsInValueSet` now also converts an Uncertain **first** region to Bad for Simple-bound aggregates. TimeAverage2/Total2 therefore omit it like any other Uncertain region (§5.4.3.7, Table 55 "Bound Uncertain"). - TimeAverage/Total (Interpolated bounds) still use an Uncertain bound and report Uncertain (Table 54). - TimeAverage2/Total2 return the PercentTime status `Bad` instead of `Bad_NoData` for an interval that has data but only omitted data (§5.4.3.2.1). - Minimum2/Maximum2 are unchanged: an Uncertain bound is a candidate only when TreatUncertainAsBad is false (§5.4.3.15). ### NumberOfTransitions (`CountAggregateCalculator`) §5.4.3.24 speaks only of "non-Bad" values, and Table 72 says "Bound Uncertain: Use as value". So: - Uncertain values are counted and can be the previous value, whatever TreatUncertainAsBad is. - The previous value is the nearest non-Bad value before the interval (the early bound). This reverses an earlier change that made NumberOfTransitions honour TreatUncertainAsBad. ### Minimum, Maximum, MinimumActualTime, MaximumActualTime, Range (`MinMaxAggregateCalculator`) Per the 11426 answer: - Every non-Bad value is a candidate, whatever TreatUncertainAsBad is. - An Uncertain extremum is returned with `UncertainDataSubNormal` plus Calculated. ActualTime keeps the timestamp of the first such value. - Otherwise the status is Good unless the interval contains Bad values (then `UncertainDataSubNormal`); it is `BadNoData` if all values are Bad. - Range is max − min over the non-Bad values. It is Uncertain if either extremum is Uncertain or Bad values exist. Before this change the server ignored an Uncertain value beyond the Good extremum and returned the Good one with an Uncertain status. Minimum2/Maximum2/Range2 are unchanged, because §5.4.3.15 ties them to TreatUncertainAsBad explicitly. ### DurationGood/DurationBad/PercentGood/PercentBad (`StatusAggregateCalculator`) - A region is Good only if its data point is Good, and Bad only if it is Bad. - Uncertain counts for neither (§5.4.3.31–34). - The first region still takes the status of the raw value at or before the start. ### Tests - New `Aggregates/BoundingValueTreatUncertainAsBadTests`: DeltaBounds, StartBound/EndBound, TimeAverage2/Total2, Interpolative, NumberOfTransitions (forward and reverse), DurationGood/Bad/PercentGood/Bad. - New Min/Max test for Bad-only and Bad-next-to-Good intervals. - Updated tests that encoded the old interpretation: - Interpolative with an Uncertain raw value at the start: 2 tests. - Duration/Percent Good/Bad: 2 tests. - NumberOfTransitions with TreatUncertainAsBad: 2 tests. - Minimum/Maximum/Range with an Uncertain extremum or only Uncertain values: 2 tests in Server.Tests, 1 in History.Tests. - `ProcessedReadUsesAdvertisedNodeAggregateDefaultsAsync`: it now probes the TreatUncertainAsBad default with Count, because PercentGood no longer depends on it. ### CTT verification CTT 1.05.513 with the patched scripts (c3674ff, which encode the 11462/11425/11426 answers), Aggregates part (p06): | Server | Errors | Warnings | Pass | NotImpl | | --- | --- | --- | --- | --- | | master |
||
|
romanett
|
9b034f8291 |
Enable nullable reference types centrally; migrate all unset and annotations-only projects (#4700)
# Description Nullable reference types are now enabled once, in `common.props`, and every project in the repository builds warning-clean with them. **Configuration** - `<Nullable>enable</Nullable>` lives in `common.props`. All per-project `<Nullable>` settings are removed: 178 project files (enable, annotations, and the `disable` in the three Aot test projects). - 419 per-file `#nullable enable`/`restore` directives and the 4 `#nullable disable` directives are removed. - Kept on purpose: - The 5 `<auto-generated>` schema files (`UANodeSet.cs`, `UA Type Dictionary.cs`, `OPCBinarySchema.cs`, `SecuredApplication.cs`, `PInvokeHelper.cs`) keep `#nullable enable`. The compiler ignores the project setting in generated code, so the directive is required there. - `tests/Opc.Ua.Interop.LegacyPeer` keeps `disable`. It builds against the 1.5.378 packages and does not import the repository props. - `docs/DeveloperGuide.md` and `.github/copilot-instructions.md` now describe the central setting. **Migration** (the 43 unset and 13 annotations-only projects, roughly 10k diagnostics) - **Production code: annotated at the declarations.** Covers `tools/Opc.Ua.SourceGeneration.Core`, `src/Opc.Ua.Lds.Server` and the LDS sample. - Members, parameters and returns are nullable where null actually flows. - Null-check helpers carry `[NotNullWhen]` (`IsNull`, `IsValidXmlName`, `IsMethodTypeDesign`). - `CoreUtils.Format`, `Utils.Format`, `ITemplateWriter.WriteLine` take `params object?[]`. `Template.AddReplacement` takes `string?` (rendering already skips null replacements). - `IRegisteredServerStore.RegisterAsync` returns `Task<RegistrationEntry?>`: it returns null when a registration is removed. - Dereferences backed by an invariant (for example, the validated model) use `!`, so runtime behaviour is unchanged. - **Behavioural changes in production code** (small, deliberate): - `NodeSetToModelDesign` throws `InvalidOperationException` when the file is not a NodeSet. Before, it threw a NullReferenceException on the next line. - `ModelDesignValidator` starts its lookup tables empty instead of null. A redundant `if (m_nodes == null) return;` guard is gone. - **Test code: minimal annotations.** - `?` on values that can be null, `!` where the test asserts non-null. - `= null!` for fixture members assigned in SetUp. - `TryGetStructure<T>(out T? x)` where the inferred `T?` broke the `IEncodeable` constraint. - A few redundant `== null` checks on structs (`NodeId`, `ArrayOf<T>`) folded into the adjacent `IsNull`/`Count` checks. **Notes for reviewers** - The diff is large (~1,500 files) but almost entirely mechanical: `?`, `!` and directive removal. The non-annotation changes are listed above. - Known follow-up, not done here: `IModelDesign.Namespaces/TargetNamespace/NamespaceUris` stay non-nullable even though the validator returns null before it loads a dictionary. Making them nullable would ripple through every generator call site. ## Related Issues - Fixes #4439 ## Checklist - [ ] 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. - [ ] 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. - [ ] 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. The "no new warnings" box covers net10.0 and net48: full non-incremental builds of `UA.slnx`, `tools/SourceGeneration.slnx`, `tools/MigrationAnalyzer.slnx` and the Aot test projects finished with 0 warnings and 0 errors under the repository's warnings-as-errors settings. net9.0 and net8.0 also had no nullable diagnostics, but they were not rebuilt after the final fixes. Tests have not been run locally yet. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
cd4dbbe66e |
WebApi: apply the REST auth opt-ins to the opcua+openapi WebSocket upgrade (#4699)
# Description Follow-up to #4674 for the WebSocket side of the OpenAPI binding. - **Plain `opcua+openapi` upgrade uses the REST auth.** When `AddWebApiBasicAuth` / `AddWebApiBearerAuth` / `AddWebApiMutualTlsAuth` is configured, the upgrade runs the same default authorization policy as the REST route group (`RequireAuthorization()`), because the upgrade is dispatched by the listener after routing. Requests that don't satisfy it get the schemes' challenge (401 + `WWW-Authenticate`) or 403. Without an auth opt-in the behaviour is unchanged. - **Upstream identity on the channel.** For both `opcua+openapi` and `opcua+openapi+<accesstoken>`, the authenticated principal is mapped through `ISessionlessIdentityProvider` into `SecureChannelContext.UpstreamIdentity`, as the REST routes already do per request. - **Token lifetime.** A credential whose `exp` claim has already passed is refused at upgrade, also within the JwtBearer clock skew, and the socket is closed when the token expires. - **Refused upgrades complete their response.** Before, the admission lease aborted the connection when an upgrade was refused, so clients could see a connection reset instead of the 401/503. Refused upgrades on HTTP/1.x now carry `Connection: close` and Kestrel closes the connection after sending the response. The other WebSocket sub-protocols (`opcua+uacp`, `opcua+uajson`) and the HTTPS binary/JSON endpoint are unchanged: per Part 6 §7.4 they authenticate through the OPC UA session layer, not HTTP credentials. ## Tests New cases in `RealHttpsListenerIntegrationTests.Authentication.cs` against a real `HttpsTransportListener`: - the plain upgrade for Basic, Bearer and mTLS with no, wrong, wrong-scheme and valid credentials (status, challenge, and that nothing reaches the server callback); - `UpstreamIdentity` set for both `opcua+openapi` variants; - an expired token, in the `Authorization` header and in the sub-protocol; - the socket closing when the token expires. Locally on net10.0: `Opc.Ua.Bindings.Https.WebApi.Tests` 499/499, `Opc.Ua.Core.Tests` (Wss/OpenApi/Https) 243/243, `Opc.Ua.Sessions.Tests` (Wss/Https/WebApi) 53/53. `Opc.Ua.Bindings.Https` builds on all TFMs with 0 warnings. ## Related Issues - Follow-up to #4674 ## Checklist - [x] 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. - [ ] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
b8f07843b2 |
Secure channel: never answer with an expired token; accept a just-expired one (+ labelled soak/stress CI) (#4698)
## Summary **Secure channel: a client could drop the channel when the server answered with a just-expired token.** Running the long-dormant ConnectionStability soak locally, the subscription channel shut down with `BadSecureChannelClosed`. The session then timed out and the subscription was abandoned. Root cause, against OPC 10000-4 §5.6.2.1: 1. **Server (rule 2: use the old token only "until the SecurityToken expires").** After a renew, the server keeps securing messages with the old token until the client sends something secured with the new one. That activation only happened on *receive*. A client that sends nothing, for example one whose Publish requests are all outstanding, kept getting responses secured with the old token after it expired. 2. **Client (rule 3: "accept Messages secured by an expired SecurityToken for up to 25 % of the token lifetime").** The client rejected any such response the moment the token expired, throwing `BadTcpSecureChannelUnknown`, and shut the channel down. From the failing run (token lifetime 10 s): the client renewed to token #11 at 09:59:01.686 and sent nothing on that channel afterwards. The server, still on #10, sent a Publish response at 09:59:04.389, 380 ms after #10 expired. The client rejected it and closed the channel. Nothing reconnected, the 10 s session timeout expired, and the server abandoned the subscription. Both halves date back to the 2016 initial commit; this is not a recent regression. It hits whenever the client sends nothing on the channel between renewing and the old token's expiry, so it is timing-dependent. ### Fix - `UaSCBinaryChannel.ActivateRenewedTokenIfDue()` activates a pending renewed token once the current one passes `TokenActivationPeriod` (95 %). This is the rule the receive path already applied. It is now shared, and `TcpServerChannel.SendResponse` calls it before securing a response. - `ReadSymmetricMessage` accepts responses (not requests) secured with an expired token for `TcpMessageLimits.TokenExpiryGracePeriod` (25 %) of its lifetime. ### Tests `TcpReconnectKeyAgreementTests` now runs the server channel and the peer on one shared `FakeTimeProvider`: - `ResponseSentAfterTheCurrentTokenExpiresUsesTheRenewedTokenAsync`: a response sent at 96 % of the old token's lifetime is secured with the renewed token. - `ResponseSecuredWithTheExpiredTokenIsAcceptedWithinTheGracePeriodAsync`: accepted at 120 % of the lifetime, rejected at 130 %. Both fail without the fix. All 40 tests in the fixture pass with it. ## CI: soak and stress runs on labelled PRs (and the soak that tested nothing) The bug stayed hidden because `stability-test.yml` still filtered `Opc.Ua.Client.Tests` after #3810 moved the test to `Opc.Ua.Sessions.Tests` (2026-05-28). Since then the nightly has run **zero tests** while reporting green: runs dropped from ~100 min to ~15 min. - `stability-test.yml` now targets `Opc.Ua.Sessions.Tests` and writes a TRX file. - New `.github/scripts/assert-tests-ran.ps1`, used by all three opt-in workflows, fails a run in which no test executed, or none passed. Contract-tested in `CiAssertTestsRanTests`. - Like `interop`, the opt-in workflows can now run on a labelled PR. They are not required checks. A later push cancels the run in progress, and adding an unrelated label neither starts nor cancels a run. | Label | Workflows | Run on a PR | | --- | --- | --- | | `stress` | Stress Tests (ChaosTCP) | Random seed, saved as `chaos-seed.txt` in the results artifact | | `soak` | Connection Stability Test, Sample HA Long-Haul Test | 30 min stability; 15 min per HA scenario | ## Test plan - [x] `TcpReconnectKeyAgreementTests`: 40/40 pass; the 2 new tests fail without the fix - [x] All `Opc.Ua.Core.Tests.Stack` tests (net10.0): 3548 passed, 0 failed - [x] `CiAssertTestsRanTests`: 4/4 - [x] `soak` + `stress` labelled run on this PR (before the fix): both stability tests, both HA long-haul scenarios and ChaosTCP passed on Linux (the label triggers and push-cancellation work) - [x] Local short-haul soak with the fix (Windows, 10 s tokens), 2 runs: both pass with full notification counts; run 2 hit the window once (`Token activated forced` on send) and kept the channel - [x] `soak` + `stress` labelled run with the fix: Connection Stability (2 tests), Sample HA Long-Haul and ChaosTCP all pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
e5b6d1352d |
Fix CI test flakes: WotCon end-to-end timeout and HTTPS listener port race (#4697)
Fixes two CI test failures. Both changes are test-only. ## WotCon: `UnavailableUpstreamEndpointFailsMappedReadAsync` timed out on Windows Debug This test failed the same way in two nightlies in a row (runs [37171493229](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/37171493229) and [36287479915](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/36287479915), `test-windows-net10.0-debug`, 4m06s). It threw `TaskCanceledException` instead of the expected `ServiceResultException`. The read did not hang. The cancellation came from the test's own 4-minute token, which is passed into `ReadAsync`. That budget covers the whole workflow: - Uploading and refreshing the 533 sample documents takes about 3m45s on a Windows Debug runner. - On Windows, each refused loopback connect to the unavailable upstream costs about 2s, because of SYN retries. The mapped read touches 9 SourceA nodes, and each one tries twice, which adds about 36s. Same commit on other legs: Windows Release passed at 2m35s, Linux Debug at 3m53s. This test and `BadTargetMappingFailsRefreshAsync` (3m52s on the same leg) now use the 8-minute budget the other full-workflow tests already have. The assertions are unchanged. ## HTTPS: `RealHttpsListenerIntegrationTests` failed with AddressInUse on macOS Run [37581622082](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/37581622082) failed in `test-macos-net10.0` with `Failed to bind to address https://[::]:49402: address already in use`. The failing tests were `AllBindingsForwardObservedPeerAddressAsync` (3 cases) and `AuthOptInIsEnforcedByRealHttpsListenerAsync`. `FindAvailableTcpPort()` probed `127.0.0.1:0`, closed the socket and handed the port to Kestrel, which then binds `[::]:port`. The port is in the ephemeral range, so another process could take it between the probe and the bind. Nothing retried. Changes: - The setup and the auth listener open the listener through a new `OpenListenerOnFreePortAsync` helper. On AddressInUse it disposes the listener and retries with a fresh listener and a new port, up to 5 attempts. The AddressInUse check uses the same matching as `ServerFixtureUtils.IsPortUnavailable`. - The port probe now uses `TcpListener.Create(0)`, a dual-mode IPv6 any socket, so it checks the same address Kestrel binds. ## Testing - `Opc.Ua.WotCon.Samples.Tests`, both affected tests: pass on net10.0 in Debug and Release. - `Opc.Ua.Bindings.Https.WebApi.Tests`: 483/483 pass on net10.0. The retry path was checked by forcing a taken port on the first probe. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
731600801a |
Release: stop retrying 404s in the feed gates; keep GitHub Release notes under the size limit (#4696)
# Description `Release to nuget.org` [run 37591540644](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/37591540644) was the stable 2.0.0 promotion. It spent more than an hour in **Verify already-published package identity**. It pushed every package to both feeds and created the `2.0.0` tag, then [failed](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/37591540644/job/112693955843) in **Tag published package candidate** before the GitHub Release existed. This PR fixes both problems. ## 1. Feed gates retried every 404 PowerShell 7's `Invoke-WebRequest -MaximumRetryCount` retries every status from 400 to 599, and that includes 404. `-SkipHttpErrorCheck` stops the error from being thrown but doesn't stop the retries. Both feed gates ask once per package whether an id/version is published, and 404 is the normal "not yet" answer. Each absent package therefore cost 3 retries × 5 s, about 15 s. A stable set is never pushed automatically, so for 2.0.0 every package, `.Debug` included, was absent on **both** feeds. - `package-version-policy.ps1`: adds `Invoke-FeedRequest`. It retries only network failures, 408, 429 and 5xx, and returns every other response straight away. After the last retry it returns the final response, or rethrows the network error. - `assert-published-packages-match.ps1` and `validate-preview-package-ordering.ps1` call it instead of `Invoke-WebRequest -MaximumRetryCount 3 -RetryIntervalSec 5`. The status handling in each caller is unchanged, so the gates still fail closed: anything other than 200 or 404 still blocks. - `PackageVersionPolicyTests.GetGitHubPackagesPublishedVersionPagesResultsAndFailsClosedAsync` now dot-sources the policy script so the extracted function can find `Invoke-FeedRequest`. The `Invoke-WebRequest` stub still intercepts the call. - New stub-based tests check that `Invoke-FeedRequest` makes one request for 200/400/403/404 and retries 408/429/5xx exactly `MaximumRetryCount` times. They also check that it stops once a retry is not transient, rethrows an exhausted network failure and passes `-OutFile` with `-PassThru`. | Response | Before | After | |---|---|---| | 404 | 4 requests, 15 s | 1 request, immediate (live nuget.org: 0.58 s) | | 403 | 4 requests | 1 request | | 503 / 429 | retried 3× | retried 3× (unchanged) | | connection refused | — | retried, then rethrown | ## 2. GitHub Release body too long ``` HTTP 422: Validation Failed (https://api.github.com/repos/OPCFoundation/UA-.NETStandard/releases) body is too long (maximum is 125000 characters) ``` `gh release create --generate-notes` diffs against the latest release when no previous tag is given. Here that was `1.5.378.182`, which is on another branch, so the generated notes for 2.0.0 came to 191,048 characters. The step now calls `POST /releases/generate-notes` with `previous_tag_name` from the new `Get-PreviousReleaseTag` helper in `package-version-policy.ps1`. It picks the greatest same-major semantic version strictly lower than the tag, ignoring non-semver tags like 1.5.378.x, so a servicing 2.0.1 published after 2.1.0 still diffs against 2.0.0. For 2.0.0 that is `2.0.0-preview.2`, and the notes come to 31,408 characters (measured against the real repo; that call has no side effects). If a body still exceeds 120,000 characters, it is cut at a line boundary and a compare link is appended. The tag and release checks are idempotent, and the pushes use `--skip-duplicate` and verify identity, so re-running the promotion finishes the release. **For run 37591540644 to pass:** the dispatch runs `release.yml` from `release/2.0`, so these changes have to reach `release/2.0` before the promotion is re-run. ## Verification - All 3 scripts parse with 0 errors. The modified workflow step also parses, and the YAML loads. - Live nuget.org: absent version → 404 in 0.58 s; package index → 200. httpbin 503 with `-RetryIntervalSec 1`: retried, 503 after 4.7 s. - `dotnet test tests/Opc.Ua.Tools.Tests`: 939 passed (1 skipped) on net10.0, plus net8/9/48. ## Related Issues - No issue; found investigating run 37591540644. ## Checklist - [ ] 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. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Klaus Landsdorf
|
fa55157ce4 |
Server: hold anonymous session-less requests to the channel security of CreateSession (#4689)
# Description `AddSessionlessInvocation(o => o.AllowAnonymous = true)` (#4659) lets a request without an Access Token run as an anonymous user. OPC 10000-4 §6.3.1 allows that only "if application authentication through the SecureChannel is sufficient". On HTTPS the channel gave no such authentication: on an endpoint with mutual TLS and a SignAndEncrypt policy, a session-less request without a TLS client certificate and without an Access Token was answered `Good`, over the REST binding and over the binary POST with `OPCUA-SecurityPolicy: Basic256Sha256`. `CreateSession` on the same endpoint answers `Bad_SecurityChecksFailed` without the certificate, and ActivateSession on a new channel does too (OPC 10000-6 §7.4.1: applications that use HTTPS and require application authentication check the application certificates during the CreateSession/ActivateSession handshake). This PR makes a session-less request without an Access Token meet what the endpoint demands of a Session. #4659 is merged, in no release tag and opt-in, so this is a change to a new feature; the default `AllowAnonymous = false` is not affected. ## Changes ### `SessionManager.ValidateSessionlessRequestAsync` (`src/Opc.Ua.Server/Session/SessionManager.cs`) With `AllowAnonymous`, a request without an Access Token now needs: | Check | Where it applies | Status otherwise | Same as | | --- | --- | --- | --- | | The client certificate of the channel (`SecureChannelContext.ClientChannelCertificate`) | endpoints with a security mode or policy other than None, and HTTPS endpoints when `HttpsMutualTls` is set | `Bad_SecurityChecksFailed` | CreateSession on the HTTPS listener; ActivateSession on a new channel | | An Anonymous user token policy on the endpoint (an endpoint without policies does not restrict) | every endpoint | `Bad_IdentityTokenRejected` | ActivateSession with an anonymous identity token | | An endpoint on the channel | every request | `Bad_SecurityModeInsufficient` | the same request with an Access Token | - The certificate is the one the transport has validated: the SecureChannel certificate of opc.tcp and opc.wss, the TLS client certificate of the HTTPS listener (REST, binary, JSON). So opc.tcp channels with security keep working as they did, and an HTTPS client that wants an anonymous session-less call presents its application certificate in the TLS handshake. - An HTTPS listener without `HttpsMutualTls` advertises SecurityMode None without an Anonymous user token policy (`HttpsServiceHost`); an anonymous session-less request there is now rejected as an anonymous Session is. - A request **with an Access Token** is unchanged: the token authenticates the caller and HTTPS keeps it confidential (HTTPS clients are not required to present a certificate, OPC 10000-6 §7.4.1). A `ValidateSessionLessRequest` handler still takes over completely. - Not changed: the transport credential authentication of the REST routes (#4674). `SessionlessInvocationOptions.AllowAnonymous` documents the rule. `docs/SessionlessInvocation.md` has a new section "Anonymous requests and the channel" with the table. ### Tests - `tests/Opc.Ua.Sessions.Tests/SessionlessChannelSecurityTests.cs` (new, net8.0+): a real reference server with an HTTPS listener, twice (mutual TLS on with a SignAndEncrypt-only endpoint; mutual TLS off), over REST and the binary POST, with and without a TLS client certificate: anonymous without a certificate, with a certificate, `AllowAnonymous = false`, a valid and a bogus Access Token, and CreateSession on the binary POST without the certificate for comparison. 44 cases. - `tests/Opc.Ua.Server.Tests/SessionlessInvocationTests.cs`: opc.tcp with and without a channel certificate (Sign, SignAndEncrypt, None), HTTPS with and without mutual TLS and with a secured endpoint, user token policies with and without Anonymous, no endpoint, an Access Token and a handler without a certificate. The existing tests that use `AllowAnonymous` set `HttpsMutualTls = false` or give the channel an endpoint. - Fail without the change: 7 of the 9 new `SessionlessInvocationTests` cases fail with the `SessionManager.cs` change removed (the opc.tcp with security, HTTPS with and without mutual TLS, user token policy and unknown endpoint tests); the 6 failing `SessionlessChannelSecurityTests` cases are below. ## Testing - **Reproduction on master (`c03c12838`)**: `SessionlessChannelSecurityTests` without the change: 6 of 44 cases fail, 38 pass. With mutual TLS on and no TLS client certificate the anonymous request is answered `Good` instead of `Bad_SecurityChecksFailed` (REST and binary). With mutual TLS off it is answered `Good` instead of `Bad_IdentityTokenRejected` (REST and binary, with and without a certificate). All 44 pass with the change; the Access Token cases pass both ways. - Results on macOS: `Opc.Ua.Server.Tests` full run on net10.0 8197 passed / 9 skipped / 0 failed; the session-less and SessionManager filters 139 passed / 0 failed on net10.0 and on net8.0. `Opc.Ua.Sessions.Tests` with the HTTPS/WebApi/Sessionless filters 125 passed / 0 failed on net10.0 and on net8.0, and with the Wss/OpenApi/ClientTest filters 320 passed / 241 skipped / 0 failed on net10.0. - Builds: `Opc.Ua.Server`, `Opc.Ua.Server.Tests` and `Opc.Ua.Sessions.Tests` for net48, net8.0, net9.0 and net10.0: 0 warnings, 0 errors. - `dotnet format` (whitespace, style, analyzers `--severity info`, one `--include` per changed file): no finding on a changed line (CA1849 at `SessionManager.cs` 235 is on master). - Coverage (`.azurepipelines/check-coverage.ps1` against master, session-less and SessionManager tests of `Opc.Ua.Server.Tests` on net10.0): patch 36/36 changed lines (100 %). The project floor is not meaningful for this subset. - CI and CodeQL on the PR branch: [CI run 37529750287](https://github.com/biancode/UA-.NETStandard/actions/runs/37529750287) green (158/158 jobs, incl. macOS and Windows net48), [CodeQL run 37529754883](https://github.com/biancode/UA-.NETStandard/actions/runs/37529754883) green, on `03a8e3d91`. ## Related Issues - Fixes #4688 - Relates to #4659 (session-less invocation), #4657 (the REST binding's session-less service set) and #4674 (REST authentication) ## Checklist - [x] 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. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the branch CI's Windows net48 legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
romanett
|
fbaabf5b28 |
Cut CI runner minutes: duration-packed test batches, CodeQL on Linux/daily, cancellable branch packing (#4694)
Reduces GitHub Actions runner minutes without removing any OS, target framework, test project or check. All figures come from master CI run 37520359929 (158 jobs, ~1,880 runner-minutes) and the last two weeks of run history. ## 1. Test batches packed by duration (`get-ci-matrix.ps1`) Previously the expander picked the **smallest** batch size that fit the 256-job limit, which meant 2 projects per job and 33 jobs per profile. Job logs show ~45% of the test matrix (674 of ~1,516 min) went to compiling. Each batch rebuilds the shared dependency graph: the first project's build took 3.8–4.9 min, the second only 0.4–0.9 min. - Projects are now packed in path order (which keeps `PubSub.*`, `Redundancy.*`, `WotCon.*` together) up to a 30-minute weight target, using `.github/ci-test-durations.json`. Each weight is the slowest PR profile's test minutes, rounded up, plus 1. - A batch never holds more projects than the job timeout can budget for: at most 7, given 20 + n×45 ≤ 350. - If the matrix would exceed the job limit, the target is raised instead. - **PR test matrix: 132 → 60 jobs** (15 per profile; macOS 33 → 15). Each batch is estimated at ≤36 min on the slowest runner, comparable to today's slowest jobs (37–43 min), so wall-clock should be about the same. Expected saving is roughly 300+ runner-min per run, plus less queueing for the scarce macOS runners. - The weekly full scope shrinks the same way (still 13 profiles). - The table only steers balance: a project missing from it gets `defaultMinutes`, so it is still scheduled. - `run-dotnet-tests.ps1` now records per-project `buildSeconds`/`testSeconds` (batch summary and job summary). The new `update-test-durations.ps1` refreshes the table from any run's `dotnet-results-*` artifacts. - `discover` output `batch_size` is renamed to `batch_target_minutes`. Nothing consumed it. New `CiMatrixScriptTests` cases: - each profile runs each of its projects exactly once; - only a lone project may exceed the target; - PR profiles average ≥3 projects per job; - the duration table names only existing projects. ## 2 + 3. CodeQL (`codeql-analysis.yml`) - **No longer runs on every push to master.** Instead, a daily schedule on master (was weekly) gives an analysis that always completes. Each analysis takes ~3 h, and with `cancel-in-progress: true` per branch, 49 of 76 push runs in two weeks were cancelled by the next merge before uploading anything. `master378`/`release/*`/`develop/*` keep their push trigger. PRs are still analyzed as before. - Only PR runs are cancelled when superseded. Push and scheduled runs always finish. - **Runs on `ubuntu-latest` instead of `windows-2025`.** Only net10.0 is built, so the extracted sources are identical. The traced Windows build alone took 70 min, against 12 min for the untraced Linux `UA.slnx` net10.0 leg. This PR touches a `.cs` file, so its own CodeQL run is the first measurement. ## 4. NuGet packing on feature branches (`nuget-publish.yml`) 298 of ~400 runs in two weeks were feature-branch pushes, each about 59 Windows-min across two signed builds. - A superseded feature-branch run is now cancelled. Protected lines and tags are never cancelled: they publish previews, and `release.yml` promotes a run by ID. Each manual (`workflow_dispatch`) run gets its own concurrency group, so a later push cannot cancel it. - A new `changes` job skips packing when a feature branch, as a whole, changes only documentation. It diffs against the merge base with master, not the previous push, so a docs-only push that cancels a code push cannot mask that code change. It uses the same `get-path-relevance.ps1` rule as CI. - Protected refs, tags, manual runs, and any branch whose merge base cannot be resolved always pack. Checked locally for docs-only branch (skip), code branch, protected ref, manual run and unknown commit (all pack). ## Verification - `CiMatrixScriptTests`, `CiPathRelevanceTests` and the new `CiTestDurationsScriptTests` (covering `update-test-durations.ps1`): all `*.Ci*` tests in `Opc.Ua.Tools.Tests` pass locally, 63/63. - This PR's own CI run shows the new matrix. Compare its total runner-minutes and longest test job against run 37520359929. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
f32259a1a7 |
GDS: pin SecurityAdmin-only delivery of ApplicationRegistrationChangedAuditEvents; document CTT defect C57 (#4693)
## Description CTT 1.05.513 (patched scripts) GDS Application Directory `011.js`, `018.js` and `028.js` fail on master ( |
||
|
romanett
|
7c2c6e7f8e |
Number commit-id-only prerelease previews 0 so they never outrank a release (#4692)
## Problem When a stable `version.json` (for example `2.0.0`) is built from a ref that isn't a public release branch, Nerdbank.GitVersioning makes the root version `2.0.0-g<commit>`. The "other prerelease label" case in `version.targets` turned that into `2.0.0-preview.g<commit>` for the preview-only package families. SemVer 2 ranks the alphanumeric `g…` identifier above every numeric one, so that version outranks every `2.0.0-preview.N` ever released, and NuGet versions are immutable. This happened on 2026-10-06 while master briefly carried `2.0.0`. Two such versions on GitHub Packages now block the stable 2.0.0 release on `release/2.0` (handled there in #4691). ## Change - `version.targets`: a new Case 2a maps a prerelease that is only a commit id to `<base>-preview.0.g<commit>`, keeping any build metadata last. Preview number 0 sorts below any committed `PreviewPackageBuildNumber`. - `package-version-policy.ps1`: `ConvertTo-PreviewPackageVersion` applies the same rule, so the package-set validator agrees with the build. - Tests: commit id only, commit id plus metadata, and `-gamma.1`, which is not a commit id. Checked with MSBuild against the real `version.targets`: `2.0.0-ge78c648295` maps to `2.0.0-preview.0.ge78c648295`. `2.0.0`, `2.0.0+g…`, `-rc.1` and `2.0.1-preview.N.g…` are unchanged. All 90 `PackageVersionPolicyTests` pass on net10.0. Same commit as the prevention part of #4691 (release/2.0). This PR doesn't include the release-only `PreviewPackageOrderingExemptions` change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
1f44b75b1b |
Update code-style agent guidance for CodeAnalysisTreatWarningsAsErrors=true (#4690)
## Proposed changes #4680 turned on `CodeAnalysisTreatWarningsAsErrors` in `common.props`, but `.github/agents/opc-ua-codestyle-enforcer.agent.md` still said it was `false` and told agents not to turn it on. That means analyzer-cleanup runs could undo the setting. - Repository context (line 16): the value is now listed as `true`, with a note that CA warnings fail the build. - Rule-promotion checklist (line 312): agents are now told to keep the setting enabled. Rules that are meant to stay unenforced belong at `suggestion`/`silent`/`none` in `.editorconfig`, and agents must clear all hits before raising a rule to `warning`. ## Related Issues - Fixes #4685 ## Types of changes - [x] Documentation Update (if none of the other choices apply) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
fa06c66dbb |
Stop versioning DI and ISA95 packages as preview (#4686)
## Summary Removes the **DI** (`Opc.Ua.Di*`) and **ISA95** (`Opc.Ua.ISA95*`) package families from the preview-only set, so they ship with the stable root version (e.g. `2.0.1` instead of `2.0.1-preview.N`). - `version.targets`: drop the two `_IsPreviewPackage` conditions. - `.azurepipelines/package-version-policy.ps1`: drop `ISA95|Di` from `Test-PreviewPackageId`. It must match `version.targets`, because the package-set validator fails if the two disagree. - `PackageVersionPolicyTests`: ISA95/Di IDs are now expected to be non-preview, they are removed from the classifier-agreement token list, and the "already-preview base version" case now uses Robotics. - Docs (`DeveloperGuide.md`, `ReleaseProcess.md`, `docs/README.md`): the preview-family list no longer includes DI or ISA95, and the "preview package" label is removed from the DI, Software Update (part of the DI packages), and ISA-95 entries. ## Test `dotnet test tests/Opc.Ua.Tools.Tests --filter PackageVersionPolicy` (net10.0): 87/87 passed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
5572107ab8 |
Push Model compliance: UpdateCertificate issuer check, TrustList audit events, NoConfiguration state, DefaultHttpsGroup (#4684)
# Description
Fixes found by a manual run of the OPC 10000-12 *Push Model for Global
Certificate and TrustList Management* test plan (183 test cases,
TC0–TC182) against the Reference Server. After these changes no test
case fails; the remaining non-pass verdicts are test-plan expectations
that contradict Part 12/Part 4/Part 20 (documented in the run report).
### Server behaviour changes
- **UpdateCertificate requires the issuer in the TrustList**
(`ConfigurationNodeManager.PushValidation.cs`). The validator classifies
an incomplete chain as suppressible; OPC 10000-4 Table 100 does not ("An
error during the chain creation may not be suppressed") and §7.10.5
requires the issuers to already be in the group's TrustList. A
certificate of an unknown CA is now rejected with
`Bad_CertificateChainIncomplete`. Expiry / host-name errors stay ignored
as §7.10.5 requires.
- **TrustList audit events reach the Server Object.**
`TrustListUpdateRequested`/`TrustListUpdated` were reported on the
TrustList node (`EventNotifier=0`, not linked to the Server), so no
client could receive them. `TrustList.SetAuditEventServer` routes them
through `IAuditEventServer`; wired for the Push TrustLists and the GDS
CertificateGroup TrustLists. New `AuditEvents` overloads keep the
node-based ones.
- **Duplicate AddCertificate** no longer makes the following
ApplyChanges fail with a bare `Bad` (store refused the second copy); the
commit is idempotent and its rollback keeps the pre-existing
certificate.
- **CloseAndUpdate validates CRLs** (incl. CRL-only updates): every CRL
must be signed by a CA of the new TrustList (§7.8.2.5). Previously an
unknown/forged CRL passed CloseAndUpdate and only failed ApplyChanges.
- **X.509 v1 certificates are rejected** by
CloseAndUpdate/AddCertificate/UpdateCertificate (OPC 10000-6 §6.2.2; new
`X509Utils.IsX509Version3`).
- **Application setup state (Annex G.2):** new
`StandardServer.StartupServerState` (virtual); the DI host returns
`NoConfiguration` when `ServerConfigurationOptions.InApplicationSetup ==
true`. `IsRunning` and `NodeManagerLifecycle` treat `NoConfiguration` as
operational. The Reference Server `--provision` mode starts in
`NoConfiguration` and exposes `InApplicationSetup=TRUE`.
- **DefaultHttpsGroup:** the group is only created when Https stores are
configured — the Reference Server config had none, so its
`opc.https`/`opc.wss` endpoints used the RSA application certificate
with an empty TLS trust list and the Https certificate could not be
pushed. The sample config now declares
`HttpsIssuerCertificates`/`TrustedHttpsCertificates` and an
`HttpsCertificateType` certificate (new `CertificateTypeString` value
`Https`). An Https certificate is assigned to DefaultHttpsGroup only (no
longer also listed in DefaultApplicationGroup). Fixed
`AddSecurityConfigurationHttpsStore`, which built the trusted Https
store path from the store *type*.
- Minor: AddCertificate logs a warning when it adds a CA without a CRL
while `RejectUnknownRevocationStatus` is enabled; UpdateCertificate
refused for an insufficient SecureChannel raises
`CertificateUpdateRequestedAuditEvent` with Status=false.
### Kept on purpose
- PEM certificates are accepted (converted to DER).
- `Bad_NotWritable` when opening an already open TrustList (OPC 10000-20
§4.2.2) and for AddCertificate while a read handle is open (§7.8.2.6).
- No validation rules for DefaultUserTokenGroup / DefaultHttpsGroup
TrustLists (Part 12 defines them only for ApplicationCertificateType
groups).
### Notes for reviewers
- Existing tests in `ConfigurationNodeManagerPushTests` and
`PushCertificateRequestRegressionTests` signed pushed certificates with
CAs that were not in the TrustList; they now trust their CA first
(behaviour change above).
- Adding the Https certificate to
`Quickstarts.ReferenceServer.Config.xml` gives the Reference Server's
opc.https/opc.wss endpoints their own TLS certificate.
### Tests
-
`tests/Opc.Ua.Server.Tests/Configuration/PushComplianceRegressionTests.cs`
— duplicate add/rollback, PEM, v1/v3, CRL issuer/forged/CRL-only, audit
routing.
- `tests/Opc.Ua.Server.Tests/ApplicationSetupServerStateTests.cs` —
NoConfiguration startup state.
- `tests/Opc.Ua.Gds.Tests/PushComplianceTests.cs` — end-to-end over a
secure channel: TrustList transaction lifecycle, open-handle rules,
masked reads, duplicate/PEM add, unknown CRL issuer, unknown certificate
issuer, cross-Session CSR key, nonce rules, RemoveCertificate chain/CRL
handling. Mutation-checked: reverting the issuer, duplicate-add and CRL
fixes makes the corresponding tests fail.
- Additional cases in `PushCertificateRequestRegressionTests`,
`CertificateIdentifierTests`, `ApplicationConfigurationBuilderTests`.
Local runs (net10.0): Opc.Ua.Gds.Tests 1257 passed / 0 failed / 56
skipped; Opc.Ua.Server.Tests push/TrustList/audit/lifecycle subset 1209
passed; Core and Configuration test classes passed. net48 build of the
affected test projects succeeds; `dotnet format whitespace` clean.
## Related Issues
- None (findings from a manual Push Model compliance run).
## Checklist
- [x] 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.
- [ ] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] 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.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
Copilot
|
c03c128384 |
Treat code analysis (CA) warnings as errors and fix all surfaced analyzer warnings (#4680)
# Description `TreatWarningsAsErrors` was already enabled, but `CodeAnalysisTreatWarningsAsErrors` was `false`, so Microsoft Code Analysis (`CAxxxx`) warnings were non-fatal. This PR sets it to `true` in `common.props` and fixes all 55 analyzer warnings the net10.0 build reported. **Production code** - `SamplingGroup.cs`: CA2213 is a false positive here because the detached `CancellationTokenSource` is disposed by `CancelSamplingLoop` once the loop completes. It is suppressed with a pragma, a justification and a TODO, following the existing pattern in `ServerBase.RequestQueue.cs`. **Tests** - CA1861: constant array arguments are moved to `static readonly` fields or replaced with `ArrayOf<T>` collection expressions. In the legacy interop server I used locals instead, because those arrays become node values that writes can change in place. - CA2025: tasks that finish before disposal (`Assert.ThrowsAsync`, or awaited before `using` scope exit) are suppressed with justification comments, following existing test patterns. - CA1812: classes created by the DI container are suppressed with a comment and a TODO. - CA2208: benchmark switch defaults now throw `InvalidOperationException`. - CA1845: `Substring(0, 8)` is replaced with `[..8]`. - CA1307: `string.Replace` is replaced with `Split`/`Join`, which also works on net48. - CA5394: `Random` is replaced with `RandomNumberGenerator`. **Docs** - `docs/DeveloperGuide.md` now says CA warnings also fail the build, and that any suppression needs a comment explaining why plus a TODO. **Validation** - I fixed the 55 warnings from a net10.0 build of `UA.slnx` (`CustomTestTarget=net10.0`). That build ran before the setting was turned on, and I have not rebuilt net10.0 since the fixes. - `CustomTestTarget=net48` build of `UA.slnx`: 0 warnings, 0 errors. - I also started a full build across all target frameworks with the setting on. Its log showed no errors or warnings while it ran, but I couldn't read its final result. - No tests have been run yet; CI should confirm the multi-TFM build and the tests. ## Related Issues - N/A ## Checklist - [ ] 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. - [ ] 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. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] 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-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> |
||
|
romanett
|
21e04bdba5 |
Make Bad_CertificateChainIncomplete unsuppressible; add UpdateCertificate renewal regression tests (#4683)
# Description Follow-up to the master378 fix in #4678 (CTT: application certificate renewal must use the configured trust stores). Master already ignores the supplied `issuerCertificates` for the default application group (OPC 10000-12 §7.10.5), but it had no regression tests for CA-signed renewals. While adding them I found a real gap: **The validator treated `Bad_CertificateChainIncomplete` as suppressible.** OPC 10000-4 Table 100 (Build Certificate Chain) says: "An error during the chain creation may not be suppressed." §6.1.3 adds: "Processing fails with Bad_SecurityChecksFailed if an element in the chain cannot be found." UpdateCertificate for the application group accepts every *suppressible* error (as §7.10.5 requires). As a result it accepted a renewal signed by a CA that is **not** in the TrustList and was only passed in the ignored issuer list, which is exactly what §7.10.5 rules out. ## Changes - `CertificateValidationCore`: remove `BadCertificateChainIncomplete` from the suppressible status codes. An accept callback (`AcceptError`) is no longer asked about it. - `TrustList`: remove the local workaround that filtered out this error; plain "accept suppressible errors" is now enough. - Docs (`Certificates.md`, `CertificateManager.md`): drop it from the suppressible list, and fix a sample that suppressed it. ## Tests - `ConfigurationNodeManagerPushTests`: - A CA-signed renewal with the CA installed succeeds with a missing, empty, unrelated or malformed issuer list, and the trust stores stay unchanged. - A renewal whose CA is only supplied as an issuer is rejected with `BadCertificateChainIncomplete`; stores and the current certificate are untouched. - End to end on an isolated server: a CA-signed renewal with no issuer list is committed through ApplyChanges. - `ConfigurationNodeManagerPushValidationTests`: the group TrustList validator accepts a CA from the trusted or issuer store and rejects an uninstalled CA. - `CertificateValidationCoreTests` / `CertificateValidatorTest`: - An incomplete chain is unsuppressible and never reaches the callback. - A leaf trusted directly **stays valid when the peer supplies its CA** (§6.1.3: chain certificates "may be provided with the application Certificate"). - One existing test attached its approver only after validating, so it never exercised suppression; it now validates with the approver attached. - Tests that only passed because of the bug (`PushCertificateRequestRegressionTests`, the regenerated-key helper and the rollback test) now install the signing CA in the TrustList first. The two new rejection tests fail on master and pass with this change. ## Reviewer notes / behavior change This was suppressible in 1.5.x as well. Application code whose accept handler approved a CA-signed peer certificate without its CA (CA neither installed nor sent by the peer) will now get a rejection. Without a handler such certificates were already rejected, and the stack's own handlers (`ApplicationInstance`, push validation) never accepted this error. Worth a release note. Local runs (net10.0), all passing: | Suite | Passed | Failed | |---|---|---| | Server.Tests | 8310 | 0 | | Core.Tests (security/transport filter) | 2039 | 0 | | Security.Certificates.Tests | 358 | 0 | | Core.Security.Tests | 558 | 0 | | Client.Tests | 2761 | 0 | | Gds.Tests (PushTest and TrustList tests) | 345 | 0 | The net48 builds of Server.Tests and Core.Tests also succeed. ## Related Issues - Related to #4678 (master378 counterpart) ## Checklist - [x] 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. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
dcd06791ac |
Advance master to 2.0.1-preview (#4682)
# Description #4677 was meant for `release/2.0` but was merged into `master`. Since then, `master`'s `version.json` has been the stable `2.0.0`. This PR moves `master` to `2.0.1-preview.{height}`. Its builds then sort above the `2.0.0` release and stay on the preview channel. - **`version.json`**: `"version"` `2.0.0` → `2.0.1-preview.{height}`. `versionHeightOffset` stays `0`. - **`preview-version.props`**: `PreviewPackageBuildNumber` stays `200`, matching `release/2.0`. It only applies when the root version is stable, so it has no effect on `master`. The stable change itself goes to `release/2.0` in a separate PR. ## Related Issues Fixes the wrong target of #4677. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Copilot
|
e78c648295 |
Images CI: publish samples once natively and stop compiling under QEMU (#4679)
# Description
Images CI took 45–100 min per image. The arm64 half of every
`linux/amd64,linux/arm64` build ran `dotnet restore` and `dotnet
publish` in an arm64 SDK container under QEMU. For `refserver`, publish
took 8 min natively for amd64 and 92 min emulated for arm64. On top of
that, every one of the 10 matrix legs recompiled the whole stack.
The output is framework-dependent portable IL, so the compile is now
done once, natively. The image legs only copy files onto the
per-platform runtime base image.
### Dockerfiles (all 10)
- **SDK stage runs on the build platform.** The `build` stage is pinned
to `--platform=$BUILDPLATFORM`. `-p:UseAppHost=false` keeps an x64 app
host out of the arm64 image.
- **New `prebuilt` stage.** It is chosen with a global `ARG
PUBLISH_SOURCE`, which defaults to `build`. Local `docker build` and the
`docker-compose` files are unchanged.
```dockerfile
FROM scratch AS prebuilt
ARG PUBLISH_DIR=artifacts/docker/publish
COPY ${PUBLISH_DIR}/ /app/publish/
FROM ${PUBLISH_SOURCE} AS publish
```
- **Pump: final-stage `RUN`s moved to a native stage.** The `sed` on
`openssl.cnf` and the `mkdir`/`chown`/`chmod` steps now run in a
`$BUILDPLATFORM` `layout` stage. The final stage copies `/layout/` to
`/`, so `/app` and `/diag` keep the `$APP_UID:0` owner and `g=u` mode
they had before. `openssl.cnf` therefore comes from the build-platform
base image. It is a plain config file, so it is expected to match
between architectures.
### `docker-image.yml`
- **Single image list.** The image list lives once, in `env.IMAGES`
(image, project, Dockerfile). `discover` emits it as the matrix.
- **New `publish-samples` job.** It runs `dotnet publish` for each
sample sequentially, so shared libraries compile once. It uses the same
options and NBGV version properties as the Dockerfiles and uploads the
output as the `docker-publish` artifact.
- **`build-and-push-image` legs.** Each leg:
- downloads the artifact;
- builds with `PUBLISH_SOURCE=prebuilt` and
`PUBLISH_DIR=artifacts/docker/publish/<image>`;
- no longer uses `setup-qemu-action`;
- is named `build-and-push-image (<image>)`.
- **`images summary`.** It now also fails when `publish-samples` fails.
### Docs
- `ContainerReferenceServer.md` describes the publish-once / copy-only
flow and the build arguments.
- `DeveloperGuide.md` mentions the `publish-samples` job in the Images
CI section.
Expected: one native publish of about 7 min, then 10 parallel
copy-and-push legs of a few minutes each. Not yet confirmed on CI: the
sandbox could not pull MCR base images, so the prebuilt multi-platform
builds were checked against a Debian-based stand-in base, and the
build-from-source path was not built.
## Related Issues
## Checklist
- [ ] 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.
- [ ] 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.
- [ ] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] 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-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
|
||
|
Copilot
|
fd360271d9 |
Set release/2.0 to stable 2.0.0 (#4677)
# Description
**Target: `release/2.0`, squash merge.** This is step 4 of [First stable
release](docs/ReleaseProcess.md#first-stable-release). It removes the
preview suffix so the next `release/2.0` build produces the stable
`2.0.0` package set.
- **`version.json`**
- `"version"`: `2.0.0-preview.{height}` → `2.0.0`.
- `versionHeightOffset`: `-32` → `0`. NBGV still adds the offset to git
height for the file-version revision when `{height}` is absent. With
`-32`, the revision is negative and `nbgv get-version` throws in
`System.Version..ctor`.
- **`preview-version.props`**
- `PreviewPackageBuildNumber`: `100` → `200`. This number sets the
`-preview.N` suffix of the package families that stay in preview.
- `master` already pushes those packages to GitHub Packages as
`2.0.0-preview.88.g…`, and the number keeps rising. nuget.org's highest
is `2.0.0-preview.6`.
- With 100, the release would soon sort below master's builds.
`validate-preview-package-ordering.ps1` still checks the ordering when
the candidate is built.
With `--public-release=true`, nbgv 3.10.94 reports `NuGetPackageVersion`
`2.0.0` and `AssemblyVersion` `2.0.0.0`.
The branch started from `master`, so it carries a merge of the
`release/2.0` tip. The diff against `release/2.0` is only the two files
above.
**After merge:** run [Candidate and dry
run](docs/ReleaseProcess.md#candidate-and-dry-run); the manifest should
show `basePackageVersion: 2.0.0` and `channel: stable`. Then run
[Approved promotion](docs/ReleaseProcess.md#approved-promotion) with
`release.yml` and `channel=stable`. Advance `master` to
`2.1.0-preview.{height}` if not done yet. Then reopen `release/2.0` as
`2.0.1-preview.{height}`.
## Related Issues
None. This is a version-only release change.
## 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.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] I have added all necessary documentation.
- [ ] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] 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: romanett <romanett98@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Klaus Landsdorf <kl@iniationware.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
|
||
|
Klaus Landsdorf
|
cc7908eaa1 |
WebApi: serve a generated OpenAPI document of the REST binding (opt-in) and map the session-less service set (#4657)
Reworked after the review: the OpenAPI document is generated from the code, not embedded; the route is opt-in; `StatusCode` is not accepted. The branch is a new history on top of master (the first version conflicted); master with #4674 is merged in. # Description The REST binding implements the routes of the OPC Foundation OpenAPI mapping (OPC 10000-6 §G.3), but it does not describe them: `GET /openapi.json` is answered with HTTP 405, and a client generator has nothing to read from the server. `docs/WebApi.md` listed a generated OpenAPI document as a deferred follow-up. The coded API is the source of truth, so this PR **generates** the document from the route table and from the structure definitions of the request and response types, and compares it with the document the model compiler publishes in tests. Nothing from `UA-Nodeset/OpenApi` ships in the package. ## Changes ### Service sets (`Opc.Ua.Core`) - `WebApiServiceSet` (`AllServices`, `Sessionless`) and `WebApiServiceRoutes.GetRoutes(serviceSet)`. `Sessionless` is the eight services of `opc.ua.openapi.sessionless.json` (OPC 10000-4 §6.3): Read, Write, HistoryRead, HistoryUpdate, Call, Browse, BrowseNext, TranslateBrowsePathsToNodeIds. ### Generator (`Opc.Ua.Core.Schema`, `Opc.Ua.Schema.OpenApi`) - `WebApiOpenApiGenerator.Generate(serviceSet, includeSchemas, serverUrl)` returns the OpenAPI 3.0 document as a `JsonObject`. It is built at runtime from `WebApiServiceRoutes` and the `StructureDefinition`/`EnumDefinition` of the message types (the same definitions the schema generation uses, resolved through `IDataTypeDefinitionResolver`). No reflection, so it is NativeAOT compatible; the AOT test project covers it. - **Two levels of detail**, because full schemas make big documents: | | Content | All 28 services | | --- | --- | --- | | default | paths, `operationId`s, every body a JSON object | about 12 KB | | `includeSchemas: true` | one component schema per message, structure and enumeration (100), for typed client generators | about 46 KB | The two published files are about 300 KB together. - The schemas describe the Compact JSON encoding, with the property names, types, formats and integer ranges of the published documents. DataValue is the one structure that differs: its status property is `Status`, as in Part 6 Table 42. - `OpenApiBuiltInSchemas` holds the schemas of the built-in types (StatusCode, LocalizedText, Variant, DataValue, DiagnosticInfo, ExtensionObject and the scalar types) as the encoder writes them. Abstract `Structure` fields are ExtensionObjects, abstract number fields are Variants. ### Serving the document (`Opc.Ua.Bindings.Https`) - `WebApiTransportOptions`: `ServiceSet`, `OpenApiDocumentPath` (default `null`: **off**) and `OpenApiIncludeSchemas` (default `false`). - `MapWebApiEndpoints(WebApiTransportOptions)` maps the routes of the service set and, when a path is set, the `GET` route of the document **in the same route group**. The route has no `AllowAnonymous` metadata. When an authentication scheme is registered, the contributor's `group.RequireAuthorization()` covers the document like `POST /read`; only `/findservers` and `/getendpoints` stay anonymous. The existing `MapWebApiEndpoints()` maps all routes as before. - The document is generated once per path base (listed as a relative `servers` URL; without a path base the list is omitted) by the `WebApiOpenApiGenerator` that `AddWebApiTransport` registers, or by a default one. - `Opc.Ua.Bindings.Https` references `Opc.Ua.Core.Schema` on net8.0 and later. ### Documentation - `docs/OpenApi.md` (new, linked from `docs/README.md`): serving the document, the two levels, generating clients, the JSON generated clients send, known issues, how the document is kept correct, troubleshooting. - `docs/WebApi.md`: new section "Service sets and the OpenAPI document"; the deferred follow-up is removed. `docs/SchemaGeneration.md` and both `NugetREADME.md` files describe the generator and the options. ## Comparison with the model compiler documents The two published documents are test resources of `Opc.Ua.Core.Schema.Tests` only (README next to them: source commit and update procedure). `WebApiOpenApiConformanceTests` compares the generated document with them for both service sets: - paths, `operationId`s, and the request and response schema of every operation; - the set of component schemas the operations reach: equal (100 for all services); - every enumeration (members and values); - for every structure: property names, types, formats, integer ranges, and inheritance. Everything both describe is equal. The test lists the differences and fails when a listed one disappears or a new one appears: | Difference | Generated | Published | Reason | | --- | --- | --- | --- | | DataValue status property | `Status` | `StatusCode` | Part 6 Table 42; UA-Nodeset#146 is open. The stack only knows `Status` (encoder and decoder). | | `description`, `default` keywords | not written | spec links, .NET defaults | documentation and defaults, not contract; Compact omits default values | `WebApiOpenApiWireFormatTests` validates what the binding really writes (the default request and response of all 28 routes, plus populated requests and responses with variants, status codes, references, identity tokens and notifications) against the generated component schemas. ## Tests - `Opc.Ua.Core.Schema.Tests` `OpenApi/`: `WebApiOpenApiGeneratorTests` (shape of the document, both levels, both service sets, servers list, sorting and determinism, concurrency, argument and resolver errors, every built-in type, arrays and ranks, enumerations, option sets, unions, base types, self-references, unknown definitions), `WebApiOpenApiConformanceTests`, `WebApiOpenApiWireFormatTests`, `OpenApiBuiltInSchemasTests`: 81 tests. - `Opc.Ua.Bindings.Https.WebApi.Tests`: `WebApiOpenApiDocumentEndpointTests` (TestServer with the startup contributor: off by default, configured path, both levels, service set maps and describes only the sessionless routes, path base, repeated requests, registered generator is used, **a request without credentials gets 401 when an authentication scheme is registered while one with credentials gets 200, the route carries `IAuthorizeData` and no `IAllowAnonymous`**, argument checks), options defaults and DI registration of the generator. - `Opc.Ua.Core.Tests`: `GetRoutes` for both sets and an undefined value. - `Opc.Ua.Aot.Tests`: the generator and a served document under the AOT fixture. - Results (macOS arm64): `Opc.Ua.Core.Schema.Tests` 260/260, `Opc.Ua.Bindings.Https.WebApi.Tests` 457/457, `Opc.Ua.Core.Tests` WebApi tests 110/110, each on net10.0 and net8.0 (Release). `Opc.Ua.Aot.Tests` (JIT run) SchemaAotTests 5/5, WebApiAotTests 8/8. - `Opc.Ua.Core`, `Opc.Ua.Core.Schema` and `Opc.Ua.Bindings.Https` and the three test projects build for net48, net8.0, net9.0 and net10.0 (Release) with 0 warnings and 0 errors (net48 builds only; the Windows legs of the fork CI run its tests). `dotnet format` whitespace, style and analyzers (`--severity info`) are clean on every changed file; the two `RCS1090` hits in `SchemaAotTests.cs` are on lines that master already has. - Patch coverage with `.azurepipelines/check-coverage.ps1` against master (`Opc.Ua.Core.Schema.Tests`, `Opc.Ua.Bindings.Https.WebApi.Tests` and the WebApi tests of `Opc.Ua.Core.Tests`, net10.0): 362/362 changed lines covered. - Fork CI (full matrix incl. macOS) and CodeQL on `f990af5db` (master with #4674 merged in): [CI run 37310622902](https://github.com/biancode/UA-.NETStandard/actions/runs/37310622902) green, [CodeQL run 37310626786](https://github.com/biancode/UA-.NETStandard/actions/runs/37310626786) green. The run on the earlier head `abda0e491` had one failure outside this PR: `McpServerProcessTests.HttpTransportAliasesExposeOnlyMcpRouteAsync("http")` timed out after 30 s on Windows net10.0; this PR does not touch `tools/`. ## Not in this PR - Authentication: the fix discussed in the review is @romanett's #4674. The document route uses whatever the REST routes use; this PR does not change authentication. - `StatusCode` tolerance in the decoder: dropped, the spec text is the reference. The first commit of the previous version is gone. - Samples. - Session-less invocation on the server side (OPC 10000-4 §6.3) is a separate draft PR. With `ServiceSet = Sessionless` this PR only limits the mapped routes and the document. ## Questions 1. `Opc.Ua.Bindings.Https` now references `Opc.Ua.Core.Schema` (net8.0 and later) because the generator lives next to the other schema generators and is trimmed away when unused. Fine, or should it move into `Opc.Ua.Core`? 2. Should `OpenApiIncludeSchemas` default to `false` as here, or should a host get typed clients out of the box? ## Related Issues - Fixes #4656 ## Checklist - [x] 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. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. |
||
|
Klaus Landsdorf
|
b0d8ca0d3a |
Server: session-less service invocation (OPC 10000-4 §6.3) with AddSessionlessInvocation (#4659)
Marc answered the five design questions (2026-10-05). This update
implements the answers; see "Design decisions" at the end.
# Description
OPC 10000-4 §6.3 lets a client invoke a subset of the services without
CreateSession/ActivateSession. The request then carries either an Access
Token in `RequestHeader.authenticationToken` or no token at all. The
server today only offers the low-level
`ISessionManager.ValidateSessionLessRequest` event. Without a handler,
every request whose token names no session is answered with
`Bad_SessionIdInvalid`, including a request without a token. That is the
code a client of an expired session expects; for a session-less request,
§6.3.1 has a server that does not support the invocation answer
`Bad_ServiceUnsupported`.
This PR adds built-in session-less invocation behind an opt-in and keeps
the existing event as the way to take over completely.
## Changes
### `AddSessionlessInvocation()`
(`src/Opc.Ua.Server/Hosting/OpcUaServerBuilderExtensions.cs`)
```csharp
services.AddOpcUa()
.AddServer(options => { /* … */ })
.AddSessionlessInvocation(o => o.AllowAnonymous = false) // the default
.AddJwtIssuer(/* … */); // validates the Access Tokens
```
- Registers `SessionlessInvocationOptions`.
`DependencyInjectionStandardServer` applies them to the `SessionManager`
in use, including one created by a registered session manager factory,
unless that manager already has options.
- Without dependency injection: `SessionManager.SessionlessInvocation =
new SessionlessInvocationOptions()`.
- Without either, nothing changes apart from the
`Bad_ServiceUnsupported` answer below.
### `SessionlessInvocationOptions` (new, `src/Opc.Ua.Server/Session/`)
- `AcceptAccessTokens` (default `true`): an Access Token in
`RequestHeader.authenticationToken`, carried as a String NodeId, is
handed to the server's identity registry (`IServerIdentityRegistry`, the
one ActivateSession uses) as an issued JWT token (OPC 10000-6
§7.6.5.2.3). The resulting identity runs the request. §6.3.1 requires an
encrypted channel for the token, so it is only accepted over
SignAndEncrypt or HTTPS.
- `AllowAnonymous` (default `false`): a request without a token runs as
an anonymous user. Without it, such a request is rejected with
`Bad_IdentityTokenInvalid`.
- `MaxConcurrentRequests` (default 64) and
`MaxConcurrentRequestsPerChannel` (default 16): the budget for
session-less requests, see below. Zero does not limit; a negative value
is rejected.
### Budget for session-less requests (`SessionlessRequestBudget`, new,
internal)
#4541 bounds channels and incomplete messages. A session-less request
has no Session to be accounted for, but it occupies a worker and keeps
its channel active. It gets a budget of its own:
- **Two limits**, checked and taken together under one
`System.Threading.Lock`: concurrent requests on the server, and on one
channel. A request refused by the channel limit takes no server-wide
place. The channel is the SecureChannel on opc.tcp. The HTTPS bindings
use one SecureChannel id per listener, so their requests are grouped by
peer address (IPv4-mapped IPv6 addresses count as IPv4).
- **Answer**: `Bad_ServerTooBusy` ("The Server does not have the
resources to process the request at this time."), the code #4541's
request queue uses when it has no room. `Bad_TooManyOperations` means
too many operations in one request, so it does not fit.
- **Lifetime**: the place is taken before the identity is checked, which
also bounds the validations that run at the same time. The lease is
attached to the `OperationContext` and returned when the context is
disposed, which every service does with `using`, however the request
ends (completed, failed, cancelled). A rejected token, a handler error
or exception, and a cancelled validation return it at once. A lease
releases once, however often it is disposed. Channel entries are removed
with their last request.
- **Scope**: it applies to every session-less request on a manager that
has `SessionlessInvocation` set, including requests a
`ValidateSessionLessRequest` handler decides. A handler without
`AddSessionlessInvocation` runs without a limit, as before. Requests
with a Session token and services outside the §6.3.1 set are not
counted.
- The limits are read for each request, so a change of the options
applies to the next request.
### `SessionManager`
- **Service sets:** a request without a session is limited to the
service sets §6.3.1 allows: View (without
RegisterNodes/UnregisterNodes), Attribute, Method, NodeManagement and
Query. Every other service keeps answering `Bad_SessionIdInvalid`.
`IsSessionlessService(RequestType)` is public.
- **Request without a token:** with session-less invocation not enabled
and no `ValidateSessionLessRequest` handler, it is answered with
`Bad_ServiceUnsupported` (§6.3.1). A token that names no session stays
`Bad_SessionIdInvalid`.
- **Precedence:** a registered `ValidateSessionLessRequest` handler
still decides first, unchanged apart from the service-set limit.
`ValidateSessionlessRequestAsync` is `protected virtual` for servers
that need their own rules.
- **Roles:** the identity of a session-less request goes through the
role resolution ActivateSession applies to the effective identity of a
Session (the `ResolveRoles` part of `AddMandatoryRoles`: the
`MustChangePassword` restriction, `IRoleManager.ResolveGrantedRoles`, so
mapped Access Token claims and the built-in AuthenticatedUser role).
This includes an identity a `ValidateSessionLessRequest` handler sets.
There is no client application certificate without a Session, so no
TrustedApplication role and no application-based mapping.
- **Cancellation:** cancelling an Access Token validation propagates as
`OperationCanceledException` so the endpoint maps it to the status of
the request lifetime (previously it became `Bad_UnexpectedError`). The
budget place is still returned.
- **Log:** a rejected Access Token is logged with a dedicated
`[LoggerMessage]` (`ServerEventIds.SessionManager + 18`, the next free
id after #4632).
### Documentation
- `docs/SessionlessInvocation.md` (new, linked from `docs/README.md`):
which services, enabling it, caller identity, the answers without the
feature, the request limits, the handler, calling the server over
opc.tcp and the REST binding.
- `docs/ResourceIsolation.md`: a short section "Requests without a
Session" pointing to the budget.
- `docs/DependencyInjection.md`: `AddSessionlessInvocation` in the
identity extension tables.
## Tests
- `Opc.Ua.Server.Tests/SessionlessInvocationTests` (mocked server
internals):
- `SessionlessRequestIsUnsupportedUnlessEnabled`,
`AnUnknownSessionTokenStaysBadSessionIdInvalid`,
`ServicesOutsideTheSessionlessSetStillNeedASession` (7 services),
`TheServiceSetLimitAppliesBeforeAValidateSessionLessRequestHandler`,
`AValidateSessionLessRequestHandlerTakesPrecedenceAsync`;
-
`SessionlessRequestWithoutIdentityIsRejectedUnlessAnonymousIsAllowedAsync`,
`SessionlessAccessTokenIsValidatedByTheIdentityRegistryAsync`,
`AnAccessTokenNoAuthenticatorHandlesIsRejected`,
`SessionlessAccessTokenRequiresAnEncryptedChannel` (None, Sign),
`SessionlessAccessTokenIsAcceptedOverSignAndEncryptAsync`,
`AccessTokensCanBeDisabled`,
`IsSessionlessServiceFollowsTheServiceSetsOfPartFour` (21 cases),
`ValidateSessionlessRequestRejectsMissingArguments`;
- wiring: `AddSessionlessInvocationRegistersTheConfiguredOptions`,
`AddSessionlessInvocationRejectsANullBuilder`,
`TheHostedServerAppliesTheOptionsToItsSessionManager`,
`TheHostedServerAppliesTheOptionsToAFactoryCreatedSessionManager`,
`TheHostedServerKeepsOptionsTheSessionManagerAlreadyHas`.
- End to end against the reference server,
`Opc.Ua.Sessions.Tests/SessionlessServiceInvocationTests` (opc.tcp,
`SessionClient` on a channel without CreateSession):
- `ReadWithAnAccessTokenNeedsNoSessionAsync` (SignAndEncrypt),
`AnAccessTokenOnAnUnencryptedChannelIsRejectedAsync`,
`ARequestWithoutAnIdentityIsRejectedAsync`,
`SubscriptionsStillNeedASessionAsync`;
- one sample per service set: `ViewServicesRunWithoutASessionAsync`
(Browse, TranslateBrowsePathsToNodeIds),
`AttributeServicesRunWithoutASessionAsync` (Write, HistoryRead),
`MethodServicesRunWithoutASessionAsync` (Call),
`NodeManagementServicesRunWithoutASessionAsync` (AddNodes). They check
that the service runs and reports its operation result, not a missing
session or an internal error, since #4632 hardened only
Browse/BrowseNext against a missing session. Query is not sampled:
`StandardServer` does not implement QueryFirst/QueryNext.
- REST binding,
`HttpsWebApiIntegrationTests.SessionlessReadOverRestRunsWithAnAccessTokenAsync`
(Compact and Verbose): `POST /read` with the Access Token as String
NodeId (`"AuthenticationToken": "s=<token>"`), and the same request
without a token.
- **Budget**, `Opc.Ua.Server.Tests/SessionlessRequestBudgetTests` (22
tests):
- the budget itself: server limit and release, channel limit that takes
nothing from the server, server limit checked first, zero does not
limit, a lease releases once, HTTPS grouped by peer address (IPv4-mapped
form too), 200 concurrent callers against the limits (exactly the limit
granted, none above the channel limit, parallel release leaves zero and
no tracked channel), 16 callers acquiring and releasing in a loop
(high-water mark never above the limit);
- through `SessionManager.ValidateRequestAsync`: `Bad_ServerTooBusy`
over the server and the channel limit, disposing a context twice
releases once, places returned after a rejected Access Token, an
unencrypted channel, a missing identity, a cancelled validation (while
it waits, the place is taken), a handler error and a handler exception;
the budget also limits requests a handler decides, and a handler-only
server has none; requests outside the service set and requests a
disabled server refuses take no place; changed limits apply to the next
request and new options start a new budget;
- options: finite defaults, negative values rejected.
- **Budget end to end**,
`Opc.Ua.Sessions.Tests/SessionlessServiceInvocationTests.SessionlessRequestsRunInABudgetOfTheirOwnAsync`
(opc.tcp, requests held in the Access Token validation): the third
request over a limit of two gets `Bad_ServerTooBusy`; after release,
five sequential reads and five reads of an unknown node (operation
result bad) all pass; a channel limit of one refuses the second request
on that channel while another channel's request is admitted.
`ARequestWithoutATokenIsUnsupportedWhileTheFeatureIsOffAsync`:
`Bad_ServiceUnsupported` for a request without token on a server without
the feature, `Bad_SessionIdInvalid` for a token that names no Session.
- **Fail without the change**: the places-returned tests fail when the
budget does not release on failure (4 of 22 fail with the release in
`SessionManager` removed; the other budget tests use API this change
adds). `SessionlessRequestIsUnsupportedUnlessEnabled` and
`TheServiceSetLimitAppliesBeforeAValidateSessionLessRequestHandler` are
unchanged from the first version.
- **Review follow-up (`e9a35980b`)**:
`SessionlessAccessTokenReceivesTheRolesMappedFromItsClaimsAsync`,
`SessionlessAccessTokenWithoutTheMappedClaimIsNotGrantedTheRoleAsync`,
`AnAnonymousSessionlessRequestIsGrantedTheAnonymousRoleOnlyAsync`,
`TheIdentityOfAValidateSessionLessRequestHandlerReceivesTheMappedRolesAsync`,
`AValidateSessionLessRequestHandlerThatSetsNoIdentityKeepsNoIdentityAsync`;
end to end
`ARoleMappedNodeIsReadWithTheRolesMappedFromTheAccessTokenAsync` (the
ServerDiagnosticsSummary, readable for SecurityAdmin only, is denied for
a token whose claim is not mapped, readable once the rule maps it,
denied for another token) and
`ACancelledAccessTokenValidationAnswersTheTimeoutAndReturnsItsPlaceAsync`.
The anonymous and no-identity tests guard unchanged behavior. With the
`SessionManager.cs` change removed the three claim-mapping tests, the
end-to-end role test (`Bad_UserAccessDenied`), the end-to-end
cancellation test (`Bad_UnexpectedError` instead of `Bad_Timeout`) and
`ACancelledValidationReturnsItsPlaceAsync` fail. Lines of this commit
were not re-measured for patch coverage; the figure below is for
`496f84fce`.
- **`ClientTest`**: the three assertions that accepted
`Bad_ServiceUnsupported` too are back to master (`Bad_SessionIdInvalid`
only), so the PR no longer touches that file. Since #4646 a closed
`Session` refuses a request without authenticationToken itself; the
server code is not reached. The three tests pass with the single code
(12 cases on net10.0).
- Results on macOS: `Opc.Ua.Server.Tests` full run on net10.0 8076
passed / 9 skipped / 1 failed
(`ACancelledValidationReturnsItsPlaceAsync`, which asserted the old
`Bad_UnexpectedError` mapping; updated to the propagated cancellation
and passing); the session-less, SessionManager and role filters 368
passed / 0 failed on net10.0 and on net8.0, and the session-less
invocation and budget tests 73 passed on net10.0 after the last test was
added. `Opc.Ua.Sessions.Tests` with the
HTTPS/WebApi/WSS/OpenApi/Sessionless/ClientTest filter: 400 passed / 241
skipped / 0 failed on net10.0 and on net8.0 (the earlier host-name
failures were an environment issue of the test machine and are gone).
- Builds: `Opc.Ua.Server`, `Opc.Ua.Server.Tests` and
`Opc.Ua.Sessions.Tests` for net48, net8.0, net9.0 and net10.0 and
`Opc.Ua.Server` in Release net10.0: 0 errors, no new warnings (the
CA2213 in `SamplingGroup.cs` and the CA1861/CA2025 in other test files
are on master too).
- `dotnet format` (whitespace, style, analyzers `--severity info`, one
`--include` per changed file): no finding on a changed line. What
remains is on master lines (`SessionManager.cs` 552, 1374, 2205 and
`DependencyInjectionStandardServer.cs` 228) and the line-ending finding
of a macOS checkout.
- Coverage (`.azurepipelines/check-coverage.ps1` against master, only
the session-less tests of `Opc.Ua.Server.Tests` on net10.0): patch
105/116 changed lines (90.5 %). The 11 uncovered lines are Session-bound
lines of `ValidateRequestAsync` and the lockout logger that moved with
the change; the rest of the suite covers them. The project floor is not
meaningful for this subset.
- Fork CI and CodeQL: previous head `0fba9f21d` (master incl. #4674
merged): [CI run
37307824769](https://github.com/biancode/UA-.NETStandard/actions/runs/37307824769)
green (one net48 timing test, `KeepAliveHandlerV2Async`, failed in the
first attempt and passed on the job rerun; the PR does not touch
subscriptions), [CodeQL run
37307829138](https://github.com/biancode/UA-.NETStandard/actions/runs/37307829138)
green. Current head `e9a35980b`: [CI run
37364178979](https://github.com/biancode/UA-.NETStandard/actions/runs/37364178979),
[CodeQL run
37364182697](https://github.com/biancode/UA-.NETStandard/actions/runs/37364182697)
(results pending).
## Design decisions
Marc's answers (2026-10-05) and what the PR does:
1. **Built in** when `AddSessionlessInvocation` was used. Done.
2. **Service-set limit for all paths**: it applies before a
`ValidateSessionLessRequest` handler too. Done, unchanged.
3. **`Bad_ServiceUnsupported`** for a request without a token on a
server without the feature. Done, unchanged. A token that names no
Session stays `Bad_SessionIdInvalid`.
4. **Identity model**: an Access Token in the RequestHeader, validated
as an issued JWT token through the identity registry, anonymous only by
opt-in. Unchanged. The REST authentication is #4674, now in master.
5. **Separate budget** for session-less requests: added, see "Budget for
session-less requests". Open: no limit per identity yet (identities are
claims-defined, so the keys would be unbounded); say if you want one.
## Related Issues
- Fixes #4658
- Relates to #4657 (the REST binding's session-less service set), #4646
(a closed `Session` no longer sends requests without token), #4541
(resource isolation) and #4674 (REST authentication)
## Checklist
- [x] 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.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed. (Affected
suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered
by the fork CI's Windows net48 legs.)
- [ ] 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: Marc Schier <marcschier@users.noreply.github.com>
|
||
|
romanett
|
7212607f16 |
OPC UA Part 3 compliance: permissions, Write semantics, NodeManagement, NodeSet DataTypes, locales (#4675)
# Description Fixes the High and Medium findings of an OPC UA Part 3 (OPC 10000-3, Address Space Model) compliance audit. Each finding was checked against the full spec text (Parts 3, 4, 5, 6, 14 and 18) before it was fixed. Every fix comes with regression tests that fail without it. ### Role permissions (Part 3 §8.55, Part 4 §5.12.2.2) - The type-hierarchy exemption in `MasterNodeManager.ValidateRolePermissions` / `ValidateAccessRestrictions` now covers only Browse, Read and ReadRolePermissions. Type nodes stay visible to generic clients, which was the intent of #3705; every other permission bit is now enforced on them. - `ReceiveEvents` is enforced on the EventType node. The standard NodeSet grants it on `AuditEventType` and its subtypes only to SecurityAdmin, so only SecurityAdmin sessions now receive audit events. - Call checks the `Call` permission and AccessRestrictions on the Object as well as on the Method. - When a Call's `methodId` is the Method declaration of the ObjectType, the Object's Method with the same BrowseName is permission-checked and invoked. The declaration is used only if the Object has no such Method. This applies to `AsyncCustomNodeManager`, `CustomNodeManager2` and the sample node manager. ### Write service (Part 3 §5.6.2, §8.40, §8.57; Part 4 §5.11.4) - The synchronous `OnWriteValue` path (fluent `OnWrite`) checks DataType and ValueRank first (`Bad_TypeMismatch`). After an IndexRange write it merges the slice into the cached value instead of replacing it. - StatusWrite / TimestampWrite are enforced (`Bad_WriteNotSupported`). - Undefined enumeration values and invalid OptionSet writes return `Bad_OutOfRange`. OptionSet writes merge only the valid bits into the stored value. - New `ReportPropertyValueChanged` / `ReportSemanticChange` let server code raise SemanticChangeEvents for changes it makes itself. - Required follow-up (Part 14 §6.2.11.1): if an external target rejects a timestamp, the PubSub subscriber's TargetVariables writer retries without it. ### NodeManagement and Call arguments (Part 3 §5.5.2, §5.7.3, §6.4.2; Part 4 §5.8, §5.12.2) - AddNodes creates the Mandatory InstanceDeclarations of the TypeDefinition and rejects abstract or Interface types (`Bad_TypeDefinitionInvalid`). - AddReferences, DeleteReferences and DeleteNodes bump NodeVersion and raise GeneralModelChangeEvents. - Clients may omit trailing optional input arguments (`HasOptionalInputArgumentDescription`). ### NodeSet DataTypes (Part 3 §5.8.3, §8.48; Part 6 Annex F.12) - `UANodeSet.Import` completes structure definitions with the base type's fields (base fields first). It creates definitions for structures without fields and sets `DefaultEncodingId`. `StructureDefinition.FirstExplicitFieldIndex` marks the subtype's own fields. - `DataTypeState` no longer mutates the stored definition on read. ### Locales (Part 3 §5.2.5, §8.5; Part 4 §5.4) - Read, Browse and data change notifications return the translation that best matches the session's LocaleIds, or all translations for `mul`. The selection lives in the server dispatcher and is built on `ResourceManager`: the new `TranslateValue` calls the overridable `Translate`. - Locale ids are matched case-insensitively, and fallback translations carry their real locale. ### Client complex types (Part 6 §5.1.7) - The Reflection.Emit builder encodes only `IsOptional` subtyped fields as ExtensionObject. It also supports OptionSet subtypes: an unsupported type is skipped instead of aborting the whole load. - `TypeInfo.GetXmlName` uses the data type's XML name for dynamic structures and OptionSets (the XML body element was `<Structure>`). Both builders now produce identical XML. ## Behaviour changes All are documented in `docs/MigrationGuide.md`. - **Audit events:** only SecurityAdmin receives them. The CTT "Auditing Connections" unit needs a SecurityAdmin user (e.g. sysadmin); `AuditingOperationTests` was updated. - **Write timestamps and status:** clients writing a SourceTimestamp or a non-Good StatusCode get `Bad_WriteNotSupported` unless the Variable has StatusWrite/TimestampWrite. No variable in `src/` sets these bits. - **AddNodes:** it rejects abstract types (including `BaseVariableType` and `BaseEventType`), and new instances now come with child nodes. - **Call:** the Object needs the Call permission. The Object's own Method runs even when the call names the type declaration. ## Not included - Low-severity findings: missing validation on optional services and author-side NodeSet rules. - LocalizedText nested inside structures or Variant arrays, and in HistoryRead or Call outputs. ## Testing net10.0, Debug, one suite at a time on the merged branch. All suites below pass: | Suite | Passed | |---|---| | Types | 11430 | | Core.Encoders | 2947 | | Core.Schema | 179 | | Core.Security | 558 | | Client | 2761 | | Server | 8210 | | InformationModel | 1080 | | Features | 145 | | History | 536 | | Di | 455 | | Machinery | 167 | | Robotics | 661 | | Robotics.Intent | 22 | | Pumps | 75 | | Scales | 122 | | Positioning | 69 | | ISA95 | 144 | | Vision | 484 | | PubSub | 1490 | | PubSub.Adapter | 151 | | PubSub.Server | 177 | | Redundancy.Server | 856 | | Lds | 173 | | Configuration | 241 | | WotCon | 1496 | - **Client.ComplexTypes:** one test wrote a SourceTimestamp to a variable without TimestampWrite. It is fixed in this PR, and the suite has not been re-run since. - **Core:** 6068 passed, 1 failure in `ActivityTraceFlags_Metrics_ShouldRecordMetricsOnly`. That telemetry test passed when re-run alone, so it looks flaky. - **Still running:** Sessions, Subscriptions (Classic, Durable, standard) and Interop. Results will follow on this PR. - `UA.slnx` builds with 0 errors for net10.0 and net48, with no new analyzer warnings in changed files. ## Related Issues - Part 3 compliance audit (no tracking issue) ## Checklist - [x] 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. - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
8bdc13ce27 |
WebApi: enforce AddWebApi*Auth on the HTTPS listener's own host (REST routes were served unauthenticated) (#4674)
# Description `AddWebApiBasicAuth` / `AddWebApiBearerAuth` / `AddWebApiMutualTlsAuth` registered their ASP.NET Core authentication schemes only on the **application** `IServiceCollection`. `HttpsTransportListener` serves the REST routes from its **own** Kestrel host with a separate DI container (own-host and shared-host paths), into which only `IHttpsListenerServiceContributor.ConfigureServices` registrations flow. `WebApiHttpsStartupContributor.Configure` checked for schemes in the listener container, found none, and therefore skipped `UseAuthentication()` / `UseAuthorization()` and `RequireAuthorization()`. Result: on a real listener the REST routes (`/read`, `/write`, `/call`, sessions, subscriptions, …) were served **without** the configured HTTP credential. The existing unit tests only checked the application container, so they passed. Present since the WebApi binding was introduced (#3880). ## Changes - **Record + replay.** Each `AddWebApi*Auth` opt-in still registers its scheme on the application container (so hosting `MapWebApiEndpoints()` in your own ASP.NET Core app keeps working), and also records it as an internal `WebApiListenerAuthRegistration`. `AddWebApiTransport()` now hands the application `IServiceProvider` to `WebApiHttpsStartupContributor`, whose `ConfigureServices` replays the recorded schemes, the policy scheme and the application's `ISessionlessIdentityProvider` into the listener container. Auth middleware and `RequireAuthorization()` are now installed on the pipeline that actually serves the routes. - Side effects fixed by the same change: `UseJwtClaimIdentityProvider()` / Bearer claims mapping now reaches the dispatcher, and the WSS `opcua+openapi+<accesstoken>` upgrade finds the Bearer scheme instead of always failing closed. - **No-credentials fallback scheme.** Previously the policy selector returned `null` for a request without a credential; ASP.NET Core's `PolicySchemeHandler` then throws on challenge (→ 500). The selector now forwards to a new internal `WebApiNoCredentialsAuthenticationHandler` that authenticates nobody and answers the challenge with **401** plus the `WWW-Authenticate` challenges of the registered Basic / Bearer schemes. - **Selector only picks registered schemes.** A `Bearer` header when only Basic is registered (or vice versa) used to forward to a non-existent scheme (→ 500); it now falls through to the no-credentials scheme (→ 401). ## Reviewer notes - Only options configured through the opt-in callbacks are replayed. Options added separately via `services.Configure<JwtBearerOptions>(scheme, …)` on the application container do not reach the listener container. - mTLS-only: a request without a client certificate gets 401 without a `WWW-Authenticate` header (there is no HTTP challenge for client certificates). - Discovery routes (`/findservers`, `/getendpoints`) keep their `AllowAnonymous` metadata. ## Tests New `RealHttpsListenerIntegrationTests.Authentication.cs` builds the services exactly as an operator would (`AddOpcUa().AddWebApiTransport().AddWebApi*Auth(...)`) and posts `/read` through a real `HttpsTransportListener` + Kestrel TLS endpoint: | Auth | Credential | Expected | |---|---|---| | Basic | none / wrong password / Bearer token | 401 + `WWW-Authenticate: Basic`, not reaching the server | | Basic | valid | 200 | | Bearer | none / `not.a.jwt` / JWT with wrong key / Basic | 401 + `WWW-Authenticate: Bearer`, not reaching the server | | Bearer | valid JWT | 200 | | mTLS | none / Basic | 401, not reaching the server | | Basic | `/findservers` without credential | 200 | With the replay disabled, all 9 negative cases fail with 200 (reproducing the bug). Locally on net10.0: `Opc.Ua.Bindings.Https.WebApi.Tests` 450/450, WebApi/OpenApi tests in `Opc.Ua.Sessions.Tests` 17/17; `Opc.Ua.Bindings.Https` builds for all target frameworks with no warnings. ## Related Issues - No tracking issue; reported externally. ## Checklist - [ ] 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. - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
b03575b4e2 |
Run the interop tests against six foreign OPC UA stacks and extend feature coverage (#4644)
## Proposed changes Runs the interop fixtures added in #4630 against peers built on six other open source OPC UA stacks, so the 2.0 client and server are tested against independent implementations, not only the released 1.5.378 stack. It also extends the fixtures beyond read/write/browse to events, subscriptions, identity and more services. ### Peers (`tests/Opc.Ua.Interop.Peers`) | Peer | Stack | Toolchain | Roles | | --- | --- | --- | --- | | `node-opcua` | node-opcua 2.186.15 (MIT) | Node.js 24 | server, client | | `milo` | Eclipse Milo 1.1.7 (EPL-2.0) | Java 17+, Maven | server, client | | `asyncua` | opcua-asyncio 2.0.1 (LGPL-3.0) | Python 3.12 | server, client | | `open62541` | open62541 1.5.8 (MPL-2.0), OpenSSL backend | C, CMake | server, client, ECC | | `async-opcua` | async-opcua 0.19.0 (MPL-2.0) | Rust | server, client | | `gopcua` | gopcua 0.9.1 (MIT) | Go | client only | Every peer speaks the command line and stdout contract of `Opc.Ua.Interop.LegacyPeer` (`server` / `client --checks`, `PEER-INFO {json}`, `RESULT {json}`). The stacks are fetched at build time and run as separate processes; nothing is linked into or shipped with any package. ### Harness - **Peer selection:** `OPCUA_INTEROP_PEER_HOST` + `OPCUA_INTEROP_PEER_HOST_ARGUMENTS` choose the host (`node`, `java -jar`, `python`) for the peer named by `OPCUA_INTEROP_LEGACY_PEER`. Native peers (open62541, async-opcua, gopcua) run without a host. Unset, the 1.5.378 peer runs exactly as before, so the main CI is unchanged. - **`PEER-INFO {json}`:** stack, version, application URI, software version and, optionally, the security policies the peer implements. `LegacyServerInteropTests` compares the ApplicationUri and SoftwareVersion against it. `EccInteropTests` runs policies beyond the four 1.05 ECC policies only against a peer that declares them. - **`expected-differences.json` per peer:** confirmed defects of a stack, keyed by check name, `Fixture.Test` or a `*` prefix, each with the source file or spec section that shows it. A listed failure is reported as inconclusive with its reason; a listed test that passes produces a warning. The 2.0 client tests apply it through the fixture-level `[PeerDifferences]` action. ### New coverage (all peers, including the 1.5.378 peer) - **16 client checks**, run by each peer's client against the 2.0 server (`LegacyClientFeatureCheckAsync`, Basic256Sha256 SignAndEncrypt): EventSubscription, ConditionRefresh, AlarmAcknowledge, DeadbandFilter, QueueOverflow, Triggering, Republish, TransferSubscription, WrongPasswordRejected, X509UserToken, RegisterNodes, HistoryReadRaw, NodeManagement, IndexRange, FindServers, SessionReconnect. - **`ServerFeatureInteropTests`**: the 2.0 client against each peer server. It covers RaiseEvent, ConditionRefresh, absolute deadband, queue overflow bit, triggering, republish errors, transfer to a new session, X509 user token, RegisterNodes, IndexRange, AddNodes/DeleteNodes, FindServers and session reconnect. Every peer server gained a `RaiseEvent` method and accepts X509 user tokens. - **ECC:** with open62541's OpenSSL backend, `EccInteropTests` adds `ECC_nistP256_AesGcm` and `ECC_nistP256_ChaChaPoly` (both directions; SignAndEncrypt anonymous and username, Sign, and token renewal across two renewals). It also adds `ECC_curve25519`/`ECC_curve448` cases, which are ignored while 2.0 marks those policies unsupported. ### CI and docs - **Workflow `interop-foreign.yml`:** opt-in (nightly, `workflow_dispatch`, PRs labelled `interop`), not a required check. It builds all six peers on Windows, Linux and macOS (open62541 from source on Linux only, against the distro OpenSSL), with dependency caching. - **`tests/Opc.Ua.Interop.Peers/README.md`:** how to install the toolchains, build each peer (including Windows without Visual Studio), run the fixtures against it, maintain expected differences and add a new stack. `DeveloperGuide.md` links to it. ## Results Run locally, sequentially, net10.0, Windows. Not passed = expected differences, inconclusive by design (e.g. AddNodes on a server without NodeManagement), or ECC cases a peer or the platform does not support. | Peer | Fixtures | Total | Passed | Failed | | --- | --- | --- | --- | --- | | 1.5.378 | whole interop project | 220 | 218 | 0 | | node-opcua | server + client + feature | 145 | 143 | 0 | | Eclipse Milo | server + client + feature | 145 | 141 | 0 | | asyncua | server + client + feature | 145 | 133 | 0 | | open62541 (OpenSSL) | server + client + feature + ECC | 199 | 175 | 0 | | async-opcua | server + client + feature | 145 | 137 | 0 | | gopcua | client | 104 | 81 | 0 | After the AEAD cases were added, `EccInteropTests` against the 1.5.378 peer ran separately: 54 total, 26 passed, 0 failed. The new cases are ignored there because the peer does not declare them. ## Findings No 2.0 defects beyond the two now fixed in #4654: - Browse with a null ViewId and a Timestamp now returns `Bad_ViewTimestampInvalid`. - Subscription transfer no longer fails when the optional `Server.GetMonitoredItems` is unavailable. After #4654 merges, `TransferSubscriptionToNewSessionAsync` can drop its expected differences for asyncua. Upstream defects found, all listed in the peers' `expected-differences.json` with evidence; issues are still to be filed: - **All five non-.NET stacks:** cannot handle the TestData `ScalarStructureDataType` (fields of the abstract Enumeration/Structure/BaseDataType). Milo encodes the abstract `Enumeration` field as a Variant (Part 6 5.2.4 says Int32). - **open62541:** - **Sign-only ECC keys:** derives encryption key and IV into the HKDF length for Sign-only ECC (Part 6 6.8.1), so Sign mode fails for the four 1.05 curves. The AEAD policies are not affected. - **HMAC mismatch:** reported as `Bad_InternalError`. - **ConditionRefresh:** needs the experimental A&C option. - **Response too large:** closes the connection instead of answering `Bad_ResponseTooLarge`. - **Array limit:** no MaxArrayLength. - **gopcua:** - **RsaPss:** `RSAOAEPMinPaddingSHA256` is 130 instead of 66, which breaks `Aes256_Sha256_RsaPss`. - **Nil LocalizedText:** encoding a nil `*LocalizedText` panics. - **asyncua server:** - **Queue overflow:** no Overflow bit. - **SetTriggering:** not implemented. - **Republish:** answers Good for an unknown sequence number or subscription. - **GetMonitoredItems:** not implemented. - **IndexRange:** ignored on Read and Write. - **Reconnect:** closes a session without subscriptions on transport loss. - **Limits:** no BrowseNext; operation limits not enforced; no `Bad_ResponseTooLarge`. - **async-opcua:** - **X509 user token (client):** treats an empty token SecurityPolicyUri as None. - **Reconnect (client):** overwrites the session nonce with the channel nonce on reconnect. - **ConditionRefresh (server):** not implemented. - **Overflow bit (server):** set without the DataValue InfoType bit. - **ECC:** no ECC policies. - **Milo:** - **Wrong password:** `BadIdentityTokenInvalid` instead of `BadUserAccessDenied` / `BadIdentityTokenRejected`. - **Array limit:** accepts arrays above its advertised MaxArrayLength. - **AddNodes:** answers `BadNotSupported`. ## Related Issues - Follows #4630. Related fixes: #4654. ## Types of changes - [x] Test / CI only ## Checklist - [x] Interop fixtures pass locally against all seven peers (net10.0, Windows). - [x] `dotnet format whitespace --verify-no-changes` on the changed C# files. - [ ] `interop-foreign.yml` has not run on GitHub yet; it needs a manual dispatch or the `interop` label. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Klaus Landsdorf
|
11ad24e73e |
Kubernetes readiness tests: give each test a port of its own (fixes the Windows "conflicts with an existing registration" flake) (#4673)
# Description `KubernetesReadinessServerHttpTests.StartIsIdempotentAndDisposeStopsListenerAsync` failed on the Windows CI legs with `HttpListenerException: Failed to listen on prefix 'http://localhost:57023/readyz/' because it conflicts with an existing registration on the machine` (error 183, `ERROR_ALREADY_EXISTS`). `HttpListener` raises that error only when http.sys already holds the exact URL prefix, and the only code that registers `http://localhost:<port>/readyz/` in the test process is the readiness tests themselves; three sibling tests of the `ParallelScope.All` fixture were running when it failed. The tests took their port from a `TcpListener` probe that is stopped again before the readiness server registers its prefixes, so tests that run in parallel can draw the same port. On Windows the probe cannot rule this out either, because an http.sys registration is an entry in the URL namespace of the kernel driver and not a socket bound to the probed loopback address. The fix is test-only: - A shared `ReadinessTestServer` helper hands out loopback ports that are never reused within the test run. Every probe stays open until a port is found that was not handed out before, so the OS cannot give the same port back on the next probe. The readiness tests can therefore no longer collide with one another, whatever the scheduling or the OS port allocation. - A port that another process takes between the probe and the start is handled by retrying the start on a fresh port, at most five times. Only `HttpListenerException` from the start is retried; anything else is rethrown at once, and a failed server is disposed before the next attempt. The production `KubernetesReadinessServer` needs no change: `Start` is idempotent under its lock (the failure was on the first call), `HttpListener.Start` releases its registrations when adding a prefix fails, and `DisposeAsync` removes them synchronously through `HttpListener.Close`. ## Changes - `tests/Opc.Ua.Redundancy.Kubernetes.Tests/Health/ReadinessTestServer.cs` (new): `GetUniqueFreePort()` and `StartAsync(createServer, startServer)`, which returns the started server together with its port. - `tests/Opc.Ua.Redundancy.Kubernetes.Tests/Health/KubernetesReadinessServerHttpTests.cs`: the HTTP tests start their servers through the helper; the local `GetFreePort()` is gone. - `tests/Opc.Ua.Redundancy.Kubernetes.Tests/Health/KubernetesReadinessStartupTaskTests.cs`: `OnServerStartedStartsReadinessServerAsync` starts the server through the startup task inside the helper; the local `GetFreePort()` is gone. ## Tests - **New** `StartFailsWhenAnotherServerRegisteredTheSamePrefixAsync`: a second server on the port of a running one fails to start with `HttpListenerException`, and the first server keeps answering `/readyz`. This is the collision behind the flake; on macOS it produces exactly the message of the Windows CI failure (checked locally with a temporary assertion on the message). - **New** `StartMovesToAFreshPortWhenThePrefixIsAlreadyRegisteredAsync`: the first attempt of the helper is pointed at an occupied port; the helper moves to a fresh port on the second attempt and the server answers there. - **New** `UniqueFreePortsAreNeverHandedOutTwice`: 32 ports drawn concurrently are distinct. - `Opc.Ua.Redundancy.Kubernetes.Tests` passed 103/103 on net10.0 and on net8.0 (macOS), and 20 consecutive runs per framework (40 runs) all passed. - `Opc.Ua.Redundancy.Kubernetes` builds for net8.0, net9.0 and net10.0 with 0 warnings; the test project builds for net8.0 and net10.0 with 0 warnings, and as the empty legacy shell with `CustomTestTarget=net48`. - `dotnet format` (whitespace, style, analyzers at info severity) on the changed files: no findings (whitespace checked with the files converted to CRLF, as `.editorconfig` asks; an LF checkout reports `ENDOFLINE` for every line of every file). - The Windows behaviour (http.sys error 183 and its URL-namespace semantics) is reasoned from the `System.Net.HttpListener` sources, not reproduced locally; the fork CI covers the Windows legs. - Fork CI (full matrix incl. macOS) and CodeQL: [CI run 37203410921](https://github.com/biancode/UA-.NETStandard/actions/runs/37203410921) and [CodeQL run 37203412518](https://github.com/biancode/UA-.NETStandard/actions/runs/37203412518), both green, including all Windows legs. ## Notes for review - The retry only covers the probe-to-start window for ports taken by other processes; collisions between the tests themselves are ruled out by the never-reused ports, not by retrying. The `HttpListenerFactory` of dotnet/runtime's own `System.Net.HttpListener` tests takes the same approach (ports never reused within the process, plus a retry on `HttpListenerException`), and `ServerFixtureUtils.IsPortUnavailable` in this repository retries server starts on a taken port as well. - `DisposingAnUnstartedServerDoesNotBindItsPort` keeps its own occupant socket on the wildcard address; that server is never started, so it does not register a prefix. ## Related Issues - Fixes #4671 - Follows up #4613. ## Checklist - [x] 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. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suite on net10.0 and net8.0 locally on macOS; the Kubernetes tests do not target .NET Framework.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |
||
|
romanett
|
cce2004fdd |
CTT: keep GeneratesEvent off Object instances and Views off non-hierarchical references; classify Deadband Err-006 as CTT defect (#4667)
# Description Three findings from the 2026-10-03 CTT run with the patched 1.05.513 scripts. Two are server/model defects and are fixed here. The third is a CTT script defect, recorded as C56 in `plans/ctt-issues.md`. ## 1. Monitored Items Deadband Filter `Err-006.js`: CTT defect (C56, no server change) The script modifies a MonitoredItem on the first static numeric scalar (a plain BaseDataVariable with no EURange) to use a PercentDeadband. It accepts only `Good` or `Bad_MonitoredItemFilterUnsupported`. Since #4629 the server returns `Bad_DeadbandFilterInvalid`, and that is the code the spec defines for this case: - OPC 10000-8 §7.2: PercentDeadband "applies only to AnalogItems with an EURange Property". - OPC 10000-8 §7.3.2 Table 61: `Bad_DeadbandFilterInvalid` is returned when "a PercentDeadband is not supported, since an EURange is not configured". CreateMonitoredItems and ModifyMonitoredItems both go through `ValidateMonitoringFilterAsync` and return the same code. Monitor Basic `Err-022.js` already accepts `Bad_FilterNotAllowed` for non-analog items, so the scripts disagree with each other here. Draft Mantis note (not filed yet): > **Monitored Items Deadband Filter Err-006.js rejects Bad_DeadbandFilterInvalid for a PercentDeadband on a node without EURange** > Err-006.js picks the first node of `/Server Test/NodeIds/Static/All Profiles/Scalar/NumericSettings`, which is normally a BaseDataVariable without an EURange. It then calls ModifyMonitoredItems with a PercentDeadband (10 %) and accepts only Good or Bad_MonitoredItemFilterUnsupported. OPC 10000-8 §7.2 limits PercentDeadband to AnalogItems with an EURange. §7.3.2 Table 61 defines Bad_DeadbandFilterInvalid for "a PercentDeadband is not supported, since an EURange is not configured", so a compliant server fails this test. Monitor Basic Err-022.js already accepts Bad_FilterNotAllowed for the non-analog fallback. > Proposed fix: add StatusCode.BadDeadbandFilterInvalid (and, consistent with Err-022.js, StatusCode.BadFilterNotAllowed) to the ExpectedAndAcceptedResults of the ModifyMonitoredItems call. ## 2. GeneratesEvent on Objects (Base Info Core Structure 2 `001.js`): source generator defect The CTT reported `ns=9;i=234` (the `<ServiceName>` placeholder of AuthorizationServicesFolderType) and `ns=9;s=AuthorizationServices/Default` as Objects that are the source of GeneratesEvent references. OPC 10000-3 §7.15 says: "The SourceNode of References of this type shall be an ObjectType, a VariableType or a Method". `OpcUaGdsModel.xml` declares both references (AccessTokenRequested/Issued audit events) correctly, on `AuthorizationServiceType`. The source generator copied the type's own non-hierarchical references onto every Object or Variable built from the type: onto instance declarations (the placeholder) and into `CreateInstanceOf<Type>`, which `AuthorizationServiceState` initialises from, and that is how the `Default` service gets them. UA-ModelCompiler does the same thing, so the official `Opc.Ua.Gds.NodeSet2.xml` in UA-Nodeset has the same two references on `ns=1;i=234`. Fix: - `TranslateReferences` now marks references declared on a type (`HierarchyReference.DefinedOnType`, which already existed but was never set). - `NodeStateGenerator.GetReferences` drops forward GeneratesEvent references (including subtypes such as AlwaysGeneratesEvent) for Object and Variable nodes when the reference came from the type. - ObjectTypes, VariableTypes and Methods keep these references. For example, the RegisterApplication methods and the Acknowledge/AddComment methods of the alarm types are unchanged. - References that a model declares explicitly on an instance are emitted as before. ## 3. View targeted by HasCause/HasEffect (Base Info Core Structure 2 `001.js`): sample model defect #4485 added `Views_Operations HasCause/HasEffect Views_Engineering` so that View Minimum Continuation Point 01 `012.js` and View Basic 2 `018.js` have at least two references to View nodes on the configured View (`/Server Test/NodeIds/NodeClasses/View` = `Views_Operations`). OPC 10000-3 §5.4 says Views "can be the SourceNode of any hierarchical Reference. They shall not be the SourceNode of any NonHierarchical Reference." `Views_Operations` now Organizes `Views_Engineering` and a new View, `Views_Maintenance`, which organizes `DataAccess`. This keeps two View-class references and adds no loops (ContainsNoLoops stays true). ## Tests - `NodeStateGeneratorTests.TypeGeneratesEventReferencesAreNotCopiedToObjectInstances`: covers the type, the placeholder, the instance, `CreateInstanceOf`, Methods and an explicit instance reference. It fails without the fix. - `ReferenceServerTest.SampleViewsHaveNoNonHierarchicalReferencesAsync`. The existing `NodeClassSampleNodesReferenceTheirOwnNodeClassAsync` still passes. - Results with net10.0: - SourceGeneration.Core.Tests: 4267 passed, 0 failed - InformationModel.Tests: 1078 passed, 0 failed - Di.Tests, including RobotOpenUsdE2e: 438 passed, 0 failed - OpenUsd.Tests: 921 passed, 0 failed - Server.Tests `ReferenceServerTest`: 56 passed, 0 failed - `UA.slnx` builds with net10.0. The changed test projects build with net48. ## CTT (patched 1.05.513 scripts, compared with the 2026-10-03 patched baseline) | Part | Change | | --- | --- | | p02-base-info | Base Info Core Structure 2 `001.js`: Error → Skipped. The GeneratesEvent and View errors are gone; the remaining skips are the CTT's own "type not defined within the test-script" notes. No other change. | | p03-base-view-method | No change. View Basic 2 `018.js` and View Minimum Continuation Point 01 `012.js` still pass. | | p13-monitored-items | Deadband `Err-006.js` still Error (C56). With the script fix it passes. Monitor Basic `007.js` gave a response-timestamp delay warning (>200 ms) caused by machine load; it also appears in the 2026-09-29 base run. | 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Klaus Landsdorf
|
b25c1f6ad4 |
Asset Management Basics: the OPC 10000-110 companion SDK (#4662)
# Description Stacked on #4650. Until that is merged, this PR also shows its commits; the six commits of this PR start after `fix(server): keep conformance unit aggregation idempotent and ordered with registrations`. This adds OPC 10000-110 Asset Management Basics 1.01.1 (`http://opcfoundation.org/UA/AMB/`) as a companion SDK, in the pattern of the DI, Machinery, Pumps and Scales packages. The tracking issue #4649 describes the scope, the design and the open questions. In short: | Package | Content | |---|---| | `Opc.Ua.AMB` | the model generated by the source generator from the unmodified NodeSet, plus the contracts server and client share (`AssetFaultSeverity`, `AssetLocationKind`, `MaintenanceStateKind`, `DocumentationLinkProperties`) | | `Opc.Ua.AMB.Server` | a sidecar node manager next to the owner of the Device Integration address space (`AddOpcUaDi`, `AddMachinery`, `AddPumps` or `AddScales`), the asset registry `IAssetManagement` with a fluent builder, `AddAssetManagement()` hosting, memory and file system stores, the AMB conformance units and server facet | | `Opc.Ua.AMB.Client` | `AmbClient`: discovery, identification, health alarm and maintenance streams, documentation links, locations, structure and snapshots, plus `AddAssetManagementClient()` | **Sidecar.** An asset stays in the node manager that owns it. Everything AMB adds to it (`DeviceHealth`, health alarms, maintenance conditions, the DocumentationLinks AddIn, AssetId and the other properties) is created there. The AMB node manager serves the AMB model, the alias categories `Assets`, `AssetsByProductInstanceUri` and `AssetsByAssetId` below `0:Aliases`, the location hierarchies below `0:Locations` and the server-specific alarm and condition types. Owners register assets with the shared registry from their setup, in either startup order. **Implemented conformance units** (Table 54): Asset Identification, Configurable Asset Identification, Discovery by ProductInstanceUri and by AssetId, Health Status Base, Alarms, Root Causes and Alarm Categories, Version Information, Operation Counters (inspected only; AMB defines no counters), DocumentationLinks Base, Edit Base and Edit Advanced, Requirements, Capabilities, Classification, Current and Future Maintenance Activities, Local Time, the three location properties and both location object hierarchies, Sub-assets and Asset relations. The two history units (Health Tracking, Past Maintenance Activities) are not part of this PR. `docs/AMB.md` has the matrix with the rule the server applies for each unit and whether a test checks it over a session. **Dependency on #4650.** The health alarms and maintenance conditions are created through `CreateAlarm<TState>`, and the conformance units follow the registered assets through the recomputation in `ConformanceUnitsManager`. ## Changes Six commits, each building on its own: 1. **`feat(amb)`: model package** (`src/Opc.Ua.AMB/`, about 2.4k lines, of which the NodeSet and the two identifier tables are 1.6k). - The NodeSet is the publication from `UA-Nodeset/latest/AMB`, byte for byte (git blob `21f9475b646e9c5508171277ffe5b74edbeb0a58`). `.gitattributes` checks it out with LF endings, because a test pins its SHA-256. - The published identifier table is vendored unchanged; a derived table pins the symbol-to-NodeId mapping, and the generator fails the build when they disagree. `tools/nodesets/README.md` documents the one new kind of renamed symbol (top-level instances below `0:Aliases` and `0:Locations`). 2. **`feat(amb)`: server package** (`src/Opc.Ua.AMB.Server/`, about 10.7k lines). Node manager, registry and builders, discovery, health, maintenance (`MaintenanceEventStateMachine` with all transitions), documentation links, locations, version information, classification (IRDI dictionary entries below `Server/Dictionaries`) and structure, stores, conformance evaluation, hosting. Log messages use the EventId block 8300 to 8379, after Pumps (8100) and Scales (8200). 3. **`feat(amb)`: client package** (`src/Opc.Ua.AMB.Client/`, about 3.0k lines). Calls AddLink and RemoveLink through the generated `DocumentationLinksTypeClient`. 4. **`test(amb)`: test suite** (`tests/Opc.Ua.AMB.Tests/`, 163 tests, about 8.8k lines; commit 6 adds one more). 5. **`docs(amb)`: guide and samples**: `docs/AMB.md`, links from `README.md`, `docs/README.md`, `docs/Samples.md` and `docs/AliasNames.md`; `samples/AMB/AmbServer` (AMB next to `AddMachinery`: two DI devices and a Machinery press as assets, with a simulation) and `samples/AMB/AmbClient`. The sample server listens on 62553, which no other sample uses. 6. **`feat(amb)`: tell the links users added from the links of the manufacturer.** OPC 10000-110 gives a client no way to see which links `RemoveLink` accepts (§10.5.4: only those `AddLink` created). As §10.5.1 lets vendors add metadata Properties to the links, the server gives every link `AddLink` created, and every such link restored after a restart, the `Boolean` Property `UserLink` (`true`), qualified with its namespace of server-specific types, with a NodeId derived from the persisted link identifier; declared links carry none, and `RemoveLink` deletes it with the link. `DocumentationLinkRecord` gets the additive `IsUserLink`, which `ReadDocumentationLinksAsync` fills with one Browse over all links and one Read of the markers found (comparing the name only, as the namespace is configurable; `false` for a server that does not mark links). The name is shared as `DocumentationLinkProperties.UserLink`. `UA.slnx` and `.azurepipelines/expected-packages.txt` get the new projects and packages in the commit that adds them. All projects target `net48;net8.0;net9.0;net10.0` through the repository properties (`LibTargetFrameworks`, `TestsTargetFrameworks`, `AppTargetFrameworks`); `IsAotCompatible` and `PublishAot` are set from net10.0 on, as in the other companion projects. ## Tests - **`Opc.Ua.AMB.Tests`: 164 tests**, most of them against a hosted server over real client sessions: - the model: the SHA-256 of the NodeSet and the generator behaviours AMB is the first companion model to rely on; - each building block in-process and over a session: discovery through every category with NodeVersion and model change events, AssetId writes, health alarms and maintenance activities through all transitions with acknowledgement, AddLink and RemoveLink, locations and structure, the advertised conformance units and the facet; - a restart with the file system stores (links and AssetId survive); - the `UserLink` marker: present on an added link only, the same NodeId after a restart, removed with `RemoveLink`, and `IsUserLink` on the client in the snapshot, after `AddLink`, after a restart and after `RemoveLink`; - composition with `AddOpcUaDi` (both startup orders), `AddPumps` and `AddMachinery`; - `AmbClient` end to end against a hosted server, including a Machinery machine as an asset. - **Results (macOS, local):** 164 passed / 0 failed on net10.0 and on net8.0. .NET Framework is covered by the fork CI's Windows legs. - **Coverage** of the three AMB assemblies (coverlet with `tests/coverlet.runsettings.xml`, net10.0, generated `*.g.cs`, `*TypeClient` proxies and `*Log` classes excluded): lines 93.9 % (4890 of 5206), branches 84.0 % (1294 of 1539); model 100 %, client 94.5 %, server 93.4 %. - **Build:** the three packages, the tests and the two samples build without warnings for net48, net8.0, net9.0 and net10.0; `UA.slnx` builds in Release (its warnings are all in files this PR does not touch, as on master). `dotnet format` (whitespace, style, analyzers with `--severity info`) reports nothing on the 95 changed C# files. - **Samples:** `AmbServer` and `AmbClient` ran against each other on net10.0: the client discovered the three assets, printed their snapshots and the location hierarchies, added and removed a documentation link and streamed alarms and maintenance. - Fork CI (full matrix incl. macOS) and CodeQL: green on the same five package commits before the rebase onto the current #4650 head — [CI run 37144486107](https://github.com/biancode/UA-.NETStandard/actions/runs/37144486107), [CodeQL run 37144487873](https://github.com/biancode/UA-.NETStandard/actions/runs/37144487873). The run on the current head (rebased, plus the user-link commit; AMB tests 164/164 locally on net10.0) is in progress: [CI run 37203517457](https://github.com/biancode/UA-.NETStandard/actions/runs/37203517457). ## Notes for review - **Size.** About 26.8k lines, larger than each of the companion PRs #4577 to #4580. Without changes to the code it splits into the model, server and server tests, and the client, end-to-end tests, samples and docs on top; the issue asks which you prefer. - **Interfaces on types.** §9.4.1 and §12.1 ask for alarm and condition types that implement `IRootCauseIndicationType` and `IMaintenanceEventType`. The server creates subtypes of the DI alarm types in its own type namespace (`urn:opcua-netstandard:amb:types`); `UseServerDefinedAlarmTypes = false` uses `HasInterface` on the instance instead, and then does not report the units that need the type-level interfaces. - **Conformance unit names** follow Tables 54 to 56; the profile database has no AMB entries yet. The QualifiedNames are in namespace 0, as DI and Machinery publish theirs (question 5 in the issue). - **Generator gap.** Children a NodeSet gives to a top-level instance (here the `NodeVersion` properties of the alias categories) get no identifier constants. The fix changes the identifiers of other generated models, the reference server's among them, so it is not part of this PR; `AmbModelTests.NodeVersionHasNoIdentifierConstant` records it and fails once the generator emits the constants. - **net48.** The code that the SDK so far only built for .NET 8 and later uses the `NET5_0_OR_GREATER` pattern of `JsonEncoder` for `Enum.IsDefined` and API forms that .NET Framework has (`string.Format`, `Guid.ToString(format, provider)`); the tests use a small async file helper instead of `File.WriteAllTextAsync`. - **User links.** The `UserLink` Property is metadata of this server, not an AMB name: commit 6 explains why and how a client uses it. Changed lines of that commit covered: 69/70 (the one line left is the loop continuation for a link that has other Properties but no marker). - **Known limitations** are listed in the guide: the `DeviceHealth` SourceTimestamp across a restart, removing optional maintenance members once set, `PatchIdentifiers`, locations only at the deepest level, and the history units. ## Related Issues - Part of #4649 - Stacked on #4650 ## Checklist - [x] 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. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (The AMB suite on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |
||
|
Klaus Landsdorf
|
15f9a5ebcc |
Machinery client: enumerate each machine, component and process value once (#4672)
# Description `MachineryClient` browses the `Machines` folder, the `Components` add-in, the `MachineryBuildingBlocks` organizer, the energy resources and the process values with `HierarchicalReferences` and subtypes, and turned every returned `ReferenceDescription` into a `MachineEntry`. Browse returns one description per reference, not per node, and `HasEventSource` and its subtype `HasNotifier` are hierarchical references. A machine that the `Machines` folder organizes and that is also registered as a notifier of the folder - how a server exposes its event hierarchy below the `Server` object once alarms are attached - was therefore returned twice by `EnumerateMachinesAsync` and `DiscoverMachinesAsync`. The same held for a component that is an event source of its `Components` folder and for a process value that is an event source of `Monitoring`. The fix: - The shared browse helper reports each target NodeId once and keeps the first description. Its `BrowseName`, `DisplayName` and `TypeDefinition` belong to the node and are the same for every reference that leads to it. - All hierarchical references are still followed. Restricting the browse to `Organizes` would lose machines, components and process values a server references with another hierarchical reference type such as `HasComponent`, so an address space without duplicate references returns exactly what it returned before. - `EnumerateProcessValuesAsync` looks below the machinery item and below its `Monitoring`; a process value reachable from both is now reported once as well. The public API does not change. ## Changes - `src/Opc.Ua.Machinery.Client/MachineryClient.cs`: `BrowseChildrenAsync` skips a NodeId it has already returned; the remarks of `EnumerateMachinesAsync` say so. - `src/Opc.Ua.Machinery.Client/MachineryClient.ProcessValues.cs`: `EnumerateProcessValuesAsync` skips a process value below `Monitoring` it has already returned below the item. Every enumeration of the client goes through `BrowseChildrenAsync`: `EnumerateMachinesAsync`, `DiscoverMachinesAsync`, `EnumerateComponentsAsync`, `EnumerateBuildingBlocksAsync`, `EnumerateEnergyResourcesAsync`, `EnumerateMeteringPointsAsync`, `EnumerateSubMetersAsync`, `EnumerateProcessValuesAsync`, and the child walks of `ReadLifetimeCountersAsync`, `EnumerateEquipmentAsync`, `ReadMeteringPointAsync` and `ReadPublishedResultsAsync`. ## Tests - **New, end-to-end** (`MachineryEndToEndTests.NodesThatAreAlsoEventSourcesOfTheirParentAreEnumeratedOnceAsync`): the test press links the machine to `Machines` with `HasNotifier`, the `HydraulicUnit` component to `Components` and the oil-temperature process value to `Monitoring` with `HasEventSource`, each in addition to its organizing reference, and expects each node once from `EnumerateMachinesAsync`, `DiscoverMachinesAsync`, `EnumerateComponentsAsync` and `EnumerateProcessValuesAsync` over `opc.tcp`. - **New, unit** (`MachineryClientTests.ANodeReachedThroughTwoReferencesIsEnumeratedOnceAsync`): a browse result that carries the same machine under `Organizes` and `HasNotifier`. - **Fail without the fix:** with `src/Opc.Ua.Machinery.Client` from master, all four assertions of the end-to-end test fail - two machines, two NodeIds, two components and two process values instead of one each. - `Opc.Ua.Machinery.Tests` passed 167/167 on net10.0 and 167/167 on net8.0 (macOS). - `Opc.Ua.Machinery.Client` builds for net48, net8.0, net9.0 and net10.0 with 0 warnings. - `dotnet format` (whitespace, style, analyzers) on the changed files: nothing on the changed lines (the `ENDOFLINE` findings concern the whole of every file in a checkout with LF line endings). - Changed lines covered: 8/8 (100 %). - Fork CI (full matrix incl. macOS) and CodeQL: [CI run 37201766371](https://github.com/biancode/UA-.NETStandard/actions/runs/37201766371) and [CodeQL run 37201768280](https://github.com/biancode/UA-.NETStandard/actions/runs/37201768280), both green. ## Notes for review - No extra request: the de-duplication happens on the result of the browse the client already made. - Of the sibling companion clients, `PumpsClient.EnumeratePumpsAsync` and `ScalesClient.EnumerateScalesAsync` already de-duplicate across their two roots. `EnumeratePumpsUnderAsync`, `EnumerateScalesUnderAsync`, `EnumerateSystemScalesAsync` and the DI device discovery follow the same browse pattern without de-duplication; they are not changed here. ## Related Issues - Fixes #4670 ## Checklist - [x] 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. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suite on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |
||
|
Klaus Landsdorf
|
dbee7b9988 |
Robotics client: fill the engineering units and ranges of the axis values and the load mass, and the gear ratio (#4661)
# Description Follow-up to #4619. It was prepared stacked on #4619 and is now one commit on top of master, with #4619's review changes in place. `AxisSnapshot.Engineering`, `LoadSnapshot.MassEngineering` and `GearSnapshot.GearRatio` are part of the snapshot contracts and documented in `docs/Robotics.md` (the URML capability manifest takes the joint limits from them), but `RoboticsClient` never set them: every axis came back without units or limits, every load without the unit of its mass and every gear without its ratio. OPC 40010-1 declares `ActualPosition` (mandatory), `ActualSpeed` and `ActualAcceleration` (optional) below the axis's DI `ParameterSet`, and the load's `Mass`, as `AnalogUnitType` variables with a mandatory `EngineeringUnits` and an optional `EURange` property, and `GearRatio` as a mandatory `RationalNumber` variable of the gear. The client now reads them: - **Axis:** `ReadAxisAsync`, and with it `ReadSystemAsync`, resolves the `EngineeringUnits` and `EURange` of the three axis values with one TranslateBrowsePathsToNodeIds and reads them with one Read per axis (none when nothing resolves) into `PositionUnit`, `SpeedUnit`, `AccelerationUnit` and `Limits.Position/Speed/Acceleration`. - **Load:** `Mass` is resolved and read together with its `EngineeringUnits` and `EURange` (`MassEngineering`), replacing the separate resolve and read. - **Gear:** `GearRatio` is read in the same request as `Pitch`. - Loads and gears cost no extra request. - A property the server does not publish, whose read fails or whose value has a bad status stays `null`. The structures go through the existing `ToStructure<T>`, which checks `StatusCode.IsGood` as for `MotorTemperatureEngineering`. The rest of the snapshot is still returned. - Because `Mass` and `Pitch` are now part of that request, a failed `Mass` or `Pitch` read keeps the server's status in the `DataValue`, like the axis values and `MotorTemperature`. Before, `Session.ReadValueAsync` threw and failed the whole snapshot read. The public API does not change. ## Changes - `src/Opc.Ua.Robotics.Client/RoboticsClient.Topology.cs`: - new `ReadAxisEngineeringAsync`, called from `ReadAxisAsync`; - `ReadLoadAsync` and `ReadGearAsync` use the existing `ReadChildrenAsync`. - `docs/Robotics.md`: - describes the members and their null cases; - adds them to the values that carry no NodeId; - names `AxisSnapshot.Engineering` instead of the non-existent `AxisSnapshot.Limits` in the URML section. ## Tests - **New, unit** (`Opc.Ua.Robotics.Tests`, `RoboticsClientFacadeTests`): - `EngineeringValuesAndTheGearRatioComeFromTheVariablesTheyDescribe`: the values are checked against same-named decoys below the axis and a `GearRatio` in namespace 0. - `AbsentOrUnreadableEngineeringValuesAndGearRatiosStayNull`: a missing `EURange`, a load without engineering properties, a failing unit read and a failing ratio read. - `BadQualityEngineeringValuesAndGearRatiosReadAsNoStructure`: a range, a unit and a ratio with a bad status read as `null`. - `AFailedMassOrPitchReadKeepsTheStatusAndTheSnapshot`. - `AnAxisWithoutAParameterSetHasNoEngineeringValues`. - **New, end-to-end** (`RoboticsClientServerE2eTests.EngineeringValuesAndGearRatiosAreReadAsync`): - the test cell now publishes a unit and range for `ActualSpeed`, a unit without a range for `ActualAcceleration`, a range for the flange load's mass and a tool load with a mass only; - the test reads the units, ranges and gear ratios back through `ReadSystemAsync` and `ReadAxisAsync`. - **Fail without the fix:** with `src/Opc.Ua.Robotics.Client` from master, five of the six new tests fail. Four get `null` units, ranges or ratios. `AFailedMassOrPitchReadKeepsTheStatusAndTheSnapshot` fails with `ServiceResultException` `Bad_NodeIdUnknown`. `AnAxisWithoutAParameterSetHasNoEngineeringValues` passes there by design: it pins that an axis without a `ParameterSet` still reads without error. - `Opc.Ua.Robotics.Tests` passed 661/661 on net10.0 and 601/601 on net8.0 (macOS). - `Opc.Ua.Robotics.Client` builds for net48, net8.0, net9.0 and net10.0 with 0 warnings. - `dotnet format` (whitespace, style, analyzers) on the changed files: nothing on the changed lines beyond the `IDE0002` info that the existing `RoboticsSessionHarness.Ua(Opc.Ua.BrowseNames.…)` calls in the same test class also get. - Changed lines covered: 52/52 (100 %). - Fork CI (full matrix incl. macOS) and CodeQL: [CI run 37147764118](https://github.com/biancode/UA-.NETStandard/actions/runs/37147764118) and [CodeQL run 37147766527](https://github.com/biancode/UA-.NETStandard/actions/runs/37147766527). CodeQL is green; CI is green except `Opc.Ua.Interop.Tests` (`LegacyClientExtendedCheckAsync("ComplexTypes")` / `LoadAndDecodeLegacyComplexTypesAsync`), which is flaky on master since #4630 (details in #4617), and except one Windows run of `StartIsIdempotentAndDisposeStopsListenerAsync` in `Opc.Ua.Redundancy.Kubernetes.Tests` (HttpListener prefix already registered), which this change does not touch either. ## Notes for review - `ReadAxisAsync` costs one TranslateBrowsePathsToNodeIds and at most one Read more per axis. The other snapshots keep their request count. - `AxisSnapshot.Engineering`, `LoadSnapshot.MassEngineering` and `GearSnapshot.GearRatio` are read once and carry no NodeId, like `MotorTemperatureEngineering`. ## Related Issues - Fixes #4660 - Follows up #4619 ## Checklist - [x] 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. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suite on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |
||
|
Klaus Landsdorf
|
3690341c80 |
DI: wire the PrepareForUpdate and Installation methods per spec and advertise only the facets that are met (#4617)
# Description
**Software update methods** (`fix(di): wire PrepareForUpdate
Abort/Resume and Installation Resume`)
- New internal `SoftwareUpdateTransitions` (one per `SoftwareUpdate`)
checks the current state and moves it under one lock, so concurrent
calls cannot both pass a check.
- `Prepare` (§8.4.8.3) is accepted only from `Idle`. The application's
`OnPrepare` callback gets a token that `Abort` cancels; the token source
is disposed only after both the Prepare call and a racing Abort are done
with it, so a handler still running after Abort can use its token.
- `Abort` (§8.4.8.4) moves `Preparing` to `Idle` (`PreparingToIdle`),
cancels the preparation in flight and reports `Failed` ("Aborted")
through `OnPrepareStateChanged`; the aborted `Prepare` call returns
`Bad_InvalidState`. Outside `Preparing` it returns `Bad_InvalidState`.
- `Resume` (§8.4.8.5) no longer runs the Prepare callback. It moves
`PreparedForUpdate` through `Resuming` to `Idle` and is rejected with
`Bad_InvalidState` outside `PreparedForUpdate` or while the installation
is `Installing`.
- `Installation.Resume` (§8.4.9) moves `Error` to `Idle`
(`ErrorToIdle`),
`Bad_InvalidState` otherwise.
- The dispatcher knows the `Resuming` state and the `ResumingToIdle`
transition, so `CurrentState` / `LastTransition` show their names.
**Advertising** (`fix(di): advertise only the DI facets whose mandatory
units are met`)
- `DI Offline`, `DI SU UpdateStatus`, `DI SU FileSystem Loading`,
`DI SU Installation for File System` and `DI SU Installation for Cached
Loading` are no longer claimed (see the issue for why each is unmet).
- The facets are derived from the advertised units with the mandatory
units of the DI 1.05 profile database, so a facet cannot be listed
without them. Today this leaves Locking and Software Update Base;
DeviceIntegrationHost, FileSystem, Direct and Cached Loading come back
by themselves once the missing pieces are built.
- `docs/DeviceIntegration.md` documents both tables and the missing
pieces.
Tests: `SoftwareUpdateMethodTransitionTests` (9 tests) and
`DiNodeManagerConformanceTests` (5 tests) are new;
`DiConformanceFacetTests`
and `PumpAddressSpaceComplianceTests` expected the old claims and are
updated. All 9 transition tests and 8 of the 10 conformance tests fail
against master; the other two are the unchanged locking test and a guard
that every advertised facet carries its mandatory units (master
over-claims
the units too, so the guard alone holds there).
Behaviour change for users: a plain DI server no longer lists the
DeviceIntegrationHost facet, and Direct/Cached/Package software updates
list only Software Update Base.
Fork CI (full matrix incl. macOS) and CodeQL: green — [CI run
36779552603](https://github.com/biancode/UA-.NETStandard/actions/runs/36779552603),
[CodeQL run
36779556125](https://github.com/biancode/UA-.NETStandard/actions/runs/36779556125).
## Related Issues
- Fixes #4616
## Checklist
- [x] 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.
- [ ] 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: Marc Schier <marcschier@users.noreply.github.com>
|
||
|
Klaus Landsdorf
|
a3166fa560 |
WebApiClient: report HTTP failures as StatusCodes and accept gzip responses (#4648)
# Description `WebApiClient` threw the exceptions of `HttpClient` for every HTTP failure except 429/503. It now throws `ServiceResultException` with a matching StatusCode, as the transport channels already do. It can also ask for and read gzip compressed JSON (OPC 10000-6 §7.4.5). Two commits: the error mapping, then gzip. ## Changes ### HTTP failures (`src/Opc.Ua.Client/WebApi/WebApiClient.cs`, new internal `WebApiHttpErrors`) | HTTP / failure | StatusCode | |---|---| | 400 | `Bad_DecodingError` | | 401 | `Bad_IdentityTokenInvalid` when the client sent no credentials (bearer token or basic credentials), otherwise `Bad_UserAccessDenied` | | 403 | `Bad_UserAccessDenied` | | 404, 405, 501 | `Bad_ServiceUnsupported` | | 408, 504 | `Bad_Timeout` | | 413 | `Bad_RequestTooLarge` | | 415 | `Bad_DataEncodingUnsupported` | | 429, 503 | `Bad_ServerTooBusy` with the Retry-After hint (unchanged) | | 500 | `Bad_InternalError` | | any other status | `Bad_CommunicationError` | | elapsed `RequestTimeout` / `HttpClient.Timeout` | `Bad_RequestTimeout`, as in `HttpsTransportChannel` and `WebApiTransportChannel` | | failure below HTTP (connection, TLS handshake) | the code `HttpsTransportChannel.MapRequestFailure` assigns, e.g. `Bad_NotConnected` for a refused connection | - The message names the HTTP status, the reason phrase and the route. - For an HTTP status, the inner exception is an `HttpRequestException` naming it (with `StatusCode` set on .NET 5 and later). For a failure below HTTP, it is the original `HttpRequestException`; for a timeout, the `OperationCanceledException`. - A cancellation of the caller's token stays an `OperationCanceledException`. - `WebApiTransportChannel` sends through `WebApiClient`. For an HTTP error status it now reports the mapped code instead of `Bad_UnknownResponse`; its codes for timeouts and connection failures do not change. - `docs/WebApi.md`: new section "Client errors". ### gzip (`WebApiClientOptions.AcceptCompressedResponses`, `src/Opc.Ua.Core/Stack/Https/HttpResponseBodyReader.cs`) - **Requesting gzip:** `AcceptCompressedResponses` (default `false`) sends `Accept-Encoding: gzip`; on a shared `HttpClient` it goes on each request instead of the default headers. `WebApiTransportChannel` passes the option through. - **Reading gzip:** `HttpResponseBodyReader` inflates a body with `Content-Encoding: gzip`. - `MaxMessageSize` applies to the inflated size, so a small compressed body cannot expand past it. - A corrupt gzip body is `Bad_DecodingError`. - A content coding other than gzip or identity is rejected with `Bad_DecodingError` instead of being decoded as if it were plain. - **Stream ownership:** the reader releases only its inflating wrapper. The response stream stays with the `HttpContent` the caller disposes. - `Opc.Ua.Core` references `System.IO.Compression` on net48 explicitly (it compiled through the expanded .NET Standard facades before, but should not depend on them). - `Opc.Ua.Client` grants `InternalsVisibleTo` to `Opc.Ua.Bindings.Https.WebApi.Tests` for the internal shared-`HttpClient` constructor. - `docs/WebApi.md`: new section "Compressed responses". ## Tests - **New:** `WebApiClientErrorAndCompressionTests` in `Opc.Ua.Bindings.Https.WebApi.Tests` (stub `HttpMessageHandler`, no server): - error mapping: `HttpErrorStatusSurfacesAsServiceResultException` (14 cases, every row of the status table), `ThrottledRequestSurfacesAsServerTooBusyWithRetryAfter` (429, 503), `ConnectionFailureMapsLikeTheTransportChannel`, `TlsHandshakeFailureMapsLikeTheTransportChannel`, `ElapsedRequestTimeoutSurfacesAsBadRequestTimeout`, `CallerCancellationStaysOperationCanceled`; - gzip: `AcceptCompressedResponsesAdvertisesGzipAndDecompressesAsync`, `SharedClientCarriesAcceptEncodingPerRequestAsync`, `GzipResponseIsDecompressedEvenWhenNotAskedForAsync`, `GzipBodyIsBoundedByMaxMessageSizeAfterInflating`, `CorruptGzipIsADecodingError`, `UnsupportedContentEncodingIsRejected`, `IdentityContentEncodingIsReadAsIsAsync`, `ResponseStreamIsLeftToItsContentAsync` (plain and gzip). - **Updated** (they expected the old exceptions): - `WebApiClientTests.InvokeAsyncThrowsServiceResultExceptionOnNon2xxStatus` (was `InvokeAsyncThrowsHttpRequestExceptionOnNon2xxStatus`): `Bad_InternalError` with the `HttpRequestException` as inner exception; - `WebApiResponseQuotaRegressionTests.RequestTimeoutRemainsActiveWhileReadingTheOpenApiBodyAsync`: the elapsed timeout is `Bad_RequestTimeout` with the `OperationCanceledException` as inner exception. - **Fail without the fix:** with `WebApiClient.cs` and `HttpResponseBodyReader.cs` from master, 24 of the 29 new test cases fail, as do `InvokeAsyncThrowsServiceResultExceptionOnNon2xxStatus` and three of the six `RequestTimeoutRemainsActiveWhileReadingTheOpenApiBodyAsync` cases. The five that pass there guard behaviour that must not change: throttling (2), caller cancellation, identity coding, and a plain body's stream. With a reader that disposes the response stream, both `ResponseStreamIsLeftToItsContentAsync` cases fail. - `Opc.Ua.Bindings.Https.WebApi.Tests` 436/436 and `Opc.Ua.Client.Tests` 2733/2733 on net10.0 and net8.0 (macOS). - `Opc.Ua.Client` and `Opc.Ua.Core` build for net48, net8.0, net9.0 and net10.0 with 0 warnings; `dotnet format` (whitespace, style, analyzers) is clean on every changed file; changed lines covered 112/112 (100 %) with the two suites above (`.azurepipelines/check-coverage.ps1` against master). - Fork CI (full matrix incl. macOS) and CodeQL: green — [CI run 37128530025](https://github.com/biancode/UA-.NETStandard/actions/runs/37128530025) (154 jobs; one net48 job, `ReconnectAsyncSwapsFaultedLeaseEntryAsync` in `Opc.Ua.Client.Tests`, timed out on the first attempt and passed on rerun; this change does not touch reconnect), [CodeQL run 37128531971](https://github.com/biancode/UA-.NETStandard/actions/runs/37128531971). ## Notes for review - `HttpResponseBodyReader` is shared with the binary `HttpsTransportChannel`, so rejecting unknown content codings applies there too. Before, such a body was decoded as plain bytes and failed later with a less helpful error. - The failure-below-HTTP and timeout codes reuse what the transport channels report, so `WebApiTransportChannel`, which sends through `WebApiClient`, keeps its codes for those cases. Its own `HttpRequestException` path no longer sees exceptions from `WebApiClient`; I left it in place. ## Related Issues - Fixes #4647 ## Checklist - [x] 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. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |
||
|
romanett
|
52f8520baa |
Detect resource and limit abuse in the encoder/decoder fuzz harness (#4666)
## Why The encoder/decoder fuzz targets only flagged crashes, unhandled exceptions and encoding-fidelity mismatches. They could not observe a decode that stays within the exception contract but consumes far more memory, CPU or stack than its input justifies, or that materialises values larger than the context's encoding limits. None of the resource-related findings of the codec audit would have been caught. ## What Adds four shared oracles to the fuzz harness (`fuzzing/Common/Fuzz/FuzzOracles*.cs`), applied by every host (libFuzzer, AFL, `--replay`, Tools playback and the NUnit replay): - **Allocation** — wraps the Binary/JSON/XML decode cores, the built-in-type targets, the PubSub UADP/JSON decoders and the UADP reassembler; fails when a decode allocates more than `64x` its input plus `8 MB`, whether it returns or throws. Uses the per-thread allocation counter (the AppDomain counter on .NET Framework). - **Time** — a per-input budget of `1 s + 100 ms/KB`, re-run twice and reported only if the fastest run is still over, so JIT/GC/CI noise does not trip it (`OPCUA_FUZZ_TIME_BUDGET_SCALE`, `0` disables). - **Stack** — every input runs on a worker thread with a `256 KB` stack, so recursion that comes close to the limit of a normal thread is caught in-process as a crash rather than near 200 levels. `StackTestcases` inputs replay through every target in a child process (`--fuzz-replay-all`), since such a crash cannot be caught. - **Decoded limits** — walks a decoded value for strings, identifiers, ByteStrings and arrays beyond the context's `MaxStringLength`/`MaxByteStringLength`/`MaxArrayLength`; matrices are additionally boxed the legacy way so a value no consumer can materialise is reported. The fuzz context uses `4096/8192/4096` so small inputs reach the limits. Supporting changes: - `ArrayOf<T>`/`MatrixOf<T>` are not `IEnumerable`, so the limit walker unpacks them via `ToArray`/`ToArrayOf` (otherwise it silently traversed nothing). - `Decimal` is registered in the fuzz context so the targets exercise `Decimal.Decode`. - **B1-2 bug fix**: `BinaryDecoder.ReadExtensionObject` dereferenced a null element for an XML body that is not an element; now rejected with `BadDecodingError`. Found by replaying the nightly crash corpus under the new exception contract; 5 corpus inputs are added as gating reproducers. - Reproducers for B1-1, T1-3, X1-1 and U1-3, built in process by documented builders (`ResourceReproducerTests`, `PubSubReproducerTests`) and checked in to the corpus, where every target replays them under the oracles. Each was verified by reverting the matching fix locally and confirming the guard fires (e.g. B1-1 then allocates ~21 MB from 481 input bytes). - `Fuzzing.md` / `CrashCorpus.md` document the oracles and the private-input log policy. ## Testing Green on net10.0 and net48: | Suite | net10.0 | net48 | |---|---|---| | `Opc.Ua.Encoders.Fuzz.Tests` | 8130 | 8130 | | `Opc.Ua.PubSub.Fuzz.Tests` | 149 | n/a (modern-only) | | `Opc.Ua.Fuzzing.Tests` | 154 | 154 | | `Opc.Ua.Types.Tests` (encoders) | 3698 | builds | Also verified: `Network`/`Certificates` fuzz test suites, all fuzz hosts and Tools projects build, the fuzz manifest validator passes, `dotnet format whitespace` is clean, and the full ~22k-input nightly crash corpus replays with no oracle false positives. Stacked on `romanett/encoder-decoder-spec-security-83c9f5`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
2871355d18 |
GDS: implement the AliasName Server facet (OPC 10000-17 Annex C) (#4668)
## Summary Implements the **GDS AliasName Server facet** (OPC 10000-17 Annex C) in `Opc.Ua.Gds.Server`, so that the CTT conformance unit *GDS AliasName Discovery* passes apart from two CTT script defects (documented, Mantis drafts in `plans/ctt-issues.md`). Spec requirements: - **C.1:** aggregate the AliasNames of registered Servers that expose the `ALIAS` capability (OPC 10000-12 Annex D). Pull all AliasNameCategory instances and merge those whose BrowseNames have the same Name. Provide the ExpandedNodeId of every referenced node and the ServerUri of the Server that contains it. - **C.2:** on registration, merge the Server's AliasNames into a single tree for TagVariables/Topics, combine aliases that share a name, and add the ServerUri to `ServerArray`. - **C.3:** on unregistration, remove that Server's aliases and categories (but keep those other Servers still provide), remove its AliasFor references, and remove its ServerUri from `ServerArray`. ### Design - **`AliasNameAggregator`** (`Opc.Ua.Gds.Server.AliasNames`) holds one snapshot per registered application and builds the merged view: - Categories are keyed by their BrowseName path below `Aliases`. Aliases are merged by name within a category; their targets are the union over all sources. - Targets are stored as namespace-URI ExpandedNodeIds. The ServerIndex is resolved against the server's `ServerArray` each time the view is built, because removing a ServerUri renumbers the entries after it. - Only ServerUris the aggregator added itself are removed again. - Also implements `FindAlias`/`FindAliasVerbose`, recursive over sub-categories. - **`SessionAliasNameSourceReader`** reads a source over a client Session by browsing the `Aliases` (i=23470) hierarchy and the AliasFor references. - It browses instead of calling `FindAlias` because the category structure has to be aggregated too, and a source whose aliases come from a NodeSet (like the CTT's embedded server) does not implement `FindAlias`. - It connects anonymously, to the most secure endpoint first, falling back to SecurityMode None. It runs GetEndpoints once per DiscoveryUrl. - **`ApplicationsNodeManager`** (new partial `ApplicationsNodeManager.AliasNames.cs`): - Hooks RegisterApplication (now `OnCallAsync`), UpdateApplication (background re-read) and UnregisterApplication. - Materializes the merged view as `AliasNameType` and `AliasNameCategoryType` nodes in the GDS application-record namespace, organized by the well-known TagVariables/Topics/Aliases objects. Each category gets a `FindAlias` answered from the merged list. - Changes and node updates are serialized, so a read that finishes after a concurrent unregister cannot re-add the source. - RegisterApplication waits at most `AliasNameRegistrationWait` (5 s) for the read; a slower or unreachable source is finished in the background and never blocks clients past their call timeout. - Optional periodic re-read via `AliasNameRefreshInterval`; it also retries sources that could not be read. - **`IAliasNameStoreRegistry.RegisterContributor`/`UnregisterContributor`** (Opc.Ua.Server): a read-only contributor's aliases are added to the `FindAlias`/`FindAliasVerbose` results of the categories it reports, next to the owning store's. This lets the aggregated aliases share the well-known categories with the server's own aliases, which matters for the reference server, an AliasName Server and a GDS at once. Contributors never receive Add/Delete calls. - **Opt-in:** `GlobalDiscoveryServerConfiguration.EnableAliasNameAggregation` is off by default, because the GDS then opens client connections to the DiscoveryUrls of registered applications. `Ctt.ReferenceServer.Config.xml` enables it. - `Opc.Ua.Gds.Server` now references `Opc.Ua.Client`; the `Opc.Ua` meta-package already depends on both. ### CTT (1.05.513 with the patched scripts), p14-gds, reference server `--ctt`, embedded CTT source + 2 extra AliasName sources | GDS AliasName Discovery | before | after | after, with the C54/C55 script fixes | | --- | --- | --- | --- | | initialize | Skipped (no sources) | Pass | Pass | | 001 | Error | Error (C55) | Pass | | 002 | Error | Error (C54) | Warning (RegisterApplication > 100 ms) | | 003 | Pass | Pass | Pass | | 004 | Error | Error (C54) | Pass | | 005 | Skipped | Error (C55) | Pass | | 006, 007, 009, 011, 012, 014, 015 | Skipped | Error (C54) | Pass | | 008 | Skipped | Pass | Pass | | 010, 013 | Skipped | Error (C55) | Pass | - **C54:** the script compares the GDS's AliasFor target (ServerIndex > 0, namespace URI, as Annex C.1 and Part 4 §7.16 require) with the source's local NodeId using `equals()`, which can never match. - **C55:** "no remote aliases" is checked as "no aliases", so it fails on the reference server's own aliases. Both are documented with draft Mantis texts (not filed). The remaining script checks pass in 15/15 test cases. No regressions: - Application Directory and Query Applications are unchanged against the baseline. The only difference is one ActivateSession timing warning in one run. - p09 (AliasName CUs) passes 32/32, identical to the baseline. ## Tests - `AliasNameAggregatorTests` (unit) covers: - merging across Servers; - categories kept while still referenced and removed after the last source; - ServerArray add, remove and renumbering; - pre-existing ServerUris kept; - local targets; - FindAlias/FindAliasVerbose scope, pattern and reference-type filter. - `GdsAliasNameAggregationTests` (end-to-end) registers a second in-process reference server with `ALIAS` and reads it with the real session reader. It checks: - aggregated nodes and remote AliasFor targets; - the ServerArray entry; - FindAlias on TagVariables and on the aggregated category; - that unregistering removes everything; - that an unreachable source still registers; - that a Server without `ALIAS` is ignored. - Registry contributor tests in `AliasNameStoreRegistryTests`. - Opc.Ua.Gds.Tests: 1246 passed, 0 failed, 56 skipped. Opc.Ua.Server.Tests AliasNames: 198/198. InformationModel and Features alias tests pass. net48 compiles. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanett
|
35c08874ca |
Server view parameter status codes; client transfer fallback when GetMonitoredItems is unavailable (#4654)
Two defects found by interop testing the 2.0 stack against foreign OPC
UA stacks (node-opcua, Milo, asyncua, open62541, async-opcua, gopcua).
## 1. Server: view timestamp/version without a ViewId returned
Bad_ViewIdUnknown
A Browse with a `ViewDescription` whose `ViewId` is null but whose
`Timestamp` (or `ViewVersion`) is set got `Bad_ViewIdUnknown` from the
node managers. `ViewDescription.IsDefault` is false, so
`ValidateViewDescription` looked up the null `ViewId` and failed with
"unknown view".
Spec (OPC 10000-4):
- §7.44: a null `viewId` means the entire AddressSpace. Timestamp and
viewVersion each select a version of the View, and "either the Timestamp
or the viewVersion parameter may be set by a Client, but not both".
- Table 178 / Browse service results (§5.9.2.3):
`Bad_ViewTimestampInvalid` = "the view timestamp is not available or not
supported", `Bad_ViewVersionInvalid` likewise, and
`Bad_ViewParameterMismatch` = the view parameters are not consistent
with each other.
Fix: a new internal `ViewDescriptionValidator.ValidateParameters`
handles the checks that don't depend on the View node:
- timestamp **and** version set → `Bad_ViewParameterMismatch` (before
this, a known view got `Bad_ViewTimestampInvalid`)
- null ViewId + timestamp → `Bad_ViewTimestampInvalid`
- null ViewId + version → `Bad_ViewVersionInvalid`
It is called at service level in
`NodeManagerServiceDispatcher.BrowseAsync`, because these are Browse
*service* results. It is also called in
`CustomNodeManager2.ValidateViewDescription`,
`AsyncCustomNodeManager.ValidateViewDescription` and the
`DiagnosticsNodeManager` override, so a node manager called directly
answers the same way. TranslateBrowsePaths and RegisterNodes take no
ViewDescription, and BrowseNext reuses the validated view from the
continuation point.
## 2. Client: transfer failed when the optional Server.GetMonitoredItems
is unavailable
After `TransferSubscriptions` returns Good, the client calls
`Server.GetMonitoredItems` (i=11492) to map server handles to client
handles. That call is only made for a subscription that is not live in
this client: one restored from saved state, or a clone. Its own live
subscriptions skip it. `GetMonitoredItems` is an **Optional** method of
ServerType (OPC 10000-5 §6.3.1, §9.1), and several stacks don't provide
it: asyncua 2.0.1 answers `BadNothingToDo`, open62541 built without
`UA_ENABLE_XML_ENCODING` answers `BadInternalError`, and others answer
`BadMethodInvalid` or `BadNotSupported`. When the call failed, the whole
transfer was reported as failed ("SubscriptionId N could not be moved")
even though the server had moved the subscription. For non-"definitive"
codes such as BadNothingToDo or BadInternalError, the transferred server
subscription was also left behind as an orphan.
### Chosen fallback
The status of a failed call is now classified
(`GetMonitoredItemsFallback.IsTransientFailure`):
- **Transient** (timeout, busy server, too many operations, lost
connection, channel or session): behaviour is unchanged. The transfer
fails and nothing is deleted, so it can be retried.
- **Anything else** (method missing, not supported, BadNothingToDo,
BadInternalError, unusable output): the server does not provide the
method. Then:
1. **The client knows server ids for its items** (the usual case:
restored state, or a reconnect of its own subscription). Use that
mapping and the transfer succeeds. A transfer keeps item ids and client
handles, which is why OPC 10000-4 §6.8 tells clients to store the
SubscriptionId together with the MonitoredItem client and server
handles. Items saved without an id were never created on the server, and
the normal apply path creates them, as before.
2. **No server ids are known** (a clone of a live subscription,
`CloneSubscriptions`). Notifications would still arrive, because clones
copy the client handles. But the items could never be modified or
deleted, and recreating them would duplicate them on the server, since
stale items can't be deleted without their ids. So:
- classic engine, when the caller just moved the subscription
(`Session.TransferSubscriptionsAsync`): delete the transferred server
subscription and **recreate it in place** with the same items and client
handles. The transfer reports success, but notifications the server
still had queued for the old subscription are lost. The subscription
gets a new id.
- classic `Subscription.TransferAsync` (also used to adopt the session's
own subscriptions, so nothing was moved): fails as before, and nothing
is deleted or created.
- new engine (`MonitoredItemManager.TrySynchronizeHandlesAsync`): fails
as before. Its callers (`SubscriptionManager`) already recreate the
subscription in that case.
Both engines now treat an unusable GetMonitoredItems answer (wrong
output arguments) the same way as an unavailable method. The two
existing
`TryCompleteTransferAsyncShouldReturnFalseWhenResponseWrong{1,2}Async`
tests were updated and renamed to match. The existing classic test that
expected a "definitive" failure to delete the subscription now checks
the fallback to the known ids instead.
Warnings are logged when the fallback is used (`Subscription + 71..73`,
`MonitoredItemManager + 6`).
## Tests
- Server:
`AsyncCustomNodeManagerTests.BrowseAsyncNullViewIdWithVersionParametersReturnsViewParameterErrorAsync`
(sync and async node manager, timestamp, version, both) and
`MasterNodeManagerDeterministicTests.BrowseAsyncNullViewIdWithVersionParametersThrowsViewParameterError`
(service level).
- Classic client (`ClassicSubscriptionCoverageTests`): fallback to known
ids for every non-transient code, while transient codes still fail; a
Bad method result (asyncua style); clone recreated in place (old
subscription deleted, items created with the original client handles);
clone adopted through `TransferAsync` is neither deleted nor recreated.
- New engine (`SubscriptionTests`): known ids used for Bad /
BadMethodInvalid / BadNotSupported / BadNothingToDo / BadInternalError;
transient failure and unknown ids still fail.
Run locally (net10.0, sequential):
- Opc.Ua.Client.Tests: 2745 passed / 0 failed
- Opc.Ua.Server.Tests: 8089 passed / 0 failed / 5 skipped
- Opc.Ua.Subscriptions.Durable.Tests: 36 passed / 11 skipped
- Opc.Ua.InformationModel.Tests (View/Browse filter): 289 passed / 4
skipped
- Opc.Ua.Sessions.Tests (Transfer/Reconnect filter): 155 passed / 103
skipped
- Opc.Ua.Subscriptions.Tests (Transfer/Durable filter): 15 passed / 1
skipped
- net48 build of both test projects: clean
The interop test that found (2),
`ServerFeatureInteropTests.TransferSubscriptionToNewSessionAsync`, is in
the uncommitted foreign-interop work and isn't on master yet.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
|
||
|
romanett
|
3bb0cd8d55 |
Fix ComplexTypes interop and TCP channel dispose CI flakes (#4663)
# Description
Fixes two intermittent CI failures on master.
## Interop: `LegacyClientExtendedCheckAsync("ComplexTypes")` /
`LoadAndDecodeLegacyComplexTypesAsync`
Flaky since #4630. The node reported as "not decoded" is always
`Data_*_UserScalar_VariantValue` (ns=5;i=2340 on the 2.0 reference
server, ns=3;i=3104 on the 1.5 one). Its DataType is the
BaseDataType-derived `VariantDataType`, and `TestDataSystem` fills it
with `DataGenerator.GetRandomVariant()`. About 2.4 % of the time that
draw is an `ExtensionObject` with a random TypeId (e.g. `s=香蕉`) that no
client can decode. Both checks counted every ExtensionObject of a
variable with a non-ns0 DataType as a structure.
**Fix:** both checks now count only variables whose DataType derives
from `Structure`. They browse the `HasSubtype` closure of `Structure` to
decide this.
- In the 2.0 test, the browse-with-continuation loop moved into a shared
`BrowseAsync` helper.
- In the 1.5 peer, the existing Structure-subtype walk in
`DiagnoseEncodingNodesAsync` moved into `BrowseStructureSubtypesAsync`,
which both places now use.
Seen in master CI for
|
||
|
romanett
|
6f55500577 |
Server diagnostics: let every session browse its own subscriptions in SubscriptionDiagnosticsArray (#4664)
# Description `Server.ServerDiagnostics.SubscriptionDiagnosticsArray` denied Browse and Read to every session except a SecurityAdmin on a SignAndEncrypt channel. A regular session, such as the CTT's default SecurityMode None session, got `Bad_UserAccessDenied` when it browsed the array, so it could not find its own subscription diagnostics. Part 5 §6.3.5 lets a session see its own diagnostics, and the CTT maintainer asked that the server return only the entries a session may see instead of blocking the whole array ([Mantis 11445](https://mantis.opcfoundation.org/view.php?id=11445)). **Change** (`DiagnosticsNodeManager.OnReadUserRolePermissions`): non-admin sessions now get `Browse` on the server-wide `SubscriptionDiagnosticsArray`. Each subscription diagnostics node already carries the permissions of its owning session, and Browse drops references to targets the caller may not browse. So a session browsing the array gets only its own subscriptions, and an administrator gets all of them. **Intentionally unchanged:** - Reading the array **value** stays admin-only. The value holds every session's subscriptions. Making it readable with per-session filtering would also need two other fixes: within one second of a diagnostics scan, `OnReadDiagnosticsArray` returns the stored, unfiltered array, and monitored items on the array receive the unfiltered scan value. - `SessionDiagnosticsArray`, `SessionSecurityDiagnosticsArray` and `ServerDiagnosticsSummary` keep their permissions. No session nodes are linked under `SessionDiagnosticsArray`, so opening Browse there would return nothing. **Note for reviewers:** CTT `Subscription Durable/012.js` now gets past the Browse and Read in Step 3, but it still fails because of a script bug (missing braces on line 37 set `result = false` unconditionally). That part is a CTT fix tracked in Mantis 11445. ## Related Issues - Mantis 11445 (CTT Subscription Durable 012.js, Step 3) ## Tests - New end-to-end test `DiagnosticsTests.SubscriptionDiagnosticsArrayBrowseReturnsOnlyOwnSubscriptionsAsync`. Two anonymous SecurityMode None sessions each create a subscription. The fixture session's Browse returns only its own subscription, reading the array value returns `BadUserAccessDenied`, and the sysadmin session sees both subscriptions. Against the old permissions, the test fails with `BadUserAccessDenied` on the Browse. - `DiagnosticsNodeManagerTests.ReadUserRolePermissions_AdminAndSessionHandlingAsync` now also checks that a non-admin gets Browse only on the subscription array, an admin can Read it, and the two session arrays still give a non-admin no permissions. - Local net10.0: `DiagnosticsTests` + `BaseInfoEnabledFlagToggleTests` 14/14 passed, `DiagnosticsNodeManagerTests` 22/22 passed. The test projects build on net48. ## Checklist - [x] 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. - [ ] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Klaus Landsdorf
|
89bbc3bd75 |
Server: public CreateAlarm<TState> fluent overload, and conformance units that follow their contributors (#4650)
# Description Two changes to `Opc.Ua.Server`, one commit each. Both are independent of any companion model; the Asset Management Basics contribution (#4649) is the first code that needs them, but any companion node manager can use them. **1. `CreateAlarm<TState>` on `INodeBuilder`.** - The fluent alarm surface offers `CreateLimitAlarm`, `CreateExclusiveLimitAlarm` and `CreateOffNormalAlarm`. The pipeline behind them (`AttachAlarm`) is private, and `FluentNodeRegistration` is internal. - A node manager that needs any other condition type cannot reach that pipeline from outside the assembly. Examples are an alarm type of a companion specification, a `TripAlarmType` or `DiscrepancyAlarmType`, or a server-specific subtype that implements an interface such as `IRootCauseIndicationType`. Such a node manager has to copy the attach, notifier and release logic, or it loses the automatic release on teardown. - `CreateAlarm<TState>(INodeBuilder, QualifiedName, Func<NodeState, TState>)` exposes the same pipeline for a state the caller's factory creates. The alarm is initialized, enabled and attached with `HasCondition`. The parent becomes its `SourceNode`, `SourceName` and event notifier. A fluent node manager releases all of it on teardown. - A factory that returns `null` fails with `InvalidOperationException` before anything is changed. The three existing helpers never take that path, so they behave as before. **2. `ConformanceUnitsManager` recomputes the advertised units from its contributors.** - On master the manager reads a contributor once, in `Register`, and adds its units and profiles to two sets that only grow. - A unit or profile that a contributor stops reporting therefore stays in `ConformanceUnits` and `ServerProfileArray` until the server restarts. One that it starts reporting after registration is never published. - The ServerCapabilities describe the current configuration of the server (OPC 10000-5 §6.3.2), so both have to follow the contributor. This matters for contributors whose support changes at runtime, for example a node manager whose units depend on the instances registered with it, or `HistorianConformanceContributor` after `RefreshAsync`. - The manager now keeps the registered contributors and reads all of them again on every `PublishAsync`: - `Register` is idempotent; - `Unregister` is new; - publishing is serialized (`SemaphoreSlim`), so the address space ends with the set read last; - the contributors are read outside the manager's lock. - `DiagnosticsNodeManager.PublishConformanceUnitsAsync` keeps the profiles that were in `ServerProfileArray` before the first publish, which are the configured ones, and replaces the contributed ones on every publish. **Behaviour change for custom contributors.** A profile or unit that no contributor reports any more is now withdrawn on the next publish. Registering a contributor twice no longer reads it twice; the result is the same set as before. A contributor whose sets are fixed at startup sees no difference. The DI, Machinery, Pumps, Scales, Robotics and Vision node managers register once at startup and publish the same sets as before (see Tests). ## Changes - `src/Opc.Ua.Server/Fluent/AlarmBuilderExtensions.cs`: `CreateAlarm<TState>`; the null check on the factory result in `AttachAlarm`. - `src/Opc.Ua.Server/Server/ConformanceUnitsManager.cs`: contributor list, `Unregister`, recompute in `PublishAsync`, publish gate, `Dispose` releases the gate. - `src/Opc.Ua.Server/Diagnostics/DiagnosticsNodeManager.cs`: configured profiles captured on the first publish, contributed profiles replaced. `IDiagnosticsNodeManager.cs` and `HistorianConformanceContributor.cs`: documentation only. - `docs/NodeManagers.md`: `CreateAlarm<TState>` next to the dedicated helpers. `docs/Profiles.md`: how contributed units and profiles are published and withdrawn. ## Tests - **New, part 1** (`Opc.Ua.Server.Tests`, `Fluent`): - `AlarmBuilderExtensionsTests`: `CreateAlarmAttachesTheFactoryStateLikeTheDedicatedHelpers`, `CreateAlarmAcceptsAConditionThatIsNoAlarm`, `CreateAlarmNullArgsThrow`, `CreateAlarmRejectsAFactoryWithoutAnAlarm`, `CreateAlarmOnVariableThrowsBadTypeMismatch`; - `FluentAlarmRegistrationIntegrationTests.CreateAlarmIsBrowsableAndFindableAsync`; - `NodeAttachBuilderExtensionsTests`: `GenericAlarmWiringIsUndoneOnTeardownAsync` (release on teardown). - **New, part 2** (`Opc.Ua.Server.Tests`): - `ConformanceUnitsManagerTests`: `PublishWithdrawsWhatAContributorNoLongerReportsAsync`, `RegisteringAgainReadsTheContributorOnceAsync`, `UnregisterWithdrawsTheUnitsOfTheContributorAsync`; - `DiagnosticsNodeManagerTests.PublishConformanceUnits_KeepsDeclaredProfilesAndWithdrawsContributedOnesAsync`. - **Fail without the change:** with the `src/` files of master, the part 1 tests and `UnregisterWithdrawsTheUnitsOfTheContributorAsync` do not compile (`CreateAlarm` and `Unregister` do not exist). With that test left out, `PublishWithdrawsWhatAContributorNoLongerReportsAsync` (the withdrawn unit is still published), `RegisteringAgainReadsTheContributorOnceAsync` (the contributor is not read on publish) and `PublishConformanceUnits_KeepsDeclaredProfilesAndWithdrawsContributedOnesAsync` (the contributed profile stays) fail; the seven existing `ConformanceUnitsManagerTests` pass. - **Existing contributors:** the conformance, facet, profile and compliance tests of the companion suites pass unchanged on net10.0: `Opc.Ua.Di.Tests` 33, `Opc.Ua.Machinery.Tests` 19, `Opc.Ua.Scales.Tests` 5, `Opc.Ua.Robotics.Tests` 128 (incl. `RobotIntentHostingComplianceTests`), `Opc.Ua.Vision.Tests` 9 (`VisionNodeManagerIntegrationTests`), and the whole `Opc.Ua.Pumps.Tests` suite (73). They check the units, facets and profiles each node manager advertises against what it built, and pass as they do on master. - `Opc.Ua.Server.Tests`: 7966 passed / 9 skipped / 0 failed on net10.0 (full suite); the affected tests (`Fluent`, `ConformanceUnitsManagerTests`, `DiagnosticsNodeManagerTests`, the historian and other conformance tests, 934) pass on net10.0 and net8.0 (macOS). - `Opc.Ua.Server` builds for net48, net8.0, net9.0 and net10.0 without new warnings (the only one, CA2213 in `SamplingGroup.cs`, is on master since #4629); `dotnet format` (whitespace, style, analyzers) reports nothing on the changed files; changed lines covered 70/70 (100 %) by `.azurepipelines/check-coverage.ps1` with the `Opc.Ua.Server.Tests` run on net10.0. - Fork CI (full matrix incl. macOS) and CodeQL: [CI run 37135986886](https://github.com/biancode/UA-.NETStandard/actions/runs/37135986886) and [CodeQL run 37135988487](https://github.com/biancode/UA-.NETStandard/actions/runs/37135988487). CodeQL is green; CI is green except `test-windows-net48 (11/32)`, the `Opc.Ua.Interop.Tests` legacy ComplexTypes check ("1 of N structures were not decoded") that came with #4630 and fails the same way on other open pull requests; this change does not touch ComplexTypes. ## Notes for review - The two commits are independent. If you prefer to discuss the behaviour change of part 2 separately, part 1 can go in on its own. - `ConformanceUnitsManager` stays a non-sealed public class, as on master. - The Robotics Intent and Vision node managers also merge their facet URIs into `ServerProfileArray` directly, at startup and when a controller host is added. They report the same URIs through `IConformanceContributor`, so a later publish keeps them. The entries they write before the first publish count as configured, as everything else that is in the array at that point. - `IServerInternal` does not expose the manager since the service-locator cleanup (#4184). A contributor that changes at runtime reaches it through `ServerInternalData.ConformanceUnitsManager`. If you would rather offer a narrower hook (for example a `RepublishConformanceAsync` on the server), we can add it in this PR. ## Related Issues - Part of #4649 ## Checklist - [x] 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. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. (Affected suites on net10.0 and net8.0 locally on macOS; .NET Framework is covered by the fork CI's Windows net48 legs.) - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> |