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.

Sign up for GitHub

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

Jump to bottom

Fix handling of errors when fetching layers by URLs #2310

Merged
merged 4 commits into from
Feb 28, 2024
Merged

Fix handling of errors when fetching layers by URLs #2310

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5fa5fc7
Rename err to remoteErrors
mtrmac Feb 26, 2024
6dd8c80
Don't ignore errors reaching out to external layer URLs
mtrmac Feb 26, 2024
a37169c
Return early in getExternalBlob
mtrmac Feb 26, 2024
321d0ec
If we fail fetching a blob from external URLs, report all failures
mtrmac Feb 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 18 additions & 17 deletions docker/docker_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -978,13 +978,10 @@ func (c *dockerClient) fetchManifest(ctx context.Context, ref dockerReference, t
// This function can return nil reader when no url is supported by this function. In this case, the caller
// should fallback to fetch the non-external blob (i.e. pull from the registry).
func (c *dockerClient) getExternalBlob(ctx context.Context, urls []string) (io.ReadCloser, int64, error) {
var (
resp *http.Response
err error
)
if len(urls) == 0 {
return nil, 0, errors.New("internal error: getExternalBlob called with no URLs")
}
var remoteErrors []error
for _, u := range urls {
blobURL, err := url.Parse(u)
if err != nil || (blobURL.Scheme != "http" && blobURL.Scheme != "https") {
Expand All @@ -993,24 +990,28 @@ func (c *dockerClient) getExternalBlob(ctx context.Context, urls []string) (io.R
// NOTE: we must not authenticate on additional URLs as those
// can be abused to leak credentials or tokens. Please
// refer to CVE-2020-15157 for more information.
resp, err = c.makeRequestToResolvedURL(ctx, http.MethodGet, blobURL, nil, nil, -1, noAuth, nil)
if err == nil {
if resp.StatusCode != http.StatusOK {
err = fmt.Errorf("error fetching external blob from %q: %d (%s)", u, resp.StatusCode, http.StatusText(resp.StatusCode))
logrus.Debug(err)
resp.Body.Close()
continue
}
break
resp, err := c.makeRequestToResolvedURL(ctx, http.MethodGet, blobURL, nil, nil, -1, noAuth, nil)
if err != nil {
remoteErrors = append(remoteErrors, err)
continue
}
if resp.StatusCode != http.StatusOK {
err := fmt.Errorf("error fetching external blob from %q: %d (%s)", u, resp.StatusCode, http.StatusText(resp.StatusCode))
remoteErrors = append(remoteErrors, err)
logrus.Debug(err)
resp.Body.Close()
continue
}
return resp.Body, getBlobSize(resp), nil
}
if resp == nil && err == nil {
if remoteErrors == nil {
return nil, 0, nil // fallback to non-external blob
}
if err != nil {
return nil, 0, err
err := fmt.Errorf("failed fetching external blob from all urls: %w", remoteErrors[0])
for _, e := range remoteErrors[1:] {
err = fmt.Errorf("%s, %w", err, e)
}
return resp.Body, getBlobSize(resp), nil
return nil, 0, err
}

func getBlobSize(resp *http.Response) int64 {
Expand Down