Merge pull request #3427 from joaodrp/tag-delete

Add tag delete API

docs/spec/api.md

@@ -1113,7 +1113,7 @@ A list of methods and URIs are covered in the table below:
 | GET | `/v2/<name>/tags/list` | Tags | Fetch the tags under the repository identified by `name`. |
 | GET | `/v2/<name>/manifests/<reference>` | Manifest | Fetch the manifest identified by `name` and `reference` where `reference` can be a tag or digest. A `HEAD` request can also be issued to this endpoint to obtain resource information without receiving all data. |
 | PUT | `/v2/<name>/manifests/<reference>` | Manifest | Put the manifest identified by `name` and `reference` where `reference` can be a tag or digest. |
-| DELETE | `/v2/<name>/manifests/<reference>` | Manifest | Delete the manifest identified by `name` and `reference`. Note that a manifest can _only_ be deleted by `digest`. |
+| DELETE | `/v2/<name>/manifests/<reference>` | Manifest | Delete the manifest or tag identified by `name` and `reference` where `reference` can be a tag or digest. Note that a manifest can _only_ be deleted by digest. |
 | GET | `/v2/<name>/blobs/<digest>` | Blob | Retrieve the blob from the registry identified by `digest`. A `HEAD` request can also be issued to this endpoint to obtain resource information without receiving all data. |
 | DELETE | `/v2/<name>/blobs/<digest>` | Blob | Delete the blob identified by `name` and `digest` |
 | POST | `/v2/<name>/blobs/uploads/` | Initiate Blob Upload | Initiate a resumable blob upload. If successful, an upload location will be provided to complete the upload. Optionally, if the `digest` parameter is present, the request body will be used to complete the upload in a single request. |
@@ -2240,7 +2240,7 @@ The error codes that may be included in the response body are enumerated below:
 
 #### DELETE Manifest
 
-Delete the manifest identified by `name` and `reference`. Note that a manifest can _only_ be deleted by `digest`.
+Delete the manifest or tag identified by `name` and `reference` where `reference` can be a tag or digest. Note that a manifest can _only_ be deleted by digest.
 
 
 
@@ -2475,7 +2475,7 @@ Content-Type: application/json
 }
 ```
 
-The specified `name` or `reference` are unknown to the registry and the delete was unable to proceed. Clients can assume the manifest was already deleted if this response is returned.
+The specified `name` or `reference` are unknown to the registry and the delete was unable to proceed. Clients can assume the manifest or tag was already deleted if this response is returned.
 
 
 
@@ -2494,7 +2494,7 @@ The error codes that may be included in the response body are enumerated below:
 405 Method Not Allowed
 ```
 
-Manifest delete is not allowed because the registry is configured as a pull-through cache or `delete` has been disabled.
+Manifest or tag delete is not allowed because the registry is configured as a pull-through cache or `delete` has been disabled.
 
 
 

notifications/listener_test.go

@@ -199,11 +199,20 @@ func checkExerciseRepository(t *testing.T, repository distribution.Repository, r
 		t.Fatalf("mismatching digest from payload and put")
 	}
 
+	if err := repository.Tags(ctx).Tag(ctx, tag, distribution.Descriptor{Digest: dgst}); err != nil {
+		t.Fatalf("unexpected error tagging manifest: %v", err)
+	}
+
 	_, err = manifests.Get(ctx, dgst)
 	if err != nil {
 		t.Fatalf("unexpected error fetching manifest: %v", err)
 	}
 
+	err = repository.Tags(ctx).Untag(ctx, m.Tag)
+	if err != nil {
+		t.Fatalf("unexpected error deleting tag: %v", err)
+	}
+
 	err = manifests.Delete(ctx, dgst)
 	if err != nil {
 		t.Fatalf("unexpected error deleting blob: %v", err)
@@ -216,11 +225,6 @@ func checkExerciseRepository(t *testing.T, repository distribution.Repository, r
 		}
 	}
 
-	err = repository.Tags(ctx).Untag(ctx, m.Tag)
-	if err != nil {
-		t.Fatalf("unexpected error deleting tag: %v", err)
-	}
-
 	err = remover.Remove(ctx, repository.Named())
 	if err != nil {
 		t.Fatalf("unexpected error deleting repo: %v", err)

registry/api/v2/descriptors.go

@@ -655,7 +655,7 @@ var routeDescriptors = []RouteDescriptor{
 			},
 			{
 				Method:      "DELETE",
-				Description: "Delete the manifest identified by `name` and `reference`. Note that a manifest can _only_ be deleted by `digest`.",
+				Description: "Delete the manifest or tag identified by `name` and `reference` where `reference` can be a tag or digest. Note that a manifest can _only_ be deleted by digest.",
 				Requests: []RequestDescriptor{
 					{
 						Headers: []ParameterDescriptor{
@@ -691,7 +691,7 @@ var routeDescriptors = []RouteDescriptor{
 							tooManyRequestsDescriptor,
 							{
 								Name:        "Unknown Manifest",
-								Description: "The specified `name` or `reference` are unknown to the registry and the delete was unable to proceed. Clients can assume the manifest was already deleted if this response is returned.",
+								Description: "The specified `name` or `reference` are unknown to the registry and the delete was unable to proceed. Clients can assume the manifest or tag was already deleted if this response is returned.",
 								StatusCode:  http.StatusNotFound,
 								ErrorCodes: []errcode.ErrorCode{
 									ErrorCodeNameUnknown,
@@ -704,7 +704,7 @@ var routeDescriptors = []RouteDescriptor{
 							},
 							{
 								Name:        "Not allowed",
-								Description: "Manifest delete is not allowed because the registry is configured as a pull-through cache or `delete` has been disabled.",
+								Description: "Manifest or tag delete is not allowed because the registry is configured as a pull-through cache or `delete` has been disabled.",
 								StatusCode:  http.StatusMethodNotAllowed,
 								ErrorCodes: []errcode.ErrorCode{
 									errcode.ErrorCodeUnsupported,

registry/client/repository.go

@@ -364,7 +364,30 @@ func (t *tags) Tag(ctx context.Context, tag string, desc distribution.Descriptor
 }
 
 func (t *tags) Untag(ctx context.Context, tag string) error {
-	panic("not implemented")
+	ref, err := reference.WithTag(t.name, tag)
+	if err != nil {
+		return err
+	}
+	u, err := t.ub.BuildManifestURL(ref)
+	if err != nil {
+		return err
+	}
+
+	req, err := http.NewRequest("DELETE", u, nil)
+	if err != nil {
+		return err
+	}
+
+	resp, err := t.client.Do(req)
+	if err != nil {
+		return err
+	}
+	defer resp.Body.Close()
+
+	if SuccessStatus(resp.StatusCode) {
+		return nil
+	}
+	return HandleErrorResponse(resp)
 }
 
 type manifests struct {

registry/client/repository_test.go

@@ -1425,6 +1425,43 @@ func TestManifestTags(t *testing.T) {
 	// TODO(dmcgowan): Check for error cases
 }
 
+func TestTagDelete(t *testing.T) {
+	tag := "latest"
+	repo, _ := reference.WithName("test.example.com/repo/delete")
+	newRandomSchemaV1Manifest(repo, tag, 1)
+
+	var m testutil.RequestResponseMap
+	m = append(m, testutil.RequestResponseMapping{
+		Request: testutil.Request{
+			Method: "DELETE",
+			Route:  "/v2/" + repo.Name() + "/manifests/" + tag,
+		},
+		Response: testutil.Response{
+			StatusCode: http.StatusAccepted,
+			Headers: map[string][]string{
+				"Content-Length": {"0"},
+			},
+		},
+	})
+
+	e, c := testServer(m)
+	defer c()
+
+	r, err := NewRepository(repo, e, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ctx := context.Background()
+	ts := r.Tags(ctx)
+
+	if err := ts.Untag(ctx, tag); err != nil {
+		t.Fatal(err)
+	}
+	if err := ts.Untag(ctx, tag); err == nil {
+		t.Fatal("expected error deleting unknown tag")
+	}
+}
+
 func TestObtainsErrorForMissingTag(t *testing.T) {
 	repo, _ := reference.WithName("test.example.com/repo")
 

registry/handlers/api_test.go

@@ -1000,6 +1000,93 @@ func TestManifestAPI(t *testing.T) {
 	testManifestAPIManifestList(t, env2, schema2Args)
 }
 
+func TestManifestAPI_DeleteTag(t *testing.T) {
+	env := newTestEnv(t, false)
+	defer env.Shutdown()
+
+	imageName, err := reference.WithName("foo/bar")
+	checkErr(t, err, "building image name")
+
+	tag := "latest"
+	dgst := createRepository(env, t, imageName.Name(), tag)
+
+	ref, err := reference.WithTag(imageName, tag)
+	checkErr(t, err, "building tag reference")
+
+	u, err := env.builder.BuildManifestURL(ref)
+	checkErr(t, err, "building tag URL")
+
+	resp, err := httpDelete(u)
+	m := "deleting tag"
+	checkErr(t, err, m)
+	defer resp.Body.Close()
+
+	checkResponse(t, m, resp, http.StatusAccepted)
+	if resp.Body != http.NoBody {
+		t.Fatal("unexpected response body")
+	}
+
+	msg := "checking tag no longer exists"
+	resp, err = http.Get(u)
+	checkErr(t, err, msg)
+	checkResponse(t, msg, resp, http.StatusNotFound)
+
+	digestRef, err := reference.WithDigest(imageName, dgst)
+	checkErr(t, err, "building manifest digest reference")
+
+	u, err = env.builder.BuildManifestURL(digestRef)
+	checkErr(t, err, "building manifest URL")
+
+	msg = "checking manifest still exists"
+	resp, err = http.Head(u)
+	checkErr(t, err, msg)
+	checkResponse(t, msg, resp, http.StatusOK)
+}
+
+func TestManifestAPI_DeleteTag_Unknown(t *testing.T) {
+	env := newTestEnv(t, false)
+	defer env.Shutdown()
+
+	imageName, err := reference.WithName("foo/bar")
+	checkErr(t, err, "building named object")
+
+	ref, err := reference.WithTag(imageName, "latest")
+	checkErr(t, err, "building tag reference")
+
+	u, err := env.builder.BuildManifestURL(ref)
+	checkErr(t, err, "building tag URL")
+
+	resp, err := httpDelete(u)
+	msg := "deleting unknown tag"
+	checkErr(t, err, msg)
+	defer resp.Body.Close()
+
+	checkResponse(t, msg, resp, http.StatusNotFound)
+	checkBodyHasErrorCodes(t, msg, resp, v2.ErrorCodeManifestUnknown)
+}
+
+func TestManifestAPI_DeleteTag_ReadOnly(t *testing.T) {
+	env := newTestEnv(t, false)
+	defer env.Shutdown()
+	env.app.readOnly = true
+
+	imageName, err := reference.WithName("foo/bar")
+	checkErr(t, err, "building named object")
+
+	ref, err := reference.WithTag(imageName, "latest")
+	checkErr(t, err, "building tag reference")
+
+	u, err := env.builder.BuildManifestURL(ref)
+	checkErr(t, err, "building URL")
+
+	resp, err := httpDelete(u)
+	msg := "deleting tag"
+	checkErr(t, err, msg)
+	defer resp.Body.Close()
+
+	checkResponse(t, msg, resp, http.StatusMethodNotAllowed)
+}
+
 // storageManifestErrDriverFactory implements the factory.StorageDriverFactory interface.
 type storageManifestErrDriverFactory struct{}
 

registry/handlers/manifests.go

@@ -17,6 +17,7 @@ import (
 	"github.com/distribution/distribution/v3/registry/api/errcode"
 	v2 "github.com/distribution/distribution/v3/registry/api/v2"
 	"github.com/distribution/distribution/v3/registry/auth"
+	"github.com/distribution/distribution/v3/registry/storage/driver"
 	"github.com/gorilla/handlers"
 	"github.com/opencontainers/go-digest"
 	v1 "github.com/opencontainers/image-spec/specs-go/v1"
@@ -481,15 +482,31 @@ func (imh *manifestHandler) applyResourcePolicy(manifest distribution.Manifest)
 
 }
 
-// DeleteManifest removes the manifest with the given digest from the registry.
+// DeleteManifest removes the manifest with the given digest or the tag with the given name from the registry.
 func (imh *manifestHandler) DeleteManifest(w http.ResponseWriter, r *http.Request) {
 	dcontext.GetLogger(imh).Debug("DeleteImageManifest")
 
-	if imh.Tag != "" {
+	if imh.App.isCache {
 		imh.Errors = append(imh.Errors, errcode.ErrorCodeUnsupported)
 		return
 	}
 
+	if imh.Tag != "" {
+		dcontext.GetLogger(imh).Debug("DeleteImageTag")
+		tagService := imh.Repository.Tags(imh.Context)
+		if err := tagService.Untag(imh.Context, imh.Tag); err != nil {
+			switch err.(type) {
+			case distribution.ErrTagUnknown, driver.PathNotFoundError:
+				imh.Errors = append(imh.Errors, v2.ErrorCodeManifestUnknown.WithDetail(err))
+			default:
+				imh.Errors = append(imh.Errors, errcode.ErrorCodeUnknown.WithDetail(err))
+			}
+			return
+		}
+		w.WriteHeader(http.StatusAccepted)
+		return
+	}
+
 	manifests, err := imh.Repository.Manifests(imh)
 	if err != nil {
 		imh.Errors = append(imh.Errors, err)

registry/storage/tagstore.go

@@ -112,16 +112,7 @@ func (ts *tagStore) Untag(ctx context.Context, tag string) error {
 		return err
 	}
 
-	if err := ts.blobStore.driver.Delete(ctx, tagPath); err != nil {
+	return ts.blobStore.driver.Delete(ctx, tagPath)
-		switch err.(type) {
-		case storagedriver.PathNotFoundError:
-			return nil // Untag is idempotent, we don't care if it didn't exist
-		default:
-			return err
-		}
-	}
-
-	return nil
 }
 
 // linkedBlobStore returns the linkedBlobStore for the named tag, allowing one

registry/storage/tagstore_test.go

@@ -98,8 +98,8 @@ func TestTagStoreUnTag(t *testing.T) {
 	desc := distribution.Descriptor{Digest: "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"}
 
 	err := tags.Untag(ctx, "latest")
-	if err != nil {
+	if err == nil {
-		t.Error(err)
+		t.Error("expected error removing unknown tag")
 	}
 
 	err = tags.Tag(ctx, "latest", desc)