CTM-613: TDR — emit the DRS 1.5 cloud field on DRS access methods - #2120
Open
mbaumann-broad wants to merge 9 commits into
Open
mbaumann-broad wants to merge 9 commits into
mbaumann-broad wants to merge 9 commits into
Conversation
Adds an optional `cloud` property to DRSAccessMethod and populates it (`gcp` / `azure`) on every access method TDR emits. This lets a consumer (DRSHub) select the correct cloud from the metadata instead of inferring it from `access_id` prefixes: `type` alone cannot name a cloud, since a signed URL is `https` for every CSP, so TDR's GCS passport access method is typed `https` just like its Azure method. - data-repository-openapi.yaml: add optional `cloud` to DRSAccessMethod (additive; DRS 1.3 consumers ignore it). `type` is unchanged. - DrsService: set `cloud` on the GCS `gs`/`https` methods (gcp) and on the shared signed-URL method (gcp for GCP-passport, azure for Azure) -- passing the value in explicitly since that method serves all three cloud/auth contexts. Companion to the DRSHub `drs-cloud-field` branch, which consumes `cloud` to select the access-method config by `type` + `cloud`. Full rationale + rollout in drs_cloud_field_proposal.md. DRAFT - requires regenerating the `bio.terra.model` client from the OpenAPI (the `.cloud(...)` setters do not exist until regen); not build-verified locally; no tests added yet. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Make bearer token optional in postAccessURL endpoint The Authorization header is only required for self-hosted snapshots accessed with an x-user-project header. Eagerly throwing when the header is absent blocked passport-auth users who don't need the token at all. Defer enforcement to the service layer, which already handles a null token correctly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Instead use try/catch pattern * bug fix/spotless * fix tests --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
CTM-611: Don't build AuthenticatedUserRequest with a null token
DrsService.postAccessUrlForObjectId built an AuthenticatedUserRequest even
when the bearer token was null, but the builder rejects a null/empty token
and throws IllegalStateException("Token is empty"). This 500'd passport-only
callers (no Authorization header) one layer past the 401 fixed in CTM-553/#2113,
since the GCS signing path already tolerates a null authUser.
Skip building the AuthenticatedUserRequest entirely when there's no bearer
token, and null-guard the email lookup in signAzureUrl so a null authUser
doesn't trade one 500 for an NPE on the Azure path.
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…2124) Remove public-policy check from passport validation bypass Bypassing passport validation only needs the NRES consent code check; the additional SAM public-reader-policy check was redundant since it was only ever combined with the consent code check. Also removes the now-unused isSnapshotPublic/getPolicyPublicV2AsSA helpers. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…Id (#2123) * Allow passport auth for public NRES snapshots without a phsId NRES (non-restricted) snapshots don't need a phsId to bypass ECM passport validation, since the bypass never builds a RAS visa criterion. Previously a blank phsId blocked passport auth entirely even when consentCode was NRES and the snapshot was public. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * move to variable to help with readability * move check into snapshot summary * use shorthand * reset * Fix tests broken by removal of public-policy bypass check getPolicyPublicV2AsSA and the public/private distinction it backed were removed from canBypassPassportValidation earlier in this branch, but SnapshotServiceTest and DrsServiceTest still referenced the removed method, breaking compilation. Update the tests to match: NRES snapshots bypass passport validation regardless of visibility. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Adds an optional `cloud` property to DRSAccessMethod and populates it (`gcp` / `azure`) on every access method TDR emits. This lets a consumer (DRSHub) select the correct cloud from the metadata instead of inferring it from `access_id` prefixes: `type` alone cannot name a cloud, since a signed URL is `https` for every CSP, so TDR's GCS passport access method is typed `https` just like its Azure method. - data-repository-openapi.yaml: add optional `cloud` to DRSAccessMethod (additive; DRS 1.3 consumers ignore it). `type` is unchanged. - DrsService: set `cloud` on the GCS `gs`/`https` methods (gcp) and on the shared signed-URL method (gcp for GCP-passport, azure for Azure) -- passing the value in explicitly since that method serves all three cloud/auth contexts. Companion to the DRSHub `drs-cloud-field` branch, which consumes `cloud` to select the access-method config by `type` + `cloud`. Full rationale + rollout in drs_cloud_field_proposal.md. DRAFT - requires regenerating the `bio.terra.model` client from the OpenAPI (the `.cloud(...)` setters do not exist until regen); not build-verified locally; no tests added yet. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Asserts `cloud` is set correctly on GCP bearer, GCP-passport, and multi-cloud (GCP + Azure) access methods, resolving the "no tests yet" draft caveat.
Asserts `cloud` is set correctly on GCP bearer, GCP-passport, and multi-cloud (GCP + Azure) access methods, resolving the "no tests yet" draft caveat.
snf2ye
changed the title
snf2ye
marked this pull request as ready for review
August 6, 2026 20:17
snf2ye
requested review from
davidangb and
snf2ye
and removed request for
a team
August 6, 2026 20:17
…ere/jade-data-repo into draft/drs-cloud-field
|
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
2 participants
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.
Why
DRS
typeis a protocol (gs,https, …). A signed URL ishttpsfor every cloud, sotypealone cannot tell a consumer (DRSHub) which cloud an access method serves. TDR types its GCS passport signed-URL methodhttps— indistinguishable bytypefrom its Azurehttpsmethod. DRSHub therefore mis-selects the Azure config for a GCS passport object and returns HTTP 500 for RAS-passport requester-pays access (CTM-613).DRS 1.5 added the
cloudfield (aws/gcp/azure) for exactly this disambiguation.What this PR does
Emits
cloudon every DRS access method TDR returns, so a consumer can select bytype+cloud:src/main/resources/api/data-repository-openapi.yaml— add an optionalcloudproperty toDRSAccessMethod(additive; DRS 1.3 consumers ignore it;typeis unchanged). Thebio.terra.modelclient regenerates automatically from the swagger-codegen build task — no manual step needed.DrsService— setcloud:gcpon the GCSgs/httpsbuilders, andgcp/azureon the shared signed-URL method (it serves GCP-passport, Azure-passport, and Azure-bearer), passed in explicitly per call site.DrsServiceTest— assertscloudon the GCP-bearer, GCP-passport, and multi-cloud (GCP + Azure) access method cases.Companion PR (DRSHub consumes
cloud): DataBiosphere/terra-drs-hub#260Scope
This is the emit half; the DRSHub PR is the consume half that fixes CTM-613's requester-pays leg. The non-requester-pays leg is completed by CTM-611 (TDR
/accessaccepting passport-only).typevalues are unchanged, so this is safe and additive for all existing consumers (including BDC-Seven Bridges / Velsera, which read this metadata).Verified locally
./gradlew compileJava compileTestJava— model regenerates the.cloud(...)setter and the project compiles cleanly../gradlew test --tests "bio.terra.service.filedata.DrsServiceTest"— passes, including newcloudassertions../gradlew spotlessCheck— passes.References
/accessfix.