chanzuckerberg / miniwdl

Workflow Description Language developer tools & local runner
MIT License
172 stars 54 forks source link

docker login to private repositories #707

Open giuliovn opened 1 month ago

giuliovn commented 1 month ago

hi,

I use miniwdl to run workflows with part of the docker images stored in private repostiories. I need to login to the repository before running, which is a bit unfortunate, especially since dokcer images are evaluated and pulled at runtime. We use miniwdl as part of a bigger automation and are going to build CI tests, which will require to add an additional step of parsing the inputs and logging in to all the repositories, while it seems a reasonable requirement that miniwdl will try to login and fail if the permissions are insufficient. It could at least support the most common docker registries which have recognizable name patterns, like AWS, GPC and GitHub.

Thanks, Giulio

giuliovn commented 1 month ago

@mlin, are you open to evaluate a PR that will address this issue?

giuliovn commented 1 month ago

@mlin, I already implemented this on our side (for AWS ECR and Google Artifact Registry), I think it could be adapted to use inside miniwdl, and would be more convenient for the login to happen at runtime. If I will open a PR, there is a chance it will be reviewed?

giuliovn commented 1 month ago

I will expand on a specific use case when changing this beahavior will be beneficial. We use Workload Identity Federation for authentication against Google Cloud, which creates short-lived credentials. We store all the docker images in a struct, they can be public or come from different private repositories, not all the images are used at runtime. We create a cloud instance for CI, everything is automated, before running a workflow we login to all the private docker registries of the images in the struct. What happens many times is that when we arrive to pull the image the credentials are expired and the workflow fails. We could always pull all the images in the struct before running, but this would be a big waste of time and money that could be avoided adding a relatively simple feature. @mlin, I would like at least to have an answer about why this is not supported and if there is a reason to not consider a PR addressing the issue. Thanks

mlin commented 1 month ago

@giuliovn a PR is certainly welcome as outlined on CONTRIBUTING.md, no need to ask. I am however working through a major life event right now, so I cannot promise to merge/release it quickly.