Repository: google/go-containerregistry
Pull request #608 (2019-11-13 18:51:44)

Make Descriptor implement Taggable

codecov-io: # [Codecov](https://codecov.io/gh/google/go-containerregistry/pull/608?src=pr&el=h1) Report
> Merging [#608](https://codecov.io/gh/google/go-containerregistry/pull/608?src=pr&el=desc) into [master](https://codecov.io/gh/google/go-containerregistry/commit/56331f7b2c8446ec29e56ceeab6dae5a500ea2ba?src=pr&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `76%`.

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

```diff
@@            Coverage Diff             @@
##           master     #608      +/-   ##
==========================================
+ Coverage   72.93%   72.96%   +0.02%     
==========================================
  Files         102      102              
  Lines        4501     4520      +19     
==========================================
+ Hits         3283     3298      +15     
- Misses        807      809       +2     
- Partials      411      413       +2
```


| [Impacted Files](https://codecov.io/gh/google/go-containerregistry/pull/608?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/v1/remote/descriptor.go](https://codecov.io/gh/google/go-containerregistry/pull/608/diff?src=pr&el=tree#diff-cGtnL3YxL3JlbW90ZS9kZXNjcmlwdG9yLmdv) | `84.78% <100%> (+0.22%)` | :arrow_up: |
| [pkg/v1/remote/write.go](https://codecov.io/gh/google/go-containerregistry/pull/608/diff?src=pr&el=tree#diff-cGtnL3YxL3JlbW90ZS93cml0ZS5nbw==) | `65.01% <73.91%> (+0.73%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/google/go-containerregistry/pull/608?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/608?src=pr&el=footer). Last update [56331f7...fa98790](https://codecov.io/gh/google/go-containerregistry/pull/608?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/pkg/v1/remote/descriptor.go b/pkg/v1/remote/descriptor.go
index 7b3c2c67..36ee6778 100644
--- a/pkg/v1/remote/descriptor.go
+++ b/pkg/v1/remote/descriptor.go
@@ -67,6 +67,11 @@ type Descriptor struct {
 	platform v1.Platform
 }
 
+// RawManifest exists to satisfy the Taggable interface.
+func (d *Descriptor) RawManifest() ([]byte, error) {
+	return d.Manifest, nil
+}
+
 // Get returns a remote.Descriptor for the given reference. The response from
 // the registry is left un-interpreted, for the most part. This is useful for
 // querying what kind of artifact a reference represents.
@@ -85,12 +90,6 @@ func Get(ref name.Reference, options ...Option) (*Descriptor, error) {
 
 // Handle options and fetch the manifest with the acceptable MediaTypes in the
 // Accept header.
-//
-// TODO: We should make it easy to turn a Descriptor into a Taggable so you can:
-// desc, _ := remote.Get(ref)
-// _ = remote.Tag(tag, desc)
-//
-// Go doesn't make this easy since the struct field names conflict with the methods names.
 func get(ref name.Reference, acceptable []types.MediaType, options ...Option) (*Descriptor, error) {
 	o, err := makeOptions(ref.Context(), options...)
 	if err != nil {
diff --git a/pkg/v1/remote/write.go b/pkg/v1/remote/write.go
index 382d5ee3..7be851f0 100644
--- a/pkg/v1/remote/write.go
+++ b/pkg/v1/remote/write.go
@@ -36,8 +36,6 @@ import (
 // Taggable is an interface that enables a manifest PUT (e.g. for tagging).
 type Taggable interface {
 	RawManifest() ([]byte, error)
-	MediaType() (types.MediaType, error)
-	Digest() (v1.Hash, error)
 }
 
 // Write pushes the provided img to the specified image reference.
@@ -366,13 +364,55 @@ func (w *writer) uploadOne(l v1.Layer) error {
 	return retry.Retry(tryUpload, retry.IsTemporary, backoff)
 }
 
+type withMediaType interface {
+	MediaType() (types.MediaType, error)
+}
+
+// This is really silly, but go interfaces don't let me satisfy remote.Taggable
+// with remote.Descriptor because of name collisions between method names and
+// struct fields.
+//
+// Use reflection to either pull the v1.Descriptor out of remote.Descriptor or
+// create a descriptor based on the RawManifest and (optionally) MediaType.
+func unpackTaggable(t Taggable) (*v1.Descriptor, error) {
+	if d, ok := t.(*Descriptor); ok {
+		return &d.Descriptor, nil
+	}
+	b, err := t.RawManifest()
+	if err != nil {
+		return nil, err
+	}
+
+	// A reasonable default if Taggable doesn't implement MediaType.
+	mt := types.DockerManifestSchema2
+
+	if wmt, ok := t.(withMediaType); ok {
+		m, err := wmt.MediaType()
+		if err != nil {
+			return nil, err
+		}
+		mt = m
+	}
+
+	h, sz, err := v1.SHA256(bytes.NewReader(b))
+	if err != nil {
+		return nil, err
+	}
+
+	return &v1.Descriptor{
+		MediaType: mt,
+		Size:      sz,
+		Digest:    h,
+	}, nil
+}
+
 // commitImage does a PUT of the image's manifest.
 func (w *writer) commitImage(t Taggable) error {
 	raw, err := t.RawManifest()
 	if err != nil {
 		return err
 	}
-	mt, err := t.MediaType()
+	desc, err := unpackTaggable(t)
 	if err != nil {
 		return err
 	}
@@ -384,7 +424,7 @@ func (w *writer) commitImage(t Taggable) error {
 	if err != nil {
 		return err
 	}
-	req.Header.Set("Content-Type", string(mt))
+	req.Header.Set("Content-Type", string(desc.MediaType))
 
 	resp, err := w.client.Do(req)
 	if err != nil {
@@ -396,13 +436,8 @@ func (w *writer) commitImage(t Taggable) error {
 		return err
 	}
 
-	digest, err := t.Digest()
-	if err != nil {
-		return err
-	}
-
 	// The image was successfully pushed!
-	logs.Progress.Printf("%v: digest: %v size: %d", w.ref, digest, len(raw))
+	logs.Progress.Printf("%v: digest: %v size: %d", w.ref, desc.Digest, len(raw))
 	return nil
 }
 


Test patch:
diff --git a/pkg/v1/remote/image_test.go b/pkg/v1/remote/image_test.go
index 9cbea49d..0095d50a 100644
--- a/pkg/v1/remote/image_test.go
+++ b/pkg/v1/remote/image_test.go
@@ -39,7 +39,11 @@ import (
 
 const bogusDigest = "sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef"
 
-func mustDigest(t *testing.T, img Taggable) v1.Hash {
+type withDigest interface {
+	Digest() (v1.Hash, error)
+}
+
+func mustDigest(t *testing.T, img withDigest) v1.Hash {
 	h, err := img.Digest()
 	if err != nil {
 		t.Fatalf("Digest() = %v", err)
diff --git a/pkg/v1/remote/index_test.go b/pkg/v1/remote/index_test.go
index 81457e5f..dbec16e5 100644
--- a/pkg/v1/remote/index_test.go
+++ b/pkg/v1/remote/index_test.go
@@ -52,7 +52,7 @@ func mustChild(t *testing.T, idx v1.ImageIndex, h v1.Hash) v1.Image {
 	return img
 }
 
-func mustMediaType(t *testing.T, tag Taggable) types.MediaType {
+func mustMediaType(t *testing.T, tag withMediaType) types.MediaType {
 	mt, err := tag.MediaType()
 	if err != nil {
 		t.Fatalf("MediaType() = %v", err)
diff --git a/pkg/v1/remote/write_test.go b/pkg/v1/remote/write_test.go
index 8ca72a37..3d356736 100644
--- a/pkg/v1/remote/write_test.go
+++ b/pkg/v1/remote/write_test.go
@@ -951,7 +951,7 @@ func TestScopesForUploadingImage(t *testing.T) {
 		{
 			name:      "mountable layers with same reference",
 			reference: referenceToUpload,
-			layers:    []v1.Layer{
+			layers: []v1.Layer{
 				&MountableLayer{
 					Layer:     dummyLayer,
 					Reference: sameReference,
@@ -1290,6 +1290,41 @@ func TestTag(t *testing.T) {
 	}
 }
 
+func TestTagDescriptor(t *testing.T) {
+	idx := setupIndex(t, 3)
+	// Set up a fake registry.
+	s := httptest.NewServer(registry.New())
+	defer s.Close()
+	u, err := url.Parse(s.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	src := fmt.Sprintf("%s/test/tag:src", u.Host)
+	srcRef, err := name.NewTag(src)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if err := WriteIndex(srcRef, idx); err != nil {
+		t.Fatal(err)
+	}
+
+	desc, err := Get(srcRef)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	dst := fmt.Sprintf("%s/test/tag:dst", u.Host)
+	dstRef, err := name.NewTag(dst)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if err := Tag(dstRef, desc); err != nil {
+		t.Fatal(err)
+	}
+}
+
 func BenchmarkWrite(b *testing.B) {
 	// unfortunately the registry _and_ the img have caching behaviour, so we need a new registry
 	// and image every iteration of benchmarking.