Repository: google/go-containerregistry
Pull request #662 (2020-01-22 00:41:32)

Change signature of remote.WriteLayer
This is breaking, but I suspect there aren't many consumers. We don't
actually need a name.Digest in order to upload a layer, it was just
easier because the writer{} took a ref. I restructured commitImage to
take the ref directly instead of stashing it away in the writer{}.

If you wanted to do a streaming layer upload with WriteLayer, you'd have
to conjure up a fake name.Digest. That is no longer required, since all
we really need is the repository.

codecov-io: # [Codecov](https://codecov.io/gh/google/go-containerregistry/pull/662?src=pr&el=h1) Report
> Merging [#662](https://codecov.io/gh/google/go-containerregistry/pull/662?src=pr&el=desc) into [master](https://codecov.io/gh/google/go-containerregistry/commit/b0ae1fc74a66d96004910e9809c2b03562a410cd?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `96.15%`.

[![Impacted file tree graph](https://codecov.io/gh/google/go-containerregistry/pull/662/graphs/tree.svg?width=650&token=wG7iiDCdZ4&height=150&src=pr)](https://codecov.io/gh/google/go-containerregistry/pull/662?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #662   +/-   ##
=======================================
  Coverage   78.37%   78.37%           
=======================================
  Files         102      102           
  Lines        4518     4518           
=======================================
  Hits         3541     3541           
  Misses        562      562           
  Partials      415      415
```


| [Impacted Files](https://codecov.io/gh/google/go-containerregistry/pull/662?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/internal/legacy/copy.go](https://codecov.io/gh/google/go-containerregistry/pull/662/diff?src=pr&el=tree#diff-cGtnL2ludGVybmFsL2xlZ2FjeS9jb3B5Lmdv) | `61.11% <0%> (ø)` | :arrow_up: |
| [pkg/v1/remote/check.go](https://codecov.io/gh/google/go-containerregistry/pull/662/diff?src=pr&el=tree#diff-cGtnL3YxL3JlbW90ZS9jaGVjay5nbw==) | `70% <100%> (ø)` | :arrow_up: |
| [pkg/v1/remote/write.go](https://codecov.io/gh/google/go-containerregistry/pull/662/diff?src=pr&el=tree#diff-cGtnL3YxL3JlbW90ZS93cml0ZS5nbw==) | `67.49% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/google/go-containerregistry/pull/662?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/google/go-containerregistry/pull/662?src=pr&el=footer). Last update [b0ae1fc...5321e35](https://codecov.io/gh/google/go-containerregistry/pull/662?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/pkg/internal/legacy/copy.go b/pkg/internal/legacy/copy.go
index 9b073d22..ba8f96ab 100644
--- a/pkg/internal/legacy/copy.go
+++ b/pkg/internal/legacy/copy.go
@@ -46,7 +46,7 @@ func CopySchema1(desc *remote.Descriptor, srcRef, dstRef name.Reference, srcAuth
 			return err
 		}
 
-		if err := remote.WriteLayer(dst, blob, remote.WithAuth(dstAuth)); err != nil {
+		if err := remote.WriteLayer(dst.Context(), blob, remote.WithAuth(dstAuth)); err != nil {
 			return err
 		}
 	}
diff --git a/pkg/v1/remote/check.go b/pkg/v1/remote/check.go
index da0fa24c..65ee4e7f 100644
--- a/pkg/v1/remote/check.go
+++ b/pkg/v1/remote/check.go
@@ -34,7 +34,7 @@ func CheckPushPermission(ref name.Reference, kc authn.Keychain, t http.RoundTrip
 	// authorize a push. Figure out how to return early here when we can,
 	// to avoid a roundtrip for spec-compliant registries.
 	w := writer{
-		ref:    ref,
+		repo:   ref.Context(),
 		client: &http.Client{Transport: tr},
 	}
 	loc, _, err := w.initiateUpload("", "")
diff --git a/pkg/v1/remote/write.go b/pkg/v1/remote/write.go
index 7be851f0..7b431c07 100644
--- a/pkg/v1/remote/write.go
+++ b/pkg/v1/remote/write.go
@@ -50,13 +50,13 @@ func Write(ref name.Reference, img v1.Image, options ...Option) error {
 		return err
 	}
 
-	scopes := scopesForUploadingImage(ref, ls)
+	scopes := scopesForUploadingImage(ref.Context(), ls)
 	tr, err := transport.New(ref.Context().Registry, o.auth, o.transport, scopes)
 	if err != nil {
 		return err
 	}
 	w := writer{
-		ref:    ref,
+		repo:   ref.Context(),
 		client: &http.Client{Transport: tr},
 	}
 
@@ -125,20 +125,20 @@ func Write(ref name.Reference, img v1.Image, options ...Option) error {
 
 	// With all of the constituent elements uploaded, upload the manifest
 	// to commit the image.
-	return w.commitImage(img)
+	return w.commitImage(img, ref)
 }
 
 // writer writes the elements of an image to a remote image reference.
 type writer struct {
-	ref    name.Reference
+	repo   name.Repository
 	client *http.Client
 }
 
 // url returns a url.Url for the specified path in the context of this remote image reference.
 func (w *writer) url(path string) url.URL {
 	return url.URL{
-		Scheme: w.ref.Context().Registry.Scheme(),
-		Host:   w.ref.Context().RegistryStr(),
+		Scheme: w.repo.Registry.Scheme(),
+		Host:   w.repo.RegistryStr(),
 		Path:   path,
 	}
 }
@@ -164,7 +164,7 @@ func (w *writer) nextLocation(resp *http.Response) (string, error) {
 // initiation if "mount" is specified, even if no "from" sources are specified.
 // However, this is not broadly applicable to all registries, e.g. ECR.
 func (w *writer) checkExistingBlob(h v1.Hash) (bool, error) {
-	u := w.url(fmt.Sprintf("/v2/%s/blobs/%s", w.ref.Context().RepositoryStr(), h.String()))
+	u := w.url(fmt.Sprintf("/v2/%s/blobs/%s", w.repo.RepositoryStr(), h.String()))
 
 	resp, err := w.client.Head(u.String())
 	if err != nil {
@@ -182,7 +182,7 @@ func (w *writer) checkExistingBlob(h v1.Hash) (bool, error) {
 // checkExistingManifest checks if a manifest exists already in the repository
 // by making a HEAD request to the manifest API.
 func (w *writer) checkExistingManifest(h v1.Hash, mt types.MediaType) (bool, error) {
-	u := w.url(fmt.Sprintf("/v2/%s/manifests/%s", w.ref.Context().RepositoryStr(), h.String()))
+	u := w.url(fmt.Sprintf("/v2/%s/manifests/%s", w.repo.RepositoryStr(), h.String()))
 
 	req, err := http.NewRequest(http.MethodHead, u.String(), nil)
 	if err != nil {
@@ -210,7 +210,7 @@ func (w *writer) checkExistingManifest(h v1.Hash, mt types.MediaType) (bool, err
 // upload was initiated and the body of that blob should be sent to the returned
 // location.
 func (w *writer) initiateUpload(from, mount string) (location string, mounted bool, err error) {
-	u := w.url(fmt.Sprintf("/v2/%s/blobs/uploads/", w.ref.Context().RepositoryStr()))
+	u := w.url(fmt.Sprintf("/v2/%s/blobs/uploads/", w.repo.RepositoryStr()))
 	uv := url.Values{}
 	if mount != "" && from != "" {
 		// Quay will fail if we specify a "mount" without a "from".
@@ -313,7 +313,7 @@ func (w *writer) uploadOne(l v1.Layer) error {
 		mount = h.String()
 	}
 	if ml, ok := l.(*MountableLayer); ok {
-		if w.ref.Context().RegistryStr() == ml.Reference.Context().RegistryStr() {
+		if w.repo.RegistryStr() == ml.Reference.Context().RegistryStr() {
 			from = ml.Reference.Context().RepositoryStr()
 		}
 	}
@@ -407,7 +407,7 @@ func unpackTaggable(t Taggable) (*v1.Descriptor, error) {
 }
 
 // commitImage does a PUT of the image's manifest.
-func (w *writer) commitImage(t Taggable) error {
+func (w *writer) commitImage(t Taggable, ref name.Reference) error {
 	raw, err := t.RawManifest()
 	if err != nil {
 		return err
@@ -417,7 +417,7 @@ func (w *writer) commitImage(t Taggable) error {
 		return err
 	}
 
-	u := w.url(fmt.Sprintf("/v2/%s/manifests/%s", w.ref.Context().RepositoryStr(), w.ref.Identifier()))
+	u := w.url(fmt.Sprintf("/v2/%s/manifests/%s", w.repo.RepositoryStr(), ref.Identifier()))
 
 	// Make the request to PUT the serialized manifest
 	req, err := http.NewRequest(http.MethodPut, u.String(), bytes.NewBuffer(raw))
@@ -437,11 +437,11 @@ func (w *writer) commitImage(t Taggable) error {
 	}
 
 	// The image was successfully pushed!
-	logs.Progress.Printf("%v: digest: %v size: %d", w.ref, desc.Digest, len(raw))
+	logs.Progress.Printf("%v: digest: %v size: %d", ref, desc.Digest, len(raw))
 	return nil
 }
 
-func scopesForUploadingImage(ref name.Reference, layers []v1.Layer) []string {
+func scopesForUploadingImage(repo name.Repository, layers []v1.Layer) []string {
 	// use a map as set to remove duplicates scope strings
 	scopeSet := map[string]struct{}{}
 
@@ -449,7 +449,7 @@ func scopesForUploadingImage(ref name.Reference, layers []v1.Layer) []string {
 		if ml, ok := l.(*MountableLayer); ok {
 			// we will add push scope for ref.Context() after the loop.
 			// for now we ask pull scope for references of the same registry
-			if ml.Reference.Context() != ref.Context() && ml.Reference.Context().Registry == ref.Context().Registry {
+			if ml.Reference.Context() != repo && ml.Reference.Context().Registry == repo.Registry {
 				scopeSet[ml.Reference.Scope(transport.PullScope)] = struct{}{}
 			}
 		}
@@ -457,7 +457,7 @@ func scopesForUploadingImage(ref name.Reference, layers []v1.Layer) []string {
 
 	scopes := make([]string, 0)
 	// Push scope should be the first element because a few registries just look at the first scope to determine access.
-	scopes = append(scopes, ref.Scope(transport.PushScope))
+	scopes = append(scopes, repo.Scope(transport.PushScope))
 
 	for scope := range scopeSet {
 		scopes = append(scopes, scope)
@@ -485,7 +485,7 @@ func WriteIndex(ref name.Reference, ii v1.ImageIndex, options ...Option) error {
 		return err
 	}
 	w := writer{
-		ref:    ref,
+		repo:   ref.Context(),
 		client: &http.Client{Transport: tr},
 	}
 
@@ -523,22 +523,22 @@ func WriteIndex(ref name.Reference, ii v1.ImageIndex, options ...Option) error {
 
 	// With all of the constituent elements uploaded, upload the manifest
 	// to commit the image.
-	return w.commitImage(ii)
+	return w.commitImage(ii, ref)
 }
 
-// WriteLayer uploads the provided Layer to the specified name.Digest.
-func WriteLayer(ref name.Digest, layer v1.Layer, options ...Option) error {
-	o, err := makeOptions(ref.Context(), options...)
+// WriteLayer uploads the provided Layer to the specified repo.
+func WriteLayer(repo name.Repository, layer v1.Layer, options ...Option) error {
+	o, err := makeOptions(repo, options...)
 	if err != nil {
 		return err
 	}
-	scopes := scopesForUploadingImage(ref, []v1.Layer{layer})
-	tr, err := transport.New(ref.Context().Registry, o.auth, o.transport, scopes)
+	scopes := scopesForUploadingImage(repo, []v1.Layer{layer})
+	tr, err := transport.New(repo.Registry, o.auth, o.transport, scopes)
 	if err != nil {
 		return err
 	}
 	w := writer{
-		ref:    ref,
+		repo:   repo,
 		client: &http.Client{Transport: tr},
 	}
 
@@ -564,9 +564,9 @@ func Tag(tag name.Tag, t Taggable, options ...Option) error {
 		return err
 	}
 	w := writer{
-		ref:    tag,
+		repo:   tag.Context(),
 		client: &http.Client{Transport: tr},
 	}
 
-	return w.commitImage(t)
+	return w.commitImage(t, tag)
 }


Test patch:
diff --git a/pkg/internal/legacy/copy_test.go b/pkg/internal/legacy/copy_test.go
index 414e3c4c..49c0c58f 100644
--- a/pkg/internal/legacy/copy_test.go
+++ b/pkg/internal/legacy/copy_test.go
@@ -66,7 +66,7 @@ func TestCopySchema1(t *testing.T) {
 	}
 
 	// Populate the registry with a layer and a schema 1 manifest referencing it.
-	if err := remote.WriteLayer(layerRef, layer); err != nil {
+	if err := remote.WriteLayer(layerRef.Context(), layer); err != nil {
 		t.Fatal(err)
 	}
 	manifest := schema1{
diff --git a/pkg/v1/remote/layer_test.go b/pkg/v1/remote/layer_test.go
index 4708d83d..70c5ef75 100644
--- a/pkg/v1/remote/layer_test.go
+++ b/pkg/v1/remote/layer_test.go
@@ -56,7 +56,7 @@ func TestRemoteLayer(t *testing.T) {
 	}
 
 	t.Log(ref)
-	if err := WriteLayer(ref, layer); err != nil {
+	if err := WriteLayer(ref.Context(), layer); err != nil {
 		t.Fatalf("failed to WriteLayer: %v", err)
 	}
 
diff --git a/pkg/v1/remote/write_test.go b/pkg/v1/remote/write_test.go
index 89161487..40b6cfef 100644
--- a/pkg/v1/remote/write_test.go
+++ b/pkg/v1/remote/write_test.go
@@ -70,7 +70,7 @@ func TestUrl(t *testing.T) {
 
 	for _, test := range tests {
 		w := &writer{
-			ref: mustNewTag(t, test.tag),
+			repo: mustNewTag(t, test.tag).Context(),
 		}
 		if got, want := w.url(test.path), test.url; got.String() != want {
 			t.Errorf("url(%v) = %v, want %v", test.path, got.String(), want)
@@ -92,7 +92,7 @@ func TestNextLocation(t *testing.T) {
 
 	ref := mustNewTag(t, "gcr.io/foo/bar:latest")
 	w := &writer{
-		ref: ref,
+		repo: ref.Context(),
 	}
 
 	for _, test := range tests {
@@ -165,7 +165,7 @@ func setupWriterWithServer(server *httptest.Server, repo string) (*writer, close
 	}
 
 	return &writer{
-		ref:    tag,
+		repo:   tag.Context(),
 		client: http.DefaultClient,
 	}, server, nil
 }
@@ -701,7 +701,7 @@ func TestUploadOne(t *testing.T) {
 	}
 	ml := &MountableLayer{
 		Layer:     l,
-		Reference: w.ref,
+		Reference: w.repo.Digest(h.String()),
 	}
 	if err := w.uploadOne(ml); err != nil {
 		t.Errorf("uploadOne() = %v", err)
@@ -811,8 +811,8 @@ func TestCommitImage(t *testing.T) {
 	}
 	defer closer.Close()
 
-	if err := w.commitImage(img); err != nil {
-		t.Errorf("commitBlob() = %v", err)
+	if err := w.commitImage(img, w.repo.Tag("latest")); err != nil {
+		t.Errorf("commitImage() = %v", err)
 	}
 }
 
@@ -1069,7 +1069,7 @@ func TestScopesForUploadingImage(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		actual := scopesForUploadingImage(tc.reference, tc.layers)
+		actual := scopesForUploadingImage(tc.reference.Context(), tc.layers)
 
 		if want, got := tc.expected[0], actual[0]; want != got {
 			t.Errorf("TestScopesForUploadingImage() %s: Wrong first scope; want %v, got %v", tc.name, want, got)