Skip to content

Conversation

@euanh
Copy link
Collaborator

@euanh euanh commented Oct 23, 2024

Motivation

Currently, ContainerRegistry handles authentication lazily. First it tries an unauthenticated request. If the registry responds with a challenge, ContainerRegistry gets a suitable credential from .netrc or from an authentication service and re-tries the request.

When pushing large blobs to some registries, the client can get stuck in a state where it has received an authentication challenge but is still waiting to finish the blob upload. The symptoms are that metadata uploads and very small image layers can be uploaded, but pushing larger images stalls and times out, as reported in #17.

Modifications

When pushing large blobs, the registry's 401 challenge arrives after the client has sent some of the blob data, but before it has completed the upload. With some registries, URLSession gets stuck in this state. Implementing the didReceiveResponse delegate allows the client to see the challenge and cancel the task but in some cases, mainly on Linux, URLSession can still get stuck.

A more reliable solution to this problem is to handle challenges eagerly. Before pushing any data, the client checks whether the registry supports the v2 distribution protocol. If it is challenged during this check, it will then eagerly send a suitable authentication token with all subsequent requests to the registry. This avoids the state where the client receives a challenge part-way through uploading a large blob and gets stuck.

Result

It is possible to upload large images to ECR and Docker Hub from macOS hosts, and to Docker Hub from Linux hosts. Uploading to ECR from Linux hosts continues to lead to a timeout.

Fixes #17

Test Plan

Automated tests continue to pass; tested manually with several different registries.

@euanh euanh added area/documentation Improvements or additions to documentation semver/major Breaks existing public API. kind/bug Something isn't working semver/minor Adds new public API. and removed semver/major Breaks existing public API. labels Oct 23, 2024
@euanh euanh merged commit 315b571 into apple:main Oct 23, 2024
19 checks passed
@euanh euanh deleted the eager-auth branch October 23, 2024 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/documentation Improvements or additions to documentation kind/bug Something isn't working semver/minor Adds new public API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pushing images to ECR and Docker Hub times out

1 participant