Implement immutable manifest reference support

This changeset implements immutable manifest references via the HTTP API. Most
of the changes follow from modifications to ManifestService. Once updates were
made across the repo to implement these changes, the http handlers were change
accordingly. The new methods on ManifestService will be broken out into a
tagging service in a later PR.

Unfortunately, due to complexities around managing the manifest tag index in an
eventually consistent manner, direct deletes of manifests have been disabled.

Signed-off-by: Stephen J Day <stephen.day@docker.com>

notifications/listener.go

@@ -67,8 +67,8 @@ type manifestServiceListener struct {
 	parent *repositoryListener
 }
 
-func (msl *manifestServiceListener) Get(tag string) (*manifest.SignedManifest, error) {
-	sm, err := msl.ManifestService.Get(tag)
+func (msl *manifestServiceListener) Get(dgst digest.Digest) (*manifest.SignedManifest, error) {
+	sm, err := msl.ManifestService.Get(dgst)
 	if err == nil {
 		if err := msl.parent.listener.ManifestPulled(msl.parent.Repository, sm); err != nil {
 			logrus.Errorf("error dispatching manifest pull to listener: %v", err)
@@ -78,8 +78,8 @@ func (msl *manifestServiceListener) Get(tag string) (*manifest.SignedManifest, e
 	return sm, err
 }
 
-func (msl *manifestServiceListener) Put(tag string, sm *manifest.SignedManifest) error {
-	err := msl.ManifestService.Put(tag, sm)
+func (msl *manifestServiceListener) Put(sm *manifest.SignedManifest) error {
+	err := msl.ManifestService.Put(sm)
 
 	if err == nil {
 		if err := msl.parent.listener.ManifestPushed(msl.parent.Repository, sm); err != nil {
@@ -90,6 +90,17 @@ func (msl *manifestServiceListener) Put(tag string, sm *manifest.SignedManifest)
 	return err
 }
 
+func (msl *manifestServiceListener) GetByTag(tag string) (*manifest.SignedManifest, error) {
+	sm, err := msl.ManifestService.GetByTag(tag)
+	if err == nil {
+		if err := msl.parent.listener.ManifestPulled(msl.parent.Repository, sm); err != nil {
+			logrus.Errorf("error dispatching manifest pull to listener: %v", err)
+		}
+	}
+
+	return sm, err
+}
+
 type layerServiceListener struct {
 	distribution.LayerService
 	parent *repositoryListener

notifications/listener_test.go

@@ -32,7 +32,7 @@ func TestListener(t *testing.T) {
 
 	expectedOps := map[string]int{
 		"manifest:push": 1,
-		"manifest:pull": 1,
+		"manifest:pull": 2,
 		// "manifest:delete": 0, // deletes not supported for now
 		"layer:push": 2,
 		"layer:pull": 2,
@@ -143,11 +143,30 @@ func checkExerciseRepository(t *testing.T, repository distribution.Repository) {
 
 	manifests := repository.Manifests()
 
-	if err := manifests.Put(tag, sm); err != nil {
+	if err := manifests.Put(sm); err != nil {
 		t.Fatalf("unexpected error putting the manifest: %v", err)
 	}
 
-	fetched, err := manifests.Get(tag)
+	p, err := sm.Payload()
+	if err != nil {
+		t.Fatalf("unexpected error getting manifest payload: %v", err)
+	}
+
+	dgst, err := digest.FromBytes(p)
+	if err != nil {
+		t.Fatalf("unexpected error digesting manifest payload: %v", err)
+	}
+
+	fetchedByManifest, err := manifests.Get(dgst)
+	if err != nil {
+		t.Fatalf("unexpected error fetching manifest: %v", err)
+	}
+
+	if fetchedByManifest.Tag != fetchedByManifest.Tag {
+		t.Fatalf("retrieved unexpected manifest: %v", err)
+	}
+
+	fetched, err := manifests.GetByTag(tag)
 	if err != nil {
 		t.Fatalf("unexpected error fetching manifest: %v", err)
 	}

registry.go

@@ -31,38 +31,41 @@ type Repository interface {
 
 // ManifestService provides operations on image manifests.
 type ManifestService interface {
-	// Tags lists the tags under the named repository.
-	Tags() ([]string, error)
-
 	// Exists returns true if the manifest exists.
-	Exists(tag string) (bool, error)
+	Exists(dgst digest.Digest) (bool, error)
+
+	// GetByTag retrieves the named manifest, if it exists.
+	Get(dgst digest.Digest) (*manifest.SignedManifest, error)
+
+	// Delete removes the manifest, if it exists.
+	Delete(dgst digest.Digest) error
 
-	// Get retrieves the named manifest, if it exists.
-	Get(tag string) (*manifest.SignedManifest, error)
+	// Put creates or updates the manifest.
+	Put(manifest *manifest.SignedManifest) error
+
+	// TODO(stevvooe): The methods after this message should be moved to a
+	// discrete TagService, per active proposals.
+
+	// Tags lists the tags under the named repository.
+	Tags() ([]string, error)
 
-	// Put creates or updates the named manifest.
-	// Put(tag string, manifest *manifest.SignedManifest) (digest.Digest, error)
-	Put(tag string, manifest *manifest.SignedManifest) error
+	// ExistsByTag returns true if the manifest exists.
+	ExistsByTag(tag string) (bool, error)
 
-	// Delete removes the named manifest, if it exists.
-	Delete(tag string) error
+	// GetByTag retrieves the named manifest, if it exists.
+	GetByTag(tag string) (*manifest.SignedManifest, error)
 
 	// TODO(stevvooe): There are several changes that need to be done to this
 	// interface:
 	//
-	//	1. Get(tag string) should be GetByTag(tag string)
-	//	2. Put(tag string, manifest *manifest.SignedManifest) should be
-	//       Put(manifest *manifest.SignedManifest). The method can read the
-	//       tag on manifest to automatically tag it in the repository.
-	//	3. Need a GetByDigest(dgst digest.Digest) method.
-	//	4. Allow explicit tagging with Tag(digest digest.Digest, tag string)
-	//	5. Support reading tags with a re-entrant reader to avoid large
+	//	1. Allow explicit tagging with Tag(digest digest.Digest, tag string)
+	//	2. Support reading tags with a re-entrant reader to avoid large
 	//       allocations in the registry.
-	//	6. Long-term: Provide All() method that lets one scroll through all of
+	//	3. Long-term: Provide All() method that lets one scroll through all of
 	//       the manifest entries.
-	//	7. Long-term: break out concept of signing from manifests. This is
+	//	4. Long-term: break out concept of signing from manifests. This is
 	//       really a part of the distribution sprint.
-	//	8. Long-term: Manifest should be an interface. This code shouldn't
+	//	5. Long-term: Manifest should be an interface. This code shouldn't
 	//       really be concerned with the storage format.
 }
 

registry/handlers/api_test.go

@@ -219,6 +219,7 @@ func TestLayerAPI(t *testing.T) {
 	checkResponse(t, "checking head on existing layer", resp, http.StatusOK)
 	checkHeaders(t, resp, http.Header{
 		"Content-Length": []string{fmt.Sprint(layerLength)},
+		"Docker-Digest":  []string{layerDigest.String()},
 	})
 
 	// ----------------
@@ -231,6 +232,7 @@ func TestLayerAPI(t *testing.T) {
 	checkResponse(t, "fetching layer", resp, http.StatusOK)
 	checkHeaders(t, resp, http.Header{
 		"Content-Length": []string{fmt.Sprint(layerLength)},
+		"Docker-Digest":  []string{layerDigest.String()},
 	})
 
 	// Verify the body
@@ -286,6 +288,9 @@ func TestManifestAPI(t *testing.T) {
 	// --------------------------------
 	// Attempt to push unsigned manifest with missing layers
 	unsignedManifest := &manifest.Manifest{
+		Versioned: manifest.Versioned{
+			SchemaVersion: 1,
+		},
 		Name: imageName,
 		Tag:  tag,
 		FSLayers: []manifest.FSLayer{
@@ -343,16 +348,43 @@ func TestManifestAPI(t *testing.T) {
 		t.Fatalf("unexpected error signing manifest: %v", err)
 	}
 
+	payload, err := signedManifest.Payload()
+	checkErr(t, err, "getting manifest payload")
+
+	dgst, err := digest.FromBytes(payload)
+	checkErr(t, err, "digesting manifest")
+
+	manifestDigestURL, err := env.builder.BuildManifestURL(imageName, dgst.String())
+	checkErr(t, err, "building manifest url")
+
 	resp = putManifest(t, "putting signed manifest", manifestURL, signedManifest)
 	checkResponse(t, "putting signed manifest", resp, http.StatusAccepted)
+	checkHeaders(t, resp, http.Header{
+		"Location":      []string{manifestDigestURL},
+		"Docker-Digest": []string{dgst.String()},
+	})
 
+	// --------------------
+	// Push by digest -- should get same result
+	resp = putManifest(t, "putting signed manifest", manifestDigestURL, signedManifest)
+	checkResponse(t, "putting signed manifest", resp, http.StatusAccepted)
+	checkHeaders(t, resp, http.Header{
+		"Location":      []string{manifestDigestURL},
+		"Docker-Digest": []string{dgst.String()},
+	})
+
+	// ------------------
+	// Fetch by tag name
 	resp, err = http.Get(manifestURL)
 	if err != nil {
 		t.Fatalf("unexpected error fetching manifest: %v", err)
 	}
 	defer resp.Body.Close()
 
 	checkResponse(t, "fetching uploaded manifest", resp, http.StatusOK)
+	checkHeaders(t, resp, http.Header{
+		"Docker-Digest": []string{dgst.String()},
+	})
 
 	var fetchedManifest manifest.SignedManifest
 	dec := json.NewDecoder(resp.Body)
@@ -364,6 +396,27 @@ func TestManifestAPI(t *testing.T) {
 		t.Fatalf("manifests do not match")
 	}
 
+	// ---------------
+	// Fetch by digest
+	resp, err = http.Get(manifestDigestURL)
+	checkErr(t, err, "fetching manifest by digest")
+	defer resp.Body.Close()
+
+	checkResponse(t, "fetching uploaded manifest", resp, http.StatusOK)
+	checkHeaders(t, resp, http.Header{
+		"Docker-Digest": []string{dgst.String()},
+	})
+
+	var fetchedManifestByDigest manifest.SignedManifest
+	dec = json.NewDecoder(resp.Body)
+	if err := dec.Decode(&fetchedManifestByDigest); err != nil {
+		t.Fatalf("error decoding fetched manifest: %v", err)
+	}
+
+	if !bytes.Equal(fetchedManifestByDigest.Raw, signedManifest.Raw) {
+		t.Fatalf("manifests do not match")
+	}
+
 	// Ensure that the tag is listed.
 	resp, err = http.Get(tagsURL)
 	if err != nil {
@@ -634,3 +687,9 @@ func checkHeaders(t *testing.T, resp *http.Response, headers http.Header) {
 		}
 	}
 }
+
+func checkErr(t *testing.T, err error, msg string) {
+	if err != nil {
+		t.Fatalf("unexpected error %s: %v", msg, err)
+	}
+}

registry/handlers/app.go

@@ -277,9 +277,8 @@ func (app *App) context(w http.ResponseWriter, r *http.Request) *Context {
 	ctx = ctxu.WithLogger(ctx, ctxu.GetRequestLogger(ctx))
 	ctx = ctxu.WithLogger(ctx, ctxu.GetLogger(ctx,
 		"vars.name",
-		"vars.tag",
+		"vars.reference",
 		"vars.digest",
-		"vars.tag",
 		"vars.uuid"))
 
 	context := &Context{

registry/handlers/app_test.go

@@ -84,7 +84,7 @@ func TestAppDispatcher(t *testing.T) {
 			endpoint: v2.RouteNameManifest,
 			vars: []string{
 				"name", "foo/bar",
-				"tag", "sometag",
+				"reference", "sometag",
 			},
 		},
 		{

registry/handlers/context.go

@@ -45,8 +45,8 @@ func getName(ctx context.Context) (name string) {
 	return ctxu.GetStringValue(ctx, "vars.name")
 }
 
-func getTag(ctx context.Context) (tag string) {
-	return ctxu.GetStringValue(ctx, "vars.tag")
+func getReference(ctx context.Context) (reference string) {
+	return ctxu.GetStringValue(ctx, "vars.reference")
 }
 
 var errDigestNotAvailable = fmt.Errorf("digest not available in context")