From c74a2339aac449ec7ca86b4f18166ef66f0f981e Mon Sep 17 00:00:00 2001 From: Oleksandr Bezdieniezhnykh Date: Tue, 12 May 2026 22:19:26 +0300 Subject: [PATCH] [AZ-505] AC-5 fix: enable TLS for HTTP/2 via ALPN Kestrel with HttpProtocols.Http1AndHttp2 on a plaintext listener silently downgrades to HTTP/1.1-only (logs "HTTP/2 is not enabled ... TLS is not enabled"), so AC-5's multiplexed-GET test failed with HTTP_1_1_REQUIRED. ALPN cannot run over plaintext, so the fix switches the dev listener to TLS on https://+:8080: - scripts/run-tests.sh generates a self-signed dev cert idempotently (./certs/api.pfx + api.crt) via openssl in an alpine container; certs/ is gitignored. - docker-compose.yml binds Kestrel to ASPNETCORE_URLS=https://+:8080 with Kestrel__Certificates__Default__Path bound to the .pfx. - docker-compose.tests.yml mounts api.crt into the integration-tests container's CA store and runs update-ca-certificates so HttpClient trusts the cert transparently; default API_URL is now https://api:8080. - Drop the obsolete Http2UnencryptedSupport AppContext switch from Http2MultiplexingTests; ALPN over TLS handles negotiation. Test-data fixes caught on the post-TLS rerun (independent of the TLS switch but surfaced together): - Http2MultiplexingTests: switch slippy coords from (154321, 95812) -- which Google Maps returns 404 for -- to (158485, 91707), the slippy projection of (47.461747, 37.647063) already exercised by JwtIntegrationTests. - TileInventoryTests + LeafletPathIndexOnlyTests: SpecifyKind to Unspecified at the binding site for raw Npgsql seed paths writing into tiles.captured_at / created_at / updated_at (TIMESTAMP without tz). Npgsql v6+ refuses Kind=Utc into plain timestamp columns; production goes through Dapper and never hits this code path. - MigrationTests Az503NewUniqueIndexCoversIntegerKeyAndFlightId: accept either idx_tiles_location_hash (migration 014) or its AZ-505 successor tiles_leaflet_path (migration 015) -- both have location_hash as the leading column, which is the AC-9 intent. Docs updated to reflect the TLS+ALPN path: tile-inventory.md Non-Goals, modules/api_program.md, module-layout.md, the AZ-505 task spec's Risk 3, and the cycle 6 implementation + completeness reports. The full integration test suite passes (mode=full, exit 0). Co-authored-by: Cursor --- .gitignore | 1 + SatelliteProvider.Api/Program.cs | 14 +++--- .../Http2MultiplexingTests.cs | 30 ++++++------ .../LeafletPathIndexOnlyTests.cs | 4 +- .../MigrationTests.cs | 16 +++++-- SatelliteProvider.IntegrationTests/Program.cs | 2 +- .../TileInventoryTests.cs | 7 ++- .../contracts/api/tile-inventory.md | 4 +- _docs/02_document/module-layout.md | 2 +- _docs/02_document/modules/api_program.md | 2 +- ...-505_tile_inventory_http2_leaflet_index.md | 4 +- ...plementation_completeness_cycle6_report.md | 6 ++- ...ementation_report_tile_inventory_cycle6.md | 22 ++++++++- _docs/_autodev_state.md | 4 +- docker-compose.tests.yml | 14 +++++- docker-compose.yml | 10 +++- scripts/run-tests.sh | 48 +++++++++++++++++++ 17 files changed, 148 insertions(+), 42 deletions(-) diff --git a/.gitignore b/.gitignore index 333906f..46c8cc2 100644 --- a/.gitignore +++ b/.gitignore @@ -16,3 +16,4 @@ coverage.cobertura.xml coverage.opencover.xml *.coverage _docs/03_implementation/test_runs/ +certs/ diff --git a/SatelliteProvider.Api/Program.cs b/SatelliteProvider.Api/Program.cs index e41c73b..d0905ec 100644 --- a/SatelliteProvider.Api/Program.cs +++ b/SatelliteProvider.Api/Program.cs @@ -40,12 +40,14 @@ builder.Services.Configure(options => { options.Limits.MaxRequestBodySize = uavBatchBodyLimit; // AZ-505: enable HTTP/2 alongside HTTP/1.1 on every Kestrel endpoint so - // programmatic clients (httpx http2=True, .NET HttpClient with - // HttpVersionPolicy.RequestVersionExact) can multiplex tile reads on a - // single TCP connection. Browsers cannot use h2c (HTTP/2 cleartext) - // without ALPN+TLS, so they continue on HTTP/1.1 — the win for browsers - // is the AZ-505 covering-index hot path, not multiplexing. HTTP/3/QUIC - // is intentionally out of scope (see AZ-505 task spec § Excluded). + // programmatic clients (httpx http2=True, .NET HttpClient) can multiplex + // tile reads on a single TCP connection. Kestrel requires TLS+ALPN for + // HTTP/2 — the dev/test compose files mount a self-signed cert at + // /app/certs/api.pfx and set ASPNETCORE_URLS=https://+:8080; production + // is expected to terminate TLS at the same layer or upstream. Browsers + // negotiate HTTP/2 via ALPN once TLS is present; legacy HTTP/1.1 + // callers continue to work over the same listener. HTTP/3/QUIC is + // intentionally out of scope (see AZ-505 task spec § Excluded). options.ConfigureEndpointDefaults(listen => { listen.Protocols = HttpProtocols.Http1AndHttp2; diff --git a/SatelliteProvider.IntegrationTests/Http2MultiplexingTests.cs b/SatelliteProvider.IntegrationTests/Http2MultiplexingTests.cs index 1ad7c8a..56f3fd0 100644 --- a/SatelliteProvider.IntegrationTests/Http2MultiplexingTests.cs +++ b/SatelliteProvider.IntegrationTests/Http2MultiplexingTests.cs @@ -3,15 +3,15 @@ using System.Net.Http.Headers; namespace SatelliteProvider.IntegrationTests; -// AZ-505 AC-5: HTTP/2 multiplexed responses on the dev plaintext endpoint. +// AZ-505 AC-5: HTTP/2 multiplexed responses. // -// Kestrel is configured with `HttpProtocols.Http1AndHttp2` (Program.cs); the -// .NET HttpClient supports HTTP/2 over plaintext (h2c, prior-knowledge) when -// the `System.Net.SocketsHttpHandler.Http2UnencryptedSupport` AppContext switch -// is on. Browsers cannot use h2c — that's documented in the AZ-505 risk -// section and in `tile-inventory.md` v1.0.0. This test exercises the -// programmatic-client path the onboard `TileDownloader` (httpx http2=True) -// uses in production. +// Kestrel is configured with `HttpProtocols.Http1AndHttp2` over TLS +// (docker-compose.yml mounts the dev cert; ASPNETCORE_URLS=https://+:8080). +// ALPN negotiates HTTP/2 with HTTP/2-capable clients and falls back to +// HTTP/1.1 for browsers and legacy callers. The integration-tests +// container trusts the dev cert via /usr/local/share/ca-certificates, +// so HttpClient negotiates HTTP/2 transparently — no h2c / unencrypted +// support switch is needed. public static class Http2MultiplexingTests { private const int ConcurrentRequestCount = 20; @@ -20,11 +20,6 @@ public static class Http2MultiplexingTests { RouteTestHelpers.PrintTestHeader("Test: HTTP/2 multiplexing on /tiles/{z}/{x}/{y} (AZ-505)"); - // The Http2UnencryptedSupport switch is process-wide on the client. - // Setting it more than once is a no-op, so it's safe to call here even - // though other tests in the same runner do not need it. - AppContext.SetSwitch("System.Net.SocketsHttpHandler.Http2UnencryptedSupport", true); - var apiUri = new Uri(apiUrl); using var handler = new SocketsHttpHandler { @@ -48,9 +43,14 @@ public static class Http2MultiplexingTests // Pick a single (z, x, y) — caching means all 20 calls hit the same // tile, which is exactly what we want: prove the responses come back // over HTTP/2 with their CDN-style headers preserved. + // + // Coords (158485, 91707) at z=18 are the slippy projection of + // (47.461747, 37.647063), the same lat/lon JwtIntegrationTests hits + // — confirmed to have Google Maps satellite coverage by every prior + // cycle's run, so the warmup download is reliable. const int z = 18; - const int x = 154321; - const int y = 95812; + const int x = 158485; + const int y = 91707; var path = $"/tiles/{z}/{x}/{y}"; // Prime the cache with a single warm-up call so the 20 concurrent diff --git a/SatelliteProvider.IntegrationTests/LeafletPathIndexOnlyTests.cs b/SatelliteProvider.IntegrationTests/LeafletPathIndexOnlyTests.cs index 5f68085..bd6b0bd 100644 --- a/SatelliteProvider.IntegrationTests/LeafletPathIndexOnlyTests.cs +++ b/SatelliteProvider.IntegrationTests/LeafletPathIndexOnlyTests.cs @@ -134,7 +134,7 @@ public static class LeafletPathIndexOnlyTests var locP = cmd.Parameters.Add("loc", NpgsqlTypes.NpgsqlDbType.Uuid); const int zoom = 18; - var baseTime = DateTime.UtcNow.AddDays(-1); + var baseTime = DateTime.SpecifyKind(DateTime.UtcNow.AddDays(-1), DateTimeKind.Unspecified); for (var i = 0; i < rowCount; i++) { var x = 100_000 + (i % 1024); @@ -171,7 +171,7 @@ public static class LeafletPathIndexOnlyTests cmd.Parameters.AddWithValue("lat", 60.5); cmd.Parameters.AddWithValue("lon", 30.5); cmd.Parameters.AddWithValue("fp", "tiles/leaflet-probe.jpg"); - cmd.Parameters.AddWithValue("t", DateTime.UtcNow); + cmd.Parameters.AddWithValue("t", DateTime.SpecifyKind(DateTime.UtcNow, DateTimeKind.Unspecified)); cmd.Parameters.AddWithValue("loc", hash); await cmd.ExecuteNonQueryAsync(); } diff --git a/SatelliteProvider.IntegrationTests/MigrationTests.cs b/SatelliteProvider.IntegrationTests/MigrationTests.cs index e5512fe..a93287a 100644 --- a/SatelliteProvider.IntegrationTests/MigrationTests.cs +++ b/SatelliteProvider.IntegrationTests/MigrationTests.cs @@ -232,12 +232,22 @@ public static class MigrationTests $"AZ-503 AC-9: idx_tiles_unique_identity must wrap flight_id in COALESCE so NULL flights collide deterministically. Definition: {newIndex.Def}"); } - // A non-unique index on location_hash should also exist so the upcoming AZ-505 covering scan has a starting point. - var locationHashIndex = rows.FirstOrDefault(r => string.Equals(r.Name, "idx_tiles_location_hash", StringComparison.Ordinal)); + // An index whose leading column is `location_hash` must exist so equality lookups + // by hash have an index-driven access path. AZ-503 introduced this as + // `idx_tiles_location_hash` in migration 014; AZ-505 supersedes it in migration 015 + // with `tiles_leaflet_path` (a covering index that keeps location_hash as the + // leading column and adds ORDER BY columns + INCLUDE projection). Either name + // satisfies the AC-9 intent — accept both so the AZ-503 contract remains + // verifiable after migration 015 has applied. + var locationHashIndex = rows.FirstOrDefault(r => + string.Equals(r.Name, "idx_tiles_location_hash", StringComparison.Ordinal) || + string.Equals(r.Name, "tiles_leaflet_path", StringComparison.Ordinal)); if (locationHashIndex.Def is null) { throw new Exception( - "AZ-503 AC-9: expected supporting index 'idx_tiles_location_hash' after migration 014, but it is not present."); + "AZ-503 AC-9: expected an index keyed by location_hash (either 'idx_tiles_location_hash' from migration 014 " + + "or its AZ-505 successor 'tiles_leaflet_path' from migration 015) on tiles, but neither is present. " + + $"Found indexes: {string.Join(", ", rows.Select(r => r.Name))}"); } Console.WriteLine($" ✓ New unique index present: {newIndex.Def}"); diff --git a/SatelliteProvider.IntegrationTests/Program.cs b/SatelliteProvider.IntegrationTests/Program.cs index c52b6fb..5d82314 100644 --- a/SatelliteProvider.IntegrationTests/Program.cs +++ b/SatelliteProvider.IntegrationTests/Program.cs @@ -21,7 +21,7 @@ class Program } } - var apiUrl = Environment.GetEnvironmentVariable("API_URL") ?? "http://api:8080"; + var apiUrl = Environment.GetEnvironmentVariable("API_URL") ?? "https://api:8080"; var modeEnv = Environment.GetEnvironmentVariable("INTEGRATION_TESTS_MODE")?.Trim().ToLowerInvariant(); var modeArg = args.FirstOrDefault(a => a.Equals("--smoke", StringComparison.OrdinalIgnoreCase) || a.Equals("--full", StringComparison.OrdinalIgnoreCase)); diff --git a/SatelliteProvider.IntegrationTests/TileInventoryTests.cs b/SatelliteProvider.IntegrationTests/TileInventoryTests.cs index c0a8649..4254bb3 100644 --- a/SatelliteProvider.IntegrationTests/TileInventoryTests.cs +++ b/SatelliteProvider.IntegrationTests/TileInventoryTests.cs @@ -371,7 +371,7 @@ public static class TileInventoryTests lonP.Value = 30.0 + random.NextDouble(); sizeP.Value = 200.0; fpP.Value = $"tiles/perf-seed/{i}.jpg"; - tP.Value = DateTime.UtcNow.AddMinutes(-i); + tP.Value = DateTime.SpecifyKind(DateTime.UtcNow.AddMinutes(-i), DateTimeKind.Unspecified); locP.Value = hash; await cmd.ExecuteNonQueryAsync(); } @@ -436,7 +436,10 @@ public static class TileInventoryTests cmd.Parameters.AddWithValue("lon", 30.0 + coord.TileY * 1e-9); cmd.Parameters.AddWithValue("fp", $"tiles/seed/{coord.TileZoom}/{coord.TileX}/{coord.TileY}.jpg"); cmd.Parameters.AddWithValue("src", source); - cmd.Parameters.AddWithValue("t", capturedAt); + // schema column is TIMESTAMP (no tz); Npgsql v6+ refuses to bind a + // Kind=Utc DateTime into a plain timestamp column. Callers pass UTC + // for clarity; normalize Kind here. + cmd.Parameters.AddWithValue("t", DateTime.SpecifyKind(capturedAt, DateTimeKind.Unspecified)); cmd.Parameters.AddWithValue("flight", (object?)flightId ?? DBNull.Value); cmd.Parameters.AddWithValue("loc", locationHash); await cmd.ExecuteNonQueryAsync(); diff --git a/_docs/02_document/contracts/api/tile-inventory.md b/_docs/02_document/contracts/api/tile-inventory.md index 1809f8d..2442a85 100644 --- a/_docs/02_document/contracts/api/tile-inventory.md +++ b/_docs/02_document/contracts/api/tile-inventory.md @@ -135,8 +135,8 @@ Order invariant: `results[i]` corresponds to `request.tiles[i]` (or `request.loc - **Not covered**: byte-size hints (`estimatedBytes`). Deferred until production profiling justifies the per-file `stat()` cost. - **Not covered**: voting / trust-promotion filtering. The `voting_status` filter is part of the future voting layer (`gps-denied-onboard` Design Task #2); inventory always returns the most-recent row regardless of any future trust state. - **Not covered**: tile body download. This endpoint returns metadata only; callers fetch bodies via `GET /tiles/{z}/{x}/{y}`. -- **Not covered**: HTTP/3 / QUIC. Kestrel is set to `Http1AndHttp2`; the HTTP/3 plumbing requires ALPN + UDP verification that's deferred per AZ-505 scope. -- **Not covered**: browser-side multiplexing. h2c (HTTP/2 over plaintext) is not supported by mainstream browsers; only programmatic clients (httpx http2=True, .NET HttpClient with `HttpVersionPolicy.RequestVersionExact`) realize the HTTP/2 multiplexing benefit. Browser Leaflet wins come from the covering-index hot path, not multiplexing. +- **Not covered**: HTTP/3 / QUIC. Kestrel is set to `Http1AndHttp2`; the HTTP/3 plumbing requires UDP verification that's deferred per AZ-505 scope. +- **Not covered**: production deployment topology. Dev Kestrel runs `Http1AndHttp2` directly over TLS on port 8080 with a self-signed cert (`./certs/api.pfx`, generated by `scripts/run-tests.sh`) so ALPN can advertise `h2` — browsers and programmatic clients (httpx `http2=True`, .NET `HttpClient` with `HttpVersionPolicy.RequestVersionExact`) both multiplex over a single TLS connection. In production, TLS is expected to terminate at the ingress (Envoy / nginx / ALB) and Kestrel runs HTTP/2 cleartext behind it; AZ-505 verifies the protocol multiplexing semantics here, not the production termination layer. - **Not covered**: PMTiles or tar/multipart bundle endpoints. Rejected by AZ-503 parent rationale (HTTP/2 multistream is sufficient). - **Not covered**: write operations. Inventory is read-only; UAV writes go through `POST /api/satellite/upload` (`uav-tile-upload.md` v1.1.0). diff --git a/_docs/02_document/module-layout.md b/_docs/02_document/module-layout.md index 3fa832c..cd1a199 100644 --- a/_docs/02_document/module-layout.md +++ b/_docs/02_document/module-layout.md @@ -122,7 +122,7 @@ The cycle-1 (AZ-487) and cycle-2 (AZ-488) code reviews each surfaced an F1 (Low - **Directory**: `SatelliteProvider.Api/` - **Public API**: - - `SatelliteProvider.Api/Program.cs` (minimal API endpoints, DI setup, middleware chain — `UseAuthentication` + `UseAuthorization` added in AZ-487; `/api/satellite/upload` rewired in AZ-488; AZ-505 added `POST /api/satellite/tiles/inventory` + `builder.WebHost.ConfigureKestrel(... Protocols = HttpProtocols.Http1AndHttp2)` for HTTP/2 over plaintext) + - `SatelliteProvider.Api/Program.cs` (minimal API endpoints, DI setup, middleware chain — `UseAuthentication` + `UseAuthorization` added in AZ-487; `/api/satellite/upload` rewired in AZ-488; AZ-505 added `POST /api/satellite/tiles/inventory` + `builder.WebHost.ConfigureKestrel(... Protocols = HttpProtocols.Http1AndHttp2)` for HTTP/2 via TLS + ALPN on the dev `https://+:8080` listener; cert is generated by `scripts/run-tests.sh` into `./certs/api.pfx` and bound through `ASPNETCORE_Kestrel__Certificates__Default__Path`) - `SatelliteProvider.Api/Authentication/AuthenticationServiceCollectionExtensions.cs` (added by AZ-487; `AddSatelliteJwt(IConfiguration)` registers `JwtBearer` with the suite-wide HS256 contract from `suite/_docs/10_auth.md`; validates `JWT_SECRET` ≥ 32 bytes at startup) - `SatelliteProvider.Api/Authentication/PermissionsRequirement.cs` + `PermissionsAuthorizationHandler` + `SatellitePermissions` (added by AZ-488; custom requirement that accepts a `permissions` claim shaped as either a single string or a JSON array; powers the `UavUploadPolicy` requiring the `GPS` permission) - `SatelliteProvider.Api/DTOs/UavTileBatchUploadRequest.cs` (added by AZ-488; multipart form binding envelope — kept in WebApi because it depends on `IFormFileCollection` + `[FromForm]`, both API-layer types) diff --git a/_docs/02_document/modules/api_program.md b/_docs/02_document/modules/api_program.md index bd50011..498f849 100644 --- a/_docs/02_document/modules/api_program.md +++ b/_docs/02_document/modules/api_program.md @@ -52,7 +52,7 @@ Application entry point. Configures DI container, sets up middleware, defines mi 8. CORS policy: `TilesCors` — configured origins from `CorsConfig:AllowedOrigins`, falls back to allow-any 9. JSON options: camelCase, case-insensitive 10. **JWT authentication (AZ-487 + AZ-494)**: `AddSatelliteJwt(builder.Configuration)` (extension in `SatelliteProvider.Api.Authentication`) registers `JwtBearer` with `TokenValidationParameters` set per the suite auth contract: signature + lifetime + issuer + audience validation, 30 s clock skew, ≥ 32-byte HMAC key. The `iss` value comes from `JWT_ISSUER` env (fallback `Jwt:Issuer` config); the `aud` value comes from `JWT_AUDIENCE` env (fallback `Jwt:Audience` config). All three values (secret, iss, aud) are fail-fast — the API throws `InvalidOperationException` at startup if any is unset or whitespace-only. Production deploys MUST set the env vars with admin-team-confirmed values; `appsettings.json` ships empty so the fail-fast triggers. `appsettings.Development.json` ships clearly-tagged DEV-ONLY values (`DEV-ONLY-iss-admin-azaion-local` / `DEV-ONLY-aud-satellite-provider`) so local dev works out-of-the-box. Followed by `AddAuthorization` with the `RequiresGpsPermission` policy (AZ-488). -11. **Kestrel HTTP/2 (AZ-505)**: `builder.WebHost.ConfigureKestrel(opts => opts.ConfigureEndpointDefaults(lo => lo.Protocols = HttpProtocols.Http1AndHttp2))`. Enables HTTP/2 over plaintext (h2c) on the dev endpoint so programmatic clients (`HttpClient` with `Version20` + `RequestVersionExact`, httpx `http2=True`) can multiplex tile reads on a single TCP connection. Browsers still negotiate HTTP/1.1 over plaintext — browser Leaflet performance is unaffected by the H2 flip and depends instead on the `tiles_leaflet_path` covering index. +11. **Kestrel HTTP/2 (AZ-505)**: `builder.WebHost.ConfigureKestrel(opts => opts.ConfigureEndpointDefaults(lo => lo.Protocols = HttpProtocols.Http1AndHttp2))`. The dev listener is now `https://+:8080` with a self-signed cert (`./certs/api.pfx`, generated idempotently by `scripts/run-tests.sh` and bound via `ASPNETCORE_Kestrel__Certificates__Default__Path` / `__Password` in `docker-compose.yml`). Kestrel needs TLS for HTTP/2 protocol negotiation; ALPN advertises both `h2` and `http/1.1` so HTTP/2-capable clients (browser Leaflet, `HttpClient` with `Version20` + `RequestVersionExact`, httpx `http2=True`) multiplex tile reads on a single TLS connection, and legacy clients fall back to HTTP/1.1. The integration-test container trusts the dev cert via `/usr/local/share/ca-certificates/` + `update-ca-certificates`. AZ-505 AC-5 verifies the multiplex semantics here; production termination is expected at the ingress (Envoy / nginx / ALB) — Kestrel can then drop to HTTP/2 cleartext behind it without changing this code. ### Startup 1. Database migration via `DatabaseMigrator.RunMigrations()` — throws on failure diff --git a/_docs/02_tasks/done/AZ-505_tile_inventory_http2_leaflet_index.md b/_docs/02_tasks/done/AZ-505_tile_inventory_http2_leaflet_index.md index 8795e72..a3b2125 100644 --- a/_docs/02_tasks/done/AZ-505_tile_inventory_http2_leaflet_index.md +++ b/_docs/02_tasks/done/AZ-505_tile_inventory_http2_leaflet_index.md @@ -150,8 +150,8 @@ Then all 20 responses succeed; each `HttpResponseMessage.Version == 2.0`; per-ti - *Risk*: `CREATE INDEX` (without CONCURRENTLY) takes an `ACCESS SHARE` + `SHARE` lock for the duration of the build, blocking writes. Production deploy could stall UAV uploads + Google Maps downloads. - *Mitigation*: Investigate whether DbUp can execute a non-transactional `CREATE INDEX CONCURRENTLY` statement (DbUp historically wraps each script in a transaction, which is incompatible with CONCURRENTLY). If yes — use it. If no — document the expected lock window in the migration header and the deploy runbook, and align deployment to a low-traffic window. -**Risk 3: HTTP/2 over plaintext (h2c) may not be reachable from all clients** -- *Risk*: Browsers do NOT support h2c (HTTP/2 over plaintext) — they require ALPN + TLS. Only programmatic clients (httpx with `http2=True`, .NET `HttpClient` configured for `Version20`, Go `net/http2`) can use the multiplexed endpoint. Leaflet in a browser will continue to use HTTP/1.1 + up-to-6 connections. +**Risk 3: HTTP/2 over plaintext (h2c) may not be reachable from all clients** — *resolved post-implementation:* Kestrel was switched to TLS on `https://+:8080` with a self-signed dev cert (`./certs/api.pfx`, generated idempotently by `scripts/run-tests.sh`) so ALPN can advertise `h2` to **both** browsers and programmatic clients. Browser Leaflet now multiplexes over the same TLS connection as the .NET / httpx clients. Production termination remains expected at the ingress (Envoy / nginx / ALB); the in-process TLS cert is dev-only and is gitignored. +- *Risk (original wording, preserved for traceability)*: Browsers do NOT support h2c (HTTP/2 over plaintext) — they require ALPN + TLS. Only programmatic clients (httpx with `http2=True`, .NET `HttpClient` configured for `Version20`, Go `net/http2`) can use the multiplexed endpoint. Leaflet in a browser will continue to use HTTP/1.1 + up-to-6 connections. - *Mitigation*: Document this in `tile-inventory.md` v1.0.0 contract and in the deploy runbook. The onboard consumer (httpx-based) IS the primary beneficiary of HTTP/2 here; browser Leaflet performance is unaffected (heap-eliminated read path via the covering index is the win there). **Risk 4: Onboard `TileDownloader` (AZ-316) calls inventory before this task lands in production** diff --git a/_docs/03_implementation/implementation_completeness_cycle6_report.md b/_docs/03_implementation/implementation_completeness_cycle6_report.md index d10ac4e..0ea9f83 100644 --- a/_docs/03_implementation/implementation_completeness_cycle6_report.md +++ b/_docs/03_implementation/implementation_completeness_cycle6_report.md @@ -54,7 +54,7 @@ $ rg -i 'placeholder|TODO|NotImplemented|scaffold|native bridge|fake|mock' \ Named technologies / integrations promised by the task: - **`tiles_leaflet_path` covering index** — created by migration 015; verified to exist when migrations run on a fresh DB. -- **Kestrel HTTP/2 (`Http1AndHttp2`)** — wired via `builder.WebHost.ConfigureKestrel` per the AZ-505 Outcome bullet 3. AC-5 integration test confirms `HttpResponseMessage.Version == 2.0` over 20 concurrent multiplexed GETs. +- **Kestrel HTTP/2 (`Http1AndHttp2`)** — wired via `builder.WebHost.ConfigureKestrel` per the AZ-505 Outcome bullet 3. Dev listener bound to `https://+:8080` with `./certs/api.pfx` so ALPN can advertise `h2` (Kestrel requires TLS for HTTP/2 negotiation); cert generation is idempotent via `scripts/run-tests.sh` and `certs/` is gitignored. AC-5 integration test confirms `HttpResponseMessage.Version == 2.0` over 20 concurrent multiplexed GETs. - **Npgsql `ANY($1::uuid[])` array binding** — used in `GetTilesByLocationHashesAsync`. The escape from Dapper is documented inline and is the production behaviour exercised by the AC-1 / AC-4 integration tests. - **Cross-repo `Uuidv5.TileNamespace`** — unchanged from AZ-503. AZ-505 consumes the existing constant via `Uuidv5.LocationHashForTile`. The sibling-repo's Python `c6_tile_cache/_uuid.py:TILE_NAMESPACE` is owned by `gps-denied-onboard` and is **out of scope for the satellite-provider workspace** per the AZ-505 Constraints section. @@ -103,3 +103,7 @@ Tests (existence + AC mapping verified): ## Required Remediation Tasks: None Cycle 6 is complete from the implementation perspective. The full integration-test gate is owned by autodev Step 11 (test-run skill) per the handoff in `implementation_report_tile_inventory_cycle6.md`. + +## Post-gate correction (Run Tests step, follow-up commit) + +The Step 11 (Run Tests) execution surfaced an AC-5 runtime gap that the source-code-only completeness gate could not catch: `HttpProtocols.Http1AndHttp2` on a plaintext listener silently downgrades to HTTP/1.1-only (Kestrel logs `HTTP/2 is not enabled for [::]:8080 ... TLS is not enabled`), so the multiplexed-GET test failed with `HTTP_1_1_REQUIRED`. The corrective commit (`[AZ-505] AC-5 fix: enable TLS for HTTP/2 via ALPN`) switches the dev listener to TLS on `https://+:8080` so ALPN can negotiate `h2`. Details — including the cert-generation script, docker-compose binding, integration-test CA trust setup, and two unrelated test-data fixes also caught on the rerun (Google-Maps-404 coords in `Http2MultiplexingTests`; `DateTime.Kind=Utc` vs `timestamp without time zone` in three raw-Npgsql seed paths; the stale `idx_tiles_location_hash` assertion in `MigrationTests`) — are in `implementation_report_tile_inventory_cycle6.md` → "Post-merge correction". The full integration test suite now passes (mode=full, exit 0). Gate verdict remains **PASS**: every named AZ-505 technology is integrated and exercised by a green AC test. diff --git a/_docs/03_implementation/implementation_report_tile_inventory_cycle6.md b/_docs/03_implementation/implementation_report_tile_inventory_cycle6.md index 0c8533f..40561e7 100644 --- a/_docs/03_implementation/implementation_report_tile_inventory_cycle6.md +++ b/_docs/03_implementation/implementation_report_tile_inventory_cycle6.md @@ -11,7 +11,7 @@ Cycle 6 ships **the consumer-facing payload of the AZ-503-foundation tile identi - **`POST /api/satellite/tiles/inventory`** — bulk-list / pre-flight cache sizing endpoint that the onboard `TileDownloader` (sibling repo `gps-denied-onboard` AZ-316) is gated behind `c11.use_bulk_list_endpoint=false` until this PBI lands in the target environment. - **`tiles_leaflet_path` covering index** — makes the Leaflet hot path (`GET /tiles/{z}/{x}/{y}`) an `Index Only Scan` against `(location_hash, captured_at DESC, updated_at DESC, id DESC) INCLUDE (file_path, source)`. `GetByTileCoordinatesAsync` was rewired to filter on `location_hash` (deterministic UUIDv5) to drive the index; behaviour is byte-identical. -- **Kestrel HTTP/2 (h2c)** — `Http1AndHttp2` on every dev listener so programmatic clients (httpx `http2=True`, .NET `HttpClient` with `Version20` + `RequestVersionExact`) can multiplex tile reads on one TCP connection. Browsers still negotiate HTTP/1.1 over plaintext — browser Leaflet wins come from the covering-index hot path. +- **Kestrel HTTP/2 (TLS + ALPN)** — `Http1AndHttp2` on the dev listener, now bound to `https://+:8080` with a self-signed cert at `./certs/api.pfx` (generated by `scripts/run-tests.sh`; gitignored). Kestrel requires TLS for HTTP/2 protocol negotiation; ALPN advertises both `h2` and `http/1.1` so programmatic clients (httpx `http2=True`, .NET `HttpClient` with `Version20` + `RequestVersionExact`) and HTTP/2-capable browsers all multiplex tile reads on a single TLS connection. The integration-test container trusts the dev cert via `/usr/local/share/ca-certificates/` + `update-ca-certificates`. (Original plan was h2c on plaintext 8080; switched mid-cycle when Kestrel logged `HTTP/2 is not enabled for [::]:8080 ... TLS is not enabled`. See "Post-merge correction" below.) - **Contract artifacts** — new `tile-inventory.md` v1.0.0 and the long-deferred `tile-storage.md` v2.0.0 major bump (architecture.md had named AZ-505 as owner since cycle 5). ## Batches @@ -61,7 +61,7 @@ Code review accepted PASS after one auto-fix round (consolidated `ComputeLocatio | AC-2 Leaflet path returns most-recent variant via `location_hash` | Covered | `TileInventoryTests.LeafletReadReturnsMostRecentViaLocationHash_AC2` (DB-level verification of the exact SELECT used by `GetByTileCoordinatesAsync`; ServeTile is a wrapper around the row read) | | AC-3 Leaflet hot path uses `Index Only Scan using tiles_leaflet_path` | Covered | `LeafletPathIndexOnlyTests.RunAll` (EXPLAIN ANALYZE + regex + Heap Fetches ≤ 1) | | AC-4 Inventory p95 ≤ 1000 ms for 2500 tiles | Covered | `TileInventoryTests.PerformanceBudget_AC4` (full-suite only; smoke prints documented skip) | -| AC-5 HTTP/2 multiplexed responses on the dev plaintext endpoint | Covered | `Http2MultiplexingTests.RunAll` | +| AC-5 HTTP/2 multiplexed responses on the dev TLS endpoint (ALPN-negotiated) | Covered | `Http2MultiplexingTests.RunAll` | | AC-6 Request validation: 400 both-populated / 400 neither / 400 > 5000 / 401 anonymous | Covered | `TileInventoryTests.ValidationRejects{BothPopulated,NeitherPopulated,OversizedBatch}_AC6` + `TileInventoryTests.UnauthenticatedRequestReturns401_AC6` | | AC-7 Contract artifacts produced in the same commit | Covered (doc-only) | `tile-inventory.md` v1.0.0 + `tile-storage.md` v2.0.0 Change Log entry + module-layout / glossary / data_model / module-doc updates | @@ -89,6 +89,24 @@ Recommendation for `test-run`: - Auto-push: enabled this session (`auto_push: true` in `_docs/_autodev_state.md`) - Commits pushed (subject lines): - `[AZ-505] Tile inventory + HTTP/2 + leaflet covering index` + - `[AZ-505] AC-5 fix: enable TLS for HTTP/2 via ALPN` (post-merge correction — see below) + +## Post-merge correction (Run Tests step) + +The initial cycle-6 commit configured Kestrel as `HttpProtocols.Http1AndHttp2` on a plaintext listener (`http://+:8080`). The `Http2MultiplexingTests` regression run revealed Kestrel logs `HTTP/2 is not enabled for [::]:8080 ... TLS is not enabled` and silently falls back to HTTP/1.1 only — ALPN cannot run over plaintext, so the protocol-multiplex semantics AC-5 verifies never engaged. The fix: + +- Generate a self-signed dev cert (`./certs/api.pfx` + `./certs/api.crt`) idempotently from `scripts/run-tests.sh` (`ensure_dev_cert` block using `openssl` inside an `alpine` container). `certs/` is gitignored. +- Switch the API listener to `https://+:8080` in `docker-compose.yml` (`ASPNETCORE_URLS`, `ASPNETCORE_Kestrel__Certificates__Default__Path`, `__Password`) and mount the `.pfx` read-only. +- In `docker-compose.tests.yml`, mount `./certs/api.crt` into `/usr/local/share/ca-certificates/` of the integration-tests container and run `update-ca-certificates` in the entrypoint so `HttpClient` trusts the dev cert with no per-test handler tweaks. Test default `API_URL` updated to `https://api:8080`. +- Drop the `AppContext.SetSwitch("System.Net.SocketsHttpHandler.Http2UnencryptedSupport", true)` line from `Http2MultiplexingTests.cs` — no longer needed once ALPN over TLS does the negotiation. + +While re-running the suite, two test-data issues in the new tests also surfaced and were fixed in the same correction commit (they are independent of the TLS switch, both about Npgsql v6+ refusing `DateTime.Kind=Utc` for `timestamp without time zone` columns, both seen first on the post-TLS rerun): + +- `Http2MultiplexingTests` was hardcoding slippy coords `(z=18, x=154321, y=95812)` that Google Maps returns 404 for. Switched to `(158485, 91707)` — the slippy projection of `(47.461747, 37.647063)` already exercised by `JwtIntegrationTests`. +- `TileInventoryTests.PerformanceBudget_AC4`, `TileInventoryTests.SeedTileAsync`, and `LeafletPathIndexOnlyTests` were binding `DateTime.UtcNow` (Kind=Utc) into the `tiles.captured_at` / `created_at` / `updated_at` `TIMESTAMP` columns via raw `NpgsqlCommand` (production goes through Dapper, which never hits this code path). `DateTime.SpecifyKind(..., Unspecified)` applied at the binding site fixes the test seed without altering the production write path. +- `MigrationTests.Az503NewUniqueIndexCoversIntegerKeyAndFlightId` was hardcoded to look for `idx_tiles_location_hash` from migration 014; migration 015 explicitly drops that index because the new covering index `tiles_leaflet_path` has the same leading column. Assertion broadened to accept either index name so the AZ-503 AC-9 intent ("a location_hash-indexed access path exists") stays verifiable. + +After the correction commit the full integration test suite passes (mode=full, exit 0). ## Open Items diff --git a/_docs/_autodev_state.md b/_docs/_autodev_state.md index a331720..e815d36 100644 --- a/_docs/_autodev_state.md +++ b/_docs/_autodev_state.md @@ -2,8 +2,8 @@ ## Current Step flow: existing-code -step: 11 -name: Run Tests +step: 12 +name: Test-Spec Sync status: not_started sub_step: phase: 0 diff --git a/docker-compose.tests.yml b/docker-compose.tests.yml index 4ba76e8..285d9f5 100644 --- a/docker-compose.tests.yml +++ b/docker-compose.tests.yml @@ -14,8 +14,13 @@ services: context: . dockerfile: SatelliteProvider.IntegrationTests/Dockerfile container_name: satellite-provider-integration-tests + # AZ-505 AC-5: API now serves HTTPS for HTTP/2 via ALPN. The matching + # public cert is mounted into /usr/local/share/ca-certificates so the + # Dockerfile entrypoint can register it with update-ca-certificates + # before tests run. After that every HttpClient trusts it transparently + # — no per-test handler shim is required. environment: - - API_URL=http://api:8080 + - API_URL=https://api:8080 - INTEGRATION_TESTS_MODE=${INTEGRATION_TESTS_MODE:-full} - INTEGRATION_KEEP_STATE=${INTEGRATION_KEEP_STATE:-} - ASPNETCORE_ENVIRONMENT=Testing @@ -26,6 +31,13 @@ services: volumes: - ./ready:/app/ready - ./tiles:/app/tiles + - ./certs/api.crt:/usr/local/share/ca-certificates/satellite-provider-dev.crt:ro + # AZ-505 AC-5: register the dev CA at runtime so HttpClient trusts the API. + # update-ca-certificates picks up everything under /usr/local/share/ca-certificates/. + entrypoint: + - /bin/sh + - -c + - update-ca-certificates >/dev/null 2>&1 && exec dotnet /app/SatelliteProvider.IntegrationTests.dll depends_on: api: condition: service_started diff --git a/docker-compose.yml b/docker-compose.yml index 38269ba..610e6c3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -24,9 +24,16 @@ services: ports: - "18980:8080" - "18981:8081" + # AZ-505 AC-5: HTTPS is required for HTTP/2 via ALPN (Kestrel silently + # disables HTTP/2 on plaintext endpoints). The cert is self-signed, + # dev-only — generated by scripts/run-tests.sh and gitignored under + # ./certs/. The integration-tests container installs the matching + # public cert into its OS CA store so every HttpClient trusts it. environment: - ASPNETCORE_ENVIRONMENT=Development - - ASPNETCORE_URLS=http://+:8080 + - ASPNETCORE_URLS=https://+:8080 + - ASPNETCORE_Kestrel__Certificates__Default__Path=/app/certs/api.pfx + - ASPNETCORE_Kestrel__Certificates__Default__Password=satellite-dev-cert - ConnectionStrings__DefaultConnection=Host=postgres;Port=5432;Database=satelliteprovider;Username=postgres;Password=postgres - MapConfig__ApiKey=${GOOGLE_MAPS_API_KEY} - JWT_SECRET=${JWT_SECRET} @@ -36,6 +43,7 @@ services: - ./tiles:/app/tiles - ./ready:/app/ready - ./logs:/app/logs + - ./certs/api.pfx:/app/certs/api.pfx:ro depends_on: postgres: condition: service_healthy diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh index 842328e..4782e4f 100755 --- a/scripts/run-tests.sh +++ b/scripts/run-tests.sh @@ -9,6 +9,54 @@ cleanup() { } trap cleanup EXIT +# AZ-505 AC-5: HTTP/2 via ALPN requires TLS on the API listener. The cert is +# self-signed for dev/test only — gitignored under certs/ and regenerated on +# demand. PFX is mounted into the API container as the Kestrel cert; the public +# PEM is mounted into the integration-tests container's CA trust store so every +# HttpClient transparently trusts it without per-test handler shims. +ensure_dev_cert() { + local certs_dir="$PROJECT_ROOT/certs" + local pfx="$certs_dir/api.pfx" + local crt="$certs_dir/api.crt" + if [[ -f "$pfx" && -f "$crt" ]]; then + echo "Step 0a: Dev cert present (./certs/api.pfx)" + return 0 + fi + echo "Step 0a: Generating dev TLS cert (./certs/api.pfx + api.crt) for HTTP/2 ALPN (AZ-505 AC-5)" + mkdir -p "$certs_dir" + docker run --rm -v "$certs_dir:/work" -w /work alpine:3.20 sh -c ' + set -e + apk add --no-cache openssl >/dev/null + cat > /tmp/openssl.cnf <