From b75e013d0ad8a0d83cc70f46799016e72a4d1606 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 7 Oct 2026 21:34:48 -0400 Subject: [PATCH] =?UTF-8?q?feat(abcip):=20final=20review=20I1=20=E2=80=94?= =?UTF-8?q?=20node-level=20Drivers:AbCip:DefaultTransport?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old AdminUI cannot author the per-driver transport key (#576), so the 'pin before upgrading' recipe had no working path. A node-level appsetting (env Drivers__AbCip__DefaultTransport) now supplies the default for AbCip drivers with no key: explicit key > node default > compiled Native. Bound and validated unconditionally (bad value fails host start); the driver factory and the Test Connect probe both read it so they agree. AbCip.md upgrade recipe rewritten around the key; Configuration.md documents it. Also documents first-wins for AllowPacking/probe and host-alias throttles (final review M6) and moves the status-bit note out of AbCip.md (M5). Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- docs/Configuration.md | 25 +++ docs/drivers/AbCip.md | 56 ++++--- .../AbCipDriverOptions.cs | 3 +- .../AbCipDriver.cs | 12 +- .../AbCipDriverFactoryExtensions.cs | 19 ++- .../AbCipDriverProbe.cs | 15 +- .../AbCipTransportResolver.cs | 45 +++++- .../Drivers/Forms/AbCipDriverForm.razor | 4 +- .../AbCipDriverDefaultsOptions.cs | 58 +++++++ .../Drivers/DriverFactoryBootstrap.cs | 25 ++- src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs | 7 + .../AbCipTransportSwitchTests.cs | 69 ++++++++ .../AbCipDriverDefaultsOptionsTests.cs | 153 ++++++++++++++++++ 13 files changed, 447 insertions(+), 44 deletions(-) create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/AbCipDriverDefaultsOptions.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Configuration/AbCipDriverDefaultsOptionsTests.cs diff --git a/docs/Configuration.md b/docs/Configuration.md index ceac456a..4f2d478b 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -445,6 +445,30 @@ for the enablement runbook (ApiKey handling, stop/start-together rule, tombstone never-`sqlite3`-a-live-WAL-DB inspection rules) and the "pair-local config cache" section of [`docs/Redundancy.md`](Redundancy.md) for what boot-from-cache does and does not cover. +### `Drivers:AbCip` (node-level AbCip defaults) + +- **Purpose:** pin the wire transport an **AbCip driver with no `transport` key** runs on, for every such + driver on this node, without editing driver configs (final review I1 of the native AB transport). The + per-driver `DriverConfig` key `transport` still wins when present. The main use is an upgrade: the old + AdminUI cannot author the per-driver key (its typed form drops unknown keys, #576), so this is the way to + keep a fleet on libplctag across the upgrade restart. See + [`drivers/AbCip.md` § Transport](drivers/AbCip.md#transport-transport-option). +- **Options class:** `AbCipDriverDefaultsOptions` (`SectionName = "Drivers:AbCip"`) — + `src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/AbCipDriverDefaultsOptions.cs`. Bound and validated + unconditionally in `Program.cs` (`AddValidatedOptions` + `ValidateOnStart`), because both consumers need + it: the driver factory (driver role) and the AdminUI Test Connect probe (admin role), so the probe + exercises the stack the driver would run. + +| Key | Type | Default | Meaning | +|---|---|---|---| +| `DefaultTransport` | string | `Native` | `Native` or `Libplctag`, case-insensitive. Blank/absent = `Native`. **Any other value fails host start** (`AbCipDriverDefaultsOptionsValidator`, naming the key), including numeric (`1`) and flags (`Native,Libplctag`) forms. Env form: `Drivers__AbCip__DefaultTransport`. | + +Precedence for an AbCip driver: explicit `transport` in its `DriverConfig` → this key → compiled `Native`. +Like the per-driver key, it takes effect when a driver instance is **constructed** (process start, or a +deploy that recreates the driver), not on `ReinitializeAsync`. The init log line reads +`transport=Libplctag (node default)` when the pin is what selected the stack. There is **no AbLegacy +counterpart**: an AbLegacy driver with no key always runs libplctag (Native is opt-in per driver). + --- ## Environment variables @@ -458,6 +482,7 @@ All names are read in this repo's source via `Environment.GetEnvironmentVariable | `OTOPCUA_ROLES` | `EffectiveRoles.Resolve` (`ZB.MOM.WW.OtOpcUa.Cluster`) — the single role authority both `Program.cs` and `AddOtOpcUaCluster` consume | Comma-separated cluster roles for the node (`admin`, `driver`, `dev`, `cluster-{ClusterId}`), read as the **fallback** only when `Cluster:Roles` is empty. Drives the conditional wiring, the per-role appsettings overlay, and the node's Akka member roles — all from the same resolved instance. See the note under `Cluster:Roles` above. | | `OTOPCUA_CONFIG_CONNECTION` | `src/Core/ZB.MOM.WW.OtOpcUa.Configuration/DesignTimeDbContextFactory.cs` (design-time / `dotnet ef` only) | Read at **design time** by `DesignTimeDbContextFactory.cs` for `dotnet ef` migrations. At **runtime** the server resolves the connection string from `ConnectionStrings:ConfigDb` (env form: `ConnectionStrings__ConfigDb`) via `configuration.GetConnectionString("ConfigDb")` in `ServiceCollectionExtensions.cs` — `OTOPCUA_CONFIG_CONNECTION` appears there only as a hint in an error message, not via `GetEnvironmentVariable`. No credential is embedded in source. | | `ASPNETCORE_ENVIRONMENT` | ASP.NET host builder (framework) | Selects `appsettings.{Environment}.json` (e.g. `Development`). | +| `Drivers__AbCip__DefaultTransport` | configuration binder (`AbCipDriverDefaultsOptions`) | Env form of [`Drivers:AbCip:DefaultTransport`](#driversabcip-node-level-abcip-defaults): `Native` (default) or `Libplctag` for AbCip drivers with no `transport` key. A bad value fails host start. | ### Historian (`ServerHistorian__ApiKey`) diff --git a/docs/drivers/AbCip.md b/docs/drivers/AbCip.md index 837bce56..12044105 100644 --- a/docs/drivers/AbCip.md +++ b/docs/drivers/AbCip.md @@ -89,23 +89,28 @@ The driver has two interchangeable wire transports, selected by one `DriverConfi | Key | Values | Default | Serialization | |---|---|---|---| -| `transport` | absent \| `Native` \| `Libplctag` | absent ⇒ `Native` | string, case-insensitive on read, written as `"Libplctag"` / `"Native"`; "use the default" is encoded by OMITTING the key | +| `transport` | absent \| `Native` \| `Libplctag` | absent ⇒ the node's `Drivers:AbCip:DefaultTransport` (itself default `Native`) | string, case-insensitive on read, written as `"Libplctag"` / `"Native"`; "use the default" is encoded by OMITTING the key | - **`Native`** (the default since Task 22) is a managed in-process EtherNet/IP + CIP client (`Driver.Ab.Transport`). - **`Libplctag`** (fallback) drives the libplctag native library, as before. - An absent key means "use the default". It is resolved in one place, `AbCipTransportResolver`, which the - driver, the Test Connect probe and the CLI all go through. An explicit value always wins. + driver, the Test Connect probe and the CLI all go through. **Precedence:** an explicit per-driver key → + the node-level appsetting **`Drivers:AbCip:DefaultTransport`** (`Native` | `Libplctag`, env + `Drivers__AbCip__DefaultTransport`; a bad value fails host start; see + [Configuration.md](../Configuration.md#driversabcip-node-level-abcip-defaults)) → compiled `Native`. The + Test Connect probe uses the same node value, so it probes the stack the driver would run. The CLI has no + node config and uses compiled `Native`. - A bad value fails like a bad `PlcFamily`; the Test Connect probe reports "Config JSON is invalid". - A change takes effect when the driver instance is **constructed**: on every process start (so the upgrade restart moves no-key drivers to Native — see the release note) and when a deploy that changes `DriverConfig` does this (`DriverHostActor` respawns the child, #516). `ReinitializeAsync` alone keeps the transport it was constructed with. -- AdminUI: the **Transport** dropdown in the advanced section of the AbCip driver form ("Default (Native)" / - "Native (managed)" / "libplctag (fallback)"). **Default omits the key on save**, so a defaulted driver stays +- AdminUI: the **Transport** dropdown in the advanced section of the AbCip driver form ("Default (node + setting; Native unless pinned)" / "Native (managed)" / "libplctag (fallback)"). **Default omits the key on save**, so a defaulted driver stays defaulted; saving another field preserves an explicit value and preserves absence. - The driver logs `transport= (default|configured)` at Information on every init, e.g. - `transport=Native (default)`, including reconnect re-inits. AbCip + `transport=Native (default)` or `transport=Libplctag (node default)`, including reconnect re-inits. AbCip drivers now receive the host logger (they previously ran on a null logger, so none of their warnings reached the node log); expect previously invisible warnings, e.g. a probe enabled with no probe tag path. - CLI: `--transport default|native|libplctag` (default `default`, which goes through the same resolver). @@ -119,19 +124,32 @@ The driver has two interchangeable wire transports, selected by one `DriverConfi - ⚠️ **Client-visible:** native reports **`BadTypeMismatch`** for a mis-authored same-width type (for example a tag authored `DINT` where the controller holds a `REAL`), where libplctag returned Good garbage. Correct the authored type; see [Native transport behaviour](#native-transport-behaviour). -- **To fall back**, set `"transport": "Libplctag"` in the driver config (or pick "libplctag (fallback)" in - the AdminUI dropdown). +- **To fall back** one driver, set `"transport": "Libplctag"` in its driver config (or pick "libplctag + (fallback)" in the AdminUI dropdown). To fall back a whole node, set `Drivers:AbCip:DefaultTransport` to + `Libplctag` (below). - ⚠️ **When it takes effect: the upgrade restart itself.** Every process start constructs the drivers - (including boot-from-cache on a driver-only node), so **every driver with no `transport` key moves to Native - the moment the upgraded node starts** — no deploy is needed. To keep a driver on libplctag, add - `"transport": "Libplctag"` and deploy it **before upgrading**: older builds ignore the unknown key. After the - upgrade, a transport change takes effect when a deploy recreates the driver (not on `ReinitializeAsync`). + (including boot-from-cache on a driver-only node), so **every driver with no `transport` key moves to the + node default — `Native` unless pinned — the moment the upgraded node starts**; no deploy is needed. After the + upgrade, a per-driver transport change takes effect when a deploy recreates the driver (not on + `ReinitializeAsync`). +- **To stay on libplctag across the upgrade (pin before upgrading):** set the node-level key on each node + you upgrade, in the same change that installs the new build — `appsettings.json` + `"Drivers": { "AbCip": { "DefaultTransport": "Libplctag" } }`, or the environment variable + `Drivers__AbCip__DefaultTransport=Libplctag` on the service. Older builds ignore the section, so it can be + staged ahead of the binaries. Every AbCip driver with no `transport` key then starts on libplctag; the init + log shows `transport=Libplctag (node default)`. Move to Native later per driver (explicit + `"transport": "Native"` in the new AdminUI) or for the node (remove the key / set `Native`, then restart). + ⚠️ Do **not** rely on adding `"transport": "Libplctag"` to driver configs through the **old** AdminUI before + upgrading: its typed AbCip form has no Transport field and drops unknown keys on save (#576), and on a fused + `admin,driver` node the AdminUI and drivers upgrade together, so there is no window. The node key works on + every node shape, including fused nodes and driver-only nodes booting from cache. - **Evidence, and its limits:** AbCip **reads** (plus browse and templates) were proven on a production CompactLogix 5069-L320ERS2 (**Z-35162**, read-only, 2026-10-08; see [gate results](../plans/2026-10-07-native-ab-transport-gate-results.md) §Real press). AbCip **writes** are - simulator-proven only (`ab_server`), as is all of AbLegacy. -- AbLegacy has its own exclusion: a defaulted AbLegacy driver with any PLC-5 or `logixpccc` device stays on - libplctag (see [AbLegacy.md §Transport](AbLegacy.md#transport-transport-option)). + simulator-proven only (`ab_server`). +- **AbLegacy is different: its default stays libplctag** and Native is opt-in per driver; the native PCCC + path has wire evidence (on `ab_server`) only for N-file reads/writes and one F8 read (see + [AbLegacy.md §Transport](AbLegacy.md#transport-transport-option)). ## Native transport behaviour @@ -140,8 +158,12 @@ The driver has two interchangeable wire transports, selected by one `DriverConfi request in flight; large reads (listing pages, Read Template) are sent unpacked and exclusive. - **Forward Open throttle:** at most one attempt in flight, at least 1 s between starts, bounded by the open budget. -- `ConnectionSize` / `Timeout` are honoured per pooled connection, and the **first** driver/device to open - a given device key wins. ControlLogix, GuardLogix and (since T21) CompactLogix default to 4002, a Large +- `ConnectionSize` / `Timeout` / `AllowPacking` are honoured per pooled connection, and the **first** + driver/device to open a given device key wins — including a Test Connect probe on a fused `admin,driver` + node (its connection carries the probe timeout). A later opener with different options is ignored and + logged once as a Warning (`DeviceSessionPool`). ⚠️ The device key compares the **host string**, so the + same controller addressed by DNS name in one driver and by IP in another gets two connections and two + Forward Open throttles (up to two opens per second); address a controller the same way everywhere. ControlLogix, GuardLogix and (since T21) CompactLogix default to 4002, a Large Forward Open (0x5B), the same as libplctag. A target that refuses Large gets one regular open at min(size, 504) inside the same throttled attempt. An explicit `ConnectionSize` in the options or on the device overrides the family default. @@ -157,8 +179,6 @@ The driver has two interchangeable wire transports, selected by one `DriverConfi writes are gated on a mismatch too. - Malformed tag names put the device into backoff, as under libplctag (#570). STRING capacity includes the alignment pad (84, not 82) (#570). -- Status-bit sub-element writes (e.g. `T4:0.DN`) are Unsupported natively (they would clobber the control - word); nothing goes on the wire. Sub-element arrays are Unsupported natively. - **A refused operation is not a comms fault.** Any operation a runtime refuses as Unsupported (in AbCip, e.g. a native write whose type cannot be determined) reaches the client as `BadNotSupported`, keeps the tag handle, and leaves driver health unchanged; only real transport failures evict and degrade. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs index 295491ee..93d5a636 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs @@ -34,7 +34,8 @@ public sealed class AbCipDriverOptions /// /// Which transport stack the driver, its Test Connect probe and the CLI use. null (the - /// transport key absent) means "use the default", which is ; + /// transport key absent) means "use the default": the node-level + /// Drivers:AbCip:DefaultTransport appsetting, which itself defaults to ; /// an explicit value always wins. The effective transport is resolved in one place, /// AbCipTransportResolver. Set to fall back. /// Changing this takes effect when the driver instance is RECREATED, not on diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index 1e3ad79a..1d7af2ac 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -141,19 +141,23 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// Optional factory for enumerating tags; when null the resolved transport's is used. /// Optional factory for reading UDT templates; when null the resolved transport's is used. /// Optional logger; uses null logger if not provided. + /// The node-level default for an absent transport key + /// (Drivers:AbCip:DefaultTransport); null means the compiled . public AbCipDriver(AbCipDriverOptions options, string driverInstanceId, IAbCipTagFactory? tagFactory = null, IAbCipTagEnumeratorFactory? enumeratorFactory = null, IAbCipTemplateReaderFactory? templateReaderFactory = null, - ILogger? logger = null) + ILogger? logger = null, + AbCipTransport? defaultTransport = null) { ArgumentNullException.ThrowIfNull(options); _options = options; _driverInstanceId = driverInstanceId; // Chosen once here and held across ReinitializeAsync (Shutdown+Initialize): a changed // Transport in a reinit config is inert; a deploy recreates the instance instead. - _transport = AbCipTransportResolver.Resolve(options); - _transportDescription = AbCipTransportResolver.Describe(options.Transport); + var nodeDefault = defaultTransport ?? AbCipTransportResolver.Default; + _transport = AbCipTransportResolver.Resolve(options, nodeDefault); + _transportDescription = AbCipTransportResolver.Describe(options.Transport, nodeDefault); var defaults = AbCipTransportFactories.For(_transport); _tagFactory = tagFactory ?? defaults.Tags; _enumeratorFactory = enumeratorFactory ?? defaults.Enumerators; @@ -182,7 +186,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// The effective transport resolved at construction (see ). /// Describes the OPTION only: injected factories (tests) are used regardless. - internal AbCipTransport EffectiveTransport => _transport; + public AbCipTransport EffectiveTransport => _transport; // The transport chosen at construction (see the ctor); logged at every init so an operator can tell // from the node log which stack serves this instance. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverFactoryExtensions.cs index db8d501a..66236c62 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverFactoryExtensions.cs @@ -24,10 +24,14 @@ public static class AbCipDriverFactoryExtensions /// /// The driver factory registry to register with. /// Optional logger factory for driver instances. - public static void Register(DriverFactoryRegistry registry, ILoggerFactory? loggerFactory = null) + /// The node-level transport for drivers whose config has no + /// transport key (Drivers:AbCip:DefaultTransport, final review I1). An explicit + /// per-driver key still wins. + public static void Register(DriverFactoryRegistry registry, ILoggerFactory? loggerFactory = null, + AbCipTransport defaultTransport = AbCipTransportResolver.Default) { ArgumentNullException.ThrowIfNull(registry); - registry.Register(DriverTypeName, (id, json) => CreateInstance(id, json, loggerFactory)); + registry.Register(DriverTypeName, (id, json) => CreateInstance(id, json, loggerFactory, defaultTransport)); } /// @@ -43,18 +47,21 @@ public static class AbCipDriverFactoryExtensions /// The unique identifier for this driver instance. /// The driver configuration as a JSON string. /// Optional logger factory for the driver instance. + /// The node-level default for an absent transport key. /// A configured instance. internal static AbCipDriver CreateInstance( - string driverInstanceId, string driverConfigJson, ILoggerFactory? loggerFactory) + string driverInstanceId, string driverConfigJson, ILoggerFactory? loggerFactory, + AbCipTransport defaultTransport = AbCipTransportResolver.Default) { ArgumentException.ThrowIfNullOrWhiteSpace(driverInstanceId); var options = ParseOptions(driverInstanceId, driverConfigJson); - return new AbCipDriver(options, driverInstanceId, logger: loggerFactory?.CreateLogger()); + return new AbCipDriver(options, driverInstanceId, logger: loggerFactory?.CreateLogger(), + defaultTransport: defaultTransport); } /// /// Deserialise an AB CIP driver-config JSON document into . - /// Shared by (first construction) and + /// Shared by (first construction) and /// / /// so a reinitialize with a changed config JSON (new device, new tag, changed timeout) /// actually takes effect rather than being silently discarded. @@ -155,7 +162,7 @@ public static class AbCipDriverFactoryExtensions internal sealed class AbCipDriverConfigDto { /// - /// Gets or sets the transport (Libplctag or Native, case-insensitive); absent means Libplctag. + /// Gets or sets the transport (Libplctag or Native, case-insensitive); absent means the node default. /// public string? Transport { get; init; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs index a6baf074..d0c21db7 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs @@ -34,11 +34,22 @@ public sealed class AbCipDriverProbe : IDriverProbe }; private readonly IAbCipTagFactory? _tagFactory; + private readonly AbCipTransport _defaultTransport; /// Initializes a new instance of the class. /// Factory for the probe's tag runtime; when null the config's /// Transport selects the native or libplctag-backed one. - public AbCipDriverProbe(IAbCipTagFactory? tagFactory = null) => _tagFactory = tagFactory; + /// The node-level default for a config with no transport key — the + /// SAME value the node's driver factory uses, so Test Connect probes the stack the driver would run. + public AbCipDriverProbe(IAbCipTagFactory? tagFactory = null, + AbCipTransport defaultTransport = AbCipTransportResolver.Default) + { + _tagFactory = tagFactory; + _defaultTransport = defaultTransport; + } + + /// The node-level default this probe resolves an absent key to (wiring tests). + public AbCipTransport DefaultTransport => _defaultTransport; /// public string DriverType => "AbCip"; @@ -123,7 +134,7 @@ public sealed class AbCipDriverProbe : IDriverProbe ConnectionSize: firstDevice?.ConnectionSize ?? profile.DefaultConnectionSize, UseConnectedMessaging: profile.SupportsConnectedMessaging); - var rt = (_tagFactory ?? AbCipTransportFactories.For(AbCipTransportResolver.Resolve(opts)).Tags).Create(p); + var rt = (_tagFactory ?? AbCipTransportFactories.For(AbCipTransportResolver.Resolve(opts, _defaultTransport)).Tags).Create(p); try { await rt.InitializeAsync(cts.Token); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipTransportResolver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipTransportResolver.cs index 07a7f889..f633e761 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipTransportResolver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipTransportResolver.cs @@ -5,32 +5,61 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; /// . The driver constructor, the Test Connect probe and the /// CLI (through the driver) all resolve through here, so they cannot disagree about what an absent /// transport key means. +/// Precedence (final review I1): an explicit per-driver transport key wins; otherwise the +/// node-level default (Drivers:AbCip:DefaultTransport, read in the Host composition root and passed +/// in as nodeDefault); otherwise the compiled . /// public static class AbCipTransportResolver { - /// The transport an absent / null transport key selects. + /// The compiled default: the transport an absent key selects when no node default is configured. public const AbCipTransport Default = AbCipTransport.Native; - /// Returns the effective transport: an explicit value always wins; null means . + /// Returns the effective transport: an explicit value always wins; null means . /// The configured transport, or null for "use the default". + /// The node-level default for drivers with no key (the compiled unless pinned). /// The transport the driver runs on. - public static AbCipTransport Resolve(AbCipTransport? configured) => configured ?? Default; + public static AbCipTransport Resolve(AbCipTransport? configured, AbCipTransport nodeDefault = Default) => + configured ?? nodeDefault; /// Returns the effective transport for . /// The driver options. + /// The node-level default for drivers with no key. /// The transport the driver runs on. - public static AbCipTransport Resolve(AbCipDriverOptions options) + public static AbCipTransport Resolve(AbCipDriverOptions options, AbCipTransport nodeDefault = Default) { ArgumentNullException.ThrowIfNull(options); - return Resolve(options.Transport); + return Resolve(options.Transport, nodeDefault); + } + + /// + /// Parses a transport name the way the driver-config transport key and the node-level default + /// are parsed: a defined enum NAME, case-insensitive. Numeric strings ("0", "7") and the flags form + /// ("Native,Libplctag") are rejected — Enum.TryParse alone accepts both. + /// + /// The raw value. + /// The parsed transport. + /// True when names a transport. + public static bool TryParse(string? raw, out AbCipTransport transport) + { + transport = Default; + if (string.IsNullOrWhiteSpace(raw)) return false; + // Exact name match only: Enum.TryParse would also accept "1" and the flags form "Native,Libplctag". + var name = Enum.GetNames() + .FirstOrDefault(n => string.Equals(n, raw.Trim(), StringComparison.OrdinalIgnoreCase)); + return name is not null && Enum.TryParse(name, out transport); } /// /// Operator-facing description of the configured-vs-effective transport for the init log line: - /// Native (default) when the key was absent, the bare transport name when it was explicit. + /// Native (default) when the key was absent and the compiled default applies, + /// Libplctag (node default) when the key was absent and the node pinned a different default, + /// Native (configured) when the key was explicit. /// /// The configured transport, or null. + /// The node-level default. /// A short description. - public static string Describe(AbCipTransport? configured) => - configured is null ? $"{Resolve(configured)} (default)" : $"{configured} (configured)"; + public static string Describe(AbCipTransport? configured, AbCipTransport nodeDefault = Default) => + configured is not null ? $"{configured} (configured)" + : nodeDefault == Default ? $"{nodeDefault} (default)" + : $"{nodeDefault} (node default)"; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/AbCipDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/AbCipDriverForm.razor index d817277b..86bb6c7b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/AbCipDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/AbCipDriverForm.razor @@ -35,11 +35,11 @@
- + -
Which protocol stack talks to the controller. Default omits the setting so the driver picks; takes effect on the next deploy.
+
Which protocol stack talks to the controller. Default omits the setting so the node-level Drivers:AbCip:DefaultTransport picks (Native unless an operator pinned it); takes effect on the next deploy.
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/AbCipDriverDefaultsOptions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/AbCipDriverDefaultsOptions.cs new file mode 100644 index 00000000..c9cc37d5 --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/AbCipDriverDefaultsOptions.cs @@ -0,0 +1,58 @@ +using ZB.MOM.WW.Configuration; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +namespace ZB.MOM.WW.OtOpcUa.Host.Configuration; + +/// +/// Node-level defaults for the AbCip driver (final review I1), bound from Drivers:AbCip. Lets an +/// operator pin the transport an AbCip driver with NO transport key runs on — at upgrade time, +/// without editing (or being able to edit, #576) every driver config. An explicit per-driver +/// transport key still wins. Overridable by environment variable +/// Drivers__AbCip__DefaultTransport. +/// +public sealed class AbCipDriverDefaultsOptions +{ + /// The configuration section path. + public const string SectionName = "Drivers:AbCip"; + + /// + /// Native (the default) or Libplctag, case-insensitive. Blank/absent = Native. + /// Any other value fails host start (). + /// + public string? DefaultTransport { get; set; } = nameof(AbCipTransport.Native); + + /// + /// The parsed default: blank resolves to the compiled . + /// Throws on an unparseable value — unreachable at runtime because the validator fails host start first. + /// + /// The node-level default transport. + public AbCipTransport ResolveDefaultTransport() + { + if (string.IsNullOrWhiteSpace(DefaultTransport)) return AbCipTransportResolver.Default; + return AbCipTransportResolver.TryParse(DefaultTransport, out var t) + ? t + : throw new InvalidOperationException(AbCipDriverDefaultsOptionsValidator.Message(DefaultTransport)); + } +} + +/// Fail-fast startup validator for . +public sealed class AbCipDriverDefaultsOptionsValidator : OptionsValidatorBase +{ + /// The operator-facing failure text for a bad value. + /// The rejected value. + /// The message. + internal static string Message(string? value) => + $"Drivers:AbCip:DefaultTransport is '{value}' — expected one of " + + $"{string.Join(", ", Enum.GetNames())} (case-insensitive). It selects the transport an " + + "AbCip driver with no \"transport\" key runs on; set it via appsettings or the environment variable " + + "Drivers__AbCip__DefaultTransport."; + + /// + protected override void Validate(ValidationBuilder builder, AbCipDriverDefaultsOptions options) + { + builder.RequireThat( + string.IsNullOrWhiteSpace(options.DefaultTransport) + || AbCipTransportResolver.TryParse(options.DefaultTransport, out _), + Message(options.DefaultTransport)); + } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Drivers/DriverFactoryBootstrap.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Drivers/DriverFactoryBootstrap.cs index 94b8b31c..fd2ec172 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Drivers/DriverFactoryBootstrap.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Drivers/DriverFactoryBootstrap.cs @@ -1,10 +1,12 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Core.Hosting; using ZB.MOM.WW.OtOpcUa.Core.Resilience; using ZB.MOM.WW.Secrets.Abstractions; +using ZB.MOM.WW.OtOpcUa.Host.Configuration; using ZB.MOM.WW.OtOpcUa.Runtime.Redundancy; namespace ZB.MOM.WW.OtOpcUa.Host.Drivers; @@ -78,7 +80,8 @@ public static class DriverFactoryBootstrap // whole mechanism exists to prevent, and it would fail as a plant outage rather than as a // startup error. var primaryRoleSource = sp.GetRequiredService(); - Register(registry, loggerFactory, secretResolver, primaryRoleSource, scriptRoot); + Register(registry, loggerFactory, secretResolver, primaryRoleSource, ResolveAbCipDefaultTransport(sp), + scriptRoot); return registry; }); services.AddSingleton(sp => @@ -140,7 +143,12 @@ public static class DriverFactoryBootstrap public static IServiceCollection AddOtOpcUaDriverProbes(this IServiceCollection services) { services.TryAddEnumerable(ServiceDescriptor.Singleton()); - services.TryAddEnumerable(ServiceDescriptor.Singleton()); + // Factory-built so the probe resolves an absent "transport" key to the SAME node default the driver + // factory uses (Drivers:AbCip:DefaultTransport, final review I1) — Test Connect probes the stack the + // driver would run. The generic factory overload keeps AbCipProbe as the implementation type, which + // TryAddEnumerable de-dups on. + services.TryAddEnumerable(ServiceDescriptor.Singleton( + sp => new AbCipProbe(tagFactory: null, defaultTransport: ResolveAbCipDefaultTransport(sp)))); services.TryAddEnumerable(ServiceDescriptor.Singleton()); services.TryAddEnumerable(ServiceDescriptor.Singleton()); services.TryAddEnumerable(ServiceDescriptor.Singleton()); @@ -155,6 +163,16 @@ public static class DriverFactoryBootstrap return services; } + /// + /// The node-level AbCip default transport (Drivers:AbCip:DefaultTransport, bound + validated in + /// Program.cs). A container without the options bound (test graphs) gets the compiled default. + /// + /// The service provider. + /// The transport an AbCip driver with no transport key runs on. + internal static Driver.AbCip.AbCipTransport ResolveAbCipDefaultTransport(IServiceProvider sp) => + sp.GetService>()?.Value.ResolveDefaultTransport() + ?? Driver.AbCip.AbCipTransportResolver.Default; + /// /// Invoke every cross-platform driver's Register extension. New driver assemblies /// get added here — one line per type. ShouldStub() in DriverInstanceActor still @@ -166,9 +184,10 @@ public static class DriverFactoryBootstrap ILoggerFactory? loggerFactory, ISecretResolver secretResolver, IDriverPrimaryRoleSource primaryRoleSource, + Driver.AbCip.AbCipTransport abCipDefaultTransport, ZB.MOM.WW.OtOpcUa.Core.Scripting.ScriptRootLogger? scriptRoot = null) { - Driver.AbCip.AbCipDriverFactoryExtensions.Register(registry, loggerFactory); + Driver.AbCip.AbCipDriverFactoryExtensions.Register(registry, loggerFactory, abCipDefaultTransport); Driver.AbLegacy.AbLegacyDriverFactoryExtensions.Register(registry, loggerFactory); Driver.Calculation.CalculationDriverFactoryExtensions.Register(registry, loggerFactory, scriptRoot); Driver.FOCAS.FocasDriverFactoryExtensions.Register(registry); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs index 6d18a34e..a2723220 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs @@ -175,6 +175,13 @@ builder.Services.AddOtOpcUaCluster(builder.Configuration); // the startup check (AddOtOpcUaAuth binds LdapOptions but does not attach a validator). builder.Services.AddValidatedOptions(builder.Configuration, LdapOptions.SectionName); +// Node-level AbCip default transport (final review I1): the transport an AbCip driver with NO "transport" +// key runs on. Unconditional because BOTH consumers need it — the driver factory (driver role) and the +// Test Connect probe (admin role, which must probe the stack the driver would run). A bad value fails host +// start here rather than silently falling back. +builder.Services.AddValidatedOptions( + builder.Configuration, AbCipDriverDefaultsOptions.SectionName); + if (hasDriver) { builder.Services.AddOtOpcUaRuntime(); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTransportSwitchTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTransportSwitchTests.cs index 24db4996..b8bfb43c 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTransportSwitchTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipTransportSwitchTests.cs @@ -191,4 +191,73 @@ public sealed class AbCipTransportSwitchTests r.Ok.ShouldBeFalse(); r.Message!.ShouldContain("Config JSON is invalid"); } + + /// + /// Final review I1 precedence: explicit key > node default > compiled Native. The pinned rows are + /// the falsification rows — a resolver that ignores the node default returns Native for them. + /// + [Theory] + [InlineData(null, AbCipTransport.Libplctag, AbCipTransport.Libplctag)] + [InlineData(null, AbCipTransport.Native, AbCipTransport.Native)] + [InlineData(AbCipTransport.Native, AbCipTransport.Libplctag, AbCipTransport.Native)] + [InlineData(AbCipTransport.Libplctag, AbCipTransport.Native, AbCipTransport.Libplctag)] + public void Explicit_key_beats_node_default_beats_compiled_default( + AbCipTransport? configured, AbCipTransport nodeDefault, AbCipTransport expected) + { + AbCipTransportResolver.Resolve(configured, nodeDefault).ShouldBe(expected); + var json = configured is null ? Config(null) : Config($"\"{configured}\""); + AbCipDriverFactoryExtensions.CreateInstance("ab1", json, loggerFactory: null, defaultTransport: nodeDefault) + .EffectiveTransport.ShouldBe(expected); + } + + /// The init-log rendering distinguishes the compiled default from a node pin. + [Fact] + public void Describe_distinguishes_compiled_default_node_default_and_configured() + { + AbCipTransportResolver.Describe(null).ShouldBe("Native (default)"); + AbCipTransportResolver.Describe(null, AbCipTransport.Libplctag).ShouldBe("Libplctag (node default)"); + AbCipTransportResolver.Describe(AbCipTransport.Native, AbCipTransport.Libplctag).ShouldBe("Native (configured)"); + } + + [Theory] + [InlineData("Native", true)] + [InlineData("libplctag", true)] + [InlineData(" Libplctag ", true)] + [InlineData("1", false)] + [InlineData("Native,Libplctag", false)] + [InlineData("", false)] + [InlineData("Bogus", false)] + public void TryParse_accepts_names_only(string raw, bool ok) => + AbCipTransportResolver.TryParse(raw, out _).ShouldBe(ok); + + /// + /// The probe honours its node default for an absent key: pinned to Libplctag it does NOT produce the + /// native failure text against the same listener the unpinned probe does (falsification control). + /// + [Fact] + public async Task Probe_with_no_transport_uses_the_node_default() + { + using var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + var port = ((IPEndPoint)listener.LocalEndpoint).Port; + _ = Task.Run(async () => + { + while (true) + { + try { (await listener.AcceptTcpClientAsync(TestContext.Current.CancellationToken)).Dispose(); } + catch { return; } + } + }, TestContext.Current.CancellationToken); + + var cfg = $$"""{"devices":[{"hostAddress":"ab://127.0.0.1:{{port}}/1,0"}]}"""; + var pinned = await new AbCipDriverProbe(defaultTransport: AbCipTransport.Libplctag) + .ProbeAsync(cfg, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); + var unpinned = await new AbCipDriverProbe() + .ProbeAsync(cfg, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); + listener.Stop(); + + unpinned.Message!.ShouldContain("CIP handshake failed: Connection lost"); + pinned.Ok.ShouldBeFalse(); + pinned.Message!.ShouldNotContain("Connection lost"); + } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Configuration/AbCipDriverDefaultsOptionsTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Configuration/AbCipDriverDefaultsOptionsTests.cs new file mode 100644 index 00000000..66d23e97 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/Configuration/AbCipDriverDefaultsOptionsTests.cs @@ -0,0 +1,153 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Shouldly; +using Xunit; +using ZB.MOM.WW.Configuration; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Core.Hosting; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip; +using ZB.MOM.WW.OtOpcUa.Host.Configuration; +using ZB.MOM.WW.OtOpcUa.Host.Drivers; +using ZB.MOM.WW.Secrets.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Host.Tests.Configuration; + +/// +/// Final review I1 — the node-level Drivers:AbCip:DefaultTransport: binding (appsettings and the +/// __ env-var form), precedence (explicit per-driver key > node default > compiled Native), the +/// probe agreeing with the driver factory, and fail-fast validation. Every precedence test runs a pinned +/// node (Libplctag) against the compiled default (Native), so a wiring that ignores the node value goes red. +/// +[Trait("Category", "Unit")] +public sealed class AbCipDriverDefaultsOptionsTests +{ + private const string NoKey = """{"devices":[{"hostAddress":"ab://10.0.0.1/1,0"}],"probe":{"enabled":false}}"""; + private const string ExplicitNative = + """{"transport":"Native","devices":[{"hostAddress":"ab://10.0.0.1/1,0"}],"probe":{"enabled":false}}"""; + + private static ServiceProvider Build(IConfiguration configuration) + { + var services = new ServiceCollection(); + services.AddLogging(); + services.AddSingleton(new StubSecretResolver()); + services.AddValidatedOptions( + configuration, AbCipDriverDefaultsOptions.SectionName); + services.AddOtOpcUaDriverFactories(); + return services.BuildServiceProvider(); + } + + private static IConfiguration Config(string? value) => + new ConfigurationBuilder() + .AddInMemoryCollection(value is null + ? [] + : new Dictionary { ["Drivers:AbCip:DefaultTransport"] = value }) + .Build(); + + private static AbCipDriver Create(ServiceProvider sp, string json) => + (AbCipDriver)sp.GetRequiredService().TryGet(DriverTypeNames.AbCip)!("drv-i1", json); + + [Fact] + public void Absent_section_defaults_to_Native() + { + using var sp = Build(Config(null)); + sp.GetRequiredService>().Value.ResolveDefaultTransport() + .ShouldBe(AbCipTransport.Native); + Create(sp, NoKey).EffectiveTransport.ShouldBe(AbCipTransport.Native); + } + + [Theory] + [InlineData("Libplctag", AbCipTransport.Libplctag)] + [InlineData("libplctag", AbCipTransport.Libplctag)] + [InlineData("NATIVE", AbCipTransport.Native)] + [InlineData("", AbCipTransport.Native)] + public void The_appsettings_key_binds(string value, AbCipTransport expected) + { + using var sp = Build(Config(value)); + sp.GetRequiredService>().Value.ResolveDefaultTransport() + .ShouldBe(expected); + } + + /// The documented env-var override form, Drivers__AbCip__DefaultTransport. + [Fact] + public void The_double_underscore_environment_variable_binds() + { + const string prefix = "OTOPCUA_I1_TEST_"; + Environment.SetEnvironmentVariable(prefix + "Drivers__AbCip__DefaultTransport", "Libplctag"); + try + { + var configuration = new ConfigurationBuilder().AddEnvironmentVariables(prefix).Build(); + using var sp = Build(configuration); + Create(sp, NoKey).EffectiveTransport.ShouldBe(AbCipTransport.Libplctag); + } + finally + { + Environment.SetEnvironmentVariable(prefix + "Drivers__AbCip__DefaultTransport", null); + } + } + + /// A pinned node moves no-key drivers; an explicit per-driver key still wins over the pin. + [Fact] + public void Node_default_applies_only_to_drivers_without_a_key() + { + using var sp = Build(Config("Libplctag")); + Create(sp, NoKey).EffectiveTransport.ShouldBe(AbCipTransport.Libplctag); + Create(sp, ExplicitNative).EffectiveTransport.ShouldBe(AbCipTransport.Native); + } + + /// The Test Connect probe resolves an absent key to the SAME node default the driver does. + [Theory] + [InlineData("Libplctag", AbCipTransport.Libplctag)] + [InlineData(null, AbCipTransport.Native)] + public void The_probe_uses_the_node_default(string? value, AbCipTransport expected) + { + using var sp = Build(Config(value)); + var probe = sp.GetServices().OfType().ShouldHaveSingleItem(); + probe.DefaultTransport.ShouldBe(expected); + Create(sp, NoKey).EffectiveTransport.ShouldBe(expected); // driver and probe agree + } + + /// The probe stays registered exactly once on a fused node (driver path + admin path). + [Fact] + public void The_factory_built_probe_is_still_deduplicated() + { + var services = new ServiceCollection(); + services.AddOtOpcUaDriverProbes(); + services.AddOtOpcUaDriverProbes(); + using var sp = services.BuildServiceProvider(); + sp.GetServices().OfType().Count().ShouldBe(1); + } + + [Theory] + [InlineData("Bogus")] + [InlineData("1")] + [InlineData("7")] + [InlineData("Native,Libplctag")] + public void A_bad_value_fails_validation_with_a_named_message(string value) + { + var result = new AbCipDriverDefaultsOptionsValidator().Validate( + null, new AbCipDriverDefaultsOptions { DefaultTransport = value }); + result.Failed.ShouldBeTrue(); + result.Failures.ShouldContain(f => f.Contains("Drivers:AbCip:DefaultTransport") && f.Contains(value)); + + using var sp = Build(Config(value)); + Should.Throw( + () => sp.GetRequiredService>().Value) + .Message.ShouldContain("Drivers:AbCip:DefaultTransport"); + } + + [Theory] + [InlineData("Native")] + [InlineData("libplctag")] + [InlineData(null)] + [InlineData(" ")] + public void A_good_or_blank_value_validates(string? value) => + new AbCipDriverDefaultsOptionsValidator() + .Validate(null, new AbCipDriverDefaultsOptions { DefaultTransport = value }) + .Succeeded.ShouldBeTrue(); + + private sealed class StubSecretResolver : ISecretResolver + { + public Task GetAsync(SecretName name, CancellationToken ct) => Task.FromResult(null); + } +}