Skip to content

Handle Blob Create when the underlying registry doesn't provide 'Docker-Upload-UUID' #2927

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

Merged
merged 2 commits into from
Jun 26, 2019
Merged

Handle Blob Create when the underlying registry doesn't provide 'Docker-Upload-UUID' #2927

merged 2 commits into from
Jun 26, 2019

Conversation

dmathieu
Copy link
Contributor

Some registries, such as ECR don't provide a Docker-Upload-UUID HTTP header when starting a blob upload. So we can't rely on that, which then fails when we try to generate the Location header we return (in blobupload.go).

This fallbacks to try reading the UUID from the Location header returned by the underlying service. If we still don't have any UUID, it then returns an error.

I didn't remove @dmcgowan's TODO because we're technically still not checking for invalid UUID, only for empty strings.

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "blob-create-uuid" git@github.com:dmathieu/distribution.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842358448512
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

@dmathieu
Copy link
Contributor Author

@caervs This is another fix for #2922

Copy link
Contributor

@caervs caervs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup LGTM.

For reference, OCI does not require the Docker-Upload-UUID header but does require the Location header. See spec

@caervs caervs requested a review from manishtomar May 24, 2019 04:09
Damien Mathieu added 2 commits June 25, 2019 09:29
Some registries (ECR) don't provide a `Docker-Upload-UUID` when creating
a blob. So we can't rely on that header. Fallback to reading it from the
URL.

Signed-off-by: Damien Mathieu <dmathieu@salesforce.com>
Signed-off-by: Damien Mathieu <dmathieu@salesforce.com>
@codecov
Copy link

codecov bot commented Jun 25, 2019

Codecov Report

Merging #2927 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2927      +/-   ##
==========================================
+ Coverage   60.45%   60.48%   +0.02%     
==========================================
  Files         102      102              
  Lines        8002     8007       +5     
==========================================
+ Hits         4838     4843       +5     
  Misses       2515     2515              
  Partials      649      649
Flag Coverage Δ
#linux 60.48% <100%> (+0.02%) ⬆️
Impacted Files Coverage Δ
registry/client/repository.go 58.26% <100%> (+0.43%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 79f6bcb...a45e5cb. Read the comment docs.

@caervs caervs merged commit 6c72ec2 into distribution:master Jun 26, 2019
@dmathieu dmathieu deleted the blob-create-uuid branch June 26, 2019 06:51
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.

3 participants