From 6ed6800b84f016ec73a9229d5fd3bb96c4c19b07 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 15:56:00 -0400 Subject: [PATCH] =?UTF-8?q?fix(host):=20clear=20pooled=20SQLite=20handles?= =?UTF-8?q?=20before=20renaming=20the=20imported=20inbound-key=20file=20?= =?UTF-8?q?=E2=80=94=20Windows=20refused=20the=20rename,=20so=20the=20impo?= =?UTF-8?q?rt=20re-ran=20every=20boot=20(QA=20vq03/vq04=202026-10-22)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/operations/inbound-api-key-reissue.md | 6 +++ .../InboundApiKeyStoreImportService.cs | 7 +++ .../InboundApiKeyStoreImportServiceTests.cs | 48 +++++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/docs/operations/inbound-api-key-reissue.md b/docs/operations/inbound-api-key-reissue.md index 0a0b57c4..55cb93ae 100644 --- a/docs/operations/inbound-api-key-reissue.md +++ b/docs/operations/inbound-api-key-reissue.md @@ -71,6 +71,12 @@ about this at boot). To move an existing pair over: 5. Check the import log line on each node (`grep -i "api-key store import\|zbauth"` in the node's log) — one node reports the keys it inserted, the other reports the ones it found already present (skipped); together they cover the union of both nodes' prior SQLite files. + On a **Windows** central running a build before 2026-10-22 the import succeeds but the + `*.imported` rename then fails with `IOException: … being used by another process` (the + importer's pooled SQLite handle; fixed by clearing the pools before the rename) — the import + re-runs harmlessly on every boot. Fix it by hand: stop that node's Central, wait for its + `ZB.MOM.WW.ScadaBridge.Host` process to exit, rename `inbound-api-keys.sqlite` and its + `-wal`/`-shm` siblings to `*.imported`, start it again (QA, 2026-10-22). 6. Run `security api-key list` against each node's management endpoint and confirm both show the same keys. 7. After a quarantine period (once you're confident the import is correct and no rollback is diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/InboundApiKeyStoreImportService.cs b/src/ZB.MOM.WW.ScadaBridge.Host/InboundApiKeyStoreImportService.cs index ffadb001..680f6570 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/InboundApiKeyStoreImportService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/InboundApiKeyStoreImportService.cs @@ -1,4 +1,5 @@ using Microsoft.Data.SqlClient; +using Microsoft.Data.Sqlite; using Microsoft.Extensions.Options; using ZB.MOM.WW.Auth.Abstractions.ApiKeys; using ZB.MOM.WW.Auth.ApiKeys.SqlServer; @@ -112,6 +113,12 @@ internal sealed class InboundApiKeyStoreImportService( path, result.KeysInserted, result.KeysSkipped, result.AuditInserted, result.AuditSkipped, result.SourceFound); + // The library importer reads the file through a POOLED SqliteConnection, and the pool + // keeps the OS handle open after the connection is disposed. Linux renames an open file + // happily, but Windows refuses ("being used by another process"), so on a Windows + // central the rename failed on every boot and the import re-ran each time (QA + // vq03/vq04, 2026-10-22). Drop the idle pooled handles first. + SqliteConnection.ClearAllPools(); RenameImported(path); } catch (Exception ex) diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/InboundApiKeyStoreImportServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/InboundApiKeyStoreImportServiceTests.cs index eed838a2..d9591491 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/InboundApiKeyStoreImportServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/InboundApiKeyStoreImportServiceTests.cs @@ -60,6 +60,35 @@ public sealed class InboundApiKeyStoreImportServiceTests : IDisposable Assert.True(File.Exists(path + "-shm.imported")); } + /// + /// Verifies that the rename succeeds after the importer read the file through a pooled + /// SQLite connection — the library's real behaviour. The idle pooled handle stays open after + /// the connection is disposed, and Windows refuses to rename a file with an open handle (the + /// failure seen on the QA centrals 2026-10-22); the service must release pooled handles first. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task StartAsync_ImporterLeftPooledSqliteHandle_StillRenamesFile() + { + var path = Path.Combine(_dir, "inbound-api-keys.sqlite"); + var importer = new PooledSqliteReadingImporter(); + var service = CreateService(importer, sqlitePath: path); + + await using (var create = new Microsoft.Data.Sqlite.SqliteConnection($"Data Source={path};Pooling=True")) + { + await create.OpenAsync(); + await using var cmd = create.CreateCommand(); + cmd.CommandText = "CREATE TABLE api_keys (key_id TEXT PRIMARY KEY)"; + await cmd.ExecuteNonQueryAsync(); + } + + await service.StartAsync(CancellationToken.None); + + Assert.True(importer.Read); + Assert.False(File.Exists(path)); + Assert.True(File.Exists(path + ".imported")); + } + /// Verifies that the importer is not called when the file is absent. /// A task that represents the asynchronous operation. [Fact] @@ -229,6 +258,25 @@ public sealed class InboundApiKeyStoreImportServiceTests : IDisposable Options.Create(new ApiKeyOptions { SqlitePath = sqlitePath }), NullLogger.Instance); + /// Reads the file through a pooled connection and disposes it, as the library importer does. + private sealed class PooledSqliteReadingImporter : IInboundApiKeyStoreImporter + { + /// Gets a value indicating whether the file was read. + public bool Read { get; private set; } + + /// + public async Task ImportAsync(string sqlitePath, CancellationToken ct) + { + await using var connection = new Microsoft.Data.Sqlite.SqliteConnection($"Data Source={sqlitePath};Pooling=True"); + await connection.OpenAsync(ct); + await using var cmd = connection.CreateCommand(); + cmd.CommandText = "SELECT COUNT(*) FROM api_keys"; + await cmd.ExecuteScalarAsync(ct); + Read = true; + return new ApiKeyImportResult(0, 0, 0, 0, SourceFound: true); + } + } + private sealed class FakeImporter : IInboundApiKeyStoreImporter { private int _calls;