Merge pull request #1319 from RichardScothern/update-tags

Remove tags referencing deleted manifests.

docs/handlers/api_test.go

@@ -1623,6 +1623,7 @@ func testManifestDelete(t *testing.T, env *testEnv, args manifestArgs) {
 	dgst := args.dgst
 	manifest := args.manifest
 	manifestDigestURL, err := env.builder.BuildManifestURL(imageName, dgst.String())
+
 	// ---------------
 	// Delete by digest
 	resp, err := httpDelete(manifestDigestURL)
@@ -1678,6 +1679,77 @@ func testManifestDelete(t *testing.T, env *testEnv, args manifestArgs) {
 	checkErr(t, err, "delting unknown manifest by digest")
 	checkResponse(t, "fetching deleted manifest", resp, http.StatusNotFound)
 
+	// --------------------
+	// Uupload manifest by tag
+	tag := signedManifest.Tag
+	manifestTagURL, err := env.builder.BuildManifestURL(imageName, tag)
+	resp = putManifest(t, "putting signed manifest by tag", manifestTagURL, signedManifest)
+	checkResponse(t, "putting signed manifest by tag", resp, http.StatusCreated)
+	checkHeaders(t, resp, http.Header{
+		"Location":              []string{manifestDigestURL},
+		"Docker-Content-Digest": []string{dgst.String()},
+	})
+
+	tagsURL, err := env.builder.BuildTagsURL(imageName)
+	if err != nil {
+		t.Fatalf("unexpected error building tags url: %v", err)
+	}
+
+	// Ensure that the tag is listed.
+	resp, err = http.Get(tagsURL)
+	if err != nil {
+		t.Fatalf("unexpected error getting unknown tags: %v", err)
+	}
+	defer resp.Body.Close()
+
+	dec := json.NewDecoder(resp.Body)
+	var tagsResponse tagsAPIResponse
+	if err := dec.Decode(&tagsResponse); err != nil {
+		t.Fatalf("unexpected error decoding error response: %v", err)
+	}
+
+	if tagsResponse.Name != imageName {
+		t.Fatalf("tags name should match image name: %v != %v", tagsResponse.Name, imageName)
+	}
+
+	if len(tagsResponse.Tags) != 1 {
+		t.Fatalf("expected some tags in response: %v", tagsResponse.Tags)
+	}
+
+	if tagsResponse.Tags[0] != tag {
+		t.Fatalf("tag not as expected: %q != %q", tagsResponse.Tags[0], tag)
+	}
+
+	// ---------------
+	// Delete by digest
+	resp, err = httpDelete(manifestDigestURL)
+	checkErr(t, err, "deleting manifest by digest")
+
+	checkResponse(t, "deleting manifest with tag", resp, http.StatusAccepted)
+	checkHeaders(t, resp, http.Header{
+		"Content-Length": []string{"0"},
+	})
+
+	// Ensure that the tag is not listed.
+	resp, err = http.Get(tagsURL)
+	if err != nil {
+		t.Fatalf("unexpected error getting unknown tags: %v", err)
+	}
+	defer resp.Body.Close()
+
+	dec = json.NewDecoder(resp.Body)
+	if err := dec.Decode(&tagsResponse); err != nil {
+		t.Fatalf("unexpected error decoding error response: %v", err)
+	}
+
+	if tagsResponse.Name != imageName {
+		t.Fatalf("tags name should match image name: %v != %v", tagsResponse.Name, imageName)
+	}
+
+	if len(tagsResponse.Tags) != 0 {
+		t.Fatalf("expected 0 tags in response: %v", tagsResponse.Tags)
+	}
+
 }
 
 type testEnv struct {

docs/handlers/images.go

@@ -320,5 +320,19 @@ func (imh *imageManifestHandler) DeleteImageManifest(w http.ResponseWriter, r *h
 		}
 	}
 
+	tagService := imh.Repository.Tags(imh)
+	referencedTags, err := tagService.Lookup(imh, distribution.Descriptor{Digest: imh.Digest})
+	if err != nil {
+		imh.Errors = append(imh.Errors, err)
+		return
+	}
+
+	for _, tag := range referencedTags {
+		if err := tagService.Untag(imh, tag); err != nil {
+			imh.Errors = append(imh.Errors, err)
+			return
+		}
+	}
+
 	w.WriteHeader(http.StatusAccepted)
 }

docs/storage/tagstore.go

@@ -116,15 +116,19 @@ func (ts *tagStore) Get(ctx context.Context, tag string) (distribution.Descripto
 	return distribution.Descriptor{Digest: revision}, nil
 }
 
-// delete removes the tag from repository, including the history of all
+// Untag removes the tag association
-// revisions that have the specified tag.
 func (ts *tagStore) Untag(ctx context.Context, tag string) error {
 	tagPath, err := pathFor(manifestTagPathSpec{
 		name: ts.repository.Name(),
 		tag:  tag,
 	})
 
-	if err != nil {
+	switch err.(type) {
+	case storagedriver.PathNotFoundError:
+		return distribution.ErrTagUnknown{Tag: tag}
+	case nil:
+		break
+	default:
 		return err
 	}
 
@@ -153,7 +157,35 @@ func (ts *tagStore) linkedBlobStore(ctx context.Context, tag string) *linkedBlob
 
 // Lookup recovers a list of tags which refer to this digest.  When a manifest is deleted by
 // digest, tag entries which point to it need to be recovered to avoid dangling tags.
-func (ts *tagStore) Lookup(ctx context.Context, digest distribution.Descriptor) ([]string, error) {
+func (ts *tagStore) Lookup(ctx context.Context, desc distribution.Descriptor) ([]string, error) {
-	// An efficient implementation of this will require changes to the S3 driver.
+	allTags, err := ts.All(ctx)
-	return make([]string, 0), nil
+	switch err.(type) {
+	case distribution.ErrRepositoryUnknown:
+		// This tag store has been initialized but not yet populated
+		break
+	case nil:
+		break
+	default:
+		return nil, err
+	}
+
+	var tags []string
+	for _, tag := range allTags {
+		tagLinkPathSpec := manifestTagCurrentPathSpec{
+			name: ts.repository.Name(),
+			tag:  tag,
+		}
+
+		tagLinkPath, err := pathFor(tagLinkPathSpec)
+		tagDigest, err := ts.blobStore.readlink(ctx, tagLinkPath)
+		if err != nil {
+			return nil, err
+		}
+
+		if tagDigest == desc.Digest {
+			tags = append(tags, tag)
+		}
+	}
+
+	return tags, nil
 }

docs/storage/tagstore_test.go

@@ -102,7 +102,7 @@ func TestTagStoreUnTag(t *testing.T) {
 	}
 }
 
-func TestTagAll(t *testing.T) {
+func TestTagStoreAll(t *testing.T) {
 	env := testTagStore(t)
 	tagStore := env.ts
 	ctx := env.ctx
@@ -148,3 +148,59 @@ func TestTagAll(t *testing.T) {
 	}
 
 }
+
+func TestTagLookup(t *testing.T) {
+	env := testTagStore(t)
+	tagStore := env.ts
+	ctx := env.ctx
+
+	descA := distribution.Descriptor{Digest: "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}
+	desc0 := distribution.Descriptor{Digest: "sha256:0000000000000000000000000000000000000000000000000000000000000000"}
+
+	tags, err := tagStore.Lookup(ctx, descA)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(tags) != 0 {
+		t.Fatalf("Lookup returned > 0 tags from empty store")
+	}
+
+	err = tagStore.Tag(ctx, "a", descA)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = tagStore.Tag(ctx, "b", descA)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = tagStore.Tag(ctx, "0", desc0)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = tagStore.Tag(ctx, "1", desc0)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tags, err = tagStore.Lookup(ctx, descA)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(tags) != 2 {
+		t.Errorf("Lookup of descA returned %d tags, expected 2", len(tags))
+	}
+
+	tags, err = tagStore.Lookup(ctx, desc0)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(tags) != 2 {
+		t.Errorf("Lookup of descB returned %d tags, expected 2", len(tags))
+	}
+
+}