00a804e6d9
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.
233 lines
10 KiB
C#
233 lines
10 KiB
C#
using Microsoft.AspNetCore.Builder;
|
|
using Microsoft.AspNetCore.Hosting;
|
|
using Microsoft.AspNetCore.Mvc.Testing;
|
|
using Microsoft.EntityFrameworkCore;
|
|
using Microsoft.Extensions.Configuration;
|
|
using Microsoft.Extensions.DependencyInjection;
|
|
using Microsoft.Extensions.Hosting;
|
|
using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase;
|
|
|
|
namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
|
|
|
|
/// <summary>
|
|
/// Deferred register row 50, mitigation 1: .NET's built-in DI scope validation is on in
|
|
/// Development for BOTH composition roots, and both roots actually survive it.
|
|
/// </summary>
|
|
/// <remarks>
|
|
/// The build-and-validate form is the load-bearing one. Asserting that
|
|
/// <c>ValidateScopes</c> is set proves only that a flag was written; asserting that the
|
|
/// full Central and Site graphs BUILD with <c>ValidateOnBuild</c> on is what proves there
|
|
/// is no latent singleton-captures-scoped violation to be surfaced — and it keeps
|
|
/// proving it as registrations are added, on every CI run, without anyone having to boot
|
|
/// a node in Development.
|
|
///
|
|
/// Both roles force the flags on explicitly rather than relying on the environment, so
|
|
/// the check runs identically in CI (where the environment is whatever the runner
|
|
/// inherits) and so a future change to the environment plumbing cannot silently turn the
|
|
/// coverage off — the exact failure mode this row is about.
|
|
/// </remarks>
|
|
public class ServiceProviderValidationOptionsTests
|
|
{
|
|
[Fact]
|
|
public void Apply_InDevelopment_EnablesBothValidations()
|
|
{
|
|
var options = new ServiceProviderOptions();
|
|
|
|
ServiceProviderValidation.Apply(options, isDevelopment: true);
|
|
|
|
Assert.True(options.ValidateScopes);
|
|
Assert.True(options.ValidateOnBuild);
|
|
}
|
|
|
|
[Fact]
|
|
public void Apply_OutsideDevelopment_LeavesBothValidationsOff()
|
|
{
|
|
// Production pays neither the per-resolve scope check nor the boot-time graph
|
|
// walk, and — more importantly — a validation failure cannot fail a production
|
|
// node closed at boot. See ServiceProviderValidation's remarks.
|
|
var options = new ServiceProviderOptions { ValidateScopes = true, ValidateOnBuild = true };
|
|
|
|
ServiceProviderValidation.Apply(options, isDevelopment: false);
|
|
|
|
Assert.False(options.ValidateScopes);
|
|
Assert.False(options.ValidateOnBuild);
|
|
}
|
|
}
|
|
|
|
/// <summary>
|
|
/// Builds the REAL Central composition root (the whole <c>Program.cs</c> Central branch)
|
|
/// with scope + build validation forced on, and asserts it builds clean.
|
|
/// </summary>
|
|
[Collection(HostBootCollection.Name)]
|
|
public class CentralServiceProviderValidationTests : IDisposable
|
|
{
|
|
private readonly string? _previousEnv;
|
|
private readonly CentralDbTestEnvironment _dbEnv;
|
|
private WebApplicationFactory<Program>? _factory;
|
|
|
|
public CentralServiceProviderValidationTests()
|
|
{
|
|
_previousEnv = Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT");
|
|
Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", "Central");
|
|
_dbEnv = new CentralDbTestEnvironment();
|
|
}
|
|
|
|
public void Dispose()
|
|
{
|
|
try
|
|
{
|
|
_factory?.Dispose();
|
|
}
|
|
finally
|
|
{
|
|
Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", _previousEnv);
|
|
_dbEnv.Dispose();
|
|
}
|
|
}
|
|
|
|
/// <summary>
|
|
/// A captive dependency anywhere in the Central graph — a singleton whose constructor
|
|
/// takes a scoped service — makes <c>ValidateOnBuild</c> throw an
|
|
/// <see cref="AggregateException"/> here listing every offender, so this test names
|
|
/// the violation rather than leaving it to be found by a rig log line months later
|
|
/// (which is how row 45 was found).
|
|
/// </summary>
|
|
[Fact]
|
|
public void Central_CompositionRoot_BuildsWithScopeValidationEnabled()
|
|
{
|
|
_factory = new WebApplicationFactory<Program>()
|
|
.WithWebHostBuilder(builder =>
|
|
{
|
|
// Force the flags on regardless of the runner's environment. This is the
|
|
// same configuration Program.cs applies in Development, exercised here as
|
|
// a hard assertion.
|
|
builder.UseDefaultServiceProvider(options =>
|
|
{
|
|
options.ValidateScopes = true;
|
|
options.ValidateOnBuild = true;
|
|
});
|
|
|
|
builder.ConfigureAppConfiguration((_, config) =>
|
|
{
|
|
config.AddInMemoryCollection(new Dictionary<string, string?>
|
|
{
|
|
["ScadaBridge:Node:NodeName"] = "central-a",
|
|
["ScadaBridge:Node:NodeHostname"] = "localhost",
|
|
["ScadaBridge:Node:RemotingPort"] = "0",
|
|
["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551",
|
|
["ScadaBridge:Cluster:SeedNodes:1"] = "akka.tcp://scadabridge@localhost:2552",
|
|
["ScadaBridge:Database:SkipMigrations"] = "true",
|
|
["ScadaBridge:Security:JwtSigningKey"] = "test-signing-key-must-be-at-least-32-chars-long!",
|
|
["ScadaBridge:Security:Ldap:Server"] = "localhost",
|
|
["ScadaBridge:Security:Ldap:Port"] = "3893",
|
|
["ScadaBridge:Security:Ldap:Transport"] = "None",
|
|
["ScadaBridge:Security:Ldap:AllowInsecure"] = "true",
|
|
["ScadaBridge:Security:Ldap:SearchBase"] = "dc=zb,dc=local",
|
|
["ScadaBridge:Security:Ldap:ServiceAccountDn"] = "cn=admin,dc=zb,dc=local",
|
|
["ScadaBridge:InboundApi:ApiKeyPepper"] = "test-inbound-api-key-pepper-at-least-32-chars!",
|
|
});
|
|
});
|
|
builder.UseSetting("ScadaBridge:Node:Role", "Central");
|
|
builder.UseSetting("ScadaBridge:Database:SkipMigrations", "true");
|
|
builder.ConfigureServices(services =>
|
|
{
|
|
var descriptorsToRemove = services
|
|
.Where(d =>
|
|
d.ServiceType == typeof(DbContextOptions<ScadaBridgeDbContext>) ||
|
|
d.ServiceType == typeof(DbContextOptions) ||
|
|
d.ServiceType == typeof(ScadaBridgeDbContext) ||
|
|
d.ServiceType.FullName?.Contains("EntityFrameworkCore") == true)
|
|
.ToList();
|
|
foreach (var d in descriptorsToRemove)
|
|
services.Remove(d);
|
|
|
|
services.AddDbContext<ScadaBridgeDbContext>(options =>
|
|
options.UseInMemoryDatabase($"ScopeValidation_{Guid.NewGuid()}"));
|
|
|
|
AkkaHostedServiceRemover.RemoveAkkaHostedServiceOnly(services);
|
|
});
|
|
});
|
|
|
|
// Building the server is the assertion: ValidateOnBuild throws from
|
|
// BuildServiceProvider, and ValidateScopes throws on the first offending resolve.
|
|
var services = _factory.Services;
|
|
Assert.NotNull(services);
|
|
}
|
|
}
|
|
|
|
/// <summary>
|
|
/// The Site half. This is the branch that actually gains coverage from the Program.cs
|
|
/// change: the docker rig sets <c>ASPNETCORE_ENVIRONMENT=Development</c> on the central
|
|
/// containers but not on the site containers, so a site node has always built its
|
|
/// provider unvalidated.
|
|
/// </summary>
|
|
public class SiteServiceProviderValidationTests : IDisposable
|
|
{
|
|
private readonly string _tempDbPath;
|
|
private readonly string _tempTrackingDbPath;
|
|
private readonly string _tempLocalDbPath;
|
|
private readonly string _tempAuditDbPath;
|
|
private WebApplication? _host;
|
|
|
|
public SiteServiceProviderValidationTests()
|
|
{
|
|
_tempDbPath = Path.Combine(Path.GetTempPath(), $"sb_scopeval_{Guid.NewGuid()}.db");
|
|
_tempTrackingDbPath = Path.Combine(Path.GetTempPath(), $"sb_scopeval_track_{Guid.NewGuid()}.db");
|
|
_tempLocalDbPath = Path.Combine(Path.GetTempPath(), $"sb_scopeval_localdb_{Guid.NewGuid()}.db");
|
|
_tempAuditDbPath = Path.Combine(Path.GetTempPath(), $"sb_scopeval_audit_{Guid.NewGuid()}.db");
|
|
}
|
|
|
|
public void Dispose()
|
|
{
|
|
(_host as IDisposable)?.Dispose();
|
|
foreach (var path in new[] { _tempDbPath, _tempTrackingDbPath, _tempLocalDbPath, _tempAuditDbPath })
|
|
{
|
|
try { File.Delete(path); } catch { /* best effort */ }
|
|
}
|
|
}
|
|
|
|
[Fact]
|
|
public void Site_CompositionRoot_BuildsWithScopeValidationEnabled()
|
|
{
|
|
var builder = WebApplication.CreateBuilder();
|
|
builder.Configuration.Sources.Clear();
|
|
builder.Configuration.AddInMemoryCollection(new Dictionary<string, string?>
|
|
{
|
|
["ScadaBridge:Node:Role"] = "Site",
|
|
["ScadaBridge:Node:NodeName"] = "node-a",
|
|
["ScadaBridge:Node:NodeHostname"] = "test-site",
|
|
["ScadaBridge:Node:SiteId"] = "TestSite",
|
|
["ScadaBridge:Node:RemotingPort"] = "0",
|
|
["ScadaBridge:Node:GrpcPort"] = "0",
|
|
["ScadaBridge:Database:SiteDbPath"] = _tempDbPath,
|
|
["ScadaBridge:OperationTracking:ConnectionString"] = $"Data Source={_tempTrackingDbPath}",
|
|
["LocalDb:Path"] = _tempLocalDbPath,
|
|
["AuditLog:SiteWriter:DatabasePath"] = _tempAuditDbPath,
|
|
["ScadaBridge:Cluster:SeedNodes:0"] = "akka.tcp://scadabridge@localhost:2551",
|
|
["ScadaBridge:Cluster:SeedNodes:1"] = "akka.tcp://scadabridge@localhost:2552",
|
|
});
|
|
|
|
// The production Program.cs Site branch applies this via
|
|
// ServiceProviderValidation.Configure; forced on here so the check does not depend
|
|
// on the test runner's environment.
|
|
builder.Host.UseDefaultServiceProvider(options =>
|
|
{
|
|
options.ValidateScopes = true;
|
|
options.ValidateOnBuild = true;
|
|
});
|
|
|
|
// Mirrors the Program.cs site section.
|
|
builder.Services.AddGrpc();
|
|
builder.Services.AddSingleton<ZB.MOM.WW.ScadaBridge.Communication.Grpc.SiteStreamGrpcServer>();
|
|
builder.Services.AddSingleton<ZB.MOM.WW.ScadaBridge.Communication.Grpc.SiteCommandGrpcService>();
|
|
|
|
SiteServiceRegistration.Configure(builder.Services, builder.Configuration);
|
|
AkkaHostedServiceRemover.RemoveAkkaHostedServiceOnly(builder.Services);
|
|
|
|
// Build is the assertion — ValidateOnBuild throws here if anything in the site
|
|
// graph is a captive dependency.
|
|
_host = builder.Build();
|
|
Assert.NotNull(_host.Services);
|
|
}
|
|
}
|