feat(download): add minimal DRS 1.5 object endpoint#2296
Open
jhagberg wants to merge 17 commits intofeature/sda-download-v2from
Open
feat(download): add minimal DRS 1.5 object endpoint#2296jhagberg wants to merge 17 commits intofeature/sda-download-v2from
jhagberg wants to merge 17 commits intofeature/sda-download-v2from
Conversation
…eencrypt New download service at sda/cmd/download/ replacing sda-download/. Core components: - main.go: entry point with production safety guards, TLS, graceful shutdown - config/: 50+ config flags via internal/config/v2 (pflag/viper framework) - health/: gRPC health server for K8s probes + HTTP ready/live endpoints - streaming/: Range header parsing (RFC 9110), If-Range ETag support, combined header+body and body-only streaming with seek-based positioning - reencrypt/: gRPC client for crypt4gh header re-encryption with lazy connection and TLS support - internal/config/v2/: shared config registration framework - swagger_v2.yml: OpenAPI spec for the v2 REST API
Database components: - database.go: PostgreSQL interface with prepared statements, keyset cursor pagination via (submission_file_path, stable_id) composite cursor, LATERAL json_agg for checksum aggregation (prevents row multiplication), LIKE prefix escaping, and CheckDatasetExists for no-existence-leakage pattern - cache.go: Ristretto-based cache wrapper (lock-free) for file lookups, permission checks, and dataset queries. Paginated queries bypass cache due to cursor variability.
Authentication middleware: - Structure-based JWT detection via looksLikeJWT() (3 dot-segments with base64url JSON header+payload) - JWT path: validate locally via loaded keyset, optional issuer match - Opaque path: call UserinfoClient.FetchUserinfo for subject resolution - Session cookie cache (sda_session + legacy sda_session_key) - Token-keyed cache (sha256(token)) with TTL bounded by min(token.exp, min(visa.exp), configTTL) - Permission model: combined/visa/ownership dataset population - SameSite=Lax on session cookies - Audit denial events (download.denied) on all 401 paths
GA4GH visa support: - validator.go: GetVisaDatasets() extracts datasets from visa JWTs, enforces (iss, jku) allowlist, verifies signatures via cached JWKS, validates ControlledAccessGrants (by, value, source, conditions, asserted), supports broker-bound/strict-sub/strict-iss-sub identity binding modes, detects multi-identity scenarios - trust.go: LoadTrustedIssuers from JSON with conditional HTTPS enforcement for JKU URLs - userinfo.go: UserinfoClient with HTTP cache, io.LimitReader safety, GA4GH passport v1 claim extraction - jwks_cache.go: JWK cache with per-request fetch limits and (iss, jku) allowlist enforcement - types.go: Identity, TrustedIssuer, VisaClaim, UserinfoResponse - Pre-validation limits: max-visas (200), max-visa-size (16KB), max-jwks-per-request (10)
…-info REST API endpoints: - GET /datasets: paginated dataset list with HMAC-signed page tokens - GET /datasets/:datasetId: dataset metadata (date, files, size) - GET /datasets/:datasetId/files: keyset-paginated file list with filePath/pathPrefix filters (mutually exclusive, 4096 char limit) - HEAD/GET /files/:fileId: combined download with re-encrypted header - HEAD/GET /files/:fileId/header: re-encrypted header only - HEAD/GET /files/:fileId/content: raw archive body (no pubkey needed) - GET /service-info: GA4GH service-info metadata - GET /health/ready, /health/live: health probes Cross-cutting: - RFC 9457 Problem Details for all errors (application/problem+json) - No existence leakage: 403 for both "not found" and "no access" - Content-Disposition with .c4gh extension - Cache-Control headers on all data endpoints - UseRawPath=true for URL-encoded slash dataset IDs - Audit denied/failed events on 403 and server errors - Correlation ID middleware (X-Correlation-ID)
Audit logging: - audit.Logger interface with StdoutLogger (JSON lines) and NoopLogger - download.complete/content/header events on successful operations - download.denied events on 401/403 (middleware + handlers) - download.failed events on storage/streaming errors with ErrorReason - Correlation ID propagated to all audit events Production guards (app.environment=production): - Fail startup if jwt.allow-all-data is enabled - Fail startup if pagination.hmac-secret is empty - Fail startup if gRPC client TLS certs are missing - validateProductionConfig extracted for unit testing
Docker Compose integration test environment: - postgres, minio (S3), mock-aai, mockoidc (OIDC + visa JWTs), reencrypt (gRPC), download service under test - database_seed with test dataset + file - make_download_credentials.sh: RSA keypair, JWT token, trusted issuers - mockoidc.py: OIDC discovery, JWKS, userinfo with visa datasets 33 integration tests covering: - Health, auth (JWT + opaque), session cookies, service-info - Dataset listing, file listing, pagination, invalid pageSize/pageToken - Encoded-slash dataset IDs (UseRawPath verification) - Range requests, multi-range rejection, If-Range ETag contract - Content-Disposition, pathPrefix filter with SQL wildcard escaping - Expired token rejection, long-transfer resume scenario - Problem Details format, access control (no existence leakage) Environment capability probes (SetupSuite): - REQUIRES_REENCRYPT, REQUIRES_STORAGE_FILE, REQUIRES_SESSION_CACHE - Tests skip on missing prerequisites, hard-fail on regressions
Benchmark infrastructure:
- benchmark.go: concurrent load tester comparing old (sda-download)
vs new (sda/cmd/download) services with auto-discovery, JWT auth,
configurable iterations/concurrency, percentile stats (p50/p95/p99)
- sda-benchmark.yml: Docker Compose with both services, shared
pipeline, and benchmark runner
- seed_benchmark_data.sh: uploads crypt4gh files through real ingest
pipeline for realistic test data
- Makefile targets: benchmark-download-{up,seed,run,down}
Result: NEW service is ~255% faster than OLD (67 vs 19 req/s).
…rflow hint
- 55_download_test.sh: update from v1 API paths (/info/datasets,
/file/{id}, public_key header) to v2 (/datasets, /files/{id},
X-C4GH-Public-Key). Fixes CI failure in sda (s3) integration job.
- middleware/auth.go: use len(a) instead of len(a)+len(b) as map
capacity hint in mergeDatasets to silence CodeQL overflow warning.
- benchmark.go: fmt.Errorf → errors.New where no format verbs, nlreturn blank lines, ifElseChain → switch, nolint for gosec G402 (TLS skip in benchmark tool) and revive deep-exit - handlers/files.go: remove duplicate ArchivePath check - middleware/auth.go: extract tokenExpiry() to reduce nestif - visa/: nolint:gosec for SSRF (URLs validated against allowlist)
Rewrites the service documentation to match the actual v2 API routes, authentication model, GA4GH visa support, and full configuration reference. The previous version documented draft/old routes that did not match the implementation.
Update curl examples to use v2 API routes (/datasets, /files) instead of old /info/ and /file/ routes. Fix public key header name from public_key to X-C4GH-Public-Key.
DRS 1.5 requires size and checksums to describe the blob bytes served by access_url. Changed from DecryptedSize/UNENCRYPTED to ArchiveSize/ARCHIVED. Added GetFileChecksums DB method to return all checksums for a given source, supporting multiple algorithms.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feature/sda-download-v2 #2296 +/- ##
===========================================================
+ Coverage 42.29% 42.50% +0.21%
===========================================================
Files 120 121 +1
Lines 12242 12327 +85
===========================================================
+ Hits 5178 5240 +62
- Misses 6432 6454 +22
- Partials 632 633 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
jhagberg
force-pushed
the
feature/drs-object-endpoint
branch
from
fd30cd9 to
eecfa9c
Compare
jhagberg
force-pushed
the
feature/sda-download-v2
branch
3 times, most recently
from
259d2d4 to
1798ca6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue(s) and PR(s)
This PR closes #2246 (SDA-side work only; htsget-rs DrsStorage contribution is separate).
Related: umccr/htsget-rs#356
Description
Adds
GET /objects/{datasetId}/{filePath}— a minimal GA4GH DRS 1.5 endpoint that resolves a dataset + file path to aDrsObjectwith a pre-resolvedaccess_urlpointing to/files/{fileId}/content.Changes:
CreatedAttoFilestruct, newGetFileChecksumsmethod for querying checksums by source (ARCHIVED/UNENCRYPTED)handlers/drs.go— path parsing, auth, file resolution, DRS response constructionGET /objects/*pathregistered with auth middlewareDrsObject,DrsChecksum,DrsAccessMethod,DrsAccessURL)DRS 1.5 compliance:
size=ArchiveSize(encrypted blob size per spec: "blob size in bytes")checksumsfromARCHIVEDsource (per spec: "computed over the bytes in the blob")self_uri,created_time,access_methodsall per DRS 1.5 required fieldsNote: The htsget-rs maintainer pivoted to a
ResolveStoragebackend using the existing/datasets/{id}/files?filePath=endpoint. This DRS endpoint is an independent, spec-compliant addition with value for any DRS-aware tooling.ADR
docs/decisions/README.md)docs/decisions/How to test
10 unit tests covering: success path, 403 (not found + no access), 400 (malformed paths), 401, 500, empty checksums, multiple checksums, checksum type normalization. Route wiring test verifies auth requirement.