From 0f509fbd3ac94c4c3759a77d80ab961821022658 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 25 Apr 2026 20:10:59 -0400 Subject: [PATCH] =?UTF-8?q?Auto:=20opcuaclient-6=20=E2=80=94=20Discovery?= =?UTF-8?q?=20URL=20FindServers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds optional `DiscoveryUrl` knob to OpcUaClientDriverOptions. When set, the driver runs `DiscoveryClient.CreateAsync` + `FindServersAsync` + `GetEndpointsAsync` against that URL during InitializeAsync and prepends the discovered endpoint URLs (filtered to matching SecurityPolicy + SecurityMode) to the failover candidate list. De-duplicates URLs that appear in both discovered and static lists (case-insensitive). Discovery failures are non-fatal — falls back to statically configured candidates. The doc comment notes that FindServers requires SecurityMode=None on the discovery channel per OPC UA spec, even when the data channel uses Sign or SignAndEncrypt. Closes #278 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../OpcUaClientDriver.cs | 136 +++++++++++++++++- .../OpcUaClientDriverOptions.cs | 28 ++++ .../OpcUaClientFailoverTests.cs | 79 ++++++++++ 3 files changed, 239 insertions(+), 4 deletions(-) diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs index 04b2cdc..cd289b7 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -127,7 +127,29 @@ public sealed class OpcUaClientDriver(OpcUaClientDriverOptions options, string d try { var appConfig = await BuildApplicationConfigurationAsync(cancellationToken).ConfigureAwait(false); - var candidates = ResolveEndpointCandidates(_options); + + // When DiscoveryUrl is set, run FindServers + GetEndpoints first and merge the + // discovered URLs into the candidate list before the failover sweep. Discovery + // failures are non-fatal: log + fall through to the statically configured + // candidates so a transient LDS outage doesn't block init. + IReadOnlyList discovered = []; + if (!string.IsNullOrWhiteSpace(_options.DiscoveryUrl)) + { + try + { + discovered = await DiscoverEndpointsAsync( + appConfig, _options.DiscoveryUrl!, _options.SecurityPolicy, _options.SecurityMode, + cancellationToken).ConfigureAwait(false); + } + catch (Exception) + { + // Swallow + continue with static candidates; the failover sweep error + // (if all static candidates also fail) will surface the situation. + discovered = []; + } + } + + var candidates = ResolveEndpointCandidates(_options, discovered); var identity = BuildUserIdentity(_options); @@ -390,10 +412,116 @@ public sealed class OpcUaClientDriver(OpcUaClientDriverOptions options, string d /// non-empty; otherwise fall back to EndpointUrl as a single-URL shortcut so /// existing single-endpoint configs keep working without migration. /// - internal static IReadOnlyList ResolveEndpointCandidates(OpcUaClientDriverOptions opts) + internal static IReadOnlyList ResolveEndpointCandidates(OpcUaClientDriverOptions opts) => + ResolveEndpointCandidates(opts, []); + + /// + /// Resolve the ordered failover candidate list with optional discovery results. + /// Discovered URLs are prepended to the static candidate list so a discovery + /// sweep gets first-attempt priority over hand-rolled fallbacks. When the static + /// list is empty (no AND only + /// the default ), the discovered + /// URLs replace the static candidate entirely so a pure-discovery deployment doesn't + /// need a hard-coded fallback URL. Duplicates are removed (case-insensitive on the + /// URL string) so a discovered URL that also appears in EndpointUrls isn't + /// attempted twice in a row. + /// + internal static IReadOnlyList ResolveEndpointCandidates( + OpcUaClientDriverOptions opts, + IReadOnlyList discovered) { - if (opts.EndpointUrls is { Count: > 0 }) return opts.EndpointUrls; - return [opts.EndpointUrl]; + var staticList = opts.EndpointUrls is { Count: > 0 } + ? (IReadOnlyList)opts.EndpointUrls + : [opts.EndpointUrl]; + + if (discovered.Count == 0) return staticList; + + // Discovered first; merge static after with case-insensitive de-dup so a single + // server that appears in both lists doesn't cause two consecutive identical attempts. + var seen = new HashSet(StringComparer.OrdinalIgnoreCase); + var merged = new List(discovered.Count + staticList.Count); + foreach (var u in discovered) + if (!string.IsNullOrWhiteSpace(u) && seen.Add(u)) + merged.Add(u); + foreach (var u in staticList) + if (!string.IsNullOrWhiteSpace(u) && seen.Add(u)) + merged.Add(u); + return merged; + } + + /// + /// Run OPC UA discovery against : FindServers + /// enumerates every server registered with the LDS (or just the one server when + /// points at a server directly), then + /// GetEndpoints on each server's discovery URL pulls its full endpoint list. + /// Endpoints are filtered to those matching the requested policy + mode before being + /// returned. + /// + /// + /// SecurityMode=None on the discovery channel is mandated by the OPC UA spec — + /// discovery is unauthenticated even when the steady-state session uses Sign or + /// SignAndEncrypt. DiscoveryClient.CreateAsync opens an unsecured channel by + /// default; we don't override that here. + /// + internal static async Task> DiscoverEndpointsAsync( + ApplicationConfiguration appConfig, + string discoveryUrl, + OpcUaSecurityPolicy policy, + OpcUaSecurityMode mode, + CancellationToken ct) + { + var wantedPolicyUri = MapSecurityPolicy(policy); + var wantedMode = mode switch + { + OpcUaSecurityMode.None => MessageSecurityMode.None, + OpcUaSecurityMode.Sign => MessageSecurityMode.Sign, + OpcUaSecurityMode.SignAndEncrypt => MessageSecurityMode.SignAndEncrypt, + _ => throw new ArgumentOutOfRangeException(nameof(mode)), + }; + + var results = new List(); + var seen = new HashSet(StringComparer.OrdinalIgnoreCase); + + // FindServers against the LDS / server discovery endpoint. Returned ApplicationDescriptions + // each carry one or more DiscoveryUrls (typically one per network interface). + ApplicationDescriptionCollection servers; + using (var lds = await DiscoveryClient.CreateAsync( + appConfig, new Uri(discoveryUrl), DiagnosticsMasks.None, ct).ConfigureAwait(false)) + { + servers = await lds.FindServersAsync(null, ct).ConfigureAwait(false); + } + + foreach (var server in servers) + { + if (server.DiscoveryUrls is null) continue; + foreach (var serverDiscoveryUrl in server.DiscoveryUrls) + { + if (string.IsNullOrWhiteSpace(serverDiscoveryUrl)) continue; + EndpointDescriptionCollection endpoints; + try + { + using var ep = await DiscoveryClient.CreateAsync( + appConfig, new Uri(serverDiscoveryUrl), DiagnosticsMasks.None, ct).ConfigureAwait(false); + endpoints = await ep.GetEndpointsAsync(null, ct).ConfigureAwait(false); + } + catch + { + // One unreachable server in the LDS list shouldn't blow up the whole + // sweep — skip it and keep going. + continue; + } + + foreach (var e in endpoints) + { + if (e.SecurityPolicyUri != wantedPolicyUri) continue; + if (e.SecurityMode != wantedMode) continue; + if (string.IsNullOrWhiteSpace(e.EndpointUrl)) continue; + if (seen.Add(e.EndpointUrl)) results.Add(e.EndpointUrl); + } + } + } + + return results; } /// diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverOptions.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverOptions.cs index 8563150..f571c83 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverOptions.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverOptions.cs @@ -39,6 +39,34 @@ public sealed class OpcUaClientDriverOptions /// public TimeSpan PerEndpointConnectTimeout { get; init; } = TimeSpan.FromSeconds(3); + /// + /// Optional discovery URL pointing at a Local Discovery Server (LDS) or a server's + /// own discovery endpoint. When set, the driver runs FindServers + + /// GetEndpoints against this URL during + /// and prepends the discovered endpoint URLs to the failover candidate list. When + /// is empty (and only is set as + /// a fallback), the discovered URLs replace the candidate list entirely so a + /// discovery-driven deployment can be configured without specifying any endpoints + /// up front. Discovery failures are non-fatal — the driver logs and falls back to the + /// statically configured candidates. + /// + /// + /// + /// FindServers requires SecurityMode=None on the discovery channel per the + /// OPC UA spec — discovery is unauthenticated even when the data channel uses + /// Sign or SignAndEncrypt. The driver opens the discovery channel + /// unsecured regardless of ; only the resulting data + /// session is bound to the configured policy. + /// + /// + /// Endpoints returned by discovery are filtered to those matching + /// + before being added to + /// the candidate list, so a discovery sweep against a multi-policy server only + /// surfaces endpoints the driver could actually connect to. + /// + /// + public string? DiscoveryUrl { get; init; } + /// /// Security policy to require when selecting an endpoint. Either a /// enum constant or a free-form string (for diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientFailoverTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientFailoverTests.cs index 73edb82..0e4cf2b 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientFailoverTests.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientFailoverTests.cs @@ -55,6 +55,85 @@ public sealed class OpcUaClientFailoverTests "pre-connect the dashboard should show the first candidate URL so operators can link back"); } + [Fact] + public void DiscoveryUrl_defaults_null_so_existing_configs_are_unaffected() + { + var opts = new OpcUaClientDriverOptions(); + opts.DiscoveryUrl.ShouldBeNull(); + } + + [Fact] + public void ResolveEndpointCandidates_prepends_discovered_urls_before_static_candidates() + { + var opts = new OpcUaClientDriverOptions + { + EndpointUrls = ["opc.tcp://static1:4840", "opc.tcp://static2:4841"], + }; + var discovered = new[] { "opc.tcp://discovered1:4840", "opc.tcp://discovered2:4841" }; + var list = OpcUaClientDriver.ResolveEndpointCandidates(opts, discovered); + list.Count.ShouldBe(4); + list[0].ShouldBe("opc.tcp://discovered1:4840"); + list[1].ShouldBe("opc.tcp://discovered2:4841"); + list[2].ShouldBe("opc.tcp://static1:4840"); + list[3].ShouldBe("opc.tcp://static2:4841"); + } + + [Fact] + public void ResolveEndpointCandidates_dedupes_url_appearing_in_both_discovered_and_static() + { + var opts = new OpcUaClientDriverOptions + { + EndpointUrls = ["opc.tcp://shared:4840", "opc.tcp://static:4841"], + }; + var discovered = new[] { "opc.tcp://shared:4840", "opc.tcp://only-discovered:4842" }; + var list = OpcUaClientDriver.ResolveEndpointCandidates(opts, discovered); + list.Count.ShouldBe(3); + list[0].ShouldBe("opc.tcp://shared:4840"); + list[1].ShouldBe("opc.tcp://only-discovered:4842"); + list[2].ShouldBe("opc.tcp://static:4841"); + } + + [Fact] + public void ResolveEndpointCandidates_dedup_is_case_insensitive() + { + // Discovery URLs sometimes return uppercase hostnames; static config typically has + // lowercase. The de-dup should treat them as the same URL so the failover sweep + // doesn't attempt the same host twice in a row. + var opts = new OpcUaClientDriverOptions + { + EndpointUrls = ["opc.tcp://host:4840"], + }; + var discovered = new[] { "OPC.TCP://HOST:4840" }; + var list = OpcUaClientDriver.ResolveEndpointCandidates(opts, discovered); + list.Count.ShouldBe(1); + } + + [Fact] + public void ResolveEndpointCandidates_with_only_default_endpoint_is_replaced_by_discovery() + { + // No EndpointUrls list, default EndpointUrl — the static "candidate" is the default + // localhost shortcut. When discovery returns URLs they should still be prepended + // (the localhost default isn't worth filtering out specially since it's harmless to + // try last and it's still a valid configured fallback). + var opts = new OpcUaClientDriverOptions(); // EndpointUrl=opc.tcp://localhost:4840 default + var discovered = new[] { "opc.tcp://discovered:4840" }; + var list = OpcUaClientDriver.ResolveEndpointCandidates(opts, discovered); + list[0].ShouldBe("opc.tcp://discovered:4840"); + list.ShouldContain("opc.tcp://localhost:4840"); + } + + [Fact] + public void ResolveEndpointCandidates_no_discovered_falls_back_to_static_behaviour() + { + var opts = new OpcUaClientDriverOptions + { + EndpointUrls = ["opc.tcp://only:4840"], + }; + var list = OpcUaClientDriver.ResolveEndpointCandidates(opts, []); + list.Count.ShouldBe(1); + list[0].ShouldBe("opc.tcp://only:4840"); + } + [Fact] public async Task Initialize_against_all_unreachable_endpoints_throws_AggregateException_listing_each() {