feat(sql): factory + connectionStringRef env resolution + string-enum guard

SqlDriverFactoryExtensions turns a deployed DriverConfig blob into a live
SqlDriver, and SqlConnectionStringResolver resolves the authored
connectionStringRef NAME from Sql__ConnectionStrings__<ref> so credentials
never ride in a config blob (design §8.2).

Three behaviours worth naming:

- String-enum guard. JsonOptions carries JsonStringEnumConverter +
  camelCase, so `provider` parses whether authored as a name or an ordinal
  and always round-trips as the NAME — the systemic AdminUI defect where a
  page serialised an enum numerically against a string-typed DTO.
- Config validation lands here because nothing below does it.
  operationTimeout must be STRICTLY greater than commandTimeout (design
  §8.3); the reader and the driver stay deliberately usable with the pair
  inverted so the frozen-database tests can prove the client-side bound
  fires. Non-positive timeouts / poll interval and maxConcurrentGroups < 1
  are rejected too.
- Credential hygiene. The resolved connection string reaches the provider
  and nothing else: messages name the ref, the environment variable, or
  SqlDriver.Endpoint's credential-free server/database rendering. Both a
  log-leak and an exception-leak test cover it.

DTO changes:
- Adds RawTags (List<RawTagEntry>), following the Modbus precedent — the
  deploy artifact delivers a driver's tags this way and the DTO had no way
  to receive them, so an authored Sql tag could never reach the driver.
- Deletes SqlProbeDto + the `probe` key. It had no consumer: SqlDriver was
  specified without a background probe loop, and deliberately so — its
  IHostConnectivityProbe state is a by-product of the Initialize liveness
  check and of every poll, i.e. a statement about traffic that actually
  happened rather than a synthetic ping. On-demand connectivity is Task
  10's SqlDriverProbe. Shipping a config key that silently does nothing is
  worse than not shipping it; UnmappedMemberHandling.Skip means a blob that
  still carries `probe` keeps parsing.

allowWrites stays inert (SqlDriver implements no write capability) but an
authored `true` now WARNS rather than passing silently — the flag cannot do
harm, an operator who believes writes are enabled can.

Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
Joseph Doherty
2026-07-24 15:06:58 -04:00
parent 48cbc26c34
commit 3cc8069150
4 changed files with 787 additions and 13 deletions
@@ -1,3 +1,5 @@
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts;
/// <summary>
@@ -41,19 +43,22 @@ public sealed class SqlDriverConfigDto
/// <summary>When <see langword="true"/>, a NULL cell publishes Bad rather than the default Uncertain.</summary>
public bool? NullIsBad { get; init; }
/// <summary>Master write kill-switch. <b>v1 is read-only</b> — writes stay disabled regardless.</summary>
/// <summary>
/// Master write kill-switch. <b>v1 is read-only and this field is inert</b> — the driver does not
/// implement <c>IWritable</c>, so no value here can produce a write. The factory <em>warns</em> when it
/// is authored <see langword="true"/> rather than failing the driver, since the flag cannot do harm but
/// an operator who believes writes are enabled can.
/// </summary>
public bool? AllowWrites { get; init; }
/// <summary>Background connectivity-probe settings. Absent ⇒ the factory default.</summary>
public SqlProbeDto? Probe { get; init; }
}
/// <summary>Background connectivity-probe settings inside a <see cref="SqlDriverConfigDto"/> (design §5.1).</summary>
public sealed class SqlProbeDto
{
/// <summary>Whether the driver runs its background connectivity probe.</summary>
public bool? Enabled { get; init; }
/// <summary>Interval between probe attempts.</summary>
public TimeSpan? Interval { get; init; }
/// <summary>
/// The authored raw tags the deploy artifact delivers for this driver instance — RawPath identity plus
/// the driver <c>TagConfig</c> blob, the same shape every other driver receives. The factory copies
/// these onto the driver's options and the driver maps each through
/// <see cref="SqlEquipmentTagParser.TryParse"/> at Initialize.
/// <para>A SQL source has no tag-discovery protocol, so this list is the complete set of tags the
/// instance serves — an absent or empty list is a driver with nothing to poll, not a driver that will
/// find its tags later.</para>
/// </summary>
public List<RawTagEntry>? RawTags { get; init; }
}
@@ -0,0 +1,62 @@
namespace ZB.MOM.WW.OtOpcUa.Driver.Sql;
/// <summary>
/// Resolves a <c>Sql</c> driver config's <c>connectionStringRef</c> — a NAME — into the connection string
/// it stands for (design §8.2). The deployed artifact carries only the name, so database credentials never
/// ride in a config blob, never land in the central config DB, and never appear in a deployment diff.
/// <para><b>The environment is read directly, on purpose.</b> A driver factory is invoked through
/// <c>DriverFactoryRegistry</c>'s <c>(driverInstanceId, driverConfigJson)</c> closure — there is no
/// <c>IConfiguration</c>, no <c>IServiceProvider</c> and no ambient scope at that seam, and widening the
/// registry's signature for one driver would change every driver in the tree. The
/// <c>Sql__ConnectionStrings__&lt;ref&gt;</c> spelling is exactly the key .NET's environment-variable
/// configuration provider would bind to <c>Sql:ConnectionStrings:&lt;ref&gt;</c>, so an operator can set it
/// the same way they set every other secret and a later move onto <c>IConfiguration</c> needs no
/// re-provisioning.</para>
/// <para><b>Nothing here ever emits the resolved value.</b> Messages name the environment
/// <em>variable</em>, which is not a secret and is the operator's next action; the value itself is returned
/// to exactly one caller and is otherwise unmentioned.</para>
/// </summary>
public static class SqlConnectionStringResolver
{
/// <summary>
/// The environment-variable prefix a <c>connectionStringRef</c> is appended to. The double underscore
/// is .NET's configuration hierarchy separator, so this is the environment spelling of
/// <c>Sql:ConnectionStrings:&lt;ref&gt;</c>.
/// </summary>
public const string EnvironmentVariablePrefix = "Sql__ConnectionStrings__";
/// <summary>The environment variable a given <c>connectionStringRef</c> is read from.</summary>
/// <param name="connectionStringRef">The authored reference name.</param>
/// <returns>The full environment-variable name.</returns>
/// <exception cref="ArgumentException"><paramref name="connectionStringRef"/> is null, empty or whitespace.</exception>
public static string EnvironmentVariableFor(string connectionStringRef)
{
ArgumentException.ThrowIfNullOrWhiteSpace(connectionStringRef);
return string.Concat(EnvironmentVariablePrefix, connectionStringRef);
}
/// <summary>
/// Resolves <paramref name="connectionStringRef"/> to its connection string.
/// <para>An absent <b>or blank</b> variable is a half-provisioned deployment and throws: handing an
/// empty string to the provider would surface as an unintelligible ADO.NET error at the first poll,
/// far from the missing secret that actually caused it.</para>
/// </summary>
/// <param name="connectionStringRef">The authored reference name.</param>
/// <returns>The resolved connection string. <b>Treat as a secret</b> — never log it.</returns>
/// <exception cref="ArgumentException"><paramref name="connectionStringRef"/> is null, empty or whitespace.</exception>
/// <exception cref="InvalidOperationException">The environment variable is absent or blank.</exception>
public static string Resolve(string connectionStringRef)
{
var variable = EnvironmentVariableFor(connectionStringRef);
var value = Environment.GetEnvironmentVariable(variable);
if (string.IsNullOrWhiteSpace(value))
{
throw new InvalidOperationException(
$"Sql connection string '{connectionStringRef}' is not provisioned: set the environment " +
$"variable '{variable}' on every node that runs this driver. The connection string is " +
$"deliberately not carried in the deployed configuration.");
}
return value;
}
}
@@ -0,0 +1,255 @@
using System.Text.Json;
using System.Text.Json.Serialization;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using ZB.MOM.WW.OtOpcUa.Core.Hosting;
using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts;
namespace ZB.MOM.WW.OtOpcUa.Driver.Sql;
/// <summary>
/// Static factory registration helper for <see cref="SqlDriver"/> — the seam between a deployed
/// <c>DriverConfig</c> blob and a live driver instance. The Server's composition root calls
/// <see cref="Register"/> once at startup; the bootstrapper then materialises <c>Sql</c> DriverInstance
/// rows into driver instances. Mirrors <c>ModbusDriverFactoryExtensions</c>.
/// <para><b>This type owns the config validation the layers below deliberately skip.</b>
/// <see cref="SqlPollReader"/> and <see cref="SqlDriver"/> stay usable with an inverted
/// <c>operationTimeout</c>/<c>commandTimeout</c> pair on purpose (the frozen-database tests need that
/// shape), so the factory is the only place a deployment can be stopped before it silently loses one of
/// its two independent deadlines. Every knob is checked here, once, at construction.</para>
/// <para><b>Enum fields are read leniently and written as names.</b> <see cref="JsonOptions"/> carries a
/// <see cref="JsonStringEnumConverter"/>, so a <c>provider</c> authored as an ordinal still parses and a
/// round-trip emits <c>"SqlServer"</c> — the systemic AdminUI defect where a page serialised an enum
/// numerically while the consuming DTO was string-typed, faulting the driver at deploy time.</para>
/// <para><b>The resolved connection string is a secret and never leaves this method except into the
/// driver.</b> Every log line and every exception message here names the <c>connectionStringRef</c>, the
/// environment variable, or <see cref="SqlDriver.Endpoint"/>'s credential-free <c>server/database</c>
/// rendering — never the string itself.</para>
/// </summary>
public static class SqlDriverFactoryExtensions
{
/// <summary>
/// The driver-type string this factory registers under.
/// <para>Chained off <see cref="SqlDriver.DriverTypeName"/> rather than <c>DriverTypeNames.Sql</c>:
/// <c>DriverTypeNamesGuardTests</c> asserts bidirectional parity between the shared constants and the
/// factories actually registered in the process, so the constant may only be added in the change that
/// also wires this factory into the Host (the plan's Task 11, which repoints both at
/// <c>DriverTypeNames.Sql</c>).</para>
/// </summary>
public const string DriverTypeName = SqlDriver.DriverTypeName;
/// <summary>
/// How a <c>Sql</c> config blob is read.
/// <list type="bullet">
/// <item><see cref="JsonStringEnumConverter"/> — accepts an enum written as a name OR as an
/// ordinal, and always writes the name (the enum-serialization guard).</item>
/// <item><see cref="JsonUnmappedMemberHandling.Skip"/> — a blob authored against a different
/// schema version must not brick the driver.</item>
/// <item><see cref="JsonNamingPolicy.CamelCase"/> — the authored spelling
/// (<c>connectionStringRef</c>, <c>rawTags</c>). Reads are case-insensitive anyway; the policy is
/// what makes a <em>write</em> round-trip to the authored shape.</item>
/// </list>
/// </summary>
internal static readonly JsonSerializerOptions JsonOptions = new()
{
PropertyNameCaseInsensitive = true,
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
ReadCommentHandling = JsonCommentHandling.Skip,
AllowTrailingCommas = true,
UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip,
Converters = { new JsonStringEnumConverter() },
};
/// <summary>The serializer options, exposed so tests can assert the enum guard on a real round-trip.</summary>
internal static JsonSerializerOptions JsonOptionsForTest => JsonOptions;
/// <summary>
/// Registers the <c>Sql</c> factory with the driver registry. The optional
/// <paramref name="loggerFactory"/> is captured at registration time and used to build a logger per
/// driver instance; without it the driver runs with the null logger.
/// </summary>
/// <param name="registry">The driver factory registry to register with.</param>
/// <param name="loggerFactory">Optional logger factory for creating loggers per driver instance.</param>
/// <exception cref="ArgumentNullException"><paramref name="registry"/> is null.</exception>
public static void Register(DriverFactoryRegistry registry, ILoggerFactory? loggerFactory = null)
{
ArgumentNullException.ThrowIfNull(registry);
registry.Register(DriverTypeName, (id, json) => CreateInstance(id, json, loggerFactory));
}
/// <summary>Builds one driver instance from its config blob. For the Server bootstrapper and tests.</summary>
/// <param name="driverInstanceId">The central config DB's identity for this driver instance.</param>
/// <param name="driverConfigJson">The instance's <c>DriverConfig</c> JSON blob.</param>
/// <returns>The constructed <see cref="SqlDriver"/>.</returns>
public static SqlDriver CreateInstance(string driverInstanceId, string driverConfigJson)
=> CreateInstance(driverInstanceId, driverConfigJson, loggerFactory: null);
/// <summary>
/// Logger-aware overload — used by <see cref="Register"/>'s closure when wired through DI.
/// <para>Constructs the driver only: the driver builds its own <see cref="SqlPollReader"/>, because
/// the reader's <c>resolve</c> delegate must read the driver's live authored-tag table, which does not
/// exist until the driver does.</para>
/// <para>No I/O happens here. The database is first contacted by
/// <see cref="SqlDriver.InitializeAsync"/>, so a database that is merely down yields a driver in
/// Reconnecting rather than a deployment that cannot be constructed.</para>
/// </summary>
/// <param name="driverInstanceId">The central config DB's identity for this driver instance.</param>
/// <param name="driverConfigJson">The instance's <c>DriverConfig</c> JSON blob.</param>
/// <param name="loggerFactory">Optional logger factory for creating loggers per driver instance.</param>
/// <returns>The constructed <see cref="SqlDriver"/>.</returns>
/// <exception cref="ArgumentException">An argument is null, empty or whitespace.</exception>
/// <exception cref="InvalidOperationException">
/// The blob is not valid JSON, omits <c>connectionStringRef</c>, names a provider this build cannot
/// construct, carries an invalid knob, or the referenced connection string is not provisioned.
/// </exception>
public static SqlDriver CreateInstance(
string driverInstanceId, string driverConfigJson, ILoggerFactory? loggerFactory)
{
ArgumentException.ThrowIfNullOrWhiteSpace(driverInstanceId);
ArgumentException.ThrowIfNullOrWhiteSpace(driverConfigJson);
var dto = Deserialize(driverInstanceId, driverConfigJson);
if (string.IsNullOrWhiteSpace(dto.ConnectionStringRef))
{
throw new InvalidOperationException(
$"Sql driver config for '{driverInstanceId}' is missing the required connectionStringRef. " +
$"Author the NAME of a connection string (the value itself is supplied out-of-band through " +
$"the '{SqlConnectionStringResolver.EnvironmentVariablePrefix}<ref>' environment variable).");
}
var dialect = CreateDialect(dto.Provider, driverInstanceId);
// Resolved BEFORE the knob validation below, so that every subsequent throw is a path that HAS the
// secret in hand — which is exactly where a careless interpolation would leak it.
var connectionString = SqlConnectionStringResolver.Resolve(dto.ConnectionStringRef!);
var options = BuildOptions(dto, driverInstanceId);
var logger = (ILogger?)loggerFactory?.CreateLogger(typeof(SqlDriverFactoryExtensions).FullName!)
?? NullLogger.Instance;
if (dto.AllowWrites is true)
{
// Inert by construction (SqlDriver does not implement IWritable), so failing the driver over it
// would brick a deployment for a flag that cannot do harm. The operator who believes writes are
// on, however, can — so this is loud.
logger.LogWarning(
"Sql driver {DriverInstanceId} authored allowWrites=true, which this build ignores: the Sql " +
"driver is read-only (it implements no write capability), so no configuration can enable a " +
"write. Remove the flag, or expect reads only.",
driverInstanceId);
}
var driver = new SqlDriver(
options,
driverInstanceId,
dialect,
connectionString,
factory: null,
logger: loggerFactory?.CreateLogger<SqlDriver>());
// Endpoint is the credential-free server/database rendering — the ONLY form of the connection string
// permitted outside the provider call.
logger.LogInformation(
"Sql driver {DriverInstanceId} configured for {Provider} at {Endpoint} with {TagCount} " +
"authored tag(s), poll {PollInterval}, operation timeout {OperationTimeout}.",
driverInstanceId, dto.Provider, driver.Endpoint, options.RawTags.Count,
options.DefaultPollInterval, options.OperationTimeout);
return driver;
}
/// <summary>Deserialises the blob, turning a malformed one into an actionable, instance-named failure.</summary>
private static SqlDriverConfigDto Deserialize(string driverInstanceId, string driverConfigJson)
{
try
{
return JsonSerializer.Deserialize<SqlDriverConfigDto>(driverConfigJson, JsonOptions)
?? throw new InvalidOperationException(
$"Sql driver config for '{driverInstanceId}' deserialised to null");
}
catch (JsonException ex)
{
throw new InvalidOperationException(
$"Sql driver config for '{driverInstanceId}' is not valid JSON: {ex.Message}", ex);
}
}
/// <summary>
/// Selects the provider seam.
/// <para>v1 constructs <see cref="SqlProvider.SqlServer"/> only. The other members exist on the shipped
/// enum as reserved names; authoring one is a config mistake that must fail at construction with a
/// message that says so, rather than silently degrading to T-SQL against a non-SQL-Server database.</para>
/// </summary>
private static ISqlDialect CreateDialect(SqlProvider provider, string driverInstanceId)
=> provider switch
{
SqlProvider.SqlServer => new SqlServerDialect(),
_ => throw new InvalidOperationException(
$"Sql driver config for '{driverInstanceId}' names provider '{provider}', which is not " +
$"available in this build. Only '{nameof(SqlProvider.SqlServer)}' is supported."),
};
/// <summary>
/// Applies the documented defaults and validates every knob (design §5.1 / §8.3).
/// <para>Internal so the defaults and the rejections can be asserted directly, without a database or a
/// provisioned connection string standing between the test and the rule.</para>
/// </summary>
/// <param name="dto">The deserialised config blob.</param>
/// <param name="driverInstanceId">Named in every rejection message, so a fleet-wide log identifies the row.</param>
/// <returns>The typed options the driver is constructed with.</returns>
/// <exception cref="InvalidOperationException">A knob is out of range, or the two deadlines are inverted.</exception>
internal static SqlDriverOptions BuildOptions(SqlDriverConfigDto dto, string driverInstanceId)
{
ArgumentNullException.ThrowIfNull(dto);
var defaultPollInterval = dto.DefaultPollInterval ?? TimeSpan.FromSeconds(5);
var operationTimeout = dto.OperationTimeout ?? TimeSpan.FromSeconds(15);
var commandTimeout = dto.CommandTimeout ?? TimeSpan.FromSeconds(10);
var maxConcurrentGroups = dto.MaxConcurrentGroups ?? 4;
RequirePositive(defaultPollInterval, "defaultPollInterval", driverInstanceId);
RequirePositive(operationTimeout, "operationTimeout", driverInstanceId);
RequirePositive(commandTimeout, "commandTimeout", driverInstanceId);
if (maxConcurrentGroups < 1)
{
throw new InvalidOperationException(
$"Sql driver config for '{driverInstanceId}' has maxConcurrentGroups {maxConcurrentGroups}; " +
$"it must be at least 1.");
}
// Design §8.3. The two deadlines are independent and BOTH are required: commandTimeout is the
// server-side backstop, operationTimeout the client-side wall-clock bound that alone survives a
// wedged socket (the R2-01/STAB-14 lesson). Inverted — or equal — the client-side abort always fires
// first and the backstop can never be reached, so a deployment quietly runs on one bound instead of
// two. Nothing downstream enforces this: the reader stays deliberately usable with it inverted.
if (operationTimeout <= commandTimeout)
{
throw new InvalidOperationException(
$"Sql driver config for '{driverInstanceId}' has operationTimeout {operationTimeout} which " +
$"is not greater than commandTimeout {commandTimeout}. The client-side operationTimeout must " +
$"be strictly greater than the server-side commandTimeout backstop, or the backstop can " +
$"never fire.");
}
return new SqlDriverOptions
{
RawTags = dto.RawTags is { Count: > 0 } rawTags ? [.. rawTags] : [],
DefaultPollInterval = defaultPollInterval,
OperationTimeout = operationTimeout,
CommandTimeout = commandTimeout,
MaxConcurrentGroups = maxConcurrentGroups,
NullIsBad = dto.NullIsBad ?? false,
};
}
/// <summary>Rejects a non-positive duration, naming the authored field.</summary>
private static void RequirePositive(TimeSpan value, string field, string driverInstanceId)
{
if (value <= TimeSpan.Zero)
{
throw new InvalidOperationException(
$"Sql driver config for '{driverInstanceId}' has {field} {value}; it must be greater than zero.");
}
}
}