Files
ScadaBridge/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/SiteHealthEndpointTests.cs
T
Joseph Doherty 00a804e6d9 guard(di): captive-dependency tripwires — scope validation + an actor field scan
Register row 50. Row 45 (fixed 9e769735) was a live captive dependency —
NotificationOutboxActor cached delivery adapters resolved from an actor-lifetime
IServiceScope, so MaxParallelDeliveries concurrent deliveries shared one EF
DbContext and central-b logged EF's "second operation was started on this context
instance" continuously. Actors take the ROOT IServiceProvider rather than typed
services, which is invisible to build-time analysis, so the bug class had no
mechanical guard at all. This adds both of the register's mitigations; neither
subsumes the other.

1. ValidateScopes + ValidateOnBuild in Development, on BOTH branches, via
   ServiceProviderValidation.Configure. WebApplication.CreateBuilder already sets
   both flags to IsDevelopment(), so on paper this restates a default — it is
   written out because that default is silent (a future move off CreateBuilder
   drops it with no diagnostic) and because this host carries three
   environment-ish signals (SCADABRIDGE_CONFIG, DOTNET_ENVIRONMENT,
   ASPNETCORE_ENVIRONMENT), so "is this node in Development" is not obvious at the
   call site. Production stays unvalidated deliberately: a validation failure is
   fail-closed, and CI now proves what a production node will not check.

   This surfaced two REAL latent violations on the SITE role — not captive
   dependencies but unconstructible descriptors, which fail ValidateOnBuild for
   the entire graph and are therefore what kept the site branch from being
   validatable at all (SiteHealthEndpointTests had pinned UseEnvironment
   ("Production") to work around exactly this):

     - ReconcileService (scoped, needs the central-only IDeploymentManagerRepository)
       moved out of the shared AddCommunication() into a new AddCentralCommunication().
     - AuditLogKpiSampleSource (scoped IKpiSampleSource, needs the central-only
       IAuditLogRepository) moved out of the shared AddAuditLog() into a new
       AddAuditLogCentralKpi().

   Both were registered on sites under the argument "harmless — only the central
   actor ever resolves it". True of the runtime, false of the container: a
   registration nothing can build is what stops the validator being usable, and it
   masks whatever real defect the validator would have found. Both are now
   central-only and SiteHealthEndpointTests boots the site branch validated again.

2. CaptiveDependencyGuard: a reflection scan over every actor class in the shipped
   assemblies (28 today; the reference graph is walked from the Host assembly
   because referenced assemblies load lazily). Fails on any instance field — auto-
   property backing fields included, they are ordinary private fields — typed as
   IServiceScope, as a DbContext, or as a service the REAL composition roots
   register Scoped. The scoped set is derived from the built Central and Site
   containers rather than hand-listed, so a lifetime change automatically re-scopes
   the check. IServiceProvider / IServiceScopeFactory fields are explicitly allowed:
   that IS the correct per-operation-scope pattern.

   No live violation exists — every actor opens a scope per operation. Two fields
   are allowlisted with their safety argument rather than exempted silently:
   AuditLogIngestActor._injectedRepository and SiteCallAuditActor._injectedRepository
   are scoped repository types assigned ONLY by explicitly test-mode constructors,
   null under production wiring (AkkaHostedService uses the IServiceProvider
   overload in both cases), and SiteCallAuditActor additionally wraps its injected
   instance in a SerializedRepository. The shape is one constructor call away from
   being row 45 again, so it is recorded, not dismissed.

Tests: 10 new. Six permanent self-tests of the guard, including positive controls
that reproduce the pre-row-45 shape (an actor caching a scoped service, an
IServiceScope, a DbContext, and a scoped auto-property) and a negative control for
the allowed root-provider pattern — a reflection guard that matches nothing passes
just as green as one that works. Plus an actor-count floor with cross-assembly
spot-checks, an allowlist-staleness test that fails any entry no longer matching a
flagged field, and the two per-role scans. Verified non-vacuous by emptying the
allowlist: the Central scan then names both real fields. Two composition-root
build tests force both validation flags on per role regardless of environment.

Doc: REQ-HOST-11 in Component-Host.md records all three rules and their guards.
2026-08-15 11:58:40 -04:00

165 lines
7.9 KiB
C#

using System.Net;
using System.Text.Json;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Mvc.Testing;
using Microsoft.AspNetCore.TestHost;
using Microsoft.Extensions.DependencyInjection;
namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
/// <summary>
/// Proves the site pipeline actually MAPS the health endpoints — the half
/// <see cref="SiteHealthCheckTests"/> cannot cover, since that fixture builds the site container
/// from <see cref="SiteServiceRegistration"/> and never runs <c>Program</c>'s endpoint wiring.
/// Without this, deleting <c>app.MapZbHealth()</c> from the site branch would leave every
/// registration test green while site nodes served 404 to the overview dashboard.
/// <para>
/// Boots the real <c>Program</c> in the Site role. <c>Program</c> builds its own
/// <c>ConfigurationBuilder</c> before the web host exists, so its role and settings come from
/// PROCESS-WIDE environment variables (<c>SCADABRIDGE_CONFIG</c> selects appsettings.Site.json)
/// rather than from <c>WithWebHostBuilder</c> — which is why this fixture joins
/// <see cref="HostBootCollection"/> and restores every var it sets.
/// </para>
/// </summary>
[Collection(HostBootCollection.Name)]
public class SiteHealthEndpointTests : IDisposable
{
private readonly List<IDisposable> _disposables = new();
private readonly Dictionary<string, string?> _previousEnv = new(StringComparer.Ordinal);
private readonly string _tempDbPath;
private readonly string _tempAuditDbPath;
public SiteHealthEndpointTests()
{
_tempDbPath = Path.Combine(Path.GetTempPath(), $"scadabridge_health_ep_{Guid.NewGuid()}.db");
_tempAuditDbPath = Path.Combine(Path.GetTempPath(), $"scadabridge_health_ep_audit_{Guid.NewGuid()}.db");
// Whole-key env overrides, the sanctioned path: supplying GrpcPsk concretely makes the
// pre-host secrets expander skip appsettings.Site.json's ${secret:SB-GRPC-PSK-site-1}
// token, so this test needs no seeded secrets store.
//
// The remoting port is moved off the site default (8082) so this fixture cannot collide
// with a locally running node, but it must be a REAL port and seed-nodes[0] must be this
// node itself — StartupValidator enforces both before the host is built, and it runs
// whether or not the Akka hosted service is later removed. Nothing binds it: the hosted
// service is removed below.
SetEnv("SCADABRIDGE_CONFIG", "Site");
SetEnv("ScadaBridge__Node__Role", "Site");
SetEnv("ScadaBridge__Node__SiteId", "TestSite");
SetEnv("ScadaBridge__Node__NodeHostname", "localhost");
SetEnv("ScadaBridge__Node__RemotingPort", "18082");
SetEnv("ScadaBridge__Cluster__SeedNodes__0", "akka.tcp://scadabridge@localhost:18082");
SetEnv("ScadaBridge__Cluster__SeedNodes__1", "akka.tcp://scadabridge@localhost:18085");
SetEnv("ScadaBridge__Communication__GrpcPsk", "test-psk-0123456789");
SetEnv("LocalDb__Path", _tempDbPath);
// arch-review remediation WP1.2: SqliteAuditWriterOptions.DatabasePath has no default
// and StartupValidator now requires it for Site nodes — same reason LocalDb__Path is
// overridden above (appsettings.Site.json's own default is CWD-relative and would
// otherwise litter the test working directory when SiteAuditBacklogReporter probes it).
SetEnv("AuditLog__SiteWriter__DatabasePath", _tempAuditDbPath);
}
private void SetEnv(string key, string? value)
{
_previousEnv[key] = Environment.GetEnvironmentVariable(key);
Environment.SetEnvironmentVariable(key, value);
}
public void Dispose()
{
foreach (var d in _disposables)
{
try { d.Dispose(); } catch { /* best effort */ }
}
foreach (var (key, value) in _previousEnv)
{
Environment.SetEnvironmentVariable(key, value);
}
try { File.Delete(_tempDbPath); } catch { /* best effort */ }
try { File.Delete(_tempAuditDbPath); } catch { /* best effort */ }
GC.SuppressFinalize(this);
}
private HttpClient CreateSiteClient()
{
var factory = new WebApplicationFactory<Program>()
.WithWebHostBuilder(builder =>
{
// The environment is left at WebApplicationFactory's Development default on
// purpose, so this fixture boots the site branch with ValidateOnBuild/
// ValidateScopes ON. It used to pin Production because the site graph carried two
// central-only scoped registrations it could not construct (ReconcileService →
// IDeploymentManagerRepository, AuditLogKpiSampleSource → IAuditLogRepository);
// deferred register row 50 moved both to central-only extensions
// (AddCentralCommunication / AddAuditLogCentralKpi), so the site container now
// validates clean and there is no reason to run it unvalidated.
// ConfigureTestServices runs after the app's own registrations, so this drops the
// hosted service that would form a real cluster while leaving the AkkaHostedService
// singleton resolvable (the site pipeline resolves it for shutdown ordering).
builder.ConfigureTestServices(AkkaHostedServiceRemover.RemoveAkkaHostedServiceOnly);
});
_disposables.Add(factory);
var client = factory.CreateClient();
_disposables.Add(client);
return client;
}
[Fact]
public async Task Site_MapsHealthReady_WithTheCanonicalJsonBody()
{
var response = await CreateSiteClient().GetAsync("/health/ready");
// Never 404. 200 vs 503 depends on cluster state this fixture does not form, so both are
// accepted — what is asserted is that the tier is mapped and answers in canonical shape.
Assert.NotEqual(HttpStatusCode.NotFound, response.StatusCode);
Assert.True(
response.StatusCode is HttpStatusCode.OK or HttpStatusCode.ServiceUnavailable,
$"Expected 200 or 503, got {(int)response.StatusCode}");
using var doc = JsonDocument.Parse(await response.Content.ReadAsStringAsync());
var entries = doc.RootElement.GetProperty("entries");
Assert.True(entries.TryGetProperty("akka-cluster", out _), "ready tier must carry akka-cluster");
Assert.True(entries.TryGetProperty("localdb", out _), "ready tier must carry localdb");
// Readiness must NOT depend on being the primary, or a healthy standby would read unready.
Assert.False(entries.TryGetProperty("active-node", out _), "active-node belongs to the Active tier");
}
[Fact]
public async Task Site_MapsHealthActive_CarryingOnlyTheActiveNodeCheck()
{
var response = await CreateSiteClient().GetAsync("/health/active");
Assert.NotEqual(HttpStatusCode.NotFound, response.StatusCode);
using var doc = JsonDocument.Parse(await response.Content.ReadAsStringAsync());
var entries = doc.RootElement.GetProperty("entries");
Assert.Equal(
new[] { "active-node" },
entries.EnumerateObject().Select(p => p.Name).ToArray());
}
[Fact]
public async Task Site_HealthEndpointsAreAnonymous()
{
// The dashboard authenticates nowhere. The site pipeline runs no authentication middleware
// today, so this is a regression pin: adding one later must not silently close these.
var client = CreateSiteClient();
foreach (var path in new[] { "/health/ready", "/health/active", "/healthz" })
{
var response = await client.GetAsync(path);
Assert.NotEqual(HttpStatusCode.Unauthorized, response.StatusCode);
Assert.NotEqual(HttpStatusCode.Forbidden, response.StatusCode);
Assert.NotEqual(HttpStatusCode.NotFound, response.StatusCode);
}
}
}