Skip to content

Conversation

@giuliovn
Copy link

@giuliovn giuliovn commented Jan 2, 2025

Motivation

Right now miniwdl requires that the login to the docker registry will be done before the start of the workflow, in case it uses private images, and the image is pulled at runtime, if not already present locally.
In case the login is done with short-lived credentials, they could expire before the workflow get to the task and then fail.

Approach

After the changes in the PR the workflow will:

  • check if the image is present locally (this was not changed by the PR)
  • if the image is not present, check if the registry is accessible
  • if not accessible check the registry name pattern and see if it is from a supported provider
  • if the provider is supported, try to login to the docker registry and fail if missing permissions

Right now GCP Artifact Registry, GCP Docker Registry, and AWS Elastic Container Registry are supported.
Only for docker swarm and singularity backends.

Checklist

  • Add appropriate test(s) to the automatic suite
  • Use make pretty to reformat the code with black
  • Use make check to statically check the code using Pyre and Pylint
  • Send PR from a dedicated branch without unrelated edits
  • Ensure compatibility with this project's MIT license

I don't know how to add tests for this, it will require to create private repositories and pass credentials to the CI.

@giuliovn giuliovn mentioned this pull request Jan 2, 2025
5 tasks
@mlin
Copy link
Collaborator

mlin commented Mar 2, 2025

@giuliovn Sorry again for my great delay in looking at this PR, I finally have some bandwidth opening for miniwdl maintenance again.

How would you weigh adding this functionality against advising users to install the ECR credential helper or gcloud auth configure-docker? I think we should try to leverage those first-party solutions where possible, but I'd like to understand remaining gaps they leave.

@mlin mlin added the question Further information is requested label Mar 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

question Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants