From 6ae6df7d75aec4f40a8d22bc1d2ff4b0908bc9c8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Pereira?=
<484633+joaodrp@users.noreply.github.com>
Date: Thu, 27 May 2021 23:13:59 +0100
Subject: [PATCH] Add tag delete API
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: João Pereira <484633+joaodrp@users.noreply.github.com>
---
docs/spec/api.md | 11 ++--
registry/api/v2/descriptors.go | 6 +--
registry/client/repository.go | 25 ++++++++-
registry/client/repository_test.go | 37 +++++++++++++
registry/handlers/api_test.go | 87 ++++++++++++++++++++++++++++++
registry/handlers/manifests.go | 21 +++++++-
registry/storage/tagstore.go | 11 +---
registry/storage/tagstore_test.go | 4 +-
8 files changed, 179 insertions(+), 23 deletions(-)
diff --git a/docs/spec/api.md b/docs/spec/api.md
index 1f7a4a7b..38689c4b 100644
--- a/docs/spec/api.md
+++ b/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. |
@@ -1142,7 +1142,8 @@ The error codes encountered via the API are enumerated in the following table:
`MANIFEST_UNVERIFIED` | manifest failed signature verification | During manifest upload, if the manifest fails signature verification, this error will be returned.
`NAME_INVALID` | invalid repository name | Invalid repository name encountered either during manifest validation or any API operation.
`NAME_UNKNOWN` | repository name not known to registry | This is returned if the name used during an operation is unknown to the registry.
- `PAGINATION_NUMBER_INVALID` | invalid number of results requested | Returned when the `n` parameter (number of results to return) is not an integer, or `n` is negative.
+ `PAGINATION_NUMBER_INVALID` | invalid number of results requested | Returned when the "n" parameter (number of results to return) is not an integer, or "n" is negative.
+ `RANGE_INVALID` | invalid content range | When a layer is uploaded, the provided range is checked against the uploaded chunk. This error is returned if the range is out of order.
`SIZE_INVALID` | provided length did not match content length | When a layer is uploaded, the provided size will be checked against the uploaded content. If they do not match, this error will be returned.
`TAG_INVALID` | manifest tag did not match URI | During a manifest upload, if the tag in the manifest does not match the uri tag, this error will be returned.
`UNAUTHORIZED` | authentication required | The access controller was unable to authenticate the client. Often this will be accompanied by a Www-Authenticate HTTP response header indicating how to authenticate.
@@ -2240,7 +2241,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 +2476,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 +2495,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.
diff --git a/registry/api/v2/descriptors.go b/registry/api/v2/descriptors.go
index 1dbe6823..b1090fc9 100644
--- a/registry/api/v2/descriptors.go
+++ b/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,
diff --git a/registry/client/repository.go b/registry/client/repository.go
index 00b31a48..a3379c0a 100644
--- a/registry/client/repository.go
+++ b/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 {
diff --git a/registry/client/repository_test.go b/registry/client/repository_test.go
index bd08bbd9..2e8e978a 100644
--- a/registry/client/repository_test.go
+++ b/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")
diff --git a/registry/handlers/api_test.go b/registry/handlers/api_test.go
index e0c30b36..695a87b7 100644
--- a/registry/handlers/api_test.go
+++ b/registry/handlers/api_test.go
@@ -844,6 +844,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{}
diff --git a/registry/handlers/manifests.go b/registry/handlers/manifests.go
index 03209754..5a208cdd 100644
--- a/registry/handlers/manifests.go
+++ b/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 != "" {
+ tagService := imh.Repository.Tags(imh.Context)
+ if err := tagService.Untag(imh.Context, imh.Tag); err != nil {
+ switch err.(type) {
+ case distribution.ErrTagUnknown:
+ case driver.PathNotFoundError:
+ imh.Errors = append(imh.Errors, v2.ErrorCodeManifestUnknown)
+ default:
+ imh.Errors = append(imh.Errors, errcode.ErrorCodeUnknown)
+ }
+ return
+ }
+ w.WriteHeader(http.StatusAccepted)
+ return
+ }
+
manifests, err := imh.Repository.Manifests(imh)
if err != nil {
imh.Errors = append(imh.Errors, err)
diff --git a/registry/storage/tagstore.go b/registry/storage/tagstore.go
index 7bbb2f01..9c16330b 100644
--- a/registry/storage/tagstore.go
+++ b/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 {
- 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
+ return ts.blobStore.driver.Delete(ctx, tagPath)
}
// linkedBlobStore returns the linkedBlobStore for the named tag, allowing one
diff --git a/registry/storage/tagstore_test.go b/registry/storage/tagstore_test.go
index dca486fd..c17137d3 100644
--- a/registry/storage/tagstore_test.go
+++ b/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 {
- t.Error(err)
+ if err == nil {
+ t.Error("expected error removing unknown tag")
}
err = tags.Tag(ctx, "latest", desc)