From 94097512db140a5726bf92cee8b2327e4de320c5 Mon Sep 17 00:00:00 2001 From: Damien Mathieu Date: Wed, 22 May 2019 11:31:25 +0200 Subject: [PATCH] extract blob upload resume into its own method I've found this logic being in a single method to be quite hard to get. I believe extracting it makes it easier to read, as we can then more easily see what the main method does and possibly ignore the intricacies of `ResumeBlobUpload`. Signed-off-by: Damien Mathieu --- registry/handlers/blobupload.go | 101 +++++++++++++++++--------------- 1 file changed, 54 insertions(+), 47 deletions(-) diff --git a/registry/handlers/blobupload.go b/registry/handlers/blobupload.go index 3dc5b75fb..d0351e3e4 100644 --- a/registry/handlers/blobupload.go +++ b/registry/handlers/blobupload.go @@ -9,7 +9,7 @@ import ( dcontext "github.com/docker/distribution/context" "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/api/errcode" - "github.com/docker/distribution/registry/api/v2" + v2 "github.com/docker/distribution/registry/api/v2" "github.com/docker/distribution/registry/storage" "github.com/gorilla/handlers" "github.com/opencontainers/go-digest" @@ -36,52 +36,8 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { } if buh.UUID != "" { - state, err := hmacKey(ctx.Config.HTTP.Secret).unpackUploadState(r.FormValue("_state")) - if err != nil { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - dcontext.GetLogger(ctx).Infof("error resolving upload: %v", err) - buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) - }) - } - buh.State = state - - if state.Name != ctx.Repository.Named().Name() { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - dcontext.GetLogger(ctx).Infof("mismatched repository name in upload state: %q != %q", state.Name, buh.Repository.Named().Name()) - buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) - }) - } - - if state.UUID != buh.UUID { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - dcontext.GetLogger(ctx).Infof("mismatched uuid in upload state: %q != %q", state.UUID, buh.UUID) - buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) - }) - } - - blobs := ctx.Repository.Blobs(buh) - upload, err := blobs.Resume(buh, buh.UUID) - if err != nil { - dcontext.GetLogger(ctx).Errorf("error resolving upload: %v", err) - if err == distribution.ErrBlobUploadUnknown { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadUnknown.WithDetail(err)) - }) - } - - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) - }) - } - buh.Upload = upload - - if size := upload.Size(); size != buh.State.Offset { - defer upload.Close() - dcontext.GetLogger(ctx).Errorf("upload resumed at wrong offset: %d != %d", size, buh.State.Offset) - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) - upload.Cancel(buh) - }) + if h := buh.ResumeBlobUpload(ctx, r); h != nil { + return h } return closeResources(handler, buh.Upload) } @@ -282,6 +238,57 @@ func (buh *blobUploadHandler) CancelBlobUpload(w http.ResponseWriter, r *http.Re w.WriteHeader(http.StatusNoContent) } +func (buh *blobUploadHandler) ResumeBlobUpload(ctx *Context, r *http.Request) http.Handler { + state, err := hmacKey(ctx.Config.HTTP.Secret).unpackUploadState(r.FormValue("_state")) + if err != nil { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + dcontext.GetLogger(ctx).Infof("error resolving upload: %v", err) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) + }) + } + buh.State = state + + if state.Name != ctx.Repository.Named().Name() { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + dcontext.GetLogger(ctx).Infof("mismatched repository name in upload state: %q != %q", state.Name, buh.Repository.Named().Name()) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) + }) + } + + if state.UUID != buh.UUID { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + dcontext.GetLogger(ctx).Infof("mismatched uuid in upload state: %q != %q", state.UUID, buh.UUID) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) + }) + } + + blobs := ctx.Repository.Blobs(buh) + upload, err := blobs.Resume(buh, buh.UUID) + if err != nil { + dcontext.GetLogger(ctx).Errorf("error resolving upload: %v", err) + if err == distribution.ErrBlobUploadUnknown { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadUnknown.WithDetail(err)) + }) + } + + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) + }) + } + buh.Upload = upload + + if size := upload.Size(); size != buh.State.Offset { + defer upload.Close() + dcontext.GetLogger(ctx).Errorf("upload resumed at wrong offset: %d != %d", size, buh.State.Offset) + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) + upload.Cancel(buh) + }) + } + return nil +} + // blobUploadResponse provides a standard request for uploading blobs and // chunk responses. This sets the correct headers but the response status is // left to the caller. The fresh argument is used to ensure that new blob