diff --git a/docs/spec/api.md b/docs/spec/api.md index 1f7a4a7b..f3d97d94 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//tags/list` | Tags | Fetch the tags under the repository identified by `name`. | | GET | `/v2//manifests/` | 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//manifests/` | Manifest | Put the manifest identified by `name` and `reference` where `reference` can be a tag or digest. | -| DELETE | `/v2//manifests/` | Manifest | Delete the manifest identified by `name` and `reference`. Note that a manifest can _only_ be deleted by `digest`. | +| DELETE | `/v2//manifests/` | 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//blobs/` | 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//blobs/` | Blob | Delete the blob identified by `name` and `digest` | | POST | `/v2//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. diff --git a/notifications/listener_test.go b/notifications/listener_test.go index 74439668..e34707d7 100644 --- a/notifications/listener_test.go +++ b/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) 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 9255629f..4fc2e076 100644 --- a/registry/handlers/api_test.go +++ b/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{} diff --git a/registry/handlers/manifests.go b/registry/handlers/manifests.go index 03209754..5af59aaf 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 != "" { + 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) 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)