Adds cross-repository blob mounting behavior #1269

BrianBland · 2015-12-15T22:28:02Z

Extends blob upload POST endpoint to support mount and from query
parameters as described in #634

aaronlehmann · 2015-12-15T23:29:32Z

registry/storage/blob_test.go

+		t.Errorf("Unexpected error type stat-ing deleted manifest: %#v", err)
+	}
+}
+


If it's not too difficult, it would be great to have a test that ensures that an attempt to mount a source image outside the authorization scope fails.

Can that be broken out into a specific test? It will need a mocked auth implementation to test this properly.

stevvooe · 2015-12-16T01:45:06Z

registry/handlers/blobupload.go

 	blobs := buh.Repository.Blobs(buh)
+
+	if mountDigest != "" && fromRepo != "" {


Could we put this in a separate method?

stevvooe · 2015-12-16T22:52:45Z

@BrianBland Have we updated the specification for these additions?

RichardScothern · 2015-12-17T22:39:36Z

registry/api/v2/routes.go and docs/spec/api.md.tmpl should be updated

dmcgowan · 2015-12-21T19:54:49Z

registry/client/repository.go

+	defer resp.Body.Close()
+
+	if SuccessStatus(resp.StatusCode) {
+		// TODO(brianbland): Check for possible Accepted response (legacy create)


We also need a mechanism here to create and get httpBlobUpload. Possibly cache it and have the next call to Create use it. Returning it as part of an error does not seem like an ideal solution for fallback.

codecov-io · 2015-12-21T20:25:52Z

Current coverage is `57.73%`

Merging #1269 into master will decrease coverage by -0.49% as of 613cfc8

@@            master   #1269   diff @@
======================================
  Files          123     123       
  Stmts        10932   11071   +139
  Branches       762     773    +11
  Methods          0       0       
======================================
+ Hit           6365    6392    +27
- Partial        762     773    +11
- Missed        3805    3906   +101

Review entire Coverage Diff as of 613cfc8

Uncovered Suggestions

+0.29% via ...ge/driver/gcs/gcs.go#185...216
+0.29% via ...ge/driver/gcs/gcs.go#104...135
+0.22% via ...ge/driver/gcs/gcs.go#474...497
See 7 more...

Powered by Codecov. Updated on successful CI builds.

vbatts · 2015-12-23T19:59:44Z

cc @jwhonce

stevvooe · 2016-01-05T20:47:32Z

docs/spec/api.md

@@ -1023,7 +1051,7 @@ A list of methods and URIs are covered in the table below:
 |------|----|------|-----------|
 | GET | `/v2/` | Base | Check that the endpoint implements Docker Registry API V2. |
 | GET | `/v2/<name>/tags/list` | Tags | Fetch the tags under the repository identified by `name`. |
-| GET | `/v2/<name>/manifests/<reference>` | Manifest | Fetch the manifest identified by `name` and `reference` where `reference` can be a tag or digest. |
+| GET | `/v2/<name>/manifests/<reference>` | Manifest | Fetch the manifest identified by `name` and `reference` where `reference` can be a tag or digest. A `HEAD` request can also be issued to this endpoint to obtain resource information without receiving all data. |


This is redundant. That is the definition of a HEAD request. Would it be better to state somewhere at the start that all GET should support HEAD?

Hmm, this was already in the api descriptors file but had not been filled into the spec template. We can update this descriptor, but it probably belongs in a separate PR.

O, ick. Separate PR. This should be removed.

stevvooe · 2016-01-06T22:32:17Z

AUTHORS

@@ -21,6 +21,7 @@ Ben Firshman <ben@firshman.co.uk>
 bin liu <liubin0329@gmail.com>


Don't include AUTHORS update in your PR. We do this at release time.

👍 removing this

stevvooe · 2016-01-06T23:25:25Z

After writing #1269 (comment), I realized it might be a good idea to have an event specific to the mount call on the target repository. It may be sufficient to use the url to detect this but we may want to consider adding an action.

Looking here, we just need to have an action for "mount" and a source indicating where the mount happened from. Optionally, it may be okay to leave these as "push" but then populate a "from" field on target.

Extends blob upload POST endpoint to support mount and from query parameters as described in distribution#634 Signed-off-by: Brian Bland <brian.bland@docker.com>

When an auth request provides the "from" query parameter, the token handler will add a "pull" scope for the provided repository, refreshing the token if the overall scope has increased Signed-off-by: Brian Bland <brian.bland@docker.com>

Adds an optional "fromRepository" field to the event target Signed-off-by: Brian Bland <brian.bland@docker.com>

aaronlehmann · 2016-01-08T22:17:46Z

registry/client/repository.go

@@ -604,6 +621,45 @@ func (bs *blobs) Resume(ctx context.Context, id string) (distribution.BlobWriter
 	panic("not implemented")
 }

+func (bs *blobs) Mount(ctx context.Context, sourceRepo string, dgst digest.Digest) (distribution.Descriptor, error) {


I think sourceRepo should be reference.Named rather than a string. Perhaps the digest should also be incorporated into this argument (in that case, it would be reference.Canonical). Not sure if that makes sense here, since canonical references are usually used for identifying manifests, not blobs. @dmcgowan @stevvooe WDYT?

As is this use of string is consistent with the rest of the client package. I would agree though that it would be desirable to refactor the interfaces to use the reference package, I would not enforce that here.

Yeah, I got confused about this. I remembered refactoring everything to use the reference package in #1270, but it turns out that didn't get merged and I closed it until other PRs got merged. I'll reopen that PR and bring it up to date, but this PR can go in as-is first.

RichardScothern · 2016-01-08T22:19:37Z

LGTM

dmcgowan · 2016-01-08T22:26:37Z

LGTM

aaronlehmann · 2016-01-08T22:26:48Z

LGTM

Adds cross-repository blob mounting behavior

BrianBland added the feature label Dec 15, 2015

BrianBland added this to the Registry/2.3 milestone Dec 15, 2015

GordonTheTurtle added group/distribution labels Dec 15, 2015

BrianBland force-pushed the crossRepositoryPush branch from f8450cb to 95ab2c0 Compare December 15, 2015 22:28

GordonTheTurtle removed the dco/no label Dec 15, 2015

aaronlehmann reviewed Dec 15, 2015
View reviewed changes

RichardScothern mentioned this pull request Dec 15, 2015

Questions about push and pull compare to registry v1 - performance issue #1216

Closed

BrianBland self-assigned this Dec 16, 2015

stevvooe reviewed Dec 16, 2015
View reviewed changes

BrianBland force-pushed the crossRepositoryPush branch from 95ab2c0 to ccf10a9 Compare December 16, 2015 02:15

BrianBland force-pushed the crossRepositoryPush branch 2 times, most recently from ac72830 to 9a7e897 Compare December 21, 2015 19:42

dmcgowan reviewed Dec 21, 2015
View reviewed changes

BrianBland force-pushed the crossRepositoryPush branch from 9a7e897 to 3d5f09e Compare December 21, 2015 20:07

aaronlehmann mentioned this pull request Dec 23, 2015

1.10: keeps pushing same image moby/moby#18866

Closed

dmcgowan mentioned this pull request Dec 23, 2015

Proposal: push performance improvements moby/moby#14018

Closed

3 tasks

BrianBland force-pushed the crossRepositoryPush branch 4 times, most recently from 80c0b4c to 16b3ef1 Compare December 30, 2015 00:37

BrianBland force-pushed the crossRepositoryPush branch from 16b3ef1 to bf453e2 Compare January 5, 2016 19:15

GordonTheTurtle removed the dco/no label Jan 5, 2016

stevvooe reviewed Jan 5, 2016
View reviewed changes

BrianBland force-pushed the crossRepositoryPush branch 5 times, most recently from d780bdc to d5127c3 Compare January 6, 2016 22:30

stevvooe reviewed Jan 6, 2016
View reviewed changes

BrianBland force-pushed the crossRepositoryPush branch from d5127c3 to ebc5b79 Compare January 6, 2016 22:46

BrianBland force-pushed the crossRepositoryPush branch 4 times, most recently from 88aebc8 to 553d03d Compare January 8, 2016 18:35

BrianBland added 3 commits January 8, 2016 13:53

Adds cross-repository blob mounting behavior

5df2157

Extends blob upload POST endpoint to support mount and from query parameters as described in distribution#634 Signed-off-by: Brian Bland <brian.bland@docker.com>

Fires a new Mount event when blobs are cross-repo mounted

613cfc8

Adds an optional "fromRepository" field to the event target Signed-off-by: Brian Bland <brian.bland@docker.com>

BrianBland force-pushed the crossRepositoryPush branch from 553d03d to 613cfc8 Compare January 8, 2016 21:53

aaronlehmann reviewed Jan 8, 2016
View reviewed changes

RichardScothern pushed a commit that referenced this pull request Jan 8, 2016

Merge pull request #1269 from BrianBland/crossRepositoryPush

93d9070

Adds cross-repository blob mounting behavior

RichardScothern merged commit 93d9070 into distribution:master Jan 8, 2016

BrianBland deleted the crossRepositoryPush branch January 8, 2016 23:05

aaronlehmann mentioned this pull request Jan 12, 2016

Cross Repository Push #908

Closed

thaJeztah pushed a commit to thaJeztah/distribution that referenced this pull request Apr 22, 2021

Merge pull request distribution#1269 from BrianBland/crossRepositoryPush

81ac52a

Adds cross-repository blob mounting behavior

thaJeztah pushed a commit to thaJeztah/distribution that referenced this pull request Jan 19, 2022

Merge pull request distribution#1269 from BrianBland/crossRepositoryPush

5fe3d68

Adds cross-repository blob mounting behavior

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds cross-repository blob mounting behavior #1269

Adds cross-repository blob mounting behavior #1269

BrianBland commented Dec 15, 2015

aaronlehmann Dec 15, 2015

stevvooe Jan 6, 2016

stevvooe Dec 16, 2015

stevvooe commented Dec 16, 2015

RichardScothern commented Dec 17, 2015

dmcgowan Dec 21, 2015

codecov-io commented Dec 21, 2015

vbatts commented Dec 23, 2015

stevvooe Jan 5, 2016

BrianBland Jan 5, 2016

stevvooe Jan 6, 2016

stevvooe Jan 6, 2016

BrianBland Jan 6, 2016

stevvooe commented Jan 6, 2016

aaronlehmann Jan 8, 2016

dmcgowan Jan 8, 2016

aaronlehmann Jan 8, 2016

RichardScothern commented Jan 8, 2016

dmcgowan commented Jan 8, 2016

aaronlehmann commented Jan 8, 2016

		blobs := buh.Repository.Blobs(buh)

		if mountDigest != "" && fromRepo != "" {

		@@ -21,6 +21,7 @@ Ben Firshman <ben@firshman.co.uk>
		bin liu <liubin0329@gmail.com>

Adds cross-repository blob mounting behavior #1269

Adds cross-repository blob mounting behavior #1269

Conversation

BrianBland commented Dec 15, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevvooe commented Dec 16, 2015

RichardScothern commented Dec 17, 2015

Choose a reason for hiding this comment

codecov-io commented Dec 21, 2015

Current coverage is 57.73%

vbatts commented Dec 23, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevvooe commented Jan 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RichardScothern commented Jan 8, 2016

dmcgowan commented Jan 8, 2016

aaronlehmann commented Jan 8, 2016

Current coverage is `57.73%`