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

docker login at runtime #710

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

giuliovn
Copy link

@giuliovn giuliovn commented Aug 6, 2024

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 backend.

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 changed the title made pretty and static check docker login at runtime Aug 6, 2024
@giuliovn giuliovn marked this pull request as ready for review August 6, 2024 08:37
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.

1 participant