Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CROM-6917 Add optional ref disk auth #6762

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

kshakir
Copy link
Contributor

@kshakir kshakir commented May 16, 2022

No description provided.

@kshakir
Copy link
Contributor Author

kshakir commented May 16, 2022

Cromwell communicates with various gcloud endpoints. Instead of having N creds for N endpoints, sometimes Cromwell reuses creds for something else. For example: using the GCS creds for docker hash lookups here.

This PR adds an optional config for reference disk validation auth falling back to the genomics auth that was used previously.

One can then use USA authentication for individual genomics calls and a separate system SA for verifying which of the reference disks are valid (at startup).

@kshakir kshakir marked this pull request as draft May 11, 2024 00:32
@kshakir kshakir changed the title Add optional ref disk auth CROM-6917 Add optional ref disk auth May 11, 2024
@kshakir kshakir force-pushed the ks_ref_disk_auth branch from 728dc88 to 8fa205e Compare May 11, 2024 00:42
@kshakir
Copy link
Contributor Author

kshakir commented May 11, 2024

If the direction is ok, I can add unit/integration tests.

@kshakir kshakir marked this pull request as ready for review May 11, 2024 06:17
@kshakir kshakir requested a review from a team as a code owner May 11, 2024 06:17
@aednichols
Copy link
Collaborator

I think we're looking to freeze GCP changes for now due the imminent migration to GCP Batch. We're also not sure if reference disks are staying around, they are maintenance-intensive in Terra.

@kshakir
Copy link
Contributor Author

kshakir commented May 13, 2024

Makes sense. This can wait for the official announcement of which way the feature is going.

In the meantime, our users are gradually migrating from on-prem to Terra. Our Cromwell instance allows users to run workflows on GCP or GridEngine. We want to ensure our instance has feature parity with launching workflows in Terra, so we needed something like this commit.

After the announcement, I'll update the PR to copy these auth config lines over to the Batch backend. Otherwise, Terra will have removed the checkbox for reference disks and we can close the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants