think
16px
820px
← all files

P2.7 Task 3 report — Azure DI async passthrough façade

Status: COMPLETE · branch feat/gateway-p2.7-native-facades · commit de5c55e
GOFLAGS=-mod=mod go test ./internal/... -count=1 green (all 8 packages), go vet ./... clean, go build ./... clean, plus a -race pass over internal/jobs + internal/server.

What was built

internal/jobs/facade_azuredi.go — AzureDIFacade

Struct exactly per the brief: Reg/Cfg/Pools/TokenTenants/UpstreamID/Upstream/Emit/Client plus a mutex-guarded correlation map map[resultID]azSubmitInfo{start, tenant, surface, user, trace, model} with TTL 1h, lazily swept on insert at most once per minute (azCorrTTL, azCorrSweepEvery), and a lazily-created zstd encoder (creation is mutex-guarded — handlers run concurrently).

Analyze() (POST, path must end :analyze; anything else → 405 METHOD_OR_PATH_UNSUPPORTED):
1. Tenant auth (shared tenantFrom, same header-trust/token semantics as SyncForward) → body read capped by MaxBodyBytes → on-prem enforcement (403 EXTERNAL_UPSTREAM_FORBIDDEN).
2. Pool admission with the class hold budget (classFromPriority with the upstream → batch default for unprioritized OCR); past budget → 429 + Retry-After + X-Queue-Depth, ticket abandoned, QUEUE_TIMEOUT error event.
3. Forward to Reg.PickEndpoint(UpstreamID) + original path + raw query (r.URL.RequestURI()), ALL request headers relayed minus hop-by-hop — the engine's Ocp-Apim-Subscription-Key travels verbatim, the gateway never injects a key from env on this path.
4. Slot released immediately after the upstream submit response is read (before relaying) — the async analysis is Azure's own concurrency, not a gateway slot. releaseOnce + defer still covers every early-error path.
5. Response relayed verbatim: status, headers minus hop-by-hop (Operation-Location unchanged — the engine rewrites it onto its own base by locating the /{dialect}/ segment itself), body.
6. Model extracted from …/documentModels/{model}:analyze (both dialects; fallback = last path segment before :analyze; never errors). On 2xx, resultId parsed from Operation-Location's /analyzeResults/{id} (query stripped) seeds the correlation entry.
7. ONE submit audit event: engine/surface/user/trace from headers, upstream=UpstreamID, model=extracted, operation=Upstream.OperationValue (fallback ocr), traffic_class, status ok/error by upstream status (HTTP_nnn), queue_ms=admission wait, upstream_ms=submit round-trip, request body zstd+capped, doc_hash from X-Doc-Hash, job_id=resultId.

Poll() (GET under a prefix): tenant auth + on-prem check, no pool admission; verbatim forward (path+query, headers minus hop-by-hop) with the response relayed untransformed. When the response is 2xx JSON with top-level status ∈ {succeeded,failed}: one completion event per poll response — status ok / error+AZURE_ANALYZE_FAILED; upstream_ms = time since correlated submit start (correlation entry consumed/deleted), or 0 on a miss with attribution falling back to the poll request (trace/tenant/surface/user headers, model re-parsed from the poll path); response body zstd+capped. Non-terminal → no event. The accepted degradation (post-restart repeat polls may duplicate completion events; audit consumers dedupe by trace_id + event semantics; never a 5xx) is documented in a comment at the emit site.

Shared helpers extracted into internal/jobs/facade.go (SyncForward behavior unchanged)

tenantFrom, checkOnPrem, readRequestBody, relayHeaders (the exact Connection/Transfer-Encoding/Content-Length skip set SyncForward already used), admitHold, writeQueueTimeout, compressCapped, syncTimeoutFor. Facade.SyncForward/Submit/tenantFor/compress now delegate to them; the full jobs+server suites (including all pre-existing P2.6 façade tests) pass unchanged.

internal/server/server.go — EnableNativeFacades

Now a switch over u.Adapter: sync-facade as before; azuredi-facade builds one AzureDIFacade per upstream and mounts, per prefix P, POST P/ → Analyze() (which itself 405s non-:analyze paths) and GET P/ → Poll(), both drain-guarded via jobGuard. Signature now returns (mountedPaths, mountedPrefixes, mountedUpstreams); cmd/gateway/main.go logs native façades enabled (N sync paths, M azure prefixes, K upstreams).

internal/jobs/manager.go — adapterFor fallback cases

  • sync-facade → SyncHTTPAdapter (no SubmitPath field exists on the adapter, it reads Upstream.SubmitPath, so the submit path is defaulted from the first sync_paths entry on a copy of the upstream — the shared config is never mutated).
  • azuredi-facade → AzureDIAdapter (the existing outbound async DI adapter).

Previously both fell through to the sync-http default with /ocr, which would have driven an azuredi upstream with the wrong protocol.

Tests added

internal/jobs/facade_azuredi_test.go (httptest Azure DI backend emulator):
- TestAzureDISubmitForwardVerbatim — path+query+binary body+key header forwarded byte-identical, Operation-Location/Apim-Request-Id relayed unchanged, submit event fields (model=bukti_setor_v2, operation=ocr, class=batch, doc_hash, compressed request body).
- TestAzureDIModelFromPath — both dialects + fallback + poll-path shape.
- TestAzureDIPollNonTerminalNoEvent — running poll relayed verbatim through a fully saturated 1-slot pool (proves polls never admit), zero events.
- TestAzureDIPollTerminalSucceeded — completion event via correlation: upstream_ms>0, trace/model/tenant from the SUBMIT (poll sent a different trace), slot back to 0 right after submit.
- TestAzureDIPollTerminalFailed — status=error, AZURE_ANALYZE_FAILED.
- TestAzureDIPollCorrelationMiss — passthrough intact, one event, upstream_ms=0/queue_ms=0, attribution from poll headers, model from poll path.
- TestAzureDISubmitHoldBudget429 — 429+Retry-After+X-Queue-Depth, backend untouched, one QUEUE_TIMEOUT event, Active()==0 after release (no slot leak).
- TestAzureDIAuthToken401 / TestAzureDIOnPrem403 — both handlers, backend never hit.
- TestAzureDIAnalyzeUnsupportedPath405.
- TestReservedFacadePathsCoverJobsTables — config.ReservedFacadePaths() ⊇ FacadePaths()+FacadeSyncPaths() + /v1,/jobs,/metrics,/health (guards the Task 1 hardcoded list; lives in jobs, no import cycle).

internal/jobs/manager_test.go: TestManagerRunsSyncFacadeUpstreamJob (job through the production adapter selection hits /layout, result relayed) and TestManagerAdapterForFacadeUpstreams (type mapping pinned, config not mutated).

internal/server/native_facade_test.go: TestAzureDIFacadeMountedThroughServer (end-to-end through the real mux: 202+Operation-Location, 405 on non-analyze, audit attribution) and TestAzureDIFacadeDormant (no azuredi upstream → POST and GET on the prefix are plain 404s).

Judgment calls / notes

  1. Correlation-miss model fallback: the brief says "falling back to poll-request headers on miss", but no header carries the model — on a miss the model is re-parsed from the poll path's /documentModels/{model}/ segment (more faithful attribution than blank); trace/tenant/surface/user do come from the poll request headers.
  2. Repeat terminal polls emit repeatedly: consuming the correlation entry on the first terminal poll means a second poll of the same terminal id is a correlation miss and (per the brief's "must still emit exactly once per poll response") emits again with upstream_ms=0. This is exactly the documented degradation; the alternative (a tombstone set) would grow unboundedly.
  3. Submit event carries request body only, completion event response body only — symmetric, per the brief's field lists; both events carry job_id=resultId so audit consumers can join/dedupe.
  4. relayHeaders keeps SyncForward's exact skip set (Connection, Transfer-Encoding, Content-Length) rather than the fuller RFC hop-by-hop list, to honor "do NOT change SyncForward behavior"; the same helper is used on the azure request path, so Authorization (in token mode) is relayed to the on-prem container along with everything else — per the brief's "ALL request headers minus hop-by-hop".
  5. Analyze forwards {prefix}/{dialect}/… verbatim including the gateway-side prefix, per the brief ("PickEndpoint + original path"); the upstream endpoint in gateway.yaml must therefore be the base the engine used to call directly (the DI container/nginx that already serves those prefixed paths).
  6. EnableNativeFacades signature change (2 → 3 return values) required touching the two existing call sites (main.go, double-call test); no other callers exist.

Fix wave 1

Three review findings fixed on branch feat/gateway-p2.7-native-facades, off HEAD de5c55e, one TDD cycle (failing test → fix → green) per finding. Commit 4c86df2.

Finding 1 (Important, security-consistency) — inbound Authorization leaked to Azure in token mode

AzureDIFacade.Analyze() and .Poll() called relayHeaders(req.Header, r.Header) to build the outbound request, which — unlike Facade.SyncForward (confirmed by re-reading facade.go: it builds its outbound request from scratch with only Content-Type + APIKeyEnv-derived Authorization, never copying r.Header at all) — relayed every inbound header including Authorization. In token mode (len(TokenTenants)>0) the inbound Authorization: Bearer <gateway-tenant-token> therefore reached the on-prem Azure DI container, which never expects or checks it (Azure uses Ocp-Apim-Subscription-Key, owned and sent by the engine itself). This contradicted note 4 of the original report, which had documented the leak as expected behavior — it was not.

Since SyncForward doesn't forward inbound headers today, only the azure façade needed the fix (per the finding's own instruction, SyncForward was left untouched).

Fix: req.Header.Del("Authorization") immediately after relayHeaders(req.Header, r.Header), in both Analyze() (facade_azuredi.go:264-268) and Poll() (facade_azuredi.go:348-351). Ocp-Apim-Subscription-Key and all other headers still relay verbatim.

Test (red → green): TestAzureDIStripsInboundAuthorization — token-mode fixture, submit and poll both sent with Authorization: Bearer tok-ocr + Ocp-Apim-Subscription-Key: engine-owned-key; asserts the httptest backend's captured Authorization is empty while Ocp-Apim-Subscription-Key arrives unchanged, for both handlers.

Finding 2 (Important, config validation) — nested facade_prefixes across upstreams not rejected

config.Load checked facade_prefixes entries for exact duplicates (seenFacadePrefixes map) and cross-field collisions against sync_paths, but never checked one facade_prefixes entry against another facade_prefixes entry via segmentCollision (only exact-string dup). Two azuredi-facade upstreams configured with /api/di and /api/di/sub therefore loaded cleanly, and server.EnableNativeFacades would mount both onto the same http.ServeMux — Go's mux resolves the more specific pattern first, silently routing all /api/di/sub/... traffic to the second upstream's pool/audit/backend while /api/di/... traffic (that doesn't match /sub) goes to the first — a silent namespace split, never a startup error.

Fix: added a loop over mountedFacadePrefixes (the running list of all facade_prefixes entries accepted so far, across every upstream) inside the per-entry validation in config.go, calling the existing segmentCollision helper and erroring with both upstream ids named, mirroring the existing sync_paths-vs-facade_prefixes cross-check already in place.

Tests (red → green):
- TestLoadRejectsNestedFacadePrefixCollision — /api/di (upstream t1-azuredi-parent) + /api/di/sub (upstream t1-azuredi-child) → Load error naming both ids.
- TestLoadDistinctSiblingFacadePrefixesStillLoad — negative control: /api/a + /api/b across two upstreams still load (2 upstreams, no error).

Finding 3 (Minor, live-correctness) — relayed response body truncated by MaxBodyBytes

Both Analyze() and Poll() capped the relayed upstream response with io.LimitReader(resp.Body, a.Cfg.MaxBodyBytes) before writing it back to the engine. MaxBodyBytes bounds the gateway's inbound request-body budget; applying it to the outbound relay meant a large analyzeResult JSON (e.g. many pages of OCR content) exceeding that limit would reach the engine truncated mid-JSON — unparseable — while the gateway itself reported 200/202 success.

Fix: removed the io.LimitReader wrapping on both relay paths (facade_azuredi.go, submit ~277-284 and poll ~358-360) — respBody, _ := io.ReadAll(resp.Body) now reads the full upstream response unconditionally. The audit copy is untouched: it still goes through a.compress() → compressCapped(), which caps at cfg.Audit.BodyCapBytes independently and records FullBodySHA256 on truncation, so audit payload size stays bounded regardless of relay size.

Test (red → green): TestAzureDIPollRelaysFullBodyAuditStaysCapped — cfg.MaxBodyBytes=200, cfg.Audit.BodyCapBytes=100, terminal poll body ~4.1KB; asserts the client-visible relayed body is byte-complete (full 4149 bytes, not 200), while the completion audit event has BodyTruncated=true and a non-empty FullBodySHA256.

Verification

GOFLAGS=-mod=mod go test ./internal/... -count=1   # ok, all 8 packages (audit, config, idem, jobs, pool, proxy, registry, server)
GOFLAGS=-mod=mod go vet ./...                       # clean
GOFLAGS=-mod=mod go build ./...                     # clean
gofmt -l internal/config/config.go internal/config/config_test.go internal/jobs/facade_azuredi.go internal/jobs/facade_azuredi_test.go   # clean

Files touched: internal/config/config.go, internal/config/config_test.go, internal/jobs/facade_azuredi.go, internal/jobs/facade_azuredi_test.go. No changes to internal/jobs/facade.go / Facade.SyncForward (confirmed it already builds outbound requests without relaying inbound headers, so it was never affected by Finding 1).