Prevent push by tag for sub-manifests

dmcgowan · dmcgowan · commit c965a6c4da0b · 2019-07-31T15:19:44.000-07:00
When pushing a manifest list, all manifests should be pushed by digest
and only the final manifest pushed by tag. The Pusher was preventing
this by mistakenly disallowing objects to contain a digest. When objects
have a digest, only push tags associated with that digest.

Signed-off-by: Derek McGowan &lt;derek@mcgstyle.net&gt;
diff --git a/client.go b/client.go
@@ -403,6 +403,11 @@ func (c *Client) Push(ctx context.Context, ref string, desc ocispec.Descriptor,
 		}
 	}
 
+	// Annotate ref with digest to push only push tag for single digest
+	if !strings.Contains(ref, "@") {
+		ref = ref + "@" + desc.Digest.String()
+	}
+
 	pusher, err := pushCtx.Resolver.Pusher(ctx, ref)
 	if err != nil {
 		return err
diff --git a/remotes/docker/pusher.go b/remotes/docker/pusher.go
@@ -37,7 +37,7 @@ import (
 
 type dockerPusher struct {
 	*dockerBase
-	tag string
+	object string
 
 	// TODO: namespace tracker
 	tracker StatusTracker
@@ -74,11 +74,7 @@ func (p dockerPusher) Push(ctx context.Context, desc ocispec.Descriptor) (conten
 	case images.MediaTypeDockerSchema2Manifest, images.MediaTypeDockerSchema2ManifestList,
 		ocispec.MediaTypeImageManifest, ocispec.MediaTypeImageIndex:
 		isManifest = true
-		if p.tag == "" {
-			existCheck = []string{"manifests", desc.Digest.String()}
-		} else {
-			existCheck = []string{"manifests", p.tag}
-		}
+		existCheck = getManifestPath(p.object, desc.Digest)
 	default:
 		existCheck = []string{"blobs", desc.Digest.String()}
 	}
@@ -97,7 +93,7 @@ func (p dockerPusher) Push(ctx context.Context, desc ocispec.Descriptor) (conten
 	} else {
 		if resp.StatusCode == http.StatusOK {
 			var exists bool
-			if isManifest && p.tag != "" {
+			if isManifest && existCheck[1] != desc.Digest.String() {
 				dgstHeader := digest.Digest(resp.Header.Get("Docker-Content-Digest"))
 				if dgstHeader == desc.Digest {
 					exists = true
@@ -122,13 +118,7 @@ func (p dockerPusher) Push(ctx context.Context, desc ocispec.Descriptor) (conten
 	}
 
 	if isManifest {
-		var putPath []string
-		if p.tag != "" {
-			putPath = []string{"manifests", p.tag}
-		} else {
-			putPath = []string{"manifests", desc.Digest.String()}
-		}
-
+		putPath := getManifestPath(p.object, desc.Digest)
 		req = p.request(host, http.MethodPut, putPath...)
 		req.header.Add("Content-Type", desc.MediaType)
 	} else {
@@ -271,6 +261,25 @@ func (p dockerPusher) Push(ctx context.Context, desc ocispec.Descriptor) (conten
 	}, nil
 }
 
+func getManifestPath(object string, dgst digest.Digest) []string {
+	if i := strings.IndexByte(object, '@'); i >= 0 {
+		if object[i+1:] != dgst.String() {
+			// use digest, not tag
+			object = ""
+		} else {
+			// strip @<digest> for registry path to make tag
+			object = object[:i]
+		}
+
+	}
+
+	if object == "" {
+		return []string{"manifests", dgst.String()}
+	}
+
+	return []string{"manifests", object}
+}
+
 type pushWriter struct {
 	base *dockerBase
 	ref  string
diff --git a/remotes/docker/resolver.go b/remotes/docker/resolver.go
@@ -399,21 +399,14 @@ func (r *dockerResolver) Pusher(ctx context.Context, ref string) (remotes.Pusher
 		return nil, err
 	}
 
-	// Manifests can be pushed by digest like any other object, but the passed in
-	// reference cannot take a digest without the associated content. A tag is allowed
-	// and will be used to tag pushed manifests.
-	if refspec.Object != "" && strings.Contains(refspec.Object, "@") {
-		return nil, errors.New("cannot use digest reference for push locator")
-	}
-
 	base, err := r.base(refspec)
 	if err != nil {
 		return nil, err
 	}
 
 	return dockerPusher{
 		dockerBase: base,
-		tag:        refspec.Object,
+		object:     refspec.Object,
 		tracker:    r.tracker,
 	}, nil
 }

Original file line number	Diff line number	Diff line change
`@@ -403,6 +403,11 @@ func (c *Client) Push(ctx context.Context, ref string, desc ocispec.Descriptor,`
`403`	`403`	`}`
`404`	`404`	`}`
`405`	`405`
	`406`	`+ // Annotate ref with digest to push only push tag for single digest`
	`407`	`+ if !strings.Contains(ref, "@") {`
	`408`	`+ ref = ref + "@" + desc.Digest.String()`
	`409`	`+ }`
	`410`	`+`
`406`	`411`	`pusher, err := pushCtx.Resolver.Pusher(ctx, ref)`
`407`	`412`	`if err != nil {`
`408`	`413`	`return err`