diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fa4fd9e..83f7d4f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,8 +48,11 @@ jobs: - name: test run: dotnet test DodoSSH.slnx --no-build --configuration Release - # Integration tests land in M1 and need Docker for Testcontainers (PostgreSQL, - # OpenSSH). They run on this ubuntu job because macOS runners have no Docker daemon. + # This includes the end-to-end suite, which starts PostgreSQL, Keycloak and an OpenSSH + # server through Testcontainers and runs the API as a child process — so it needs a + # Docker daemon and gets one here. That is why the tests run on ubuntu rather than + # macOS, whose runners have no daemon at all. Expect the Keycloak image pull to + # dominate a cold run. build-windows: name: build (windows) diff --git a/README.md b/README.md index 345cfe3..0571cfa 100644 --- a/README.md +++ b/README.md @@ -74,6 +74,10 @@ dotnet build DodoSSH.slnx dotnet test DodoSSH.slnx ``` +The tests need a Docker daemon. Everything that touches the database, the identity provider or an SSH +server uses Testcontainers rather than a stub or a shared instance, so there is nothing to start first and +nothing to clean up after — but with no daemon those suites fail rather than skip. + Run the API locally: ```bash @@ -85,32 +89,28 @@ Development — `/openapi/v1.json`. ### End-to-end verification -One suite runs against a real server rather than a stub, and it is opt-in because it needs the stack: +One suite runs against a real server rather than a stub. It needs a Docker daemon and nothing else, so it +is part of the ordinary test run: ```bash -docker compose -f deploy/docker-compose.dev.yml up -d +dotnet test tests/DodoSSH.SystemTests ``` -```bash -dotnet ef database update --project src/DodoSSH.Infrastructure -``` +It brings up PostgreSQL, Keycloak and an OpenSSH server in containers, applies the committed migrations, +starts the API as a child process out of its own build output, and then drives the real client: sign in +through Keycloak, enroll, unlock, create a host, sync it, read it back on a second simulated machine, +unlock again with no network, and open a shell on the `sshd`. Roughly 25 seconds once the images are +pulled. -```bash -dotnet run --project src/DodoSSH.Api -``` +What makes it worth its weight is that it consumes the artefacts that ship — the realm file from +`deploy/keycloak`, the EF migrations, the API's own `appsettings` — rather than a fixture written to match +them. On its first run it found a loopback redirect URI the realm registered in a form Keycloak rejects, +and a JSON configuration gap that made the whole sync surface unreachable from the real client while every +other test passed. Both are the same class of bug: two sides of a stub agreeing with each other about +something the specification never said. -```bash -DODOSSH_E2E=1 dotnet test tests/DodoSSH.SystemTests -``` - -It signs in through a real Keycloak, enrolls, unlocks, creates a host, syncs it, reads it back on a second -simulated machine, unlocks again with no network, and opens a shell on a real `sshd`. Skipped otherwise, -with a message naming the commands above. - -It is worth its weight: on its first run it found a loopback redirect URI the realm registered in a form -Keycloak rejects, and a JSON configuration gap that made the whole sync surface unreachable from the real -client while every other test passed. Both are the same class of bug — two sides of a stub agreeing with -each other about something the specification never said. +The one value it cannot take from a committed file is `Oidc:Authority`, since the container's port is +assigned at start. Everything that authority points at is still the real realm. Development and testing are currently **Windows-only**. Anything known or suspected to differ on Linux and macOS is tracked in [`docs/platform-flags.md`](docs/platform-flags.md), along with the diff --git a/docs/platform-flags.md b/docs/platform-flags.md index 893b6d9..e2e6fd2 100644 --- a/docs/platform-flags.md +++ b/docs/platform-flags.md @@ -137,6 +137,10 @@ database inside the container, so the realm has to be recreated along with it: `docker compose rm -sf keycloak && docker compose up -d keycloak`. Cost an otherwise inexplicable debugging detour. +`DodoSSH.SystemTests` is immune to this by construction — its Keycloak is created and destroyed per run — +which is a second reason the end-to-end suite starts its own containers rather than reusing the developer's +stack. Editing the realm file and rerunning the suite always tests the edit. + ## Local cache **The cache location is per-OS and must stay non-roaming.** `ClientPaths` chooses it: @@ -189,6 +193,18 @@ macOS runners have no Docker daemon, and the Windows CI job is deliberately buil anything proved by an integration test is proved on Linux only — which is the right place for server code, and no coverage at all for client platform behaviour. +**The end-to-end suite launches the API's own launcher executable**, falling back to `dotnet exec` on the +assembly. The fallback exists for one reason: a checkout or artefact copy that lost the execute bit +produces a `Win32Exception` on Linux and nothing whatsoever on Windows. *Verified on Windows only* — the +launcher path is what runs here, so the fallback itself is reasoned rather than exercised. If the suite +fails in CI with a permission error before any container work, that is the path to look at. + +**It also depends on `Server:PublicBaseUrl` being knowable before startup.** The port is chosen by binding +a loopback socket and releasing it, because the API reads that URL at startup and advertises it to clients, +so it cannot be discovered from Kestrel afterwards. The window for another process to take the port is a +few milliseconds; if the suite ever fails with an address-in-use, this is why, and a retry is the fix +rather than a redesign. + **`[CallerFilePath]` is rewritten to `/_/...` under `ContinuousIntegrationBuild`.** Any test that locates a fixture by source path passes locally and fails in CI. Copy fixtures to the output directory and read them via `AppContext.BaseDirectory` instead; `GoldenVectorTests` shows the diff --git a/src/DodoSSH.Infrastructure/DodoDbContextFactory.cs b/src/DodoSSH.Infrastructure/DodoDbContextFactory.cs index 1c14113..71d8262 100644 --- a/src/DodoSSH.Infrastructure/DodoDbContextFactory.cs +++ b/src/DodoSSH.Infrastructure/DodoDbContextFactory.cs @@ -7,18 +7,36 @@ namespace DodoSSH.Infrastructure; /// Builds a context for dotnet ef at design time. /// /// -/// Deliberately independent of the API host: generating a migration should not require the web -/// application to start, nor a reachable database. The connection string here is never used to -/// connect — only to select the provider so the model can be built. +/// Deliberately independent of the API host: generating or applying a migration should not require the web +/// application to start. The API never migrates in production — it fails readiness instead — so this is the +/// one place that knows how to open the database without a web host, and the end-to-end suite uses it for +/// the same reason dotnet ef does. /// public sealed class DodoDbContextFactory : IDesignTimeDbContextFactory { - /// - public DodoDbContext CreateDbContext(string[] args) - { - var connectionString = Environment.GetEnvironmentVariable("DODOSSH_DESIGN_CONNECTION") - ?? "Host=localhost;Database=dodossh_design;Username=postgres"; + /// Environment variable naming the database to work against. + /// + /// Only database update and dbcontext script actually connect. Generating a migration + /// needs a provider but not a reachable server, which is why there is a usable default at all. + /// + public const string ConnectionVariable = "DODOSSH_DESIGN_CONNECTION"; + /// + public DodoDbContext CreateDbContext(string[] args) => + Create(Environment.GetEnvironmentVariable(ConnectionVariable) + ?? "Host=localhost;Database=dodossh_design;Username=postgres"); + + /// + /// Builds a context for one database, configured as the API configures its own. + /// + /// Where to connect. + /// + /// The migrations history table has to be placed exactly where AddDodoPersistence looks for it. + /// Defined here once so that a schema applied outside the API is a schema the API agrees is current, + /// rather than one it reports as entirely pending. + /// + public static DodoDbContext Create(string connectionString) + { var options = new DbContextOptionsBuilder() .UseNpgsql(connectionString, npgsql => npgsql.MigrationsHistoryTable("__EFMigrationsHistory", DodoDbContext.SchemaName)) diff --git a/tests/DodoSSH.SystemTests/ApiProcess.cs b/tests/DodoSSH.SystemTests/ApiProcess.cs new file mode 100644 index 0000000..c1d5891 --- /dev/null +++ b/tests/DodoSSH.SystemTests/ApiProcess.cs @@ -0,0 +1,302 @@ +using System.ComponentModel; +using System.Diagnostics; +using System.Globalization; +using System.Net; +using System.Net.Sockets; +using System.Reflection; +using System.Text; + +namespace DodoSSH.SystemTests; + +/// +/// The DodoSSH API, running as a child process for the length of a test run. +/// +/// +/// +/// A process rather than a WebApplicationFactory, which is what the API's own integration suite +/// uses. Two reasons, and both are the point of this suite. The client establishes its connection by +/// creating an HttpClient for a URL the user typed, so there is no seam to hand a test handler +/// through without inventing one that exists only for tests. And a test host replaces the entry point, +/// Kestrel and the content root — so it never proves that Program.cs composes, that the committed +/// appsettings.json is found and layered in the documented order, or that the server answers on a +/// socket. +/// +/// +/// It runs out of the API's own output directory, which is what makes its configuration real: the working +/// directory becomes the content root, so the appsettings.json beside the assembly is the one that +/// ships. Only the values that cannot be known before the containers start are overridden, through the +/// DODOSSH_ environment prefix that Program.cs adds last and which therefore wins. +/// +/// +internal sealed class ApiProcess : IAsyncDisposable +{ + /// Assembly metadata written by the DodoCaptureApiPath target in this csproj. + private const string ApiPathKey = "DodoSSH.ApiAssemblyPath"; + + private static readonly TimeSpan ReadyTimeout = TimeSpan.FromSeconds(90); + private static readonly TimeSpan PollInterval = TimeSpan.FromMilliseconds(250); + + private readonly Process process; + private readonly StringBuilder log = new(); + + private ApiProcess(Process process, Uri baseUrl) + { + this.process = process; + BaseUrl = baseUrl; + + process.OutputDataReceived += Capture; + process.ErrorDataReceived += Capture; + process.BeginOutputReadLine(); + process.BeginErrorReadLine(); + } + + /// Where the API is listening. + internal Uri BaseUrl { get; } + + /// Everything the API has written to its console. + /// + /// Kept so a failure can be explained. A child process that dies during startup is otherwise a + /// connection refused with no cause, which is a genuinely miserable thing to debug. + /// + internal string Log + { + get + { + lock (log) + { + return log.ToString(); + } + } + } + + /// Starts the API against the given database and identity provider, and waits for readiness. + internal static async Task StartAsync( + string connectionString, + Uri authority, + CancellationToken cancellationToken) + { + var baseUrl = new Uri( + string.Create(CultureInfo.InvariantCulture, $"http://127.0.0.1:{FreeLoopbackPort()}")); + + var api = new ApiProcess(Launch(baseUrl, connectionString, authority), baseUrl); + + try + { + await api.WaitUntilReadyAsync(cancellationToken); + return api; + } + catch + { + await api.DisposeAsync(); + throw; + } + } + + /// + public async ValueTask DisposeAsync() + { + try + { + if (!process.HasExited) + { + // No graceful shutdown: Ctrl+C cannot be delivered to a specific child on Windows, and + // the drain this would exercise belongs to the relay, which M1 does not ship. + process.Kill(entireProcessTree: true); + } + } + catch (InvalidOperationException) + { + // Exited between the check and the kill. + } + + await process.WaitForExitAsync(CancellationToken.None); + + process.Dispose(); + } + + private static Process Launch(Uri baseUrl, string connectionString, Uri authority) + { + var assembly = ResolveApiAssembly(); + var launcher = AppHost(assembly); + + var startInfo = new ProcessStartInfo + { + FileName = launcher ?? "dotnet", + + // The content root: WebApplication.CreateBuilder takes the working directory, and the + // appsettings files live beside the assembly. + WorkingDirectory = Path.GetDirectoryName(assembly)!, + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false, + }; + + if (launcher is null) + { + startInfo.ArgumentList.Add("exec"); + startInfo.ArgumentList.Add(assembly); + } + + // Development, because the committed development configuration is precisely what this suite + // exists to check — the realm file and appsettings.Development.json are a pair, and the first bug + // this suite found lived in exactly that pair. + startInfo.Environment["ASPNETCORE_ENVIRONMENT"] = "Development"; + startInfo.Environment["ASPNETCORE_URLS"] = baseUrl.ToString(); + + startInfo.Environment["DODOSSH_ConnectionStrings__Postgres"] = connectionString; + startInfo.Environment["DODOSSH_Oidc__Authority"] = authority.ToString(); + + // Advertised to clients in the discovery document, so it has to be the port actually bound + // rather than the 5233 a developer runs on. + startInfo.Environment["DODOSSH_Server__PublicBaseUrl"] = baseUrl.ToString(); + startInfo.Environment["DODOSSH_Relay__WebSocketUrl"] = + new UriBuilder(baseUrl) { Scheme = "ws", Path = "/api/v1/relay/connect" }.Uri.ToString(); + + try + { + return Start(startInfo); + } + catch (Win32Exception) when (launcher is not null) + { + // The launcher is there but would not run. On Linux a checkout or artefact copy that lost the + // execute bit is the usual cause, and the runtime can load the assembly directly regardless — + // so this falls back rather than failing CI on a file mode. + startInfo.FileName = "dotnet"; + startInfo.ArgumentList.Insert(0, assembly); + startInfo.ArgumentList.Insert(0, "exec"); + + return Start(startInfo); + } + } + + private static Process Start(ProcessStartInfo startInfo) => + Process.Start(startInfo) + ?? throw new InvalidOperationException( + $"Could not start the API: {startInfo.FileName} produced no process."); + + /// + /// The native launcher beside the assembly, if the build produced one. + /// + /// + /// Preferred over dotnet exec only because it needs nothing on PATH. The fallback is not + /// hypothetical tidiness: a build with UseAppHost=false produces no launcher at all. + /// + private static string? AppHost(string assembly) + { + var candidate = Path.ChangeExtension(assembly, OperatingSystem.IsWindows() ? ".exe" : null); + + return File.Exists(candidate) ? candidate : null; + } + + private static string ResolveApiAssembly() + { + var path = typeof(ApiProcess).Assembly + .GetCustomAttributes() + .FirstOrDefault(attribute => string.Equals(attribute.Key, ApiPathKey, StringComparison.Ordinal)) + ?.Value; + + if (string.IsNullOrEmpty(path)) + { + throw new InvalidOperationException( + $"The build did not record {ApiPathKey}. The DodoCaptureApiPath target in " + + "DodoSSH.SystemTests.csproj is what writes it."); + } + + if (!File.Exists(path)) + { + throw new InvalidOperationException( + $"The API was recorded at {path}, but nothing is there. Build the solution."); + } + + return path; + } + + /// + /// Bound and released rather than left at zero, because the port has to be known before the process + /// starts: it goes into Server:PublicBaseUrl, which the API reads at startup and hands to + /// clients. The window in which another process could take it is a few milliseconds wide, and the + /// alternative — reading the bound port back out of Kestrel's log line — cannot be fed to + /// configuration that has already been read. + /// + private static int FreeLoopbackPort() + { + using var probe = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); + + probe.Bind(new System.Net.IPEndPoint(System.Net.IPAddress.Loopback, 0)); + + return ((System.Net.IPEndPoint)probe.LocalEndPoint!).Port; + } + + private void Capture(object sender, DataReceivedEventArgs args) + { + if (args.Data is null) + { + return; + } + + lock (log) + { + log.AppendLine(args.Data); + } + } + + /// + /// Readiness rather than liveness, and that distinction is load-bearing: /healthz/ready also + /// asserts the database is reachable and that no migration is pending. Waiting on it means a schema the + /// fixture failed to apply is reported here, against the API's own opinion of it, rather than as an + /// inscrutable 500 in the middle of enrollment. + /// + private async Task WaitUntilReadyAsync(CancellationToken cancellationToken) + { + using var deadline = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + deadline.CancelAfter(ReadyTimeout); + + using var http = new HttpClient { Timeout = TimeSpan.FromSeconds(5) }; + + var ready = new Uri(BaseUrl, "healthz/ready"); + + try + { + while (!deadline.IsCancellationRequested && !process.HasExited) + { + if (await RespondsAsync(http, ready, deadline.Token)) + { + return; + } + + await Task.Delay(PollInterval, deadline.Token); + } + } + catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) + { + // The deadline, not the caller. Reported below with the API's own output. + } + + cancellationToken.ThrowIfCancellationRequested(); + + throw new InvalidOperationException( + string.Create( + CultureInfo.InvariantCulture, + $"The API did not become ready at {ready} within {ReadyTimeout.TotalSeconds:N0}s. " + + $"{(process.HasExited ? $"It exited with code {process.ExitCode}." : "It is still running.")}" + + $" Its output was:{Environment.NewLine}{Log}")); + } + + private static async Task RespondsAsync(HttpClient http, Uri url, CancellationToken token) + { + try + { + using var response = await http.GetAsync(url, token); + return response.IsSuccessStatusCode; + } + catch (HttpRequestException) + { + return false; + } + catch (TaskCanceledException) + { + // Either the per-request timeout or the deadline. The loop condition decides which. + return false; + } + } +} diff --git a/tests/DodoSSH.SystemTests/DevStack.cs b/tests/DodoSSH.SystemTests/DevStack.cs index 6ff3f0a..d73bd4a 100644 --- a/tests/DodoSSH.SystemTests/DevStack.cs +++ b/tests/DodoSSH.SystemTests/DevStack.cs @@ -2,85 +2,194 @@ using System.Globalization; using System.Net.Http.Json; using System.Text.Json; using System.Text.Json.Nodes; +using DodoSSH.Infrastructure; +using DotNet.Testcontainers.Builders; +using DotNet.Testcontainers.Containers; +using Microsoft.EntityFrameworkCore; +using Testcontainers.PostgreSql; namespace DodoSSH.SystemTests; /// -/// The development stack this suite talks to, and how to find out whether it is there. +/// The whole stack the end-to-end suite talks to, brought up per run and thrown away after. /// /// -/// Deliberately targets a stack the developer brought up rather than starting its own containers. That -/// makes the suite test the configuration that is actually committed — the realm file, the API's -/// appsettings, the migration history — instead of a parallel arrangement assembled for the test, which is -/// where a divergence between "works in the suite" and "works when you run it" comes from. /// -/// The trade is that it cannot run unattended, so it is opt-in and says exactly what to start. +/// PostgreSQL, Keycloak and an OpenSSH server in containers; the API as a child process out of its own +/// build output. Nothing is stubbed and nothing has to be started by hand, which is what allows this suite +/// to run in CI rather than only on the machine of whoever remembered the commands. +/// +/// +/// It deliberately consumes the artefacts that ship — the realm file from deploy/keycloak, the +/// committed EF migrations, the API's own appsettings — rather than an arrangement written to match +/// them. That distinction is the whole value of the suite: a fixture that restates the configuration agrees +/// with itself by construction, and both bugs this suite has found were in the committed configuration, +/// invisible to every other test. +/// +/// +/// One thing cannot come from a committed file: the identity provider's port is assigned when its container +/// starts, so Oidc:Authority is overridden to match. Everything the authority points at — the realm, +/// the public client, the loopback redirect URI, the audience mapper — is still the real file, and the URL +/// is the one value a self-hosted deployment is guaranteed to change anyway. +/// +/// +/// A fresh Keycloak per run also removes a footgun the development stack has: --import-realm skips a +/// realm that already exists, so editing the realm file and restarting the container serves the old +/// configuration and looks exactly like the edit being wrong. /// /// -internal static class DevStack +public sealed class DevStack : IAsyncLifetime { - internal const string ApiBaseUrl = "http://localhost:5233"; - internal const string KeycloakBaseUrl = "http://localhost:18080"; + /// The realm the committed file defines. internal const string Realm = "dodossh"; - private const string OptInVariable = "DODOSSH_E2E"; + /// The account the OpenSSH container is built around. + internal const string SshUsername = "dodo"; + + /// Its password. A container that lives for one test run, on a random loopback port. + internal const string SshPassword = "correct-horse-battery-staple"; - /// - /// Explains why the suite cannot run, or returns null when it can. - /// /// - /// A message rather than a boolean, because a skipped end-to-end suite that does not say what is - /// missing is a suite nobody ever runs again. + /// Pinned to the same version as deploy/docker-compose.dev.yml. Two places, deliberately: the + /// suite cannot read the compose file without becoming a compose parser, and a version skew between + /// them shows up here as a failure rather than as a difference nobody notices. /// - internal static async Task WhyUnavailableAsync(CancellationToken cancellationToken) + private const string KeycloakImage = "quay.io/keycloak/keycloak:26.4"; + + private const string PostgresImage = "postgres:18-alpine"; + private const string SshImage = "linuxserver/openssh-server:latest"; + + private const ushort KeycloakPort = 8080; + private const ushort SshPort = 2222; + + /// + /// Generous, and covering image pulls on a cold machine. It exists so that a daemon that never answers + /// fails with a sentence rather than hanging until the test host is killed. + /// + private static readonly TimeSpan StartupTimeout = TimeSpan.FromMinutes(8); + + private readonly PostgreSqlContainer postgres = new PostgreSqlBuilder(PostgresImage) + .WithDatabase("dodossh") + .WithUsername("dodossh") + .WithPassword("dodossh") + + // Matching the compose stack: deterministic collation, as the server's InvariantGlobalization + // implies. Ordering differences between a C locale and a language-aware one are exactly the kind + // of thing that behaves in development and not in production. + .WithEnvironment("POSTGRES_INITDB_ARGS", "--encoding=UTF8 --locale=C") + .Build(); + + private readonly IContainer keycloak = new ContainerBuilder(KeycloakImage) + // start-dev, as the development stack does: no HTTPS enforcement and throwaway H2 state. The realm + // is imported from the committed file, which is the artefact under test here. + .WithCommand("start-dev", "--import-realm") + .WithEnvironment("KC_BOOTSTRAP_ADMIN_USERNAME", "admin") + .WithEnvironment("KC_BOOTSTRAP_ADMIN_PASSWORD", "admin") + .WithPortBinding(KeycloakPort, assignRandomHostPort: true) + .WithResourceMapping( + new FileInfo(Path.Combine(AppContext.BaseDirectory, "realm-dodossh.json")), + "/opt/keycloak/data/import/") + + // Serving the realm's discovery document is the only readiness that means anything: the port opens + // well before the import finishes, and an unimported realm answers 404 to every request the client + // is about to make. + .WithWaitStrategy(Wait.ForUnixContainer().UntilHttpRequestIsSucceeded(request => request + .ForPort(KeycloakPort) + .ForPath($"/realms/{Realm}/.well-known/openid-configuration"))) + .Build(); + + private readonly IContainer sshd = new ContainerBuilder(SshImage) + .WithEnvironment("PUID", "1000") + .WithEnvironment("PGID", "1000") + .WithEnvironment("USER_NAME", SshUsername) + .WithEnvironment("USER_PASSWORD", SshPassword) + .WithEnvironment("PASSWORD_ACCESS", "true") + .WithEnvironment("SUDO_ACCESS", "false") + .WithPortBinding(SshPort, assignRandomHostPort: true) + + // A published port is not readiness: the entrypoint generates host keys and rewrites sshd_config + // first. This is the check that actually proves sshd is accepting. + .WithWaitStrategy(Wait.ForUnixContainer() + .UntilCommandIsCompleted("sh", "-c", $"netstat -ltn | grep -q ':{SshPort}'")) + .Build(); + + private ApiProcess? api; + + /// Where the API is listening, which is the only URL the client is told. + internal Uri ApiBaseUrl => Api.BaseUrl; + + /// The identity provider's base URL, port included. + internal Uri KeycloakBaseUrl => new(string.Create( + CultureInfo.InvariantCulture, + $"http://{keycloak.Hostname}:{keycloak.GetMappedPublicPort(KeycloakPort)}")); + + /// The issuer the API is configured to trust. + internal Uri Authority => new(KeycloakBaseUrl, $"/realms/{Realm}"); + + /// Where the OpenSSH container answers. + internal string SshHostname => sshd.Hostname; + + /// Its mapped port. + internal int SshHostPort => sshd.GetMappedPublicPort(SshPort); + + /// The API's console output, for explaining a failure. + internal string ApiLog => Api.Log; + + private ApiProcess Api => api + ?? throw new InvalidOperationException("The stack has not been started."); + + /// + public async ValueTask InitializeAsync() { - if (Environment.GetEnvironmentVariable(OptInVariable) is not "1") + using var deadline = new CancellationTokenSource(StartupTimeout); + + // Concurrently: none of the three depends on another, and Keycloak alone takes longer than the + // other two together. + await Task.WhenAll( + postgres.StartAsync(deadline.Token), + keycloak.StartAsync(deadline.Token), + sshd.StartAsync(deadline.Token)); + + await MigrateAsync(deadline.Token); + + api = await ApiProcess.StartAsync(postgres.GetConnectionString(), Authority, deadline.Token); + } + + /// + public async ValueTask DisposeAsync() + { + // The API first: it holds connections to PostgreSQL, and tearing the database out from under it + // produces a page of exceptions that look like a failure rather than a shutdown. + if (api is not null) { - return $"Set {OptInVariable}=1 to run the end-to-end suite. It needs the development stack:" - + "\n docker compose -f deploy/docker-compose.dev.yml up -d" - + "\n dotnet ef database update --project src/DodoSSH.Infrastructure" - + "\n dotnet run --project src/DodoSSH.Api"; + await api.DisposeAsync(); } - using var http = new HttpClient { Timeout = TimeSpan.FromSeconds(5) }; - - if (!await RespondsAsync(http, $"{ApiBaseUrl}/healthz/ready", cancellationToken).ConfigureAwait(false)) - { - return $"The API is not ready at {ApiBaseUrl}. Start it with " - + "`dotnet run --project src/DodoSSH.Api`, and check that migrations have been applied — " - + "readiness fails while any are pending, by design."; - } - - var discovery = $"{KeycloakBaseUrl}/realms/{Realm}/.well-known/openid-configuration"; - - if (!await RespondsAsync(http, discovery, cancellationToken).ConfigureAwait(false)) - { - return $"Keycloak is not serving the '{Realm}' realm at {KeycloakBaseUrl}. Bring it up with " - + "`docker compose -f deploy/docker-compose.dev.yml up -d`."; - } - - return null; + await Task.WhenAll( + postgres.DisposeAsync().AsTask(), + keycloak.DisposeAsync().AsTask(), + sshd.DisposeAsync().AsTask()); } /// /// Creates a Keycloak user this test run owns. /// /// - /// A fresh account per run, rather than the realm's alice. Enrollment happens once per account - /// and cannot be undone from the client, so reusing an account would mean the second run exercises a - /// different path from the first and neither could assert an exact vault state. This way every run - /// starts from "no identity key, no vault". + /// A fresh account rather than the realm's alice. Enrollment happens once per account and cannot + /// be undone from the client, so an account shared between tests would mean the second one exercises a + /// different path from the first and neither could assert an exact vault state. The realm is thrown + /// away with the container, so this only has to be unique within a run — but making it unique per call + /// is what keeps a second test from silently depending on the first. /// - internal static async Task CreateUserAsync(CancellationToken cancellationToken) + internal async Task CreateUserAsync(CancellationToken cancellationToken) { - var username = string.Create( - CultureInfo.InvariantCulture, $"e2e-{Guid.CreateVersion7():N}"); + var username = string.Create(CultureInfo.InvariantCulture, $"e2e-{Guid.CreateVersion7():N}"); const string Password = "e2e-password"; - using var http = new HttpClient { BaseAddress = new Uri(KeycloakBaseUrl) }; + using var http = new HttpClient { BaseAddress = KeycloakBaseUrl }; - var token = await AdminTokenAsync(http, cancellationToken).ConfigureAwait(false); + var token = await AdminTokenAsync(http, cancellationToken); http.DefaultRequestHeaders.Authorization = new("Bearer", token); @@ -103,15 +212,28 @@ internal static class DevStack }, }; - using var response = await http - .PostAsJsonAsync($"/admin/realms/{Realm}/users", user, cancellationToken) - .ConfigureAwait(false); + using var response = await http.PostAsJsonAsync( + $"/admin/realms/{Realm}/users", user, cancellationToken); response.EnsureSuccessStatusCode(); return new DevStackUser(username, Password); } + /// + /// Through the same design-time factory dotnet ef database update uses, so the migrations and + /// the history table land exactly where the API expects them. If they did not, the API's readiness + /// check would refuse to pass and would say so — the schema is verified by the + /// server's own opinion of it rather than by this fixture's. + /// + private async Task MigrateAsync(CancellationToken cancellationToken) + { + var context = DodoDbContextFactory.Create(postgres.GetConnectionString()); + await using var scope = context.ConfigureAwait(false); + + await context.Database.MigrateAsync(cancellationToken); + } + private static async Task AdminTokenAsync(HttpClient http, CancellationToken cancellationToken) { using var form = new FormUrlEncodedContent( @@ -123,37 +245,16 @@ internal static class DevStack ["grant_type"] = "password", }); - using var response = await http - .PostAsync("/realms/master/protocol/openid-connect/token", form, cancellationToken) - .ConfigureAwait(false); + using var response = await http.PostAsync( + "/realms/master/protocol/openid-connect/token", form, cancellationToken); response.EnsureSuccessStatusCode(); - var body = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); + var body = await response.Content.ReadAsStringAsync(cancellationToken); return JsonDocument.Parse(body).RootElement.GetProperty("access_token").GetString() ?? throw new InvalidOperationException("Keycloak returned no admin access token."); } - - private static async Task RespondsAsync( - HttpClient http, - string url, - CancellationToken cancellationToken) - { - try - { - using var response = await http.GetAsync(url, cancellationToken).ConfigureAwait(false); - return response.IsSuccessStatusCode; - } - catch (HttpRequestException) - { - return false; - } - catch (TaskCanceledException) - { - return false; - } - } } /// An account created for one test run. diff --git a/tests/DodoSSH.SystemTests/DodoSSH.SystemTests.csproj b/tests/DodoSSH.SystemTests/DodoSSH.SystemTests.csproj index fb4df43..34ea7f7 100644 --- a/tests/DodoSSH.SystemTests/DodoSSH.SystemTests.csproj +++ b/tests/DodoSSH.SystemTests/DodoSSH.SystemTests.csproj @@ -7,34 +7,60 @@ Nothing here is stubbed. Every other suite in the repository substitutes something — a stubbed identity provider, an in-memory vault server, an in-memory cache — and each of those substitutions is a place a misunderstanding of the protocol can hide on both sides at once. This suite exists to catch exactly - that, and it already has: the realm file registered a loopback redirect URI that Keycloak rejects, - which no stub would ever have noticed. + that, and it already has: the realm file registered a loopback redirect URI that Keycloak rejects, and + the API never applied the contract's JSON settings, which put the whole sync surface out of reach of + the real client. No stub would have noticed either. - It needs the development stack up and is skipped otherwise. The commands are in the README under - "End-to-end verification"; a skipped run prints them too. (They cannot be repeated here: an XML - comment may not contain a double hyphen, and every one of them has a flag.) + It needs a Docker daemon and nothing else. See DevStack for what it starts and why. --> - - - --ignore-exit-code 8 - - + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/DodoSSH.SystemTests/M1VerticalSliceTests.cs b/tests/DodoSSH.SystemTests/M1VerticalSliceTests.cs index a28a31f..c01cf1e 100644 --- a/tests/DodoSSH.SystemTests/M1VerticalSliceTests.cs +++ b/tests/DodoSSH.SystemTests/M1VerticalSliceTests.cs @@ -5,8 +5,6 @@ using DodoSSH.Client.Ssh; using DodoSSH.Client.Storage; using DodoSSH.Contracts; using DodoSSH.Crypto; -using DotNet.Testcontainers.Builders; -using DotNet.Testcontainers.Containers; namespace DodoSSH.SystemTests; @@ -28,12 +26,9 @@ namespace DodoSSH.SystemTests; /// between tests or repeating a minute of setup per assertion. /// /// -public sealed class M1VerticalSliceTests : IAsyncLifetime +public sealed class M1VerticalSliceTests(DevStack stack) : IClassFixture, IAsyncDisposable { private const string Passphrase = "an end to end passphrase"; - private const string SshUsername = "dodo"; - private const string SshPassword = "correct-horse-battery-staple"; - private const int SshPort = 2222; /// /// 64 MiB is the floor EnrollmentLimits enforces, and this suite has to respect it — the other @@ -46,68 +41,25 @@ public sealed class M1VerticalSliceTests : IAsyncLifetime private readonly List directories = []; - private string? unavailable; - private IContainer? sshd; - /// - public async ValueTask InitializeAsync() + public ValueTask DisposeAsync() { - unavailable = await DevStack.WhyUnavailableAsync(TestContext.Current.CancellationToken); - - if (unavailable is not null) - { - // No container either. Paying half a minute of startup for a suite about to skip is how an - // opt-in suite becomes one nobody opts into. - return; - } - - sshd = new ContainerBuilder("linuxserver/openssh-server:latest") - .WithEnvironment("PUID", "1000") - .WithEnvironment("PGID", "1000") - .WithEnvironment("USER_NAME", SshUsername) - .WithEnvironment("USER_PASSWORD", SshPassword) - .WithEnvironment("PASSWORD_ACCESS", "true") - .WithEnvironment("SUDO_ACCESS", "false") - .WithPortBinding(SshPort, assignRandomHostPort: true) - - // A published port is not readiness: the entrypoint generates host keys and rewrites - // sshd_config first. Both conditions are needed, and the port check is the one that actually - // proves sshd is accepting. - .WithWaitStrategy(Wait.ForUnixContainer() - .UntilCommandIsCompleted("sh", "-c", $"netstat -ltn | grep -q ':{SshPort}'")) - - .Build(); - - await sshd.StartAsync(TestContext.Current.CancellationToken); - } - - /// - public async ValueTask DisposeAsync() - { - if (sshd is not null) - { - await sshd.DisposeAsync(); - } - foreach (var directory in directories.Where(Directory.Exists)) { Directory.Delete(directory, recursive: true); } + + return ValueTask.CompletedTask; } [Fact] public async Task TheWholeSlice() { - if (unavailable is not null) - { - Assert.Skip(unavailable); - } - - var account = await DevStack.CreateUserAsync(Token); + var account = await stack.CreateUserAsync(Token); var browser = new ScriptedBrowser(account.Username, account.Password); using var connection = await ServerConnection - .SignInAsync(new Uri(DevStack.ApiBaseUrl), browser, TimeProvider.System, Token); + .SignInAsync(stack.ApiBaseUrl, browser, TimeProvider.System, Token); AssertDiscoveredFromTheServer(connection); @@ -139,17 +91,23 @@ public sealed class M1VerticalSliceTests : IAsyncLifetime /// The user typed one server URL. Everything about the identity provider — the authority, the client /// id, the scopes — came back from the server, which is the whole onboarding story. /// - private static void AssertDiscoveredFromTheServer(ServerConnection connection) + private void AssertDiscoveredFromTheServer(ServerConnection connection) { connection.Configuration.Oidc.Authority.ToString() - .ShouldStartWith($"{DevStack.KeycloakBaseUrl}/realms/{DevStack.Realm}"); + .ShouldStartWith(stack.Authority.ToString()); connection.Configuration.Oidc.ClientId.ShouldBe("dodossh-desktop"); + + // Server:PublicBaseUrl, which is what a client behind a proxy would follow. Worth asserting + // because it is configuration the server states about itself and nothing else would notice it + // being wrong. + connection.Configuration.ApiBaseUrl.ShouldBe(stack.ApiBaseUrl); + connection.Meta.SyncProtocolVersion.ShouldBe(1); connection.Meta.CryptoSpecVersion.ShouldBe(1); } - private static async Task EnrollAsync( + private async Task EnrollAsync( ServerConnection connection, ClientCacheFactory caches, ScriptedBrowser browser) @@ -157,11 +115,11 @@ public sealed class M1VerticalSliceTests : IAsyncLifetime var provisioner = new AccountProvisioner( connection.Account, connection.KeyBinding, caches, TimeProvider.System, ServerFloorProfile); - var before = await provisioner.RefreshAsync(DevStack.ApiBaseUrl, Token); + var before = await provisioner.RefreshAsync(ServerUrl, Token); before.Status.ShouldBe(ProvisionStatus.EnrollmentRequired); var enrolled = await provisioner.EnrollAsync( - DevStack.ApiBaseUrl, Passphrase, "e2e-laptop", "Personal", Token); + ServerUrl, Passphrase, "e2e-laptop", "Personal", Token); enrolled.Status.ShouldBe(ProvisionStatus.Ready); enrolled.RecoveryCode.ShouldNotBeNullOrWhiteSpace(); @@ -211,7 +169,7 @@ public sealed class M1VerticalSliceTests : IAsyncLifetime connection.Account, connection.KeyBinding, desktopCache, TimeProvider.System, ServerFloorProfile); // Already enrolled, so this only caches what an offline unlock will need. - (await provisioner.RefreshAsync(DevStack.ApiBaseUrl, Token)).Status + (await provisioner.RefreshAsync(ServerUrl, Token)).Status .ShouldBe(ProvisionStatus.Ready); var desktop = await UnlockAsync(desktopCache); @@ -253,7 +211,7 @@ public sealed class M1VerticalSliceTests : IAsyncLifetime var factory = new SshNetConnectionFactory(knownHosts); var request = new SshConnectionRequest( - host.Hostname, host.Port, host.Username!, new SshPasswordCredential(SshPassword)); + host.Hostname, host.Port, host.Username!, new SshPasswordCredential(DevStack.SshPassword)); try { @@ -280,20 +238,23 @@ public sealed class M1VerticalSliceTests : IAsyncLifetime private static CancellationToken Token => TestContext.Current.CancellationToken; - private static HostSecret BuildHostFor(string hostname, int port) => + /// + /// The provisioner takes the URL as a string because it is also the cache's identity — the value an + /// offline unlock compares against to refuse a cache belonging to another server. + /// + private string ServerUrl => stack.ApiBaseUrl.ToString(); + + private HostSecret BuildHost() => new() { Label = "e2e-target", - Hostname = hostname, - Port = port, - Username = SshUsername, + Hostname = stack.SshHostname, + Port = stack.SshHostPort, + Username = DevStack.SshUsername, Notes = "created by the end-to-end slice", Options = HostOptions.Create([new HostOption("ServerAliveInterval", "30")]), }; - private HostSecret BuildHost() => - BuildHostFor(sshd!.Hostname, sshd.GetMappedPublicPort(SshPort)); - private async Task OpenCacheAsync() { var directory = Path.Combine(Path.GetTempPath(), $"dodossh-e2e-{Guid.CreateVersion7():N}"); diff --git a/tests/DodoSSH.SystemTests/packages.lock.json b/tests/DodoSSH.SystemTests/packages.lock.json index 842c0cb..1f93669 100644 --- a/tests/DodoSSH.SystemTests/packages.lock.json +++ b/tests/DodoSSH.SystemTests/packages.lock.json @@ -46,6 +46,15 @@ "SharpZipLib": "1.4.2" } }, + "Testcontainers.PostgreSql": { + "type": "Direct", + "requested": "[4.13.0, )", + "resolved": "4.13.0", + "contentHash": "2ow4AE8drI9iA9Fr4ycAPusXPB1lJfQyyNONSMLE/XqLpm8VuNAh3pK38fOjkOWtTrnD03s4hAIdl4036Ik69A==", + "dependencies": { + "Testcontainers": "4.13.0" + } + }, "xunit.v3": { "type": "Direct", "requested": "[3.2.2, )", @@ -293,6 +302,14 @@ "resolved": "5.0.0", "contentHash": "dDoKi0PnDz31yAyETfRntsLArTlVAVzUzCIvvEDsDsucrl33Dl8pIJG06ePTJTI3tGpeyHS9Cq7Foc/s4EeKcg==" }, + "Npgsql": { + "type": "Transitive", + "resolved": "10.0.3", + "contentHash": "7nb5YzXuvWWJxB0J8DiyL3we+X4FOctZrt0fIBnucOIaIevFEEwGQVZKtiu9olXdlNAK1eNgqSral6r/jlhI4w==", + "dependencies": { + "Microsoft.Extensions.Logging.Abstractions": "10.0.0" + } + }, "SharpZipLib": { "type": "Transitive", "resolved": "1.4.2", @@ -447,6 +464,17 @@ "NSec.Cryptography": "[26.4.0, )" } }, + "dodossh.domain": { + "type": "Project" + }, + "dodossh.infrastructure": { + "type": "Project", + "dependencies": { + "DodoSSH.Domain": "[1.0.0, )", + "EFCore.NamingConventions": "[10.0.1, )", + "Npgsql.EntityFrameworkCore.PostgreSQL": "[10.0.3, )" + } + }, "BouncyCastle.Cryptography": { "type": "CentralTransitive", "requested": "[2.6.2, )", @@ -509,6 +537,17 @@ "SQLitePCLRaw.core": "2.1.11" } }, + "Npgsql.EntityFrameworkCore.PostgreSQL": { + "type": "CentralTransitive", + "requested": "[10.0.3, )", + "resolved": "10.0.3", + "contentHash": "IPGrrZnRkuW7OlHDhUESZz4G5DLkW7Nej/O3Cx+0iTsgyU5XJxBgpsvTHLloo3WWuAKKbDHXBvWPVkX1deRh1Q==", + "dependencies": { + "Microsoft.EntityFrameworkCore": "[10.0.4, 11.0.0)", + "Microsoft.EntityFrameworkCore.Relational": "[10.0.4, 11.0.0)", + "Npgsql": "10.0.3" + } + }, "NSec.Cryptography": { "type": "CentralTransitive", "requested": "[26.4.0, )",